-
Notifications
You must be signed in to change notification settings - Fork 25k
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 updates of API key attributes [service layer] #87924
Support updates of API key attributes [service layer] #87924
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 is almost ready. I don't have major points. Other than below comments, I also felt we are a bit light on debug loggings. For example, when the version number gets updated, I think it's worth for a logging message.
...e/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateApiKeyRequest.java
Outdated
Show resolved
Hide resolved
...e/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateApiKeyRequest.java
Show resolved
Hide resolved
.../test/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateApiKeyRequestTests.java
Outdated
Show resolved
Hide resolved
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java
Outdated
Show resolved
Hide resolved
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java
Outdated
Show resolved
Hide resolved
...ty/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java
Show resolved
Hide resolved
...ty/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java
Outdated
Show resolved
Hide resolved
...ty/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java
Outdated
Show resolved
Hide resolved
...ty/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java
Outdated
Show resolved
Hide resolved
...ty/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java
Outdated
Show resolved
Hide resolved
Build failure is unrelated and tracked here |
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
Thanks for the iterations!
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java
Outdated
Show resolved
Hide resolved
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java
Outdated
Show resolved
Hide resolved
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java
Outdated
Show resolved
Hide resolved
@elasticmachine run elasticsearch-ci/part-1-fips |
Service level implementation to add support for updating attributes of
existing API keys. This allows end-users to modify privileges and
metadata associated with API keys dynamically, without requiring
rolling out new API keys every time there is a change.
Updatable attributes are
role_descriptors
andmetadata
. Severalother attributes are updated automatically, on every update call,
including
limited_by_role_descriptors
,creator
, andversion
. APIkey attributes are replaced, not merged.
On every update, the API key doc cache is cleared for the updated API
key.
This PR implements the necessary service layer changes in
ApiKeyService
. I will integrate this with the REST and transportlayers in a subsequent PR.
Relates: #87870
Note: labeling
>non-issue
since I would rather include a>feature
tag and changelog entry on the REST & transport layer PR.