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

Fix aws dx gateway association proposal #19741

Conversation

dkujawski
Copy link
Contributor

@dkujawski dkujawski commented Jun 9, 2021

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" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #18608.

Output from acceptance testing:

$> make testacc TESTARGS='\-run=TestAccAwsDxGatewayAssociationProposal_endOfLife'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 \-run=TestAccAwsDxGatewayAssociationProposal_endOfLife -timeout 180m
=== RUN   TestAccAwsDxGatewayAssociationProposal_endOfLife
=== PAUSE TestAccAwsDxGatewayAssociationProposal_endOfLife
=== CONT  TestAccAwsDxGatewayAssociationProposal_endOfLife
--- PASS: TestAccAwsDxGatewayAssociationProposal_endOfLife (1941.31s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       1941.405s

Currently the aws_dx_gateway_association_proposal automatically gets deleted by AWS some time after the proposal is accepted by the dx gateway owner. Terraform sees this and wants to re-create the proposal, cascading into a proposal id change on the accepter.

The fix here is to:

  1. In the resource Read function, if the proposal is not found by ID, check for the association existing and use the metadata from that as the current state
  2. Create a customized Import function that allows passing the proposal ID, target gateway, and dx gateway. If the proposal ID doesn’t exist TF will still see it as existing based on the gateway IDs

Pros:

  • Proposal resource now shares a lifespan with the association resource as far as TF is concerned – this is basically the behavior that we’d expect if AWS wasn’t deleting the proposals
  • Can import “placeholder” proposals – useful for when you’re importing a cross-account DX gateway association that wasn’t created directly in TF
  • Pretty easy update

Cons:

  • The actual state of the proposal resource is now indeterminate, which seems like a pretty big departure from how TF is supposed to work, and probably the reason the existing PRs haven’t been merged
  • Resource may show pending updates but not be able to apply them

@github-actions github-actions bot added needs-triage Waiting for first response or review from a maintainer. service/directconnect Issues and PRs that pertain to the directconnect service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. size/L Managed by automation to categorize the size of a PR. labels Jun 9, 2021
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome @dkujawski 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

@github-actions github-actions bot added service/directconnect Issues and PRs that pertain to the directconnect service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Jun 10, 2021
@dkujawski dkujawski marked this pull request as ready for review June 21, 2021 22:19
@ewbankkit
Copy link
Contributor

@github-actions github-actions bot added size/XL Managed by automation to categorize the size of a PR. and removed size/L Managed by automation to categorize the size of a PR. labels Jun 23, 2021
@ewbankkit ewbankkit removed the needs-triage Waiting for first response or review from a maintainer. label Jun 23, 2021
@ewbankkit ewbankkit self-assigned this Jun 23, 2021
dkujawski and others added 15 commits July 7, 2021 13:26
Once the proposal has been accepted and deleted, locate the association,
populate the proposal state to prevent recreating the proposal
unnecessarily.
accept an import string composed of the proposal id, dx gateway id, and
target associated gateway id when the proposal has been removed by AWS
due to it being accepted.
move proposal existance logic out of custom diff function and over into
the Read function.

add more debug logs to track when proposal id values exist
@ewbankkit ewbankkit force-pushed the dk-FCC-175-fix-aws_dx_gateway_association_proposal-2 branch from 2e480e7 to e75ba1b Compare July 12, 2021 13:19
@github-actions github-actions bot added the documentation Introduces or discusses updates to documentation. label Jul 12, 2021
Copy link
Contributor

@ewbankkit ewbankkit left a comment

Choose a reason for hiding this comment

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

LGTM 🚀.

% make testacc TEST=./aws TESTARGS='-run=TestAccAwsDxGateway_'     
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAwsDxGateway_ -timeout 180m
=== RUN   TestAccAwsDxGateway_basic
=== PAUSE TestAccAwsDxGateway_basic
=== RUN   TestAccAwsDxGateway_disappears
=== PAUSE TestAccAwsDxGateway_disappears
=== RUN   TestAccAwsDxGateway_complex
=== PAUSE TestAccAwsDxGateway_complex
=== CONT  TestAccAwsDxGateway_basic
=== CONT  TestAccAwsDxGateway_complex
=== CONT  TestAccAwsDxGateway_disappears
--- PASS: TestAccAwsDxGateway_disappears (22.54s)
--- PASS: TestAccAwsDxGateway_basic (23.22s)
--- PASS: TestAccAwsDxGateway_complex (761.57s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	764.824s
% ACCTEST_PARALLELISM=2 make testacc TEST=./aws TESTARGS='-run=TestAccAwsDxGatewayAssociation_\|TestAccAwsDxGatewayAssociationProposal_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 2 -run=TestAccAwsDxGatewayAssociation_\|TestAccAwsDxGatewayAssociationProposal_ -timeout 180m
=== RUN   TestAccAwsDxGatewayAssociationProposal_basicVpnGateway
=== PAUSE TestAccAwsDxGatewayAssociationProposal_basicVpnGateway
=== RUN   TestAccAwsDxGatewayAssociationProposal_basicTransitGateway
=== PAUSE TestAccAwsDxGatewayAssociationProposal_basicTransitGateway
=== RUN   TestAccAwsDxGatewayAssociationProposal_disappears
=== PAUSE TestAccAwsDxGatewayAssociationProposal_disappears
=== RUN   TestAccAwsDxGatewayAssociationProposal_endOfLifeVpn
=== PAUSE TestAccAwsDxGatewayAssociationProposal_endOfLifeVpn
=== RUN   TestAccAwsDxGatewayAssociationProposal_endOfLifeTgw
=== PAUSE TestAccAwsDxGatewayAssociationProposal_endOfLifeTgw
=== RUN   TestAccAwsDxGatewayAssociationProposal_AllowedPrefixes
=== PAUSE TestAccAwsDxGatewayAssociationProposal_AllowedPrefixes
=== RUN   TestAccAwsDxGatewayAssociation_V0StateUpgrade
=== PAUSE TestAccAwsDxGatewayAssociation_V0StateUpgrade
=== RUN   TestAccAwsDxGatewayAssociation_basicVpnGatewaySingleAccount
=== PAUSE TestAccAwsDxGatewayAssociation_basicVpnGatewaySingleAccount
=== RUN   TestAccAwsDxGatewayAssociation_basicVpnGatewayCrossAccount
=== PAUSE TestAccAwsDxGatewayAssociation_basicVpnGatewayCrossAccount
=== RUN   TestAccAwsDxGatewayAssociation_basicTransitGatewaySingleAccount
=== PAUSE TestAccAwsDxGatewayAssociation_basicTransitGatewaySingleAccount
=== RUN   TestAccAwsDxGatewayAssociation_basicTransitGatewayCrossAccount
=== PAUSE TestAccAwsDxGatewayAssociation_basicTransitGatewayCrossAccount
=== RUN   TestAccAwsDxGatewayAssociation_multiVpnGatewaysSingleAccount
=== PAUSE TestAccAwsDxGatewayAssociation_multiVpnGatewaysSingleAccount
=== RUN   TestAccAwsDxGatewayAssociation_allowedPrefixesVpnGatewaySingleAccount
=== PAUSE TestAccAwsDxGatewayAssociation_allowedPrefixesVpnGatewaySingleAccount
=== RUN   TestAccAwsDxGatewayAssociation_allowedPrefixesVpnGatewayCrossAccount
=== PAUSE TestAccAwsDxGatewayAssociation_allowedPrefixesVpnGatewayCrossAccount
=== RUN   TestAccAwsDxGatewayAssociation_recreateProposal
=== PAUSE TestAccAwsDxGatewayAssociation_recreateProposal
=== CONT  TestAccAwsDxGatewayAssociationProposal_basicVpnGateway
=== CONT  TestAccAwsDxGatewayAssociation_basicVpnGatewayCrossAccount
--- PASS: TestAccAwsDxGatewayAssociationProposal_basicVpnGateway (57.20s)
=== CONT  TestAccAwsDxGatewayAssociation_allowedPrefixesVpnGatewaySingleAccount
--- PASS: TestAccAwsDxGatewayAssociation_allowedPrefixesVpnGatewaySingleAccount (1578.84s)
=== CONT  TestAccAwsDxGatewayAssociation_recreateProposal
--- PASS: TestAccAwsDxGatewayAssociation_basicVpnGatewayCrossAccount (1755.63s)
=== CONT  TestAccAwsDxGatewayAssociation_allowedPrefixesVpnGatewayCrossAccount
=== CONT  TestAccAwsDxGatewayAssociation_recreateProposal
--- PASS: TestAccAwsDxGatewayAssociation_recreateProposal (2071.69s)
=== CONT  TestAccAwsDxGatewayAssociation_basicTransitGatewayCrossAccount
--- PASS: TestAccAwsDxGatewayAssociation_allowedPrefixesVpnGatewayCrossAccount (1510.18s)
=== CONT  TestAccAwsDxGatewayAssociation_multiVpnGatewaysSingleAccount
--- PASS: TestAccAwsDxGatewayAssociation_basicTransitGatewayCrossAccount (1271.75s)
=== CONT  TestAccAwsDxGatewayAssociationProposal_endOfLifeTgw
--- PASS: TestAccAwsDxGatewayAssociation_multiVpnGatewaysSingleAccount (1152.73s)
=== CONT  TestAccAwsDxGatewayAssociation_basicVpnGatewaySingleAccount
--- PASS: TestAccAwsDxGatewayAssociationProposal_endOfLifeTgw (1179.68s)
=== CONT  TestAccAwsDxGatewayAssociation_V0StateUpgrade
--- PASS: TestAccAwsDxGatewayAssociation_basicVpnGatewaySingleAccount (1327.53s)
=== CONT  TestAccAwsDxGatewayAssociationProposal_AllowedPrefixes
--- PASS: TestAccAwsDxGatewayAssociationProposal_AllowedPrefixes (86.29s)
=== CONT  TestAccAwsDxGatewayAssociationProposal_disappears
--- PASS: TestAccAwsDxGatewayAssociationProposal_disappears (55.90s)
=== CONT  TestAccAwsDxGatewayAssociationProposal_endOfLifeVpn
--- PASS: TestAccAwsDxGatewayAssociation_V0StateUpgrade (998.55s)
=== CONT  TestAccAwsDxGatewayAssociation_basicTransitGatewaySingleAccount
--- PASS: TestAccAwsDxGatewayAssociationProposal_endOfLifeVpn (1183.03s)
=== CONT  TestAccAwsDxGatewayAssociationProposal_basicTransitGateway
--- PASS: TestAccAwsDxGatewayAssociation_basicTransitGatewaySingleAccount (1160.12s)
--- PASS: TestAccAwsDxGatewayAssociationProposal_basicTransitGateway (207.98s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	7282.567s

@ewbankkit
Copy link
Contributor

Verified on a test Association Proposal that was EOLed by AWS.

After initial terraform apply
% aws --region us-west-2 directconnect describe-direct-connect-gateway-association-proposals --proposal-id 8e0af368-25b3-45b6-a551-450dd926ad44
{
    "directConnectGatewayAssociationProposals": [
        {
            "proposalId": "8e0af368-25b3-45b6-a551-450dd926ad44",
            "directConnectGatewayId": "7d9db843-d600-44db-8f4e-617a1dc21c67",
            "directConnectGatewayOwnerAccount": "123456789012",
            "proposalState": "accepted",
            "associatedGateway": {
                "id": "vgw-07699f257cc1eebc8",
                "type": "virtualPrivateGateway",
                "ownerAccount": "234567890123",
                "region": "us-west-2"
            },
            "existingAllowedPrefixesToDirectConnectGateway": [
                {
                    "cidr": "10.255.255.0/28"
                }
            ],
            "requestedAllowedPrefixesToDirectConnectGateway": [
                {
                    "cidr": "10.255.255.0/28"
                }
            ]
        }
    ]
}
Wait 4 days
% aws --region us-west-2 directconnect describe-direct-connect-gateway-association-proposals --proposal-id 8e0af368-25b3-45b6-a551-450dd926ad44
{
    "directConnectGatewayAssociationProposals": []
}
Run terraform plan
% 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.

data.aws_caller_identity.creator: Refreshing state...
aws_vpn_gateway.test: Refreshing state... [id=vgw-07699f257cc1eebc8]
aws_vpc.test: Refreshing state... [id=vpc-0279ccbb6e8392569]
aws_dx_gateway.test: Refreshing state... [id=7d9db843-d600-44db-8f4e-617a1dc21c67]
aws_vpn_gateway_attachment.test: Refreshing state... [id=vpn-attachment-cf62e598]
aws_dx_gateway_association_proposal.test: Refreshing state... [id=8e0af368-25b3-45b6-a551-450dd926ad44]
aws_dx_gateway_association.test: Refreshing state... [id=ga-7d9db843-d600-44db-8f4e-617a1dc21c67vgw-07699f257cc1eebc8]

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

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.

@ewbankkit ewbankkit merged commit 6dc2d6f into hashicorp:main Jul 12, 2021
@github-actions github-actions bot added this to the v3.50.0 milestone Jul 12, 2021
@github-actions
Copy link

This functionality has been released in v3.50.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!

@github-actions
Copy link

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. service/directconnect Issues and PRs that pertain to the directconnect 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.

aws_dx_gateway_association_proposal still recreated
2 participants