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

Check ErrPluginStaticUnsupported for fallback to RotateRootCredentials #11585

Merged
merged 1 commit into from
May 12, 2021

Conversation

pcman312
Copy link
Contributor

When using the V4 DB engine and the user wants to rotate the root credentials, the SetCredentials call is attempted. If SetCredentials comes back with an "unimplemented" error, the DB engine should fall back to running RotateRootCredentials. Previously, it was making the check to determine if it should fall back by checking for an Unimplemented gRPC status code. This is never being hit because ErrPluginStaticUnsupported is returned instead: https://github.com/hashicorp/vault/blob/master/sdk/database/dbplugin/grpc_transport.go#L341-L345

This PR fixes this by adding ErrPluginStaticUnsupported as an additional check to determine if it should fall back to RotateRootCredentials.

@pcman312 pcman312 added bug Used to indicate a potential bug secret/database labels May 11, 2021
Copy link
Contributor

@austingebauer austingebauer left a comment

Choose a reason for hiding this comment

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

This looks good to me! I think we'll want to update the conditional here as well? rollback.go#L107

I remember hitting codes.Unimplemented for SetCredentials when I tested this a while ago. Interested in knowing what (if anything at all) changed. Thanks for doing this 👍

@calvn calvn added this to the 1.6.5 milestone May 11, 2021
@pcman312 pcman312 merged commit 67ca3be into master May 12, 2021
@pcman312 pcman312 deleted the fix-v4-root-fallback branch May 12, 2021 21:22
@pcman312
Copy link
Contributor Author

@austingebauer and I talked about this offline, but we think this issue came up because the plugin in question was an external plugin. The external plugin goes through the gRPC layer which was converting the gRPC status to the ErrPluginStaticUnsupported type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to indicate a potential bug secret/database
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants