Force vpc endpoint type to gateway in govcloud partition. #3317

jmcarp · 2018-02-10T05:08:29Z

The aws govcloud partition only supports gateway type vpc endpoints
and doesn't return a type when listing connections. To fix, temporarily
force vpc endpoint types to gateway if partition is govcloud.

After vpc endpoint types were added in 1.9, govcloud vpc endpoints
want to destroy and recreate on every apply, since the aws api doesn't
return an endpoint type.

cc @wjwoodson

jmcarp · 2018-02-10T05:15:59Z

By the way, it might be useful to add an integration test that creates a resource, then verifies that the plan is empty, for all resource types. That should help catch regressions like this, where a resource is recreated every apply.

ewbankkit · 2018-02-11T21:42:02Z

aws/resource_aws_vpc_endpoint.go

+	if isGovCloud {
+		d.Set("vpc_endpoint_type", ec2.VpcEndpointTypeGateway)
+	} else {
+		vpceType := aws.StringValue(vpce.VpcEndpointType)


No need to call aws.StringValue on the VPC endpoint type as d.Set will handle *string parameters. I only call it above for the service name as the value is used later.

ewbankkit · 2018-02-11T21:44:06Z

aws/resource_aws_vpc_endpoint.go

@@ -341,14 +341,18 @@ func vpcEndpointWaitUntilAvailable(d *schema.ResourceData, conn *ec2.EC2) error
 	return nil
 }

-func vpcEndpointAttributes(d *schema.ResourceData, vpce *ec2.VpcEndpoint, conn *ec2.EC2) error {
+func vpcEndpointAttributes(d *schema.ResourceData, vpce *ec2.VpcEndpoint, conn *ec2.EC2, isGovCloud bool) error {


Consider changing the function signature to
func vpcEndpointAttributes(d *schema.ResourceData, meta interface{}, vpce *ec2.VpcEndpoint) error
as both conn and isGovCloud can be derived from meta?

jmcarp · 2018-02-16T05:01:09Z

Updated. WDYT @ewbankkit ?

jmcarp · 2018-02-19T18:13:20Z

Pinging @radeksimko or @bflad for review when you have time.

jmcarp · 2018-03-07T15:50:45Z

ping @bflad

bflad · 2018-03-07T16:21:48Z

Can you provide a previously failing configuration and the outputs from plan/apply?

jmcarp · 2018-03-07T17:03:58Z

@bflad: terraform plan/apply doesn't fail because of this issue--it recreates the endpoint on every apply. Every plan says something like module.stack.module.base.module.vpc.aws_vpc_endpoint.private-s3 (new resource required) because it wants to change the endpoint type: vpc_endpoint_type: "" => "Gateway" (forces new resource). Every apply recreates the resource, but since govcloud ignores the endpoint type parameter, we repeat the cycle on the next plan.

jmcarp · 2018-03-09T18:16:51Z

@bflad: does that description make sense? Let me know if more output would be helpful.

bflad

Let's simplify this PR and make it a little future proof if/when AWS updates US Gov to join Standard functionality. 👍

bflad · 2018-03-09T20:35:21Z

aws/resource_aws_vpc_endpoint.go

@@ -341,14 +341,17 @@ func vpcEndpointWaitUntilAvailable(d *schema.ResourceData, conn *ec2.EC2) error
 	return nil
 }

-func vpcEndpointAttributes(d *schema.ResourceData, vpce *ec2.VpcEndpoint, conn *ec2.EC2) error {
+func vpcEndpointAttributes(d *schema.ResourceData, vpce *ec2.VpcEndpoint, meta interface{}) error {


Let's revert this, I'll explain below. 😄

bflad · 2018-03-09T20:37:48Z

aws/resource_aws_vpc_endpoint.go

 	d.Set("state", vpce.State)
 	d.Set("vpc_id", vpce.VpcId)

 	serviceName := aws.StringValue(vpce.ServiceName)
 	d.Set("service_name", serviceName)
-	vpceType := aws.StringValue(vpce.VpcEndpointType)
-	d.Set("vpc_endpoint_type", vpceType)
+	if meta.(*AWSClient).IsGovCloud() && aws.StringValue(vpce.VpcEndpointType) == "" {


Instead of relying on a meta.(*AWSClient).IsGovCloud() check, which one day may become obsolete, let's just perform the second check aws.StringValue(vpce.VpcEndpointType) == "" with a comment above it why the logic is in place. 👍

The aws govcloud partition only supports gateway type vpc endpoints and doesn't return a type when listing connections. To fix, temporarily force vpc endpoint types to gateway if partition is govcloud.

@ewbankkit

h/t @ewbankkit

@wjwoodson

Once govcloud implements multiple vpc endpoint types, we should stop forcing the endpoint type to gateway. h/t @wjwoodson

jmcarp · 2018-03-09T22:31:32Z

Sounds good, updated.

@bflad

h/t @bflad

bflad

LGTM! 🚀

Standard partition (test failure unrelated):

Tests failed: 1, passed: 14
=== RUN   TestAccAWSVpcEndpoint_gatewayBasic
--- PASS: TestAccAWSVpcEndpoint_gatewayBasic (81.58s)
=== RUN   TestAccAWSVpcEndpointSubnetAssociation_basic
--- PASS: TestAccAWSVpcEndpointSubnetAssociation_basic (86.20s)
=== RUN   TestAccAWSVpcEndpointRouteTableAssociation_basic
--- PASS: TestAccAWSVpcEndpointRouteTableAssociation_basic (87.36s)
=== RUN   TestAccAWSVpcEndpoint_interfaceBasic
--- PASS: TestAccAWSVpcEndpoint_interfaceBasic (89.52s)
=== RUN   TestAccAWSVpcEndpoint_removed
--- PASS: TestAccAWSVpcEndpoint_removed (90.47s)
=== RUN   TestAccAWSVpcEndpoint_importBasic
--- PASS: TestAccAWSVpcEndpoint_importBasic (104.60s)
=== RUN   TestAccAWSVpcEndpoint_gatewayWithRouteTableAndPolicy
--- PASS: TestAccAWSVpcEndpoint_gatewayWithRouteTableAndPolicy (131.70s)
=== RUN   TestAccAWSVpcEndpoint_interfaceNonAWSService
--- PASS: TestAccAWSVpcEndpoint_interfaceNonAWSService (245.79s)
=== RUN   TestAccAWSVpcEndpoint_interfaceWithSubnetAndSecurityGroup
--- PASS: TestAccAWSVpcEndpoint_interfaceWithSubnetAndSecurityGroup (280.00s)
=== RUN   TestAccAWSVpcEndpointConnectionNotification_importBasic
--- PASS: TestAccAWSVpcEndpointConnectionNotification_importBasic (331.16s)
=== RUN   TestAccAWSVpcEndpointService_importBasic
--- PASS: TestAccAWSVpcEndpointService_importBasic (362.66s)
=== RUN   TestAccAWSVpcEndpointConnectionNotification_basic
--- FAIL: TestAccAWSVpcEndpointConnectionNotification_basic (428.44s)
=== RUN   TestAccAWSVpcEndpointServiceAllowedPrincipal_basic
--- PASS: TestAccAWSVpcEndpointServiceAllowedPrincipal_basic (447.05s)
=== RUN   TestAccAWSVpcEndpointService_basic
--- PASS: TestAccAWSVpcEndpointService_basic (481.79s)
=== RUN   TestAccAWSVpcEndpointService_removed
--- PASS: TestAccAWSVpcEndpointService_removed (1145.25s)

US Gov partition (manually):

terraform plan
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

aws_vpc.main: Refreshing state... (ID: vpc-410c6524)
aws_vpc_endpoint.test: Refreshing state... (ID: vpce-06fd0c6f)

------------------------------------------------------------------------

No changes. Infrastructure is up-to-date.

This means that Terraform did not detect any differences between your
configuration and real physical resources that exist. As a result, no
actions need to be performed.

bflad · 2018-03-23T21:16:39Z

This has been released in version 1.12.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

lorengordon · 2018-03-26T20:15:57Z

Thanks @bflad, tested and it does indeed solve the problem I was having.

ghost · 2020-04-07T17:18:40Z

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!

ghost added the size/S Managed by automation to categorize the size of a PR. label Feb 10, 2018

radeksimko added bug Addresses a defect in current functionality. service/ec2 Issues and PRs that pertain to the ec2 service. labels Feb 11, 2018

ewbankkit reviewed Feb 11, 2018

View reviewed changes

ghost added the size/S Managed by automation to categorize the size of a PR. label Feb 12, 2018

ewbankkit mentioned this pull request Feb 23, 2018

v1.9.0: VPC Endpoint Data Resource no longer works in GovCloud #3506

Closed

bflad reviewed Mar 9, 2018

View reviewed changes

ghost added size/XS Managed by automation to categorize the size of a PR. and removed size/S Managed by automation to categorize the size of a PR. labels Mar 9, 2018

jmcarp added 3 commits March 9, 2018 17:30

Force vpc endpoint type to gateway in govcloud partition.

788721d

The aws govcloud partition only supports gateway type vpc endpoints and doesn't return a type when listing connections. To fix, temporarily force vpc endpoint types to gateway if partition is govcloud.

Clean up govcloud check for vpc endpoint resource.

d0ef740

h/t @ewbankkit

Only override vpc endpoint type on govcloud if empty.

7a106be

Once govcloud implements multiple vpc endpoint types, we should stop forcing the endpoint type to gateway. h/t @wjwoodson

jmcarp force-pushed the govcloud-vpc-endpoint-type branch from 6d43ed9 to 5b7080e Compare March 9, 2018 22:31

ghost added the size/XS Managed by automation to categorize the size of a PR. label Mar 9, 2018

Drop explicit govcloud check from vpc endpoint type check.

e7b035c

h/t @bflad

jmcarp force-pushed the govcloud-vpc-endpoint-type branch from 5b7080e to e7b035c Compare March 9, 2018 22:37

ghost added the size/XS Managed by automation to categorize the size of a PR. label Mar 9, 2018

bflad added the partition/aws-us-gov Pertains to the aws-us-gov partition. label Mar 12, 2018

bflad mentioned this pull request Mar 12, 2018

Service details not yet available in GovCloud. #3514

Merged

bflad added this to the v1.12.0 milestone Mar 12, 2018

sodre mentioned this pull request Mar 12, 2018

Add option to force Gov/China Cloud #3727

Closed

bflad approved these changes Mar 12, 2018

View reviewed changes

bflad merged commit a542f6e into hashicorp:master Mar 12, 2018

bflad added a commit that referenced this pull request Mar 12, 2018

Update CHANGELOG for #3317

7bdce07

ewbankkit mentioned this pull request Mar 14, 2018

aws_vpc_endpoint get recreated on each apply on AWS PrivateLink unsuported regions. #3777

Closed

wjwoodson mentioned this pull request Mar 14, 2018

New version of dependency terraform available: 1.11.0 cloud-gov/terraform-provision#367

Closed

ghost locked and limited conversation to collaborators Apr 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Force vpc endpoint type to gateway in govcloud partition. #3317

Force vpc endpoint type to gateway in govcloud partition. #3317

jmcarp commented Feb 10, 2018

jmcarp commented Feb 10, 2018 •

edited

Loading

ewbankkit Feb 11, 2018

ewbankkit Feb 11, 2018

jmcarp commented Feb 16, 2018

jmcarp commented Feb 19, 2018

jmcarp commented Mar 7, 2018

bflad commented Mar 7, 2018

jmcarp commented Mar 7, 2018

jmcarp commented Mar 9, 2018

bflad left a comment

bflad Mar 9, 2018

bflad Mar 9, 2018

jmcarp commented Mar 9, 2018

bflad left a comment

bflad commented Mar 23, 2018

lorengordon commented Mar 26, 2018

ghost commented Apr 7, 2020

Force vpc endpoint type to gateway in govcloud partition. #3317

Force vpc endpoint type to gateway in govcloud partition. #3317

Conversation

jmcarp commented Feb 10, 2018

jmcarp commented Feb 10, 2018 • edited Loading

ewbankkit Feb 11, 2018

Choose a reason for hiding this comment

ewbankkit Feb 11, 2018

Choose a reason for hiding this comment

jmcarp commented Feb 16, 2018

jmcarp commented Feb 19, 2018

jmcarp commented Mar 7, 2018

bflad commented Mar 7, 2018

jmcarp commented Mar 7, 2018

jmcarp commented Mar 9, 2018

bflad left a comment

Choose a reason for hiding this comment

bflad Mar 9, 2018

Choose a reason for hiding this comment

bflad Mar 9, 2018

Choose a reason for hiding this comment

jmcarp commented Mar 9, 2018

bflad left a comment

Choose a reason for hiding this comment

bflad commented Mar 23, 2018

lorengordon commented Mar 26, 2018

ghost commented Apr 7, 2020

jmcarp commented Feb 10, 2018 •

edited

Loading