-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
add flag to allow /operator/keyring requests to only hit local servers #6279
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.
This looks really good. I have a few requests regarding code style, test framework usage and some error handling but nothing major.
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 was wondering if we want to provide the same functionality on the cli command consul keyring -list
as well.
@i0rek
I'll leave this to @mkeeler to answer! Happy to add it in. |
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.
@i0rek brings up a good point. This PR should include changes for both the API module in api/operator_keyring.go and for the CLI command in command/keyring/keyring.go
…e instead of writing headers/response directly
@mkeeler @i0rek this is ready for another review |
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.
LGTM
Add parameter
local-only
to operator keyring list requests to force queries to only hit local servers (no WAN traffic).HTTP API:
GET /operator/keyring?local-only=true
CLI:
consul keyring -list --local-only
Sending the
local-only
flag with any non-GET
/list
request will result in an error.