-
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
Only reject tags for standard EIPs on EC2 Classic #18909
Only reject tags for standard EIPs on EC2 Classic #18909
Conversation
@rick-masters Cool! Thanks for the awesomely quick response. I want to make sure we catch all cases. Forgive/correct my ignorances of all the EC2 classic nuances as I go through these. This is how it should behave?
|
I'm not sure about the column "default vpc?". Does that mean "Is there a default VPC present in the region?" If so, then I'm not sure that is relevant but I suppose it can't hurt to test. With regards to EC2 Classic or VPC there are two factors: whether the account supports EC2 classic (was it created circa 2013 or earlier) and the second factor is does the region support EC2 Classic (us-east-1, us-west-1, us-west-2 are yes, us-east-2 is no). EC2 Classic is available only when both are true. BTW, I have access to both types of accounts for testing. |
Are tags precisely on allocation terribly important? If not, we could simply skip tags in create and let update (called by create) take care of them. The benefit there is that we're relying on what AWS says the domain type is rather than trying to make sure we guess correctly. I think 🤞 we've got the logic down but I'm concerned about edge cases. If a VPC account/region has no default VPC and the config doesn't set |
Yes, it looks like based on your comment here #16881 (comment), that tags are indeed important on creation. |
My understanding is that the present of a default vpc is not relevant to the type of EIP that is created. https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_AllocateAddress.html
I suspect this means "If your account and region supports EC2-Classic, ...." but I'll confirm when I run my test in a few minutes. |
I didn't run all variations, but I ran these and have not seen a problem so far:
main.tf
test.sh
|
Passed all EIP acceptance tests. There are two runs here, one with a VPC style account and one with an EC2 Classic account. VPC style account:
With EC2 Classic account:
|
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.
Looks great! 🎉
I ran tests in 4 environments with varying levels of EC2VPC, Classicness.
Acceptance tests in us-west-2
(187) (EC2VPC & EC2Classic):
--- PASS: TestAccAWSEIP_basic (15.49s)
--- PASS: TestAccAWSEIP_carrierIP (19.02s)
--- PASS: TestAccAWSEIP_disappears (10.27s)
--- PASS: TestAccAWSEIP_Instance (75.28s)
--- PASS: TestAccAWSEIP_Instance_ec2Classic (105.01s)
--- PASS: TestAccAWSEIP_Instance_notAssociated (119.49s)
--- PASS: TestAccAWSEIP_Instance_reassociate (136.69s)
--- PASS: TestAccAWSEIP_networkBorderGroup (14.44s)
--- PASS: TestAccAWSEIP_NetworkInterface (75.25s)
--- PASS: TestAccAWSEIP_NetworkInterface_twoEIPsOneInterface (74.76s)
--- PASS: TestAccAWSEIP_PublicIPv4Pool_default (14.41s)
--- PASS: TestAccAWSEIP_Tags_EC2Classic_withoutVPCTrue (3.07s)
--- PASS: TestAccAWSEIP_Tags_EC2Classic_withVPCTrue (15.42s)
--- PASS: TestAccAWSEIP_Tags_EC2VPC_withoutVPCTrue (21.91s)
--- PASS: TestAccAWSEIP_Tags_EC2VPC_withVPCTrue (22.41s)
--- SKIP: TestAccAWSEIP_CustomerOwnedIPv4Pool (0.64s)
--- SKIP: TestAccAWSEIP_PublicIPv4Pool_custom (0.00s)
--- PASS: TestAccAWSEIPAssociation_spotInstance (91.77s)
--- PASS: TestAccAWSEIPAssociation_instance (104.42s)
--- PASS: TestAccAWSEIPAssociation_disappears (109.00s)
--- PASS: TestAccAWSEIPAssociation_networkInterface (65.75s)
--- PASS: TestAccAWSEIPAssociation_basic (164.08s)
--- PASS: TestAccAWSEIPAssociation_ec2Classic (215.81s)
Acceptance tests in us-west-2
(153) (EC2VPC):
--- PASS: TestAccAWSEIP_basic (17.51s)
--- PASS: TestAccAWSEIP_disappears (9.69s)
--- PASS: TestAccAWSEIP_Instance (87.05s)
--- PASS: TestAccAWSEIP_Instance_associatedUserPrivateIP (254.11s)
--- PASS: TestAccAWSEIP_Instance_notAssociated (127.85s)
--- PASS: TestAccAWSEIP_Instance_reassociate (141.58s)
--- PASS: TestAccAWSEIP_networkBorderGroup (14.70s)
--- PASS: TestAccAWSEIP_NetworkInterface (73.95s)
--- PASS: TestAccAWSEIP_NetworkInterface_twoEIPsOneInterface (75.22s)
--- PASS: TestAccAWSEIP_PublicIPv4Pool_default (14.07s)
--- PASS: TestAccAWSEIP_Tags_EC2VPC_withoutVPCTrue (21.62s)
--- PASS: TestAccAWSEIP_Tags_EC2VPC_withVPCTrue (23.98s)
--- SKIP: TestAccAWSEIP_carrierIP (0.36s)
--- SKIP: TestAccAWSEIP_CustomerOwnedIPv4Pool (0.61s)
--- SKIP: TestAccAWSEIP_Instance_ec2Classic (0.00s)
--- SKIP: TestAccAWSEIP_PublicIPv4Pool_custom (0.00s)
--- SKIP: TestAccAWSEIP_Tags_EC2Classic_withoutVPCTrue (0.41s)
--- SKIP: TestAccAWSEIP_Tags_EC2Classic_withVPCTrue (0.00s)
--- PASS: TestAccAWSEIPAssociation_spotInstance (79.88s)
--- PASS: TestAccAWSEIPAssociation_instance (123.40s)
--- PASS: TestAccAWSEIPAssociation_basic (164.44s)
--- SKIP: TestAccAWSEIPAssociation_ec2Classic (0.44s)
--- PASS: TestAccAWSEIPAssociation_disappears (97.00s)
--- PASS: TestAccAWSEIPAssociation_networkInterface (66.06s)
Acceptance tests in us-east-1
(187) (EC2Classic):
--- PASS: TestAccAWSEIP_basic (10.29s)
--- PASS: TestAccAWSEIP_carrierIP (11.19s)
--- PASS: TestAccAWSEIP_disappears (7.13s)
--- PASS: TestAccAWSEIP_Instance (68.33s)
--- PASS: TestAccAWSEIP_Instance_associatedUserPrivateIP (250.69s)
--- PASS: TestAccAWSEIP_Instance_ec2Classic (236.33s)
--- PASS: TestAccAWSEIP_Instance_notAssociated (118.52s)
--- PASS: TestAccAWSEIP_Instance_reassociate (143.94s)
--- PASS: TestAccAWSEIP_networkBorderGroup (9.74s)
--- PASS: TestAccAWSEIP_NetworkInterface (60.48s)
--- PASS: TestAccAWSEIP_NetworkInterface_twoEIPsOneInterface (59.32s)
--- PASS: TestAccAWSEIP_PublicIPv4Pool_default (9.81s)
--- PASS: TestAccAWSEIP_Tags_EC2Classic_withoutVPCTrue (2.69s)
--- PASS: TestAccAWSEIP_Tags_EC2Classic_withVPCTrue (15.09s)
--- SKIP: TestAccAWSEIP_CustomerOwnedIPv4Pool (0.33s)
--- SKIP: TestAccAWSEIP_PublicIPv4Pool_custom (0.00s)
--- SKIP: TestAccAWSEIP_Tags_EC2VPC_withoutVPCTrue (0.00s)
--- SKIP: TestAccAWSEIP_Tags_EC2VPC_withVPCTrue (0.52s)
--- SKIP: TestAccAWSEIPAssociation_basic (0.44s)
--- PASS: TestAccAWSEIPAssociation_networkInterface (54.55s)
--- PASS: TestAccAWSEIPAssociation_disappears (125.24s)
--- PASS: TestAccAWSEIPAssociation_ec2Classic (102.34s)
--- PASS: TestAccAWSEIPAssociation_spotInstance (292.23s)
--- PASS: TestAccAWSEIPAssociation_instance (330.69s)
Acceptance tests in GovCloud (EC2VPC):
--- PASS: TestAccAWSEIP_basic (19.27s)
--- PASS: TestAccAWSEIP_disappears (12.02s)
--- PASS: TestAccAWSEIP_Instance (96.93s)
--- PASS: TestAccAWSEIP_Instance_associatedUserPrivateIP (210.48s)
--- PASS: TestAccAWSEIP_Instance_notAssociated (117.47s)
--- PASS: TestAccAWSEIP_Instance_reassociate (158.41s)
--- PASS: TestAccAWSEIP_networkBorderGroup (17.91s)
--- PASS: TestAccAWSEIP_NetworkInterface (76.14s)
--- PASS: TestAccAWSEIP_NetworkInterface_twoEIPsOneInterface (76.64s)
--- PASS: TestAccAWSEIP_PublicIPv4Pool_default (17.88s)
--- PASS: TestAccAWSEIP_Tags_EC2VPC_withoutVPCTrue (27.81s)
--- PASS: TestAccAWSEIP_Tags_EC2VPC_withVPCTrue (28.55s)
--- SKIP: TestAccAWSEIP_carrierIP (0.35s)
--- SKIP: TestAccAWSEIP_CustomerOwnedIPv4Pool (1.91s)
--- SKIP: TestAccAWSEIP_Instance_ec2Classic (0.00s)
--- SKIP: TestAccAWSEIP_PublicIPv4Pool_custom (0.00s)
--- SKIP: TestAccAWSEIP_Tags_EC2Classic_withoutVPCTrue (1.00s)
--- SKIP: TestAccAWSEIP_Tags_EC2Classic_withVPCTrue (0.48s)
--- PASS: TestAccAWSEIPAssociation_spotInstance (102.45s)
--- SKIP: TestAccAWSEIPAssociation_ec2Classic (1.06s)
--- PASS: TestAccAWSEIPAssociation_instance (114.41s)
--- PASS: TestAccAWSEIPAssociation_disappears (120.92s)
--- PASS: TestAccAWSEIPAssociation_networkInterface (68.43s)
--- PASS: TestAccAWSEIPAssociation_basic (173.32s)
This has been released in version 3.37.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! |
Only reject tags for standard EIPs for EC2 Classic platforms.
Fixes a regression introduced here:
https://github.com/hashicorp/terraform-provider-aws/pull/17612/files#diff-5cc53a93a886b4b2474bd02b135f8aabd7f4aeb82d77e8ac0378386eaa01ae84L186
The code above did not take account of the fact that accounts without EC2 Classic support will automatically default to type "vpc" for EIPs and so tags are ok.
Community Note
Closes #18756
Output from acceptance testing:
Pending...