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

Support custom renewal statements in Postgres #2788

Merged

Conversation

ConstantineXVI
Copy link
Contributor

This is to support a quirk of Redshift that prevents the standard ALTER ROLE from working correctly.

(Related: I want to add the option of adding caps to the generated passwords to meet Redshift's password requirements; but not immediately obvious where to make that configurable)

@calvn
Copy link
Contributor

calvn commented Jun 1, 2017

I think you can do something similar to https://github.com/hashicorp/vault/blob/master/plugins/database/mysql/mysql.go#L165 and have a const defaultRenewUser instead having the two methods customRenewUser and defaultRenewUser to reduce code duplication.

@ConstantineXVI
Copy link
Contributor Author

Valid. Rewritten to eliminate the split.

@calvn calvn requested review from calvn and briankassouf June 1, 2017 19:49
Copy link
Contributor

@calvn calvn left a comment

Choose a reason for hiding this comment

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

LGTM!

@ConstantineXVI
Copy link
Contributor Author

I think I inherited those Travis failures from master when I forked. At least I'm hoping I didn't somehow break http from my changes.

Copy link
Contributor

@briankassouf briankassouf left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks for the contribution!

@briankassouf briankassouf merged commit d004ad7 into hashicorp:master Jun 1, 2017
@briankassouf
Copy link
Contributor

@ConstantineXVI As for the password requirements. You could so something similar to how mysql has multiple implementations for different versions of mysql each with their own username requirements. See https://github.com/hashicorp/vault/blob/master/helper/builtinplugins/builtin.go#L17 for an example

@briankassouf briankassouf mentioned this pull request Jun 6, 2017
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.

3 participants