Skip to content

Commit

Permalink
Delete schema elements that don't match target OpenSearch version.
Browse files Browse the repository at this point in the history
Signed-off-by: dblock <dblock@amazon.com>
  • Loading branch information
dblock committed Jul 16, 2024
1 parent 0c59033 commit c86af16
Show file tree
Hide file tree
Showing 10 changed files with 123 additions and 36 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test-spec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,4 @@ jobs:
run: docker-compose up -d

- name: Run Tests
run: npm run test:spec -- --opensearch-insecure
run: npm run test:spec -- --opensearch-insecure --opensearch-version=${{ matrix.entry.version }}
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Added missing fields to `/_nodes/stats` ([#415](https://github.com/opensearch-project/opensearch-api-specification/pull/415))
- Added missing metrics options to `/_nodes/stats` ([#422](https://github.com/opensearch-project/opensearch-api-specification/pull/422))
- Added tests against OpenSearch 1.3 ([#424](https://github.com/opensearch-project/opensearch-api-specification/pull/424))
- Added support for excluding schema elements that were removed before or added after target version ([#428](https://github.com/opensearch-project/opensearch-api-specification/pull/428))

### Changed

Expand Down
3 changes: 2 additions & 1 deletion tools/src/linter/SchemasValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ export default class SchemasValidator {
}

validate (): ValidationError[] {
this.spec = new OpenApiMerger(this.root_folder, new Logger(LogLevel.error)).merge().components as Record<string, any>
const merger = new OpenApiMerger(this.root_folder, undefined, new Logger(LogLevel.error))
this.spec = merger.merge().spec().components as Record<string, any>
const named_schemas_errors = this.validate_named_schemas()
if (named_schemas_errors.length > 0) return named_schemas_errors
return [
Expand Down
96 changes: 70 additions & 26 deletions tools/src/merger/OpenApiMerger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,25 @@ import { read_yaml, write_yaml } from '../helpers'
import SupersededOpsGenerator from './SupersededOpsGenerator'
import GlobalParamsGenerator from './GlobalParamsGenerator'
import { Logger } from '../Logger'
import * as semver from 'semver'

// Create a single-file OpenAPI spec from multiple files for OpenAPI validation and programmatic consumption
export default class OpenApiMerger {
root_folder: string
spec: Record<string, any>
logger: Logger
target_version?: string

protected _spec: Record<string, any>
protected _merged: boolean = false

paths: Record<string, Record<string, OpenAPIV3.PathItemObject>> = {} // namespace -> path -> path_item_object
schemas: Record<string, Record<string, OpenAPIV3.SchemaObject>> = {} // category -> schema -> schema_object

constructor (root_folder: string, logger: Logger = new Logger()) {
constructor (root_folder: string, target_version?: string, logger: Logger = new Logger()) {
this.logger = logger
this.root_folder = fs.realpathSync(root_folder)
this.spec = {
this.target_version = target_version
this._spec = {
openapi: '3.1.0',
info: read_yaml(`${this.root_folder}/_info.yaml`, true),
paths: {},
Expand All @@ -40,17 +45,27 @@ export default class OpenApiMerger {
}
}

merge (output_path: string = ''): OpenAPIV3.Document {
write_to(output_path: string): OpenApiMerger {
if (!this._merged) { throw "Call merge() first." }
this.logger.info(`Writing ${output_path} ...`)
write_yaml(output_path, this._spec)
return this
}

merge (): OpenApiMerger {
if (this._merged) { throw "Spec already merged." }
this.#merge_schemas()
this.#merge_namespaces()
this.#sort_spec_keys()
this.#generate_global_params()
this.#generate_superseded_ops()
this._merged = true
return this
}

this.logger.info(`Writing ${output_path} ...`)

if (output_path !== '') write_yaml(output_path, this.spec)
return this.spec as OpenAPIV3.Document
spec(): OpenAPIV3.Document {
if (!this._merged) { throw "Call merge() first." }
return this._spec as OpenAPIV3.Document
}

// Merge files from <spec_root>/namespaces folder.
Expand All @@ -59,21 +74,50 @@ export default class OpenApiMerger {
fs.readdirSync(folder).forEach(file => {
this.logger.info(`Merging namespaces in ${folder}/${file} ...`)
const spec = read_yaml(`${folder}/${file}`)
this.redirect_refs_in_namespace(spec)
this.spec.paths = { ...this.spec.paths, ...spec.paths }
this.spec.components.parameters = { ...this.spec.components.parameters, ...spec.components.parameters }
this.spec.components.responses = { ...this.spec.components.responses, ...spec.components.responses }
this.spec.components.requestBodies = { ...this.spec.components.requestBodies, ...spec.components.requestBodies }
this.#redirect_refs_in_namespace(spec)
this._spec.paths = { ...this._spec.paths, ...spec.paths }
this._spec.components.parameters = { ...this._spec.components.parameters, ...spec.components.parameters }
this._spec.components.responses = { ...this._spec.components.responses, ...spec.components.responses }
this._spec.components.requestBodies = { ...this._spec.components.requestBodies, ...spec.components.requestBodies }
})

this.#remove_per_semver(this._spec.paths)
this.#remove_per_semver(this._spec.components.parameters)
this.#remove_per_semver(this._spec.components.responses)
this.#remove_per_semver(this._spec.components.requestBodies)
}

// Remove any elements that are x-version-added/removed incompatible with the target server version.
#remove_per_semver(obj: any): any {
if (this.target_version === undefined) return

for (const key in obj) {
if (typeof obj[key] === 'object') {
const x_version_added = obj[key]['x-version-added']
const x_version_removed = obj[key]['x-version-removed']

if (this.target_version !== undefined) {
if (x_version_added !== undefined && !semver.satisfies(this.target_version, `>= ${x_version_added}`)) {
// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
delete obj[key]
} else if (x_version_removed!== undefined && !semver.satisfies(this.target_version, `< ${x_version_removed}`)) {
// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
delete obj[key]
} else {
this.#remove_per_semver(obj[key])
}
}
}
}
}

// Redirect schema references in namespace files to local references in single-file spec.
redirect_refs_in_namespace (obj: any): void {
#redirect_refs_in_namespace (obj: any): void {
const ref: string = obj?.$ref
if (ref?.startsWith('../schemas/')) { obj.$ref = ref.replace('../schemas/', '#/components/schemas/').replace('.yaml#/components/schemas/', ':') }

for (const key in obj) {
if (typeof obj[key] === 'object') { this.redirect_refs_in_namespace(obj[key]) }
if (typeof obj[key] === 'object') { this.#redirect_refs_in_namespace(obj[key]) }
}
}

Expand All @@ -92,7 +136,7 @@ export default class OpenApiMerger {

Object.entries(this.schemas).forEach(([category, schemas]) => {
Object.entries(schemas).forEach(([name, schema_obj]) => {
this.spec.components.schemas[`${category}:${name}`] = schema_obj
this._spec.components.schemas[`${category}:${name}`] = schema_obj
})
})
}
Expand All @@ -115,26 +159,26 @@ export default class OpenApiMerger {

// Sort keys in the spec to make it easier to read and compare.
#sort_spec_keys (): void {
this.spec.components.schemas = _.fromPairs(Object.entries(this.spec.components.schemas as Document).sort((a, b) => a[0].localeCompare(b[0])))
this.spec.components.parameters = _.fromPairs(Object.entries(this.spec.components.parameters as Document).sort((a, b) => a[0].localeCompare(b[0])))
this.spec.components.responses = _.fromPairs(Object.entries(this.spec.components.responses as Document).sort((a, b) => a[0].localeCompare(b[0])))
this.spec.components.requestBodies = _.fromPairs(Object.entries(this.spec.components.requestBodies as Document).sort((a, b) => a[0].localeCompare(b[0])))

this.spec.paths = _.fromPairs(Object.entries(this.spec.paths as Document).sort((a, b) => a[0].localeCompare(b[0])))
Object.entries(this.spec.paths as Document).forEach(([path, path_item]) => {
this.spec.paths[path] = _.fromPairs(Object.entries(path_item as Document).sort((a, b) => a[0].localeCompare(b[0])))
this._spec.components.schemas = _.fromPairs(Object.entries(this._spec.components.schemas as Document).sort((a, b) => a[0].localeCompare(b[0])))
this._spec.components.parameters = _.fromPairs(Object.entries(this._spec.components.parameters as Document).sort((a, b) => a[0].localeCompare(b[0])))
this._spec.components.responses = _.fromPairs(Object.entries(this._spec.components.responses as Document).sort((a, b) => a[0].localeCompare(b[0])))
this._spec.components.requestBodies = _.fromPairs(Object.entries(this._spec.components.requestBodies as Document).sort((a, b) => a[0].localeCompare(b[0])))

this._spec.paths = _.fromPairs(Object.entries(this._spec.paths as Document).sort((a, b) => a[0].localeCompare(b[0])))
Object.entries(this._spec.paths as Document).forEach(([path, path_item]) => {
this._spec.paths[path] = _.fromPairs(Object.entries(path_item as Document).sort((a, b) => a[0].localeCompare(b[0])))
})
}

// Generate global parameters from _global_params.yaml file.
#generate_global_params (): void {
const gen = new GlobalParamsGenerator(this.root_folder)
gen.generate(this.spec)
gen.generate(this._spec)
}

// Generate superseded operations from _superseded_operations.yaml file.
#generate_superseded_ops (): void {
const gen = new SupersededOpsGenerator(this.root_folder, this.logger)
gen.generate(this.spec)
gen.generate(this._spec)
}
}
5 changes: 3 additions & 2 deletions tools/src/merger/merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@ const command = new Command()
.addOption(new Option('-s, --source <path>', 'path to the root folder of the multi-file spec').default(resolve(__dirname, '../../../spec')))
.addOption(new Option('-o, --output <path>', 'output file name').default(resolve(__dirname, '../../../build/opensearch-openapi.yaml')))
.addOption(new Option('--verbose', 'show merge details').default(false))
.addOption(new Option('--opensearch-version <number>', 'target OpenSearch schema version').default(undefined))
.allowExcessArguments(false)
.parse()

const opts = command.opts()
const logger = new Logger(opts.verbose ? LogLevel.info : LogLevel.warn)
const merger = new OpenApiMerger(opts.source, logger)
const merger = new OpenApiMerger(opts.source, opts.opensearchVersion, logger)
logger.log(`Merging ${opts.source} into ${opts.output} ...`)
merger.merge(opts.output)
merger.merge().write_to(opts.output)
logger.log('Done.')
8 changes: 6 additions & 2 deletions tools/src/tester/MergedOpenApiSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,20 @@ import OpenApiMerger from '../merger/OpenApiMerger';
export default class MergedOpenApiSpec {
logger: Logger
file_path: string
target_version?: string

protected _spec: OpenAPIV3.Document | undefined

constructor (spec_path: string, logger: Logger = new Logger()) {
constructor (spec_path: string, target_version?: string, logger: Logger = new Logger()) {
this.logger = logger
this.file_path = spec_path
this.target_version = target_version
}

spec (): OpenAPIV3.Document {
if (this._spec) return this._spec
const spec = (new OpenApiMerger(this.file_path, this.logger)).merge()
const merger = new OpenApiMerger(this.file_path, this.target_version, this.logger)
const spec = merger.merge().spec()
const ctx = new SpecificationContext(this.file_path)
this.inject_additional_properties(ctx, spec)
this._spec = spec
Expand Down
3 changes: 2 additions & 1 deletion tools/src/tester/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const command = new Command()
)
.addOption(new Option('--verbose', 'whether to print the full stack trace of errors').default(false))
.addOption(new Option('--dry-run', 'dry run only, do not make HTTP requests').default(false))
.addOption(new Option('--opensearch-version <number>', 'target OpenSearch schema version').default(undefined))
.addOption(OPENSEARCH_URL_OPTION)
.addOption(OPENSEARCH_USERNAME_OPTION)
.addOption(OPENSEARCH_PASSWORD_OPTION)
Expand All @@ -50,7 +51,7 @@ const command = new Command()
const opts = command.opts()
const logger = new Logger(opts.verbose ? LogLevel.info : LogLevel.warn)

const spec = new MergedOpenApiSpec(opts.specPath, new Logger(LogLevel.error))
const spec = new MergedOpenApiSpec(opts.specPath, opts.opensearchVersion, new Logger(LogLevel.error))
const http_client = new OpenSearchHttpClient(get_opensearch_opts_from_cli({ opensearchResponseType: 'arraybuffer', ...opts }))
const chapter_reader = new ChapterReader(http_client, logger)
const chapter_evaluator = new ChapterEvaluator(new OperationLocator(spec.spec()), chapter_reader, new SchemaValidator(spec.spec(), logger), logger)
Expand Down
4 changes: 2 additions & 2 deletions tools/tests/merger/OpenApiMerger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import fs from 'fs'
import { Logger, LogLevel } from 'Logger'

test('merge()', () => {
const merger = new OpenApiMerger('./tools/tests/merger/fixtures/spec/', new Logger(LogLevel.error))
merger.merge('./tools/tests/merger/opensearch-openapi.yaml')
const merger = new OpenApiMerger('./tools/tests/merger/fixtures/spec/','2.0', new Logger(LogLevel.error))
merger.merge().write_to('./tools/tests/merger/opensearch-openapi.yaml')
expect(fs.readFileSync('./tools/tests/merger/fixtures/expected.yaml', 'utf8'))
.toEqual(fs.readFileSync('./tools/tests/merger/opensearch-openapi.yaml', 'utf8'))
fs.unlinkSync('./tools/tests/merger/opensearch-openapi.yaml')
Expand Down
21 changes: 20 additions & 1 deletion tools/tests/tester/MergedOpenApiSpec.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@
*/

import { Logger } from 'Logger'
import _ from 'lodash'
import MergedOpenApiSpec from "tester/MergedOpenApiSpec"

describe('unevaluatedProperties', () => {
const spec = new MergedOpenApiSpec('tools/tests/tester/fixtures/specs/complete', new Logger())
const spec = new MergedOpenApiSpec('tools/tests/tester/fixtures/specs/complete', '2.0', new Logger())
const responses: any = spec.spec().components?.responses

test('has an api version', () => {
Expand Down Expand Up @@ -42,4 +43,22 @@ describe('unevaluatedProperties', () => {
const schema = responses['info@404'].content['application/json'].schema
expect(schema.unevaluatedProperties).toEqual({ type: 'object' })
})

test('has matching 2.0 responses', () => {
expect(_.keys(spec.spec().paths['/index']?.get?.responses)).toEqual([
'200', '201', '404', '407', '500'
])
})

test('excludes schema added after 2.0', () => {
expect(spec.spec().paths['/index']?.get?.responses['405']).toBeUndefined()
})

test('excludes schema removed before 2.0', () => {
expect(spec.spec().paths['/index']?.get?.responses['406']).toBeUndefined()
})

test('excludes schema added/removed in the response type', () => {
expect(responses['info@407']).toBeUndefined()
})
})
16 changes: 16 additions & 0 deletions tools/tests/tester/fixtures/specs/complete/namespaces/index.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ paths:
$ref: '#/components/responses/info@201'
'404':
$ref: '#/components/responses/info@404'
'405':
$ref: '#/components/responses/info@405'
x-version-added: 2.99
'406':
$ref: '#/components/responses/info@406'
x-version-removed: 1.99
'407':
$ref: '#/components/responses/info@407'
'500':
$ref: '#/components/responses/info@500'
components:
Expand Down Expand Up @@ -54,6 +62,14 @@ components:
- tagline
unevaluatedProperties:
type: object
info@405:
description: ''
info@406:
description: ''
info@407:
description: ''
x-version-added: 1.99
x-version-removed: 2.99
info@500:
description: ''
content:
Expand Down

0 comments on commit c86af16

Please sign in to comment.