Tech debt: Consistently handle overlapping resource tags when updating

[ewbankkit]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

Prompted by #6911 (comment) which references #5108.
Ensure that we are consistently handling overlapping resource tags when updating a resource.
Because each AWS service has its own types and methods for resource tagging, code has been copy-pasted and is now inconsistent.

[ewbankkit]

This brings up the bigger issue of making tag handling functions generic via Go reflection.

[bflad]

I really don't want to start any sort of flame war on the subject, but I think a few of the maintainers are strongly opposed to reflection. 😅 Type assertion (via creating our own generic type, call it something like keyValueResourceTag, that can be implemented easily for each of the AWS Go SDK service types, and paring down our functions to just those operations with our type) is more likely to be accepted.

[ewbankkit]

I wasn't expressing any preference or opinion 😄.
For this issue I'd like to identify and fix the services which are inconsistent WRT overlapping tags. This will give us a known good state from which to explore a generic solution.

[ewbankkit]

Service	Source File	Correct
Amazon API Gateway	tags_apigateway.go diffTagsGeneric()	❌
AWS App Mesh	tagsAppmesh.go #8111	❌
Amazon Athena	tagsAthena.go #8136	❌
AWS Auto Scaling	autoscaling_tags.go	❌
AWS Certificate Manager (ACM)	tagsACM.go	❌
AWS Certificate Manager (ACM) Private Certificate Authority (CA)	tagsACMPCA.go	❌
Amazon CloudFront	tagsCloudFront.go	❌
AWS CloudHSM v2	resource_aws_cloudhsm2_cluster.go diffTagsGeneric()	❌
Amazon CloudWatch Events	tagsCloudWatchEvent..go #8076	❌
Amazon CloudWatch Metrics	tagsCloudWatch.go #8168	❌
AWS CloudTrail	tagsCloudtrail.go	❌
AWS Direct Connect	tagsDX.go	❌
AWS Directory Service	tagsDS.go	❌
Amazon DocumentDB	tagsDocDB.go	✅
Amazon DynamoDB	tags.go diffTagsDynamoDb()	❌
Amazon DynamoDB Accelerator (DAX)	tagsDAX.go	❌
Amazon EC2	tags.go diffTags()	✅
AWS Elastic Beanstalk	tagsBeanstalk.go	❌
Amazon ElastiCache	tagsEC.go	❌
Amazon Elastic Container Registry (ECR)	tagsECR.go	✅
Amazon Elastic Container Service (ECS)	tagsECS.go	✅
Amazon Elastic File System (EFS)	tagsEFS.go	❌
AWS Elastic Load Balancing (ELB)	tagsELB.go	❌
AWS Elastic Load Balancing (ELB) v2	tags.go diffElbV2Tags()	❌
Amazon Elasticsearch Service	tags_elasticsearchservice.go	❌
AWS Elemental MediaPackage	tagsMediapackage.go diffTagsGeneric() #7984	❌
AWS Identity and Access Management (IAM)	tagsIAM.go	✅
AWS Key Management Service (KMS)	tagsKMS.go	❌
Amazon Kinesis	tags_kinesis.go	❌
Amazon Kinesis Data Firehose	tagsKinesisFirehose.go	❌
AWS Lambda	tagsLambda.go diffTagsGeneric()	❌
AWS License Manager	tagsLicenseManager.go	❌
Amazon MQ	diffTagsGeneric() #7193	❌
Amazon Neptune	tagsNeptune.go	❌
AWS OpsWorks	tagsOpsworks.go diffTagsGeneric()	❌
Amazon Relational Database Service (RDS)	tagsRDS.go	❌
Amazon Redshift	tagsRedshift.go	❌
AWS Resource Access Manager (RAM)	tagsRAM.go #6528	✅
Amazon Route 53	tags_route53.go	❌
Amazon Route 53 Resolver	tagsRoute53Resolver.go #6574	✅
Amazon S3	s3_tags.go	❌
AWS Secrets Manager	tagsSecretsManager.go	❌
AWS SNS	tagsSNS.go #8468	✅
Amazon Simple Queue Service (SQS)	resource_aws_sqs_queue.go diffTagsGeneric()	❌
AWS Step Functions	tagsSfn.go	✅
AWS Systems Manager	tagsSSM.go	❌
AWS Transfer for SFTP	tagsTransfer.go	✅

[ewbankkit]

Note that some of these diffTags functions may not be needed as the underlying AWS API doesn't deal with tag set differences but instead just deletes all tags and applies a tag set. e.g. for S3: https://github.com/terraform-providers/terraform-provider-aws/blob/9a2ba295b2f1ccbb54fc2c62a31312962f362da1/aws/s3_tags.go#L25-L28 and https://github.com/terraform-providers/terraform-provider-aws/blob/9a2ba295b2f1ccbb54fc2c62a31312962f362da1/aws/s3_tags.go#L36-L45.

[ewbankkit]

We need to also consider the issues brought up in #7323, #7342 for those APIs that don't deal with tag set differences but may have system tags.

[ewbankkit]

I will close this issue when #10018 is merged as the functionality that PR implements will fix this for all existing services and new services in the future.

[ewbankkit]

I'm closing this issue in favor of #10688, implementation of which will ensure overlapping tags are handled correctly.

[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!