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 #128: vault_aws_auth_backend_role.resolve_aws_unique_ids setting not detected #129

Closed
wants to merge 1 commit into from
Closed

Fix #128: vault_aws_auth_backend_role.resolve_aws_unique_ids setting not detected #129

wants to merge 1 commit into from

Conversation

daveadams
Copy link

Per the schema.ResourceData docs[0], GetOkExists is needed for properly detecting whether optional boolean values have been set.

[0] https://godoc.org/github.com/hashicorp/terraform/helper/schema#ResourceData.GetOkExists

Per the schema.ResourceData docs[0], GetOkExists is needed for properly
detecting whether optional boolean values have been set.

refs #128

[0] https://godoc.org/github.com/hashicorp/terraform/helper/schema#ResourceData.GetOkExists
@vancluever
Copy link

@daveadams thanks for this!

The only issue with GetOkExists currently is that it actually does not work beyond the first apply. You can find detail here - the gist being that while the first apply will work as intended, the moment the value hits state, "exists" will always be true, regardless of what is in config at any point in time.

I apologize for the misleading docs. 😕

I'm noodling on whether or not this is good to add given that it gets us at least part of the way there or if it might be better to do it a different way. I also have a workaround that employs using strings for boolean values instead, but let me check with some of my colleagues to see if this is something that would be worthwhile recommending (especially since later versions of Terraform will be addressing the lack of nils completely).

Thanks!

@tyrannosaurus-becks
Copy link
Contributor

Hi @vancluever ! Thanks for pointing that out! Was a field ever added that would be more appropriate?

@tyrannosaurus-becks tyrannosaurus-becks self-assigned this Sep 14, 2018
@tyrannosaurus-becks
Copy link
Contributor

In this case, I think it would be better to default these values to the same defaults as found in Vault. Thank you for surfacing this! This PR seems inactive so I'm going to close it, and do a separate PR defaulting them.

@tyrannosaurus-becks
Copy link
Contributor

#222

@StyleT
Copy link

StyleT commented Dec 4, 2018

@tyrannosaurus-becks it's definitely doesn't solve an issue with resolve_aws_unique_ids due to hashicorp/terraform#4936.
Now it's impossible to create a role with resolve_aws_unique_ids = false as d.GetOk("resolve_aws_unique_ids") will return false in this case and default value will be used which is true :( Can you pls work on it asap as it's really a blocker for us...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants