Clear REST user cache when invalidating user cache#17250
Closed
LJW21-02 wants to merge 3 commits intoapache:masterapache/iotdb:masterfrom
LJW21-02:feat_clear_rest_user_cacheLJW21-02/iotdb:feat_clear_rest_user_cacheCopy head branch name to clipboard
Closed
Clear REST user cache when invalidating user cache#17250LJW21-02 wants to merge 3 commits intoapache:masterapache/iotdb:masterfrom LJW21-02:feat_clear_rest_user_cacheLJW21-02/iotdb:feat_clear_rest_user_cacheCopy head branch name to clipboard
LJW21-02 wants to merge 3 commits intoapache:masterapache/iotdb:masterfrom
LJW21-02:feat_clear_rest_user_cacheLJW21-02/iotdb:feat_clear_rest_user_cacheCopy head branch name to clipboard
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR wires user/role permission cache invalidation into the external-service subsystem so that running external services (notably REST) can clear per-user authentication caches when permissions change.
Changes:
- Adds a
clearUserCache(String userName)hook to the external service API (IExternalService) and implements it in REST (no-op in MQTT). - Introduces
ExternalServiceManagementService.clearServiceUserCache(...)and calls it fromAuthorityChecker.invalidateCache(...). - Adds username-based eviction support to the REST
UserCache.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/service/externalservice/ExternalServiceManagementService.java | Adds a method to propagate per-user cache clearing to running external services. |
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/auth/AuthorityChecker.java | Triggers external-service user cache clearing during auth cache invalidation. |
| iotdb-api/external-service-api/src/main/java/org/apache/iotdb/externalservice/api/IExternalService.java | Extends the external service API with a per-user cache clearing method. |
| external-service-impl/rest/src/main/java/org/apache/iotdb/rest/protocol/filter/UserCache.java | Adds logic to evict cached REST auth entries for a given username. |
| external-service-impl/rest/src/main/java/org/apache/iotdb/rest/RestService.java | Implements the new external-service cache clearing hook for REST. |
| external-service-impl/mqtt/src/main/java/org/apache/iotdb/mqtt/MQTTService.java | Implements the new API method as a no-op for MQTT. |
Comments suppressed due to low confidence (1)
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/auth/AuthorityChecker.java:135
- PR description mentions invoking REST cache invalidation via reflection and updating
IoTDBRestServiceDescriptorto considerCONFIGNODE_HOME, but the current changes callExternalServiceManagementServicedirectly andIoTDBRestServiceDescriptordoes not appear to referenceCONFIGNODE_HOME. Please update the PR description (or include the missing code changes) so reviewers/operators have an accurate picture of what is being shipped.
public static boolean invalidateCache(String username, String roleName) {
PipeInsertionDataNodeListener.getInstance().invalidateAllCache();
ExternalServiceManagementService.getInstance().clearServiceUserCache(username);
return authorityFetcher.get().getAuthorCache().invalidateCache(username, roleName);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.../main/java/org/apache/iotdb/db/service/externalservice/ExternalServiceManagementService.java
Show resolved
Hide resolved
.../main/java/org/apache/iotdb/db/service/externalservice/ExternalServiceManagementService.java
Show resolved
Hide resolved
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/auth/AuthorityChecker.java
Show resolved
Hide resolved
...xternal-service-api/src/main/java/org/apache/iotdb/externalservice/api/IExternalService.java
Show resolved
Hide resolved
external-service-impl/rest/src/main/java/org/apache/iotdb/rest/protocol/filter/UserCache.java
Show resolved
Hide resolved
Contributor
|
Fixed in #17321 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a mechanism to clear cached user authentication data in the REST external service when a user's permissions are invalidated, ensuring that permission changes take effect immediately across all service endpoints. It also improves configuration file resolution by considering an additional environment variable. The most important changes are grouped below:
User Cache Invalidation Integration:
clearUserCache(String userName)toRestServiceto allow clearing a specific user's cache from outside the REST service.clearUserCache(String userName)inUserCacheto remove cached entries for the specified user.AuthorityChecker.invalidateCacheto invoke REST user cache invalidation by dynamically locating and calling theclearUserCachemethod via reflection, if the REST service is running. This ensures that user permission changes are immediately reflected in the REST service.AuthorityChecker. [1] [2]Configuration Resolution Enhancement:
IoTDBRestServiceDescriptorto check for theCONFIGNODE_HOMEenvironment variable when resolving configuration file locations, improving flexibility in deployment environments. [1] [2] [3]