From 9e90ca8e1cb274381e3ce0baf747eb68a560e6c3 Mon Sep 17 00:00:00 2001 From: dblock Date: Fri, 12 Jul 2024 16:56:09 -0400 Subject: [PATCH 1/7] Removed one of the duplicate '/_nodes/{metric}' and '/_nodes/{node_id}' paths. Signed-off-by: dblock --- CHANGELOG.md | 1 + spec/namespaces/nodes.yaml | 17 +------------- .../src/linter/components/NamespacesFolder.ts | 23 +++++++++++++++---- tools/tests/linter/NamespacesFolder.test.ts | 5 ++++ .../namespaces/invalid_folder/dup_path_d.yaml | 3 +++ 5 files changed, 28 insertions(+), 21 deletions(-) create mode 100644 tools/tests/linter/fixtures/folder_validators/namespaces/invalid_folder/dup_path_d.yaml 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/spec/namespaces/nodes.yaml b/spec/namespaces/nodes.yaml index 4db9ebfe..4e12beb5 100644 --- a/spec/namespaces/nodes.yaml +++ b/spec/namespaces/nodes.yaml @@ -253,24 +253,9 @@ paths: responses: '200': $ref: '#/components/responses/nodes.usage@200' - /_nodes/{metric}: - get: - operationId: nodes.info.1 - 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.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 + operationId: nodes.info.1 x-operation-group: nodes.info x-version-added: '1.0' description: Returns information about nodes in the cluster. 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..dcd05090 100644 --- a/tools/tests/linter/NamespacesFolder.test.ts +++ b/tools/tests/linter/NamespacesFolder.test.ts @@ -56,6 +56,11 @@ 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." } ]) }) 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}': {} From 5b399e3187d27f6959c769fc4d15a6a6e3921465 Mon Sep 17 00:00:00 2001 From: dblock Date: Mon, 15 Jul 2024 15:00:50 -0400 Subject: [PATCH 2/7] Allow either node ID or metric in /_nodes. Signed-off-by: dblock --- spec/namespaces/nodes.yaml | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/spec/namespaces/nodes.yaml b/spec/namespaces/nodes.yaml index 4e12beb5..c4283dba 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/{node_id}: + /_nodes/{node_id_or_metric}: get: operationId: nodes.info.1 x-operation-group: nodes.info @@ -262,7 +262,7 @@ paths: 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: @@ -530,15 +530,19 @@ 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: + - $ref: '../schemas/_common.yaml#/components/schemas/NodeIds' + - $ref: '../schemas/nodes.info.yaml#/components/schemas/Metric' style: simple nodes.info::path.node_id: in: path @@ -548,6 +552,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 From b3da76c27c337fce57973102e77e65eb8ed75635 Mon Sep 17 00:00:00 2001 From: dblock Date: Mon, 15 Jul 2024 15:16:51 -0400 Subject: [PATCH 3/7] Added tests across namespace. Signed-off-by: dblock --- tools/tests/linter/NamespacesFolder.test.ts | 5 +++++ .../namespaces/invalid_folder/dup_path_namespace_a.yaml | 6 ++++++ .../namespaces/invalid_folder/dup_path_namespace_b.yaml | 6 ++++++ 3 files changed, 17 insertions(+) create mode 100644 tools/tests/linter/fixtures/folder_validators/namespaces/invalid_folder/dup_path_namespace_a.yaml create mode 100644 tools/tests/linter/fixtures/folder_validators/namespaces/invalid_folder/dup_path_namespace_b.yaml diff --git a/tools/tests/linter/NamespacesFolder.test.ts b/tools/tests/linter/NamespacesFolder.test.ts index dcd05090..92f7dc94 100644 --- a/tools/tests/linter/NamespacesFolder.test.ts +++ b/tools/tests/linter/NamespacesFolder.test.ts @@ -61,6 +61,11 @@ describe('validate()', () => { 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_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}': {} From 899a9e076b5d2a215bbf6d80ea5580f8f4dad862 Mon Sep 17 00:00:00 2001 From: dblock Date: Mon, 15 Jul 2024 17:37:22 -0400 Subject: [PATCH 4/7] Added title to optional refs. Signed-off-by: dblock --- spec/namespaces/nodes.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/namespaces/nodes.yaml b/spec/namespaces/nodes.yaml index c4283dba..25084b77 100644 --- a/spec/namespaces/nodes.yaml +++ b/spec/namespaces/nodes.yaml @@ -542,7 +542,9 @@ components: items: anyOf: - $ref: '../schemas/_common.yaml#/components/schemas/NodeIds' + title: node_id - $ref: '../schemas/nodes.info.yaml#/components/schemas/Metric' + title: metric style: simple nodes.info::path.node_id: in: path From 081b1f20937cdc129abc4b65a891fdbf922fb453 Mon Sep 17 00:00:00 2001 From: "Daniel (dB.) Doubrovkine" Date: Mon, 15 Jul 2024 17:46:42 -0400 Subject: [PATCH 5/7] Fix metric schema. Co-authored-by: Thomas Farr Signed-off-by: Daniel (dB.) Doubrovkine --- spec/namespaces/nodes.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/spec/namespaces/nodes.yaml b/spec/namespaces/nodes.yaml index 25084b77..8610fb38 100644 --- a/spec/namespaces/nodes.yaml +++ b/spec/namespaces/nodes.yaml @@ -538,6 +538,13 @@ components: Supports a comma-separated list, such as node1,node2 or http,ingest. required: true 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' type: array items: anyOf: From d4f3a55c9fd8cab683faa1c30416e25936724a39 Mon Sep 17 00:00:00 2001 From: dblock Date: Mon, 15 Jul 2024 17:52:16 -0400 Subject: [PATCH 6/7] Updated client generator guide. Signed-off-by: dblock --- CLIENT_GENERATOR_GUIDE.md | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) 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. From de7fd22173a8c8bca9fc10c4f94b01bc20818ccb Mon Sep 17 00:00:00 2001 From: dblock Date: Mon, 15 Jul 2024 19:06:28 -0400 Subject: [PATCH 7/7] Removed dup parts. Signed-off-by: dblock --- spec/namespaces/nodes.yaml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/spec/namespaces/nodes.yaml b/spec/namespaces/nodes.yaml index 8610fb38..e99653b1 100644 --- a/spec/namespaces/nodes.yaml +++ b/spec/namespaces/nodes.yaml @@ -545,13 +545,6 @@ components: type: array items: $ref: '../schemas/nodes.info.yaml#/components/schemas/Metric' - type: array - items: - anyOf: - - $ref: '../schemas/_common.yaml#/components/schemas/NodeIds' - title: node_id - - $ref: '../schemas/nodes.info.yaml#/components/schemas/Metric' - title: metric style: simple nodes.info::path.node_id: in: path