Azure / azure-sdk-tools

Tools repository leveraged by the Azure SDK team.
MIT License
114 stars 177 forks source link

[LintDiff] is failing to run `arm` rules if `rpaas` or `providerHub` `openapi-subtype` is defined in `README.md` #6549

Closed konrad-jamrozik closed 1 year ago

konrad-jamrozik commented 1 year ago

The problem

Expected behavior: arm ruleset is used. I.e. we want to see log:

{"pluginName":"spectral","extensionName":"@microsoft.azure/openapi-validator","level":"information","message":"Loaded 107 spectral rules, for OpenAPI type 'arm':"}

Actual behavior: rpaas ruleset is used. I.e. we can see log:

{"pluginName":"spectral","extensionName":"@microsoft.azure/openapi-validator","level":"information","message":"Loaded 38 spectral rules, for OpenAPI type 'rpaas':"}

Examples

Exhibit A

README from the PR branch:

https://github.com/Azure/azure-rest-api-specs-pr/blob/5d6f06b46a563622daff6fcf44dac78766e5ba02/specification/hybridnetwork/resource-manager/readme.md?plain=1#L29

Relevant snippet:

## Configuration

### Basic Information

These are the global settings for the hybridnetwork.

    ```yaml
    openapi-type: arm
    openapi-subtype: rpaas // <-- ==================== HERE IS THE CULPRIT ====================
    tag: package-2023-01-01

Tag: package-2023-04-01-preview

These settings apply only when --tag=package-2023-04-01-preview is specified on the command line.


## Exhibit B

- https://github.com/Azure/azure-rest-api-specs-pr/pull/13612/checks?check_run_id=15032584340
- https://dev.azure.com/azure-sdk/internal/_build/results?buildId=2918805&view=logs&j=0574a2a6-2d0a-5ec6-40e4-4c6e2f70bea2&t=80c3e782-49f0-5d1c-70dd-cbee57bdd0c7&l=51

Executing: node /mnt/vss/_work/_tasks/AzureApiValidation_5654d05d-82c1-48da-ad8f-161b817f6d41/0.0.54/private/azure-swagger-validation/azureSwaggerValidation/node_modules/autorest/dist/app.js --v3 --spectral --azure-validator --semantic-validator=false --model-validator=false --message-format=json --openapi-type=arm --use=@microsoft.azure/openapi-validator@2.1.3 /mnt/vss/_work/1/azure-rest-api-specs-pr/specification/providerhub/resource-manager/readme.md


README from the PR branch:
- https://github.com/Azure/azure-rest-api-specs-pr/blob/2bed83e938f1ffe18e3445f79f008f9c11070e3b/specification/providerhub/resource-manager/readme.md?plain=1#L29C23-L29C23

## Exhibit C

- https://github.com/Azure/azure-rest-api-specs-pr/pull/13636/checks?check_run_id=15129053335
- https://dev.azure.com/azure-sdk/internal/_build/results?buildId=2927655&view=logs&j=688669d0-441c-57c3-cf6d-f89a22ccfa5d&t=b91b1e88-b042-5e18-36d8-34e4fb3a9b3b&l=63

Executing: npx autorest --v3 --version:next --spectral --validation --azure-validator --semantic-validator=false --model-validator=false --message-format=json --is-staging-run=true --openapi-type=arm --use=@microsoft.azure/openapi-validator@beta --tag=package-2023-07-07-preview /mnt/vss/_work/1/azure-rest-api-specs-pr/specification/scom/resource-manager/readme.md


README from the PR branch: 
- https://github.com/Azure/azure-rest-api-specs-pr/blob/88d043488cd796d9900d339f847cb44522a8caad/specification/scom/resource-manager/readme.md?plain=1#L29

# The root cause

From the [LintDiff source code](https://github.com/Azure/azure-openapi-validator/blob/cffffcf5baa2e612e254c5afba4a68004b8e49aa/packages/azure-openapi-validator/autorest/src/plugin-common.ts#L38-L46)

``` typescript
export async function getOpenapiTypeStr(initiator: IAutoRestPluginInitiator) {
  let openapiType: string = await initiator.GetValue("openapi-type")
  let subType: string = await initiator.GetValue("openapi-subtype")
  subType = subType === "providerHub" ? "rpaas" : subType
  if (subType === "rpaas") {
    openapiType = "rpaas"
  }
  return openapiType
}

The await initiator.GetValue("openapi-type") ends up calling GetValue, which, according to AutoRest doc, requests the value for a particular AutoRest configuration setting..

Turns out this includes settings in the Basic information section, which corresponds to General settings section in the relevant AutoRest doc. In all the examples provided above, such setting is always present in the README file, i.e. AutoRest configuration.

As a result, the LintDiff is running rpaas instead of arm ruleset. This is clearly not intended, as there is a separate LintDiff check, named Swagger Lint(RPaaS), whose purpose is to run rpaas rules. Here is an example from exhibit A.

The bug impact

Most likely all the specifications that defined openapi-subtype as rpaas or providerHub in the README AutoRest config file never had the arm or any other ruleset executed on them.

FYI @rkmanda @Akhilailla @weshaggard

konrad-jamrozik commented 1 year ago

PR with a fix: