Enforce unified API versions in RP specs

mikeharder commented 7 months ago

Implement a tool to detect if any spec "tags" are mixing API versions.

If we name the parts of the path as follows:

Microsoft.Monitor/stable/2023-04-03/monitoringAccounts_API.json
<rpnamespace>/<lifecycle-stage>/<api-version>/<filename>

We might want rules like:

The <lifecycle-stage> of all files must be the same ("preview" or "stable")
Within an <rpnamespace>, the of all files must be the same
If <lifecycle-stage> is "preview", must end with "-preview". Else, <api-version> must not end with "-preview".
The <rpnamespace> of all files must match

Questions:

Are versions required to be aligned across different <rpnamespace>?

konrad-jamrozik commented 7 months ago

Related doc:

aka.ms/azsdk/pr-api-versions

Relevant email threads with subjects:

Re: Enforcing Unified API Versions in azure-rest-api-specs
Re: Preview specs in stable folder - wrong documentation?

konrad-jamrozik commented 7 months ago

@mikeharder the nomenclature that is used in docs for what you call <release-type> is lifecycle stage. See here: https://eng.ms/docs/products/azure-developer-experience/design/api-specs-pr/api-versions-and-branches#api-version-lifecycle-stages

Specifically:

For ARM resource providers, these stages corresponds to service tree lifecycle stages as elaborated on in ARM lifecycle stages requirements.

and also here:

https://github.com/Azure/azure-rest-api-specs/blob/main/README.md#directory-structure (PR with changes: https://github.com/Azure/azure-rest-api-specs/pull/28464)

'preview' and 'stable' Folders: This maps to the service/component lifecycle stage: Preview and GA. For example, if a service is in Preview stage, no matter Private Preview or Public Preview, the API specs of the service should be placed in the preview folder. If the service is GAed, but a component is in preview, then the API version contains the preview component entity should be placed in the preview folder as well. The stable folder should contain API versions of a GAed service and all GAed components.

I think it would be great if we avoid synonyms and call it lifecycle stage everywhere.

Also, we usually say API version not just version, to avoid ambiguity.

Also, spec "tag" is, to my knowledge, just another name for "API version". Tag, to my knowledge, is a name adopted in AutoRest. I think it would be best if we always use "API version" and mention "tag" only when necessary, e.g. because legacy code uses it.

konrad-jamrozik commented 7 months ago

@mikeharder the tool in our spec validation toolchain that detects issues with AutoRest README contents is Avocado:

https://github.com/Azure/avocado

However, we made it non-blocking as it was extremely noisy: IIRC it reported multiple false positives per day, wasting too much of our time.

In any case, if you are planning on adding a check that validates AutoRest README in some form, I think we should seriously consider patching up and reusing Avocado, or at least salvaging relevant code from it.

For open Avocado issues, see:

with this probably being a first / warm-up task:

https://github.com/Azure/azure-sdk-tools/issues/6325

tombuildsstuff commented 7 months ago

mikekistler commented 7 months ago

@JeffreyRichter asked for an update of an analysis I did over a year ago that found about two dozen services with mixed versions. I couldn't find my notes / scripts from that original analysis so I have recreated it.

The analysis uses the Avocado tool to check every service in the azure-rest-api-specs repo for a "MULTIPLE_API_VERSION" issue. The script for this is pretty simple:

find ./azure-rest-api-specs/specification -name 'readme.md' | sort | \
while read -r file; do
  # Run avocado tool on the readme.md file
  avocado -d $(dirname $file) | grep "MULTIPLE_API_VERSION" > /dev/null && echo $file
done

Here's the results that I obtained from running this on the current main branch:

>./multi-api-version.sh       
./azure-rest-api-specs/specification/alertsmanagement/resource-manager/readme.md
./azure-rest-api-specs/specification/applicationinsights/resource-manager/readme.md
./azure-rest-api-specs/specification/authorization/resource-manager/readme.md
./azure-rest-api-specs/specification/automation/resource-manager/readme.md
./azure-rest-api-specs/specification/azsadmin/resource-manager/compute/readme.md
./azure-rest-api-specs/specification/azsadmin/resource-manager/fabric/readme.md
./azure-rest-api-specs/specification/billing/resource-manager/readme.md
./azure-rest-api-specs/specification/compute/resource-manager/readme.md
./azure-rest-api-specs/specification/containerregistry/resource-manager/readme.md
./azure-rest-api-specs/specification/databricks/resource-manager/readme.md
./azure-rest-api-specs/specification/frontdoor/resource-manager/readme.md
./azure-rest-api-specs/specification/maps/data-plane/readme.md
./azure-rest-api-specs/specification/mariadb/resource-manager/readme.md
./azure-rest-api-specs/specification/mediaservices/resource-manager/readme.md
./azure-rest-api-specs/specification/monitor/resource-manager/readme.md
./azure-rest-api-specs/specification/mysql/resource-manager/readme.md
./azure-rest-api-specs/specification/operationalinsights/resource-manager/readme.md
./azure-rest-api-specs/specification/policyinsights/resource-manager/readme.md
./azure-rest-api-specs/specification/postgresql/resource-manager/readme.md
./azure-rest-api-specs/specification/reservations/resource-manager/readme.md
./azure-rest-api-specs/specification/resourcegraph/resource-manager/readme.md
./azure-rest-api-specs/specification/security/resource-manager/readme.md
./azure-rest-api-specs/specification/sql/resource-manager/readme.md
./azure-rest-api-specs/specification/subscription/resource-manager/readme.md
./azure-rest-api-specs/specification/synapse/resource-manager/readme.md

I should note that I believe Avocado suppresses the "MULTIPLE_API_VERSION" errors for data-plane services -- I think this is why all the results above are for resource-manager services.

Azure / azure-rest-api-specs

Enforce unified API versions in RP specs #28466