-
Notifications
You must be signed in to change notification settings - Fork 7
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
A bunch of little fixes #23
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One request on the role/
API path, otherwise 👍
path_dynamic_roles.go
Outdated
@@ -18,8 +18,8 @@ import ( | |||
const ( | |||
secretCredsType = "creds" | |||
|
|||
dynamicRolePath = "role/" | |||
dynamicCredPath = "cred/" | |||
dynamicRolePath = "roles/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider reverting this back to role/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
scripts/acceptance-tests.bats
Outdated
@@ -183,11 +183,11 @@ teardown() { | |||
vault delete openldap/config | |||
|
|||
# Remove any roles that were created so they don't bleed over to other tests | |||
output=$(vault list -format=json openldap/role || true) # "or true" so it doesn't show an error if there are no roles | |||
output=$(vault list -format=json openldap/roles || true) # "or true" so it doesn't show an error if there are no roles |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'd have to revert these paths as well 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/cred
->/creds
prefix
argument from static creds endpoint (it isn't supported & was creating problems with thepath-help
command on Vault)