Declare property defaults so unset optional properties have a value - #260
Open
AzureMike wants to merge 1 commit into
Open
Declare property defaults so unset optional properties have a value#260AzureMike wants to merge 1 commit into
AzureMike wants to merge 1 commit into
Conversation
Several resource types documented a default in prose without declaring it, and the ones that did declare a default had no effect because Radius never materialized them. Declare the missing keys so they take effect alongside radius-project/radius schema defaulting. Added: size on redisCaches and postgreSqlDatabases, version and database on mySqlDatabases, and database on postgreSqlDatabases. neo4jDatabases is left alone because its documented default is the resource name, which a static default cannot express. The mySqlDatabases version default is 8.4, which is what the AWS Terraform and Kubernetes Bicep recipes already fall back to when version is unset. The Azure pack maps everything except 5.7 to 8.0.21, so it deploys the same image either way and only AWS and Kubernetes would notice a different value. Declaring 8.0 here would have silently downgraded them, and a running RDS instance rejects a major version downgrade outright. Related to radius-project/radius#12532 Signed-off-by: Mike Azure <127820851+AzureMike@users.noreply.github.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
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
1 participant
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.
Summary
Several resource types promise a default in prose without declaring one, and
sizeis the case in radius-project/radius#12532 — "Defaults toSif not provided" is English in adescription:field, and nothing reads it. This declares the real thing:sizeonredisCachesandpostgreSqlDatabases,versionanddatabaseonmySqlDatabases, anddatabaseonpostgreSqlDatabases.neo4jDatabasesis left alone, since its documented default is the resource name and a staticdefault:can't express that.Reason for change
Deploying one of these resources without setting the optional property puts a literal
{{context.resource.properties.size}}expression into the ARM request, because there's nothing for the recipe to substitute. Declaring the default is half the fix. The other half is radius-project/radius#12563, which makes a declareddefault:actually get applied — until that ships, nothing here changes behavior, which is why the two are separate and this one goes first.mySqlDatabasesversiondefaults to8.4, which is what the AWS Terraform recipe (Data/mySqlDatabases/recipes/aws/terraform/main.tf:31) and the Kubernetes Bicep recipe already fall back to whenversionis unset. Declaring8.0instead would have downgraded both, and a running RDS instance rejects a major version downgrade outright.Related to radius-project/radius#12532
How to test
Each declared default matches what the recipes already do when the property is unset, so the intended result is no change for anyone who was relying on the existing fallback:
redisCachessizeSsize == "S"branch of the Azure pack, which the description already documentedpostgreSqlDatabasessizeSpostgreSqlDatabasesdatabasepostgres_dbmySqlDatabasesdatabasemysql_dbmySqlDatabasesversion8.4try(var.version, "8.4")in the AWS recipe and?? '8.4'in the Kubernetes recipeWith radius-project/radius#12563 applied, an unset
sizeresolves toBalanced_B0on Redis andStandard_B1ms/Burstableon PostgreSQL, and an unsetdatabaseresolves topostgres_dbandmysql_db.File change summary
Data/redisCaches/redisCaches.yamldefault: 'S'onsize.Data/postgreSqlDatabases/postgreSqlDatabases.yamldefault: 'S'onsizeanddefault: postgres_dbondatabase.Data/mySqlDatabases/mySqlDatabases.yamldefault: mysql_dbondatabaseanddefault: '8.4'onversion, and reword theversiondescription from "Assumed to be 8.4" to match the other optional properties.