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

Update bindpass in vault_ldap_auth_backend #184

Merged
merged 2 commits into from
Sep 25, 2018

Conversation

lawliet89
Copy link
Contributor

  • Set the attribute to sensitive
  • Do not read back from the API since the API does not return it

I am not sure if this is the right way to handle it. I tried to mimic how this is done in the vault_aws_secret_backend resource.

@ghost ghost added the size/XS label Sep 20, 2018
Copy link
Contributor

@tyrannosaurus-becks tyrannosaurus-becks left a comment

Choose a reason for hiding this comment

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

@lawliet89 great catch, thanks for contributing this!

I'd wager that this change would cause the tests to fail because of how the field is checked here. Would you be willing to take a look at that?

Otherwise, it does look correct to me, and I'd be happy to merge it as soon as the related tests are confirmed as passing.

@tyrannosaurus-becks tyrannosaurus-becks self-assigned this Sep 24, 2018
- Set the attribute to sensitive
- Do not read back from the API since the API does not return it
@ghost ghost added size/XS size/S and removed size/XS labels Sep 25, 2018
@lawliet89
Copy link
Contributor Author

lawliet89 commented Sep 25, 2018

Rebased from master

Added bindpass to the fixture resource created and added an extra test to check that it's not returned by Vault. Please see if this is OK.

@ghost ghost added the size/S label Sep 25, 2018
@ghost ghost added the size/S label Sep 25, 2018
@ghost ghost added the size/S label Sep 25, 2018
@ghost ghost added size/XS and removed size/S labels Sep 25, 2018
Copy link
Contributor

@tyrannosaurus-becks tyrannosaurus-becks left a comment

Choose a reason for hiding this comment

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

@lawliet89 this looks fantastic. Thank you for doing this.

@tyrannosaurus-becks tyrannosaurus-becks merged commit 8c57133 into hashicorp:master Sep 25, 2018
@lawliet89 lawliet89 deleted the ldap-auth-bindpass branch September 25, 2018 23:34
dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
Update `bindpass` in `vault_ldap_auth_backend`
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.

2 participants