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
ISPN-12868 Configure ACL cache #9165
ISPN-12868 Configure ACL cache #9165
Conversation
*/ | ||
void flushGlobalACLCache(); | ||
CompletionStage<Void> flushGlobalACLCache(); |
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 public API, so we should deprecate rather than change it?
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.
Not really: the only way to obtain a GlobalSecurityManager is through the component registry. I wouldn't worry about it
.create(); | ||
} | ||
|
||
private CompletionStage<RestResponse> aclCacheFlush(RestRequest request) { | ||
EmbeddedCacheManager cm = invocationHelper.getRestCacheManager().getInstance(); | ||
return SecurityActions.getGlobalComponentRegistry(cm).getComponent(GlobalSecurityManager.class).flushGlobalACLCache() |
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.
You can cache the GlobalSecurityManager
instead of looking it up on every request I think
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.
Will do
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.
Rethinking about it: this is such a rare operation, that caching the GSM would be wasteful
@@ -80,9 +82,18 @@ public Invocations getInvocations() { | |||
.invocation().methods(PUT).path("/v2/security/roles/{principal}").withAction("deny") | |||
.permission(AuthorizationPermission.ADMIN).name("ROLES DENY").auditContext(AuditContext.SERVER) | |||
.handleWith(this::deny) | |||
.invocation().methods(POST).path("/v2/security/cache").withAction("flush") | |||
.permission(AuthorizationPermission.ADMIN).name("ACL CACHE FLUSH").auditContext(AuditContext.SERVER) | |||
.handleWith(this::aclCacheFlush) |
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.
Not strictly related to this PR, but well, let's not waste the context :)
Could add an indent or a line break between each invocation declaration to make it easier to read?
return SecurityActions.getGlobalComponentRegistry(cm).getComponent(GlobalSecurityManager.class).flushGlobalACLCache() | ||
.thenApply(v -> new NettyRestResponse.Builder().status(NO_CONTENT).build()); | ||
} | ||
|
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 couldn't find any documentation for the SecurityResource
, could you add it? It could be in another PR
65e0569
to
1ea1d60
Compare
@tristantarrant FYI I pushed some suggested doc for the ACL cache. |
c8f0b6b
to
ee61fc2
Compare
I've squashed the commits and added docs for the public security resource methods (I'd rather not document the methods which are of exclusive use to the console) |
ee61fc2
to
147f884
Compare
Thanks @tristantarrant |
https://issues.redhat.com/browse/ISPN-12868