Add Delete Privileges API to HLRC #35454
Conversation
|
Pinging @elastic/es-core-infra |
client/rest-high-level/src/main/java/org/elasticsearch/client/SecurityClient.java
Outdated
Show resolved
Hide resolved
|
|
||
| /** | ||
| * Asynchronously removes an application privilege | ||
| * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-delete-role.html"> |
There was a problem hiding this comment.
s/delete-role/delete-privilege
There was a problem hiding this comment.
I just saw it and pushed 85bfdc8
| public DeletePrivilegesRequest(String application, String[] privileges, RefreshPolicy refreshPolicy) { | ||
| this.application = Objects.requireNonNull(application, "application is required"); | ||
| this.privileges = Objects.requireNonNull(privileges, "privileges are required"); | ||
| this.refreshPolicy = Objects.requireNonNull(refreshPolicy, "refresh policy is required"); |
There was a problem hiding this comment.
In other APIs, we have allowed null and used RefreshPolicy.getDefault(). I don't have strong feelings for implicit vs explicit in this case but I think consistency helps. Would it make sense to make a decision and use this for all APIs? @hub-cap WDYT ?
There was a problem hiding this comment.
I made it consistent and changed the code to use the RefreshPolicy.getDefault().
We can still change this later if someone thinks it should be immediate too.
...est-high-level/src/main/java/org/elasticsearch/client/security/DeletePrivilegesResponse.java
Show resolved
Hide resolved
| public void testDeletePrivilege() throws Exception { | ||
| RestHighLevelClient client = highLevelClient(); | ||
| { | ||
| final Request createPrivilegeRequest = new Request("POST", "/_xpack/security/privilege"); |
There was a problem hiding this comment.
Mental note to possibly replcae this when Put Privilege is in place
There was a problem hiding this comment.
Yes, that's why I asked you to review this :)
...est-high-level/src/main/java/org/elasticsearch/client/security/DeletePrivilegesResponse.java
Outdated
Show resolved
Hide resolved
...est-high-level/src/main/java/org/elasticsearch/client/security/DeletePrivilegesResponse.java
Show resolved
Hide resolved
...rest-high-level/src/main/java/org/elasticsearch/client/security/DeletePrivilegesRequest.java
Outdated
Show resolved
Hide resolved
...rest-high-level/src/main/java/org/elasticsearch/client/security/DeletePrivilegesRequest.java
Outdated
Show resolved
Hide resolved
client/rest-high-level/src/main/java/org/elasticsearch/client/SecurityClient.java
Outdated
Show resolved
Hide resolved
client/rest-high-level/src/main/java/org/elasticsearch/client/SecurityClient.java
Show resolved
Hide resolved
| if (token == null) { | ||
| token = parser.nextToken(); | ||
| } | ||
| ensureExpectedToken(token, XContentParser.Token.START_OBJECT, parser::getTokenLocation); |
There was a problem hiding this comment.
Sorry I missed this earlier, I think this should be ensureExpectedToken(XContentParser.Token.START_OBJECT,token, parser::getTokenLocation)
(expected, actual, getTokenLocation or else might result in wrong error message.
...est-high-level/src/main/java/org/elasticsearch/client/security/DeletePrivilegesResponse.java
Outdated
Show resolved
Hide resolved
This commit adds the Delete Privileges API to the high level REST client. Related to elastic#29827
tlrx
force-pushed
the
add-delete-privileges-to-hlrc
branch
from
217acac
to
e33ac10
Compare
This commit adds the Delete Privileges API to the high level REST client. Related to #29827
This commit adds the Delete Privileges API to the high level REST client.
Related to #29827