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

provider/aws: Fixed the aws_vpc enable_dns_support handling on creation #10171

Merged
merged 1 commit into from Nov 16, 2016

Conversation

Ninir
Copy link
Contributor

@Ninir Ninir commented Nov 16, 2016

Fixes #10168

This is an attempt in fixing the related issue. Still experimenting Go & Terraform codebase, be kind 😅

@mattmoyer
Copy link
Contributor

I confirmed that this does fix the buggy behavior, but I'm not sure I understand how.

It's worth noting that I don't see the buggy behavior with enable_dns_hostnames, I think because it defaults to false on new VPCs whereas enable_dns_support defaults to true.

I don't 100% understand the bug but I suspect it's related to the initial default value not being properly reflected in the initial resource data structure when it's first passed to resourceAwsVpcUpdate.

@Ninir
Copy link
Contributor Author

Ninir commented Nov 16, 2016

@mattmoyer In fact it does the following thing from the code POV:

  1. The API endpoint creates the VPC without the option (it is how the API is designed).
  2. Since there are no modifications on the given attribute, it is not written to the state.
  3. After creating the resource, it updates the resource (in order to add attributes).
  4. Since it does not have the attribute in the state, this line resolves to false, because the value has not changed.
  5. When you apply again, Terraform will read the value from AWS, and will set the retrieved value (true)
  6. Since the value is different from your config one, it will warn about the update

Hope it explains better what happens!

Copy link
Member

@catsby catsby left a comment

Choose a reason for hiding this comment

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

LGTM!

if d.HasChange("enable_dns_support") {
_, hasEnableDnsSupportOption := d.GetOk("enable_dns_support")

if !hasEnableDnsSupportOption || d.HasChange("enable_dns_support") {
Copy link
Member

Choose a reason for hiding this comment

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

seems enable_dns_support is the only attribute that defaults true on AWS side here, which is why this is the only one that needs this check (if I understand correctly)

@catsby catsby merged commit d004a24 into hashicorp:master Nov 16, 2016
@Ninir Ninir deleted the vpc_enable_dns_support branch November 16, 2016 19:44
@ghost
Copy link

ghost commented Apr 20, 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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@hashicorp hashicorp locked and limited conversation to collaborators Apr 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws_vpc: enable_dns_support not set correctly at initial creation
3 participants