Skip to content

Add exceptions for resources where the user needs to specify the API version#3985

Merged
thomas11 merged 3 commits into
masterfrom
tkappler/user-api-version
Feb 26, 2025
Merged

Add exceptions for resources where the user needs to specify the API version#3985
thomas11 merged 3 commits into
masterfrom
tkappler/user-api-version

Conversation

@thomas11

@thomas11 thomas11 commented Feb 25, 2025

Copy link
Copy Markdown
Contributor

Resolves #3524

This PR adds an exception for two resources where the api-version query parameter works differently. Here, the endpoints receiving the API version merely pass it on to another service, and it's meant to express the API version the second service should use. Therefore, our usual logic of auto-adding the API version doesn't work. The user must instead specify the API version for the second service.

Example:

resources.NewResource(ctx, "acc", &resources.ResourceArgs{
	ResourceProviderNamespace: pulumi.String("Microsoft.Storage"),
	ResourceType:              pulumi.String("storageAccounts"),
	ApiVersion:                pulumi.String("2022-09-01"),
})

The provider will make a request to the resources service (RP) here. The resources service will then in turn make a request to the Microsoft.Storage service. The API version is meant for the latter to receive, so we cannot use whatever API version we have as default for resources.Resource.

This PR, specifically fixing resource.Resource, unlocks the creation of GitHub.Network/networkSettings and other resources that are not in the Azure spec. It will also allow users to use Resource as an escape hatch when there are problems with the provider schema.

#2467 is closely related - it's just a matter of adding the ApiVersion property for all resources, maybe with a different name.

Potential improvement: the description "The API version to use for the operation." for ApiVersion is not useful, we could replace it.

@thomas11 thomas11 self-assigned this Feb 25, 2025
@thomas11 thomas11 force-pushed the tkappler/user-api-version branch from 4b462bd to 52bcb44 Compare February 25, 2025 14:43
@github-actions

github-actions Bot commented Feb 25, 2025

Copy link
Copy Markdown
Contributor

Does the PR have any schema changes?

Found 4 breaking changes:

Resources

  • 🟢 "azure-native:authorization:ManagementLockAtResourceLevel": required inputs: "apiVersion" input has changed to Required
  • 🟢 "azure-native:resources:Resource": required inputs: "apiVersion" input has changed to Required

Functions

  • 🟢 "azure-native:authorization:getManagementLockAtResourceLevel": inputs: required: "apiVersion" input has changed to Required
  • 🟢 "azure-native:resources:getResource": inputs: required: "apiVersion" input has changed to Required
    No new resources/functions.

@thomas11 thomas11 force-pushed the tkappler/user-api-version branch from 52bcb44 to 31f379a Compare February 25, 2025 14:52
@thomas11 thomas11 requested a review from a team February 25, 2025 14:52
@thomas11 thomas11 force-pushed the tkappler/user-api-version branch from 31f379a to 8c50247 Compare February 25, 2025 14:54
@codecov

codecov Bot commented Feb 25, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 57.51%. Comparing base (6e70edd) to head (45d2b4c).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3985      +/-   ##
==========================================
+ Coverage   57.47%   57.51%   +0.04%     
==========================================
  Files          82       82              
  Lines       13081    13092      +11     
==========================================
+ Hits         7518     7530      +12     
+ Misses       4987     4986       -1     
  Partials      576      576              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@danielrbradley danielrbradley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the logic looks okay based on the tests, but I think both the implementation function name and tests could be quite easily refactored to be more understandable.

Comment thread provider/pkg/gen/schema.go Outdated
Comment thread provider/pkg/gen/schema_test.go Outdated
@thomas11 thomas11 force-pushed the tkappler/user-api-version branch from 8c50247 to 45d2b4c Compare February 26, 2025 12:33

@danielrbradley danielrbradley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@pulumi-bot

Copy link
Copy Markdown
Contributor

This PR has been shipped in release v2.89.0.

thomas11 added a commit that referenced this pull request Mar 20, 2025
…Delete (#4043)

This PR is a follow-up to #3985. It was incomplete which went undetected
because of an accidental poor choice of API version in the integration
test. The test used to use 2022-09-01 which happened to be a valid
version for both the `resources` and the `storage` module.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Api version 2024-04-02 for resources Resource

3 participants