-
Notifications
You must be signed in to change notification settings - Fork 5k
ARM API version linter: prevent preview API from being used in azurerm #30612
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.
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
Changes from 2 commits
c421e7f
8083dab
a0b2a80
9ca30dc
4076b42
8202e5f
05d9b78
1ab3d1a
debaf62
205c261
3a66ec3
4991675
4503ce2
e00adc3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| --- | ||
| name: ARM API Version Linting | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: read | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: ["opened", "synchronize"] | ||
| paths: | ||
| - ".github/workflows/api-version-lint.yaml" | ||
| - "internal/tools/api-version-lint/**" | ||
| - "vendor/**" | ||
|
|
||
| jobs: | ||
| api-version-lint: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 | ||
| - uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 | ||
| with: | ||
| go-version-file: ./.go-version | ||
| - run: go run internal/tools/api-version-lint/main.go | ||
| comment-on-fail: | ||
| if: ${{ needs.depscheck.result }} == 'failure' | ||
| uses: ./.github/workflows/comment-failure.yaml |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,53 @@ | ||||||||||||||||||||||
| # Guide: ARM API Version | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Provider logic should be implemented using stable Azure Resource Manager (ARM) API version. Preview versions are prone to breaking changes which results in very poor azurerm user experience (eg: removed property). There are [automated checks on azure-rest-api-specs that prevents breaking changes against stable version](https://github.com/Azure/azure-rest-api-specs/blob/main/documentation/ci-fix.md#sdk-breaking-change-review). Such checks are not applicable to preview versions. | ||||||||||||||||||||||
|
||||||||||||||||||||||
| Provider logic should be implemented using stable Azure Resource Manager (ARM) API version. Preview versions are prone to breaking changes which results in very poor azurerm user experience (eg: removed property). There are [automated checks on azure-rest-api-specs that prevents breaking changes against stable version](https://github.com/Azure/azure-rest-api-specs/blob/main/documentation/ci-fix.md#sdk-breaking-change-review). Such checks are not applicable to preview versions. | |
| The provider should be implemented using stable Azure Resource Manager (ARM) API/SDK version. Preview versions are prone to sudden breaking changes which can result in a less then ideal user experience (eg: removed property or behavioural change). There are [automated checks on azure-rest-api-specs that prevents breaking changes against stable version](https://github.com/Azure/azure-rest-api-specs/blob/main/documentation/ci-fix.md#sdk-breaking-change-review) but they do not catch everything and are not applicable to preview versions. |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest not mentioning this concrete v3.0 -> v4.0 example, because breaking api changes brought by preview API could be part of the reason that increases the duration between major versions, but it does not constitute the decisive factor. Mentioning the v3.0 -> v4.0 example implies its duration was due to breaking api changes brought by preview API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mentioning v3 -> v4 timeline here is only to highlight the potential user pain if a major version breaking change occur. While I understand it's not necessarily the future schedule going forward, it is a historical fact that can be viewed simply from the repository releases section.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Breaking API changes often materialise into [azurerm breaking change](guide-breaking-changes.md) which involves non-trivial upgrade steps and long major version release wait time. v3.0.0 was released in March 2022 and v4.0.0 in August 2024. | |
| These breaking API changes often materialise into [breaking changes](guide-breaking-changes.md) which can involve non-trivial upgrade steps and/or require waiting till a major version release to make the breaking change. v3.0.0 was released in March 2022 and v4.0.0 in August 2024. |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Day-0 Terraform support for preview API should be done via [azapi provider](https://registry.terraform.io/providers/Azure/azapi/latest/docs) where users have full control of API version choice. | |
| Day-0 Terraform support for preview APIs can be done via [azapi provider](https://registry.terraform.io/providers/Azure/azapi/latest/docs) where users have full control of API version choice. |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| > preview API version usage is risky, prone to human error and can result in very poor azurerm user experience. Add an exception as a last resort only when all the consequences are fully understood. | |
| > using a preview API version can be risky, prone to human error, and can result in a substandard user experience. An exception is a last resort only when all the consequences are fully understood and there is no alternitive. |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| To add an exception to use preview API version, following criteria must be met: | |
| To add an exception to use preview API version, the following criteria must be met: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixedin 205c261
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 1. There is a clear business reason for not using stable API version, for example: reputational damage to Microsoft / Azure because azurerm Terraform support for the feature cannot be provided in time to meet customers' expectation and azapi support is insufficient | |
| 1. Guarantee that no breaking change will be made to the relevant preview API that could negatively impact azurerm users | |
| 1. There is a commitment to release a stable API version in the near future, a specific target date has to be set | |
| 1. There is a responsible individual with deep knowledge of the API that can be contacted in the future if required | |
| 1. There is an agreement between Microsoft and Hashicorp that the exception is appropriate | |
| 1. There is a clear and compelling reason for not using a stable API version. For example: the fix for a critical security vulnerability or impactful bug is only available in a the preview version. | |
| 1. There is a commitment from the service that no breaking changes will be made to the relevant preview API that could negatively impact azurerm users. | |
| 1. There is a commitment from the service team to release a stable API version in the near future, a specific target date has to be set. | |
| 1. There is a responsible individual with deep knowledge of the API that can be contacted in the future if required. | |
| 1. There is an agreement between Microsoft and Hashicorp that the exception is appropriate. |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| > Feature being in preview phase is not a sufficient reason to add this exception. The concept of preview should be decoupled between feature and ARM API. It is okay to leave the feature in preview phase while having the API promoted to stable. This will safeguard the API against breaking changes and ensure azurerm support for the feature can be shipped sooner to customers. Otherwise Terraform support for the feature should be provided via azapi. | |
| > A feature being in preview phase is not a sufficient reason to add this exception. The concept of preview should be decoupled between feature and ARM API. It is okay to leave the feature in preview phase while having the API promoted to stable. This will safeguard the API against breaking changes and ensure azurerm support for the feature can be shipped sooner to customers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 205c261
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| ## Tool: `api-version-lint` | ||
|
|
||
| Check and fail if preview ARM API version is imported in the vendor folder. Exceptions can be made for specific | ||
| circumstances. More info: https://github.com/hashicorp/terraform-provider-azurerm/blob/main/contributing/topics/guide-api-version.md. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| # Warning! Preview API usage is risky and can cause poor azurerm user experience. | ||
| # Please ensure all the consequences are fully understood before adding an exception: | ||
| # https://github.com/hashicorp/terraform-provider-azurerm/tree/main/contributing/topics/guide-api-version.md | ||
|
|
||
| # Entries have to be sorted alphabetically by module, service, version. | ||
| # Example: | ||
| # - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| # service: compute | ||
| # version: 2021-06-01-preview | ||
| # stableVersionTargetDate: 2026-01-01 | ||
| # responsibleIndividual: github.com/gerrytan |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,166 @@ | ||
| # Exceptions for historical uses of preview API before the linter tool is setup | ||
| # DO NOT ADD NEW ENTRY | ||
|
|
||
| - module: github.com/Azure/azure-sdk-for-go | ||
| service: resources | ||
| version: 2021-06-01-preview | ||
|
|
||
| - module: github.com/Azure/azure-sdk-for-go | ||
| service: security | ||
| version: v3.0 | ||
|
|
||
| - module: github.com/Azure/azure-sdk-for-go | ||
| service: securityinsight | ||
| version: 2021-09-01-preview | ||
|
|
||
| - module: github.com/Azure/azure-sdk-for-go | ||
| service: sql | ||
| version: v5.0 | ||
|
|
||
| - module: github.com/Azure/azure-sdk-for-go | ||
| service: synapse | ||
| version: v2.0 | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: aadb2c | ||
| version: 2021-04-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: appplatform | ||
| version: 2024-01-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: authorization | ||
| version: 2022-05-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: automation | ||
| version: 2020-01-13-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: blueprints | ||
| version: 2018-11-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: codesigning | ||
| version: 2024-09-30-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: containerregistry | ||
| version: 2019-06-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: containerregistry | ||
| version: 2023-11-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: customproviders | ||
| version: 2018-09-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: databricks | ||
| version: 2022-04-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: databricks | ||
| version: 2022-10-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: eventgrid | ||
| version: 2023-12-15-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: eventhub | ||
| version: 2022-01-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: insights | ||
| version: 2019-10-17-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: insights | ||
| version: 2021-05-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: insights | ||
| version: 2021-07-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: insights | ||
| version: 2023-03-15-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: iotcentral | ||
| version: 2021-11-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: nginx | ||
| version: 2024-11-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: operationsmanagement | ||
| version: 2015-11-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: portal | ||
| version: 2019-01-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: search | ||
| version: 2024-06-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: security | ||
| version: 2019-01-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: security | ||
| version: 2022-12-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: securityinsights | ||
| version: 2022-10-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: securityinsights | ||
| version: 2023-12-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: sql | ||
| version: 2023-08-01-preview | ||
|
|
||
| - module: github.com/hashicorp/go-azure-sdk/resource-manager | ||
| service: streamanalytics | ||
| version: 2021-10-01-preview | ||
|
|
||
| - module: github.com/jackofallops/kermit/sdk | ||
| service: appplatform | ||
| version: 2023-05-01-preview | ||
|
|
||
| - module: github.com/jackofallops/kermit/sdk | ||
| service: botservice | ||
| version: 2021-05-01-preview | ||
|
|
||
| - module: github.com/jackofallops/kermit/sdk | ||
| service: iotcentral | ||
| version: 2022-10-31-preview | ||
|
|
||
| - module: github.com/jackofallops/kermit/sdk | ||
| service: iothub | ||
| version: 2022-04-30-preview | ||
|
|
||
| - module: github.com/jackofallops/kermit/sdk | ||
| service: securityinsights | ||
| version: 2022-10-01-preview | ||
|
|
||
| - module: github.com/jackofallops/kermit/sdk | ||
| service: synapse | ||
| version: 2019-06-01-preview | ||
|
|
||
| - module: github.com/jackofallops/kermit/sdk | ||
| service: synapse | ||
| version: 2020-08-01-preview | ||
|
|
||
| - module: github.com/jackofallops/kermit/sdk | ||
| service: synapse | ||
| version: 2021-06-01-preview |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| package main | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "os" | ||
| "path/filepath" | ||
| "sort" | ||
| "strings" | ||
|
|
||
| "github.com/hashicorp/terraform-provider-azurerm/internal/tools/api-version-lint/sdk" | ||
| "github.com/hashicorp/terraform-provider-azurerm/internal/tools/api-version-lint/version" | ||
| ) | ||
|
|
||
| const ( | ||
| EXCEPTIONS_FILE = "internal/tools/api-version-lint/exceptions.yml" | ||
| HISTORICAL_EXCEPTIONS_FILE = "internal/tools/api-version-lint/historical-exceptions.yml" | ||
| VENDOR_DIR = "vendor" | ||
| ) | ||
|
|
||
| func main() { | ||
| historicalExceptions, err := version.ParseHistoricalExceptions(filepath.FromSlash(HISTORICAL_EXCEPTIONS_FILE)) | ||
| if err != nil { | ||
| fmt.Fprintf(os.Stderr, "failed to parse historical exceptions file %s:\n\n%s\n\n", HISTORICAL_EXCEPTIONS_FILE, err) | ||
| printErrorFooterAndExit() | ||
| } | ||
| exceptions, err := version.ParseExceptions(filepath.FromSlash(EXCEPTIONS_FILE)) | ||
| if err != nil { | ||
| fmt.Fprintf(os.Stderr, "failed to parse exceptions file %s:\n\n%s\n\n", EXCEPTIONS_FILE, err) | ||
| printErrorFooterAndExit() | ||
| } | ||
|
|
||
| previewVersions := map[version.Version]bool{} | ||
| for _, sdkType := range sdk.SdkTypes { | ||
| if err := populatePreviewVersions(sdkType, previewVersions); err != nil { | ||
| fmt.Fprintf(os.Stderr, "failed to populate preview versions for SDK type %s:\n\n%s\n\n", sdkType.Module, err) | ||
| printErrorFooterAndExit() | ||
| } | ||
| } | ||
|
|
||
| unusedExceptions := []string{} | ||
| for _, exception := range historicalExceptions { | ||
| if !previewVersions[exception] { | ||
| unusedExceptions = append(unusedExceptions, fmt.Sprintf("module: %s, service: %s, version: %s", exception.Module, exception.Service, exception.Version)) | ||
| } else { | ||
| delete(previewVersions, exception) | ||
| } | ||
| } | ||
| failIfAnyUnusuedExceptions(unusedExceptions, HISTORICAL_EXCEPTIONS_FILE) | ||
|
|
||
| for _, exception := range exceptions { | ||
| if !previewVersions[exception] { | ||
| unusedExceptions = append(unusedExceptions, fmt.Sprintf("module: %s, service: %s, version: %s", exception.Module, exception.Service, exception.Version)) | ||
| } else { | ||
| delete(previewVersions, exception) | ||
| } | ||
| } | ||
| failIfAnyUnusuedExceptions(unusedExceptions, EXCEPTIONS_FILE) | ||
|
|
||
| if len(previewVersions) > 0 { | ||
| invalidPreviewVersions := []string{} | ||
| for svcVer := range previewVersions { | ||
| invalidPreviewVersions = append(invalidPreviewVersions, fmt.Sprintf("module: %s, service: %s, version: %s", svcVer.Module, svcVer.Service, svcVer.Version)) | ||
| } | ||
| sort.Strings(invalidPreviewVersions) | ||
|
|
||
| fmt.Fprintf(os.Stderr, "❌ Invalid use of preview ARM API versions detected in `vendor` folder:\n\n") | ||
| for _, v := range invalidPreviewVersions { | ||
| fmt.Fprintf(os.Stderr, "%s\n", v) | ||
| } | ||
| fmt.Fprintf(os.Stderr, ` | ||
| Preview versions are prone to breaking changes resulting in very bad user experience, | ||
| please use stable version instead. | ||
|
|
||
| `) | ||
| printErrorFooterAndExit() | ||
| } | ||
|
|
||
| fmt.Printf("✅ No invalid use of preview ARM API versions detected in %s folder.\n", VENDOR_DIR) | ||
| } | ||
|
|
||
| func populatePreviewVersions(sdkType sdk.SdkType, previewServiceVersions map[version.Version]bool) error { | ||
| return filepath.WalkDir(filepath.FromSlash(VENDOR_DIR+"/"+sdkType.Module), func(path string, dir os.DirEntry, err error) error { | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if dir.IsDir() { | ||
| matches := sdkType.ServiceAndVersionRegex.FindStringSubmatch(filepath.ToSlash(path)) | ||
| if len(matches) == 3 { | ||
| previewServiceVersions[version.Version{ | ||
| Module: sdkType.Module, | ||
| Service: strings.ToLower(matches[1]), | ||
| Version: strings.ToLower(matches[2]), | ||
| }] = true | ||
| } | ||
| } | ||
| return nil | ||
| }) | ||
| } | ||
|
|
||
| func printErrorFooterAndExit() { | ||
| fmt.Fprintf(os.Stderr, `More information: https://github.com/hashicorp/terraform-provider-azurerm/blob/main/contributing/topics/guide-api-version.md | ||
|
|
||
| To rerun this check locally, use: go run internal/tools/api-version-lint/main.go | ||
| `) | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| func failIfAnyUnusuedExceptions(unusedExceptions []string, exceptionsFile string) { | ||
| if len(unusedExceptions) > 0 { | ||
| fmt.Fprintf(os.Stderr, "❌ Unused exceptions detected in `%s` file, remove these entries:\n\n", exceptionsFile) | ||
| for _, unusedException := range unusedExceptions { | ||
| fmt.Fprintln(os.Stderr, unusedException) | ||
| } | ||
| fmt.Fprintf(os.Stderr, "\n") | ||
| printErrorFooterAndExit() | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.