From c939b9c400f593d021d167effaf47b081a947850 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Wed, 9 Oct 2024 15:37:54 -0700 Subject: [PATCH] [Backport 2.x] Add ApiSpecFetcher for Fetching and Comparing API Specifications (#906) * Add ApiSpecFetcher for Fetching and Comparing API Specifications (#900) * Added ApiSpecFetcher with test Signed-off-by: Junwei Dai * remove duplication license Signed-off-by: Junwei Dai * Add more test to pass test coverage check Signed-off-by: Junwei Dai * new commit address all comments Signed-off-by: Junwei Dai * new commit address all comments Signed-off-by: Junwei Dai * Addressed all comments Signed-off-by: Junwei Dai --------- Signed-off-by: Junwei Dai Co-authored-by: Junwei Dai (cherry picked from commit 57b8b598423ad3992ff10b4293472dbd36b4f729) Signed-off-by: github-actions[bot] * Add slf4j-api and jackson-core dependencies Signed-off-by: Daniel Widdis --------- Signed-off-by: Junwei Dai Signed-off-by: github-actions[bot] Signed-off-by: Daniel Widdis Co-authored-by: github-actions[bot] Co-authored-by: Junwei Dai Co-authored-by: Daniel Widdis --- CHANGELOG.md | 2 + build.gradle | 19 ++- .../flowframework/common/CommonValue.java | 5 + .../exception/ApiSpecParseException.java | 48 +++++++ .../flowframework/util/ApiSpecFetcher.java | 120 ++++++++++++++++ .../exception/ApiSpecParseExceptionTests.java | 42 ++++++ .../util/ApiSpecFetcherTests.java | 130 ++++++++++++++++++ .../workflow/RegisterAgentTests.java | 18 +++ 8 files changed, 382 insertions(+), 2 deletions(-) create mode 100644 src/main/java/org/opensearch/flowframework/exception/ApiSpecParseException.java create mode 100644 src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java create mode 100644 src/test/java/org/opensearch/flowframework/exception/ApiSpecParseExceptionTests.java create mode 100644 src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java diff --git a/CHANGELOG.md b/CHANGELOG.md index d7bc82134..75b7d95ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) ## [Unreleased 2.x](https://github.com/opensearch-project/flow-framework/compare/2.17...2.x) ### Features +- Add ApiSpecFetcher for Fetching and Comparing API Specifications ([#651](https://github.com/opensearch-project/flow-framework/issues/651)) + ### Enhancements - Incrementally remove resources from workflow state during deprovisioning ([#898](https://github.com/opensearch-project/flow-framework/pull/898)) diff --git a/build.gradle b/build.gradle index c61a9c793..2054cfb19 100644 --- a/build.gradle +++ b/build.gradle @@ -24,7 +24,7 @@ buildscript { opensearch_no_snapshot = opensearch_build.replace("-SNAPSHOT","") System.setProperty('tests.security.manager', 'false') common_utils_version = System.getProperty("common_utils.version", opensearch_build) - + swaggerCoreVersion = "2.2.23" bwcVersionShort = "2.12.0" bwcVersion = bwcVersionShort + ".0" bwcOpenSearchFFDownload = 'https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/' + bwcVersionShort + '/latest/linux/x64/tar/builds/' + @@ -34,6 +34,10 @@ buildscript { bwcFlowFrameworkPath = bwcFilePath + "flowframework/" isSameMajorVersion = opensearch_version.split("\\.")[0] == bwcVersionShort.split("\\.")[0] + swaggerVersion = "2.1.22" + jacksonVersion = "2.18.0" + swaggerCoreVersion = "2.2.23" + } repositories { @@ -167,6 +171,7 @@ dependencies { implementation 'org.junit.jupiter:junit-jupiter:5.11.2' api group: 'org.opensearch', name:'opensearch-ml-client', version: "${opensearch_build}" api group: 'org.opensearch.client', name: 'opensearch-rest-client', version: "${opensearch_version}" + api group: 'org.slf4j', name: 'slf4j-api', version: '1.7.36' implementation group: 'org.apache.commons', name: 'commons-lang3', version: '3.17.0' implementation "org.opensearch:common-utils:${common_utils_version}" implementation "com.amazonaws:aws-encryption-sdk-java:3.0.1" @@ -178,6 +183,16 @@ dependencies { implementation "org.glassfish:jakarta.json:2.0.1" implementation "org.eclipse:yasson:3.0.4" implementation "com.google.code.gson:gson:2.11.0" + // Swagger-Parser dependencies for API consistency tests + implementation "io.swagger.core.v3:swagger-models:${swaggerCoreVersion}" + implementation "io.swagger.core.v3:swagger-core:${swaggerCoreVersion}" + implementation "io.swagger.parser.v3:swagger-parser-core:${swaggerVersion}" + implementation "io.swagger.parser.v3:swagger-parser:${swaggerVersion}" + implementation "io.swagger.parser.v3:swagger-parser-v3:${swaggerVersion}" + implementation "com.fasterxml.jackson.core:jackson-core:${jacksonVersion}" + implementation "com.fasterxml.jackson.core:jackson-databind:${jacksonVersion}" + implementation "com.fasterxml.jackson.datatype:jackson-datatype-jsr310:${jacksonVersion}" + implementation "com.fasterxml.jackson.core:jackson-annotations:${jacksonVersion}" // ZipArchive dependencies used for integration tests zipArchive group: 'org.opensearch.plugin', name:'opensearch-ml-plugin', version: "${opensearch_build}" @@ -188,7 +203,7 @@ dependencies { configurations.all { resolutionStrategy { force("com.google.guava:guava:33.3.1-jre") // CVE for 31.1, keep to force transitive dependencies - force("com.fasterxml.jackson.core:jackson-core:2.17.2") // Dependency Jar Hell + force("com.fasterxml.jackson.core:jackson-core:${jacksonVersion}") // Dependency Jar Hell } } } diff --git a/src/main/java/org/opensearch/flowframework/common/CommonValue.java b/src/main/java/org/opensearch/flowframework/common/CommonValue.java index f291cff1c..898675d94 100644 --- a/src/main/java/org/opensearch/flowframework/common/CommonValue.java +++ b/src/main/java/org/opensearch/flowframework/common/CommonValue.java @@ -233,4 +233,9 @@ private CommonValue() {} public static final String CREATE_INGEST_PIPELINE_MODEL_ID = "create_ingest_pipeline.model_id"; /** The field name for reindex source index substitution */ public static final String REINDEX_SOURCE_INDEX = "reindex.source_index"; + + /**URI for the YAML file of the ML Commons API specification.*/ + public static final String ML_COMMONS_API_SPEC_YAML_URI = + "https://raw.githubusercontent.com/opensearch-project/opensearch-api-specification/refs/heads/main/spec/namespaces/ml.yaml"; + } diff --git a/src/main/java/org/opensearch/flowframework/exception/ApiSpecParseException.java b/src/main/java/org/opensearch/flowframework/exception/ApiSpecParseException.java new file mode 100644 index 000000000..ae77452c7 --- /dev/null +++ b/src/main/java/org/opensearch/flowframework/exception/ApiSpecParseException.java @@ -0,0 +1,48 @@ +/* + * 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.exception; + +import org.opensearch.OpenSearchException; + +import java.util.List; + +/** + * Custom exception to be thrown when an error occurs during the parsing of an API specification. + */ +public class ApiSpecParseException extends OpenSearchException { + + /** + * Constructor with message. + * + * @param message The detail message. + */ + public ApiSpecParseException(String message) { + super(message); + } + + /** + * Constructor with message and cause. + * + * @param message The detail message. + * @param cause The cause of the exception. + */ + public ApiSpecParseException(String message, Throwable cause) { + super(message, cause); + } + + /** + * Constructor with message and list of detailed errors. + * + * @param message The detail message. + * @param details The list of errors encountered during the parsing process. + */ + public ApiSpecParseException(String message, List details) { + super(message + ": " + String.join(", ", details)); + } +} diff --git a/src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java b/src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java new file mode 100644 index 000000000..80be71b65 --- /dev/null +++ b/src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java @@ -0,0 +1,120 @@ +/* + * 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.util; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.flowframework.exception.ApiSpecParseException; +import org.opensearch.rest.RestRequest; + +import java.util.HashSet; +import java.util.List; + +import io.swagger.v3.oas.models.OpenAPI; +import io.swagger.v3.oas.models.Operation; +import io.swagger.v3.oas.models.PathItem; +import io.swagger.v3.oas.models.media.Content; +import io.swagger.v3.oas.models.media.MediaType; +import io.swagger.v3.oas.models.media.Schema; +import io.swagger.v3.oas.models.parameters.RequestBody; +import io.swagger.v3.parser.OpenAPIV3Parser; +import io.swagger.v3.parser.core.models.ParseOptions; +import io.swagger.v3.parser.core.models.SwaggerParseResult; + +/** + * Utility class for fetching and parsing OpenAPI specifications. + */ +public class ApiSpecFetcher { + private static final Logger logger = LogManager.getLogger(ApiSpecFetcher.class); + private static final ParseOptions PARSE_OPTIONS = new ParseOptions(); + private static final OpenAPIV3Parser OPENAPI_PARSER = new OpenAPIV3Parser(); + + static { + PARSE_OPTIONS.setResolve(true); + PARSE_OPTIONS.setResolveFully(true); + } + + /** + * Parses the OpenAPI specification directly from the URI. + * + * @param apiSpecUri URI to the API specification (can be file path or web URI). + * @return Parsed OpenAPI object. + * @throws ApiSpecParseException If parsing fails. + */ + public static OpenAPI fetchApiSpec(String apiSpecUri) { + logger.info("Parsing API spec from URI: {}", apiSpecUri); + SwaggerParseResult result = OPENAPI_PARSER.readLocation(apiSpecUri, null, PARSE_OPTIONS); + OpenAPI openApi = result.getOpenAPI(); + + if (openApi == null) { + throw new ApiSpecParseException("Unable to parse spec from URI: " + apiSpecUri, result.getMessages()); + } + + return openApi; + } + + /** + * Compares the required fields in the API spec with the required enum parameters. + * + * @param requiredEnumParams List of required parameters from the enum. + * @param apiSpecUri URI of the API spec to fetch and compare. + * @param path The API path to check. + * @param method The HTTP method (POST, GET, etc.). + * @return boolean indicating if the required fields match. + */ + public static boolean compareRequiredFields(List requiredEnumParams, String apiSpecUri, String path, RestRequest.Method method) + throws IllegalArgumentException, ApiSpecParseException { + OpenAPI openAPI = fetchApiSpec(apiSpecUri); + + PathItem pathItem = openAPI.getPaths().get(path); + Content content = getContent(method, pathItem); + MediaType mediaType = content.get(XContentType.JSON.mediaTypeWithoutParameters()); + if (mediaType != null) { + Schema schema = mediaType.getSchema(); + + List requiredApiParams = schema.getRequired(); + if (requiredApiParams != null && !requiredApiParams.isEmpty()) { + return new HashSet<>(requiredEnumParams).equals(new HashSet<>(requiredApiParams)); + } + } + return false; + } + + private static Content getContent(RestRequest.Method method, PathItem pathItem) throws IllegalArgumentException, ApiSpecParseException { + Operation operation; + switch (method) { + case POST: + operation = pathItem.getPost(); + break; + case GET: + operation = pathItem.getGet(); + break; + case PUT: + operation = pathItem.getPut(); + break; + case DELETE: + operation = pathItem.getDelete(); + break; + default: + throw new IllegalArgumentException("Unsupported HTTP method: " + method); + } + + if (operation == null) { + throw new IllegalArgumentException("No operation found for the specified method: " + method); + } + + RequestBody requestBody = operation.getRequestBody(); + if (requestBody == null) { + throw new ApiSpecParseException("No requestBody defined for this operation."); + } + + return requestBody.getContent(); + } +} diff --git a/src/test/java/org/opensearch/flowframework/exception/ApiSpecParseExceptionTests.java b/src/test/java/org/opensearch/flowframework/exception/ApiSpecParseExceptionTests.java new file mode 100644 index 000000000..ab93bd66c --- /dev/null +++ b/src/test/java/org/opensearch/flowframework/exception/ApiSpecParseExceptionTests.java @@ -0,0 +1,42 @@ +/* + * 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.exception; + +import org.opensearch.OpenSearchException; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.Arrays; +import java.util.List; + +public class ApiSpecParseExceptionTests extends OpenSearchTestCase { + + public void testApiSpecParseException() { + ApiSpecParseException exception = new ApiSpecParseException("API spec parsing failed"); + assertTrue(exception instanceof OpenSearchException); + assertEquals("API spec parsing failed", exception.getMessage()); + } + + public void testApiSpecParseExceptionWithCause() { + Throwable cause = new RuntimeException("Underlying issue"); + ApiSpecParseException exception = new ApiSpecParseException("API spec parsing failed", cause); + assertTrue(exception instanceof OpenSearchException); + assertEquals("API spec parsing failed", exception.getMessage()); + assertEquals(cause, exception.getCause()); + } + + public void testApiSpecParseExceptionWithDetailedErrors() { + String message = "API spec parsing failed"; + List details = Arrays.asList("Missing required field", "Invalid type"); + ApiSpecParseException exception = new ApiSpecParseException(message, details); + assertTrue(exception instanceof OpenSearchException); + String expectedMessage = "API spec parsing failed: Missing required field, Invalid type"; + assertEquals(expectedMessage, exception.getMessage()); + } + +} diff --git a/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java b/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java new file mode 100644 index 000000000..fb60ae08d --- /dev/null +++ b/src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java @@ -0,0 +1,130 @@ +/* + * 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.util; + +import org.opensearch.flowframework.exception.ApiSpecParseException; +import org.opensearch.rest.RestRequest; +import org.opensearch.test.OpenSearchTestCase; +import org.junit.Before; + +import java.util.Arrays; +import java.util.List; + +import io.swagger.v3.oas.models.OpenAPI; + +import static org.opensearch.flowframework.common.CommonValue.ML_COMMONS_API_SPEC_YAML_URI; +import static org.opensearch.rest.RestRequest.Method.DELETE; +import static org.opensearch.rest.RestRequest.Method.PATCH; +import static org.opensearch.rest.RestRequest.Method.POST; +import static org.opensearch.rest.RestRequest.Method.PUT; + +public class ApiSpecFetcherTests extends OpenSearchTestCase { + + private ApiSpecFetcher apiSpecFetcher; + + @Before + public void setUp() throws Exception { + super.setUp(); + } + + public void testFetchApiSpecSuccess() throws Exception { + + OpenAPI result = ApiSpecFetcher.fetchApiSpec(ML_COMMONS_API_SPEC_YAML_URI); + + assertNotNull("The fetched OpenAPI spec should not be null", result); + } + + public void testFetchApiSpecThrowsException() throws Exception { + String invalidUri = "http://invalid-url.com/fail.yaml"; + + ApiSpecParseException exception = expectThrows(ApiSpecParseException.class, () -> { ApiSpecFetcher.fetchApiSpec(invalidUri); }); + + assertNotNull("Exception should be thrown for invalid URI", exception); + assertTrue(exception.getMessage().contains("Unable to parse spec")); + } + + public void testCompareRequiredFieldsSuccess() throws Exception { + + String path = "/_plugins/_ml/agents/_register"; + RestRequest.Method method = POST; + + // Assuming REGISTER_AGENT step in the enum has these required fields + List expectedRequiredParams = Arrays.asList("name", "type"); + + boolean comparisonResult = ApiSpecFetcher.compareRequiredFields(expectedRequiredParams, ML_COMMONS_API_SPEC_YAML_URI, path, method); + + assertTrue("The required fields should match between API spec and enum", comparisonResult); + } + + public void testCompareRequiredFieldsFailure() throws Exception { + + String path = "/_plugins/_ml/agents/_register"; + RestRequest.Method method = POST; + + List wrongRequiredParams = Arrays.asList("nonexistent_param"); + + boolean comparisonResult = ApiSpecFetcher.compareRequiredFields(wrongRequiredParams, ML_COMMONS_API_SPEC_YAML_URI, path, method); + + assertFalse("The required fields should not match for incorrect input", comparisonResult); + } + + public void testCompareRequiredFieldsThrowsException() throws Exception { + String invalidUri = "http://invalid-url.com/fail.yaml"; + String path = "/_plugins/_ml/agents/_register"; + RestRequest.Method method = PUT; + + Exception exception = expectThrows( + Exception.class, + () -> { ApiSpecFetcher.compareRequiredFields(List.of(), invalidUri, path, method); } + ); + + assertNotNull("An exception should be thrown for an invalid API spec Uri", exception); + assertTrue(exception.getMessage().contains("Unable to parse spec")); + } + + public void testUnsupportedMethodException() throws IllegalArgumentException { + Exception exception = expectThrows(Exception.class, () -> { + ApiSpecFetcher.compareRequiredFields( + List.of("name", "type"), + ML_COMMONS_API_SPEC_YAML_URI, + "/_plugins/_ml/agents/_register", + PATCH + ); + }); + + assertEquals("Unsupported HTTP method: PATCH", exception.getMessage()); + } + + public void testNoOperationFoundException() throws Exception { + Exception exception = expectThrows(IllegalArgumentException.class, () -> { + ApiSpecFetcher.compareRequiredFields( + List.of("name", "type"), + ML_COMMONS_API_SPEC_YAML_URI, + "/_plugins/_ml/agents/_register", + DELETE + ); + }); + + assertEquals("No operation found for the specified method: DELETE", exception.getMessage()); + } + + public void testNoRequestBodyDefinedException() throws ApiSpecParseException { + Exception exception = expectThrows(ApiSpecParseException.class, () -> { + ApiSpecFetcher.compareRequiredFields( + List.of("name", "type"), + ML_COMMONS_API_SPEC_YAML_URI, + "/_plugins/_ml/model_groups/{model_group_id}", + RestRequest.Method.GET + ); + }); + + assertEquals("No requestBody defined for this operation.", exception.getMessage()); + } + +} diff --git a/src/test/java/org/opensearch/flowframework/workflow/RegisterAgentTests.java b/src/test/java/org/opensearch/flowframework/workflow/RegisterAgentTests.java index 626dfdfa1..c2b3dcca1 100644 --- a/src/test/java/org/opensearch/flowframework/workflow/RegisterAgentTests.java +++ b/src/test/java/org/opensearch/flowframework/workflow/RegisterAgentTests.java @@ -13,6 +13,7 @@ import org.opensearch.core.rest.RestStatus; import org.opensearch.flowframework.exception.FlowFrameworkException; import org.opensearch.flowframework.indices.FlowFrameworkIndicesHandler; +import org.opensearch.flowframework.util.ApiSpecFetcher; import org.opensearch.ml.client.MachineLearningNodeClient; import org.opensearch.ml.common.MLAgentType; import org.opensearch.ml.common.agent.LLMSpec; @@ -20,10 +21,12 @@ import org.opensearch.ml.common.agent.MLMemorySpec; import org.opensearch.ml.common.agent.MLToolSpec; import org.opensearch.ml.common.transport.agent.MLRegisterAgentResponse; +import org.opensearch.rest.RestRequest; import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.concurrent.ExecutionException; @@ -31,6 +34,7 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import static org.opensearch.flowframework.common.CommonValue.ML_COMMONS_API_SPEC_YAML_URI; import static org.opensearch.flowframework.common.WorkflowResources.AGENT_ID; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; @@ -150,4 +154,18 @@ public void testRegisterAgentFailure() throws IOException { assertTrue(ex.getCause() instanceof FlowFrameworkException); assertEquals("Failed to register the agent", ex.getCause().getMessage()); } + + public void testApiSpecRegisterAgentInputParamComparison() throws Exception { + List requiredEnumParams = WorkflowStepFactory.WorkflowSteps.REGISTER_AGENT.inputs(); + + boolean isMatch = ApiSpecFetcher.compareRequiredFields( + requiredEnumParams, + ML_COMMONS_API_SPEC_YAML_URI, + "/_plugins/_ml/agents/_register", + RestRequest.Method.POST + ); + + assertTrue(isMatch); + } + }