Skip to content

fix(cli): resolve API versions consistently for generic resource operations - #13232

Open
lakshmimsft wants to merge 1 commit into
mainfrom
fix/resource-create-api-version
Open

lakshmimsft wants to merge 1 commit into
mainfrom
fix/resource-create-api-version

Conversation

@lakshmimsft

Copy link
Copy Markdown
Contributor

Summary

rad resource create sent api-version=2023-10-01-preview on every request, regardless of the resource type being created, and regardless of any API version that type declared. This PR makes it resolve the API version from the resource provider like the other generic resource operations already do, and makes that resolution deterministic for all of them.

Three related changes:

  1. CreateOrUpdateResource now resolves its API version. It was the only generic CRUD operation that did not. It called a separate helper, createGenericClient, whose version argument was variadic — and the sole caller passed nothing, so clientOptions.APIVersion was never set and the generated client's hardcoded 2023-10-01-preview applied. The resource provider's advertised API versions were not overridden on this path; they were never looked up at all, because getApiVersionsForResourceType was never called from it. Declaring an API version on a resource type therefore had no effect on rad resource create. createGenericClient also lacked the isRadiusCoreType pin that getGenericClient applies, so even Radius.Core types were created as 2023-10-01-preview. createGenericClient is dead once that call site moves to getGenericClient, so it is deleted.

This went unnoticed because Applications.Core genuinely is 2023-10-01-preview, so the hardcoded default was accidentally correct for the only types being created today. Every Radius.* type is served at 2025-08-01-preview only, so creates against them were addressed with a version their provider does not serve. Fixing this is a prerequisite for #11803.

  1. API version selection is now deterministic. The resolver returned []string built from maps.Keys(...), and both consumers used only apiVersions[0]. Because Go randomizes map iteration order, a resource type advertising more than one version could get a different API version on each invocation. The resolver now returns a single string: it prefers the resource provider's declared defaultApiVersion, and otherwise sorts and takes the lowest. The signature change ([]string to string) reflects what the function always did — a single HTTP request carries a single api-version, so the caller could never use more than one element.

  2. Behavior when a type advertises no API versions is unchanged. The resolver returns an empty version, which leaves the generated client on its built-in 2023-10-01-preview default, exactly as before. This case is deliberately left alone rather than turned into an error — see "Reason for change" below.

This is a no-op for every resource type that exists today: no manifest in deploy/manifest/ sets defaultApiVersion, and every built-in type advertises exactly one API version (2025-08-01-preview), so the default-preference branch never fires and sorting a one-element list returns that element. The change can only select differently than before for a user-defined type that declares a default or advertises two or more versions.

Reason for change

Groundwork for #11803, which makes the new Radius.* resource types the default for the imperative CLI commands. Those commands cannot become the default while rad resource create addresses every type with the Applications.Core API version. No separate issue was filed for this bug; it surfaced while preparing that work.

How to test

Automated coverage is included. Test_CreateOrUpdateResource_APIVersion asserts the actual api-version query parameter on the outgoing PUT using a real transport, rather than a mock client factory — the factory short-circuits getGenericClient before client options are applied, so a factory-based test cannot observe the wire format. The four cases cover a provider-advertised version, a non-Radius.Core type resolving dynamically, the Radius.Core pin, and a lookup failure asserting that no PUT is sent.

Each case was confirmed to fail with the production fix reverted, emitting api-version=2023-10-01-preview.

Commands run, with results:

Command Result
go build ./... passed
go vet ./... passed
go test ./pkg/cli/... -count=1 passed, 105 packages, 0 failures
go test ./pkg/cli/clients/ -race -count=1 passed
gofmt -l on changed files passed, no output

Functional and end-to-end suites under test/ compile (covered by go vet ./...) but were not executed, as they require a live cluster. They exercise only built-in resource types, for which this change is provably a no-op per the Summary.

To verify manually against a cluster, create a resource of a non-Applications.Core type and confirm the request carries the provider's API version rather than 2023-10-01-preview:

rad resource create 'Radius.Compute/containers' mycontainer -f /path/to/input.json

File change summary

File Summary of change
pkg/cli/clients/management.go CreateOrUpdateResource resolves the API version before building its client. Deleted the now-unused createGenericClient. Renamed getApiVersionsForResourceType to getAPIVersionForResourceType, returning a single version that prefers defaultApiVersion and otherwise sorts for stability. Changed getGenericClient to take a single apiVersion string and updated all six call sites. The Radius.Core pin is retained.
pkg/cli/clients/management_test.go Added Test_CreateOrUpdateResource_APIVersion, a four-case table test asserting the outgoing api-version query parameter via a real transport. Updated the existing CreateOrUpdateResource subtest to wire resourceProviderClientFactory, which the new lookup requires.
pkg/cli/cmd/resource/create/create.go Added a Radius.Compute/containers example to the command help, showing that any registered resource type is supported.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@lakshmimsft
lakshmimsft requested a lite review from Copilot October 7, 2026 21:34
@lakshmimsft lakshmimsft added the pr:standard Ongoing maintenance, minor improvements, documentation updates, and routine development work label Oct 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Empty-version handling and coverage for default and multi-version selection branches need correction.

1 open finding
What changed in this PR

Fixes generic resource creation to resolve provider API versions consistently and deterministically.

Changes:

  • Resolves API versions during resource creation.
  • Prefers provider defaults and stable fallback versions.
  • Adds wire-level tests and a CLI example.
File Summary
pkg/​cli/​cmd/​resource/​create/​create.go Documents creating arbitrary registered resource types.
pkg/​cli/​clients/​management.go Updates API-version resolution and generic client construction.
pkg/​cli/​clients/​management_test.go Adds API-version request coverage and updates test setup.

🧠 Review effort: Lite


💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread pkg/cli/clients/management.go
@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Unit Tests

    2 files  ±0    460 suites  ±0   15m 34s ⏱️ -1s
7 325 tests +4  7 323 ✅ +4  2 💤 ±0  0 ❌ ±0 
8 808 runs  +4  8 806 ✅ +4  2 💤 ±0  0 ❌ ±0 

Results for commit 4e632b4. ± Comparison against base commit ebe182b.

This pull request removes 4 and adds 8 tests. Note that renamed tests count towards both.
github.com/radius-project/radius/pkg/cli/clients ‑ Test_CreateOrUpdateResource_RadiusCoreAPIVersion
github.com/radius-project/radius/pkg/cli/clients ‑ Test_CreateOrUpdateResource_RadiusCoreAPIVersion/applications_core_keeps_the_default
github.com/radius-project/radius/pkg/cli/clients ‑ Test_CreateOrUpdateResource_RadiusCoreAPIVersion/radius_core_is_matched_case-insensitively
github.com/radius-project/radius/pkg/cli/clients ‑ Test_CreateOrUpdateResource_RadiusCoreAPIVersion/radius_core_pins_the_supported_version
github.com/radius-project/radius/pkg/cli/clients ‑ Test_CreateOrUpdateResource_APIVersion
github.com/radius-project/radius/pkg/cli/clients ‑ Test_CreateOrUpdateResource_APIVersion/does_not_send_a_request_when_the_API_version_lookup_fails
github.com/radius-project/radius/pkg/cli/clients ‑ Test_CreateOrUpdateResource_APIVersion/falls_back_to_the_lowest_advertised_version_when_no_default_is_declared
github.com/radius-project/radius/pkg/cli/clients ‑ Test_CreateOrUpdateResource_APIVersion/pins_Radius.Core_resources_matched_case-insensitively
github.com/radius-project/radius/pkg/cli/clients ‑ Test_CreateOrUpdateResource_APIVersion/pins_Radius.Core_resources_to_their_required_API_version
github.com/radius-project/radius/pkg/cli/clients ‑ Test_CreateOrUpdateResource_APIVersion/prefers_the_declared_default_API_version_over_the_advertised_versions
github.com/radius-project/radius/pkg/cli/clients ‑ Test_CreateOrUpdateResource_APIVersion/resolves_non-Core_Radius_types_from_the_resource_provider
github.com/radius-project/radius/pkg/cli/clients ‑ Test_CreateOrUpdateResource_APIVersion/uses_the_API_version_advertised_by_the_resource_provider

♻️ This comment has been updated with latest results.

@codecov

codecov Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 60.66%. Comparing base (ebe182b) to head (4e632b4).

Files with missing lines Patch % Lines
pkg/cli/clients/management.go 96.29% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #13232   +/-   ##
=======================================
  Coverage   60.66%   60.66%           
=======================================
  Files         777      777           
  Lines       45875    45878    +3     
=======================================
+ Hits        27829    27832    +3     
  Misses      18046    18046           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lakshmimsft
lakshmimsft force-pushed the fix/resource-create-api-version branch 3 times, most recently from f0228f9 to d13e10a Compare October 8, 2026 17:36
@lakshmimsft
lakshmimsft marked this pull request as ready for review October 8, 2026 17:36
@lakshmimsft
lakshmimsft requested review from a team as code owners October 8, 2026 17:36
@github-actions github-actions Bot added the pr:waiting-for-review A reviewer owns the next action; required approval is pending or unverifiable label Oct 8, 2026
brooke-hamilton
brooke-hamilton previously approved these changes Oct 9, 2026
@github-actions github-actions Bot added pr:ready-for-queue Review, checks and mergeability permit adding this PR to the queue pr:review-approved Required reviews are approved; checks may still be pending pr:needs-rebase The pull request has merge conflicts and removed pr:waiting-for-review A reviewer owns the next action; required approval is pending or unverifiable pr:ready-for-queue Review, checks and mergeability permit adding this PR to the queue labels Oct 9, 2026
…ations

Signed-off-by: lakshmimsft <ljavadekar@microsoft.com>
@lakshmimsft
lakshmimsft force-pushed the fix/resource-create-api-version branch from d13e10a to 4e632b4 Compare October 9, 2026 23:28
@github-actions github-actions Bot added pr:needs-reviewer No pending review request or active submitted human review and removed pr:review-approved Required reviews are approved; checks may still be pending pr:needs-rebase The pull request has merge conflicts labels Oct 9, 2026
@radius-functional-tests

radius-functional-tests Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Radius functional test overview

🔍 Go to test action run

Click here to see the test run details
Name Value
Repository radius-project/radius
Commit ref 4e632b4
Unique ID func8a5dc5e288
Image tag pr-func8a5dc5e288
  • Dapr: 1.14.4
  • Azure KeyVault CSI driver: 1.4.2
  • Azure Workload identity webhook: 1.3.0
  • Bicep recipe location ghcr.io/radius-project/dev/test/testrecipes/test-bicep-recipes/<name>:pr-func8a5dc5e288
  • Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
  • applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-func8a5dc5e288
  • dynamic-rp test image location: ghcr.io/radius-project/dev/dynamic-rp:pr-func8a5dc5e288
  • controller test image location: ghcr.io/radius-project/dev/controller:pr-func8a5dc5e288
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-func8a5dc5e288
  • deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting corerp-cloud functional tests...
⌛ Starting ucp-cloud functional tests...
✅ ucp-cloud functional tests succeeded
✅ corerp-cloud functional tests succeeded

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:needs-reviewer No pending review request or active submitted human review pr:standard Ongoing maintenance, minor improvements, documentation updates, and routine development work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants