CORS-4382: Migrate legacy azure sdk to v2#10484
Open
jhixson74 wants to merge 16 commits intoopenshift:mainfrom
Open
CORS-4382: Migrate legacy azure sdk to v2#10484jhixson74 wants to merge 16 commits intoopenshift:mainfrom
jhixson74 wants to merge 16 commits intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
@jhixson74: This pull request references CORS-4382 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the spike to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReplaces legacy Azure autorest/profile SDK usage with Azure SDK for Go v2 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
jhixson74
force-pushed
the
main-migrate-legacy-azure-sdk-to-v2
branch
from
bca29c8 to
e5080d1
Compare
jhixson74
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
pkg/asset/machines/clusterapi.go (1)
265-266: Extract the marketplace publisher normalization once.This is now one of three copies of the same Azure marketplace publisher reconciliation (
pkg/asset/machines/master.goandpkg/asset/machines/worker.gohave the same block). A shared helper would make the next SDK shape change a one-file update instead of three.As per coding guidelines, Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/asset/machines/clusterapi.go` around lines 265 - 266, Extract the repeated Azure marketplace publisher reconciliation into a single helper and call it from clusterapi.go, master.go and worker.go; specifically, move the logic that sets mpool.OSImage.Publisher from img.Properties.Plan.Publisher into a new package-level function (e.g., setOSImagePublisherFromAzureImage(img, mpool)) that checks img.Properties, img.Properties.Plan and img.Properties.Plan.Publisher and assigns mpool.OSImage.Publisher when present, then replace the three inline blocks (the one referencing mpool.OSImage.Publisher and img.Properties.Plan.Publisher) with a call to that new helper so future SDK shape changes are fixed in one place.pkg/asset/manifests/azure/cluster.go (1)
598-614: Collapse the subnet-prefix parsing into one helper.The V2
AddressPrefix/AddressPrefixeshandling now lives in three places. Keeping them separate makes pointer-slice edge fixes easy to miss; return raw prefixes once and convert tonet.IPNet/ipnet.IPNetat the call sites.Also applies to: 656-673, 743-759
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/asset/manifests/azure/cluster.go` around lines 598 - 614, The repeated parsing of controlPlaneSubnet.Properties.AddressPrefixes and AddressPrefix into net.IPNet slices (using net.ParseCIDR and appending to subnetList) should be collapsed into a single helper (e.g., a new function like parseAzureSubnetPrefixes or getAzureSubnetPrefixes) that accepts the subnet object (controlPlaneSubnet) and returns the raw prefix strings or a unified []string; replace the three duplicated blocks (the current block handling AddressPrefixes/AddressPrefix and the similar blocks at the other mentioned regions) with calls to this helper, and move the net.ParseCIDR conversion to the call sites that need net.IPNet so pointer vs slice/pointer edge cases are handled centrally and conversion is consistent.pkg/asset/installconfig/azure/validation_test.go (1)
1358-1376: Keep these fixtures local to the test.Reassigning
locationsAPIResultandresourcesProviderAPIResultmutates package state, which makes latert.Parallel()adoption and helper reuse brittle. Build local values and wire them directly into this test's mock setup instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/asset/installconfig/azure/validation_test.go` around lines 1358 - 1376, The test currently mutates package-level variables locationsAPIResult and resourcesProviderAPIResult; instead, construct fresh local variables inside this test (e.g., localLocations := make([]*armsubscriptions.Location, 0) and localResourcesProvider := &armresources.Provider{...}) populated the same way as the current fixtures (using to.Ptr and toStringPtrSlice), and pass those locals into the mock expectations (azureClient.EXPECT().ListLocations(...).Return(localLocations, nil) and the provider-related EXPECT returning localResourcesProvider) so no package state is reassigned.pkg/destroy/azure/azure.go (1)
791-797: Auth error detection range may be overly broad.The check
respErr.StatusCode >= 400 && respErr.StatusCode <= 403includes HTTP 400 (Bad Request) and 402 (Payment Required), which are not authentication/authorization errors. Typically only 401 (Unauthorized) and 403 (Forbidden) indicate auth issues.Suggested fix
var respErr *azcore.ResponseError if errors.As(err, &respErr) { - if respErr.StatusCode >= 400 && respErr.StatusCode <= 403 { + if respErr.StatusCode == http.StatusUnauthorized || respErr.StatusCode == http.StatusForbidden { return true } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/destroy/azure/azure.go` around lines 791 - 797, The current auth-detection block that inspects azcore.ResponseError (the local variable respErr used with errors.As(err, &respErr) in azure.go) treats any 4xx up to 403 as an auth error; narrow this to only treat 401 and 403 as authentication/authorization failures by changing the condition to explicitly check respErr.StatusCode == 401 || respErr.StatusCode == 403 so 400/402/etc. are not misclassified.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.coderabbit.yaml:
- Around line 20-462: The pre_merge_checks.custom_checks list exceeds the schema
limit of five entries; locate the top-level pre_merge_checks.custom_checks block
and reduce the entries to five by merging related checks (e.g., combine
"MicroShift Test Compatibility" and "Single Node OpenShift (SNO) Test
Compatibility" into one compatibility check) or remove an unneeded entry (one of
the named checks like "Topology-Aware Scheduling Compatibility" or "IPv6 and
Disconnected Network Test Compatibility"), update any merged instruction text
accordingly (retain unique guidance from removed checks), and validate the YAML
so reviews.pre_merge_checks.custom_checks contains exactly five items.
- Around line 4-6: The YAML places the key "inheritance" under the "reviews"
block which is invalid; remove "inheritance: true" from inside the "reviews"
mapping and add it as a top-level key (same indentation as "reviews"), e.g. keep
"reviews:" with its "profile: chill" child and add "inheritance: true" on its
own line at the document root so the keys are siblings.
In `@docs/superpowers/plans/2026-04-08-azure-sdk-v2-migration.md`:
- Around line 2129-2147: The checklist step "Step 10.1.1: Remove legacy imports"
incorrectly instructs removal of the azurestackdns import; keep the
azurestackdns symbol (azurestackdns
"github.com/Azure/azure-sdk-for-go/profiles/2018-03-01/dns/mgmt/dns") as an
explicit exception and update the step text to exclude it from the removal list,
and make the same correction in the other occurrence referenced (the block
around lines 2284-2301) so the later verification/summary remains consistent
with preserving legacy DNS for Azure Stack.
In `@pkg/infrastructure/azure/storage.go`:
- Around line 577-605: The storage client creation in createAccountOnStack and
getStorageAccountKey omits the stack APIVersion; update the arm.ClientOptions
passed to armstorage.NewAccountsClient (and any storage clients) to include
APIVersion: stackAPIVersion (matching how compute/dns/network clients use it) or
reuse the existing preconfigured client options (e.g., p.clientOptions /
CreateBlockBlobInput.ClientOpts) so Stack deployments use the correct API
version; ensure the change is applied to both the BeginCreate/poller block and
the getStorageAccountKey client creation paths.
---
Nitpick comments:
In `@pkg/asset/installconfig/azure/validation_test.go`:
- Around line 1358-1376: The test currently mutates package-level variables
locationsAPIResult and resourcesProviderAPIResult; instead, construct fresh
local variables inside this test (e.g., localLocations :=
make([]*armsubscriptions.Location, 0) and localResourcesProvider :=
&armresources.Provider{...}) populated the same way as the current fixtures
(using to.Ptr and toStringPtrSlice), and pass those locals into the mock
expectations (azureClient.EXPECT().ListLocations(...).Return(localLocations,
nil) and the provider-related EXPECT returning localResourcesProvider) so no
package state is reassigned.
In `@pkg/asset/machines/clusterapi.go`:
- Around line 265-266: Extract the repeated Azure marketplace publisher
reconciliation into a single helper and call it from clusterapi.go, master.go
and worker.go; specifically, move the logic that sets mpool.OSImage.Publisher
from img.Properties.Plan.Publisher into a new package-level function (e.g.,
setOSImagePublisherFromAzureImage(img, mpool)) that checks img.Properties,
img.Properties.Plan and img.Properties.Plan.Publisher and assigns
mpool.OSImage.Publisher when present, then replace the three inline blocks (the
one referencing mpool.OSImage.Publisher and img.Properties.Plan.Publisher) with
a call to that new helper so future SDK shape changes are fixed in one place.
In `@pkg/asset/manifests/azure/cluster.go`:
- Around line 598-614: The repeated parsing of
controlPlaneSubnet.Properties.AddressPrefixes and AddressPrefix into net.IPNet
slices (using net.ParseCIDR and appending to subnetList) should be collapsed
into a single helper (e.g., a new function like parseAzureSubnetPrefixes or
getAzureSubnetPrefixes) that accepts the subnet object (controlPlaneSubnet) and
returns the raw prefix strings or a unified []string; replace the three
duplicated blocks (the current block handling AddressPrefixes/AddressPrefix and
the similar blocks at the other mentioned regions) with calls to this helper,
and move the net.ParseCIDR conversion to the call sites that need net.IPNet so
pointer vs slice/pointer edge cases are handled centrally and conversion is
consistent.
In `@pkg/destroy/azure/azure.go`:
- Around line 791-797: The current auth-detection block that inspects
azcore.ResponseError (the local variable respErr used with errors.As(err,
&respErr) in azure.go) treats any 4xx up to 403 as an auth error; narrow this to
only treat 401 and 403 as authentication/authorization failures by changing the
condition to explicitly check respErr.StatusCode == 401 || respErr.StatusCode ==
403 so 400/402/etc. are not misclassified.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d7b62906-3edd-4200-a87a-cd46c990d908
⛔ Files ignored due to path filters (109)
go.sumis excluded by!**/*.sumnode_modules/.package-lock.jsonis excluded by!**/node_modules/**vendor/github.com/Azure/azure-sdk-for-go/profiles/2018-03-01/resources/mgmt/subscriptions/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/profiles/latest/compute/mgmt/compute/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/profiles/latest/marketplaceordering/mgmt/marketplaceordering/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/autorest.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/build.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/ci.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/client_factory.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/constants.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/marketplaceagreements_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/models_serde.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/operations_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/response_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/time_rfc3339.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/assets.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/autorest.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/build.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/ci.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/client_factory.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/constants.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/models_serde.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/operations_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/response_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/subscription_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/tenants_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/_meta.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/availabilitysets.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/capacityreservationgroups.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/capacityreservations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/cloudserviceoperatingsystems.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/cloudserviceroleinstances.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/cloudserviceroles.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/cloudservices.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/cloudservicesupdatedomain.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/communitygalleries.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/communitygalleryimages.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/communitygalleryimageversions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/dedicatedhostgroups.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/dedicatedhosts.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/diskaccesses.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/diskencryptionsets.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/diskrestorepoint.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/disks.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/enums.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/galleries.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/galleryapplications.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/galleryapplicationversions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/galleryimages.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/galleryimageversions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/gallerysharingprofile.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/images.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/loganalytics.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/operations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/proximityplacementgroups.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/resourceskus.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/restorepointcollections.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/restorepoints.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/sharedgalleries.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/sharedgalleryimages.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/sharedgalleryimageversions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/snapshots.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/sshpublickeys.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/usage.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachineextensionimages.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachineextensions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachineimages.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachineimagesedgezone.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachineruncommands.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachines.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinescalesetextensions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinescalesetrollingupgrades.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinescalesets.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinescalesetvmextensions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinescalesetvmruncommands.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinescalesetvms.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinesizes.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/_meta.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/marketplaceagreements.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/operations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/_meta.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/enums.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/operations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/subscriptions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/tenants.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/version.gois excluded by!vendor/**,!**/vendor/**vendor/modules.txtis excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (18)
.coderabbit.yamlazure.testdocs/superpowers/plans/2026-04-08-azure-sdk-v2-migration.mdgo.modlegacy-files.txtpkg/asset/cluster/azure/azure.gopkg/asset/installconfig/azure/azure.gopkg/asset/installconfig/azure/client.gopkg/asset/installconfig/azure/dns.gopkg/asset/installconfig/azure/mock/azureclient_generated.gopkg/asset/installconfig/azure/validation.gopkg/asset/installconfig/azure/validation_test.gopkg/asset/machines/clusterapi.gopkg/asset/machines/master.gopkg/asset/machines/worker.gopkg/asset/manifests/azure/cluster.gopkg/destroy/azure/azure.gopkg/infrastructure/azure/storage.go
jhixson74
force-pushed
the
main-migrate-legacy-azure-sdk-to-v2
branch
from
e5080d1 to
89c6537
Compare
Replace legacy Azure SDK imports with V2 SDK for DNS functionality: - Update ZonesClient and RecordSetsClient to use armdns types - Update newZonesClient and newRecordSetsClient to return errors and use V2 client creation pattern with arm.ClientOptions - Update GetAllPublicZones to use V2 pager pattern - Update GetRecordSet to use V2 Get pattern - Update GetDNSRecordSet to handle errors from client creation - Update GetDNSZone to handle errors from newZonesClient - Update validation.go DNS RecordType references to use armdns types - Replace to.String/to.Int32Ptr with direct pointer dereference/to.Ptr Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Migrate network-related methods in pkg/asset/installconfig/azure/client.go from legacy Azure SDK profiles to V2 SDK. This includes: - Update GetVirtualNetwork to use armnetwork.NewVirtualNetworksClient - Update CheckIPAddressAvailability to use V2 SDK - Update getSubnet, GetComputeSubnet, GetControlPlaneSubnet to use armnetwork.NewSubnetsClient - Update helper methods getVirtualNetworksClient and getSubnetsClient to V2 pattern - Update validation.go to use armnetwork types instead of aznetwork - Update validation_test.go to use armnetwork types - Regenerate mocks to match new API interface Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Migrate the ListLocations method and getSubscriptionsClient helper in pkg/asset/installconfig/azure/client.go from the legacy Azure SDK profiles to the V2 SDK (armsubscriptions). This change: - Adds armsubscriptions import and removes azsubs import - Updates ListLocations to return []*armsubscriptions.Location - Updates getSubscriptionsClient to use V2 client creation pattern - Updates azure.go, validation.go, and validation_test.go to use the new return type (removing pointer indirection) - Regenerates mocks to match the updated interface - Adds armsubscriptions dependency to go.mod and vendors it Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Migrate the resources/providers client methods in client.go from the legacy Azure SDK profiles to the V2 SDK (armresources package): - Update GetResourcesProvider to use armresources.ProvidersClient - Update GetGroup to use armresources.ResourceGroupsClient - Update ListResourceIDsByGroup to use armresources.Client - Update API interface signatures to use armresources types - Update validation.go to handle V2 SDK property access patterns - Update azure.go getResourceCapableRegions for V2 types - Update validation_test.go to use armresources types - Regenerate mocks Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Convert compute-related methods from legacy SDK to V2: - Replace azenc (profiles/latest/compute) with armcompute/v5 - Update ResourceSKUsClient, DiskEncryptionSetsClient, VirtualMachineImagesClient to V2 - Update API interface method signatures for compute types - Update pagination to use V2 pager pattern - Replace autorest/to with azcore/to - Update validation.go to use V2 types (armcompute.DiskEncryptionSetTypeConfidentialVMEncryptedWithCustomerKey, Properties instead of EncryptionSetProperties, etc.) - Update validation_test.go for V2 types - Regenerate mocks Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Migrate AreMarketplaceImageTermsAccepted from legacy Azure SDK profiles (profiles/latest/marketplaceordering/mgmt/marketplaceordering) to V2 SDK (resourcemanager/marketplaceordering/armmarketplaceordering). This is the last legacy client migration in client.go. All Azure API clients in this file now use the V2 SDK with TokenCreds and CloudConfig. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Migrates the cluster uninstaller in pkg/destroy/azure/azure.go from legacy Azure SDK (autorest-based) to Azure SDK for Go V2. Changes: - Replace legacy DNS clients (dns.ZonesClient, dns.RecordSetsClient, privatedns.PrivateZonesClient, privatedns.RecordSetsClient) with V2 clients (armdns.ZonesClient, armdns.RecordSetsClient, armprivatedns.PrivateZonesClient, armprivatedns.RecordSetsClient) - Replace legacy resources.GroupsClient with armresources.ResourceGroupsClient - Update configureClients() to use V2 client constructors with TokenCredential and CloudConfig - Update all pagination to use V2 pager patterns (NewListPager, More(), NextPage()) - Update deleteResourceGroup() to use BeginDelete/PollUntilDone pattern - Update error handling functions (isNotFoundError, isAuthError, isResourceGroupBlockedError) to handle both V2 azcore.ResponseError and legacy autorest.DetailedError (for Azure Stack compatibility) - Keep azurestackdns using legacy SDK as Azure Stack requires older API versions not supported by V2 Legacy imports removed: - github.com/Azure/azure-sdk-for-go/services/preview/dns/mgmt/2018-03-01-preview/dns - github.com/Azure/azure-sdk-for-go/services/privatedns/mgmt/2018-09-01/privatedns - github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources - github.com/Azure/go-autorest/autorest/azure (azureenv) V2 imports added: - github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/dns/armdns - github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/privatedns/armprivatedns Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update property access patterns for V2 SDK types across consumers: - VirtualNetwork.AddressSpace -> Properties.AddressSpace - Subnet.AddressPrefixes -> Properties.AddressPrefixes - VirtualMachineImage.Plan -> Properties.Plan Migrate remaining legacy SDK usages: - pkg/asset/cluster/azure: resources client - pkg/infrastructure/azure: storage accounts client Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add APIVersion field to CreateStorageAccountInput to allow callers to
specify a custom API version for storage management operations.
For Azure Stack Hub, use the AzureStackStorageAPIVersion constant
("2019-06-01") which is required for compatibility with Azure Stack Hub's
older API surface.
The Azure SDK v2 now supports API version override via the
policy.ClientOptions.APIVersion field, removing the need for separate
legacy SDK code paths in some cases.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
jhixson74
force-pushed
the
main-migrate-legacy-azure-sdk-to-v2
branch
from
7f9e254 to
8aa1c58
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/superpowers/plans/2026-04-08-azure-sdk-v2-migration.md`:
- Around line 313-321: The package-wide build/test checkpoint (Step 1.11/1.12)
runs before consumer updates and will fail due to the DNS API surface change;
change these steps to either (a) run go build/go test only for the specific
files or smaller packages that were changed (e.g., build the modified azure DNS
code only) or (b) move the full-package `go build
./pkg/asset/installconfig/azure/...` and `go test
./pkg/asset/installconfig/azure/...` checks to after the tasks that update all
consumers (addressing dns.go and validation_test.go). Update the step
descriptions to reference Step 1 and dns.go/validation_test.go so the CI
checkpoint is executed at the correct point.
- Around line 662-669: Update the ARM client examples to use
azcore.ClientOptions instead of policy.ClientOptions: in the
getVirtualNetworksClient function (and the other similar snippets) replace the
type reference policy.ClientOptions with azcore.ClientOptions when constructing
arm.ClientOptions (i.e., the inner ClientOptions field should be
azcore.ClientOptions). Ensure you import azcore where needed and leave
arm.ClientOptions wrapping that azcore.ClientOptions so
armnetwork.NewVirtualNetworksClient(...) compiles correctly.
In `@pkg/asset/installconfig/azure/validation_test.go`:
- Around line 1358-1376: The test currently reassigns package-level fixtures
locationsAPIResult and resourcesProviderAPIResult, which mutates shared state;
instead, create local variables (e.g., localLocations and
localResourcesProvider) inside the test and populate them from validRegionList,
validResourceGroupNamespace, validResourceGroupResourceType (and
toStringPtrSlice) and use those local variables in the mock setup
(azureClient.EXPECT().ListLocations(...).Return(localLocations, nil) and any
expectation that referenced resourcesProviderAPIResult). Ensure you do not
reassign the package-level names so the test becomes order- and
concurrency-safe.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8287d73b-4f22-40ab-b210-a079624a8df3
⛔ Files ignored due to path filters (109)
go.sumis excluded by!**/*.sumnode_modules/.package-lock.jsonis excluded by!**/node_modules/**vendor/github.com/Azure/azure-sdk-for-go/profiles/2018-03-01/resources/mgmt/subscriptions/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/profiles/latest/compute/mgmt/compute/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/profiles/latest/marketplaceordering/mgmt/marketplaceordering/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/autorest.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/build.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/ci.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/client_factory.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/constants.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/marketplaceagreements_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/models_serde.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/operations_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/response_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/time_rfc3339.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/assets.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/autorest.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/build.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/ci.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/client_factory.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/constants.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/models_serde.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/operations_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/response_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/subscription_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/tenants_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/_meta.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/availabilitysets.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/capacityreservationgroups.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/capacityreservations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/cloudserviceoperatingsystems.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/cloudserviceroleinstances.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/cloudserviceroles.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/cloudservices.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/cloudservicesupdatedomain.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/communitygalleries.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/communitygalleryimages.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/communitygalleryimageversions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/dedicatedhostgroups.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/dedicatedhosts.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/diskaccesses.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/diskencryptionsets.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/diskrestorepoint.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/disks.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/enums.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/galleries.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/galleryapplications.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/galleryapplicationversions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/galleryimages.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/galleryimageversions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/gallerysharingprofile.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/images.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/loganalytics.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/operations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/proximityplacementgroups.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/resourceskus.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/restorepointcollections.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/restorepoints.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/sharedgalleries.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/sharedgalleryimages.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/sharedgalleryimageversions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/snapshots.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/sshpublickeys.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/usage.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachineextensionimages.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachineextensions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachineimages.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachineimagesedgezone.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachineruncommands.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachines.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinescalesetextensions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinescalesetrollingupgrades.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinescalesets.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinescalesetvmextensions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinescalesetvmruncommands.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinescalesetvms.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinesizes.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/_meta.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/marketplaceagreements.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/operations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/_meta.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/enums.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/operations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/subscriptions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/tenants.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/version.gois excluded by!vendor/**,!**/vendor/**vendor/modules.txtis excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (17)
azure.testdocs/superpowers/plans/2026-04-08-azure-sdk-v2-migration.mdgo.modlegacy-files.txtpkg/asset/cluster/azure/azure.gopkg/asset/installconfig/azure/azure.gopkg/asset/installconfig/azure/client.gopkg/asset/installconfig/azure/dns.gopkg/asset/installconfig/azure/mock/azureclient_generated.gopkg/asset/installconfig/azure/validation.gopkg/asset/installconfig/azure/validation_test.gopkg/asset/machines/clusterapi.gopkg/asset/machines/master.gopkg/asset/machines/worker.gopkg/asset/manifests/azure/cluster.gopkg/destroy/azure/azure.gopkg/infrastructure/azure/storage.go
✅ Files skipped from review due to trivial changes (4)
- legacy-files.txt
- pkg/asset/manifests/azure/cluster.go
- pkg/asset/installconfig/azure/validation.go
- pkg/infrastructure/azure/storage.go
🚧 Files skipped from review as they are similar to previous changes (6)
- pkg/asset/machines/worker.go
- pkg/asset/installconfig/azure/azure.go
- go.mod
- pkg/destroy/azure/azure.go
- pkg/asset/installconfig/azure/mock/azureclient_generated.go
- pkg/asset/installconfig/azure/client.go
Comment on lines
+1358
to
1376
| locationsAPIResult = func() []*armsubscriptions.Location { | ||
| r := []*armsubscriptions.Location{} | ||
| for i := 0; i < len(validRegionList); i++ { | ||
| r = append(r, azsubs.Location{Name: to.StringPtr(validRegionList[i]), DisplayName: to.StringPtr(validRegionList[i])}) | ||
| r = append(r, &armsubscriptions.Location{Name: to.Ptr(validRegionList[i]), DisplayName: to.Ptr(validRegionList[i])}) | ||
| } | ||
| return &r | ||
| return r | ||
| }() | ||
| // Location | ||
| azureClient.EXPECT().ListLocations(gomock.Any()).Return(locationsAPIResult, nil).AnyTimes() | ||
|
|
||
| resourcesProviderAPIResult = &azres.Provider{ | ||
| Namespace: to.StringPtr(validResourceGroupNamespace), | ||
| ResourceTypes: &[]azres.ProviderResourceType{ | ||
| resourcesProviderAPIResult = &armresources.Provider{ | ||
| Namespace: to.Ptr(validResourceGroupNamespace), | ||
| ResourceTypes: []*armresources.ProviderResourceType{ | ||
| { | ||
| ResourceType: &validResourceGroupResourceType, | ||
| Locations: &validRegionList, | ||
| Locations: toStringPtrSlice(validRegionList), | ||
| }, | ||
| }, | ||
| } |
There was a problem hiding this comment.
Avoid mutating shared fixtures inside this test.
Reassigning the package-level locationsAPIResult and resourcesProviderAPIResult makes the suite order-dependent under go test -shuffle and harder to parallelize. Keep the UltraSSD-specific fixtures local to this test instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/asset/installconfig/azure/validation_test.go` around lines 1358 - 1376,
The test currently reassigns package-level fixtures locationsAPIResult and
resourcesProviderAPIResult, which mutates shared state; instead, create local
variables (e.g., localLocations and localResourcesProvider) inside the test and
populate them from validRegionList, validResourceGroupNamespace,
validResourceGroupResourceType (and toStringPtrSlice) and use those local
variables in the mock setup
(azureClient.EXPECT().ListLocations(...).Return(localLocations, nil) and any
expectation that referenced resourcesProviderAPIResult). Ensure you do not
reassign the package-level names so the test becomes order- and
concurrency-safe.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/superpowers/plans/2026-04-08-azure-sdk-v2-migration.md (1)
154-166: Documentation showsazcore.ClientOptionsbut actual implementation usespolicy.ClientOptions.The plan's code examples (e.g., lines 156-158) use
azcore.ClientOptions, but the actual migrated code inpkg/asset/installconfig/azure/client.gousespolicy.ClientOptions. Both are valid sincepolicy.ClientOptionsandazcore.ClientOptionsare functionally equivalent (same underlying type), but for consistency between documentation and implementation, consider aligning them.This is a documentation-only issue and doesn't affect functionality.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/superpowers/plans/2026-04-08-azure-sdk-v2-migration.md` around lines 154 - 166, The docs example uses azcore.ClientOptions while the codebase's implementation of newZonesClient (returning ZonesGetter / ZonesClient) uses policy.ClientOptions; update the documentation to use policy.ClientOptions (or alternatively update the implementation to azcore.ClientOptions) so the types match for consistency—choose the same symbol (policy.ClientOptions or azcore.ClientOptions) across the docs and the newZonesClient example and ensure the example imports/mentions the same type you use in the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/superpowers/plans/2026-04-08-azure-sdk-v2-migration.md`:
- Around line 154-166: The docs example uses azcore.ClientOptions while the
codebase's implementation of newZonesClient (returning ZonesGetter /
ZonesClient) uses policy.ClientOptions; update the documentation to use
policy.ClientOptions (or alternatively update the implementation to
azcore.ClientOptions) so the types match for consistency—choose the same symbol
(policy.ClientOptions or azcore.ClientOptions) across the docs and the
newZonesClient example and ensure the example imports/mentions the same type you
use in the code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b3df9408-6f70-42dd-9478-6eac44749338
⛔ Files ignored due to path filters (108)
go.sumis excluded by!**/*.sumvendor/github.com/Azure/azure-sdk-for-go/profiles/2018-03-01/resources/mgmt/subscriptions/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/profiles/latest/compute/mgmt/compute/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/profiles/latest/marketplaceordering/mgmt/marketplaceordering/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/autorest.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/build.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/ci.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/client_factory.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/constants.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/marketplaceagreements_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/models_serde.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/operations_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/response_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering/time_rfc3339.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/assets.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/autorest.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/build.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/ci.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/client_factory.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/constants.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/models_serde.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/operations_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/response_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/subscription_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions/tenants_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/_meta.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/availabilitysets.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/capacityreservationgroups.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/capacityreservations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/cloudserviceoperatingsystems.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/cloudserviceroleinstances.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/cloudserviceroles.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/cloudservices.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/cloudservicesupdatedomain.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/communitygalleries.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/communitygalleryimages.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/communitygalleryimageversions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/dedicatedhostgroups.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/dedicatedhosts.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/diskaccesses.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/diskencryptionsets.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/diskrestorepoint.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/disks.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/enums.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/galleries.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/galleryapplications.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/galleryapplicationversions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/galleryimages.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/galleryimageversions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/gallerysharingprofile.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/images.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/loganalytics.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/operations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/proximityplacementgroups.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/resourceskus.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/restorepointcollections.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/restorepoints.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/sharedgalleries.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/sharedgalleryimages.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/sharedgalleryimageversions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/snapshots.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/sshpublickeys.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/usage.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachineextensionimages.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachineextensions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachineimages.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachineimagesedgezone.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachineruncommands.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachines.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinescalesetextensions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinescalesetrollingupgrades.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinescalesets.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinescalesetvmextensions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinescalesetvmruncommands.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinescalesetvms.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute/virtualmachinesizes.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/_meta.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/marketplaceagreements.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/operations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/marketplaceordering/mgmt/2015-06-01/marketplaceordering/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/_meta.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/enums.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/operations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/subscriptions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/tenants.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions/version.gois excluded by!vendor/**,!**/vendor/**vendor/modules.txtis excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (16)
.gitignoredocs/superpowers/plans/2026-04-08-azure-sdk-v2-migration.mdgo.modpkg/asset/cluster/azure/azure.gopkg/asset/installconfig/azure/azure.gopkg/asset/installconfig/azure/client.gopkg/asset/installconfig/azure/dns.gopkg/asset/installconfig/azure/mock/azureclient_generated.gopkg/asset/installconfig/azure/validation.gopkg/asset/installconfig/azure/validation_test.gopkg/asset/machines/clusterapi.gopkg/asset/machines/master.gopkg/asset/machines/worker.gopkg/asset/manifests/azure/cluster.gopkg/destroy/azure/azure.gopkg/infrastructure/azure/storage.go
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- pkg/asset/installconfig/azure/validation_test.go
🚧 Files skipped from review as they are similar to previous changes (9)
- pkg/asset/machines/worker.go
- pkg/asset/machines/clusterapi.go
- pkg/asset/installconfig/azure/azure.go
- pkg/asset/manifests/azure/cluster.go
- pkg/infrastructure/azure/storage.go
- go.mod
- pkg/destroy/azure/azure.go
- pkg/asset/installconfig/azure/dns.go
- pkg/asset/installconfig/azure/mock/azureclient_generated.go
- pkg/asset/installconfig/azure/config.go - Centralized Azure client configuration with:
- ClientConfig struct holding cloud configuration and API version overrides
- Service constants (ServiceStorage, ServiceCompute, ServiceDNS, ServiceNetwork)
- Environment variable support (AZURE_API_VERSION_STORAGE, AZURE_API_VERSION_COMPUTE, AZURE_API_VERSION_DNS, AZURE_API_VERSION_NETWORK)
- Default Azure Stack Hub API versions applied automatically
- ClientOptions(service) and DefaultClientOptions() helper methods
Key Features
1. Centralized API version management - All Azure SDK client creation now goes through ClientConfig
2. Environment variable overrides - API versions can be overridden via environment variables for debugging/testing
3. Automatic Azure Stack Hub support - Default API versions for Stack Hub are applied based on cloud environment
4. Backward compatible - APIVersion constant retained with deprecation notice
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/asset/installconfig/azure/client.go (1)
493-497: Consider usingDefaultClientOptions()for the providers client.The
ProvidersClientis a general ARM resource management client, not a network-specific client. UsingServiceNetworkoptions here applies network-specific API version overrides to a non-network endpoint, which may cause issues on Azure Stack Hub.Suggested fix
- providersClient, err := armresources.NewProvidersClient(c.ssn.Credentials.SubscriptionID, c.ssn.TokenCreds, c.ssn.ClientConfig.ClientOptions(ServiceNetwork)) + providersClient, err := armresources.NewProvidersClient(c.ssn.Credentials.SubscriptionID, c.ssn.TokenCreds, c.ssn.ClientConfig.DefaultClientOptions())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/asset/installconfig/azure/client.go` around lines 493 - 497, In GetRegionAvailabilityZones, the ProvidersClient is being constructed with network-specific options (c.ssn.ClientConfig.ClientOptions(ServiceNetwork)); replace that call with the generic default options (c.ssn.ClientConfig.DefaultClientOptions()) so the providers ARM client uses non-network-specific API/version settings (update the armresources.NewProvidersClient invocation accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/destroy/azure/azure.go`:
- Around line 67-70: The code assumes o.Session.ClientConfig is always non-nil;
add a defensive nil check before calling DefaultClientOptions()/ClientOptions()
on ClientConfig. In the function containing these lines, verify o.Session != nil
and o.Session.ClientConfig != nil and if nil either return a descriptive error
(e.g., "missing Session.ClientConfig") or initialize sensible default client
options, then proceed to call DefaultClientOptions(),
ClientOptions(azuresession.ServiceDNS) and
ClientOptions(azuresession.ServiceNetwork); reference the symbols ClientConfig,
o.Session, DefaultClientOptions, ClientOptions, and
azuresession.ServiceDNS/ServiceNetwork when making the change.
---
Nitpick comments:
In `@pkg/asset/installconfig/azure/client.go`:
- Around line 493-497: In GetRegionAvailabilityZones, the ProvidersClient is
being constructed with network-specific options
(c.ssn.ClientConfig.ClientOptions(ServiceNetwork)); replace that call with the
generic default options (c.ssn.ClientConfig.DefaultClientOptions()) so the
providers ARM client uses non-network-specific API/version settings (update the
armresources.NewProvidersClient invocation accordingly).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7e6e3bda-7f7d-43da-bf48-126e4960ed57
⛔ Files ignored due to path filters (130)
vendor/github.com/Azure/azure-sdk-for-go/profiles/2018-03-01/resources/mgmt/resources/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/profiles/2020-09-01/network/mgmt/network/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/profiles/2020-09-01/storage/mgmt/storage/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/_meta.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/applicationgateways.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/applicationsecuritygroups.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/availabledelegations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/availableendpointservices.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/availableresourcegroupdelegations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/azurefirewallfqdntags.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/azurefirewalls.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/bgpservicecommunities.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/connectionmonitors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/ddoscustompolicies.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/ddosprotectionplans.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/defaultsecurityrules.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/enums.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/expressroutecircuitauthorizations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/expressroutecircuitconnections.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/expressroutecircuitpeerings.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/expressroutecircuits.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/expressrouteconnections.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/expressroutecrossconnectionpeerings.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/expressroutecrossconnections.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/expressroutegateways.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/expressroutelinks.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/expressrouteports.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/expressrouteportslocations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/expressrouteserviceproviders.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/hubvirtualnetworkconnections.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/inboundnatrules.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/interfaceendpoints.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/interfaceipconfigurations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/interfaceloadbalancers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/interfacesgroup.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/interfacetapconfigurations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/loadbalancerbackendaddresspools.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/loadbalancerfrontendipconfigurations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/loadbalancerloadbalancingrules.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/loadbalancernetworkinterfaces.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/loadbalanceroutboundrules.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/loadbalancerprobes.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/loadbalancers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/localnetworkgateways.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/operations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/p2svpngateways.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/p2svpnserverconfigurations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/packetcaptures.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/profiles.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/publicipaddresses.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/publicipprefixes.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/routefilterrules.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/routefilters.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/routes.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/routetables.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/securitygroups.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/securityrules.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/serviceendpointpolicies.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/serviceendpointpolicydefinitions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/subnets.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/usages.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/virtualhubs.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/virtualnetworkgatewayconnections.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/virtualnetworkgateways.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/virtualnetworkpeerings.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/virtualnetworks.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/virtualnetworktaps.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/virtualwans.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/vpnconnections.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/vpngateways.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/vpnsites.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/vpnsitesconfiguration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-11-01/network/watchers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/preview/dns/mgmt/2018-03-01-preview/dns/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/preview/dns/mgmt/2018-03-01-preview/dns/_meta.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/preview/dns/mgmt/2018-03-01-preview/dns/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/preview/dns/mgmt/2018-03-01-preview/dns/enums.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/preview/dns/mgmt/2018-03-01-preview/dns/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/preview/dns/mgmt/2018-03-01-preview/dns/recordsets.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/preview/dns/mgmt/2018-03-01-preview/dns/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/preview/dns/mgmt/2018-03-01-preview/dns/zones.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/privatedns/mgmt/2018-09-01/privatedns/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/privatedns/mgmt/2018-09-01/privatedns/_meta.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/privatedns/mgmt/2018-09-01/privatedns/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/privatedns/mgmt/2018-09-01/privatedns/enums.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/privatedns/mgmt/2018-09-01/privatedns/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/privatedns/mgmt/2018-09-01/privatedns/privatezones.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/privatedns/mgmt/2018-09-01/privatedns/recordsets.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/privatedns/mgmt/2018-09-01/privatedns/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/privatedns/mgmt/2018-09-01/privatedns/virtualnetworklinks.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources/_meta.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources/deploymentoperations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources/deployments.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources/enums.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources/groups.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources/providers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources/resources.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources/tags.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources/_meta.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources/deploymentoperations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources/deployments.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources/enums.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources/groups.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources/operations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources/providers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources/resources.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources/tags.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2017-10-01/storage/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2017-10-01/storage/_meta.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2017-10-01/storage/accounts.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2017-10-01/storage/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2017-10-01/storage/enums.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2017-10-01/storage/models.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2017-10-01/storage/operations.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2017-10-01/storage/skus.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2017-10-01/storage/usage.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2017-10-01/storage/version.gois excluded by!vendor/**,!**/vendor/**vendor/modules.txtis excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (7)
pkg/asset/installconfig/azure/client.gopkg/asset/installconfig/azure/config.gopkg/asset/installconfig/azure/session.gopkg/destroy/azure/azure.gopkg/gather/azure/azure.gopkg/infrastructure/azure/azure.gopkg/infrastructure/azure/dns.go
jhixson74
force-pushed
the
main-migrate-legacy-azure-sdk-to-v2
branch
from
9099145 to
2717cd6
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/asset/installconfig/azure/validation.go (1)
556-574:⚠️ Potential issue | 🟠 MajorValidate every returned subnet prefix.
When Azure returns multiple subnet prefixes, this only inspects
AddressPrefixes[0]. That can reject valid BYO subnets based on ordering, or pass a subnet even though another prefix sits outsidemachineNetwork. It also dereferences the first element without a nil check.Suggested fix
- var addressPrefix string - switch { - case subnet.Properties.AddressPrefix != nil: - addressPrefix = *subnet.Properties.AddressPrefix - // NOTE: if the subscription has the `AllowMultipleAddressPrefixesOnSubnet` feature, the Azure API will return a - // `addressPrefixes` field with a slice of addresses instead of a single value via `addressPrefix`. - case len(subnet.Properties.AddressPrefixes) > 0: - addressPrefix = *subnet.Properties.AddressPrefixes[0] - default: - return append(allErrs, field.Invalid(fieldPath, subnetName, "subnet does not have an address prefix")) - } - - subnetIP, _, err := net.ParseCIDR(addressPrefix) - if err != nil { - return append(allErrs, field.Invalid(fieldPath, subnetName, "unable to parse subnet CIDR")) - } - - allErrs = append(allErrs, validateMachineNetworksContainIP(fieldPath, networks, *subnet.Name, subnetIP)...) + prefixes := make([]string, 0, 1+len(subnet.Properties.AddressPrefixes)) + if subnet.Properties.AddressPrefix != nil { + prefixes = append(prefixes, *subnet.Properties.AddressPrefix) + } + for _, prefix := range subnet.Properties.AddressPrefixes { + if prefix != nil { + prefixes = append(prefixes, *prefix) + } + } + if len(prefixes) == 0 { + return append(allErrs, field.Invalid(fieldPath, subnetName, "subnet does not have an address prefix")) + } + + for _, prefix := range prefixes { + subnetIP, _, err := net.ParseCIDR(prefix) + if err != nil { + return append(allErrs, field.Invalid(fieldPath, subnetName, "unable to parse subnet CIDR")) + } + allErrs = append(allErrs, validateMachineNetworksContainIP(fieldPath, networks, *subnet.Name, subnetIP)...) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/asset/installconfig/azure/validation.go` around lines 556 - 574, The code currently only checks the first element of subnet.Properties.AddressPrefixes (and dereferences it without a nil check), which can miss or mis-validate BYO subnets; update the logic in the block around subnet.Properties.AddressPrefix / subnet.Properties.AddressPrefixes to: if AddressPrefix is non-nil validate it as before; if AddressPrefixes is non-nil and len>0 iterate over every non-nil element, for each safely read the string, call net.ParseCIDR (handle parse errors by appending field.Invalid) and call validateMachineNetworksContainIP (using a per-iteration subnetIP variable) collecting all errors; ensure you do not assume AddressPrefixes[0] exists or non-nil and return aggregated allErrs at the end.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/asset/installconfig/azure/client.go`:
- Around line 437-450: GetLocationInfo currently iterates the ResourceSKUs pager
without a deadline; wrap the incoming ctx with a short timeout (e.g.,
context.WithTimeout(ctx, 30*time.Second)) at the start of GetLocationInfo, defer
the cancel, and use the wrapped ctx for pager.NextPage calls so the SKU lookup
cannot hang indefinitely; mirror the approach used in
GetDiskSkus/GetVirtualMachineSku and add an import for time if needed.
- Around line 494-495: The code creates an armresources.ProvidersClient with
ClientOptions(ServiceNetwork), which incorrectly applies the network API-version
override; replace that construction with a call to c.getProvidersClient() so
ProvidersClient is created using DefaultClientOptions() instead; update the
variable used (providersClient) and ensure error handling remains the same so
GetRegionAvailabilityZones and other Microsoft.Resources calls use the correct
client.
In `@pkg/destroy/azure/azure.go`:
- Around line 593-615: The variable allPrivateRecords is declared but never
initialized before use, which will cause a panic when calling
allPrivateRecords.Insert in the loop; initialize it (e.g., with
sets.New[string]() or the project's set constructor) before using it in the
function that iterates over privateRecordsClient.NewListPager(zoneGroup,
zoneName, ...), so that calls to allPrivateRecords.Insert(fmt.Sprintf("%s.%s",
recordName, zoneName)) are safe; ensure the initialization happens prior to the
for recordPager.More() loop (referencing allPrivateRecords,
privateRecordsClient.NewListPager, toRecordType, and the armdns.RecordTypeSOA/NS
checks).
- Around line 562-584: allPrivateRecords is declared as a sets.Set[string] but
never initialized, causing a nil-map panic when calling Insert; initialize it
before use (e.g., right before calling recordsClient.NewListByDNSZonePager or
immediately after declaration) by creating a new set (use sets.New[string]() or
the project's canonical constructor) so subsequent calls to
allPrivateRecords.Insert(...) succeed; ensure the initialization happens before
the loop that calls recordPager.NextPage and before any Insert calls.
---
Outside diff comments:
In `@pkg/asset/installconfig/azure/validation.go`:
- Around line 556-574: The code currently only checks the first element of
subnet.Properties.AddressPrefixes (and dereferences it without a nil check),
which can miss or mis-validate BYO subnets; update the logic in the block around
subnet.Properties.AddressPrefix / subnet.Properties.AddressPrefixes to: if
AddressPrefix is non-nil validate it as before; if AddressPrefixes is non-nil
and len>0 iterate over every non-nil element, for each safely read the string,
call net.ParseCIDR (handle parse errors by appending field.Invalid) and call
validateMachineNetworksContainIP (using a per-iteration subnetIP variable)
collecting all errors; ensure you do not assume AddressPrefixes[0] exists or
non-nil and return aggregated allErrs at the end.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 750c5d66-906f-4425-a0bc-b2e198b07d3b
📒 Files selected for processing (4)
pkg/asset/installconfig/azure/client.gopkg/asset/installconfig/azure/validation.gopkg/destroy/azure/azure.gopkg/infrastructure/azure/storage.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/infrastructure/azure/storage.go
Remove %w error wrapping in cases where err is guaranteed to be nil. When API calls succeed but return missing properties, the error paths that could set err have already returned, leaving err as nil. Wrapping nil produces confusing messages like "failed to get subnet: %!w()". Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
jhixson74
force-pushed
the
main-migrate-legacy-azure-sdk-to-v2
branch
from
6296a36 to
d9607c1
Compare
Member
Author
|
/test e2e-azurestack |
jhixson74
force-pushed
the
main-migrate-legacy-azure-sdk-to-v2
branch
from
2f64edd to
0400a40
Compare
Member
Author
|
/test e2e-azurestack |
3 similar comments
Member
Author
|
/test e2e-azurestack |
Member
Author
|
/test e2e-azurestack |
Member
Author
|
/test e2e-azurestack |
Member
Author
|
/test e2e-azure-ovn |
Contributor
|
@jhixson74: This pull request references CORS-4382 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the spike to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Contributor
|
@jhixson74: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR migrates all legacy Azure SDK calls to v2 SDK calls. This was done using Claude.