resource/aws_elasticache_replication_group: Always set "primary_endpoint_address" and "configuration_endpoint_address"

[legal90]

Fixes #6869

Changes proposed in this pull request:

This changeset ensures that primary_endpoint_address and configuration_endpoint_address computed attribute will always be defined, so the resource interface becomes consistent regardless of the "cluster_mode" value.

If cluster_mode is turned on, then configuration_endpoint_address is set to a real value and primary_endpoint_address is an empty string. And vise versa: if cluster_mode is not enabled, then primary_endpoint_address is real value and configuration_endpoint_address is an empty string.

That allows to avoid issues like #6869. For example, it will be possible to call the aws_elasticache_replication_groupresource from the module and use the output for endpoint address. And it will work for both states of cluster_mode feature without throwing an error:

output "configuration_endpoint_address" {
  value = "${join("",aws_elasticache_replication_group.default.*.configuration_endpoint_address)}"
}

output "primary_endpoint_address" {
  value = "${join("",aws_elasticache_replication_group.default.*.primary_endpoint_address)}"
}

Output from acceptance testing:

$ make testacc TESTARGS='-run="TestAccAWSElasticacheReplicationGroup_ClusterMode_Basic|TestAccAWSElasticacheReplicationGroup_redisClusterInVpc2"'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -parallel 20 -run="TestAccAWSElasticacheReplicationGroup_ClusterMode_Basic|TestAccAWSElasticacheReplicationGroup_redisClusterInVpc2" -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSElasticacheReplicationGroup_redisClusterInVpc2
=== PAUSE TestAccAWSElasticacheReplicationGroup_redisClusterInVpc2
=== RUN   TestAccAWSElasticacheReplicationGroup_ClusterMode_Basic
=== PAUSE TestAccAWSElasticacheReplicationGroup_ClusterMode_Basic
=== CONT  TestAccAWSElasticacheReplicationGroup_redisClusterInVpc2
=== CONT  TestAccAWSElasticacheReplicationGroup_ClusterMode_Basic
--- PASS: TestAccAWSElasticacheReplicationGroup_redisClusterInVpc2 (847.94s)
--- PASS: TestAccAWSElasticacheReplicationGroup_ClusterMode_Basic (1055.40s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1055.446s
...

[legal90]

@bflad Could you please have a look on this PR & the issue linked?

[legal90]

@bflad Could you please review this PR? I'm sorry for bothering, but we really depend on this functionality.

[netproteus]

+1 the existing behaviour prevents sharing config between environments where you want a single instance in dev and cluster in production

[saurabh-hirani]

Thanks for fixing this @legal90 - we are also facing the same issue as a script depends on reading Terraform output to get the primary endpoint address.

It would really help to merge this as it would solve terraform output as well as remote state reference problems.

Update: As per https://www.terraform.io/docs/providers/aws/r/elasticache_replication_group.html - primary_endpoint_address is available for cluster_mode_disabled. I was trying to get it for cluster_mode_enabled - for which the key is configuration_endpoint_address which is available.

[legal90]

I rebased the branch and fixed the merge conflict in test file. @bflad could you please review this PR?

[tburow]

Is this going to get fixed anytime soon? this is a really old problem that is still an active issue in the latest v2 provider code.

[zhelding]

Pull request #21306 has significantly refactored the AWS Provider codebase. As a result, most PRs opened prior to the refactor now have merge conflicts that must be resolved before proceeding.

Specifically, PR #21306 relocated the code for all AWS resources and data sources from a single aws directory to a large number of separate directories in internal/service, each corresponding to a particular AWS service. This separation of code has also allowed for us to simplify the names of underlying functions -- while still avoiding namespace collisions.

We recognize that many pull requests have been open for some time without yet being addressed by our maintainers. Therefore, we want to make it clear that resolving these conflicts in no way affects the prioritization of a particular pull request. Once a pull request has been prioritized for review, the necessary changes will be made by a maintainer -- either directly or in collaboration with the pull request author.

For a more complete description of this refactor, including examples of how old filepaths and function names correspond to their new counterparts: please refer to issue #20000.

For a quick guide on how to amend your pull request to resolve the merge conflicts resulting from this refactor and bring it in line with our new code patterns: please refer to our Service Package Refactor Pull Request Guide.

[legal90]

Thank you @zhelding ! I have rebased this PR according to the latest changes in main. There are no conflicts anymore

[legal90]

Hi dear provider maintainers! Now, after the long time waiting (3.5 years), it makes less sense to still keep this PR open. The original issue addressed in the top message can now be worked around using try() function in terraform v0.12.20 and higher.

Should I close this PR?

[legal90]

I'm closing this since there is no response from provider maintainers for a very long time.

[github-actions[bot]]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.