View API and Rollback API for Versioned Security Configurations#5366
Closed
divyanshi-0402 wants to merge 4 commits intoopensearch-project:mainfrom
Closed
View API and Rollback API for Versioned Security Configurations#5366divyanshi-0402 wants to merge 4 commits intoopensearch-project:mainfrom
divyanshi-0402 wants to merge 4 commits intoopensearch-project:mainfrom
Conversation
added 3 commits
May 22, 2025 02:01
Signed-off-by: Divyanshi Chouksey <divvv@amazon.com>
Signed-off-by: Divyanshi Chouksey <divvv@amazon.com>
Signed-off-by: Divyanshi Chouksey <divvv@amazon.com>
Signed-off-by: Divyanshi Chouksey <77962346+divyanshi-0402@users.noreply.github.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5366 +/- ##
==========================================
+ Coverage 72.18% 72.24% +0.06%
==========================================
Files 382 388 +6
Lines 23628 24185 +557
Branches 3629 3681 +52
==========================================
+ Hits 17055 17473 +418
- Misses 4789 4901 +112
- Partials 1784 1811 +27
🚀 New features to boost your workflow:
|
5 tasks
| SSL; | ||
| SSL, | ||
| VIEW_VERSION, | ||
| ROLLBACK_VERSION; |
Member
There was a problem hiding this comment.
These two should be grouped under VERSION.
Contributor
There was a problem hiding this comment.
Do we need changes here to build endpoint permissions? https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/dlic/rest/api/RestApiAdminPrivilegesEvaluator.java#L63-L74
| /** | ||
| * REST endpoint: | ||
| * POST /_plugins/_security/api/rollback | ||
| * POST /_plugins/_security/api/rollback/version/{versionID} |
Member
There was a problem hiding this comment.
the endpoints should be prefixed as /_plugins/_security/api/version/rollback to follow correct verbiage.
| */ | ||
| protected void consumeParameters(final RestRequest request) { | ||
| request.param("name"); | ||
| request.param("versionID"); |
Member
There was a problem hiding this comment.
move this to the view and rollback API actions and over-ride consumeParams there to consume versionID
|
|
||
| private ValidationResult<SecurityConfiguration> rollbackToPreviousVersion() throws IOException { | ||
| SecurityConfigVersionDocument doc = versionsLoader.loadFullDocument(); | ||
| SecurityConfigVersionsLoader.sortVersionsById(doc.getVersions()); |
Member
There was a problem hiding this comment.
sorting should be done post validation of the size
Comment on lines
+131
to
+202
| private ValidationResult<SecurityConfiguration> handlePostRequest(RestRequest request) throws IOException { | ||
| String versionParam = request.param("versionID"); | ||
| try { | ||
| if (versionParam == null) { | ||
| return rollbackToPreviousVersion(); | ||
| } else { | ||
| return rollbackToSpecificVersion(versionParam); | ||
| } | ||
| } catch (Exception e) { | ||
| log.error("Rollback request failed", e); | ||
| return ValidationResult.error(INTERNAL_SERVER_ERROR, payload(INTERNAL_SERVER_ERROR, e.getMessage())); | ||
| } | ||
| } | ||
|
|
||
| private ValidationResult<SecurityConfiguration> rollbackToPreviousVersion() throws IOException { | ||
| SecurityConfigVersionDocument doc = versionsLoader.loadFullDocument(); | ||
| SecurityConfigVersionsLoader.sortVersionsById(doc.getVersions()); | ||
| var versions = doc.getVersions(); | ||
|
|
||
| if (versions.size() < 2) { | ||
| return ValidationResult.error(NOT_FOUND, payload(NOT_FOUND, "No previous version available to rollback")); | ||
| } | ||
|
|
||
| String previousVersionId = versions.get(versions.size() - 2).getVersion_id(); | ||
| return rollbackCommon(previousVersionId, doc); | ||
| } | ||
|
|
||
| private ValidationResult<SecurityConfiguration> rollbackToSpecificVersion(String versionId) throws IOException { | ||
| SecurityConfigVersionDocument doc = versionsLoader.loadFullDocument(); | ||
| SecurityConfigVersionsLoader.sortVersionsById(doc.getVersions()); | ||
|
|
||
| var maybeVer = doc.getVersions().stream().filter(v -> versionId.equals(v.getVersion_id())).findFirst(); | ||
|
|
||
| if (maybeVer.isEmpty()) { | ||
| return ValidationResult.error(NOT_FOUND, payload(NOT_FOUND, "Version " + versionId + " not found")); | ||
| } | ||
|
|
||
| return rollbackCommon(versionId, doc); | ||
| } | ||
|
|
||
| private ValidationResult<SecurityConfiguration> rollbackCommon(String versionId, SecurityConfigVersionDocument doc) throws IOException { | ||
| SecurityConfigVersionsLoader.sortVersionsById(doc.getVersions()); | ||
| var maybeVer = doc.getVersions().stream().filter(v -> versionId.equals(v.getVersion_id())).findFirst().orElse(null); | ||
|
|
||
| if (maybeVer == null) { | ||
| return ValidationResult.error(NOT_FOUND, payload(NOT_FOUND, "Version " + versionId + " not found")); | ||
| } | ||
|
|
||
| try { | ||
|
|
||
| rollbackConfigsToSecurityIndex(maybeVer); | ||
|
|
||
| ThreadContext threadContext = threadPool.getThreadContext(); | ||
|
|
||
| EventBus.getDefault() | ||
| .register( | ||
| new SecurityConfigVersionHandler(configRepository, clusterService.getSettings(), threadContext, threadPool, client) | ||
| ); | ||
|
|
||
| EventBus.getDefault().post(new SecurityConfigChangeEvent()); | ||
|
|
||
| return ValidationResult.error(OK, (builder, params) -> { | ||
| XContentBuilder inner = buildRollbackResponseJson(versionId); | ||
| builder.copyCurrentStructure(JsonXContent.jsonXContent.createParser(null, null, BytesReference.bytes(inner).streamInput())); | ||
| return builder; | ||
| }); | ||
|
|
||
| } catch (Exception e) { | ||
| log.error("Rollback to version {} failed", versionId, e); | ||
| return ValidationResult.error(INTERNAL_SERVER_ERROR, payload(INTERNAL_SERVER_ERROR, "Rollback failed: " + e.getMessage())); | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
Suggested change
| private ValidationResult<SecurityConfiguration> handlePostRequest(RestRequest request) throws IOException { | |
| String versionParam = request.param("versionID"); | |
| try { | |
| if (versionParam == null) { | |
| return rollbackToPreviousVersion(); | |
| } else { | |
| return rollbackToSpecificVersion(versionParam); | |
| } | |
| } catch (Exception e) { | |
| log.error("Rollback request failed", e); | |
| return ValidationResult.error(INTERNAL_SERVER_ERROR, payload(INTERNAL_SERVER_ERROR, e.getMessage())); | |
| } | |
| } | |
| private ValidationResult<SecurityConfiguration> rollbackToPreviousVersion() throws IOException { | |
| SecurityConfigVersionDocument doc = versionsLoader.loadFullDocument(); | |
| SecurityConfigVersionsLoader.sortVersionsById(doc.getVersions()); | |
| var versions = doc.getVersions(); | |
| if (versions.size() < 2) { | |
| return ValidationResult.error(NOT_FOUND, payload(NOT_FOUND, "No previous version available to rollback")); | |
| } | |
| String previousVersionId = versions.get(versions.size() - 2).getVersion_id(); | |
| return rollbackCommon(previousVersionId, doc); | |
| } | |
| private ValidationResult<SecurityConfiguration> rollbackToSpecificVersion(String versionId) throws IOException { | |
| SecurityConfigVersionDocument doc = versionsLoader.loadFullDocument(); | |
| SecurityConfigVersionsLoader.sortVersionsById(doc.getVersions()); | |
| var maybeVer = doc.getVersions().stream().filter(v -> versionId.equals(v.getVersion_id())).findFirst(); | |
| if (maybeVer.isEmpty()) { | |
| return ValidationResult.error(NOT_FOUND, payload(NOT_FOUND, "Version " + versionId + " not found")); | |
| } | |
| return rollbackCommon(versionId, doc); | |
| } | |
| private ValidationResult<SecurityConfiguration> rollbackCommon(String versionId, SecurityConfigVersionDocument doc) throws IOException { | |
| SecurityConfigVersionsLoader.sortVersionsById(doc.getVersions()); | |
| var maybeVer = doc.getVersions().stream().filter(v -> versionId.equals(v.getVersion_id())).findFirst().orElse(null); | |
| if (maybeVer == null) { | |
| return ValidationResult.error(NOT_FOUND, payload(NOT_FOUND, "Version " + versionId + " not found")); | |
| } | |
| try { | |
| rollbackConfigsToSecurityIndex(maybeVer); | |
| ThreadContext threadContext = threadPool.getThreadContext(); | |
| EventBus.getDefault() | |
| .register( | |
| new SecurityConfigVersionHandler(configRepository, clusterService.getSettings(), threadContext, threadPool, client) | |
| ); | |
| EventBus.getDefault().post(new SecurityConfigChangeEvent()); | |
| return ValidationResult.error(OK, (builder, params) -> { | |
| XContentBuilder inner = buildRollbackResponseJson(versionId); | |
| builder.copyCurrentStructure(JsonXContent.jsonXContent.createParser(null, null, BytesReference.bytes(inner).streamInput())); | |
| return builder; | |
| }); | |
| } catch (Exception e) { | |
| log.error("Rollback to version {} failed", versionId, e); | |
| return ValidationResult.error(INTERNAL_SERVER_ERROR, payload(INTERNAL_SERVER_ERROR, "Rollback failed: " + e.getMessage())); | |
| } | |
| } | |
| private ValidationResult<SecurityConfiguration> handlePostRequest(RestRequest request) throws IOException { | |
| String versionParam = request.param("versionID"); | |
| try { | |
| rollBack(versionParam); | |
| } catch (Exception e) { | |
| log.error("Rollback request failed", e); | |
| return ValidationResult.error(INTERNAL_SERVER_ERROR, payload(INTERNAL_SERVER_ERROR, e.getMessage())); | |
| } | |
| } | |
| private ValidationResult<SecurityConfiguration> rollBack(String rollBackVersionId) throws IOException { | |
| SecurityConfigVersionDocument doc = versionsLoader.loadFullDocument(); | |
| if (doc.getVersions().size() < 2) { | |
| return ValidationResult.error(NOT_FOUND, payload(NOT_FOUND, "No previous version available to rollback")); | |
| } | |
| SecurityConfigVersionsLoader.sortVersionsById(doc.getVersions()); | |
| var docVersions = doc.getVersions(); | |
| if (rollBackVersionId != null) { | |
| rollBackVersionId = doc.getVersions().stream().filter(v -> rollBackVersionId.equals(v.getVersion_id())).findFirst(); | |
| if (rollBackVersionId.isEmpty()) { | |
| return ValidationResult.error(NOT_FOUND, payload(NOT_FOUND, "Version " + rollBackVersionId + " not found")); | |
| } | |
| } else { | |
| rollBackVersionId = docVersions.get(docVersions.size() - 2).getVersion_id(); | |
| } | |
| try { | |
| rollbackConfigsToSecurityIndex(rollBackVersionI); | |
| ThreadContext threadContext = threadPool.getThreadContext(); | |
| EventBus.getDefault() | |
| .register( | |
| new SecurityConfigVersionHandler(configRepository, clusterService.getSettings(), threadContext, threadPool, client) | |
| ); | |
| EventBus.getDefault().post(new SecurityConfigChangeEvent()); | |
| return ValidationResult.error(OK, (builder, params) -> { | |
| XContentBuilder inner = buildRollbackResponseJson(versionId); | |
| builder.copyCurrentStructure(JsonXContent.jsonXContent.createParser(null, null, BytesReference.bytes(inner).streamInput())); | |
| return builder; | |
| }); | |
| } catch (Exception e) { | |
| log.error("Rollback to version {} failed", versionId, e); | |
| return ValidationResult.error(INTERNAL_SERVER_ERROR, payload(INTERNAL_SERVER_ERROR, "Rollback failed: " + e.getMessage())); | |
| } | |
| } |
| } | ||
|
|
||
| @Override | ||
| protected EndpointValidator createEndpointValidator() { |
Member
There was a problem hiding this comment.
why do we need to over-ride this method?
Comment on lines
+89
to
+128
| private ValidationResult<SecurityConfiguration> handleGetRequest(String versionParam) throws IOException { | ||
| LOGGER.info("Called handleGetRequest with versionParam={}", versionParam); | ||
| try { | ||
| if (versionParam == null) { | ||
| return viewAllVersions(); | ||
| } else { | ||
| return viewSpecificVersion(versionParam); | ||
| } | ||
| } catch (Exception e) { | ||
| return ValidationResult.error(RestStatus.INTERNAL_SERVER_ERROR, payload(RestStatus.INTERNAL_SERVER_ERROR, e.getMessage())); | ||
| } | ||
| } | ||
|
|
||
| private ValidationResult<SecurityConfiguration> viewAllVersions() throws IOException { | ||
| SecurityConfigVersionDocument doc = versionsLoader.loadFullDocument(); | ||
| SecurityConfigVersionsLoader.sortVersionsById(doc.getVersions()); | ||
|
|
||
| return ValidationResult.error(OK, (builder, params) -> { | ||
| XContentBuilder inner = buildVersionsJsonBuilder(doc.getVersions()); | ||
| builder.copyCurrentStructure(JsonXContent.jsonXContent.createParser(null, null, BytesReference.bytes(inner).streamInput())); | ||
| return builder; | ||
| }); | ||
| } | ||
|
|
||
| private ValidationResult<SecurityConfiguration> viewSpecificVersion(String versionId) throws IOException { | ||
| SecurityConfigVersionDocument doc = versionsLoader.loadFullDocument(); | ||
| SecurityConfigVersionsLoader.sortVersionsById(doc.getVersions()); | ||
|
|
||
| var maybeVer = doc.getVersions().stream().filter(v -> versionId.equals(v.getVersion_id())).findFirst(); | ||
|
|
||
| if (maybeVer.isEmpty()) { | ||
| return ValidationResult.error(NOT_FOUND, payload(NOT_FOUND, "Version " + versionId + " not found")); | ||
| } | ||
|
|
||
| return ValidationResult.error(OK, (builder, params) -> { | ||
| XContentBuilder inner = buildVersionsJsonBuilder(List.of(maybeVer.get())); | ||
| builder.copyCurrentStructure(JsonXContent.jsonXContent.createParser(null, null, BytesReference.bytes(inner).streamInput())); | ||
| return builder; | ||
| }); | ||
| } |
Member
There was a problem hiding this comment.
Suggested change
| private ValidationResult<SecurityConfiguration> handleGetRequest(String versionParam) throws IOException { | |
| LOGGER.info("Called handleGetRequest with versionParam={}", versionParam); | |
| try { | |
| if (versionParam == null) { | |
| return viewAllVersions(); | |
| } else { | |
| return viewSpecificVersion(versionParam); | |
| } | |
| } catch (Exception e) { | |
| return ValidationResult.error(RestStatus.INTERNAL_SERVER_ERROR, payload(RestStatus.INTERNAL_SERVER_ERROR, e.getMessage())); | |
| } | |
| } | |
| private ValidationResult<SecurityConfiguration> viewAllVersions() throws IOException { | |
| SecurityConfigVersionDocument doc = versionsLoader.loadFullDocument(); | |
| SecurityConfigVersionsLoader.sortVersionsById(doc.getVersions()); | |
| return ValidationResult.error(OK, (builder, params) -> { | |
| XContentBuilder inner = buildVersionsJsonBuilder(doc.getVersions()); | |
| builder.copyCurrentStructure(JsonXContent.jsonXContent.createParser(null, null, BytesReference.bytes(inner).streamInput())); | |
| return builder; | |
| }); | |
| } | |
| private ValidationResult<SecurityConfiguration> viewSpecificVersion(String versionId) throws IOException { | |
| SecurityConfigVersionDocument doc = versionsLoader.loadFullDocument(); | |
| SecurityConfigVersionsLoader.sortVersionsById(doc.getVersions()); | |
| var maybeVer = doc.getVersions().stream().filter(v -> versionId.equals(v.getVersion_id())).findFirst(); | |
| if (maybeVer.isEmpty()) { | |
| return ValidationResult.error(NOT_FOUND, payload(NOT_FOUND, "Version " + versionId + " not found")); | |
| } | |
| return ValidationResult.error(OK, (builder, params) -> { | |
| XContentBuilder inner = buildVersionsJsonBuilder(List.of(maybeVer.get())); | |
| builder.copyCurrentStructure(JsonXContent.jsonXContent.createParser(null, null, BytesReference.bytes(inner).streamInput())); | |
| return builder; | |
| }); | |
| } | |
| private ValidationResult<SecurityConfiguration> handleGetRequest(String versionId) throws IOException { | |
| try { | |
| viewVersions(versionId); | |
| } catch (Exception e) { | |
| return ValidationResult.error(RestStatus.INTERNAL_SERVER_ERROR, payload(RestStatus.INTERNAL_SERVER_ERROR, e.getMessage())); | |
| } | |
| } | |
| private ValidationResult<SecurityConfiguration> viewVersion(String versionId) throws IOException { | |
| SecurityConfigVersionDocument doc = versionsLoader.loadFullDocument(); | |
| if (versionId.isEmpty()) { | |
| LOGGER.info("Fetching doc with all versions"); | |
| } else if (.isEmpty()) { | |
| return ValidationResult.error(NOT_FOUND, payload(NOT_FOUND, "Version " + versionId + " not found")); | |
| } else { | |
| LOGGER.info("Fetching doc version {}", versionId); | |
| } | |
| return ValidationResult.error(OK, (builder, params) -> { | |
| XContentBuilder inner = buildVersionsJsonBuilder(versionId.isEmpty() ? doc.getVersions() : List.of(versionId)); | |
| builder.copyCurrentStructure(JsonXContent.jsonXContent.createParser(null, null, BytesReference.bytes(inner).streamInput())); | |
| return builder; | |
| }); | |
| } |
| * Build the JSON structure: | ||
| */ | ||
|
|
||
| private XContentBuilder buildVersionsJsonBuilder(List<SecurityConfigVersionDocument.Version<?>> versions) throws IOException { |
Member
There was a problem hiding this comment.
we usually name these methods as toXContent()
| } | ||
|
|
||
| @Override | ||
| protected EndpointValidator createEndpointValidator() { |
Member
There was a problem hiding this comment.
do we need this override?
Member
|
In addition to the comments above we should make these APIs protected with rest-admin API permission. |
Contributor
|
@divyanshi-0402 Lets mark this PR as draft until me make all the required changes based on changes in previous PR |
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
3 participants
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.
Description
Implementing View API and Rollback API for Versioned Security Configurations, build upon the changes from the PR : Versioned Security Configuration Management #5357.
Hence, kindly just review the final commit for this PR : 989c1e6
API Design
View Security Configuration Versions
Purpose: View the complete history of security configuration changes, or any specific version of the security configurations.
Endpoint:
Access Control: Admin/Security Manager permissions required
Response Format for specific version:
Response Format for all versions :
Response format if the version doesn't exist :
Rollback to desired Security Configuration Versions
Purpose : Allows rollback to immediate or any desired version of security
Endpoint:
Access Control: Admin/Security Manager permissions required
Response format to rollback to immediate previous version :
Response format to rollback to a specific version :
Response format if the version doesn't exist :
Issues Resolved
Related to #5093
Testing
Unit Tests and Integration Tests included
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.