From 130f86c8e0da4b07a8bd3d4dcebc5e7445306932 Mon Sep 17 00:00:00 2001 From: dblock Date: Mon, 17 Jun 2024 16:35:55 -0400 Subject: [PATCH] Fail on missing properties. Signed-off-by: dblock --- package-lock.json | 27 +++++----- package.json | 1 + spec/schemas/_common.yaml | 3 -- tools/src/tester/SchemaValidator.ts | 2 + tools/src/tester/StricterMergedOpenApiSpec.ts | 49 +++++++++++++++++++ tools/src/tester/test.ts | 5 +- 6 files changed, 69 insertions(+), 18 deletions(-) create mode 100644 tools/src/tester/StricterMergedOpenApiSpec.ts diff --git a/package-lock.json b/package-lock.json index e4a923b7a..4ee775469 100644 --- a/package-lock.json +++ b/package-lock.json @@ -27,6 +27,7 @@ "@eslint/js": "^9.1.1", "@types/jest": "^29.5.12", "@typescript-eslint/eslint-plugin": "^6.21.0", + "ajv-errors": "^3.0.0", "eslint": "^8.57.0", "eslint-config-standard-with-typescript": "^43.0.1", "eslint-plugin-eslint-comments": "^3.2.0", @@ -1826,14 +1827,14 @@ } }, "node_modules/ajv": { - "version": "8.15.0", - "resolved": "https://registry.npmjs.org/ajv/-/ajv-8.15.0.tgz", - "integrity": "sha512-15BTtQUOsSrmHCy+B4VnAiJAJxJ8IFgu6fcjFQF3jQYZ78nLSQthlFg4ehp+NLIyfvFgOlxNsjKIEhydtFPVHQ==", + "version": "8.16.0", + "resolved": "https://registry.npmjs.org/ajv/-/ajv-8.16.0.tgz", + "integrity": "sha512-F0twR8U1ZU67JIEtekUcLkXkoO5mMMmgGD8sK/xUFzJ805jxHQl92hImFAqqXMyMYjSPOyUPAwHYhB72g5sTXw==", "dependencies": { "fast-deep-equal": "^3.1.3", - "fast-uri": "^2.3.0", "json-schema-traverse": "^1.0.0", - "require-from-string": "^2.0.2" + "require-from-string": "^2.0.2", + "uri-js": "^4.4.1" }, "funding": { "type": "github", @@ -1853,6 +1854,15 @@ } } }, + "node_modules/ajv-errors": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/ajv-errors/-/ajv-errors-3.0.0.tgz", + "integrity": "sha512-V3wD15YHfHz6y0KdhYFjyy9vWtEVALT9UrxfN3zqlI6dMioHnJrqOYfyPKol3oqrnCM9uwkcdCwkJ0WUcbLMTQ==", + "dev": true, + "peerDependencies": { + "ajv": "^8.0.1" + } + }, "node_modules/ajv-formats": { "version": "3.0.1", "resolved": "https://registry.npmjs.org/ajv-formats/-/ajv-formats-3.0.1.tgz", @@ -3744,11 +3754,6 @@ "integrity": "sha512-DCXu6Ifhqcks7TZKY3Hxp3y6qphY5SJZmrWMDrKcERSOXWQdMhU9Ig/PYrzyw/ul9jOIyh0N4M0tbC5hodg8dw==", "dev": true }, - "node_modules/fast-uri": { - "version": "2.3.0", - "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-2.3.0.tgz", - "integrity": "sha512-eel5UKGn369gGEWOqBShmFJWfq/xSJvsgDzgLYC845GneayWvXBf0lJCBn5qTABfewy1ZDPoaR5OZCP+kssfuw==" - }, "node_modules/fastq": { "version": "1.17.1", "resolved": "https://registry.npmjs.org/fastq/-/fastq-1.17.1.tgz", @@ -6285,7 +6290,6 @@ "version": "2.3.1", "resolved": "https://registry.npmjs.org/punycode/-/punycode-2.3.1.tgz", "integrity": "sha512-vYt7UD1U9Wg6138shLtLOvdAu+8DsC/ilFtEVHcH+wydcSpNE20AfSOduf6MkRFahL5FY7X1oU7nKVZFtfq8Fg==", - "dev": true, "engines": { "node": ">=6" } @@ -7237,7 +7241,6 @@ "version": "4.4.1", "resolved": "https://registry.npmjs.org/uri-js/-/uri-js-4.4.1.tgz", "integrity": "sha512-7rKUyy33Q1yc98pQ1DAmLtwX109F7TIfWlW1Ydo8Wl1ii1SeHieeh0HHfPeL2fMXK6z0s8ecKs9frCuLJvndBg==", - "dev": true, "dependencies": { "punycode": "^2.1.0" } diff --git a/package.json b/package.json index a495dc641..219190098 100644 --- a/package.json +++ b/package.json @@ -36,6 +36,7 @@ "@eslint/js": "^9.1.1", "@types/jest": "^29.5.12", "@typescript-eslint/eslint-plugin": "^6.21.0", + "ajv-errors": "^3.0.0", "eslint": "^8.57.0", "eslint-config-standard-with-typescript": "^43.0.1", "eslint-plugin-eslint-comments": "^3.2.0", diff --git a/spec/schemas/_common.yaml b/spec/schemas/_common.yaml index ce7510e20..392b7fc68 100644 --- a/spec/schemas/_common.yaml +++ b/spec/schemas/_common.yaml @@ -1800,8 +1800,6 @@ components: type: boolean build_type: type: string - distribution: - type: string lucene_version: $ref: '#/components/schemas/VersionString' minimum_index_compatibility_version: @@ -1815,7 +1813,6 @@ components: - build_hash - build_snapshot - build_type - - distribution - lucene_version - minimum_index_compatibility_version - minimum_wire_compatibility_version diff --git a/tools/src/tester/SchemaValidator.ts b/tools/src/tester/SchemaValidator.ts index c96d88575..c5a5ec5a5 100644 --- a/tools/src/tester/SchemaValidator.ts +++ b/tools/src/tester/SchemaValidator.ts @@ -8,6 +8,7 @@ */ import AJV from 'ajv' +import ajv_errors from 'ajv-errors' import addFormats from 'ajv-formats' import { type OpenAPIV3 } from 'openapi-types' import { type Evaluation, Result } from './types/eval.types' @@ -17,6 +18,7 @@ export default class SchemaValidator { constructor (spec: OpenAPIV3.Document) { this.ajv = new AJV({ allErrors: true, strict: true }) addFormats(this.ajv) + ajv_errors(this.ajv, { singleError: true }) this.ajv.addKeyword('discriminator') const schemas = spec.components?.schemas ?? {} for (const key in schemas) this.ajv.addSchema(schemas[key], `#/components/schemas/${key}`) diff --git a/tools/src/tester/StricterMergedOpenApiSpec.ts b/tools/src/tester/StricterMergedOpenApiSpec.ts new file mode 100644 index 000000000..6765b3dd7 --- /dev/null +++ b/tools/src/tester/StricterMergedOpenApiSpec.ts @@ -0,0 +1,49 @@ +/* +* 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. +*/ + +import { type OpenAPIV3 } from 'openapi-types' +import { Logger } from '../Logger' +import { SpecificationContext } from '../linter/utils'; +import { SchemaVisitor } from '../linter/utils/SpecificationVisitor'; +import OpenApiMerger from '../merger/OpenApiMerger'; + +// An augmented spec with additionalProperties: false. +export default class StricterMergedOpenApiSpec { + logger: Logger + file_path: string + protected _spec: OpenAPIV3.Document | undefined + + constructor (specPath: string, logger: Logger = new Logger()) { + this.logger = logger + this.file_path = specPath + } + + spec (): OpenAPIV3.Document { + if (this._spec) return this._spec + const spec = (new OpenApiMerger(this.file_path, this.logger)).merge() + const ctx = new SpecificationContext(this.file_path) + this.inject_additional_properties(ctx, spec) + this._spec = spec + return this._spec + } + + private inject_additional_properties(ctx: SpecificationContext, spec: OpenAPIV3.Document) { + const visitor = new SchemaVisitor((_ctx, schema) => { + if ('properties' in schema) { + // causes any undeclared field in the response to produce an error + (schema as any).additionalProperties = { + not: true, + errorMessage: "property is not defined in the spec" + } + } + }); + + visitor.visit_specification(ctx, spec) + } +} diff --git a/tools/src/tester/test.ts b/tools/src/tester/test.ts index 0d002df95..3d7b3a063 100644 --- a/tools/src/tester/test.ts +++ b/tools/src/tester/test.ts @@ -7,7 +7,6 @@ * compatible open source license. */ -import OpenApiMerger from '../merger/OpenApiMerger' import { LogLevel, Logger } from '../Logger' import TestRunner from './TestRunner' import { Command, Option } from '@commander-js/extra-typings' @@ -26,6 +25,7 @@ import StoryEvaluator from './StoryEvaluator' import { ConsoleResultLogger } from './ResultLogger' import * as process from 'node:process' import SupplementalChapterEvaluator from './SupplementalChapterEvaluator' +import StricterMergedOpenApiSpec from './StricterMergedOpenApiSpec' const command = new Command() .description('Run test stories against the OpenSearch spec.') @@ -48,8 +48,7 @@ const command = new Command() const opts = command.opts() const logger = new Logger(opts.verbose ? LogLevel.info : LogLevel.warn) -const spec = (new OpenApiMerger(opts.specPath, logger)).merge() - +const spec = (new StricterMergedOpenApiSpec(opts.specPath, logger)).spec() const http_client = new OpenSearchHttpClient(get_opensearch_opts_from_cli(opts)) const chapter_reader = new ChapterReader(http_client) const chapter_evaluator = new ChapterEvaluator(new OperationLocator(spec), chapter_reader, new SchemaValidator(spec))