Skip to content

Commit

Permalink
Prepare files for Vale.
Browse files Browse the repository at this point in the history
Signed-off-by: dblock <[email protected]>
  • Loading branch information
dblock committed Oct 21, 2024
1 parent 91de0fa commit efba839
Show file tree
Hide file tree
Showing 13 changed files with 286 additions and 51 deletions.
6 changes: 0 additions & 6 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,3 @@ jobs:

- name: Lint
run: npm run lint

- name: Check Style
uses: errata-ai/[email protected]
with:
version: 3.7.1
files: '["spec","tests"]'
31 changes: 31 additions & 0 deletions .github/workflows/style.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
name: Style

on: [pull_request, push]

jobs:
check:
runs-on: ubuntu-latest
steps:
- name: Checkout Repo
uses: actions/checkout@v4

- name: Setup Node.js
uses: actions/setup-node@v3
with:
node-version: "20"

Check failure on line 15 in .github/workflows/style.yml

View workflow job for this annotation

GitHub Actions / lint

Strings must use singlequote

- name: Install Dependencies
run: npm ci

- name: Prepare for Vale (spec)
run: npm run style:prepare -- --source spec

- name: Prepare for Vale (test)
run: npm run style:prepare -- --source tests

- name: Check Style
uses: errata-ai/[email protected]

Check warning on line 27 in .github/workflows/style.yml

View workflow job for this annotation

GitHub Actions / check

[vale] reported by reviewdog 🐶 [OpenSearch.Version] In 'v2.1', spell out 'version'. Raw Output: {"message": "[OpenSearch.Version] In 'v2.1', spell out 'version'.", "location": {"path": ".github/workflows/style.yml", "range": {"start": {"line": 27, "column": 37}}}, "severity": "WARNING"}
with:
version: 3.7.1
vale_flags: --ignore-syntax
files: "['spec','tests']"
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Added `404` response for `DELETE /{index}`, `GET /{index}/_doc/{id}`, `DELETE /{index}/_doc/{id}` ([#589](https://github.com/opensearch-project/opensearch-api-specification/pull/589))
- Added ability to pass `InlineScript` as a simple string ([#605](https://github.com/opensearch-project/opensearch-api-specification/pull/605))
- Added `config_id` and `config_id_list` to `/_plugins/_notifications/configs` query parameters ([#594](https://github.com/opensearch-project/opensearch-api-specification/pull/594))
- Added a spec style checker [#620](https://github.com/opensearch-project/opensearch-api-specification/pull/620).

### Changed

Expand Down
93 changes: 50 additions & 43 deletions DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
@@ -1,39 +1,32 @@
<!-- TOC -->
* [Developer Guide](#developer-guide)
* [Getting Started](#getting-started)
* [Specification](#specification)
* [File Structure](#file-structure)
* [Grouping Operations](#grouping-operations)
* [Grouping Schemas](#grouping-schemas)
* [Superseded Operations](#superseded-operations)
* [Global Parameters](#global-parameters)
* [OpenAPI Extensions](#openapi-extensions)
* [Writing Spec Tests](#writing-spec-tests)
* [Tools](#tools)
* [Setup](#setup)
* [Spec Merger](#spec-merger)
* [Arguments](#arguments)
* [Example](#example)
* [Spec Linter](#spec-linter)
* [Arguments](#arguments-1)
* [Example](#example-1)
* [Spec Tester](#spec-tester)
* [Dump Cluster Spec](#dump-cluster-spec)
* [Arguments](#arguments-2)
* [Example](#example-2)
* [Coverage](#coverage)
* [Arguments](#arguments-3)
* [Example](#example-3)
* [Tools Testing](#tools-testing)
* [Tools Linting](#tools-linting)
* [Workflows](#workflows)
* [Analyze PR Changes](#analyze-pr-changes)
* [Build](#build)
* [Deploy GitHub Pages](#deploy-github-pages)
* [Comment on PR](#comment-on-pr)
* [Test Tools (Unit)](#test-tools--unit-)
* [Test Tools (Integration)](#test-tools--integration-)
* [Validate Spec](#validate-spec)
- [Developer Guide](#developer-guide)
- [Getting Started](#getting-started)
- [Specification](#specification)
- [File Structure](#file-structure)
- [Grouping Operations](#grouping-operations)
- [Grouping Schemas](#grouping-schemas)
- [Superseded Operations](#superseded-operations)
- [Global Parameters](#global-parameters)
- [OpenAPI Extensions](#openapi-extensions)
- [Writing Spec Tests](#writing-spec-tests)
- [Tools](#tools)
- [Setup](#setup)
- [Spec Merger](#spec-merger)
- [Spec Linter](#spec-linter)
- [Spec Tester](#spec-tester)
- [Spec Style](#spec-style)
- [Dump Cluster Spec](#dump-cluster-spec)
- [Coverage](#coverage)
- [Tools Testing](#tools-testing)
- [Tools Linting](#tools-linting)
- [Workflows](#workflows)
- [Analyze PR Changes](#analyze-pr-changes)
- [Build](#build)
- [Deploy GitHub Pages](#deploy-github-pages)
- [Comment on PR](#comment-on-pr)
- [Test Tools (Unit)](#test-tools-unit)
- [Test Tools (Integration)](#test-tools-integration)
- [Validate Spec](#validate-spec)
<!-- TOC -->

# Developer Guide
Expand Down Expand Up @@ -173,13 +166,13 @@ npm run merge -- --help

The merger tool merges the multi-file OpenSearch spec into a single file for programmatic use.

#### Arguments
**Arguments

- `--source <path>`: The path to the root folder of the multi-file spec, defaults to `<repository-root>/spec`.
- `--output <path>`: The path to write the final merged spec to, defaults to `<repository-root>/build/opensearch-openapi.yaml`.
- `--opensearch-version`: An optional target version of OpenSearch, checking values of `x-version-added` and `x-version-removed`.

#### Example
**Example

We can take advantage of the default values and simply merge the specification via:
```bash
Expand All @@ -200,11 +193,11 @@ npm run lint:spec -- --help

The linter tool validates the OpenSearch multi-file spec, and will print out all the errors and warnings in it.

#### Arguments
**Arguments

- `--source <path>`: The path to the root folder of the multi-file spec, defaults to `<repository-root>/spec`.

#### Example
**Example

We can take advantage of the default values and simply lint the specification via:
```bash
Expand All @@ -219,6 +212,20 @@ npm run test:spec -- --help

The spec test framework validates the OpenSearch spec against a running OpenSearch cluster. As you modify the spec, you should add or update the spec test stories in the [./tests](tests) directory. For information on this topic, see [TESTING_GUIDE.md](TESTING_GUIDE.md).

### Spec Style

This repo runs [Vale](https://github.com/errata-ai/vale) on the text contents of the spec, such as descriptions.

The [Style prepare tool](tools/src/prepare-for-vale/) clears YAML files from all markup and leaves text in-place in the [style workflow](.github/workflows/style.yml), allowing for comments to appear in pull requests on GitHub.

```bash
npm run style:prepare -- --help
```

**Arguments

- `--source <path>`: The path to the root folder of the multi-file spec, defaults to `<repository-root>/spec`.

### [Dump Cluster Spec](tools/src/dump-cluster-spec)

```bash
Expand All @@ -227,15 +234,15 @@ npm run dump-cluster-spec -- --help

The dump-cluster-spec tool connects to an OpenSearch cluster which has the [opensearch-api plugin](https://github.com/dblock/opensearch-api) installed and dumps the skeleton OpenAPI specification it provides to a file.

#### Arguments
**Arguments

- `--opensearch-url <url>`: The URL at which the cluster is accessible, defaults to `https://localhost:9200`.
- `--opensearch-insecure`: Disable SSL/TLS certificate verification, defaults to performing verification.
- `--opensearch-username <username>`: The username to authenticate with the cluster, defaults to `admin`, only used when `--opensearch-password` is set.
- `--opensearch-password <password>`: The password to authenticate with the cluster, also settable via the `OPENSEARCH_PASSWORD` environment variable.
- `--output <path>`: The path to write the dumped spec to, defaults to `<repository-root>/build/opensearch-openapi-CLUSTER.yaml`.

#### Example
**Example

You can use this repo's [docker image which includes the opensearch-api plugin](coverage/Dockerfile) to spin up a local development cluster with a self-signed certificate (e.g. `https://localhost:9200`) and security enabled, to then dump the skeleton specification:
```bash
Expand Down Expand Up @@ -264,13 +271,13 @@ npm run coverage:spec -- --help

The coverage tool determines which APIs from the OpenSearch cluster's reference skeleton specification (dumped by the [dump-cluster-spec tool](#dump-cluster-spec)) are covered by this specification (as built by the [merger tool](#merger)).

#### Arguments
**Arguments

- `--cluster <path>`: The path to the cluster's reference skeleton specification, as dumped by [dump-cluster-spec](#dump-cluster-spec), defaults to `<repository-root>/build/opensearch-openapi-CLUSTER.yaml`.
- `--specification <path>`: The path to the merged specification, as built by [merger](#merger), defaults to `<repository-root>/build/opensearch-openapi.yaml`.
- `--output <path>`: The path to write the coverage data to, defaults to `<repository-root>/build/coverage.json`.

#### Example
**Example

Assuming you've already followed the previous examples to build the merged specification with the [merger](#example) and dump the cluster's specification with [dump-cluster-spec](#example-2), you can then calculate the API coverage:
```bash
Expand Down
1 change: 1 addition & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"lint": "eslint . --report-unused-disable-directives",
"lint--fix": "eslint . --fix --report-unused-disable-directives",
"merge": "ts-node tools/src/merger/merge.ts",
"style:prepare": "ts-node tools/src/prepare-for-vale/prepare-for-vale.ts",

Check failure on line 15 in package.json

View workflow job for this annotation

GitHub Actions / check

[vale] reported by reviewdog 🐶 [OpenSearch.SpacingPunctuation] There should be no space before and one space after the punctuation mark in 'vale.ts'. Raw Output: {"message": "[OpenSearch.SpacingPunctuation] There should be no space before and one space after the punctuation mark in 'vale.ts'.", "location": {"path": "package.json", "range": {"start": {"line": 15, "column": 70}}}, "severity": "ERROR"}
"test": "npm run test:unit && npm run test:integ",
"jest": "jest",
"test:unit": "jest --testMatch='**/*.test.ts' --testPathIgnorePatterns=/integ/",
Expand All @@ -34,12 +35,12 @@
"@types/titlecase": "^1.1.2",
"@types/tmp": "^0.2.6",
"@typescript-eslint/eslint-plugin": "^6.21.0",
"axios-mock-adapter": "^2.0.0",
"ajv": "^8.13.0",
"ajv-errors": "^3.0.0",
"ajv-formats": "^3.0.1",
"aws4-axios": "^3.3.7",
"axios": "^1.7.5",
"axios-mock-adapter": "^2.0.0",
"cbor": "^9.0.2",
"commander": "^12.0.0",
"eslint": "^8.57.0",
Expand All @@ -51,6 +52,7 @@
"eslint-plugin-n": "^16.6.2",
"eslint-plugin-promise": "^6.1.1",
"eslint-plugin-yml": "^1.14.0",
"fast-glob": "^3.3.2",
"globals": "^15.0.0",
"json-diff-ts": "^4.0.1",
"json-schema-to-typescript": "^14.0.4",
Expand Down
2 changes: 1 addition & 1 deletion tests/default/security/api/internalusers/authtoken.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
$schema: ../../../../../json_schemas/test_story.schema.yaml

description: Test internalusers/authtoken endpoint.
version: '> 2.16' # Fixed via https://github.com/opensearch-project/security/pull/4628
version: '>= 2.17' # Fixed via https://github.com/opensearch-project/security/pull/4628
distributions:
excluded:
- amazon-managed
Expand Down
55 changes: 55 additions & 0 deletions tools/src/prepare-for-vale/KeepDescriptions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* 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 fs from 'fs'
import fg from 'fast-glob'
import { Logger } from '../Logger'

/**
* Keeps only description: field values.
*/
export default class KeepDescriptions {
root_folder: string
logger: Logger

constructor (root_folder: string, logger: Logger = new Logger()) {
this.logger = logger
this.root_folder = fs.realpathSync(root_folder)
}

process(): void {
this.root_folder
const files = fg.globSync([`${this.root_folder}/**/*.yaml`])
files.forEach((path) => {
this.logger.log(path)
this.process_file(path)
})
}

process_file(filename: string): void {
const contents = fs.readFileSync(filename, 'utf-8')
var writer = fs.openSync(filename, 'w+')

var inside_description = false
contents.split(/\r?\n/).forEach((line) => {
if (line.match(/^[\s]+(description: \|)/)) {
inside_description = true
} else if (line.match(/^[\s]+(description:)[\s]+/)) {
fs.writeSync(writer, line.replace("description:", " "))
} else if (inside_description && line.match(/^[\s]*[\w]*:/)) {
inside_description = false
} else if (inside_description) {
fs.writeSync(writer, line)
}
if (line.length > 0) {
fs.writeSync(writer, "\n")
}
})
}
}
27 changes: 27 additions & 0 deletions tools/src/prepare-for-vale/prepare-for-vale.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* 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 { Command, Option } from '@commander-js/extra-typings'
import { Logger, LogLevel } from '../Logger'
import { resolve } from 'path'
import KeepDescriptions from './KeepDescriptions'

const command = new Command()
.description('Convert YAML files to text.')
.addOption(new Option('-s, --source <path>', 'path to the root folder of the multi-file spec').default(resolve(__dirname, '../../../spec')))
.addOption(new Option('--verbose', 'show merge details').default(false))
.allowExcessArguments(false)
.parse()

const opts = command.opts()
const logger = new Logger(opts.verbose ? LogLevel.info : LogLevel.warn)
const keep_descriptions = new KeepDescriptions(opts.source, logger)
logger.log(`Preparing ${opts.source} ...`)
keep_descriptions.process()
logger.log('Done.')
43 changes: 43 additions & 0 deletions tools/tests/prepare-for-vale/KeepDescriptions.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* 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 KeepDescriptions from 'prepare-for-vale/KeepDescriptions'
import fs from 'fs'
import fg from 'fast-glob'
import tmp from 'tmp'

describe('KeepDescriptions', () => {
var temp: tmp.DirResult
var fixture_path: string = './tools/tests/prepare-for-vale/fixtures'
var fixtures = fg.globSync(`${fixture_path}/**/*.yaml`)

describe('defaults', () => {
beforeAll(() => {
temp = tmp.dirSync()
fs.cpSync(fixture_path, temp.name, { recursive: true })
new KeepDescriptions(temp.name).process()
})

afterAll(() => {
temp.removeCallback()
})

describe('converts files to text in-place', () => {
fixtures.forEach((filename) => {
test(filename, () => {
const processed_yaml = filename.replace(fixture_path, temp.name)
const filename_txt = processed_yaml.replace(".yaml", ".txt")
expect(fs.readFileSync(processed_yaml, 'utf8')).toEqual(fs.readFileSync(filename_txt, 'utf8'))
fs.rmSync(processed_yaml)
fs.rmSync(filename_txt)
})
})
})
})
})
Loading

0 comments on commit efba839

Please sign in to comment.