diff --git a/CHANGELOG.md b/CHANGELOG.md index fd6064a8..a70d4efe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Fixed `/_data_stream` health status and required fields ([#401](https://github.com/opensearch-project/opensearch-api-specification/pull/401)) - Fixed query DSL `match` that supports a field name and value ([#405](https://github.com/opensearch-project/opensearch-api-specification/pull/405)) - Fixed `/_mapping` with `index` in query ([#385](https://github.com/opensearch-project/opensearch-api-specification/pull/385)) +- Fixed duplicate `/_nodes/{node_id}` path ([#416](https://github.com/opensearch-project/opensearch-api-specification/pull/416)) ### Security diff --git a/CLIENT_GENERATOR_GUIDE.md b/CLIENT_GENERATOR_GUIDE.md index 29077c22..84ea7beb 100644 --- a/CLIENT_GENERATOR_GUIDE.md +++ b/CLIENT_GENERATOR_GUIDE.md @@ -1,3 +1,12 @@ +- [Generate Clients for OpenSearch using OpenAPI Specification](#generate-clients-for-opensearch-using-openapi-specification) + - [The Grouping of API Operations](#the-grouping-of-api-operations) + - [Overloaded Name](#overloaded-name) + - [Handling Bulk Operations](#handling-bulk-operations) + - [Parameter Validation](#parameter-validation) + - [Global Parameters](#global-parameters) + - [Default Parameter Values](#default-parameter-values) + - [Duplicate Paths](#duplicate-paths) + # Generate Clients for OpenSearch using OpenAPI Specification OpenSearch Clients are available in multiple programming languages. The biggest challenge with this is keeping the clients up to date with the latest changes in OpenSearch. To solve this problem, we're automating the process of generating clients for OpenSearch using the OpenAPI specification. While OpenAPI comes with many well established off-the-shelf client generators for most languages, the OpenSearch APIs come with a lot of quirkiness that makes it near impossible to use these off-the-shelf generators. For this reason, we've opted to write our own client generators that is specifically tailored to the OpenSearch APIs. This document will walk you through the process of generating clients from [the published OpenSearch OpenAPI spec](https://github.com/opensearch-project/opensearch-api-specification/releases), more specifically client API methods. @@ -58,4 +67,23 @@ Some clients also check for the validity of query string parameter names to guar All operations in the spec contain a set of parameters that are common across all operations. These parameters are denoted with `x-global: true` vendor extension. The generated clients should find a way to DRY these parameters in type definitions and method documentation. ## Default Parameter Values -Parameters can have default values either through schema or the `x-default` vendor extension. When both are present, `x-default` will take precedence. \ No newline at end of file +Parameters can have default values either through schema or the `x-default` vendor extension. When both are present, `x-default` will take precedence. + +## Duplicate Paths +OpenAPI does not allow duplicate paths, which makes it challenging to express certain OpenSearch APIs, such as `/_nodes/{node_id}` and `/_nodes/{metric}`. In this example the server expects either a `node_id` or a `metric`. The API specification handles this by defining a `/_nodes/{node_id_or_metric}` path, and decorating the `node_id_or_metric` type with a `title` field. + +```yaml +name: node_id_or_metric +schema: +anyOf: + - title: node_id + $ref: '../schemas/_common.yaml#/components/schemas/NodeIds' + - title: metric + type: array + items: + $ref: '../schemas/nodes.info.yaml#/components/schemas/Metric' +``` + +See [#416](https://github.com/opensearch-project/opensearch-api-specification/pull/416) for a complete example. + +Clients should generate multiple methods or a method that accepts multiple parameters in this case. diff --git a/spec/namespaces/nodes.yaml b/spec/namespaces/nodes.yaml index 4db9ebfe..e99653b1 100644 --- a/spec/namespaces/nodes.yaml +++ b/spec/namespaces/nodes.yaml @@ -253,7 +253,7 @@ paths: responses: '200': $ref: '#/components/responses/nodes.usage@200' - /_nodes/{metric}: + /_nodes/{node_id_or_metric}: get: operationId: nodes.info.1 x-operation-group: nodes.info @@ -262,22 +262,7 @@ paths: externalDocs: url: https://opensearch.org/docs/latest/api-reference/nodes-apis/nodes-info/ parameters: - - $ref: '#/components/parameters/nodes.info::path.metric' - - $ref: '#/components/parameters/nodes.info::query.flat_settings' - - $ref: '#/components/parameters/nodes.info::query.timeout' - responses: - '200': - $ref: '#/components/responses/nodes.info@200' - /_nodes/{node_id}: - get: - operationId: nodes.info.2 - x-operation-group: nodes.info - x-version-added: '1.0' - description: Returns information about nodes in the cluster. - externalDocs: - url: https://opensearch.org/docs/latest/api-reference/nodes-apis/nodes-info/ - parameters: - - $ref: '#/components/parameters/nodes.info::path.node_id' + - $ref: '#/components/parameters/nodes.info::path.node_id_or_metric' - $ref: '#/components/parameters/nodes.info::query.flat_settings' - $ref: '#/components/parameters/nodes.info::query.timeout' responses: @@ -545,15 +530,21 @@ components: description: The type to sample. schema: $ref: '../schemas/nodes._common.yaml#/components/schemas/SampleType' - nodes.info::path.metric: + nodes.info::path.node_id_or_metric: in: path - name: metric - description: Limits the information returned to the specific metrics. Supports a comma-separated list, such as http,ingest. + name: node_id_or_metric + description: | + Limits the information returned to a list of node IDs or specific metrics. + Supports a comma-separated list, such as node1,node2 or http,ingest. required: true schema: - type: array - items: - $ref: '../schemas/nodes.info.yaml#/components/schemas/Metric' + anyOf: + - title: node_id + $ref: '../schemas/_common.yaml#/components/schemas/NodeIds' + - title: metric + type: array + items: + $ref: '../schemas/nodes.info.yaml#/components/schemas/Metric' style: simple nodes.info::path.node_id: in: path @@ -563,6 +554,16 @@ components: schema: $ref: '../schemas/_common.yaml#/components/schemas/NodeIds' style: simple + nodes.info::path.metric: + in: path + name: metric + description: Limits the information returned to the specific metrics. Supports a comma-separated list, such as http,ingest. + required: true + schema: + type: array + items: + $ref: '../schemas/nodes.info.yaml#/components/schemas/Metric' + style: simple nodes.info::query.flat_settings: in: query name: flat_settings diff --git a/tools/src/linter/components/NamespacesFolder.ts b/tools/src/linter/components/NamespacesFolder.ts index d7ebf437..71507f7c 100644 --- a/tools/src/linter/components/NamespacesFolder.ts +++ b/tools/src/linter/components/NamespacesFolder.ts @@ -10,6 +10,7 @@ import NamespaceFile from './NamespaceFile' import { type ValidationError } from 'types' import FolderValidator from './base/FolderValidator' +import _ from 'lodash' export default class NamespacesFolder extends FolderValidator { constructor (folder_path: string) { @@ -21,16 +22,28 @@ export default class NamespacesFolder extends FolderValidator { } validate_duplicate_paths (): ValidationError[] { - const paths: Record = {} + const paths: Record = {} for (const file of this.files) { if (file.spec().paths == null) continue Object.keys(file.spec().paths).sort().forEach((path) => { - if (paths[path] == null) paths[path] = [file.namespace] - else paths[path].push(file.namespace) + const normalized_path = path.replaceAll(/\{[^}]+}/g, '{}') + const path_entry = { + path, + namespace: file.namespace + } + if (paths[normalized_path] == null) { + paths[normalized_path] = [path_entry] + } else { + paths[normalized_path].push(path_entry) + } }) } - return Object.entries(paths).map(([path, namespaces]) => { - if (namespaces.length > 1) { return this.error(`Duplicate path '${path}' found in namespaces: ${namespaces.sort().join(', ')}.`) } + return Object.entries(paths).map(([_normalized_path, namespaces]) => { + if (namespaces.length > 1) { + const dup_paths = _.uniq(_.map(namespaces, (entry) => { return `'${entry.path}'` })) + const dup_namespaces = _.uniq(_.map(namespaces, (entry) => entry.namespace)) + return this.error(`Duplicate path${dup_paths.length > 1 ? 's' : ''} ${_.join(dup_paths, ', ')} found in namespace${dup_namespaces.length > 1 ? 's' : ''}: ${_.join(dup_namespaces, ', ')}.`) + } }).filter((e) => e) as ValidationError[] } } diff --git a/tools/tests/linter/NamespacesFolder.test.ts b/tools/tests/linter/NamespacesFolder.test.ts index 35b48bab..92f7dc94 100644 --- a/tools/tests/linter/NamespacesFolder.test.ts +++ b/tools/tests/linter/NamespacesFolder.test.ts @@ -56,6 +56,16 @@ describe('validate()', () => { file: 'invalid_folder/', location: 'Folder', message: "Duplicate path '/{index}/_rollover' found in namespaces: dup_path_a, dup_path_b, dup_path_c." + }, + { + file: 'invalid_folder/', + location: 'Folder', + message: "Duplicate paths '/nodes/{metric}', '/nodes/{node_id}' found in namespace: dup_path_d." + }, + { + file: 'invalid_folder/', + location: 'Folder', + message: "Duplicate paths '/indices/{metric}', '/indices/{node_id}' found in namespaces: dup_path_namespace_a, dup_path_namespace_b." } ]) }) diff --git a/tools/tests/linter/fixtures/folder_validators/namespaces/invalid_folder/dup_path_d.yaml b/tools/tests/linter/fixtures/folder_validators/namespaces/invalid_folder/dup_path_d.yaml new file mode 100644 index 00000000..20067892 --- /dev/null +++ b/tools/tests/linter/fixtures/folder_validators/namespaces/invalid_folder/dup_path_d.yaml @@ -0,0 +1,3 @@ +paths: + '/nodes/{metric}': {} + '/nodes/{node_id}': {} diff --git a/tools/tests/linter/fixtures/folder_validators/namespaces/invalid_folder/dup_path_namespace_a.yaml b/tools/tests/linter/fixtures/folder_validators/namespaces/invalid_folder/dup_path_namespace_a.yaml new file mode 100644 index 00000000..88c1dc7c --- /dev/null +++ b/tools/tests/linter/fixtures/folder_validators/namespaces/invalid_folder/dup_path_namespace_a.yaml @@ -0,0 +1,6 @@ +openapi: 3.1.0 +info: + title: OpenSearch Indices API (Namespace 1) + version: 1.0.0 +paths: + '/indices/{metric}': {} diff --git a/tools/tests/linter/fixtures/folder_validators/namespaces/invalid_folder/dup_path_namespace_b.yaml b/tools/tests/linter/fixtures/folder_validators/namespaces/invalid_folder/dup_path_namespace_b.yaml new file mode 100644 index 00000000..6e8dcd79 --- /dev/null +++ b/tools/tests/linter/fixtures/folder_validators/namespaces/invalid_folder/dup_path_namespace_b.yaml @@ -0,0 +1,6 @@ +openapi: 3.1.0 +info: + title: OpenSearch Indices API (Namespace 2) + version: 1.0.0 +paths: + '/indices/{node_id}': {}