Skip to content
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

resource/aws_route53_zone_association: Support resource import and recreate resource on all updates #7966

Merged
merged 3 commits into from
Mar 20, 2019

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Mar 16, 2019

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Closes #7964
Closes #2832

Changes proposed in this pull request:

  • tests/resource/aws_vpc: Add acceptance testing to verify Terraform resource recreation after external deletion
  • tests/resource/aws_route53_zone: Add acceptance testing to verify Terraform resource recreation after external deletion
  • resource/aws_route53_zone_association: Support resource import and recreate resource on all updates

Previously the resource implemented an Update function that did not perform any updates. All attributes were switched to ForceNew: true and the Update function removed.

The new TestAccAWSRoute53ZoneAssociation_disappears_VPC acceptance test explicitly verified #2832 by showing DESTROY/CREATE on the resource for properly referenced VPC that was externally deleted from Terraform.

DESTROY/CREATE: aws_route53_zone_association.foobar
  vpc_id:     "vpc-0760bf69d6ba598c6" => "<computed>" (forces new resource)
  vpc_region: "us-west-2" => "<computed>"
  zone_id:    "Z3FP49IAL6M01X" => "Z3FP49IAL6M01X"
CREATE: aws_vpc.bar
  arn:                              "" => "<computed>"
  assign_generated_ipv6_cidr_block: "" => "false"
  cidr_block:                       "" => "10.7.0.0/16" (forces new resource)
  default_network_acl_id:           "" => "<computed>"
  default_route_table_id:           "" => "<computed>"
  default_security_group_id:        "" => "<computed>"
  dhcp_options_id:                  "" => "<computed>"
  enable_classiclink:               "" => "<computed>"
  enable_classiclink_dns_support:   "" => "<computed>"
  enable_dns_hostnames:             "" => "true"
  enable_dns_support:               "" => "true"
  instance_tenancy:                 "" => "default" (forces new resource)
  ipv6_association_id:              "" => "<computed>"
  ipv6_cidr_block:                  "" => "<computed>"
  main_route_table_id:              "" => "<computed>"
  owner_id:                         "" => "<computed>"
  tags.%:                           "" => "1"
  tags.Name:                        "" => "terraform-testacc-route53-zone-association-bar"

Output from acceptance testing:

--- PASS: TestAccAWSVpc_disappears (13.48s)

--- PASS: TestAccAWSRoute53Zone_disappears (40.98s)

--- PASS: TestAccAWSRoute53ZoneAssociation_basic (114.23s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears (131.71s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears_VPC (115.04s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears_Zone (123.63s)
--- PASS: TestAccAWSRoute53ZoneAssociation_region (115.49s)

…source recreation after external deletion

Output from acceptance testing:

```
--- PASS: TestAccAWSVpc_disappears (13.48s)
```
…raform resource recreation after external deletion

Output from acceptance testing:

```
--- PASS: TestAccAWSRoute53Zone_disappears (40.98s)
```
@bflad bflad added bug Addresses a defect in current functionality. enhancement Requests to existing resources that expand the functionality or scope. service/route53 Issues and PRs that pertain to the route53 service. labels Mar 16, 2019
@bflad bflad added this to the v2.3.0 milestone Mar 16, 2019
@ghost ghost added size/XL Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. service/ec2 Issues and PRs that pertain to the ec2 service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Mar 16, 2019
…create resource on all updates

References:

* #7964
* #2832

Previously the resource implemented an `Update` function that did not perform any updates. All attributes were switched to `ForceNew: true` and the `Update` function removed.

The new `TestAccAWSRoute53ZoneAssociation_disappears_VPC` acceptance test explicitly verified #2832 by showing DESTROY/CREATE on the resource for properly referenced VPC that was externally deleted from Terraform.

```
DESTROY/CREATE: aws_route53_zone_association.foobar
  vpc_id:     "vpc-0760bf69d6ba598c6" => "<computed>" (forces new resource)
  vpc_region: "us-west-2" => "<computed>"
  zone_id:    "Z3FP49IAL6M01X" => "Z3FP49IAL6M01X"
CREATE: aws_vpc.bar
  arn:                              "" => "<computed>"
  assign_generated_ipv6_cidr_block: "" => "false"
  cidr_block:                       "" => "10.7.0.0/16" (forces new resource)
  default_network_acl_id:           "" => "<computed>"
  default_route_table_id:           "" => "<computed>"
  default_security_group_id:        "" => "<computed>"
  dhcp_options_id:                  "" => "<computed>"
  enable_classiclink:               "" => "<computed>"
  enable_classiclink_dns_support:   "" => "<computed>"
  enable_dns_hostnames:             "" => "true"
  enable_dns_support:               "" => "true"
  instance_tenancy:                 "" => "default" (forces new resource)
  ipv6_association_id:              "" => "<computed>"
  ipv6_cidr_block:                  "" => "<computed>"
  main_route_table_id:              "" => "<computed>"
  owner_id:                         "" => "<computed>"
  tags.%:                           "" => "1"
  tags.Name:                        "" => "terraform-testacc-route53-zone-association-bar"
```

Output from acceptance testing:

```
--- PASS: TestAccAWSVpc_disappears (13.48s)

--- PASS: TestAccAWSRoute53Zone_disappears (40.98s)

--- PASS: TestAccAWSRoute53ZoneAssociation_basic (114.23s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears (131.71s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears_VPC (115.04s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears_Zone (123.63s)
--- PASS: TestAccAWSRoute53ZoneAssociation_region (115.49s)
```
@bflad bflad force-pushed the f-aws_route53_zone_association-import branch from cf51de6 to 6458021 Compare March 16, 2019 02:47
@bflad bflad requested a review from a team March 18, 2019 14:59
Copy link
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@bflad bflad merged commit 288765b into master Mar 20, 2019
@bflad bflad deleted the f-aws_route53_zone_association-import branch March 20, 2019 06:43
bflad added a commit that referenced this pull request Mar 20, 2019
@bflad
Copy link
Contributor Author

bflad commented Mar 21, 2019

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

@borgstrom
Copy link
Contributor

@bflad This caused breaking changes for anyone working around missing cross-account route53 association while waiting on #2005

If you have done authorization and association across account out-of-band (i.e. using the AWS CLI) this change causes those associations to now want to be removed when you pick up the new version of the provider.

For now we're going to ensure we don't pick up the 2.3.0 version of the provider, but I'm curious your thoughts on how we can move forward? Can we get a solution to #2005, or some how change this resource to NOT remove existing associations added out of band like before?

@bflad
Copy link
Contributor Author

bflad commented Apr 11, 2019

Hi @borgstrom 👋 Thanks for the report and sorry for any trouble. Can you please file a new top level GitHub issue following the bug report template so we have all the details to appropriately triage? Thanks.

It will be helpful to get the relevant configuration and plan output to see what can be done or if any immediate workarounds can be provided. Since cross-account authorizations are not explicitly supported by the Terraform AWS Provider currently, there is no acceptance testing for this scenario. Given the additional information, we may be able to write up new testing covering this scenario to prevent what sounds like a legitimate issue in the future, even without native Terraform support for cross-account authorizations.

@borgstrom
Copy link
Contributor

Hi @bflad 👋

This actually turned out to be an internal change that was overlooked, and I was too quick to jump the gun and point fingers at your change.

Many apologies! 🙇

We're back in a good state.

Thanks for all the work on this project!

@ghost
Copy link

ghost commented Mar 30, 2020

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 ghost locked and limited conversation to collaborators Mar 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/ec2 Issues and PRs that pertain to the ec2 service. service/route53 Issues and PRs that pertain to the route53 service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support aws_route53_zone_association Resource Import Route53 Zone Association with stale VPC ID
3 participants