feat(rulesets): add oas3-server-variables rule (#2526)

Co-authored-by: Nauman Ali <mnaumanali94@gmail.com> Co-authored-by: Billie Hilton <587740+billiegoose@users.noreply.github.com>
stoplightio · Aug 29, 2023 · 4c4de85 · fidgi · Aug 31, 2023 · 4c4de85
1 parent 2a553a9
commit 4c4de85
Show file tree

Hide file tree

Showing 18 changed files with 673 additions and 85 deletions.
diff --git a/docs/reference/openapi-rules.md b/docs/reference/openapi-rules.md
@@ -846,3 +846,79 @@ schemas:
       - name
       - petType
 ```
+
+### oas3-server-variables
+
+This rule ensures that server variables defined in OpenAPI Specification 3 (OAS3) and 3.1 are valid, not unused, and result in a valid URL. Properly defining and using server variables is crucial for the accurate representation of API endpoints and preventing potential misconfigurations or security issues.
+
+**Recommended**: Yes
+
+**Bad Examples**
+
+1. **Missing definition for a URL variable**:
+
+```yaml
+servers:
+  - url: "https://api.{region}.example.com/v1"
+    variables:
+      version:
+        default: "v1"
+```
+
+In this example, the variable **`{region}`** in the URL is not defined within the **`variables`** object.
+
+2. **Unused URL variable:**
+
+```yaml
+servers:
+  - url: "https://api.example.com/v1"
+    variables:
+      region:
+        default: "us-west"
+```
+
+Here, the variable **`region`** is defined but not used in the server URL.
+
+3. **Invalid default value for an allowed value variable**:
+
+```yaml
+servers:
+  - url: "https://api.{region}.example.com/v1"
+    variables:
+      region:
+        default: "us-south"
+        enum:
+          - "us-west"
+          - "us-east"
+```
+
+The default value 'us-south' isn't one of the allowed values in the **`enum`**.
+
+4. **Invalid resultant URL**:
+
+```yaml
+servers:
+  - url: "https://api.example.com:{port}/v1"
+    variables:
+      port:
+        default: "8o80"
+```
+
+Substituting the default value of **`{port}`** results in an invalid URL.
+
+**Good Example**
+
+```yaml
+servers:
+  - url: "https://api.{region}.example.com/{version}"
+    variables:
+      region:
+        default: "us-west"
+        enum:
+          - "us-west"
+          - "us-east"
+      version:
+        default: "v1"
+```
+
+In this example, both **`{region}`** and **`{version}`** variables are properly defined and used in the server URL. Also, the default value for **`region`** is within the allowed values.
diff --git a/packages/rulesets/src/asyncapi/__tests__/asyncapi-server-variables.test.ts b/packages/rulesets/src/asyncapi/__tests__/asyncapi-server-variables.test.ts
@@ -92,7 +92,7 @@ testRule('asyncapi-server-variables', [
   },
 
   {
-    name: 'server has redundant url variables',
+    name: 'server has unused url variables',
     document: {
       asyncapi: '2.0.0',
       servers: {
@@ -109,20 +109,20 @@ testRule('asyncapi-server-variables', [
     },
     errors: [
       {
-        message: 'Server\'s "variables" object has redundant defined "anotherParam1" url variable.',
+        message: 'Server\'s "variables" object has unused defined "anotherParam1" url variable.',
         path: ['servers', 'production', 'variables', 'anotherParam1'],
         severity: DiagnosticSeverity.Error,
       },
       {
-        message: 'Server\'s "variables" object has redundant defined "anotherParam2" url variable.',
+        message: 'Server\'s "variables" object has unused defined "anotherParam2" url variable.',
         path: ['servers', 'production', 'variables', 'anotherParam2'],
         severity: DiagnosticSeverity.Error,
       },
     ],
   },
 
   {
-    name: 'server has redundant url variables (in the components.servers)',
+    name: 'server has unused url variables (in the components.servers)',
     document: {
       asyncapi: '2.3.0',
       components: {
@@ -141,12 +141,12 @@ testRule('asyncapi-server-variables', [
     },
     errors: [
       {
-        message: 'Server\'s "variables" object has redundant defined "anotherParam1" url variable.',
+        message: 'Server\'s "variables" object has unused defined "anotherParam1" url variable.',
         path: ['components', 'servers', 'production', 'variables', 'anotherParam1'],
         severity: DiagnosticSeverity.Error,
       },
       {
-        message: 'Server\'s "variables" object has redundant defined "anotherParam2" url variable.',
+        message: 'Server\'s "variables" object has unused defined "anotherParam2" url variable.',
         path: ['components', 'servers', 'production', 'variables', 'anotherParam2'],
         severity: DiagnosticSeverity.Error,
       },

diff --git a/packages/rulesets/src/asyncapi/functions/asyncApi2ChannelParameters.ts b/packages/rulesets/src/asyncapi/functions/asyncApi2ChannelParameters.ts
@@ -1,10 +1,9 @@
 import { createRulesetFunction } from '@stoplight/spectral-core';
 
-import { parseUrlVariables } from './utils/parseUrlVariables';
-import { getMissingProps } from './utils/getMissingProps';
-import { getRedundantProps } from './utils/getRedundantProps';
-
 import type { IFunctionResult } from '@stoplight/spectral-core';
+import { parseUrlVariables } from '../../shared/functions/serverVariables/utils/parseUrlVariables';
+import { getMissingProps } from '../../shared/utils/getMissingProps';
+import { getRedundantProps } from '../../shared/utils/getRedundantProps';
 
 export default createRulesetFunction<{ parameters: Record<string, unknown> }, null>(
   {
@@ -26,7 +25,7 @@ export default createRulesetFunction<{ parameters: Record<string, unknown> }, nu
     const parameters = parseUrlVariables(path);
     if (parameters.length === 0) return;
 
-    const missingParameters = getMissingProps(parameters, targetVal.parameters);
+    const missingParameters = getMissingProps(parameters, Object.keys(targetVal.parameters));
     if (missingParameters.length) {
       results.push({
         message: `Not all channel's parameters are described with "parameters" object. Missed: ${missingParameters.join(
@@ -36,7 +35,7 @@ export default createRulesetFunction<{ parameters: Record<string, unknown> }, nu
       });
     }
 
-    const redundantParameters = getRedundantProps(parameters, targetVal.parameters);
+    const redundantParameters = getRedundantProps(parameters, Object.keys(targetVal.parameters));
     if (redundantParameters.length) {
       redundantParameters.forEach(param => {
         results.push({

diff --git a/packages/rulesets/src/asyncapi/functions/asyncApi2ServerVariables.ts b/packages/rulesets/src/asyncapi/functions/asyncApi2ServerVariables.ts
diff --git a/packages/rulesets/src/asyncapi/functions/utils/getMissingProps.ts b/packages/rulesets/src/asyncapi/functions/utils/getMissingProps.ts
diff --git a/packages/rulesets/src/asyncapi/functions/utils/getRedundantProps.ts b/packages/rulesets/src/asyncapi/functions/utils/getRedundantProps.ts
diff --git a/packages/rulesets/src/asyncapi/index.ts b/packages/rulesets/src/asyncapi/index.ts
@@ -16,7 +16,7 @@ import asyncApi2MessageIdUniqueness from './functions/asyncApi2MessageIdUniquene
 import asyncApi2OperationIdUniqueness from './functions/asyncApi2OperationIdUniqueness';
 import asyncApi2SchemaValidation from './functions/asyncApi2SchemaValidation';
 import asyncApi2PayloadValidation from './functions/asyncApi2PayloadValidation';
-import asyncApi2ServerVariables from './functions/asyncApi2ServerVariables';
+import serverVariables from '../shared/functions/serverVariables';
 import { uniquenessTags } from '../shared/functions';
 import asyncApi2Security from './functions/asyncApi2Security';
 import { latestVersion } from './functions/utils/specs';
@@ -370,7 +370,7 @@ export default {
       recommended: true,
       given: ['$.servers.*', '$.components.servers.*'],
       then: {
-        function: asyncApi2ServerVariables,
+        function: serverVariables,
       },
     },
     'asyncapi-server-no-empty-variable': {