Removed one of the duplicate '/_nodes/{metric}' and '/_nodes/{node_id…

…}' paths. (#416) * Removed one of the duplicate '/_nodes/{metric}' and '/_nodes/{node_id}' paths. Signed-off-by: dblock <dblock@amazon.com> * Allow either node ID or metric in /_nodes. Signed-off-by: dblock <dblock@amazon.com> * Added tests across namespace. Signed-off-by: dblock <dblock@amazon.com> * Added title to optional refs. Signed-off-by: dblock <dblock@amazon.com> * Fix metric schema. Co-authored-by: Thomas Farr <xtansia@xtansia.com> Signed-off-by: Daniel (dB.) Doubrovkine <dblock@dblock.org> * Updated client generator guide. Signed-off-by: dblock <dblock@amazon.com> * Removed dup parts. Signed-off-by: dblock <dblock@amazon.com> --------- Signed-off-by: dblock <dblock@amazon.com> Signed-off-by: Daniel (dB.) Doubrovkine <dblock@dblock.org> Co-authored-by: Thomas Farr <xtansia@xtansia.com>
opensearch-project · Jul 15, 2024 · b2af866 · b2af866
1 parent fd069ea
commit b2af866
Show file tree

Hide file tree

Showing 8 changed files with 97 additions and 29 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -79,6 +79,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
@@ -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.
+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
@@ -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
@@ -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<NamespaceFile> {
   constructor (folder_path: string) {
@@ -21,16 +22,28 @@ export default class NamespacesFolder extends FolderValidator<NamespaceFile> {
   }
 
   validate_duplicate_paths (): ValidationError[] {
-    const paths: Record<string, string[]> = {}
+    const paths: Record<string, [{ path: string, namespace: string }]> = {}
     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
@@ -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
@@ -0,0 +1,3 @@
+paths:
+  '/nodes/{metric}': {}
+  '/nodes/{node_id}': {}
diff --git a/...sts/linter/fixtures/folder_validators/namespaces/invalid_folder/dup_path_namespace_a.yaml b/...sts/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/...sts/linter/fixtures/folder_validators/namespaces/invalid_folder/dup_path_namespace_b.yaml b/...sts/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}': {}