diff --git a/src/integrationTest/java/org/opensearch/security/DefaultConfigurationTests.java b/src/integrationTest/java/org/opensearch/security/DefaultConfigurationTests.java index fe6bf2d7a3..18822d1d5d 100644 --- a/src/integrationTest/java/org/opensearch/security/DefaultConfigurationTests.java +++ b/src/integrationTest/java/org/opensearch/security/DefaultConfigurationTests.java @@ -87,38 +87,53 @@ public void securityRolesUgrade() throws Exception { try (var client = cluster.getRestClient(ADMIN_USER)) { Awaitility.await().alias("Load default configuration").until(() -> client.getAuthInfo().getStatusCode(), equalTo(200)); - final var defaultRolesResponse = client.get("_plugins/_security/api/roles/"); - final var rolesNames = extractFieldNames(defaultRolesResponse.getBodyAs(JsonNode.class)); + final var expectedRoles = client.get("_plugins/_security/api/roles/"); + final var expectedRoleNames = extractFieldNames(expectedRoles.getBodyAs(JsonNode.class)); - final var checkForUpgrade = client.get("_plugins/_security/api/_upgrade_check"); - System.out.println("checkForUpgrade Response: " + checkForUpgrade.getBody()); + final var upgradeCheck = client.get("_plugins/_security/api/_upgrade_check"); + upgradeCheck.assertStatusCode(200); + assertThat(upgradeCheck.getBooleanFromJsonBody("/upgradeAvaliable"), equalTo(false)); final var roleToDelete = "flow_framework_full_access"; - final var deleteRoleResponse = client.delete("_plugins/_security/api/roles/" + roleToDelete); - deleteRoleResponse.assertStatusCode(200); - - final var checkForUpgrade3 = client.get("_plugins/_security/api/_upgrade_check"); - System.out.println("checkForUpgrade3 Response: " + checkForUpgrade3.getBody()); + client.delete("_plugins/_security/api/roles/" + roleToDelete).assertStatusCode(200); final var roleToAlter = "flow_framework_read_access"; - final String patchBody = "[{ \"op\": \"replace\", \"path\": \"/cluster_permissions\", \"value\":" - + "[\"a\",\"b\",\"c\"]" - + "},{ \"op\": \"add\", \"path\": \"/index_permissions\", \"value\":" - + "[{\"index_patterns\":[\"*\"],\"allowed_actions\":[\"*\"]}]" - + "}]"; - final var updateRoleResponse = client.patch("_plugins/_security/api/roles/" + roleToAlter, patchBody); - updateRoleResponse.assertStatusCode(200); - System.out.println("Updated Role Response: " + updateRoleResponse.getBody()); - - final var checkForUpgrade2 = client.get("_plugins/_security/api/_upgrade_check"); - System.out.println("checkForUpgrade2 Response: " + checkForUpgrade2.getBody()); - - final var upgradeResponse = client.post("_plugins/_security/api/_upgrade_perform"); - System.out.println("upgrade Response: " + upgradeResponse.getBody()); - - final var afterUpgradeRolesResponse = client.get("_plugins/_security/api/roles/"); - final var afterUpgradeRolesNames = extractFieldNames(afterUpgradeRolesResponse.getBodyAs(JsonNode.class)); - assertThat(afterUpgradeRolesResponse.getBody(), afterUpgradeRolesNames, equalTo(rolesNames)); + client.patch("_plugins/_security/api/roles/" + roleToAlter, "[\n" + // + " {\n" + // + " \"op\": \"replace\",\n" + // + " \"path\": \"/cluster_permissions\",\n" + // + " \"value\": [\"a\", \"b\", \"c\"]\n" + // + " },\n" + // + " {\n" + // + " \"op\": \"add\",\n" + // + " \"path\": \"/index_permissions\",\n" + // + " \"value\": [ {\n" + // + " \"index_patterns\": [\"*\"],\n" + // + " \"allowed_actions\": [\"*\"]\n" + // + " }\n" + // + " ]\n" + // + " }\n" + // + "]").assertStatusCode(200); + + final var upgradeCheckAfterChanges = client.get("_plugins/_security/api/_upgrade_check"); + upgradeCheckAfterChanges.assertStatusCode(200); + assertThat( + upgradeCheckAfterChanges.getTextArrayFromJsonBody("/upgradeActions/roles/add"), + equalTo(List.of("flow_framework_full_access")) + ); + assertThat( + upgradeCheckAfterChanges.getTextArrayFromJsonBody("/upgradeActions/roles/modify"), + equalTo(List.of("flow_framework_read_access")) + ); + + final var performUpgrade = client.post("_plugins/_security/api/_upgrade_perform"); + performUpgrade.assertStatusCode(200); + assertThat(performUpgrade.getTextArrayFromJsonBody("/upgrades/roles/add"), equalTo(List.of("flow_framework_full_access"))); + assertThat(performUpgrade.getTextArrayFromJsonBody("/upgrades/roles/modify"), equalTo(List.of("flow_framework_read_access"))); + + final var afterUpgradeRoles = client.get("_plugins/_security/api/roles/"); + final var afterUpgradeRolesNames = extractFieldNames(afterUpgradeRoles.getBodyAs(JsonNode.class)); + assertThat(afterUpgradeRolesNames, equalTo(expectedRoleNames)); } } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ConfigUpgradeApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/ConfigUpgradeApiAction.java index 0cb51e3bb5..063f0f7281 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/ConfigUpgradeApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/ConfigUpgradeApiAction.java @@ -47,7 +47,6 @@ import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestRequest.Method; import org.opensearch.security.configuration.ConfigurationRepository; -import org.opensearch.security.dlic.rest.api.ConfigUpgradeApiAction.ConfigItemChanges; import org.opensearch.security.dlic.rest.support.Utils; import org.opensearch.security.dlic.rest.validation.EndpointValidator; import org.opensearch.security.dlic.rest.validation.RequestContentValidator; @@ -127,7 +126,7 @@ void performUpgrade(final RestChannel channel, final RestRequest request, final .error((status, toXContent) -> response(channel, status, toXContent)); } - ValidationResult> applyDifferences( + private ValidationResult> applyDifferences( final RestRequest request, final Client client, final List> differencesToUpdate @@ -183,7 +182,7 @@ ValidationResult>> verifyHasDifferences(List>> configurationDifferences(final Set configurations) { + private ValidationResult>> configurationDifferences(final Set configurations) { try { final var differences = new ArrayList>>(); for (final var configuration : configurations) { @@ -208,7 +207,7 @@ ValidationResult> computeDifferenceToUpdate(final CType c })); } - ValidationResult> getAndValidateConfigurationsToUpgrade(final RestRequest request) { + private ValidationResult> getAndValidateConfigurationsToUpgrade(final RestRequest request) { final String[] configs = request.paramAsStringArray(REQUEST_PARAM_CONFIGS_KEY, null); final Set configurations; @@ -233,7 +232,7 @@ ValidationResult> getAndValidateConfigurationsToUpgrade(final RestReq return ValidationResult.success(configurations); } - JsonNode filterRemoveOperations(final JsonNode diff) { + private JsonNode filterRemoveOperations(final JsonNode diff) { final ArrayNode filteredDiff = JsonNodeFactory.instance.arrayNode(); diff.forEach(node -> { if (!isRemoveOperation(node)) { @@ -261,7 +260,7 @@ private static boolean isRemoveOperation(final JsonNode node) { return node.get("op").asText().equals("remove"); } - SecurityDynamicConfiguration loadYamlFile(final String filepath, final CType cType) throws IOException { + private SecurityDynamicConfiguration loadYamlFile(final String filepath, final CType cType) throws IOException { return ConfigHelper.fromYamlFile(filepath, cType, ConfigurationRepository.DEFAULT_CONFIG_VERSION, 0, 0); } diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/ConfigUpgradeApiActionUnitTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/ConfigUpgradeApiActionUnitTest.java index b8332104c3..8f9ef2b2c7 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/ConfigUpgradeApiActionUnitTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/ConfigUpgradeApiActionUnitTest.java @@ -1,3 +1,14 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + package org.opensearch.security.dlic.rest.api; import java.io.IOException; @@ -8,7 +19,7 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import org.junit.Before; import org.junit.Test; - +import org.mockito.Mock; import org.opensearch.action.index.IndexResponse; import org.opensearch.client.Client; import org.opensearch.common.action.ActionFuture; @@ -22,14 +33,17 @@ import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; -import org.mockito.Mock; - import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.argThat; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; public class ConfigUpgradeApiActionUnitTest extends AbstractApiActionValidationTest {