Skip to content

Materialize schema property defaults before the resource is saved - #12563

Open
AzureMike wants to merge 1 commit into
mainfrom
feat/materialize-schema-defaults
Open

Materialize schema property defaults before the resource is saved#12563
AzureMike wants to merge 1 commit into
mainfrom
feat/materialize-schema-defaults

Conversation

@AzureMike

Copy link
Copy Markdown
Contributor

Summary

A resource type schema can declare a default for a property, and Radius never applied it. schema.ApplyDefaults fills absent properties from the default declared in the schema, and a new UpdateFilter on the dynamic resource PUT path runs it before the resource is saved.

Explicit values are never overwritten, and required properties are never defaulted, so leaving one out still fails validation instead of being quietly satisfied.

Reason for change

Fixes #12532

Deploying a resource without setting an optional property puts a literal template expression into the ARM request. The Azure recipe pack writes parameters as expressions over the resource:

skuName: '{{context.resource.properties.size == "S" ? "Balanced_B0" : "Balanced_B1"}}'

Leave size unset and there's nothing to substitute, so the raw {{...}} string is handed to ARM as the SKU name and rejected against allowedValues.

The cause sits a level below the recipe. Validation checked a resource against its schema without filling anything in, so every default: key in every resource type has been inert — including the ones objectStorage, mongoDatabases, models, kafka and rabbitMQ already declare.

With the defaults from radius-project/resource-types-contrib#260 declared, the existing recipe expressions resolve on their own, and neither the resolver nor the recipe pack needs to change. That covers ten optional properties across the Azure pack, not just size:

Type Property Unset today With defaults
redisCaches size leaks Balanced_B0
postgreSqlDatabases size leaks Standard_B1ms / Burstable
mySqlDatabases version leaks 8.0.21
models model leaks gpt-5-mini
mongoDatabases database leaks mongo_db
mySqlDatabases database leaks mysql_db
postgreSqlDatabases database leaks postgres_db
rabbitMQ queue leaks jobs
kafka topic leaks events
objectStorage containerName leaks data

Six of those defaults already existed, so half the fix is Radius finally reading what contrib already wrote down.

Two things reviewers should know. The filter sits ahead of the encryption filter, so a defaulted sensitive field is still encrypted. And defaults are reapplied on every write rather than carried forward from the previous version of the resource, which is what full-replace PUT semantics ask for — the consequence is that changing a declared default later changes what an existing resource gets on its next deployment, so a default is part of a resource type's API contract.

How to test

go test ./pkg/schema/... ./pkg/dynamicrp/... covers the new code directly. pkg/schema/defaults_test.go asserts that explicit values survive, required properties are never defaulted, read-only properties are skipped, absent nested objects aren't created, and map and slice defaults are deep copied so one resource can't mutate what the next one sees.

End to end, all ten optional properties above were run through ApplyDefaults and the parameter resolver against the real contrib schemas and the real recipepack/azure/aks-recipepack.bicep expressions. Every one emits a literal {{...}} without defaults and resolves to the value in the table with them, on the unmodified resolver and the unmodified recipe expressions.

go vet and gofmt are clean, and pkg/schema/..., pkg/dynamicrp/..., pkg/recipes/..., pkg/cli/manifest/... and pkg/ucp/integrationtests/resourceproviders/... all pass, including after rebasing onto the kin-openapi 0.144.0 bump in #12561.

Built-in resource types pick this up once radius-project/resource-types-contrib#260 merges and make update-resource-types bumps the pin in go.mod, since the manifests under deploy/manifest/built-in-providers are copies CI regenerates from it.

File change summary

File Summary of change
pkg/schema/defaults.go New. ApplyDefaults walks a schema and fills absent properties from their declared default, returning the number applied.
pkg/schema/defaults_test.go New. Covers explicit values, required properties, read-only properties, nested objects, and deep copying of map and slice defaults.
pkg/dynamicrp/frontend/defaultsfilter.go New. UpdateFilter that fetches the resource type schema and applies defaults before the resource is saved, attaching the filled map only when something was applied.
pkg/dynamicrp/frontend/routes.go Register the defaults filter ahead of the encryption filter.

@github-actions

Copy link
Copy Markdown

Dependency Review

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

Scanned Files

None

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown

Unit Tests

    2 files  ± 0    456 suites  ±0   6m 27s ⏱️ ±0s
6 123 tests +19  6 121 ✅ +19  2 💤 ±0  0 ❌ ±0 
7 353 runs  +19  7 351 ✅ +19  2 💤 ±0  0 ❌ ±0 

Results for commit 5c69ad6. ± Comparison against base commit 3281530.

♻️ This comment has been updated with latest results.

@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 54.02299% with 40 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.99%. Comparing base (f7ee28c) to head (5c69ad6).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pkg/dynamicrp/frontend/defaultsfilter.go 0.00% 36 Missing ⚠️
pkg/dynamicrp/frontend/routes.go 0.00% 2 Missing ⚠️
pkg/schema/defaults.go 95.91% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #12563    +/-   ##
========================================
  Coverage   53.99%   53.99%            
========================================
  Files         765      767     +2     
  Lines       50708    50813   +105     
========================================
+ Hits        27379    27439    +60     
- Misses      20758    20794    +36     
- Partials     2571     2580     +9     

☔ 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.

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown

Functional Tests - upgrade-noncloud

3 tests  ±0   3 ✅ ±0   3m 34s ⏱️ -28s
1 suites ±0   0 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 5c69ad6. ± Comparison against base commit 3281530.

♻️ This comment has been updated with latest results.

Resource type schemas could declare a default for a property, but nothing ever
applied it. ValidateResourceAgainstSchema only validates, so declared defaults
in objectStorage, mongoDatabases, kafka, rabbitMQ and models had no effect, and
recipes reading an unset optional property through context.resource.properties.*
found nothing to resolve.

Add schema.ApplyDefaults, which fills absent properties from the default
declared in the schema, and run it as an UpdateFilter on the dynamic resource
PUT path ahead of the encryption filter so a defaulted sensitive field is still
encrypted. Explicit values are never overwritten. Read-only properties are
skipped because they are recipe outputs, and an absent object is not created
just to hold defaults.

Required properties are never defaulted, so omitting one still fails validation
rather than being quietly satisfied. The rule blocks defaulting only, not
recursion: a required object the author did supply is still descended into,
because its own optional properties may declare defaults of their own.

A body carrying no properties at all is defaulted too. The filled map is
attached only when something was actually applied, so a genuinely empty resource
serializes exactly as it did before.

Defaults are reapplied on every write rather than carried forward from the
previous version of the resource, which is what full-replace PUT semantics ask
for. The consequence is that changing a declared default changes what an
existing resource gets on its next deployment, so a default is part of a
resource type's API contract and should change only alongside the API version.

The manifests under deploy/manifest/built-in-providers are verbatim copies of
the resource-types-contrib version pinned in go.mod, and CI re-runs the copy and
fails on any diff. Built-in types therefore pick up the new defaults only after
the contrib change merges and go.mod is bumped. Until then this change is inert
for them rather than broken.

Known follow-up: this filter and the encryption filter each fetch the same
schema from UCP, so a PUT now makes two identical round trips. Sharing one fetch
needs either a request-scoped cache seeded in the shared controller or the two
filters merged, both of which reach past this change.

With defaults declared, this resolves #12532 on its own:
an unset size resolves to Burstable/Standard_B1ms/Balanced_B0, and plain
non-ternary paths such as database stop leaking too.

Signed-off-by: Mike Azure <127820851+AzureMike@users.noreply.github.com>
@AzureMike
AzureMike force-pushed the feat/materialize-schema-defaults branch from ec7598b to 5c69ad6 Compare July 28, 2026 17:39
@radius-functional-tests

radius-functional-tests Bot commented Jul 28, 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 5c69ad6
Unique ID funce68cb422cd
Image tag pr-funce68cb422cd
  • 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-funce68cb422cd
  • 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-funce68cb422cd
  • dynamic-rp test image location: ghcr.io/radius-project/dev/dynamic-rp:pr-funce68cb422cd
  • controller test image location: ghcr.io/radius-project/dev/controller:pr-funce68cb422cd
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-funce68cb422cd
  • 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 ucp-cloud functional tests...
⌛ Starting corerp-cloud functional tests...
✅ ucp-cloud functional tests succeeded
✅ corerp-cloud functional tests succeeded

@AzureMike
AzureMike requested a review from kachawla July 29, 2026 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Azure recipe size ternary leaks literally to ARM when properties.size is unset (postgres + redis)

2 participants