From 6de70eb7fe602f1a8946c9928152403a0bc0b3d6 Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Thu, 11 Jan 2024 19:41:44 +0000 Subject: [PATCH 01/20] Adding intiial security integration tests, addin test security workflow Signed-off-by: Joshua Palis --- .github/workflows/test_security.yml | 77 +++++++ build.gradle | 29 +++ .../rest/RestCreateWorkflowAction.java | 8 +- .../rest/RestProvisionWorkflowAction.java | 6 +- .../FlowFrameworkRestTestCase.java | 199 ++++++++++++------ .../rest/FlowFrameworkRestApiIT.java | 54 ++--- .../rest/FlowFrameworkSecureRestApiIT.java | 50 +++++ 7 files changed, 332 insertions(+), 91 deletions(-) create mode 100644 .github/workflows/test_security.yml create mode 100644 src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java diff --git a/.github/workflows/test_security.yml b/.github/workflows/test_security.yml new file mode 100644 index 000000000..9a014e895 --- /dev/null +++ b/.github/workflows/test_security.yml @@ -0,0 +1,77 @@ +name: Security test workflow for Flow Framework +on: + push: + branches: + - "*" + pull_request: + branches: + - "*" + +jobs: + Build-ad: + strategy: + matrix: + java: [11,17,21] + fail-fast: false + + name: Security test workflow for Flow Framework + runs-on: ubuntu-latest + + steps: + - name: Setup Java ${{ matrix.java }} + uses: actions/setup-java@v3 + with: + distribution: 'temurin' + java-version: ${{ matrix.java }} + + - name: Checkout Flow Framework + uses: actions/checkout@v4 + + - name: Build Flow Framework + run: | + ./gradlew assemble + - name: Pull and Run Docker + run: | + plugin=`basename $(ls build/distributions/*.zip)` + version=`echo $plugin|awk -F- '{print $5}'| cut -d. -f 1-3` + plugin_version=`echo $plugin|awk -F- '{print $5}'| cut -d. -f 1-4` + qualifier=`echo $plugin|awk -F- '{print $6}'| cut -d. -f 1-1` + + if $qualifier!=SNAPSHOT + then + docker_version=$version-$qualifier + else + docker_version=$version + fi + echo plugin version plugin_version qualifier docker_version + echo "($plugin) ($version) ($plugin_version) ($qualifier) ($docker_version)" + + cd .. + if docker pull opensearchstaging/opensearch:$docker_version + then + echo "FROM opensearchstaging/opensearch:$docker_version" >> Dockerfile + echo "RUN if [ -d /usr/share/opensearch/plugins/opensearch-flow-framework ]; then /usr/share/opensearch/bin/opensearch-plugin remove opensearch-flow-framework; fi" >> Dockerfile + echo "ADD flow-framework/build/distributions/$plugin /tmp/" >> Dockerfile + echo "RUN /usr/share/opensearch/bin/opensearch-plugin install --batch file:/tmp/$plugin" >> Dockerfile + docker build -t opensearch-flow-framework:test . + echo "imagePresent=true" >> $GITHUB_ENV + else + echo "imagePresent=false" >> $GITHUB_ENV + fi + - name: Run Docker Image + if: env.imagePresent == 'true' + run: | + cd .. + docker run -p 9200:9200 -d -p 9600:9600 -e "discovery.type=single-node" opensearch-ad:test + sleep 90 + - name: Run Flow Framework Test + if: env.imagePresent == 'true' + run: | + security=`curl -XGET https://localhost:9200/_cat/plugins?v -u admin:admin --insecure |grep opensearch-security|wc -l` + if [ $security -gt 0 ] + then + echo "Security plugin is available" + ./gradlew integTest -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername="docker-cluster" -Dhttps=true -Duser=admin -Dpassword=admin + else + echo "Security plugin is NOT available, skipping integration tests" + fi diff --git a/build.gradle b/build.gradle index 8a957fe1c..6a74fe08a 100644 --- a/build.gradle +++ b/build.gradle @@ -209,6 +209,20 @@ integTest { } } + // Exclude integration tests that require security plugin + if (System.getProperty("https") == null || System.getProperty("https") == "false") { + filter { + excludeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkSecureRestApiIT" + } + } + + // Include only secure integration tests in security enabled clusters + if (System.getProperty("https") != null && System.getProperty("https") == "true") { + filter { + includeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkSecureRestApiIT" + excludeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkRestApiIT" + } + } // doFirst delays this block until execution time doFirst { @@ -285,6 +299,21 @@ task integTestRemote(type: RestIntegTestTask) { includeTestsMatching "org.opensearch.flowframework.rest.*IT" } } + + // Exclude integration tests that require security plugin + if (System.getProperty("https") == null || System.getProperty("https") == "false") { + filter { + excludeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkSecureRestApiIT" + } + } + + // Include only secure integration tests in security enabled clusters + if (System.getProperty("https") != null && System.getProperty("https") == "true") { + filter { + includeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkSecureRestApiIT" + excludeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkRestApiIT" + } + } } // Automatically sets up the integration test cluster locally diff --git a/src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java b/src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java index ed0ae670b..54c485ec0 100644 --- a/src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java +++ b/src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.ExceptionsHelper; import org.opensearch.client.node.NodeClient; import org.opensearch.core.action.ActionListener; import org.opensearch.core.rest.RestStatus; @@ -95,11 +96,14 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli channel.sendResponse(new BytesRestResponse(RestStatus.CREATED, builder)); }, exception -> { try { - FlowFrameworkException ex = (FlowFrameworkException) exception; + FlowFrameworkException ex = exception instanceof FlowFrameworkException + ? (FlowFrameworkException) exception + : new FlowFrameworkException(exception.getMessage(), ExceptionsHelper.status(exception)); XContentBuilder exceptionBuilder = ex.toXContent(channel.newErrorBuilder(), ToXContent.EMPTY_PARAMS); channel.sendResponse(new BytesRestResponse(ex.getRestStatus(), exceptionBuilder)); } catch (IOException e) { - logger.error("Failed to send back create workflow exception", e); + logger.error("Failed to send back provision workflow exception", e); + channel.sendResponse(new BytesRestResponse(ExceptionsHelper.status(e), e.getMessage())); } })); } catch (Exception e) { diff --git a/src/main/java/org/opensearch/flowframework/rest/RestProvisionWorkflowAction.java b/src/main/java/org/opensearch/flowframework/rest/RestProvisionWorkflowAction.java index aad82766d..124b6bf49 100644 --- a/src/main/java/org/opensearch/flowframework/rest/RestProvisionWorkflowAction.java +++ b/src/main/java/org/opensearch/flowframework/rest/RestProvisionWorkflowAction.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.ExceptionsHelper; import org.opensearch.client.node.NodeClient; import org.opensearch.core.action.ActionListener; import org.opensearch.core.rest.RestStatus; @@ -90,11 +91,14 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli channel.sendResponse(new BytesRestResponse(RestStatus.OK, builder)); }, exception -> { try { - FlowFrameworkException ex = (FlowFrameworkException) exception; + FlowFrameworkException ex = exception instanceof FlowFrameworkException + ? (FlowFrameworkException) exception + : new FlowFrameworkException(exception.getMessage(), ExceptionsHelper.status(exception)); XContentBuilder exceptionBuilder = ex.toXContent(channel.newErrorBuilder(), ToXContent.EMPTY_PARAMS); channel.sendResponse(new BytesRestResponse(ex.getRestStatus(), exceptionBuilder)); } catch (IOException e) { logger.error("Failed to send back provision workflow exception", e); + channel.sendResponse(new BytesRestResponse(ExceptionsHelper.status(e), e.getMessage())); } })); } catch (FlowFrameworkException ex) { diff --git a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java index 5b7c62e81..5115aa3bf 100644 --- a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java +++ b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java @@ -8,6 +8,7 @@ */ package org.opensearch.flowframework; +import org.apache.commons.lang3.RandomStringUtils; import org.apache.hc.client5.http.auth.AuthScope; import org.apache.hc.client5.http.auth.UsernamePasswordCredentials; import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider; @@ -46,7 +47,6 @@ import org.opensearch.flowframework.model.Template; import org.opensearch.flowframework.model.WorkflowState; import org.opensearch.test.rest.OpenSearchRestTestCase; -import org.junit.AfterClass; import org.junit.Before; import javax.net.ssl.SSLEngine; @@ -62,7 +62,6 @@ import java.util.Objects; import java.util.Optional; import java.util.concurrent.TimeUnit; -import java.util.stream.Collectors; import static org.opensearch.client.RestClientBuilder.DEFAULT_MAX_CONN_PER_ROUTE; import static org.opensearch.client.RestClientBuilder.DEFAULT_MAX_CONN_TOTAL; @@ -79,11 +78,17 @@ */ public abstract class FlowFrameworkRestTestCase extends OpenSearchRestTestCase { + private static String FULL_ACCESS_ROLE = "flow_framework_full_access"; + private static String READ_ACCESS_ROLE = "flow_framework_read_access"; + public static String FULL_ACCESS_USER = "fullAccessUser"; + public static String READ_ACCESS_USER = "readAccessUser"; + private static RestClient readAccessClient; + private static RestClient fullAccessClient; + @Before - public void setUpSettings() throws Exception { + protected void setUpSettings() throws Exception { if (!indexExistsWithAdminClient(".plugins-ml-config")) { - // Initial cluster set up // Enable Flow Framework Plugin Rest APIs @@ -134,6 +139,38 @@ public void setUpSettings() throws Exception { assertBusy(() -> { assertTrue(indexExistsWithAdminClient(".plugins-ml-config")); }, 60, TimeUnit.SECONDS); } + // Set up clients if running in security enabled cluster + if (isHttps()) { + String fullAccessUserPassword = generatePassword(FULL_ACCESS_USER); + String readAccessUserPassword = generatePassword(READ_ACCESS_USER); + + // Configure full access user and client + Response response = createUser(FULL_ACCESS_USER, fullAccessUserPassword, FULL_ACCESS_ROLE); + fullAccessClient = new SecureRestClientBuilder( + getClusterHosts().toArray(new HttpHost[0]), + isHttps(), + FULL_ACCESS_USER, + fullAccessUserPassword + ).setSocketTimeout(60000).build(); + + // Configure read access user and client + response = createUser(READ_ACCESS_USER, readAccessUserPassword, READ_ACCESS_ROLE); + readAccessClient = new SecureRestClientBuilder( + getClusterHosts().toArray(new HttpHost[0]), + isHttps(), + READ_ACCESS_USER, + readAccessUserPassword + ).setSocketTimeout(60000).build(); + } + + } + + protected static RestClient fullAccessClient() { + return fullAccessClient; + } + + protected static RestClient readAccessClient() { + return readAccessClient; } protected boolean isHttps() { @@ -216,37 +253,6 @@ protected RestClient buildClient(Settings settings, HttpHost[] hosts) throws IOE } - // Cleans up resources after all test execution has been completed - @SuppressWarnings("unchecked") - @AfterClass - protected static void wipeAllSystemIndices() throws IOException { - Response response = adminClient().performRequest(new Request("GET", "/_cat/indices?format=json&expand_wildcards=all")); - MediaType xContentType = MediaType.fromMediaType(response.getEntity().getContentType()); - try ( - XContentParser parser = xContentType.xContent() - .createParser( - NamedXContentRegistry.EMPTY, - DeprecationHandler.THROW_UNSUPPORTED_OPERATION, - response.getEntity().getContent() - ) - ) { - XContentParser.Token token = parser.nextToken(); - List> parserList = null; - if (token == XContentParser.Token.START_ARRAY) { - parserList = parser.listOrderedMap().stream().map(obj -> (Map) obj).collect(Collectors.toList()); - } else { - parserList = Collections.singletonList(parser.mapOrdered()); - } - - for (Map index : parserList) { - String indexName = (String) index.get("index"); - if (indexName != null && !".opendistro_security".equals(indexName)) { - adminClient().performRequest(new Request("DELETE", "/" + indexName)); - } - } - } - } - protected static void configureHttpsClient(RestClientBuilder builder, Settings settings) throws IOException { Map headers = ThreadContext.buildDefaultHeaders(settings); Header[] defaultHeaders = new Header[headers.size()]; @@ -318,14 +324,23 @@ protected boolean preserveClusterSettings() { return true; } + /** + * Create an unguessable password. Simple password are weak due to https://tinyurl.com/383em9zk + * @return a random password. + */ + public static String generatePassword(String username) { + return RandomStringUtils.random(15, true, true); + } + /** * Helper method to invoke the Create Workflow Rest Action without validation + * @param client the rest client * @param template the template to create * @throws Exception if the request fails * @return a rest response */ - protected Response createWorkflow(Template template) throws Exception { - return TestHelpers.makeRequest(client(), "POST", WORKFLOW_URI + "?validation=off", Collections.emptyMap(), template.toJson(), null); + protected Response createWorkflow(RestClient client, Template template) throws Exception { + return TestHelpers.makeRequest(client, "POST", WORKFLOW_URI + "?validation=off", Collections.emptyMap(), template.toJson(), null); } /** @@ -340,24 +355,26 @@ protected Response createWorkflowWithProvision(Template template) throws Excepti /** * Helper method to invoke the Create Workflow Rest Action with validation + * @param client the rest client * @param template the template to create * @throws Exception if the request fails * @return a rest response */ - protected Response createWorkflowValidation(Template template) throws Exception { - return TestHelpers.makeRequest(client(), "POST", WORKFLOW_URI, Collections.emptyMap(), template.toJson(), null); + protected Response createWorkflowValidation(RestClient client, Template template) throws Exception { + return TestHelpers.makeRequest(client, "POST", WORKFLOW_URI, Collections.emptyMap(), template.toJson(), null); } /** * Helper method to invoke the Update Workflow API + * @param client the rest client * @param workflowId the document id * @param template the template used to update * @throws Exception if the request fails * @return a rest response */ - protected Response updateWorkflow(String workflowId, Template template) throws Exception { + protected Response updateWorkflow(RestClient client, String workflowId, Template template) throws Exception { return TestHelpers.makeRequest( - client(), + client, "PUT", String.format(Locale.ROOT, "%s/%s", WORKFLOW_URI, workflowId), Collections.emptyMap(), @@ -368,13 +385,14 @@ protected Response updateWorkflow(String workflowId, Template template) throws E /** * Helper method to invoke the Provision Workflow Rest Action + * @param client the rest client * @param workflowId the workflow ID to provision * @throws Exception if the request fails * @return a rest response */ - protected Response provisionWorkflow(String workflowId) throws Exception { + protected Response provisionWorkflow(RestClient client, String workflowId) throws Exception { return TestHelpers.makeRequest( - client(), + client, "POST", String.format(Locale.ROOT, "%s/%s/%s", WORKFLOW_URI, workflowId, "_provision"), Collections.emptyMap(), @@ -385,13 +403,14 @@ protected Response provisionWorkflow(String workflowId) throws Exception { /** * Helper method to invoke the Deprovision Workflow Rest Action + * @param client the rest client * @param workflowId the workflow ID to deprovision * @return a rest response * @throws Exception if the request fails */ - protected Response deprovisionWorkflow(String workflowId) throws Exception { + protected Response deprovisionWorkflow(RestClient client, String workflowId) throws Exception { return TestHelpers.makeRequest( - client(), + client, "POST", String.format(Locale.ROOT, "%s/%s/%s", WORKFLOW_URI, workflowId, "_deprovision"), Collections.emptyMap(), @@ -402,13 +421,14 @@ protected Response deprovisionWorkflow(String workflowId) throws Exception { /** * Helper method to invoke the Delete Workflow Rest Action + * @param client the rest client * @param workflowId the workflow ID to delete * @return a rest response * @throws Exception if the request fails */ - protected Response deleteWorkflow(String workflowId) throws Exception { + protected Response deleteWorkflow(RestClient client, String workflowId) throws Exception { return TestHelpers.makeRequest( - client(), + client, "DELETE", String.format(Locale.ROOT, "%s/%s", WORKFLOW_URI, workflowId), Collections.emptyMap(), @@ -419,14 +439,15 @@ protected Response deleteWorkflow(String workflowId) throws Exception { /** * Helper method to invoke the Get Workflow Rest Action + * @param client the rest client * @param workflowId the workflow ID to get the status * @param all verbose status flag * @throws Exception if the request fails * @return rest response */ - protected Response getWorkflowStatus(String workflowId, boolean all) throws Exception { + protected Response getWorkflowStatus(RestClient client, String workflowId, boolean all) throws Exception { return TestHelpers.makeRequest( - client(), + client, "GET", String.format(Locale.ROOT, "%s/%s/%s?all=%s", WORKFLOW_URI, workflowId, "_status", all), Collections.emptyMap(), @@ -436,9 +457,15 @@ protected Response getWorkflowStatus(String workflowId, boolean all) throws Exce } - protected Response getWorkflowStep() throws Exception { + /** + * Helper method to invoke the Get Workflow Steps Rest Action + * @param client the rest client + * @return rest response + * @throws Exception + */ + protected Response getWorkflowStep(RestClient client) throws Exception { return TestHelpers.makeRequest( - client(), + client, "GET", String.format(Locale.ROOT, "%s/%s", WORKFLOW_URI, "_steps"), Collections.emptyMap(), @@ -449,15 +476,16 @@ protected Response getWorkflowStep() throws Exception { /** * Helper method to invoke the Search Workflow Rest Action with the given query + * @param client the rest client * @param query the search query * @return rest response * @throws Exception if the request fails */ - protected SearchResponse searchWorkflows(String query) throws Exception { + protected SearchResponse searchWorkflows(RestClient client, String query) throws Exception { // Execute search Response restSearchResponse = TestHelpers.makeRequest( - client(), + client, "GET", String.format(Locale.ROOT, "%s/_search", WORKFLOW_URI), Collections.emptyMap(), @@ -480,9 +508,16 @@ protected SearchResponse searchWorkflows(String query) throws Exception { } } - protected SearchResponse searchWorkflowState(String query) throws Exception { + /** + * Helper method to invoke the Search Workflow State Rest Action + * @param client the rest client + * @param query the search query + * @return + * @throws Exception + */ + protected SearchResponse searchWorkflowState(RestClient client, String query) throws Exception { Response restSearchResponse = TestHelpers.makeRequest( - client(), + client, "GET", String.format(Locale.ROOT, "%s/state/_search", WORKFLOW_URI), Collections.emptyMap(), @@ -507,14 +542,19 @@ protected SearchResponse searchWorkflowState(String query) throws Exception { /** * Helper method to invoke the Get Workflow Rest Action and assert the provisioning and state status + * @param client the rest client * @param workflowId the workflow ID to get the status * @param stateStatus the state status name * @param provisioningStatus the provisioning status name * @throws Exception if the request fails */ - protected void getAndAssertWorkflowStatus(String workflowId, State stateStatus, ProvisioningProgress provisioningStatus) - throws Exception { - Response response = getWorkflowStatus(workflowId, true); + protected void getAndAssertWorkflowStatus( + RestClient client, + String workflowId, + State stateStatus, + ProvisioningProgress provisioningStatus + ) throws Exception { + Response response = getWorkflowStatus(client, workflowId, true); assertEquals(RestStatus.OK, TestHelpers.restStatus(response)); Map responseMap = entityAsMap(response); @@ -523,28 +563,34 @@ protected void getAndAssertWorkflowStatus(String workflowId, State stateStatus, } - protected void getAndAssertWorkflowStep() throws Exception { - Response response = getWorkflowStep(); + /** + * Helper method to get and assert a workflow step + * @param client the rest client + * @throws Exception + */ + protected void getAndAssertWorkflowStep(RestClient client) throws Exception { + Response response = getWorkflowStep(client); assertEquals(RestStatus.OK, TestHelpers.restStatus(response)); } /** * Helper method to wait until a workflow provisioning has completed and retrieve any resources created + * @param client the rest client * @param workflowId the workflow id to retrieve resources from * @param timeout the max wait time in seconds * @return a list of created resources * @throws Exception if the request fails */ - protected List getResourcesCreated(String workflowId, int timeout) throws Exception { + protected List getResourcesCreated(RestClient client, String workflowId, int timeout) throws Exception { // wait and ensure state is completed/done assertBusy( - () -> { getAndAssertWorkflowStatus(workflowId, State.COMPLETED, ProvisioningProgress.DONE); }, + () -> { getAndAssertWorkflowStatus(client, workflowId, State.COMPLETED, ProvisioningProgress.DONE); }, timeout, TimeUnit.SECONDS ); - Response response = getWorkflowStatus(workflowId, true); + Response response = getWorkflowStatus(client, workflowId, true); // Parse workflow state from response and retreieve resources created MediaType mediaType = MediaType.fromMediaType(response.getEntity().getContentType()); @@ -561,4 +607,31 @@ protected List getResourcesCreated(String workflowId, int timeo return workflowState.resourcesCreated(); } } + + protected Response createUser(String name, String password, String backendRole) throws IOException { + String json = "{\"password\": \"" + + password + + "\",\"opendistro_security_roles\": [\"" + + backendRole + + "\"],\"backend_roles\": [],\"attributes\": {}}"; + return TestHelpers.makeRequest( + client(), + "PUT", + "/_opendistro/_security/api/internalusers/" + name, + null, + TestHelpers.toHttpEntity(json), + List.of(new BasicHeader(HttpHeaders.USER_AGENT, "Kibana")) + ); + } + + protected Response deleteUser(String user) throws IOException { + return TestHelpers.makeRequest( + client(), + "DELETE", + "/_opendistro/_security/api/internalusers/" + user, + null, + "", + List.of(new BasicHeader(HttpHeaders.USER_AGENT, "Kibana")) + ); + } } diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java index 3a5e36ff6..54fb368d7 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java @@ -42,7 +42,7 @@ public void testSearchWorkflows() throws Exception { // Create a Workflow that has a credential 12345 Template template = TestHelpers.createTemplateFromFile("createconnector-registerremotemodel-deploymodel.json"); - Response response = createWorkflow(template); + Response response = createWorkflow(client(), template); assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response)); // Retrieve WorkflowID @@ -51,7 +51,7 @@ public void testSearchWorkflows() throws Exception { // Hit Search Workflows API String termIdQuery = "{\"query\":{\"ids\":{\"values\":[\"" + workflowId + "\"]}}}"; - SearchResponse searchResponse = searchWorkflows(termIdQuery); + SearchResponse searchResponse = searchWorkflows(client(), termIdQuery); assertEquals(1, searchResponse.getHits().getTotalHits().value); String searchHitSource = searchResponse.getHits().getAt(0).getSourceAsString(); @@ -99,33 +99,33 @@ public void testCreateAndProvisionLocalModelWorkflow() throws Exception { .build(); // Hit Create Workflow API with invalid template - Response response = createWorkflow(templateWithMissingInputs); + Response response = createWorkflow(client(), templateWithMissingInputs); assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response)); - getAndAssertWorkflowStep(); + getAndAssertWorkflowStep(client()); // Retrieve workflow ID Map responseMap = entityAsMap(response); String workflowId = (String) responseMap.get(WORKFLOW_ID); - getAndAssertWorkflowStatus(workflowId, State.NOT_STARTED, ProvisioningProgress.NOT_STARTED); + getAndAssertWorkflowStatus(client(), workflowId, State.NOT_STARTED, ProvisioningProgress.NOT_STARTED); // Attempt provision - ResponseException exception = expectThrows(ResponseException.class, () -> provisionWorkflow(workflowId)); + ResponseException exception = expectThrows(ResponseException.class, () -> provisionWorkflow(client(), workflowId)); assertTrue(exception.getMessage().contains("Invalid workflow, node [workflow_step_1] missing the following required inputs")); - getAndAssertWorkflowStatus(workflowId, State.NOT_STARTED, ProvisioningProgress.NOT_STARTED); + getAndAssertWorkflowStatus(client(), workflowId, State.NOT_STARTED, ProvisioningProgress.NOT_STARTED); // update workflow with updated inputs - response = updateWorkflow(workflowId, template); + response = updateWorkflow(client(), workflowId, template); assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response)); - getAndAssertWorkflowStatus(workflowId, State.NOT_STARTED, ProvisioningProgress.NOT_STARTED); + getAndAssertWorkflowStatus(client(), workflowId, State.NOT_STARTED, ProvisioningProgress.NOT_STARTED); // Reattempt Provision - response = provisionWorkflow(workflowId); + response = provisionWorkflow(client(), workflowId); assertEquals(RestStatus.OK, TestHelpers.restStatus(response)); - getAndAssertWorkflowStatus(workflowId, State.PROVISIONING, ProvisioningProgress.IN_PROGRESS); + getAndAssertWorkflowStatus(client(), workflowId, State.PROVISIONING, ProvisioningProgress.IN_PROGRESS); // Wait until provisioning has completed successfully before attempting to retrieve created resources - List resourcesCreated = getResourcesCreated(workflowId, 100); + List resourcesCreated = getResourcesCreated(client(), workflowId, 100); // This template should create 2 resources, registered_model_id and deployed model_id assertEquals(2, resourcesCreated.size()); @@ -135,7 +135,7 @@ public void testCreateAndProvisionLocalModelWorkflow() throws Exception { assertNotNull(resourcesCreated.get(1).resourceId()); // Deprovision the workflow to avoid opening circut breaker when running additional tests - Response deprovisionResponse = deprovisionWorkflow(workflowId); + Response deprovisionResponse = deprovisionWorkflow(client(), workflowId); // wait for deprovision to complete Thread.sleep(5000); @@ -165,27 +165,27 @@ public void testCreateAndProvisionRemoteModelWorkflow() throws Exception { .build(); // Hit dry run - ResponseException exception = expectThrows(ResponseException.class, () -> createWorkflowValidation(cyclicalTemplate)); + ResponseException exception = expectThrows(ResponseException.class, () -> createWorkflowValidation(client(), cyclicalTemplate)); // output order not guaranteed assertTrue(exception.getMessage().contains("Cycle detected")); assertTrue(exception.getMessage().contains("workflow_step_2->workflow_step_3")); assertTrue(exception.getMessage().contains("workflow_step_3->workflow_step_2")); // Hit Create Workflow API with original template - Response response = createWorkflow(template); + Response response = createWorkflow(client(), template); assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response)); Map responseMap = entityAsMap(response); String workflowId = (String) responseMap.get(WORKFLOW_ID); - getAndAssertWorkflowStatus(workflowId, State.NOT_STARTED, ProvisioningProgress.NOT_STARTED); + getAndAssertWorkflowStatus(client(), workflowId, State.NOT_STARTED, ProvisioningProgress.NOT_STARTED); // Hit Provision API and assert status - response = provisionWorkflow(workflowId); + response = provisionWorkflow(client(), workflowId); assertEquals(RestStatus.OK, TestHelpers.restStatus(response)); - getAndAssertWorkflowStatus(workflowId, State.PROVISIONING, ProvisioningProgress.IN_PROGRESS); + getAndAssertWorkflowStatus(client(), workflowId, State.PROVISIONING, ProvisioningProgress.IN_PROGRESS); // Wait until provisioning has completed successfully before attempting to retrieve created resources - List resourcesCreated = getResourcesCreated(workflowId, 30); + List resourcesCreated = getResourcesCreated(client(), workflowId, 30); // This template should create 3 resources, connector_id, registered model_id and deployed model_id assertEquals(3, resourcesCreated.size()); @@ -197,7 +197,7 @@ public void testCreateAndProvisionRemoteModelWorkflow() throws Exception { assertNotNull(resourcesCreated.get(2).resourceId()); // Deprovision the workflow to avoid opening circut breaker when running additional tests - Response deprovisionResponse = deprovisionWorkflow(workflowId); + Response deprovisionResponse = deprovisionWorkflow(client(), workflowId); // wait for deprovision to complete Thread.sleep(5000); @@ -212,11 +212,15 @@ public void testCreateAndProvisionAgentFrameworkWorkflow() throws Exception { Map responseMap = entityAsMap(response); String workflowId = (String) responseMap.get(WORKFLOW_ID); // wait and ensure state is completed/done - assertBusy(() -> { getAndAssertWorkflowStatus(workflowId, State.COMPLETED, ProvisioningProgress.DONE); }, 30, TimeUnit.SECONDS); + assertBusy( + () -> { getAndAssertWorkflowStatus(client(), workflowId, State.COMPLETED, ProvisioningProgress.DONE); }, + 30, + TimeUnit.SECONDS + ); // Hit Search State API with the workflow id created above String query = "{\"query\":{\"ids\":{\"values\":[\"" + workflowId + "\"]}}}"; - SearchResponse searchResponse = searchWorkflowState(query); + SearchResponse searchResponse = searchWorkflowState(client(), query); assertEquals(1, searchResponse.getHits().getTotalHits().value); String searchHitSource = searchResponse.getHits().getAt(0).getSourceAsString(); WorkflowState searchHitWorkflowState = WorkflowState.parse(searchHitSource); @@ -235,15 +239,15 @@ public void testCreateAndProvisionAgentFrameworkWorkflow() throws Exception { assertNotNull(resourcesCreated.get(0).resourceId()); // Hit Deprovision API - Response deprovisionResponse = deprovisionWorkflow(workflowId); + Response deprovisionResponse = deprovisionWorkflow(client(), workflowId); assertBusy( - () -> { getAndAssertWorkflowStatus(workflowId, State.NOT_STARTED, ProvisioningProgress.NOT_STARTED); }, + () -> { getAndAssertWorkflowStatus(client(), workflowId, State.NOT_STARTED, ProvisioningProgress.NOT_STARTED); }, 60, TimeUnit.SECONDS ); // Hit Delete API - Response deleteResponse = deleteWorkflow(workflowId); + Response deleteResponse = deleteWorkflow(client(), workflowId); assertEquals(RestStatus.OK, TestHelpers.restStatus(deleteResponse)); } diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java new file mode 100644 index 000000000..f179cf7ad --- /dev/null +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java @@ -0,0 +1,50 @@ +/* + * Copyright OpenSearch Contributors + * 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. + */ +package org.opensearch.flowframework.rest; + +import org.opensearch.client.ResponseException; +import org.opensearch.common.util.io.IOUtils; +import org.opensearch.flowframework.FlowFrameworkRestTestCase; +import org.opensearch.flowframework.TestHelpers; +import org.opensearch.flowframework.model.Template; +import org.junit.After; + +import java.io.IOException; + +public class FlowFrameworkSecureRestApiIT extends FlowFrameworkRestTestCase { + + @After + public void tearDownSecureTests() throws IOException { + IOUtils.close(fullAccessClient(), readAccessClient()); + deleteUser(FULL_ACCESS_USER); + deleteUser(READ_ACCESS_USER); + } + + public void testCreateWorkflowWithReadAccess() throws Exception { + Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json"); + ResponseException exception = expectThrows(ResponseException.class, () -> createWorkflow(readAccessClient(), template)); + assertTrue(exception.getMessage().contains("no permissions for [cluster:admin/opensearch/flow_framework/workflow/create]")); + } + + public void testProvisionWorkflowWithReadAccess() throws Exception { + ResponseException exception = expectThrows(ResponseException.class, () -> provisionWorkflow(readAccessClient(), "test")); + assertTrue(exception.getMessage().contains("no permissions for [cluster:admin/opensearch/flow_framework/workflow/provision]")); + } + + public void testDeleteWorkflowWithReadAccess() throws Exception { + ResponseException exception = expectThrows(ResponseException.class, () -> deleteWorkflow(readAccessClient(), "test")); + assertTrue(exception.getMessage().contains("no permissions for [cluster:admin/opensearch/flow_framework/workflow/delete]")); + } + + public void testDeprovisionWorkflowWithReadAcess() throws Exception { + ResponseException exception = expectThrows(ResponseException.class, () -> deprovisionWorkflow(readAccessClient(), "test")); + assertTrue(exception.getMessage().contains("no permissions for [cluster:admin/opensearch/flow_framework/workflow/deprovision]")); + } + +} From 8b94e139af2edef877c5ff677867e76febc849e3 Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Thu, 11 Jan 2024 19:48:33 +0000 Subject: [PATCH 02/20] updating set up to v4 Signed-off-by: Joshua Palis --- .github/workflows/test_security.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test_security.yml b/.github/workflows/test_security.yml index 9a014e895..f7ea9bf72 100644 --- a/.github/workflows/test_security.yml +++ b/.github/workflows/test_security.yml @@ -19,7 +19,7 @@ jobs: steps: - name: Setup Java ${{ matrix.java }} - uses: actions/setup-java@v3 + uses: actions/setup-java@v4 with: distribution: 'temurin' java-version: ${{ matrix.java }} From e52989ed1b545116b7743325e6b3b3a5eba4107a Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Thu, 11 Jan 2024 19:52:49 +0000 Subject: [PATCH 03/20] Fixing run docker image task Signed-off-by: Joshua Palis --- .github/workflows/test_security.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test_security.yml b/.github/workflows/test_security.yml index f7ea9bf72..f07437fce 100644 --- a/.github/workflows/test_security.yml +++ b/.github/workflows/test_security.yml @@ -62,7 +62,7 @@ jobs: if: env.imagePresent == 'true' run: | cd .. - docker run -p 9200:9200 -d -p 9600:9600 -e "discovery.type=single-node" opensearch-ad:test + docker run -p 9200:9200 -d -p 9600:9600 -e "discovery.type=single-node" opensearch-flow-framework:test sleep 90 - name: Run Flow Framework Test if: env.imagePresent == 'true' From f25c9d53a442e35ce2660a62cf3276444571c33f Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Thu, 11 Jan 2024 20:01:16 +0000 Subject: [PATCH 04/20] Fixing pull and run docket Signed-off-by: Joshua Palis --- .github/workflows/test_security.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test_security.yml b/.github/workflows/test_security.yml index f07437fce..3e56f0066 100644 --- a/.github/workflows/test_security.yml +++ b/.github/workflows/test_security.yml @@ -30,12 +30,15 @@ jobs: - name: Build Flow Framework run: | ./gradlew assemble + # example of variables: + # plugin = opensearch-time-series-analytics-2.10.0.0-SNAPSHOT.zip + # version = 2.10.0, plugin_version = 2.10.0.0, qualifier = SNAPSHOT - name: Pull and Run Docker run: | plugin=`basename $(ls build/distributions/*.zip)` - version=`echo $plugin|awk -F- '{print $5}'| cut -d. -f 1-3` - plugin_version=`echo $plugin|awk -F- '{print $5}'| cut -d. -f 1-4` - qualifier=`echo $plugin|awk -F- '{print $6}'| cut -d. -f 1-1` + version=`echo $plugin|awk -F- '{print $3}'| cut -d. -f 1-3` + plugin_version=`echo $plugin|awk -F- '{print $3}'| cut -d. -f 1-4` + qualifier=`echo $plugin|awk -F- '{print $4}'| cut -d. -f 1-1` if $qualifier!=SNAPSHOT then From 57dde10a23255c847242bed840cd48b8ddbfd3f0 Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Thu, 11 Jan 2024 20:03:43 +0000 Subject: [PATCH 05/20] Fixing pull and run docket Signed-off-by: Joshua Palis --- .github/workflows/test_security.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test_security.yml b/.github/workflows/test_security.yml index 3e56f0066..461e8919c 100644 --- a/.github/workflows/test_security.yml +++ b/.github/workflows/test_security.yml @@ -36,9 +36,9 @@ jobs: - name: Pull and Run Docker run: | plugin=`basename $(ls build/distributions/*.zip)` - version=`echo $plugin|awk -F- '{print $3}'| cut -d. -f 1-3` - plugin_version=`echo $plugin|awk -F- '{print $3}'| cut -d. -f 1-4` - qualifier=`echo $plugin|awk -F- '{print $4}'| cut -d. -f 1-1` + version=`echo $plugin|awk -F- '{print $4}'| cut -d. -f 1-3` + plugin_version=`echo $plugin|awk -F- '{print $4}'| cut -d. -f 1-4` + qualifier=`echo $plugin|awk -F- '{print $5}'| cut -d. -f 1-1` if $qualifier!=SNAPSHOT then From 494f00df94de40b0b30f3d2029d6e9fb06e41e1e Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Thu, 11 Jan 2024 20:10:30 +0000 Subject: [PATCH 06/20] Testing integ test if security is not available Signed-off-by: Joshua Palis --- .github/workflows/test_security.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test_security.yml b/.github/workflows/test_security.yml index 461e8919c..458f17540 100644 --- a/.github/workflows/test_security.yml +++ b/.github/workflows/test_security.yml @@ -31,7 +31,7 @@ jobs: run: | ./gradlew assemble # example of variables: - # plugin = opensearch-time-series-analytics-2.10.0.0-SNAPSHOT.zip + # plugin = opensearch-flow-framework-2.10.0.0-SNAPSHOT.zip # version = 2.10.0, plugin_version = 2.10.0.0, qualifier = SNAPSHOT - name: Pull and Run Docker run: | @@ -76,5 +76,6 @@ jobs: echo "Security plugin is available" ./gradlew integTest -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername="docker-cluster" -Dhttps=true -Duser=admin -Dpassword=admin else - echo "Security plugin is NOT available, skipping integration tests" + echo "Security plugin is NOT available" + ./gradlew integTest -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername="docker-cluster" fi From a745de025f41b3743b7a9af1d16f8fd851e9edec Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Thu, 11 Jan 2024 20:19:10 +0000 Subject: [PATCH 07/20] Removing non-security integ test from workflow Signed-off-by: Joshua Palis --- .github/workflows/test_security.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/test_security.yml b/.github/workflows/test_security.yml index 458f17540..098ce838e 100644 --- a/.github/workflows/test_security.yml +++ b/.github/workflows/test_security.yml @@ -76,6 +76,5 @@ jobs: echo "Security plugin is available" ./gradlew integTest -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername="docker-cluster" -Dhttps=true -Duser=admin -Dpassword=admin else - echo "Security plugin is NOT available" - ./gradlew integTest -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername="docker-cluster" + echo "Security plugin is NOT available, skipping integration tests" fi From 7ce2d59592bf84437e35ffc4d92dd490df1fd611 Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Thu, 11 Jan 2024 23:37:03 +0000 Subject: [PATCH 08/20] test Signed-off-by: Joshua Palis --- .github/workflows/test_security.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test_security.yml b/.github/workflows/test_security.yml index 098ce838e..fc2d0c69e 100644 --- a/.github/workflows/test_security.yml +++ b/.github/workflows/test_security.yml @@ -61,15 +61,13 @@ jobs: else echo "imagePresent=false" >> $GITHUB_ENV fi - - name: Run Docker Image + - name: Run Flow Framework Test if: env.imagePresent == 'true' run: | cd .. docker run -p 9200:9200 -d -p 9600:9600 -e "discovery.type=single-node" opensearch-flow-framework:test sleep 90 - - name: Run Flow Framework Test - if: env.imagePresent == 'true' - run: | + security=`curl -XGET https://localhost:9200/_cat/plugins?v -u admin:admin --insecure |grep opensearch-security|wc -l` if [ $security -gt 0 ] then From 67613f11c21f2f32b622154e180040cb4c5caacf Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Thu, 11 Jan 2024 23:40:53 +0000 Subject: [PATCH 09/20] test Signed-off-by: Joshua Palis --- .github/workflows/test_security.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test_security.yml b/.github/workflows/test_security.yml index fc2d0c69e..12642bd07 100644 --- a/.github/workflows/test_security.yml +++ b/.github/workflows/test_security.yml @@ -61,13 +61,17 @@ jobs: else echo "imagePresent=false" >> $GITHUB_ENV fi - - name: Run Flow Framework Test + - name: Run Docker Image if: env.imagePresent == 'true' run: | cd .. docker run -p 9200:9200 -d -p 9600:9600 -e "discovery.type=single-node" opensearch-flow-framework:test sleep 90 + docker ps + - name: Run Flow Framework Test + if: env.imagePresent == 'true' + run: | security=`curl -XGET https://localhost:9200/_cat/plugins?v -u admin:admin --insecure |grep opensearch-security|wc -l` if [ $security -gt 0 ] then From 6b2dfb31f2e7f915a63f70c3bc01fc86980b6b2b Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Fri, 12 Jan 2024 00:08:06 +0000 Subject: [PATCH 10/20] Removing docker -ps Signed-off-by: Joshua Palis --- .github/workflows/test_security.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/test_security.yml b/.github/workflows/test_security.yml index 12642bd07..083440eac 100644 --- a/.github/workflows/test_security.yml +++ b/.github/workflows/test_security.yml @@ -67,7 +67,6 @@ jobs: cd .. docker run -p 9200:9200 -d -p 9600:9600 -e "discovery.type=single-node" opensearch-flow-framework:test sleep 90 - docker ps - name: Run Flow Framework Test if: env.imagePresent == 'true' From 714ee3463c391d78bcd666ed8bd2a001463ac96c Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Fri, 12 Jan 2024 19:21:20 +0000 Subject: [PATCH 11/20] Pulling in secuirty as a zipArchive dependency, installed and configured only for security y enabled clusters Signed-off-by: Joshua Palis --- .github/workflows/test_security.yml | 83 +++----- build.gradle | 197 ++++++++++++++---- .../FlowFrameworkRestTestCase.java | 97 ++------- src/test/resources/security/sample.pem | 25 --- src/test/resources/security/test-kirk.jks | Bin 4504 -> 0 bytes 5 files changed, 193 insertions(+), 209 deletions(-) delete mode 100644 src/test/resources/security/sample.pem delete mode 100644 src/test/resources/security/test-kirk.jks diff --git a/.github/workflows/test_security.yml b/.github/workflows/test_security.yml index 083440eac..9e3b6d7ca 100644 --- a/.github/workflows/test_security.yml +++ b/.github/workflows/test_security.yml @@ -8,74 +8,39 @@ on: - "*" jobs: - Build-ad: + Get-CI-Image-Tag: + uses: opensearch-project/opensearch-build/.github/workflows/get-ci-image-tag.yml@main + with: + product: opensearch + + integ-test-with-security-linux: strategy: matrix: - java: [11,17,21] - fail-fast: false + java: [11, 17, 21] - name: Security test workflow for Flow Framework + name: Run Secuirty Integration Tests on Linux runs-on: ubuntu-latest + needs: Get-CI-Image-Tag + container: + # using the same image which is used by opensearch-build team to build the OpenSearch Distribution + # this image tag is subject to change as more dependencies and updates will arrive over time + image: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-version-linux }} + # need to switch to root so that github actions can install runner binary on container without permission issues. + options: --user root steps: + - name: Checkout Flow Framework + uses: actions/checkout@v4 + with: + submodules: true + - name: Setup Java ${{ matrix.java }} uses: actions/setup-java@v4 with: - distribution: 'temurin' java-version: ${{ matrix.java }} - - name: Checkout Flow Framework - uses: actions/checkout@v4 - - - name: Build Flow Framework - run: | - ./gradlew assemble - # example of variables: - # plugin = opensearch-flow-framework-2.10.0.0-SNAPSHOT.zip - # version = 2.10.0, plugin_version = 2.10.0.0, qualifier = SNAPSHOT - - name: Pull and Run Docker - run: | - plugin=`basename $(ls build/distributions/*.zip)` - version=`echo $plugin|awk -F- '{print $4}'| cut -d. -f 1-3` - plugin_version=`echo $plugin|awk -F- '{print $4}'| cut -d. -f 1-4` - qualifier=`echo $plugin|awk -F- '{print $5}'| cut -d. -f 1-1` - - if $qualifier!=SNAPSHOT - then - docker_version=$version-$qualifier - else - docker_version=$version - fi - echo plugin version plugin_version qualifier docker_version - echo "($plugin) ($version) ($plugin_version) ($qualifier) ($docker_version)" - - cd .. - if docker pull opensearchstaging/opensearch:$docker_version - then - echo "FROM opensearchstaging/opensearch:$docker_version" >> Dockerfile - echo "RUN if [ -d /usr/share/opensearch/plugins/opensearch-flow-framework ]; then /usr/share/opensearch/bin/opensearch-plugin remove opensearch-flow-framework; fi" >> Dockerfile - echo "ADD flow-framework/build/distributions/$plugin /tmp/" >> Dockerfile - echo "RUN /usr/share/opensearch/bin/opensearch-plugin install --batch file:/tmp/$plugin" >> Dockerfile - docker build -t opensearch-flow-framework:test . - echo "imagePresent=true" >> $GITHUB_ENV - else - echo "imagePresent=false" >> $GITHUB_ENV - fi - - name: Run Docker Image - if: env.imagePresent == 'true' - run: | - cd .. - docker run -p 9200:9200 -d -p 9600:9600 -e "discovery.type=single-node" opensearch-flow-framework:test - sleep 90 - - - name: Run Flow Framework Test - if: env.imagePresent == 'true' + - name: Run build + # switching the user, as OpenSearch cluster can only be started as root/Administrator on linux-deb/linux-rpm/windows-zip. run: | - security=`curl -XGET https://localhost:9200/_cat/plugins?v -u admin:admin --insecure |grep opensearch-security|wc -l` - if [ $security -gt 0 ] - then - echo "Security plugin is available" - ./gradlew integTest -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername="docker-cluster" -Dhttps=true -Duser=admin -Dpassword=admin - else - echo "Security plugin is NOT available, skipping integration tests" - fi + chown -R 1000:1000 `pwd` + su `id -un 1000` -c "whoami && java -version && ./gradlew integTest -Dhttps=true -Duser=admin -Dpassword=admin" diff --git a/build.gradle b/build.gradle index 6a74fe08a..e0ff929ed 100644 --- a/build.gradle +++ b/build.gradle @@ -1,6 +1,48 @@ import java.nio.file.Files +import org.opensearch.gradle.testclusters.OpenSearchCluster import org.opensearch.gradle.test.RestIntegTestTask import java.util.concurrent.Callable +import java.nio.file.Paths + +buildscript { + ext { + opensearch_version = System.getProperty("opensearch.version", "3.0.0-SNAPSHOT") + buildVersionQualifier = System.getProperty("build.version_qualifier", "") + isSnapshot = "true" == System.getProperty("build.snapshot", "true") + version_tokens = opensearch_version.tokenize('-') + opensearch_build = version_tokens[0] + '.0' + plugin_no_snapshot = opensearch_build + if (buildVersionQualifier) { + opensearch_build += "-${buildVersionQualifier}" + plugin_no_snapshot += "-${buildVersionQualifier}" + } + if (isSnapshot) { + opensearch_build += "-SNAPSHOT" + } + opensearch_group = "org.opensearch" + opensearch_no_snapshot = opensearch_build.replace("-SNAPSHOT","") + System.setProperty('tests.security.manager', 'false') + common_utils_version = System.getProperty("common_utils.version", opensearch_build) + } + + repositories { + mavenLocal() + maven { url "https://aws.oss.sonatype.org/content/repositories/snapshots" } + mavenCentral() + maven { url "https://plugins.gradle.org/m2/" } + maven { url 'https://jitpack.io' } + } + + dependencies { + classpath "org.opensearch.gradle:build-tools:${opensearch_version}" + classpath "com.diffplug.spotless:spotless-plugin-gradle:6.23.3" + classpath "com.github.form-com.diff-coverage-gradle:diff-coverage:0.9.5" + } +} + +plugins { + id "de.undercouch.download" version "5.3.0" +} apply plugin: 'java' apply plugin: 'idea' @@ -39,42 +81,6 @@ thirdPartyAudit.enabled = false // No need to validate pom, as we do not upload to maven/sonatype validateNebulaPom.enabled = false -buildscript { - ext { - opensearch_version = System.getProperty("opensearch.version", "3.0.0-SNAPSHOT") - buildVersionQualifier = System.getProperty("build.version_qualifier", "") - isSnapshot = "true" == System.getProperty("build.snapshot", "true") - version_tokens = opensearch_version.tokenize('-') - opensearch_build = version_tokens[0] + '.0' - plugin_no_snapshot = opensearch_build - if (buildVersionQualifier) { - opensearch_build += "-${buildVersionQualifier}" - plugin_no_snapshot += "-${buildVersionQualifier}" - } - if (isSnapshot) { - opensearch_build += "-SNAPSHOT" - } - opensearch_group = "org.opensearch" - opensearch_no_snapshot = opensearch_build.replace("-SNAPSHOT","") - System.setProperty('tests.security.manager', 'false') - common_utils_version = System.getProperty("common_utils.version", opensearch_build) - } - - repositories { - mavenLocal() - maven { url "https://aws.oss.sonatype.org/content/repositories/snapshots" } - mavenCentral() - maven { url "https://plugins.gradle.org/m2/" } - maven { url 'https://jitpack.io' } - } - - dependencies { - classpath "org.opensearch.gradle:build-tools:${opensearch_version}" - classpath "com.diffplug.spotless:spotless-plugin-gradle:6.23.3" - classpath "com.github.form-com.diff-coverage-gradle:diff-coverage:0.9.5" - } -} - allprojects { // Default to the apache license project.ext.licenseName = 'The Apache Software License, Version 2.0' @@ -156,6 +162,7 @@ dependencies { // ZipArchive dependencies used for integration tests zipArchive group: 'org.opensearch.plugin', name:'opensearch-ml-plugin', version: "${opensearch_build}" + zipArchive group: 'org.opensearch.plugin', name:'opensearch-security', version: "${opensearch_build}" configurations.all { resolutionStrategy { @@ -171,6 +178,96 @@ def opensearch_tmp_dir = rootProject.file('build/private/opensearch_tmp').absolu opensearch_tmp_dir.mkdirs() def _numNodes = findProperty('numNodes') as Integer ?: 1 +ext{ + + configureSecurityPlugin = { OpenSearchCluster cluster -> + + // Retrieve Security Plugin Zip from zipArchive + configurations.zipArchive.asFileTree.each { + if(it.name.contains("opensearch-security")) { + cluster.plugin(provider(new Callable(){ + @Override + RegularFile call() throws Exception { + return new RegularFile() { + @Override + File getAsFile() { + return it + } + } + } + }) + ) + } + } + + cluster.getNodes().forEach { node -> + var creds = node.getCredentials() + if (creds.isEmpty()) { + creds.add(Map.of('username', 'admin', 'password', 'admin')) + } else { + creds.get(0).putAll(Map.of('username', 'admin', 'password', 'admin')) + } + } + + // Config below including files are copied from security demo configuration + ['esnode.pem', 'esnode-key.pem', 'root-ca.pem'].forEach { file -> + File local = Paths.get(opensearch_tmp_dir.absolutePath, file).toFile() + download.run { + src "https://raw.githubusercontent.com/opensearch-project/security/main/bwc-test/src/test/resources/security/" + file + dest local + overwrite false + } + cluster.extraConfigFile(file, local) + } + + // This configuration is copied from the security plugins demo install: + // https://github.com/opensearch-project/security/blob/2.11.1.0/tools/install_demo_configuration.sh#L365-L388 + cluster.setting("plugins.security.ssl.transport.pemcert_filepath", "esnode.pem") + cluster.setting("plugins.security.ssl.transport.pemkey_filepath", "esnode-key.pem") + cluster.setting("plugins.security.ssl.transport.pemtrustedcas_filepath", "root-ca.pem") + cluster.setting("plugins.security.ssl.transport.enforce_hostname_verification", "false") + cluster.setting("plugins.security.ssl.http.enabled", "true") + cluster.setting("plugins.security.ssl.http.pemcert_filepath", "esnode.pem") + cluster.setting("plugins.security.ssl.http.pemkey_filepath", "esnode-key.pem") + cluster.setting("plugins.security.ssl.http.pemtrustedcas_filepath", "root-ca.pem") + cluster.setting("plugins.security.allow_unsafe_democertificates", "true") + cluster.setting("plugins.security.allow_default_init_securityindex", "true") + cluster.setting("plugins.security.unsupported.inject_user.enabled", "true") + + cluster.setting("plugins.security.authcz.admin_dn", "\n- CN=kirk,OU=client,O=client,L=test, C=de") + cluster.setting('plugins.security.restapi.roles_enabled', '["all_access", "security_rest_api_access"]') + cluster.setting('plugins.security.system_indices.enabled', "true") + cluster.setting('plugins.security.system_indices.indices', '[' + + '".plugins-ml-config", ' + + '".plugins-ml-connector", ' + + '".plugins-ml-model-group", ' + + '".plugins-ml-model", ".plugins-ml-task", ' + + '".plugins-ml-conversation-meta", ' + + '".plugins-ml-conversation-interactions", ' + + '".opendistro-alerting-config", ' + + '".opendistro-alerting-alert*", ' + + '".opendistro-anomaly-results*", ' + + '".opendistro-anomaly-detector*", ' + + '".opendistro-anomaly-checkpoints", ' + + '".opendistro-anomaly-detection-state", ' + + '".opendistro-reports-*", ' + + '".opensearch-notifications-*", ' + + '".opensearch-notebooks", ' + + '".opensearch-observability", ' + + '".ql-datasources", ' + + '".opendistro-asynchronous-search-response*", ' + + '".replication-metadata-store", ' + + '".opensearch-knn-models", ' + + '".geospatial-ip2geo-data*", ' + + '".plugins-flow-framework-config", ' + + '".plugins-flow-framework-templates", ' + + '".plugins-flow-framework-state"' + + ']' + ) + cluster.setSecure(true) + } +} + test { include '**/*Tests.class' } @@ -249,19 +346,27 @@ integTest { testClusters.integTest { testDistribution = "ARCHIVE" - // Installs all registered zipArchive dependencies on integTest cluster nodes + // Optionally install security + if (System.getProperty("https") != null && System.getProperty("https") == "true") { + configureSecurityPlugin(testClusters.integTest) + } + + // Installs all registered zipArchive dependencies on integTest cluster nodes except security configurations.zipArchive.asFileTree.each { - plugin(provider(new Callable(){ - @Override - RegularFile call() throws Exception { - return new RegularFile() { + if(!it.name.contains("opensearch-security")) { + plugin(provider(new Callable(){ @Override - File getAsFile() { - return it + RegularFile call() throws Exception { + return new RegularFile() { + @Override + File getAsFile() { + return it + } + } } - } - } - })) + }) + ) + } } // Install Flow Framwork Plugin on integTest cluster nodes diff --git a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java index 5115aa3bf..70aeb2141 100644 --- a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java +++ b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java @@ -16,7 +16,6 @@ import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder; import org.apache.hc.client5.http.ssl.ClientTlsStrategyBuilder; import org.apache.hc.client5.http.ssl.NoopHostnameVerifier; -import org.apache.hc.core5.function.Factory; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpHeaders; import org.apache.hc.core5.http.HttpHost; @@ -30,7 +29,6 @@ import org.opensearch.client.Response; import org.opensearch.client.RestClient; import org.opensearch.client.RestClientBuilder; -import org.opensearch.common.io.PathUtils; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.concurrent.ThreadContext; @@ -49,27 +47,14 @@ import org.opensearch.test.rest.OpenSearchRestTestCase; import org.junit.Before; -import javax.net.ssl.SSLEngine; - import java.io.IOException; -import java.net.URI; -import java.net.URISyntaxException; -import java.nio.file.Path; import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.Objects; import java.util.Optional; import java.util.concurrent.TimeUnit; -import static org.opensearch.client.RestClientBuilder.DEFAULT_MAX_CONN_PER_ROUTE; -import static org.opensearch.client.RestClientBuilder.DEFAULT_MAX_CONN_TOTAL; -import static org.opensearch.commons.ConfigConstants.OPENSEARCH_SECURITY_SSL_HTTP_ENABLED; -import static org.opensearch.commons.ConfigConstants.OPENSEARCH_SECURITY_SSL_HTTP_KEYSTORE_FILEPATH; -import static org.opensearch.commons.ConfigConstants.OPENSEARCH_SECURITY_SSL_HTTP_KEYSTORE_KEYPASSWORD; -import static org.opensearch.commons.ConfigConstants.OPENSEARCH_SECURITY_SSL_HTTP_KEYSTORE_PASSWORD; -import static org.opensearch.commons.ConfigConstants.OPENSEARCH_SECURITY_SSL_HTTP_PEMCERT_FILEPATH; import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_URI; @@ -174,15 +159,7 @@ protected static RestClient readAccessClient() { } protected boolean isHttps() { - boolean isHttps = Optional.ofNullable(System.getProperty("https")).map("true"::equalsIgnoreCase).orElse(false); - if (isHttps) { - // currently only external cluster is supported for security enabled testing - if (!Optional.ofNullable(System.getProperty("tests.rest.cluster")).isPresent()) { - throw new RuntimeException("cluster url should be provided for security enabled testing"); - } - } - - return isHttps; + return Optional.ofNullable(System.getProperty("https")).map("true"::equalsIgnoreCase).orElse(false); } @Override @@ -195,20 +172,6 @@ protected String getProtocol() { return isHttps() ? "https" : "http"; } - @Override - protected Settings restAdminSettings() { - return Settings.builder() - // disable the warning exception for admin client since it's only used for cleanup. - .put("strictDeprecationMode", false) - .put("http.port", 9200) - .put(OPENSEARCH_SECURITY_SSL_HTTP_ENABLED, isHttps()) - .put(OPENSEARCH_SECURITY_SSL_HTTP_PEMCERT_FILEPATH, "sample.pem") - .put(OPENSEARCH_SECURITY_SSL_HTTP_KEYSTORE_FILEPATH, "test-kirk.jks") - .put(OPENSEARCH_SECURITY_SSL_HTTP_KEYSTORE_PASSWORD, "changeit") - .put(OPENSEARCH_SECURITY_SSL_HTTP_KEYSTORE_KEYPASSWORD, "changeit") - .build(); - } - // Utility fn for deleting indices. Should only be used when not allowed in a regular context // (e.g., deleting system indices) protected static void deleteIndexWithAdminClient(String name) throws IOException { @@ -226,41 +189,21 @@ protected static boolean indexExistsWithAdminClient(String indexName) throws IOE @Override protected RestClient buildClient(Settings settings, HttpHost[] hosts) throws IOException { - boolean strictDeprecationMode = settings.getAsBoolean("strictDeprecationMode", true); RestClientBuilder builder = RestClient.builder(hosts); if (isHttps()) { - String keystore = settings.get(OPENSEARCH_SECURITY_SSL_HTTP_KEYSTORE_FILEPATH); - if (Objects.nonNull(keystore)) { - URI uri = null; - try { - uri = this.getClass().getClassLoader().getResource("security/sample.pem").toURI(); - } catch (URISyntaxException e) { - throw new RuntimeException(e); - } - Path configPath = PathUtils.get(uri).getParent().toAbsolutePath(); - return new SecureRestClientBuilder(settings, configPath).build(); - } else { - configureHttpsClient(builder, settings); - builder.setStrictDeprecationMode(strictDeprecationMode); - return builder.build(); - } - + configureHttpsClient(builder, settings); } else { configureClient(builder, settings); - builder.setStrictDeprecationMode(strictDeprecationMode); - return builder.build(); } + builder.setStrictDeprecationMode(false); + return builder.build(); } protected static void configureHttpsClient(RestClientBuilder builder, Settings settings) throws IOException { - Map headers = ThreadContext.buildDefaultHeaders(settings); - Header[] defaultHeaders = new Header[headers.size()]; - int i = 0; - for (Map.Entry entry : headers.entrySet()) { - defaultHeaders[i++] = new BasicHeader(entry.getKey(), entry.getValue()); - } - builder.setDefaultHeaders(defaultHeaders); + // Similar to client configuration with OpenSearch: + // https://github.com/opensearch-project/OpenSearch/blob/2.11.1/test/framework/src/main/java/org/opensearch/test/rest/OpenSearchRestTestCase.java#L841-L863 + // except we set the user name and password builder.setHttpClientConfigCallback(httpClientBuilder -> { String userName = Optional.ofNullable(System.getProperty("user")) .orElseThrow(() -> new RuntimeException("user name is missing")); @@ -274,16 +217,9 @@ protected static void configureHttpsClient(RestClientBuilder builder, Settings s .setHostnameVerifier(NoopHostnameVerifier.INSTANCE) .setSslContext(SSLContextBuilder.create().loadTrustMaterial(null, (chains, authType) -> true).build()) // See https://issues.apache.org/jira/browse/HTTPCLIENT-2219 - .setTlsDetailsFactory(new Factory() { - @Override - public TlsDetails create(final SSLEngine sslEngine) { - return new TlsDetails(sslEngine.getSession(), sslEngine.getApplicationProtocol()); - } - }) + .setTlsDetailsFactory(sslEngine -> new TlsDetails(sslEngine.getSession(), sslEngine.getApplicationProtocol())) .build(); final PoolingAsyncClientConnectionManager connectionManager = PoolingAsyncClientConnectionManagerBuilder.create() - .setMaxConnPerRoute(DEFAULT_MAX_CONN_PER_ROUTE) - .setMaxConnTotal(DEFAULT_MAX_CONN_TOTAL) .setTlsStrategy(tlsStrategy) .build(); return httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider).setConnectionManager(connectionManager); @@ -291,18 +227,21 @@ public TlsDetails create(final SSLEngine sslEngine) { throw new RuntimeException(e); } }); - + Map headers = ThreadContext.buildDefaultHeaders(settings); + Header[] defaultHeaders = new Header[headers.size()]; + int i = 0; + for (Map.Entry entry : headers.entrySet()) { + defaultHeaders[i++] = new BasicHeader(entry.getKey(), entry.getValue()); + } + builder.setDefaultHeaders(defaultHeaders); final String socketTimeoutString = settings.get(CLIENT_SOCKET_TIMEOUT); final TimeValue socketTimeout = TimeValue.parseTimeValue( socketTimeoutString == null ? "60s" : socketTimeoutString, CLIENT_SOCKET_TIMEOUT ); - builder.setRequestConfigCallback(conf -> { - Timeout timeout = Timeout.ofMilliseconds(Math.toIntExact(socketTimeout.getMillis())); - conf.setConnectTimeout(timeout); - conf.setResponseTimeout(timeout); - return conf; - }); + builder.setRequestConfigCallback( + conf -> conf.setResponseTimeout(Timeout.ofMilliseconds(Math.toIntExact(socketTimeout.getMillis()))) + ); if (settings.hasValue(CLIENT_PATH_PREFIX)) { builder.setPathPrefix(settings.get(CLIENT_PATH_PREFIX)); } diff --git a/src/test/resources/security/sample.pem b/src/test/resources/security/sample.pem deleted file mode 100644 index a1fc20a77..000000000 --- a/src/test/resources/security/sample.pem +++ /dev/null @@ -1,25 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIEPDCCAySgAwIBAgIUZjrlDPP8azRDPZchA/XEsx0X2iIwDQYJKoZIhvcNAQEL -BQAwgY8xEzARBgoJkiaJk/IsZAEZFgNjb20xFzAVBgoJkiaJk/IsZAEZFgdleGFt -cGxlMRkwFwYDVQQKDBBFeGFtcGxlIENvbSBJbmMuMSEwHwYDVQQLDBhFeGFtcGxl -IENvbSBJbmMuIFJvb3QgQ0ExITAfBgNVBAMMGEV4YW1wbGUgQ29tIEluYy4gUm9v -dCBDQTAeFw0yMzA4MjkwNDIzMTJaFw0zMzA4MjYwNDIzMTJaMFcxCzAJBgNVBAYT -AmRlMQ0wCwYDVQQHDAR0ZXN0MQ0wCwYDVQQKDARub2RlMQ0wCwYDVQQLDARub2Rl -MRswGQYDVQQDDBJub2RlLTAuZXhhbXBsZS5jb20wggEiMA0GCSqGSIb3DQEBAQUA -A4IBDwAwggEKAoIBAQCm93kXteDQHMAvbUPNPW5pyRHKDD42XGWSgq0k1D29C/Ud -yL21HLzTJa49ZU2ldIkSKs9JqbkHdyK0o8MO6L8dotLoYbxDWbJFW8bp1w6tDTU0 -HGkn47XVu3EwbfrTENg3jFu+Oem6a/501SzITzJWtS0cn2dIFOBimTVpT/4Zv5qr -XA6Cp4biOmoTYWhi/qQl8d0IaADiqoZ1MvZbZ6x76qTrRAbg+UWkpTEXoH1xTc8n -dibR7+HP6OTqCKvo1NhE8uP4pY+fWd6b6l+KLo3IKpfTbAIJXIO+M67FLtWKtttD -ao94B069skzKk6FPgW/OZh6PRCD0oxOavV+ld2SjAgMBAAGjgcYwgcMwRwYDVR0R -BEAwPogFKgMEBQWCEm5vZGUtMC5leGFtcGxlLmNvbYIJbG9jYWxob3N0hxAAAAAA -AAAAAAAAAAAAAAABhwR/AAABMAsGA1UdDwQEAwIF4DAdBgNVHSUEFjAUBggrBgEF -BQcDAQYIKwYBBQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQU0/qDQaY10jIo -wCjLUpz/HfQXyt8wHwYDVR0jBBgwFoAUF4ffoFrrZhKn1dD4uhJFPLcrAJwwDQYJ -KoZIhvcNAQELBQADggEBAD2hkndVih6TWxoe/oOW0i2Bq7ScNO/n7/yHWL04HJmR -MaHv/Xjc8zLFLgHuHaRvC02ikWIJyQf5xJt0Oqu2GVbqXH9PBGKuEP2kCsRRyU27 -zTclAzfQhqmKBTYQ/3lJ3GhRQvXIdYTe+t4aq78TCawp1nSN+vdH/1geG6QjMn5N -1FU8tovDd4x8Ib/0dv8RJx+n9gytI8n/giIaDCEbfLLpe4EkV5e5UNpOnRgJjjuy -vtZutc81TQnzBtkS9XuulovDE0qI+jQrKkKu8xgGLhgH0zxnPkKtUg2I3Aq6zl1L -zYkEOUF8Y25J6WeY88Yfnc0iigI+Pnz5NK8R9GL7TYo= ------END CERTIFICATE----- diff --git a/src/test/resources/security/test-kirk.jks b/src/test/resources/security/test-kirk.jks deleted file mode 100644 index 6dbc51e714784fa58a4209c75deab8b9ed1698ff..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 4504 zcma)AXEYp+vt7GZ$?DyT=tPUf>Rt32Rtcg+B4PQKLo)5nT`xBt(f8 zz4zYx{`1az=l47B(|aH0%$a-V&c}OZ28N+d1QLK?7-~f#Qh{)-@KbUEVuBnDwFn`G zTJSH-2g86X{uc$#Cd7a<{=zALBY_C=KPs|Y1i%~&Sotp~4}12H0!$9GfJy&blEDNC z=>%hA9@l)1y-8vD6#cH^U}=KBI0FdeqXH7J!^nt8{(B;j6byi|5|P@4YY{kr2nhrT zsl1TD93_M516EPM#9d4EG(rsFKtBW4^r*(5KwKbTLB){+^0E(}Q+A7HoW0lrA)@i+ zydGtY^95cAh7C?*2qIcESObb&7%#|($|(-eXIiQ#0>bYpj@=?*4?U=5@-ISTdSa4x zOtEjIWb0hr)D^1HVpX7-CjwnsDG8#WM@AVZvyufeW?}`^GtGW7WcGsVl)G*$?lP3S z^GYelg04B!ZBp4GnwCzq@uOLfB4xY#hE;StB61*Yd8?%(Nl9NW{s3+HODy#ik72s%Hj($a8 zhF0>hs}=106=eHlR<&9zT@LuHAUIZWLFWrKQ#$R3^=pv*&-7e6{O_Ji`|s`^^4v@-Hr>`?(V#!ktZ-$-0?Jt1G-G? zE9HvN@-0iPpKSDRsLacPB>#JY4d$KM!zs7xPBvUu4HQ}!Bz$qc)A`=Ver4EBC?!g7b zuW7GvE*puJA=;!bv2_S?8ZQx_n`M?F&kkb{-h zKwO=OA_@auvAUmAsQW~NjYK|}m{>`{*n^45MJ^ph*%K9}8GnxA%-;D^^-}ih8oWP* zXJ#vzJY3e4?&oSey+_=qv19lq zeLI>%Gjx=y!qVzf%Y&c7dgkjEw?^rl8^KxGs^%{Fd_(b51&l(wYCO&Rc~ZUl5^~y> zc}BJ!4+n2KaS|<{vd#M44my1W|M0Y-gfk9<&l%IBje@31-Sr1Mt!fvT(Pe+Gt$Bz? z_up@HJf$b!)YfI|4{%l^JDxgWvp75|nMzg7E)(qZ%=alvt zXMfZg7Z=_eanGP?tBXFKyvFRu$?uMAzg|k-(32orZccxnHGr$(gM%4Hgc&3blJCi; z6j@^Y3XVg*doBz7pms~Jn7 z9>1&oI7bPBOnn7vyV1x>YahPMDy_bySw!71ij);ebzBEUSZK&o1y43I-AuJKXJ~C3 z{ScF0neCZB8?5r>Px#3V%} zq$OY&i2FZH#6&q5i2Yy421o$-o6P@Z2>vgd4p$sB)+@I7CAQvk>m=OVG#EC`^#8Hx zXo}&oS5+Eg(sw4>QN4_Cy_0U!W9o!pxS@}|4s+L{ow)59*P>fYuDV~JqCwTL5s{)3(v zzbM`$E?)E;`zu*Kjpah> zgQl1ucOJOd1|%MDBk_Lsu64*-#r>9orWT19xT!DnCoNv_AnWczl?5a3@Sd4mtPrx@ z;QPqXK#%ve%3=_Sa$)(zJ)mvCYW0$Uim6bQ!S}#H@uPFY+qvmT_x`cr%&q*~6sufG zKKVZ8ebd?WhVYT)or=?jzV*~PLH&t?CH^KO=IX%=oHNr75%vVz=nN9ipHOrX*7{h! zNkaI3@a@JfTINcbD<@;DNwqa&=S5v4pM=tBEMN8HU3}euq?(dEFWfNC>H+2C+1dBA zFs|s&27315cK^vG`LRKX~{Ugw!|2K~TP_VAqXtzNY6)j={rQ zv73v$!psb1ph9o6`kKlGjC8GEdFX9+@{I}q{33}%?v>$a-cw6HGOOLVnv3ITN_D~k zo^QL%)6K#_{j)b&>8Qy@Eweq=Ne8rKsjJTe)mfDw?scqlc&US2dxU0@o5$(Zu(GB4 zujr5^yZdwlP>E{wrkq=NiW~PQZm5`fJz5m&9I}B^zPVNSSa9vWcXu^m%+bU|aOg5q zK%|a72J^vxGy)&3GlNod=Wt|FBG=mgP)o%{(2PCL$9s$dMvIcv^FdM?hbNYQrX%I| z{binoW_?J27M3L2H_Y4n0!3PGL#b*UxRbpd3l$RLC#I})-32((m#4}vP%kHB3Q7PGLpvuro4~7i2u6z$3ar+YSP2?_%+^%f* zR}5Rl@nUnDVdT&uE_ZP%NU-(Zn*^k2*4S;xubW_f3f-cK+=>uy-sK;&F{mRdpgwIgSHfJSw=22paH-mu>R=3Kf9cR*A_Sjg7q#MM< zqobyHu#q_oM3;REOf&nTGa=n6MK4QZ{pey;iGwX&bnAUCVq`=c0{gykLm{VZo%ulF z*n_LEk%}KbmVW1)L+Ab3sSZPR+Fe*5p$^HC|Oyb{_is> zsuD42;l;BT-a#X6fP(~C+`TP&(``5KD7dp9)GD&EVfNN4Bf@5N63j4c_IOZZ`^gF1 zphj9>;b1JVOWrk`HhO{mmk*Lp>wXpL*r|VQth!^2ajO2-Q$=;E0ZcMzj9V;D}3k7ej?g$MEOSvfr*p<&b z6B?7p3F^a78y9pEd$#q2Pm1b zU#?c^Op~TXSZ`3z2a{A=UzcS`zB%Z|XG2xth@1`h=wY$wyp|u2)s&QN#af+k>`vF! z&{oB;K{Wblwtcc`JH%E!TwV2q%vd}p>iZ9d@C(kwR>Dm)p? zV-i0tv8PP66)jD1#I*Qm*`@U`^o)}|58+bGD1y(EEM_dJh-O9xP^xdF-_Z#qZ&m{c zbC6W;iNU!24Cvnj14>>_V8a{IB$GXu&z39rEKNX_07*3xp*W3rJo!}pp2M0Hwe$#* zi#HgV_>>SSD;YT=uK8*Lu|$a+IIXPF$${!eaPU%X#jh@y96VcWEFGqB#<_hE8QPmQ zO_C$p_nXzGgQtqVrC1t-5`*juoj0Q%VLnw`@Yt&eCg!x)84Pq&N%`@t**O@LYz3OR(@+})Hu&$>gJ;6oxdO{ z&KR3!hDx52>YBb*JE@4B`8}j*yOg=37>&zbSN}#T@GA6n9+dFcA*9q_l2eI%Xh*7~ ziU87?k{%5!@e5oasj8xTY|ysPyOMR3W;w?vvG}prD%~$8wf$j!6&K4LI%aD1$6B&8 zG|Bq_{em<75I~pVeMNJ6Dv9e{<=x@Es?2r|L;d(lJhNv+5~$`ps7`1lAq>B{Ot5Ga z6qD6CeNHKADuYBeC(!$C>E5yJ7O5IFfdN*2lPV*LTj(fX$`T*h6!l7_BFQ%HhbJFp zKUVk@Dl`5ZH)LoQ^{7N6?HyY_;Jo?*Uu#dn_XW`49o!xdK!+JJN_3KD7k@2J((0h0 z?0!++a*3VkR_Y8-s+o<1M(>PCz=|sJMqa z0+r0sNH_$gvD_@AC}TCb8}m~2v}_leWOtWdheZwxJl0i{OGIRcO0iVJ-B>5CgP^O-M7OYVJ*8(0|euX~UGp`sq@@gaEw*bHD4*Dj8_ zPO4*=dce-k-f;9Xl`P>A2U6SzIPhFWQT>2(PjqTMlBf}zL3<&dS*!E0mM}&jbXhc- zAb9}5!V(`=H1zl4fM|8TdAE{XwAuTJ>dTw3o}wzSb&xhxCijhe4Q#{|l(FXGy+A)j zH>IZrWy4|#?wJ-1?zBm;cKLHK*H5ngXeiJE?k?6Lz1i+02rcMG7kNDQlDJ_??0D#; z(Bju>vbV@>IGl97vC?TD(|fa!E?NjDA;*m&#_ZiX>Vgi+wr`atYOngkRp_w%?M~sv zUVImV4>dX4Ih+MO4LU`Ui=K%20a~JOwq1$6)KUw@81y#uUGKMV4>O0ioDGDvtZ{Jl zmay)x!zLD>Hl1jqnzX9b_da}w9xr9S`kQwUZPAei4I5Ao#$N}f9I10=!}MXIF!F!C z6+i+ofRKI2Rvlk8erCmgYu2%A6S_nSX7!cGJQ6pQ{xw*Iw(KXQGft90Ft(YQ<7nw! ROz*Khv5A{`^It3We*oUlR=)rM From cd7ab7006d2610e81d83791bb7e81f6e5afd210b Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Fri, 12 Jan 2024 19:25:38 +0000 Subject: [PATCH 12/20] fixing ci Signed-off-by: Joshua Palis --- .github/workflows/test_security.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.github/workflows/test_security.yml b/.github/workflows/test_security.yml index 9e3b6d7ca..a97fae28d 100644 --- a/.github/workflows/test_security.yml +++ b/.github/workflows/test_security.yml @@ -31,14 +31,11 @@ jobs: steps: - name: Checkout Flow Framework uses: actions/checkout@v4 - with: - submodules: true - - name: Setup Java ${{ matrix.java }} uses: actions/setup-java@v4 with: + distribution: 'temurin' java-version: ${{ matrix.java }} - - name: Run build # switching the user, as OpenSearch cluster can only be started as root/Administrator on linux-deb/linux-rpm/windows-zip. run: | From 7c83c40aa51b839697d65c7bd5e6b75e3511de65 Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Fri, 12 Jan 2024 19:31:49 +0000 Subject: [PATCH 13/20] using v1 Signed-off-by: Joshua Palis --- .github/workflows/test_security.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test_security.yml b/.github/workflows/test_security.yml index a97fae28d..ba600abec 100644 --- a/.github/workflows/test_security.yml +++ b/.github/workflows/test_security.yml @@ -30,9 +30,9 @@ jobs: steps: - name: Checkout Flow Framework - uses: actions/checkout@v4 + uses: actions/checkout@v1 - name: Setup Java ${{ matrix.java }} - uses: actions/setup-java@v4 + uses: actions/setup-java@v1 with: distribution: 'temurin' java-version: ${{ matrix.java }} From 5284003960491db3397534251cd0f53948bc243d Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Fri, 12 Jan 2024 20:28:22 +0000 Subject: [PATCH 14/20] Addressing PR comments, using security.emabled system property instead Signed-off-by: Joshua Palis --- .github/workflows/test_security.yml | 2 +- build.gradle | 42 +++++++++++------------------ 2 files changed, 16 insertions(+), 28 deletions(-) diff --git a/.github/workflows/test_security.yml b/.github/workflows/test_security.yml index ba600abec..373d857ed 100644 --- a/.github/workflows/test_security.yml +++ b/.github/workflows/test_security.yml @@ -40,4 +40,4 @@ jobs: # switching the user, as OpenSearch cluster can only be started as root/Administrator on linux-deb/linux-rpm/windows-zip. run: | chown -R 1000:1000 `pwd` - su `id -un 1000` -c "whoami && java -version && ./gradlew integTest -Dhttps=true -Duser=admin -Dpassword=admin" + su `id -un 1000` -c "whoami && java -version && ./gradlew integTest -Dsecurity.enabled=true" diff --git a/build.gradle b/build.gradle index e0ff929ed..723b26c5e 100644 --- a/build.gradle +++ b/build.gradle @@ -238,27 +238,6 @@ ext{ cluster.setting('plugins.security.restapi.roles_enabled', '["all_access", "security_rest_api_access"]') cluster.setting('plugins.security.system_indices.enabled', "true") cluster.setting('plugins.security.system_indices.indices', '[' + - '".plugins-ml-config", ' + - '".plugins-ml-connector", ' + - '".plugins-ml-model-group", ' + - '".plugins-ml-model", ".plugins-ml-task", ' + - '".plugins-ml-conversation-meta", ' + - '".plugins-ml-conversation-interactions", ' + - '".opendistro-alerting-config", ' + - '".opendistro-alerting-alert*", ' + - '".opendistro-anomaly-results*", ' + - '".opendistro-anomaly-detector*", ' + - '".opendistro-anomaly-checkpoints", ' + - '".opendistro-anomaly-detection-state", ' + - '".opendistro-reports-*", ' + - '".opensearch-notifications-*", ' + - '".opensearch-notebooks", ' + - '".opensearch-observability", ' + - '".ql-datasources", ' + - '".opendistro-asynchronous-search-response*", ' + - '".replication-metadata-store", ' + - '".opensearch-knn-models", ' + - '".geospatial-ip2geo-data*", ' + '".plugins-flow-framework-config", ' + '".plugins-flow-framework-templates", ' + '".plugins-flow-framework-state"' + @@ -295,9 +274,18 @@ integTest { systemProperty 'tests.security.manager', 'false' systemProperty 'java.io.tmpdir', opensearch_tmp_dir.absolutePath systemProperty('project.root', project.rootDir.absolutePath) - systemProperty "https", System.getProperty("https") - systemProperty "user", System.getProperty("user") - systemProperty "password", System.getProperty("password") + systemProperty 'security.enabled', System.getProperty('security.enabled') + var is_https = System.getProperty('https') + var user = System.getProperty('user') + var password = System.getProperty('password') + if (System.getProperty('security.enabled') != null) { + is_https = is_https == null ? 'true' : is_https + user = user == null ? 'admin' : user + password = password == null ? 'admin' : password + } + systemProperty('https', is_https) + systemProperty('user', user) + systemProperty('password', password) // Only rest case can run with remote cluster if (System.getProperty("tests.rest.cluster") != null) { @@ -307,14 +295,14 @@ integTest { } // Exclude integration tests that require security plugin - if (System.getProperty("https") == null || System.getProperty("https") == "false") { + if (System.getProperty("security.enabled") == null || System.getProperty("security.enabled") == "false") { filter { excludeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkSecureRestApiIT" } } // Include only secure integration tests in security enabled clusters - if (System.getProperty("https") != null && System.getProperty("https") == "true") { + if (System.getProperty("security.enabled") != null && System.getProperty("security.enabled") == "true") { filter { includeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkSecureRestApiIT" excludeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkRestApiIT" @@ -347,7 +335,7 @@ testClusters.integTest { testDistribution = "ARCHIVE" // Optionally install security - if (System.getProperty("https") != null && System.getProperty("https") == "true") { + if (System.getProperty("security.enabled") != null && System.getProperty("security.enabled") == "true") { configureSecurityPlugin(testClusters.integTest) } From d4d51387fc8c7a8b1f7883d7e8f9057bff05cfca Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Fri, 12 Jan 2024 21:07:39 +0000 Subject: [PATCH 15/20] Adding remaining read access role tests Signed-off-by: Joshua Palis --- .../FlowFrameworkRestTestCase.java | 19 ++++++++++ .../rest/FlowFrameworkSecureRestApiIT.java | 38 +++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java index 70aeb2141..d8dc7af5e 100644 --- a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java +++ b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java @@ -56,6 +56,7 @@ import java.util.concurrent.TimeUnit; import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; +import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_ID; import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_URI; /** @@ -396,6 +397,24 @@ protected Response getWorkflowStatus(RestClient client, String workflowId, boole } + /** + * Helper method to invoke the Get Workflow Rest Action + * @param client the rest client + * @param workflowId the workflow ID + * @return rest response + * @throws Exception + */ + protected Response getWorkflow(RestClient client, String workflowId) throws Exception { + return TestHelpers.makeRequest( + client, + "GET", + String.format(Locale.ROOT, "%s/%s", WORKFLOW_URI, workflowId), + Collections.emptyMap(), + "", + null + ); + } + /** * Helper method to invoke the Get Workflow Steps Rest Action * @param client the rest client diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java index f179cf7ad..c93a3dd0f 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java @@ -8,8 +8,11 @@ */ package org.opensearch.flowframework.rest; +import org.opensearch.action.search.SearchResponse; +import org.opensearch.client.Response; import org.opensearch.client.ResponseException; import org.opensearch.common.util.io.IOUtils; +import org.opensearch.core.rest.RestStatus; import org.opensearch.flowframework.FlowFrameworkRestTestCase; import org.opensearch.flowframework.TestHelpers; import org.opensearch.flowframework.model.Template; @@ -47,4 +50,39 @@ public void testDeprovisionWorkflowWithReadAcess() throws Exception { assertTrue(exception.getMessage().contains("no permissions for [cluster:admin/opensearch/flow_framework/workflow/deprovision]")); } + public void testGetWorkflowStepsWithReadAccess() throws Exception{ + Response response = getWorkflowStep(readAccessClient()); + assertEquals(RestStatus.OK, TestHelpers.restStatus(response)); + } + + public void testGetWorkflowWithReadAccess() throws Exception { + // No permissions to create, so we assert only that the response status isnt forbidden + ResponseException exception = expectThrows(ResponseException.class, ()-> getWorkflow(readAccessClient(), "test")); + assertTrue(exception.getMessage().contains("There are no templates in the global_context")); + assertEquals(RestStatus.NOT_FOUND, TestHelpers.restStatus(exception.getResponse())); + } + + public void testSearchWorkflowWithReadAccess() throws Exception { + // No permissions to create, so we assert only that the response status isnt forbidden + String termIdQuery = "{\"query\":{\"ids\":{\"values\":[\"test\"]}}}"; + ResponseException exception = expectThrows(ResponseException.class, ()-> searchWorkflows(readAccessClient(), termIdQuery)); + assertTrue(exception.getMessage().contains("no such index [.plugins-flow-framework-templates]")); + assertEquals(RestStatus.NOT_FOUND, TestHelpers.restStatus(exception.getResponse())); + } + + public void testGetWorkflowStateWithReadAccess() throws Exception { + // No permissions to create or provision, so we assert only that the response status isnt forbidden + ResponseException exception = expectThrows(ResponseException.class, ()-> getWorkflowStatus(readAccessClient(), "test", false)); + assertTrue(exception.getMessage().contains("Fail to find workflow")); + assertEquals(RestStatus.NOT_FOUND, TestHelpers.restStatus(exception.getResponse())); + } + + public void testSearchWorkflowStateWithReadAccess() throws Exception { + // No permissions to create, so we assert only that the response status isnt forbidden + String termIdQuery = "{\"query\":{\"ids\":{\"values\":[\"test\"]}}}"; + ResponseException exception = expectThrows(ResponseException.class, ()-> searchWorkflowState(readAccessClient(), termIdQuery)); + assertTrue(exception.getMessage().contains("no such index [.plugins-flow-framework-state]")); + assertEquals(RestStatus.NOT_FOUND, TestHelpers.restStatus(exception.getResponse())); + } + } From f06022eccaba623308eb3b02575a74778f104ead Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Fri, 12 Jan 2024 21:09:50 +0000 Subject: [PATCH 16/20] spotless Signed-off-by: Joshua Palis --- .../flowframework/FlowFrameworkRestTestCase.java | 1 - .../rest/FlowFrameworkSecureRestApiIT.java | 11 +++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java index d8dc7af5e..ff64fdc57 100644 --- a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java +++ b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java @@ -56,7 +56,6 @@ import java.util.concurrent.TimeUnit; import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; -import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_ID; import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_URI; /** diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java index c93a3dd0f..f0ffb63d9 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java @@ -8,7 +8,6 @@ */ package org.opensearch.flowframework.rest; -import org.opensearch.action.search.SearchResponse; import org.opensearch.client.Response; import org.opensearch.client.ResponseException; import org.opensearch.common.util.io.IOUtils; @@ -50,14 +49,14 @@ public void testDeprovisionWorkflowWithReadAcess() throws Exception { assertTrue(exception.getMessage().contains("no permissions for [cluster:admin/opensearch/flow_framework/workflow/deprovision]")); } - public void testGetWorkflowStepsWithReadAccess() throws Exception{ + public void testGetWorkflowStepsWithReadAccess() throws Exception { Response response = getWorkflowStep(readAccessClient()); assertEquals(RestStatus.OK, TestHelpers.restStatus(response)); } public void testGetWorkflowWithReadAccess() throws Exception { // No permissions to create, so we assert only that the response status isnt forbidden - ResponseException exception = expectThrows(ResponseException.class, ()-> getWorkflow(readAccessClient(), "test")); + ResponseException exception = expectThrows(ResponseException.class, () -> getWorkflow(readAccessClient(), "test")); assertTrue(exception.getMessage().contains("There are no templates in the global_context")); assertEquals(RestStatus.NOT_FOUND, TestHelpers.restStatus(exception.getResponse())); } @@ -65,14 +64,14 @@ public void testGetWorkflowWithReadAccess() throws Exception { public void testSearchWorkflowWithReadAccess() throws Exception { // No permissions to create, so we assert only that the response status isnt forbidden String termIdQuery = "{\"query\":{\"ids\":{\"values\":[\"test\"]}}}"; - ResponseException exception = expectThrows(ResponseException.class, ()-> searchWorkflows(readAccessClient(), termIdQuery)); + ResponseException exception = expectThrows(ResponseException.class, () -> searchWorkflows(readAccessClient(), termIdQuery)); assertTrue(exception.getMessage().contains("no such index [.plugins-flow-framework-templates]")); assertEquals(RestStatus.NOT_FOUND, TestHelpers.restStatus(exception.getResponse())); } public void testGetWorkflowStateWithReadAccess() throws Exception { // No permissions to create or provision, so we assert only that the response status isnt forbidden - ResponseException exception = expectThrows(ResponseException.class, ()-> getWorkflowStatus(readAccessClient(), "test", false)); + ResponseException exception = expectThrows(ResponseException.class, () -> getWorkflowStatus(readAccessClient(), "test", false)); assertTrue(exception.getMessage().contains("Fail to find workflow")); assertEquals(RestStatus.NOT_FOUND, TestHelpers.restStatus(exception.getResponse())); } @@ -80,7 +79,7 @@ public void testGetWorkflowStateWithReadAccess() throws Exception { public void testSearchWorkflowStateWithReadAccess() throws Exception { // No permissions to create, so we assert only that the response status isnt forbidden String termIdQuery = "{\"query\":{\"ids\":{\"values\":[\"test\"]}}}"; - ResponseException exception = expectThrows(ResponseException.class, ()-> searchWorkflowState(readAccessClient(), termIdQuery)); + ResponseException exception = expectThrows(ResponseException.class, () -> searchWorkflowState(readAccessClient(), termIdQuery)); assertTrue(exception.getMessage().contains("no such index [.plugins-flow-framework-state]")); assertEquals(RestStatus.NOT_FOUND, TestHelpers.restStatus(exception.getResponse())); } From 359eb809326518fcfdc4e3feb85afc234d59bf31 Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Fri, 12 Jan 2024 22:24:45 +0000 Subject: [PATCH 17/20] Addressing PR comments, adding full access tests, fixing create workflow bug Signed-off-by: Joshua Palis --- .github/workflows/test_security.yml | 4 +- .../CreateWorkflowTransportAction.java | 19 +++--- .../FlowFrameworkRestTestCase.java | 25 +++++--- .../rest/FlowFrameworkSecureRestApiIT.java | 58 ++++++++++++++++--- 4 files changed, 81 insertions(+), 25 deletions(-) diff --git a/.github/workflows/test_security.yml b/.github/workflows/test_security.yml index 373d857ed..c18b2a11a 100644 --- a/.github/workflows/test_security.yml +++ b/.github/workflows/test_security.yml @@ -18,7 +18,7 @@ jobs: matrix: java: [11, 17, 21] - name: Run Secuirty Integration Tests on Linux + name: Run Security Integration Tests on Linux runs-on: ubuntu-latest needs: Get-CI-Image-Tag container: @@ -36,7 +36,7 @@ jobs: with: distribution: 'temurin' java-version: ${{ matrix.java }} - - name: Run build + - name: Run tests # switching the user, as OpenSearch cluster can only be started as root/Administrator on linux-deb/linux-rpm/windows-zip. run: | chown -R 1000:1000 `pwd` diff --git a/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java index 2fd9bd042..2c80969d6 100644 --- a/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java +++ b/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java @@ -17,6 +17,7 @@ import org.opensearch.client.Client; import org.opensearch.common.inject.Inject; import org.opensearch.common.unit.TimeValue; +import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.commons.authuser.User; import org.opensearch.core.action.ActionListener; import org.opensearch.core.rest.RestStatus; @@ -265,13 +266,17 @@ void checkMaxWorkflows(TimeValue requestTimeOut, Integer maxWorkflow, ActionList SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder().query(query).size(0).timeout(requestTimeOut); SearchRequest searchRequest = new SearchRequest(CommonValue.GLOBAL_CONTEXT_INDEX).source(searchSourceBuilder); - - client.search(searchRequest, ActionListener.wrap(searchResponse -> { - internalListener.onResponse(searchResponse.getHits().getTotalHits().value < maxWorkflow); - }, exception -> { - logger.error("Unable to fetch the workflows", exception); - internalListener.onFailure(new FlowFrameworkException("Unable to fetch the workflows", RestStatus.BAD_REQUEST)); - })); + try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) { + client.search(searchRequest, ActionListener.wrap(searchResponse -> { + internalListener.onResponse(searchResponse.getHits().getTotalHits().value < maxWorkflow); + }, exception -> { + logger.error("Unable to fetch the workflows", exception); + internalListener.onFailure(new FlowFrameworkException("Unable to fetch the workflows", RestStatus.BAD_REQUEST)); + })); + } catch (Exception e) { + logger.error("Unable to fetch the workflows", e); + internalListener.onFailure(new FlowFrameworkException(e.getMessage(), ExceptionsHelper.status(e))); + } } } diff --git a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java index ff64fdc57..1d450cd26 100644 --- a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java +++ b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java @@ -54,6 +54,7 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_URI; @@ -63,7 +64,8 @@ */ public abstract class FlowFrameworkRestTestCase extends OpenSearchRestTestCase { - private static String FULL_ACCESS_ROLE = "flow_framework_full_access"; + private static String FLOW_FRAMEWORK_FULL_ACCESS_ROLE = "flow_framework_full_access"; + private static String ML_COMMONS_FULL_ACCESS_ROLE = "ml_full_access"; private static String READ_ACCESS_ROLE = "flow_framework_read_access"; public static String FULL_ACCESS_USER = "fullAccessUser"; public static String READ_ACCESS_USER = "readAccessUser"; @@ -129,8 +131,12 @@ protected void setUpSettings() throws Exception { String fullAccessUserPassword = generatePassword(FULL_ACCESS_USER); String readAccessUserPassword = generatePassword(READ_ACCESS_USER); - // Configure full access user and client - Response response = createUser(FULL_ACCESS_USER, fullAccessUserPassword, FULL_ACCESS_ROLE); + // Configure full access user and client, needs ML Full Access role as well + Response response = createUser( + FULL_ACCESS_USER, + fullAccessUserPassword, + List.of(FLOW_FRAMEWORK_FULL_ACCESS_ROLE, ML_COMMONS_FULL_ACCESS_ROLE) + ); fullAccessClient = new SecureRestClientBuilder( getClusterHosts().toArray(new HttpHost[0]), isHttps(), @@ -139,7 +145,7 @@ protected void setUpSettings() throws Exception { ).setSocketTimeout(60000).build(); // Configure read access user and client - response = createUser(READ_ACCESS_USER, readAccessUserPassword, READ_ACCESS_ROLE); + response = createUser(READ_ACCESS_USER, readAccessUserPassword, List.of(READ_ACCESS_ROLE)); readAccessClient = new SecureRestClientBuilder( getClusterHosts().toArray(new HttpHost[0]), isHttps(), @@ -264,7 +270,7 @@ protected boolean preserveClusterSettings() { } /** - * Create an unguessable password. Simple password are weak due to https://tinyurl.com/383em9zk + * Create an unique password. Simple password are weak due to https://tinyurl.com/383em9zk * @return a random password. */ public static String generatePassword(String username) { @@ -565,12 +571,13 @@ protected List getResourcesCreated(RestClient client, String wo } } - protected Response createUser(String name, String password, String backendRole) throws IOException { + protected Response createUser(String name, String password, List backendRoles) throws IOException { + String backendRolesString = backendRoles.stream().map(item -> "\"" + item + "\"").collect(Collectors.joining(",")); String json = "{\"password\": \"" + password - + "\",\"opendistro_security_roles\": [\"" - + backendRole - + "\"],\"backend_roles\": [],\"attributes\": {}}"; + + "\",\"opendistro_security_roles\": [" + + backendRolesString + + "],\"backend_roles\": [],\"attributes\": {}}"; return TestHelpers.makeRequest( client(), "PUT", diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java index f0ffb63d9..44fed3b5b 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java @@ -8,6 +8,7 @@ */ package org.opensearch.flowframework.rest; +import org.opensearch.action.search.SearchResponse; import org.opensearch.client.Response; import org.opensearch.client.ResponseException; import org.opensearch.common.util.io.IOUtils; @@ -18,6 +19,9 @@ import org.junit.After; import java.io.IOException; +import java.util.Map; + +import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_ID; public class FlowFrameworkSecureRestApiIT extends FlowFrameworkRestTestCase { @@ -57,19 +61,27 @@ public void testGetWorkflowStepsWithReadAccess() throws Exception { public void testGetWorkflowWithReadAccess() throws Exception { // No permissions to create, so we assert only that the response status isnt forbidden ResponseException exception = expectThrows(ResponseException.class, () -> getWorkflow(readAccessClient(), "test")); - assertTrue(exception.getMessage().contains("There are no templates in the global_context")); assertEquals(RestStatus.NOT_FOUND, TestHelpers.restStatus(exception.getResponse())); } public void testSearchWorkflowWithReadAccess() throws Exception { + // Use full access client to invoke create workflow to ensure the template/state indices are created + Template template = TestHelpers.createTemplateFromFile("createconnector-registerremotemodel-deploymodel.json"); + Response response = createWorkflow(fullAccessClient(), template); + assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response)); + // No permissions to create, so we assert only that the response status isnt forbidden String termIdQuery = "{\"query\":{\"ids\":{\"values\":[\"test\"]}}}"; - ResponseException exception = expectThrows(ResponseException.class, () -> searchWorkflows(readAccessClient(), termIdQuery)); - assertTrue(exception.getMessage().contains("no such index [.plugins-flow-framework-templates]")); - assertEquals(RestStatus.NOT_FOUND, TestHelpers.restStatus(exception.getResponse())); + SearchResponse seachResponse = searchWorkflows(readAccessClient(), termIdQuery); + assertEquals(RestStatus.OK, seachResponse.status()); } public void testGetWorkflowStateWithReadAccess() throws Exception { + // Use the full access client to invoke create workflow to ensure the template/state indices are created + Template template = TestHelpers.createTemplateFromFile("createconnector-registerremotemodel-deploymodel.json"); + Response response = createWorkflow(fullAccessClient(), template); + assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response)); + // No permissions to create or provision, so we assert only that the response status isnt forbidden ResponseException exception = expectThrows(ResponseException.class, () -> getWorkflowStatus(readAccessClient(), "test", false)); assertTrue(exception.getMessage().contains("Fail to find workflow")); @@ -77,11 +89,43 @@ public void testGetWorkflowStateWithReadAccess() throws Exception { } public void testSearchWorkflowStateWithReadAccess() throws Exception { + // Use the full access client to invoke create workflow to ensure the template/state indices are created + Template template = TestHelpers.createTemplateFromFile("createconnector-registerremotemodel-deploymodel.json"); + Response response = createWorkflow(fullAccessClient(), template); + assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response)); + // No permissions to create, so we assert only that the response status isnt forbidden String termIdQuery = "{\"query\":{\"ids\":{\"values\":[\"test\"]}}}"; - ResponseException exception = expectThrows(ResponseException.class, () -> searchWorkflowState(readAccessClient(), termIdQuery)); - assertTrue(exception.getMessage().contains("no such index [.plugins-flow-framework-state]")); - assertEquals(RestStatus.NOT_FOUND, TestHelpers.restStatus(exception.getResponse())); + SearchResponse searchResponse = searchWorkflowState(readAccessClient(), termIdQuery); + assertEquals(RestStatus.OK, searchResponse.status()); + } + + public void testCreateProvisionDeprovisionWorkflowWithFullAccess() throws Exception { + // Invoke create workflow API + Template template = TestHelpers.createTemplateFromFile("createconnector-registerremotemodel-deploymodel.json"); + Response response = createWorkflow(fullAccessClient(), template); + assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response)); + + // Retrieve workflow ID + Map responseMap = entityAsMap(response); + String workflowId = (String) responseMap.get(WORKFLOW_ID); + + // Invoke provision API + response = provisionWorkflow(fullAccessClient(), workflowId); + assertEquals(RestStatus.OK, TestHelpers.restStatus(response)); + + // Invoke status API + response = getWorkflowStatus(fullAccessClient(), workflowId, false); + assertEquals(RestStatus.OK, TestHelpers.restStatus(response)); + + // Invoke deprovision API + response = deprovisionWorkflow(fullAccessClient(), workflowId); + assertEquals(RestStatus.OK, TestHelpers.restStatus(response)); + } + + public void testGetWorkflowStepWithFullAccess() throws Exception { + Response response = getWorkflowStep(fullAccessClient()); + assertEquals(RestStatus.OK, TestHelpers.restStatus(response)); } } From c8eb31bbd285c890d615ab47519a9c72542539cc Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Fri, 12 Jan 2024 22:40:51 +0000 Subject: [PATCH 18/20] Added more APIs to full access client test Signed-off-by: Joshua Palis --- .../flowframework/rest/FlowFrameworkSecureRestApiIT.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java index 44fed3b5b..e83e7f08e 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java @@ -110,6 +110,11 @@ public void testCreateProvisionDeprovisionWorkflowWithFullAccess() throws Except Map responseMap = entityAsMap(response); String workflowId = (String) responseMap.get(WORKFLOW_ID); + // Invoke search workflows API + String termIdQuery = "{\"query\":{\"ids\":{\"values\":[\"" + workflowId + "\"]}}}"; + SearchResponse searchResponse = searchWorkflows(fullAccessClient(), termIdQuery); + assertEquals(RestStatus.OK, searchResponse.status()); + // Invoke provision API response = provisionWorkflow(fullAccessClient(), workflowId); assertEquals(RestStatus.OK, TestHelpers.restStatus(response)); @@ -121,6 +126,10 @@ public void testCreateProvisionDeprovisionWorkflowWithFullAccess() throws Except // Invoke deprovision API response = deprovisionWorkflow(fullAccessClient(), workflowId); assertEquals(RestStatus.OK, TestHelpers.restStatus(response)); + + // Invoke delete API + response = deleteWorkflow(fullAccessClient(), workflowId); + assertEquals(RestStatus.OK, TestHelpers.restStatus(response)); } public void testGetWorkflowStepWithFullAccess() throws Exception { From 71fb85bd929ee925ec74dd57bcec50318d966aeb Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Fri, 12 Jan 2024 22:47:36 +0000 Subject: [PATCH 19/20] updating DEVELOPER_GUIDE Signed-off-by: Joshua Palis --- DEVELOPER_GUIDE.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 5e68587e2..16719a890 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -37,8 +37,11 @@ This package uses the [Gradle](https://docs.gradle.org/current/userguide/usergui 1. `./gradlew check` builds and tests. 2. `./gradlew :run` installs and runs ML-Commons and Flow Framework Plugins into a local cluster -3. `./gradlew spotlessApply` formats code. And/or import formatting rules in [formatterConfig.xml](formatter/formatterConfig.xml) with IDE. -4. `./gradlew test` to run the complete test suite. +3. `./gradlew run -Dsecurity.enabled=true` installs, configures and runs ML-Commons, Flow Framework and Security Plugins into a local cluster +4. `./gradlew spotlessApply` formats code. And/or import formatting rules in [formatterConfig.xml](formatter/formatterConfig.xml) with IDE. +5. `./gradlew test` to run the complete test suite. +6. `./gradlew integTest` to run only the non-security enabled integration tests +7. `./gradlew integTest -Dsecurity.enabled=true` to run only the security enabled integration tests #### Building from the IDE From 9402f586ed2b14e807062c231d779ca74addc691 Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Fri, 12 Jan 2024 23:07:30 +0000 Subject: [PATCH 20/20] Updating developer guide, adding back ML Commons security system indices to security plugin configuration Signed-off-by: Joshua Palis --- DEVELOPER_GUIDE.md | 2 ++ build.gradle | 21 +++++++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 16719a890..1a01a02da 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -42,6 +42,8 @@ This package uses the [Gradle](https://docs.gradle.org/current/userguide/usergui 5. `./gradlew test` to run the complete test suite. 6. `./gradlew integTest` to run only the non-security enabled integration tests 7. `./gradlew integTest -Dsecurity.enabled=true` to run only the security enabled integration tests +6. `./gradlew integTestRemote -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername=docker-cluster` to run only the non-security enabled integration tests on a remote cluster +7. `./gradlew integTestRemote -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername=docker-cluster -Dsecurity.enabled=true` to run only the security enabled integration tests on a remote cluster #### Building from the IDE diff --git a/build.gradle b/build.gradle index 723b26c5e..a58d42dce 100644 --- a/build.gradle +++ b/build.gradle @@ -238,6 +238,12 @@ ext{ cluster.setting('plugins.security.restapi.roles_enabled', '["all_access", "security_rest_api_access"]') cluster.setting('plugins.security.system_indices.enabled', "true") cluster.setting('plugins.security.system_indices.indices', '[' + + '".plugins-ml-config", ' + + '".plugins-ml-connector", ' + + '".plugins-ml-model-group", ' + + '".plugins-ml-model", ".plugins-ml-task", ' + + '".plugins-ml-conversation-meta", ' + + '".plugins-ml-conversation-interactions", ' + '".plugins-flow-framework-config", ' + '".plugins-flow-framework-templates", ' + '".plugins-flow-framework-state"' + @@ -379,10 +385,17 @@ testClusters.integTest { task integTestRemote(type: RestIntegTestTask) { testClassesDirs = sourceSets.test.output.classesDirs classpath = sourceSets.test.runtimeClasspath - - systemProperty "https", System.getProperty("https") - systemProperty "user", System.getProperty("user") - systemProperty "password", System.getProperty("password") + var is_https = System.getProperty('https') + var user = System.getProperty('user') + var password = System.getProperty('password') + if (System.getProperty('security.enabled') != null) { + is_https = is_https == null ? 'true' : is_https + user = user == null ? 'admin' : user + password = password == null ? 'admin' : password + } + systemProperty('https', is_https) + systemProperty('user', user) + systemProperty('password', password) systemProperty 'cluster.number_of_nodes', "${_numNodes}" systemProperty 'tests.security.manager', 'false'