aws_elasticache_replication_group: Implement noop MigrateState for v0..v1

[akatrevorjay]

Community Note

• Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
• Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Fixes #8828

Release note for CHANGELOG:

aws_elasticache_replication_group will now migrate v0 resources to v1 in state.

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'
==> Checking that code complies with gofmt requirements...
gofmt needs running on the following files:
./aws/config.go
./aws/data_source_aws_cloudformation_export_test.go
./aws/opsworks_layers.go
./aws/provider.go
./aws/resource_aws_api_gateway_integration.go
./aws/resource_aws_appmesh_virtual_node_migrate_test.go
./aws/resource_aws_cloudfront_origin_access_identity.go
./aws/resource_aws_codedeploy_deployment_group_test.go
./aws/resource_aws_db_instance.go
./aws/resource_aws_db_security_group.go
./aws/resource_aws_dms_replication_instance.go
./aws/resource_aws_docdb_cluster_instance.go
./aws/resource_aws_docdb_cluster_parameter_group.go
./aws/resource_aws_ec2_fleet.go
./aws/resource_aws_emr_security_configuration.go
./aws/resource_aws_gamelift_fleet.go
./aws/resource_aws_glue_security_configuration.go
./aws/resource_aws_guardduty_detector.go
./aws/resource_aws_instance.go
./aws/resource_aws_instance_migrate_test.go
./aws/resource_aws_kinesis_analytics_application.go
./aws/resource_aws_lb_listener.go
./aws/resource_aws_lb_listener_rule.go
./aws/resource_aws_licensemanager_license_configuration.go
./aws/resource_aws_neptune_cluster_instance.go
./aws/resource_aws_neptune_cluster_parameter_group.go
./aws/resource_aws_organizations_test.go
./aws/resource_aws_ram_resource_share.go
./aws/resource_aws_rds_cluster_instance.go
./aws/resource_aws_rds_cluster_parameter_group.go
./aws/resource_aws_redshift_cluster.go
./aws/resource_aws_route53_record.go
./aws/resource_aws_s3_bucket.go
./aws/resource_aws_s3_bucket_inventory.go
./aws/resource_aws_s3_bucket_notification.go
./aws/resource_aws_security_group_migrate_test.go
./aws/resource_aws_sns_topic.go
./aws/resource_aws_spot_fleet_request.go
./aws/resource_aws_spot_instance_request.go
./aws/resource_aws_ssm_maintenance_window.go
./aws/resource_aws_ssm_patch_baseline.go
./aws/resource_aws_swf_domain.go
./aws/resource_aws_vpc.go
./aws/validators_test.go
./aws/resource_aws_acmpca_certificate_authority.go
You can use the command: `make fmt` to reformat code.
make: *** [GNUmakefile:28: fmtcheck] Error 1
...

[akatrevorjay]

Hey so, I /think/ that this is fine as a no-op (from TypeSet to TypeList), but please let me know if it's not and I'll update it.

[bflad]

Hey @akatrevorjay 👋 Thanks so much for reporting the original issue and submitting this pull request! This is definitely something worth fixing up in the resource. Other than simplifying this a little bit, this seems like the right implementation here!

[bflad]

Since this is an exceptional case and we want to perform the simplest possible upgrade for compatibility, can you please inline this with the following comments?

Suggested change

	MigrateState: resourceAwsElasticacheReplicationGroupMigrateState,
	// SchemaVersion: 1 did not include any state changes via MigrateState.
	// Perform a no-operation state upgrade for Terraform 0.12 compatibility.
	// Future state migrations should be performed with StateUpgraders.
	MigrateState: func(v int, inst *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) {
	return inst, nil
	},

I verified this successfully with a Terraform AWS Provider 1.13.0 aws_elasticache_replication_group resource on Terraform 0.12.1. 👍

[akatrevorjay]

Sure, I'll swap over. I do find the replacement more error prone, but either way is fine by me.

[bflad]

Hi @akatrevorjay, the passthrough implementation is not more error prone -- MigrateState and dealing with flatmap state migrations in of the Terraform v0.12's Provider SDK has been superseded with JSON state migrations via StateUpgraders. In this case, we do not need to worry about doing anything other than passing through the existing InstanceState to bypass the Terraform runtime error, which matches the Terraform v0.11 Provider SDK in this situation where MigrateState was not implemented alongside a SchemaVersion update. We'll be removing the upstream runtime error in the next version of Terraform v0.12 (hashicorp/terraform#21625) to match the old Terraform v0.11 behavior.

[akatrevorjay]

Aha! I didn't see any mention of StateUpgraders in the docs at the time of writing this, I'm guessing I must have missed it! Thanks so much for the explanation!

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!