-
Notifications
You must be signed in to change notification settings - Fork 9.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
provider: Support arbitrary additional tag data, implement keyvaluetags and ignore_tags support in aws_autoscaling_group resource, return empty strings with key-only tags instead of panicking #13868
Conversation
91f02a9
to
a22c22b
Compare
Rebased and re-verified:
|
a22c22b
to
d4a6d1e
Compare
Rebased and re-verified:
|
@@ -133,25 +134,27 @@ func isResourceTimeoutError(err error) bool { | |||
// {{ . | Title }}CreateTags creates {{ . }} service tags for new resources. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isAWSErrCode()
and isAWSErrCodeContains()
above can also be removed now and replaced by functions in github.com/hashicorp/aws-sdk-go-base/tfawserr
.
I have a similar internal package github.com/terraform-providers/terraform-provider-aws/aws/internal/resourceerr
in #13036.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice yeah. Thanks for the reminder on that -- could you ensure there's a covering GitHub issue for this. I'll save that general refactoring for after this change, since this is already too big. 😭
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry, now reading #13036 I see this is already a part of that. We'll take a look after this gets in.
Is there an opportunity to address #11480 with this one, modifying |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome! I second @ewbankkit 's comment to see if the issue re: the special aws:
prefixed tags could be easily addressed with these changes, otherwise as-is (aside from the 1 emerging merge conflict) LGTM 🚀
Output of acceptance tests:
--- PASS: TestAccAWSAutoScalingGroup_namePrefix (45.59s)
--- PASS: TestAccAWSAutoScalingGroup_launchTemplate (64.69s)
--- PASS: TestAccAWSAutoScalingGroup_serviceLinkedRoleARN (69.06s)
--- PASS: TestAccAWSAutoScalingGroup_classicVpcZoneIdentifier (104.27s)
--- PASS: TestAccAWSAutoScalingGroup_withMetrics (110.31s)
--- PASS: TestAccAWSAutoScalingGroup_VpcUpdates (113.28s)
--- PASS: TestAccAWSAutoScalingGroup_autoGeneratedName (117.27s)
--- PASS: TestAccAWSAutoScalingGroup_LaunchTemplate_IAMInstanceProfile (70.47s)
--- PASS: TestAccAWSAutoScalingGroup_terminationPolicies (154.61s)
--- PASS: TestAccAWSAutoScalingGroup_launchTemplate_update (118.49s)
--- PASS: TestAccAWSAutoScalingGroup_MaxInstanceLifetime (178.23s)
--- PASS: TestAccAWSAutoScalingGroup_withPlacementGroup (183.23s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy (81.79s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_OnDemandPercentageAboveBaseCapacity (52.13s)
--- PASS: TestAccAWSAutoScalingGroup_enablingMetrics (201.28s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_SpotAllocationStrategy (55.28s)
--- PASS: TestAccAWSAutoScalingGroup_initialLifecycleHook (210.29s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_OnDemandAllocationStrategy (110.69s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_UpdateToZeroOnDemandBaseCapacity (105.23s)
--- PASS: TestAccAWSAutoScalingGroup_TargetGroupArns (224.75s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_LaunchTemplate_LaunchTemplateSpecification_LaunchTemplateName (40.96s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_OnDemandBaseCapacity (122.30s)
--- PASS: TestAccAWSAutoScalingGroup_suspendingProcesses (238.03s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_SpotInstancePools (75.54s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_LaunchTemplate_LaunchTemplateSpecification_Version (58.45s)
--- PASS: TestAccAWSAutoScalingGroup_tags (248.04s)
--- PASS: TestAccAWSAutoScalingGroup_launchTempPartitionNum (38.50s)
--- PASS: TestAccAWSAutoScalingGroup_ALB_TargetGroups (261.58s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_SpotMaxPrice (100.01s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_LaunchTemplate_Override_InstanceType (100.70s)
--- PASS: TestAccAWSAutoScalingGroup_ALB_TargetGroups_ELBCapacity (294.04s)
--- PASS: TestAccAWSAutoScalingGroup_basic (301.35s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_LaunchTemplate_Override_WeightedCapacity (140.95s)
--- PASS: TestAccAWSAutoScalingGroup_WithLoadBalancer (404.02s)
--- PASS: TestAccAWSAutoScalingGroup_WithLoadBalancer_ToTargetGroup (447.12s)
--- PASS: TestAccAWSAutoScalingGroup_LoadBalancers (583.19s)
…ovider ignore_tags Reference: #13808 Output from acceptance testing: ``` --- PASS: TestAccAWSAutoScalingGroup_ALB_TargetGroups (179.86s) --- PASS: TestAccAWSAutoScalingGroup_ALB_TargetGroups_ELBCapacity (333.20s) --- PASS: TestAccAWSAutoScalingGroup_autoGeneratedName (51.77s) --- PASS: TestAccAWSAutoScalingGroup_basic (240.41s) --- PASS: TestAccAWSAutoScalingGroup_classicVpcZoneIdentifier (134.91s) --- PASS: TestAccAWSAutoScalingGroup_enablingMetrics (177.09s) --- PASS: TestAccAWSAutoScalingGroup_initialLifecycleHook (243.57s) --- PASS: TestAccAWSAutoScalingGroup_launchTemplate (45.88s) --- PASS: TestAccAWSAutoScalingGroup_LaunchTemplate_IAMInstanceProfile (58.72s) --- PASS: TestAccAWSAutoScalingGroup_launchTemplate_update (106.61s) --- PASS: TestAccAWSAutoScalingGroup_launchTempPartitionNum (51.25s) --- PASS: TestAccAWSAutoScalingGroup_LoadBalancers (618.13s) --- PASS: TestAccAWSAutoScalingGroup_MaxInstanceLifetime (165.12s) --- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy (44.59s) --- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_OnDemandAllocationStrategy (42.10s) --- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_OnDemandBaseCapacity (76.16s) --- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_OnDemandPercentageAboveBaseCapacity (47.32s) --- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_SpotAllocationStrategy (43.62s) --- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_SpotInstancePools (82.44s) --- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_SpotMaxPrice (150.94s) --- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_UpdateToZeroOnDemandBaseCapacity (78.70s) --- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_LaunchTemplate_LaunchTemplateSpecification_LaunchTemplateName (45.47s) --- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_LaunchTemplate_LaunchTemplateSpecification_Version (84.44s) --- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_LaunchTemplate_Override_InstanceType (76.53s) --- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_LaunchTemplate_Override_WeightedCapacity (207.67s) --- PASS: TestAccAWSAutoScalingGroup_namePrefix (50.25s) --- PASS: TestAccAWSAutoScalingGroup_serviceLinkedRoleARN (54.73s) --- PASS: TestAccAWSAutoScalingGroup_suspendingProcesses (198.11s) --- PASS: TestAccAWSAutoScalingGroup_tags (232.54s) --- PASS: TestAccAWSAutoScalingGroup_TargetGroupArns (247.63s) --- PASS: TestAccAWSAutoScalingGroup_terminationPolicies (105.39s) --- PASS: TestAccAWSAutoScalingGroup_VpcUpdates (175.27s) --- PASS: TestAccAWSAutoScalingGroup_WithLoadBalancer (251.24s) --- PASS: TestAccAWSAutoScalingGroup_WithLoadBalancer_ToTargetGroup (392.43s) --- PASS: TestAccAWSAutoScalingGroup_withMetrics (81.00s) --- PASS: TestAccAWSAutoScalingGroup_withPlacementGroup (146.95s) ```
d4a6d1e
to
d51ee66
Compare
The changes necessary for #11480 are not related to the intentions of this pull request, but a followup can certainly be submitted with covering acceptance test(s). 👍 |
This has been released in version 3.5.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks! |
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! |
Community Note
Closes #12368
Closes #12701
Closes #13808
Closes #14768
Release note for CHANGELOG:
Output from acceptance testing: