-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Provide Descriptive Error when Enterprise-only Paths Called in Open-source Version #18870
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.
Looks really good! I wonder if it'd be worthwhile to add some go tests for this change?
I've refactored the code a bit, so that we can add a test in the enterprise repository that will flag any discrepancies between the results of the entStubPaths and the entPaths functions. |
3eb38c3
to
0934785
Compare
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.
Still looks good 😄. I added one nit about the commented-out path, but it is not a blocker.
vault/logical_system_helpers.go
Outdated
"replication/dr/secondary/recover": {logical.UpdateOperation}, | ||
"replication/dr/secondary/reindex": {logical.UpdateOperation}, | ||
"replication/reindex": {logical.UpdateOperation}, | ||
// "replication/status": {logical.ReadOperation}, |
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.
Might be worthwhile to drop this or pull the comment from below up here and note that it will be handled separately below.
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.
I think dropping the comment is better. Without that comment the elements of the slice will all be aligned.
This PR addresses Issue #11539. It expands the entPaths slice of
framework.Path
pointers, which currently only contains a single path,replication/status
, to include all paths for enterprise-only features. This will allow the open-source version of Vault to return a more descriptive error if enterprise Vault CLI commands are sent to it.All paths related to enterprise-only features will share a common handler that returns a
logical.Response
that contains the error message enterprise-only feature along with alogical.ErrUnsupportedPath
error. From the client perspective, here's the response received from an open-source edition Vault server, using curl:And using the vault CLI:
Behaviour Change
This change includes a minor behavioural change to the handling of the existing
replication/status
path, so that requests sent to it will result in the same response as all of the other paths being introduced. Sending a read operation to this path in the open-source edition, used to result in an HTTP 200 response with the following body:After this change, this endpoint will behave as described above.