Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various improvements to ci-fix.md #27484

Merged
merged 1 commit into from
Jan 22, 2024
+74 −29
Merged

Various improvements to ci-fix.md #27484

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 74 additions & 29 deletions documentation/ci-fix.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,47 @@
# Table of Contents

- [Table of Contents](#table-of-contents)
- [CI Fix Guide](#ci-fix-guide)
- [Prerequisites](#prerequisites)
- [`Swagger SpellCheck`](#swagger-spellcheck)
- [`Swagger PrettierCheck`](#swagger-prettiercheck)
- [Prettier reference](#prettier-reference)
- [`Swagger ModelValidation`](#swagger-modelvalidation)
- [`Swagger SemanticValidation`](#swagger-semanticvalidation)
- [`Swagger BreakingChange` and `BreakingChange(Cross-Version)`](#swagger-breakingchange-and-breakingchangecross-version)
- [Adding label on PR automatically](#adding-label-on-pr-automatically)
- [Run `oad` locally](#run-oad-locally)
- [`Swagger LintDiff` and `Swagger Lint(RPaaS)`](#swagger-lintdiff-and-swagger-lintrpaas)
- [`Swagger LintDiff` for TypeSpec: troubleshooting guides](#swagger-lintdiff-for-typespec-troubleshooting-guides)
- [`Record<unkown>` causes `AvoidAdditionalProperties` and `PropertiesTypeObjectNoDefinition`](#recordunkown-causes-avoidadditionalproperties-and-propertiestypeobjectnodefinition)
- [`RequestBodyMustExistForPutPatch`](#requestbodymustexistforputpatch)
- [`PatchPropertiesCorrespondToPutProperties`](#patchpropertiescorrespondtoputproperties)
- [`Swagger Avocado`](#swagger-avocado)
- [Get help fixing Avocado validation failures](#get-help-fixing-avocado-validation-failures)
- [Run avocado locally](#run-avocado-locally)
- [`Swagger ApiDocPreview`](#swagger-apidocpreview)
- [`TypeSpec Validation`](#typespec-validation)
- [Run `tsv` locally](#run-tsv-locally)
- [Suppression Process](#suppression-process)
- [Checks not covered by this guide](#checks-not-covered-by-this-guide)
- [Obsolete checks](#obsolete-checks)

# CI Fix Guide

Short link: https://aka.ms/azsdk/ci-fix

This page provides detailed instructions on how to diagnose, reproduce, fix and get help on various [automated validation tooling] failures on your [Azure REST API specs PR].

For more help, see [aka.ms/azsdk/pr-getting-help] and [aka.ms/azsdk/support].

## Prerequisites

Most guides here require for you to have `npm` installed, which you can get by installing [Node.js](https://nodejs.org/en/download).

## Spell check
## `Swagger SpellCheck`
konrad-jamrozik marked this conversation as resolved.
Show resolved Hide resolved

If you receive a spelling failure either fix the spelling to match or if there are words that need to be suppressed for your service then add the word to the override list in [cspell.json](https://github.com/Azure/azure-rest-api-specs/blob/main/cSpell.json). Either
add to your existing section or create a new section for your specific spec or service family if the work is more generally used in losts of files under your service.
add to your existing section or create a new section for your specific spec or service family if the work is more generally used in lots of files under your service.
```
"overrides": [
... example of specific file override
Expand All @@ -36,7 +66,7 @@ If you need more information on see [cspell configuration](https://cspell.org/co

*Note*: We are trying to move away from one shared dictionary file so try and avoid editing custom-words.txt in the root as it will likely go away in the future.

## Prettier check
## `Swagger PrettierCheck`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These heading updates will likely break anchor links like https://aka.ms/azsdk/specs/prettier which is currently pointing at https://github.com/Azure/azure-rest-api-specs/blob/main/documentation/ci-fix.md#prettier-check. So once these go in be sure to update the aka links.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found these:

image


First, ensure you have fulfilled `Prerequisites` as explained above.

Expand All @@ -46,7 +76,7 @@ To update all the spec files for a given service run the following:
# To fix all the files in the repo run from the root of the repo
cd <local_repo_clone_root>

# OPTIONAL STEP: To fix a particular service swagger cd to that directory like
# OPTIONAL STEP: To fix a particular service OpenAPI spec cd to that directory like
cd specification/contosowidgetmanager

# Install the dependencies to the local 'node_modules' folder.
Expand All @@ -67,45 +97,45 @@ Then please commit and push changes made by prettier.
- [Source: Swagger-Prettier-Check.ps1](https://github.com/Azure/azure-rest-api-specs/blob/main/eng/scripts/Swagger-Prettier-Check.ps1)
- [Pipeline: Swagger PrettierCheck](https://dev.azure.com/azure-sdk/public/_build?definitionId=6405)

## Model Validation
## `Swagger ModelValidation`

Run Model Validation locally:
To repro issues with `Swagger ModelValidation` locally:
```
npm install -g oav
oav validate-example <swagger-spec-path>
oav validate-example <openapi-spec-path>
```
Please see [readme](https://github.com/Azure/oav/blob/bd04e228b4181c53769ed88e561dec5212e77253/README.md) for how to install or run tool in details.
Or you can run it in [OpenAPI Hub](https://portal.azure-devex-tools.com/tools/static-validation/static/errors/default).
Refer to [Semantic and Model Violations Reference](https://github.com/Azure/azure-rest-api-specs/blob/main/documentation/Semantic-and-Model-Violations-Reference.md) for detailed description of validations and how-to-fix guidance.
Refer to [Swagger-Example-Generation](https://github.com/Azure/oav/blob/develop/documentation/example-generation.md) for example automatic generation.

## Semantic Validation
## `Swagger SemanticValidation`

Run Semantic Validation locally:
To repro issues with `Swagger SemanticValidation` locally:
```
npm install -g oav
oav validate-spec <swagger-spec-path>
oav validate-spec <openapi-spec-path>
```
Please see [readme](https://github.com/Azure/oav/blob/bd04e228b4181c53769ed88e561dec5212e77253/README.md) for how to install or run tool in details.
Or you can run it in [OpenAPI Hub](https://portal.azure-devex-tools.com/tools/static-validation/static/errors/default)
Refer to [Semantic and Model Violations Reference](https://github.com/Azure/azure-rest-api-specs/blob/main/documentation/Semantic-and-Model-Violations-Reference.md) for detailed description of validations and how-to-fix guidance.

## Breaking Change Check
## `Swagger BreakingChange` and `BreakingChange(Cross-Version)`

- An API contract is identified by its api-version value. Once published, no changes to this API contract are allowed. This applies regardless of whether the API contract is for private preview, public preview, or GA (stable).
- The same-version breaking change linter rules check for changes to an existing api-version swagger.
- The same-version breaking change linter rules check for changes to an existing api-version OpenAPI spec.
- When introducing a new API contract (preview or not), the new API contract must be backwards compatible with the previous GA’s API contract.
- However, during a (private or public) preview cycle, a new preview API contract does not have to be backwards compatible with the previous preview API contract although it must still be backwards compatible with the latest GA API contract.
- The cross version breaking change linter rules checks for this by comparing the new swagger with the latest GA swagger. If there is no latest GA swagger, then the latest preview if it > 1 year old. If nether a GA or preview > 1 year old exists, then the swagger is considered good.
- The cross version breaking change linter rules checks for this by comparing the new OpenAPI spec with the latest GA OpenAPI spec. If there is no latest GA OpenAPI spec, then the latest preview if it > 1 year old. If nether a GA or preview > 1 year old exists, then the OpenAPI spec is considered good.

### Adding label on PR automatically

The breaking change check has two types of violations: one is breaking change in the same version but not breaking change in a new version, the other is breaking change even in a new version.
For the former, a label 'NewApiVersionRequired' will be added automatically; For the latter, a label 'BreakingChangeReviewRequired' will be added automatically. Adding each label will trigger a github comment with guildance on how to fix.
For the former, a label 'NewApiVersionRequired' will be added automatically; For the latter, a label 'BreakingChangeReviewRequired' will be added automatically. Adding each label will trigger a github comment with guldance on how to fix.

### Run locally
### Run `oad` locally

run oad locally (the breaking change is reported by oad tool):
To repro issues with "breaking changes" checks, you can locally run the tool that powers them [Azure/openapi-diff](https://github.com/Azure/openapi-diff) aka `oad`:
```
npm install -g @azure/oad
oad compare <old-spec-path> <new-spec-path>
Expand All @@ -114,7 +144,7 @@ Please see [readme](https://github.com/Azure/openapi-diff/blob/main/README.md) f
Or you can run it in [OpenAPI Hub](https://portal.azure-devex-tools.com/tools/diff).
Refer to [Oad Docs](https://github.com/Azure/openapi-diff/tree/main/docs) for detailed description of all oad rules.

## Swagger LintDiff
## `Swagger LintDiff` and `Swagger Lint(RPaaS)`

The [LintDiff validation tool](https://github.com/Azure/azure-openapi-validator) runs linting rules against specification difference. Two specifications are compared: the specification as it would be when proposed PR is merged, vs the specification as seen before the PR is merged.

Expand All @@ -123,11 +153,11 @@ If that guidance is not enough, please also refer to the [LintDiff rules.md doc]

To reproduce LintDiff failures locally, see [CONTRIBUTING.md / How to locally reproduce a LintDiff failure occurring on a PR](https://github.com/Azure/azure-openapi-validator/blob/main/CONTRIBUTING.md#how-to-locally-reproduce-a-lintdiff-failure-occurring-on-a-pr).

## Swagger LintDiff for TypeSpec
## `Swagger LintDiff` for TypeSpec: troubleshooting guides

### `Record<unkown>` causes `AvoidAdditionalProperties` and `PropertiesTypeObjectNoDefinition`

The use of `Record<unkown>` in TypeSpec is discouraged, and there is a TypeSpec lint rule to enforce this. If you still need to use `Record<unknown>`, the swagger generated will cause many LintDiff errors of types `AvoidAdditionalProperties` and `PropertiesTypeObjectNoDefinition`. You will need to suppress both the TypeSpec violation (in TypeSpec source code) and the LintDiff violations (in `readme.md`).
The use of `Record<unkown>` in TypeSpec is discouraged, and there is a TypeSpec lint rule to enforce this. If you still need to use `Record<unknown>`, the OpenAPI spec generated will cause many LintDiff errors of types `AvoidAdditionalProperties` and `PropertiesTypeObjectNoDefinition`. You will need to suppress both the TypeSpec violation (in TypeSpec source code) and the LintDiff violations (in `readme.md`).

### `RequestBodyMustExistForPutPatch`

Expand All @@ -137,7 +167,12 @@ We believe this is a false positive: https://github.com/Azure/azure-openapi-vali

We believe this is a false positive: https://github.com/Azure/azure-openapi-validator/issues/642. Until fixed, spec authors should **not** suppress the violations in `readme.md`, but rather have label `Approved-LintDiff` applied to their PR to ignore the errors.

## Avocado
## `Swagger Avocado`

>[!IMPORTANT]
> `Swagger Avocado` check is not a blocking for merging your PR, even if it fails.
> It is left to the discretion of the PR reviewer if the Avocado failure actually
> needs to be addressed or suppressed.

### Get help fixing Avocado validation failures

Expand All @@ -153,26 +188,27 @@ avocado

When type avocado in command line, avocado will validate in the current directory.

Note: When running in Swagger PR pipeline, Avocado only report errors with file updates in the PR, but ignore the errors existing in base. However when running Avocado against local directory, it reports all errors existing in the files.
Note: When running in OpenAPI spec PR pipeline, Avocado only report errors with file updates in the PR, but ignore the errors existing in base. However when running Avocado against local directory, it reports all errors existing in the files.

- Run all specs: Clone the repo `azure/azure-rest-api-specs` and run "avocado" in folder `azure/azure-rest-api-specs`.
- Run single service specs: create a folder `specification`. and move your service specs folder in `specification`. run "avocado"

## API Doc Preview
## `Swagger ApiDocPreview`

If you see `Swagger ApiDocPreview ` check fail with a failure [like this one](https://github.com/Azure/azure-rest-api-specs/pull/24841/checks?check_run_id=15056283615):

| Rule | Message |
|-|-|
| ❌ RestBuild error | "logUrl":"https://apidrop.visualstudio.com/Content%20CI/_build/results?buildId=373646&view=logs&j=fd490c07-0b22-5182-fac9-6d67fe1e939b",<br/>"detail":"Run.ps1 failed with exit code 1 " |

Refer to [troubleshoothing REST API documentation](https://eng.ms/docs/products/azure-developer-experience/design/api-docs-troubleshooting).
Refer to [troubleshooting REST API documentation](https://eng.ms/docs/products/azure-developer-experience/design/api-docs-troubleshooting).

## TypeSpec Validation
## `TypeSpec Validation`

This validator will help ensure your TypeSpec project follows [standard conventions](https://github.com/Azure/azure-rest-api-specs/blob/main/documentation/typespec-structure-guidelines.md) as well ensures that the [generated swagger](https://azure.github.io/typespec-azure/docs/emitters/typespec-autorest) files are in-sync with your project.
This validator will help ensure your TypeSpec project follows [standard conventions](https://github.com/Azure/azure-rest-api-specs/blob/main/documentation/typespec-structure-guidelines.md) as well ensures that the [generated OpenAPI spec](https://azure.github.io/typespec-azure/docs/emitters/typespec-autorest) files are in-sync with your project.

### Run `tsv` locally

### Run tsv locally
```
cd <repo>
git checkout <your-branch>
Expand All @@ -184,19 +220,28 @@ git commit; git push (if any changes)
# example
npx tsv specification/contosowidgetmanager/Contoso.WidgetManager
```
Then check any errors that might be outputted and address any issues as needed. If there are changed files after the run it generally means that the generated swagger files were not in-sync with the TypeSpec project and you should include those changes in your pull request as well.
Then check any errors that might be outputted and address any issues as needed. If there are changed files after the runit generally means
that the generated OpenAPI spec files were not in-sync with the TypeSpec project and you should include those changes in your pull request as well.

## Suppression Process

In case there are validation errors reported against your service that you believe do not apply, we have a suppression process you can follow to permanently remove these reported errors for your specs. Refer to the [suppression guide](https://aka.ms/pr-suppressions) for detailed guidance.

## Obsolete tools
## Checks not covered by this guide

If you have an issue with a check that is not covered by this guide and the help at [aka.ms/azsdk/pr-getting-help] is not enough,
please reach out on the Teams channel: [aka.ms/azsdk/support/specreview-channel].

## Obsolete checks

Following tools have been removed from the validation toolchain as of August 2023.
Following checks have been removed from the validation toolchain as of August 2023.

- API Readiness Check
- Service API Readiness Test
- Traffic validation

[automated validation tooling]: https://eng.ms/docs/products/azure-developer-experience/design/api-specs/api-tooling
[Azure REST API specs PR]: https://eng.ms/docs/products/azure-developer-experience/design/api-specs-pr/api-specs-pr
[aka.ms/azsdk/pr-getting-help]: https://aka.ms/azsdk/pr-getting-help
[aka.ms/azsdk/support/specreview-channel]: https://aka.ms/azsdk/support/specreview-channel
[aka.ms/azsdk/support]: https://aka.ms/azsdk/support