Add Route 53 Resolver endpoint resource #6574

ewbankkit · 2018-11-24T23:20:10Z

Fixes #6563.
Includes:

Acceptance tests (so far):

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsRoute53ResolverEndpoint_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccAwsRoute53ResolverEndpoint_ -timeout 120m
=== RUN   TestAccAwsRoute53ResolverEndpoint_importBasic
=== PAUSE TestAccAwsRoute53ResolverEndpoint_importBasic
=== RUN   TestAccAwsRoute53ResolverEndpoint_basicInbound
=== PAUSE TestAccAwsRoute53ResolverEndpoint_basicInbound
=== RUN   TestAccAwsRoute53ResolverEndpoint_updateOutbound
=== PAUSE TestAccAwsRoute53ResolverEndpoint_updateOutbound
=== CONT  TestAccAwsRoute53ResolverEndpoint_importBasic
=== CONT  TestAccAwsRoute53ResolverEndpoint_updateOutbound
=== CONT  TestAccAwsRoute53ResolverEndpoint_basicInbound
--- PASS: TestAccAwsRoute53ResolverEndpoint_importBasic (108.99s)
--- PASS: TestAccAwsRoute53ResolverEndpoint_basicInbound (129.35s)
--- PASS: TestAccAwsRoute53ResolverEndpoint_updateOutbound (503.80s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	503.819s

ewbankkit · 2018-11-24T23:29:01Z

This is still a WIP as I'm having problems supporting more than one "IP address" (subnet ID and optional specified IP) per subnet.
The issue is that I want to use a TypeSet for the ip_address attribute (using TypeList has the problem that the order of IP addresses returned from ListResolverEndpointIpAddresses is non-deterministic and state diffs will keep showing) but there are a couple of wrinkles:

ip is computed - If it's blank when creating or updating the Resolver endpoint then AWS assigns a value, so it can't be used in the set's hash function
More than one ip_address per subnet_id is allowed so I can't just use the hash of the subnet ID as the set's hash value although that is what I am doing right now to get the tests working

gazoakley · 2018-11-24T23:57:03Z

You might be able to use TypeList with DiffSuppressFunc and match things up by ip_id. That sounds like a pain, but it might work.

gazoakley · 2018-11-25T02:45:24Z

Your DiffSuppressFunc might:

Check the ip_address entries where ip is specified all exist and are associated with the correct subnet_id
Check there are the correct number of entries per subnet_id for the remaining ip_address entries
Suppress the diff if all the above pass

ewbankkit · 2018-12-07T22:53:34Z

Not having much luck with the ip_address diff issue and won't have time to work on this for the next week or so.
I've reverted to a set for now with still the same concerns:

	// TODO
	// TODO * "ip" may be computed - Blank on input, set by API on output; How to handle?
	// TODO * Multiple "ip_address" values per "subnet_id" are allowed in the API
	// TODO   but a List (instead of a Set) won't work because of non-deterministic order
	// TODO   on listing ip_address, plus how to determine diffs for update?
	// TODO

Acceptance tests:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsRoute53ResolverEndpoint_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccAwsRoute53ResolverEndpoint_ -timeout 120m
=== RUN   TestAccAwsRoute53ResolverEndpoint_basicInbound
=== PAUSE TestAccAwsRoute53ResolverEndpoint_basicInbound
=== RUN   TestAccAwsRoute53ResolverEndpoint_updateOutbound
=== PAUSE TestAccAwsRoute53ResolverEndpoint_updateOutbound
=== CONT  TestAccAwsRoute53ResolverEndpoint_basicInbound
=== CONT  TestAccAwsRoute53ResolverEndpoint_updateOutbound
--- PASS: TestAccAwsRoute53ResolverEndpoint_basicInbound (135.30s)
--- PASS: TestAccAwsRoute53ResolverEndpoint_updateOutbound (516.18s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	516.201s

ewbankkit · 2018-12-16T23:12:55Z

Rebased to fix conflicts.
Re-ran acceptance tests:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsRoute53ResolverEndpoint_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccAwsRoute53ResolverEndpoint_ -timeout 120m
=== RUN   TestAccAwsRoute53ResolverEndpoint_basicInbound
=== PAUSE TestAccAwsRoute53ResolverEndpoint_basicInbound
=== RUN   TestAccAwsRoute53ResolverEndpoint_updateOutbound
=== PAUSE TestAccAwsRoute53ResolverEndpoint_updateOutbound
=== CONT  TestAccAwsRoute53ResolverEndpoint_basicInbound
=== CONT  TestAccAwsRoute53ResolverEndpoint_updateOutbound
--- PASS: TestAccAwsRoute53ResolverEndpoint_basicInbound (132.30s)
--- PASS: TestAccAwsRoute53ResolverEndpoint_updateOutbound (569.06s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	569.081s

ewbankkit · 2018-12-16T23:15:21Z

@bflad Looking for advice on the best way of dealing with the set/list conundrum for the ip_address attribute. It best fits the requirements for a set - Order is unimportant, but there are problems with determining the correct hash function.
Maybe we leave as I have it now - Just use subnet_id, which is the one Required field, as the hash value?

ewbankkit · 2018-12-18T21:09:07Z

~~TODO: Update tag handling code to reflect comments here.~~

rbadillo · 2019-01-04T16:02:26Z

Hi team, any updates when this will be available ?

@ewbankkit

ewbankkit · 2019-01-07T19:45:37Z

@bflad Any thoughts on my stumbling block - #6574 (comment)?

aar6ncai · 2019-01-09T04:33:40Z

hi guys, any update for this feature become available ? thanks

ewbankkit · 2019-01-09T13:51:37Z

@aar6ncai I'm going to need this quite soon too 😄.
I'll remove WIP and submit for formal review with the caveat that there is the known concern around ip in the ip_address block.

kmcquade · 2019-01-22T21:18:45Z

@ewbankkit @gazoakley - any word? We need this pretty soon as well.

ewbankkit · 2019-01-23T17:16:46Z

Rebased to remove conflict
Correct acceptance tests to use tags = {} syntax for TF 0.12

Acceptance tests:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsRoute53ResolverEndpoint_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccAwsRoute53ResolverEndpoint_ -timeout 120m
=== RUN   TestAccAwsRoute53ResolverEndpoint_basicInbound
=== PAUSE TestAccAwsRoute53ResolverEndpoint_basicInbound
=== RUN   TestAccAwsRoute53ResolverEndpoint_updateOutbound
=== PAUSE TestAccAwsRoute53ResolverEndpoint_updateOutbound
=== CONT  TestAccAwsRoute53ResolverEndpoint_basicInbound
=== CONT  TestAccAwsRoute53ResolverEndpoint_updateOutbound
--- PASS: TestAccAwsRoute53ResolverEndpoint_basicInbound (108.17s)
--- PASS: TestAccAwsRoute53ResolverEndpoint_updateOutbound (614.07s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	629.443s

…eted'.

ewbankkit · 2019-02-28T14:52:33Z

Rebased to remove the merge conflict (and use route53resolverconn from the AWSClient structure).
Acceptance tests:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsRoute53ResolverEndpoint_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccAwsRoute53ResolverEndpoint_ -timeout 120m
=== RUN   TestAccAwsRoute53ResolverEndpoint_basicInbound
=== PAUSE TestAccAwsRoute53ResolverEndpoint_basicInbound
=== RUN   TestAccAwsRoute53ResolverEndpoint_updateOutbound
=== PAUSE TestAccAwsRoute53ResolverEndpoint_updateOutbound
=== CONT  TestAccAwsRoute53ResolverEndpoint_basicInbound
=== CONT  TestAccAwsRoute53ResolverEndpoint_updateOutbound
--- PASS: TestAccAwsRoute53ResolverEndpoint_basicInbound (96.67s)
--- PASS: TestAccAwsRoute53ResolverEndpoint_updateOutbound (561.65s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	561.708s

nywilken · 2019-02-28T15:12:23Z

Maybe we leave as I have it now - Just use subnet_id, which is the one Required field, as the hash value?

@ewbankkit @kmcquade do you have a sense of what the general use case would be for the subnet selection when creating resolvers? More specifically, would you say that the need is to configure resolvers with two different subnet_ids or is it likely that most consumers would need to define the same subnet_id multiple times? If it is the former that I would feel more comfortable merging this in its current state and handling the multiple subnet_id post release.

ewbankkit · 2019-02-28T15:37:16Z

The AWS recommendation is two or more IP addresses in different Availability Zones which means distinct subnet IDs (and I think the API even enforces this, I'll check on that) and that is certainly how we will configuring our setup.

Verified that distinct subnet IDs must be specified:

$ aws route53resolver create-resolver-endpoint --name foo --security-group-ids sg-01234567 --creator-request-id foo --direction INBOUND --ip-addresses SubnetId=subnet-01234567,SubnetId=subnet-01234567

An error occurred (InvalidRequestException) when calling the CreateResolverEndpoint operation: [RSLVR-00400] Resolver endpoint need to have at least 2 IP addresses.

nywilken · 2019-02-28T15:55:35Z

The AWS recommendation is two or more IP addresses in different Availability Zones which means distinct subnet IDs (and I think the API even enforces this, I'll check on that) and that is certainly how we will configuring our setup.

I've tested locally using a TypeList and the API doesn't seem to enforce it. But I think we are in agreement on it not being a big enough use case.

> TF_LOG_PATH=log.txt TF_LOG=debug TF_ACC=1 go test ./aws -v -timeout 120m -parallel 10 -run=TestAccAwsRoute53ResolverEndpoint_basicInbound 
=== RUN   TestAccAwsRoute53ResolverEndpoint_basicInbound
=== PAUSE TestAccAwsRoute53ResolverEndpoint_basicInbound
=== CONT  TestAccAwsRoute53ResolverEndpoint_basicInbound
EXPAND [{
  SubnetId: "subnet-031524f87f5a1f18e"
} {
  SubnetId: "subnet-031524f87f5a1f18e"
}]
FLATTEN: [map[ip_id:rni-ea0df5ff66fa430d9 subnet_id:subnet-031524f87f5a1f18e ip:10.0.8.78] map[subnet_id:subnet-031524f87f5a1f18e ip:10.0.55.95 ip_id:rni-e638b90de437
4742a]]
FLATTEN: [map[subnet_id:subnet-031524f87f5a1f18e ip:10.0.8.78 ip_id:rni-ea0df5ff66fa430d9] map[subnet_id:subnet-031524f87f5a1f18e ip:10.0.55.95 ip_id:rni-e638b90de4374
742a]]
FLATTEN: [map[subnet_id:subnet-031524f87f5a1f18e ip:10.0.8.78 ip_id:rni-ea0df5ff66fa430d9] map[subnet_id:subnet-031524f87f5a1f18e ip:10.0.55.95 ip_id:rni-e638b90de4374
742a]]
FLATTEN: [map[ip_id:rni-ea0df5ff66fa430d9 subnet_id:subnet-031524f87f5a1f18e ip:10.0.8.78] map[subnet_id:subnet-031524f87f5a1f18e ip:10.0.55.95 ip_id:rni-e638b90de4374
742a]]
--- PASS: TestAccAwsRoute53ResolverEndpoint_basicInbound (372.50s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       372.519s

ewbankkit · 2019-02-28T16:06:33Z

Agreed, my test may have not been a real test of the API as the CLI might have collapsed the duplicated subnet IDs.

kmcquade · 2019-02-28T19:41:44Z

@nywilken - definitely not the same subnet_id multiple times... two or more different subnet IDs.

nywilken

@ewbankkit thanks again for pushing this forward and for working through this PR with me. I left a few nitpicks around if/err checks but this is otherwise good to go.

nywilken · 2019-02-28T19:00:29Z

aws/resource_aws_route53_resolver_endpoint.go

+		resp, err := conn.GetResolverEndpoint(&route53resolver.GetResolverEndpointInput{
+			ResolverEndpointId: aws.String(epId),
+		})
+		if err != nil {


For readability we tend to go with the following convention:

if isAWSErr(err, route53resolver.ErrCodeResourceNotFoundException, "") { return &route53resolver.ResolverEndpoint{}, route53ResolverEndpointStatusDeleted, nil } if err != nil { return nil, "", err }

nywilken · 2019-02-28T19:02:04Z

aws/resource_aws_route53_resolver_endpoint.go

+	buf.WriteString(fmt.Sprintf("%s-", m["subnet_id"].(string)))
+	// TODO
+	// TODO * "ip" may be computed - Blank on input, set by API on output; How to handle?
+	// TODO * Multiple "ip_address" values per "subnet_id" are allowed in the API


Seems like we have debunked this via the documentation so it is safe to remove the multiple ip_address per subnet_id TODO.

nywilken · 2019-02-28T20:16:34Z

aws/resource_aws_route53_resolver_endpoint_test.go

+		_, err := conn.GetResolverEndpoint(&route53resolver.GetResolverEndpointInput{
+			ResolverEndpointId: aws.String(rs.Primary.ID),
+		})
+		if err != nil {


nywilken · 2019-02-28T20:16:51Z

aws/resource_aws_route53_resolver_endpoint_test.go

+			_, err := conn.DeleteResolverEndpoint(&route53resolver.DeleteResolverEndpointInput{
+				ResolverEndpointId: aws.String(id),
+			})
+			if err != nil {


ewbankkit · 2019-03-01T15:04:53Z

Additional code review changes made.
Acceptance test:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsRoute53ResolverEndpoint_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccAwsRoute53ResolverEndpoint_ -timeout 120m
=== RUN   TestAccAwsRoute53ResolverEndpoint_basicInbound
=== PAUSE TestAccAwsRoute53ResolverEndpoint_basicInbound
=== RUN   TestAccAwsRoute53ResolverEndpoint_updateOutbound
=== PAUSE TestAccAwsRoute53ResolverEndpoint_updateOutbound
=== CONT  TestAccAwsRoute53ResolverEndpoint_basicInbound
=== CONT  TestAccAwsRoute53ResolverEndpoint_updateOutbound
--- PASS: TestAccAwsRoute53ResolverEndpoint_basicInbound (206.00s)
--- PASS: TestAccAwsRoute53ResolverEndpoint_updateOutbound (707.33s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	707.352s

nywilken

This is great @ewbankkit! Thanks again.

ghost · 2020-03-31T17:43:19Z

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!

ewbankkit mentioned this pull request Nov 24, 2018

Support route53resolver #6525

Closed

ewbankkit force-pushed the issue-6563 branch from 350fbf4 to 65fb362 Compare November 27, 2018 22:17

ewbankkit mentioned this pull request Nov 28, 2018

Add scaffolding for new Route53 Resolver service #6554

Closed

bflad added new-resource Introduces a new resource. and removed dependencies Used to indicate dependency changes. labels Nov 30, 2018

ewbankkit force-pushed the issue-6563 branch from a2501a0 to 7d24122 Compare December 7, 2018 22:50

ewbankkit force-pushed the issue-6563 branch from 7d24122 to 038c0bf Compare December 16, 2018 23:00

gazoakley mentioned this pull request Jan 8, 2019

[WIP] r/aws_route53_resolver_rule: Add resource #6571

Closed

ewbankkit changed the title ~~[WIP] Add Route 53 Resolver endpoint resource~~ Add Route 53 Resolver endpoint resource Jan 9, 2019

ewbankkit mentioned this pull request Jan 10, 2019

Tech debt: Consistently handle overlapping resource tags when updating #7052

Closed

ewbankkit force-pushed the issue-6563 branch from f4d40c5 to 02b26ee Compare January 23, 2019 17:15

Kit Ewbank and others added 7 commits February 28, 2019 09:17

Update tag handling code to handle overlaps.

399d816

Correct 'tags' syntax for TF 0.12.

76abae8

Add sweeper for aws_route53_resolver_endpoint.

1e10736

Fix 'make lint' errors.

5615071

Remove test sweeper prefix check.

a0d408b

Changes after review - Don't export 'route53ResolverEndpointStatusDel…

a2b96e2

…eted'.

Use 'route53resolverconn'.

2c56405

ewbankkit force-pushed the issue-6563 branch from 39dbee3 to 2c56405 Compare February 28, 2019 14:50

nywilken added this to the v2.2.0 milestone Feb 28, 2019

bflad modified the milestones: v2.2.0, v2.1.0 Feb 28, 2019

nywilken reviewed Feb 28, 2019

View reviewed changes

Additional code review changes.

33ef7af

nywilken approved these changes Mar 1, 2019

View reviewed changes

nywilken merged commit 27d2204 into hashicorp:master Mar 1, 2019

ewbankkit deleted the issue-6563 branch March 1, 2019 18:44

fabiendelpierre mentioned this pull request Mar 4, 2019

Add Route 53 Resolver rule and rule association resources #7799

Merged

mikecook mentioned this pull request Mar 15, 2019

Route53 Resolver Endpoint IP addresses not being marked for change #7950

Closed

ewbankkit mentioned this pull request Mar 15, 2019

Feature request: Route 53 Resolver endpoint resource #6563

Closed

yorinasub17 mentioned this pull request Apr 17, 2019

Feature Request: aws_route53_resolver_endpoint data source #8355

Closed

ewbankkit mentioned this pull request Jun 6, 2019

Route53 Resolver Endpoint failing when specifying more than two ip addresses #7942

Closed

ghost locked and limited conversation to collaborators Mar 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Route 53 Resolver endpoint resource #6574

Add Route 53 Resolver endpoint resource #6574

ewbankkit commented Nov 24, 2018

ewbankkit commented Nov 24, 2018

gazoakley commented Nov 24, 2018

gazoakley commented Nov 25, 2018

ewbankkit commented Dec 7, 2018

ewbankkit commented Dec 16, 2018

ewbankkit commented Dec 16, 2018 •

edited

Loading

ewbankkit commented Dec 18, 2018 •

edited

Loading

rbadillo commented Jan 4, 2019

ewbankkit commented Jan 7, 2019

aar6ncai commented Jan 9, 2019

ewbankkit commented Jan 9, 2019

kmcquade commented Jan 22, 2019

ewbankkit commented Jan 23, 2019

ewbankkit commented Feb 28, 2019

nywilken commented Feb 28, 2019

ewbankkit commented Feb 28, 2019 •

edited

Loading

nywilken commented Feb 28, 2019

ewbankkit commented Feb 28, 2019

kmcquade commented Feb 28, 2019

nywilken left a comment

nywilken Feb 28, 2019

nywilken Feb 28, 2019

nywilken Feb 28, 2019

nywilken Feb 28, 2019

ewbankkit commented Mar 1, 2019

nywilken left a comment

ghost commented Mar 31, 2020

Add Route 53 Resolver endpoint resource #6574

Add Route 53 Resolver endpoint resource #6574

Conversation

ewbankkit commented Nov 24, 2018

ewbankkit commented Nov 24, 2018

gazoakley commented Nov 24, 2018

gazoakley commented Nov 25, 2018

ewbankkit commented Dec 7, 2018

ewbankkit commented Dec 16, 2018

ewbankkit commented Dec 16, 2018 • edited Loading

ewbankkit commented Dec 18, 2018 • edited Loading

rbadillo commented Jan 4, 2019

ewbankkit commented Jan 7, 2019

aar6ncai commented Jan 9, 2019

ewbankkit commented Jan 9, 2019

kmcquade commented Jan 22, 2019

ewbankkit commented Jan 23, 2019

ewbankkit commented Feb 28, 2019

nywilken commented Feb 28, 2019

ewbankkit commented Feb 28, 2019 • edited Loading

nywilken commented Feb 28, 2019

ewbankkit commented Feb 28, 2019

kmcquade commented Feb 28, 2019

nywilken left a comment

Choose a reason for hiding this comment

nywilken Feb 28, 2019

Choose a reason for hiding this comment

nywilken Feb 28, 2019

Choose a reason for hiding this comment

nywilken Feb 28, 2019

Choose a reason for hiding this comment

nywilken Feb 28, 2019

Choose a reason for hiding this comment

ewbankkit commented Mar 1, 2019

nywilken left a comment

Choose a reason for hiding this comment

ghost commented Mar 31, 2020

ewbankkit commented Dec 16, 2018 •

edited

Loading

ewbankkit commented Dec 18, 2018 •

edited

Loading

ewbankkit commented Feb 28, 2019 •

edited

Loading