Clean up tests

Signed-off-by: Peter Nied <petern@amazon.com>
peternied · Mar 8, 2024 · ddc04e5 · ddc04e5
1 parent 7bec1f6
commit ddc04e5
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 38 deletions.
diff --git 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
@@ -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<List<ConfigItemChanges>> applyDifferences(
+    private ValidationResult<List<ConfigItemChanges>> applyDifferences(
         final RestRequest request,
         final Client client,
         final List<Tuple<CType, JsonNode>> differencesToUpdate
@@ -183,7 +182,7 @@ ValidationResult<List<Tuple<CType, JsonNode>>> verifyHasDifferences(List<Tuple<C
         return ValidationResult.success(diffs);
     }
 
-    ValidationResult<List<Tuple<CType, JsonNode>>> configurationDifferences(final Set<CType> configurations) {
+    private ValidationResult<List<Tuple<CType, JsonNode>>> configurationDifferences(final Set<CType> configurations) {
         try {
             final var differences = new ArrayList<ValidationResult<Tuple<CType, JsonNode>>>();
             for (final var configuration : configurations) {
@@ -208,7 +207,7 @@ ValidationResult<Tuple<CType, JsonNode>> computeDifferenceToUpdate(final CType c
         }));
     }
 
-    ValidationResult<Set<CType>> getAndValidateConfigurationsToUpgrade(final RestRequest request) {
+    private ValidationResult<Set<CType>> getAndValidateConfigurationsToUpgrade(final RestRequest request) {
         final String[] configs = request.paramAsStringArray(REQUEST_PARAM_CONFIGS_KEY, null);
 
         final Set<CType> configurations;
@@ -233,7 +232,7 @@ ValidationResult<Set<CType>> 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");
     }
 
-    <T> SecurityDynamicConfiguration<T> loadYamlFile(final String filepath, final CType cType) throws IOException {
+    private <T> SecurityDynamicConfiguration<T> 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
@@ -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 {