Documentation: Spec PR tools suppressions mechanisms reference

[konrad-jamrozik]

Important

This document was written on 4/19/2024. Hence "current state" refers to this date.

In this issue I describe current state of various kinds of "suppressions" work for the spec PR tools on the specs repos.

Official documentation for spec PRs authors

The official document describing to the PR authors the various kinds of PR suppressions is:

• https://aka.ms/pr-suppressions

It is linked from, among other places:

• The PR template, additional information section
• The checks TSGs guide: https://aka.ms/ci-fix

File-based suppressions kinds

AutoRest config README.md files suppressions:

By looking at https://aka.ms/pr-suppressions you can observe that he AutoRest README.md config file can be used to suppress several different checks like Swagger LintDiff or Swagger ModelValidation. Aa a result, SuppressionReviewRequired label will be added. The logic adding this label lives in prSummary.ts / processSuppression.

To unblock a PR once it has this label, one has to add Approved-Suppression, per this line in requiredLabelRules.ts

suppressions.yaml

On 3/4/2024 in the PR Azure/azure-rest-api-specs#28043 a support for file-based suppressions for TypeSpec Requirement has been added.

As of current state:

• we are working on implementing corresponding labels, per [suppressions.yaml] Add PR label when edited azure-rest-api-specs#28756
• while currently we use this file to suppress TypeSpec Requirement, going forward we will use it to suppress other checks/tools, like breaking change detectors. See [Breaking Change] Add a mechanism for permanent suppressions (baselining) #7259.

sdk-suppressions.yaml

On 3/19/2024 in the openapi-alps PR 524197 and few other PRs around that time a new file-based suppression mechanism has been added for suppression SDK generation issues. The new suppressions live in the sdk-suppressions.yaml file and are supported by a set of labels described in this gist:

• https://gist.github.com/raych1/353949d19371b69fb82a10dd70032a51

For example, there are labels like BreakingChange-Go-Sdk-Suppression or BreakingChange-Go-Sdk-Suppression-Approved

You can read more about the general design of these here:

• [Breaking Change][PR Workflow] Use more granular labels for Breaking Changes approvals #6374 (comment)

Caution

sdk-suppressions.yaml and related labels are not yet documented in https://aka.ms/pr-suppressions
See #8134
@raych1 @JackTn FYI

Suppression labels for the file-based suppressions

To summarize the suppression labels for File-based suppressions kinds:

• The AutoRest config README.md suppressions: use SuppressionReviewRequired and Approved-Suppression
• The suppressions.yaml has no corresponding labels yet; it is work in progress, per: Typo on preview folder. azure-rest-api-specs#2875
• The sdk-suppressions.yaml has labels as explained in this gist

Design considerations

Before we took over the system the dominant way to suppress check failures was to add Approved-*** label to given PR. E.g. Approved-LintDiff. We are moving away from this model. This is because such approvals are transient. As such, we often have to re-approve the same exception when the same code gets reviewed again. Instead, we are moving to file-based model which persists the suppression.

Furthermore, previously the existing file-based check suppression model was via AutoRest config README.md entries like suppressions: or the legacy suppression directives. Going forward, for all the newly supported suppresions we move away from this model for several reasons:

1. The README.md file is already overloaded with too many responsibilities.
2. This is an AutoRest suppressions sub-system and so it only works on AutoRest-based tools.
3. This syntax is hard to get right.

Instead, we introduced suppressions.yaml file for clean and easy suppression syntax for all other tools. We also introduced sibling sdk-suppressions.yaml file owned by our partners in Shanghai. The SDK suppressions are materially different as they pertain to suppressing compilation violations from varied languages for which we generate suppressions. The suppressions.yaml file instead pertains to all other spec PR GitHub checks suppressions.

Note that for existing checks using the README.md suppressions: syntax we still continue to use it. We may migrate away from it into suppressions.yaml at one point into the future.

Suppressions work backlog

There are several items on the backlog with relevant discussion about improving the labelling.

• Provide method and guidance for suppressing PoliCheck failures (reported by tg-msft) #6091
• [SDK Automation] Support label suppression for SDK automation CI check #6509
• [PR Workflow] The SuppressionReviewRequired label should be split into subcategories #6622
• [Breaking Change] Add a mechanism for permanent suppressions (baselining) #7259
• [PR Workflow] Suppression labels should be cleaned up when suppressions are removed from source #7277
• Block ARM review approval (label ARMSignedOff) if suppressions have not been addressed (related to "suppression review queue") #7908
• [suppressions.yaml] Add PR label when edited azure-rest-api-specs#28756
• Rename SuppressionReviewRequired and Approved-Suppression #8135