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

[RM-1480] Suppress InvalidVpcID.NotFound on describe VPC classic link calls #5

Merged
merged 1 commit into from
Feb 21, 2019

Conversation

curtis-fugue
Copy link

Why

Some VPCs incorrectly fail to refresh due to InvalidVpcID.NotFound.

What

In some research I found a potential cause that cropped up for other people: describe VPC classic link calls can fail with this:

ansible/ansible#36083

This PR suppresses the error on DescribeVpcClassicLink and DescribeVpcClassicLinkDnsSupport which are made to set the optional attributes enable_classiclink and enable_classiclink_dns_support.

https://www.terraform.io/docs/providers/aws/r/vpc.html#enable_classiclink

The provider already suppresses UnsupportedOperation for both of these calls, but my suspicion is AWS may also return VPC not found in certain cases.

Copy link

@grim-luminal grim-luminal left a comment

Choose a reason for hiding this comment

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

LGTM

@curtis-fugue
Copy link
Author

Update: the VPC that triggers the issue is a shared VPC which does not support classic link. I've reproduced the error and confirmed this change fixes it.

@curtis-fugue curtis-fugue merged commit cdd346c into fugue Feb 21, 2019
@curtis-fugue curtis-fugue deleted the bug/RM-1480/vpc-not-found branch February 21, 2019 18:16
@tyler-luminal
Copy link

Raising a flag here that this may cause future conflicts with the Terraform provider. They added support for VPC Sharing in December (hashicorp#6642). It doesn't look like they've addressed the issue we're seeing.

mike-luminal pushed a commit that referenced this pull request Dec 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants