-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
r/aws_vpc_endpoint: Fix private_dns_enabled
when false
or undefined
#37715
r/aws_vpc_endpoint: Fix private_dns_enabled
when false
or undefined
#37715
Conversation
Community NoteVoting for Prioritization
For Submitters
|
Before taking this PR out of draft, I'm tempted to add an additional test for the explicit |
New test: make testacc PKG=ec2 TESTS="TestAccVPCEndpoint_interfaceNoPrivateDNS"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.22.2 test ./internal/service/ec2/... -v -count 1 -parallel 20 -run='TestAccVPCEndpoint_interfaceNoPrivateDNS' -timeout 360m
=== RUN TestAccVPCEndpoint_interfaceNoPrivateDNS
=== PAUSE TestAccVPCEndpoint_interfaceNoPrivateDNS
=== CONT TestAccVPCEndpoint_interfaceNoPrivateDNS
--- PASS: TestAccVPCEndpoint_interfaceNoPrivateDNS (131.97s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/ec2 132.434s |
private_dns_enabled = false
(or undefined)private_dns_enabled
when false
or undefined
Further tests: $ make testacc PKG=ec2 TESTS="TestAccVPCEndpointPrivateDNS_"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.22.2 test ./internal/service/ec2/... -v -count 1 -parallel 20 -run='TestAccVPCEndpointPrivateDNS_' -timeout 360m
=== RUN TestAccVPCEndpointPrivateDNS_basic
=== PAUSE TestAccVPCEndpointPrivateDNS_basic
=== RUN TestAccVPCEndpointPrivateDNS_disabled
=== PAUSE TestAccVPCEndpointPrivateDNS_disabled
=== RUN TestAccVPCEndpointPrivateDNS_disappears_Endpoint
=== PAUSE TestAccVPCEndpointPrivateDNS_disappears_Endpoint
=== RUN TestAccVPCEndpointPrivateDNS_update
=== PAUSE TestAccVPCEndpointPrivateDNS_update
=== CONT TestAccVPCEndpointPrivateDNS_basic
=== CONT TestAccVPCEndpointPrivateDNS_disappears_Endpoint
=== CONT TestAccVPCEndpointPrivateDNS_disabled
=== CONT TestAccVPCEndpointPrivateDNS_update
--- PASS: TestAccVPCEndpointPrivateDNS_disabled (91.20s)
--- PASS: TestAccVPCEndpointPrivateDNS_basic (171.95s)
--- PASS: TestAccVPCEndpointPrivateDNS_disappears_Endpoint (180.20s)
--- PASS: TestAccVPCEndpointPrivateDNS_update (219.22s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/ec2 219.365s |
AWS SDK endpoint defaults to true, whereas TF provider defaults to false (at least, prior to bug). Appreciate there's a new resource `aws_vpc_endpoint_private_dns`, which is designed to take precedence over `aws_vpc_endpoint.private_dns_enabled`, which this re-implementation of will cause drift (if undefined, then one-off drift; if defined and not equal to the `aws_vpc_endpoint_private_dns.private_dns_enabled`, then perpetual drift). However, introducing some drift is still preferable than a regression / breaking change.
906459e
to
f14d1b7
Compare
And for completeness, proving that the same tests are failing without the fix in place: $ make testacc PKG=ec2 TESTS="TestAccVPCEndpoint_|TestAccVPCEndpointPrivateDNS_"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.22.2 test ./internal/service/ec2/... -v -count 1 -parallel 20 -run='TestAccVPCEndpoint_|TestAccVPCEndpointPrivateDNS_' -timeout 360m
=== RUN TestAccVPCEndpointPrivateDNS_basic
=== PAUSE TestAccVPCEndpointPrivateDNS_basic
=== RUN TestAccVPCEndpointPrivateDNS_disabled
=== PAUSE TestAccVPCEndpointPrivateDNS_disabled
=== RUN TestAccVPCEndpointPrivateDNS_disappears_Endpoint
=== PAUSE TestAccVPCEndpointPrivateDNS_disappears_Endpoint
=== RUN TestAccVPCEndpointPrivateDNS_update
=== PAUSE TestAccVPCEndpointPrivateDNS_update
=== RUN TestAccVPCEndpoint_gatewayBasic
=== PAUSE TestAccVPCEndpoint_gatewayBasic
=== RUN TestAccVPCEndpoint_interfaceBasic
=== PAUSE TestAccVPCEndpoint_interfaceBasic
=== RUN TestAccVPCEndpoint_interfaceNoPrivateDNS
=== PAUSE TestAccVPCEndpoint_interfaceNoPrivateDNS
=== RUN TestAccVPCEndpoint_interfacePrivateDNS
=== PAUSE TestAccVPCEndpoint_interfacePrivateDNS
=== RUN TestAccVPCEndpoint_interfacePrivateDNSNoGateway
=== PAUSE TestAccVPCEndpoint_interfacePrivateDNSNoGateway
=== RUN TestAccVPCEndpoint_disappears
=== PAUSE TestAccVPCEndpoint_disappears
=== RUN TestAccVPCEndpoint_tags
=== PAUSE TestAccVPCEndpoint_tags
=== RUN TestAccVPCEndpoint_gatewayWithRouteTableAndPolicy
=== PAUSE TestAccVPCEndpoint_gatewayWithRouteTableAndPolicy
=== RUN TestAccVPCEndpoint_gatewayPolicy
=== PAUSE TestAccVPCEndpoint_gatewayPolicy
=== RUN TestAccVPCEndpoint_ignoreEquivalent
=== PAUSE TestAccVPCEndpoint_ignoreEquivalent
=== RUN TestAccVPCEndpoint_ipAddressType
=== PAUSE TestAccVPCEndpoint_ipAddressType
=== RUN TestAccVPCEndpoint_interfaceWithSubnetAndSecurityGroup
=== PAUSE TestAccVPCEndpoint_interfaceWithSubnetAndSecurityGroup
=== RUN TestAccVPCEndpoint_interfaceNonAWSServiceAcceptOnCreate
=== PAUSE TestAccVPCEndpoint_interfaceNonAWSServiceAcceptOnCreate
=== RUN TestAccVPCEndpoint_interfaceNonAWSServiceAcceptOnUpdate
=== PAUSE TestAccVPCEndpoint_interfaceNonAWSServiceAcceptOnUpdate
=== RUN TestAccVPCEndpoint_VPCEndpointType_gatewayLoadBalancer
=== PAUSE TestAccVPCEndpoint_VPCEndpointType_gatewayLoadBalancer
=== CONT TestAccVPCEndpointPrivateDNS_basic
=== CONT TestAccVPCEndpoint_tags
=== CONT TestAccVPCEndpoint_VPCEndpointType_gatewayLoadBalancer
=== CONT TestAccVPCEndpoint_interfaceNonAWSServiceAcceptOnUpdate
=== CONT TestAccVPCEndpoint_interfaceNonAWSServiceAcceptOnCreate
=== CONT TestAccVPCEndpoint_interfaceWithSubnetAndSecurityGroup
=== CONT TestAccVPCEndpoint_ipAddressType
=== CONT TestAccVPCEndpoint_ignoreEquivalent
=== CONT TestAccVPCEndpoint_gatewayPolicy
=== CONT TestAccVPCEndpoint_gatewayWithRouteTableAndPolicy
=== CONT TestAccVPCEndpoint_interfaceBasic
=== CONT TestAccVPCEndpoint_disappears
=== CONT TestAccVPCEndpoint_interfacePrivateDNSNoGateway
=== CONT TestAccVPCEndpoint_interfacePrivateDNS
=== CONT TestAccVPCEndpoint_interfaceNoPrivateDNS
=== CONT TestAccVPCEndpointPrivateDNS_update
=== CONT TestAccVPCEndpoint_gatewayBasic
=== CONT TestAccVPCEndpointPrivateDNS_disappears_Endpoint
=== CONT TestAccVPCEndpointPrivateDNS_disabled
=== NAME TestAccVPCEndpoint_interfaceNoPrivateDNS
vpc_endpoint_test.go:116: Step 1/2 error: Error running apply: exit status 1
Error: creating EC2 VPC Endpoint (com.amazonaws.eu-west-1.ec2): operation error EC2: CreateVpcEndpoint, https response error StatusCode: 400, RequestID: 13e4610f-884b-4803-b7c8-c13a5db92fbc, api error InvalidParameter: Enabling private DNS requires both enableDnsSupport and enableDnsHostnames VPC attributes set to true for vpc-09a6058864bc29dee
with aws_vpc_endpoint.test,
on terraform_plugin_test.tf line 22, in resource "aws_vpc_endpoint" "test":
22: resource "aws_vpc_endpoint" "test" {
=== NAME TestAccVPCEndpoint_interfaceBasic
vpc_endpoint_test.go:72: Step 1/2 error: Error running apply: exit status 1
Error: creating EC2 VPC Endpoint (com.amazonaws.eu-west-1.ec2): operation error EC2: CreateVpcEndpoint, https response error StatusCode: 400, RequestID: 690febc4-0bd3-4ec5-b9fb-3fc6e3ca99a6, api error InvalidParameter: Enabling private DNS requires both enableDnsSupport and enableDnsHostnames VPC attributes set to true for vpc-0fbaae57225283b9c
with aws_vpc_endpoint.test,
on terraform_plugin_test.tf line 22, in resource "aws_vpc_endpoint" "test":
22: resource "aws_vpc_endpoint" "test" {
=== NAME TestAccVPCEndpointPrivateDNS_disabled
vpc_endpoint_private_dns_test.go:74: Step 1/2 error: Error running apply: exit status 1
Error: creating EC2 VPC Endpoint (com.amazonaws.eu-west-1.ec2): operation error EC2: CreateVpcEndpoint, https response error StatusCode: 400, RequestID: 8811ef0e-0427-4e13-9cdd-06bfc8af3014, api error InvalidParameter: Enabling private DNS requires both enableDnsSupport and enableDnsHostnames VPC attributes set to true for vpc-0e429663f50dc9428
with aws_vpc_endpoint.test,
on terraform_plugin_test.tf line 22, in resource "aws_vpc_endpoint" "test":
22: resource "aws_vpc_endpoint" "test" {
--- FAIL: TestAccVPCEndpoint_interfaceNoPrivateDNS (100.30s)
--- FAIL: TestAccVPCEndpoint_interfaceBasic (100.40s)
--- FAIL: TestAccVPCEndpointPrivateDNS_disabled (100.67s)
--- PASS: TestAccVPCEndpoint_disappears (168.03s)
--- PASS: TestAccVPCEndpoint_gatewayBasic (179.64s)
--- PASS: TestAccVPCEndpoint_ignoreEquivalent (199.34s)
=== NAME TestAccVPCEndpoint_interfaceWithSubnetAndSecurityGroup
vpc_endpoint_test.go:483: Step 1/3 error: After applying this test step, the non-refresh plan was not empty.
stdout:
Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
~ update in-place
Terraform will perform the following actions:
# aws_vpc_endpoint.test will be updated in-place
~ resource "aws_vpc_endpoint" "test" {
id = "vpce-026ef09596ae5bef8"
~ private_dns_enabled = true -> false
tags = {
"Name" = "tf-acc-test-8479936531020150750"
}
# (16 unchanged attributes hidden)
# (1 unchanged block hidden)
}
Plan: 0 to add, 1 to change, 0 to destroy.
--- PASS: TestAccVPCEndpointPrivateDNS_disappears_Endpoint (260.56s)
--- PASS: TestAccVPCEndpoint_gatewayPolicy (260.66s)
--- PASS: TestAccVPCEndpoint_gatewayWithRouteTableAndPolicy (272.17s)
--- PASS: TestAccVPCEndpoint_tags (282.44s)
--- PASS: TestAccVPCEndpointPrivateDNS_basic (310.50s)
--- PASS: TestAccVPCEndpoint_interfaceNonAWSServiceAcceptOnCreate (341.61s)
--- PASS: TestAccVPCEndpointPrivateDNS_update (356.04s)
--- PASS: TestAccVPCEndpoint_interfaceNonAWSServiceAcceptOnUpdate (394.39s)
--- FAIL: TestAccVPCEndpoint_interfaceWithSubnetAndSecurityGroup (415.18s)
--- PASS: TestAccVPCEndpoint_ipAddressType (462.08s)
--- PASS: TestAccVPCEndpoint_interfacePrivateDNSNoGateway (482.52s)
--- PASS: TestAccVPCEndpoint_VPCEndpointType_gatewayLoadBalancer (487.79s)
--- PASS: TestAccVPCEndpoint_interfacePrivateDNS (612.36s)
FAIL
FAIL github.com/hashicorp/terraform-provider-aws/internal/service/ec2 612.799s
FAIL |
Hi, any news on this please? It's ready for review. |
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.
LGTM 🚀.
% make testacc TESTARGS='-run=TestAccVPCEndpoint_\|TestAccVPCEndpointPrivateDNS_' PKG=ec2 ACCTEST_PARALLELISM=4
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.22.2 test ./internal/service/ec2/... -v -count 1 -parallel 4 -run=TestAccVPCEndpoint_\|TestAccVPCEndpointPrivateDNS_ -timeout 360m
=== RUN TestAccVPCEndpointPrivateDNS_basic
=== PAUSE TestAccVPCEndpointPrivateDNS_basic
=== RUN TestAccVPCEndpointPrivateDNS_disabled
=== PAUSE TestAccVPCEndpointPrivateDNS_disabled
=== RUN TestAccVPCEndpointPrivateDNS_disappears_Endpoint
=== PAUSE TestAccVPCEndpointPrivateDNS_disappears_Endpoint
=== RUN TestAccVPCEndpointPrivateDNS_update
=== PAUSE TestAccVPCEndpointPrivateDNS_update
=== RUN TestAccVPCEndpoint_gatewayBasic
=== PAUSE TestAccVPCEndpoint_gatewayBasic
=== RUN TestAccVPCEndpoint_interfaceBasic
=== PAUSE TestAccVPCEndpoint_interfaceBasic
=== RUN TestAccVPCEndpoint_interfaceNoPrivateDNS
=== PAUSE TestAccVPCEndpoint_interfaceNoPrivateDNS
=== RUN TestAccVPCEndpoint_interfacePrivateDNS
=== PAUSE TestAccVPCEndpoint_interfacePrivateDNS
=== RUN TestAccVPCEndpoint_interfacePrivateDNSNoGateway
=== PAUSE TestAccVPCEndpoint_interfacePrivateDNSNoGateway
=== RUN TestAccVPCEndpoint_disappears
=== PAUSE TestAccVPCEndpoint_disappears
=== RUN TestAccVPCEndpoint_tags
=== PAUSE TestAccVPCEndpoint_tags
=== RUN TestAccVPCEndpoint_gatewayWithRouteTableAndPolicy
=== PAUSE TestAccVPCEndpoint_gatewayWithRouteTableAndPolicy
=== RUN TestAccVPCEndpoint_gatewayPolicy
=== PAUSE TestAccVPCEndpoint_gatewayPolicy
=== RUN TestAccVPCEndpoint_ignoreEquivalent
=== PAUSE TestAccVPCEndpoint_ignoreEquivalent
=== RUN TestAccVPCEndpoint_ipAddressType
=== PAUSE TestAccVPCEndpoint_ipAddressType
=== RUN TestAccVPCEndpoint_interfaceWithSubnetAndSecurityGroup
=== PAUSE TestAccVPCEndpoint_interfaceWithSubnetAndSecurityGroup
=== RUN TestAccVPCEndpoint_interfaceNonAWSServiceAcceptOnCreate
=== PAUSE TestAccVPCEndpoint_interfaceNonAWSServiceAcceptOnCreate
=== RUN TestAccVPCEndpoint_interfaceNonAWSServiceAcceptOnUpdate
=== PAUSE TestAccVPCEndpoint_interfaceNonAWSServiceAcceptOnUpdate
=== RUN TestAccVPCEndpoint_VPCEndpointType_gatewayLoadBalancer
=== PAUSE TestAccVPCEndpoint_VPCEndpointType_gatewayLoadBalancer
=== CONT TestAccVPCEndpointPrivateDNS_basic
=== CONT TestAccVPCEndpoint_tags
=== CONT TestAccVPCEndpoint_VPCEndpointType_gatewayLoadBalancer
=== CONT TestAccVPCEndpoint_interfaceNonAWSServiceAcceptOnUpdate
--- PASS: TestAccVPCEndpoint_tags (57.95s)
=== CONT TestAccVPCEndpoint_interfaceNonAWSServiceAcceptOnCreate
--- PASS: TestAccVPCEndpointPrivateDNS_basic (144.41s)
=== CONT TestAccVPCEndpoint_interfaceWithSubnetAndSecurityGroup
--- PASS: TestAccVPCEndpoint_interfaceNonAWSServiceAcceptOnUpdate (308.16s)
=== CONT TestAccVPCEndpoint_ipAddressType
--- PASS: TestAccVPCEndpoint_interfaceNonAWSServiceAcceptOnCreate (272.29s)
=== CONT TestAccVPCEndpoint_ignoreEquivalent
--- PASS: TestAccVPCEndpoint_VPCEndpointType_gatewayLoadBalancer (350.41s)
=== CONT TestAccVPCEndpoint_gatewayPolicy
--- PASS: TestAccVPCEndpoint_ignoreEquivalent (33.30s)
=== CONT TestAccVPCEndpoint_gatewayWithRouteTableAndPolicy
--- PASS: TestAccVPCEndpoint_gatewayPolicy (48.18s)
=== CONT TestAccVPCEndpoint_interfaceBasic
--- PASS: TestAccVPCEndpoint_gatewayWithRouteTableAndPolicy (47.18s)
=== CONT TestAccVPCEndpoint_disappears
--- PASS: TestAccVPCEndpoint_disappears (25.61s)
=== CONT TestAccVPCEndpoint_interfacePrivateDNSNoGateway
--- PASS: TestAccVPCEndpoint_interfaceBasic (43.60s)
=== CONT TestAccVPCEndpoint_interfacePrivateDNS
--- PASS: TestAccVPCEndpoint_interfaceWithSubnetAndSecurityGroup (341.44s)
=== CONT TestAccVPCEndpoint_interfaceNoPrivateDNS
--- PASS: TestAccVPCEndpoint_interfaceNoPrivateDNS (77.96s)
=== CONT TestAccVPCEndpointPrivateDNS_update
--- PASS: TestAccVPCEndpoint_ipAddressType (318.50s)
=== CONT TestAccVPCEndpoint_gatewayBasic
--- PASS: TestAccVPCEndpoint_gatewayBasic (28.01s)
=== CONT TestAccVPCEndpointPrivateDNS_disappears_Endpoint
--- PASS: TestAccVPCEndpointPrivateDNS_update (202.70s)
=== CONT TestAccVPCEndpointPrivateDNS_disabled
--- PASS: TestAccVPCEndpointPrivateDNS_disappears_Endpoint (127.90s)
--- PASS: TestAccVPCEndpointPrivateDNS_disabled (63.81s)
--- PASS: TestAccVPCEndpoint_interfacePrivateDNSNoGateway (464.42s)
--- PASS: TestAccVPCEndpoint_interfacePrivateDNS (478.17s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/ec2 925.199s
This functionality has been released in v5.53.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. Thank you! |
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. |
Description
The API documentation for the
CreateVpcEndpoint
operation documents that thePrivateDnsEnabled
attribute defaults totrue
, whereas this TF provider'saws_vpc_endpoint
resource'sprivate_dns_enabled
attribute defaults tofalse
(at least, prior to bug).This means that if the user wants the attribute to be
false
(either by explicitly configuringprivate_dns_enabled = false
or leavingprivate_dns_enabled
undefined), then theaws_vpc_endpoint
resource implementation must explicitly passPrivateDnsEnabled = false
in the SDK call.Notes
Appreciate there's a recently introduced resource
aws_vpc_endpoint_private_dns
, which is designed to take precedence overaws_vpc_endpoint.private_dns_enabled
, and if used in conjunction with this fix will cause drift (ifaws_vpc_endpoint.private_dns_enabled
is undefined, then one-off drift; if defined and not equal to theaws_vpc_endpoint_private_dns.private_dns_enabled
, then perpetual drift).However, introducing some drift is still preferable than a regression / breaking change, which is the case without this fix. Furthermore, the documentation for
aws_vpc_endpoint_private_dns
sufficiently documents and discourages this conflicting usage.Relations
Closes #37694
References
Output from Acceptance Testing