Azure / Azure/azure-sdk-tools

[Breaking Change] Add a mechanism for permanent suppressions (baselining)

Open
#7,259 13 comments 0 reactions 1 assignee Claimed by @konrad-jamrozik View on GitHub
Breaking Changes Central-EngSys Spec PR Tools
Dominant language
C#
Stars
135
Forks
260
Avg merge
1d 22h
Merged PRs (30d)
115

Description

Currently, there is no way to permanently suppress breaking change violations, as can be seen at:
[aka.ms/azsdk/pr-suppressions](https://aka.ms/azsdk/pr-suppressions)

This is a major gap causing tons of rework. We want to fix it by providing a mechanism for this. One idea we have is a solution that does comparison between the tool error output and a baseline text file checked into the repository. If all the output error lines have a match in the baseline file, the tool reports success.

## Preliminary design considerations

There are several design considerations to make before we proceed with implementation. Some of the aspects we identified so far:

- we probably want for the baselining solution to work with no or minimal format differences across all tools listed at [aka.ms/azsdk/pr-suppressions](https://aka.ms/azsdk/pr-suppressions). This probably means some work on consolidating on all the tools error message format
- we could consider SARIF. Some info about it from Gazdo folks:
> SARIF is meant to report vulnerabilities in code, so if the errors you are finding are pointing to a source file where the issue could be fixed, then SARIF might be an option.
> SARIF is meant to be used as a format understood by many different systems, VSCode, microsoft security tools, etc. so if you think your errors need to be displayed in many clients and understood by deferent systems, this could also be a good option.
- the baseline file should disregard line numbers, otherwise it will be too brittle
- as a result, it needs to be able to understand which part of the error message is a line number - see the bullet about common format above
- if line numbers are disregarded we may run into the risk of the tool of applying suppression to too many places - hence we need to ensure it is not ambiguous
- we could possibly try to match based on hash of the line and surrounding context - a pointer I got from Gazdo folks: [hash](https://dev.azure.com/mseng/AzureDevOps/_git/AdvancedSecurity.BuildTasks?path=/tasks/codeql-common/fingerprints.ts&version=GBmain&line=39&lineEnd=40&lineStartColumn=1&lineEndColumn=1&lineStyle=plain&_a=contents)
- Additional info on how Gazdo does it:
> we compute a fingerprint for each finding and the fingerprint has this format: {repo/scope}{toolId}|{ruleId}|{filePathHash}|{lineRollingHash} and we store it in the database. When we get new findings, we check if we have seen it before. When someone dismisses an alert, we store the dismissed fingerprint and delete it if someone reactivates an alert.
- if given tool is executed locally on a dev box, we want for the suppression mechanism to still work, not only in the CI
- we should consider the ergonomics of integrating with IDEs like VS Code
- we should consider support for suppressing given rule across bigger swatches of directory structure. One possible solution is to use regex to match about a common subset of rule error
- having strings copy-pasted to baseline file will lead to duplication and brittleness no matter what
- we should consider how it plays with other suppressions mechanisms, like e.g. those built into LintDiff (stagingOnly rules and also already ignoring previous errors), or TypeSpec Validator and the tool it calls internally, or all the tools using Autorest suppressions

## Related work, docs and context

- https://dev.azure.com/azure-sdk/internal/_wiki/wikis/internal.wiki/966/API-spec-PR-ARM-review-workflow
- https://dev.azure.com/azure-sdk/internal/_wiki/wikis/internal.wiki/1009/Breaking-Change-Detection-Rules
- https://github.com/Azure/azure-sdk-tools/issues/5838
- https://github.com/Azure/azure-sdk-tools/issues/6396
- https://github.com/Azure/azure-sdk-tools/issues/6859
- https://github.com/Azure/azure-sdk-tools/issues/6900
- https://github.com/Azure/azure-sdk-tools/issues/7239
- Email thread `RE: [backlog discussion] Breaking Changes for Microsoft.Sql 2023-02-01-preview Version Release`
- Email thread `RE: Breaking Change Review - MobileNetwork ARM API`

@weshaggard @mikeharder @heaths FYI

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.