Add conditional mode to ReferenceExpression with polyglot codegen support#14919
Add conditional mode to ReferenceExpression with polyglot codegen support#14919danegsta merged 48 commits intorelease/13.2from
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14919Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14919" |
There was a problem hiding this comment.
Pull request overview
Adds a new lazy/deferred value provider to support reference-expression fragments that depend on state only known later in the lifecycle (notably Redis TLS), and updates Redis endpoint/URI scheme handling to be scheme-driven rather than hard-coded.
Changes:
- Introduces
DeferredValueProvider(runtime + manifest expression callbacks) and anEndpointReference.TlsValue(...)helper. - Adds
EndpointAnnotation.TlsEnabled/EndpointReference.TlsEnabledand wires Redis TLS to endpoint annotations (includingssl=trueconnection string fragment). - Updates Redis tests/manifests to use binding
{...scheme}and default Redis endpoint scheme toredis(andredisswhen TLS is enabled).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.Tests/DeferredValueProviderTests.cs | New unit tests covering deferred provider behavior and interaction with ReferenceExpressionBuilder. |
| tests/Aspire.Hosting.Redis.Tests/ConnectionPropertiesTests.cs | Updates expected Redis URI manifest expression to use {...scheme} binding. |
| tests/Aspire.Hosting.Redis.Tests/AddRedisTests.cs | Updates Redis endpoint scheme expectations and adds TLS/dynamic-resolution coverage. |
| src/Aspire.Hosting/ApplicationModel/EndpointReference.cs | Adds TlsEnabled and TlsValue(...) to support TLS-dependent dynamic fragments. |
| src/Aspire.Hosting/ApplicationModel/EndpointAnnotation.cs | Adds TlsEnabled flag for endpoint-level TLS state. |
| src/Aspire.Hosting/ApplicationModel/DeferredValueProvider.cs | Adds new general-purpose deferred value provider implementation. |
| src/Aspire.Hosting.Redis/RedisResource.cs | Switches TLS handling to endpoint-based TLS state and scheme-driven URI/connection string fragments. |
| src/Aspire.Hosting.Redis/RedisBuilderExtensions.cs | Sets Redis primary endpoint scheme to redis and updates TLS enablement to mutate endpoint annotation state. |
You can also share your feedback on Copilot code review. Take the survey.
🎬 CLI E2E Test RecordingsThe following terminal recordings are available for commit
📹 Recordings uploaded automatically from CI run #22884320770 |
9404e2f to
a5aa70a
Compare
|
How does this affect the typescript support? |
Replace DeferredValueProvider with ConditionalReferenceExpression, a new type that models conditional values in connection strings (e.g., TLS ssl=true/empty). The CRE auto-generates its manifest name from the condition's ValueExpression at construction time. Key changes: - ConditionalReferenceExpression type with Create() factory, auto-name generation via condition sanitization, and manifest value.v0 support - EndpointReference.GetTlsValue returns CRE instead of using closure - ManifestPublishingContext writes CRE entries as value.v0 resources - Polyglot create()/toJSON() in all 5 ATS base files (TS, Go, Python, Java, Rust) with $condExpr JSON serialization format - ConditionalReferenceExpressionRef for server-side $condExpr unmarshalling in AtsMarshaller - Redis connection string tests updated with pattern matching to handle auto-generated CRE names and verify manifest value entries Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The MarshalToJson_ConditionalReferenceExpression_PreservesValueAfterRoundTrip
test was asserting an exact name ('test-tls') but names are now auto-generated
from the condition's ValueExpression. Use StartsWith assertion instead.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace wrapIfHandle (a deserialization helper) with proper serialization logic in ReferenceExpression.toJSON() for the condition field. Remove the now-unnecessary import. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| } | ||
| } | ||
|
|
||
| impl<'de> Deserialize<'de> for ReferenceExpression { |
There was a problem hiding this comment.
@sebastienros this is why we should delete all other language 😄
There was a problem hiding this comment.
I've had to walk back several overzealous copilot changes such as this. It seems that it gets a bit carried away once it starts trying to update all the languages.
|
docker and kubernetes both have special ReferenceExpression logic as well. |
The EndToEnd tests are incompatible with TLS-enabled Redis. Opt out of the developer certificate for the Redis resource in TestProgram. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…mpose, and Kubernetes publish contexts - Azure App Service: Handle conditional expressions with Bicep ternary fallback for parameter-based conditions - Docker Compose: Convert ProcessValue to async, resolve conditions statically at generation time via GetValueAsync - Kubernetes: Resolve conditions via GetValueAsync with ValueProviderContext at generation time - Add tests with both static and parameter-based conditions for all three contexts with Verify snapshots Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sher
When a conditional ReferenceExpression's condition is a ParameterResource,
emit a Helm ternary expression (e.g., {{ ternary "val1" "val2"
(eq .Values.parameters.x "True") | quote }}) instead of resolving
statically. This defers evaluation to helm install/upgrade time, allowing
users to override the condition parameter in values.yaml.
- Add BuildHelmTernary helper in KubernetesResource
- Add HelmFlowControlPattern regex for ternary expression detection
- Update ShouldDoubleQuoteString to render ternary as plain YAML
- Non-parameter conditions still resolve statically at generation time
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The is HelmValue type check failed because ProcessValueAsync's Format=={0}
optimization converts HelmValues to strings via ToString(). Changed to detect
Helm expressions in string content using ContainsHelmExpression() instead.
Added HelmExtensionsTests with Theory tests for ShouldDoubleQuoteString and
HelmFlowControlPattern regex. Added fallback integration test verifying that
conditionals with parameter branches fall back to static resolution.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace static fallback with Go template {{ if eq ... }}...{{ else }}...{{ end }}
syntax when conditional branches contain Helm expressions. This preserves
deploy-time dynamism for parameter-based conditions even when branches
reference other parameters.
Key changes:
- BuildHelmTernary → BuildHelmConditional with ternary for literals,
if/else for expression branches, static fallback for complex cases
- TryFormatBranch: scalar expressions get | quote, literals wrapped as
{{ "literal" | quote }} to avoid bare quotes in YAML plain scalars
- AllocateBranchParameters: stores branch parameter values in
AdditionalConfigValues to populate values.yaml without case-insensitive
key collisions in ToConfigMap's processedKeys
- HelmFlowControlPattern regex now matches {{ if in addition to {{ ternary
- ShouldDoubleQuoteString checks flow control before ScalarExpressionPattern
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Re-generated the verified snapshot by running the test against the current codebase. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Conditional expressions now expose the union of both branches' ValueProviders, enabling publish contexts to discover all referenced parameters and resources without inspecting WhenTrue/WhenFalse directly. Guard RegisterFormattedParameters against the parallel array mismatch (ValueProviders is now non-empty but ManifestExpressions/StringFormats remain empty for conditionals) by recursing into each branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…egistration The hash-based conditional name (XxHash32 of condition + branches) is now stable, so caching the built ReferenceExpression in RedisResource is unnecessary. Fix RegisterConditionalExpressions to register the expression itself when it is conditional, not just nested conditionals in ValueProviders. With ValueProviders now containing the union of both branches' providers (parameters, endpoints, etc.), the top-level conditional was no longer being discovered through the nested provider scan. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Align with the TypeScript base.ts which already uses the current $expr key for reference expression serialization. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rences Validate that ValueProviders returns the union of both branches' providers, References includes condition + all branch references, nested conditionals propagate providers through the chain, and duplicate conditionals produce identical hash-based names. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
We should make sure that the new Relevant test file: https://github.com/dotnet/aspire/blob/release/13.2/tests/Aspire.Hosting.Tests/ResourceDependencyTests.cs |
Add 4 test cases for resource dependency tracking with conditional reference expressions: basic branch dependencies, endpoint references, nested conditionals, and deduplication. Fix bug in ReferenceExpression.References where the condition object itself was not yielded - only its sub-references via IValueWithReferences. Since ParameterResource does not implement IValueWithReferences, the condition resource was silently dropped from dependency tracking. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Added test coverage. |
ExpressionResolver.EvalExpressionAsync had no handling for conditional ReferenceExpressions. Since conditional expressions have Format="" (empty string), the method returned null, and the dashboard silently dropped the environment variable (resolvedValue?.Value != null check). Add conditional branch to EvalExpressionAsync that evaluates the condition, selects the matching branch, and recurses — mirroring the logic already present in ReferenceExpression.GetValueAsync. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rison
Remove ternary and static fallback paths from BuildHelmConditional. All
conditionals now use {{ if eq ... }}...{{ else }}...{{ end }} syntax which
natively handles mixed content (literal + Helm expressions) without needing
TryFormatBranch or printf workarounds.
Add | lower pipe for case-insensitive comparison, matching .NET's
StringComparison.OrdinalIgnoreCase used in other execution/publish paths.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Apply toLower() to condition values in Azure Container Apps and App Service Bicep ternary expressions, matching the OrdinalIgnoreCase semantics used in all other execution and publish paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@sebastienros @mitchdenny follow up here to make sure deployment works well e2e |
|
Those don’t auto run without explicit triggering |
Description
Extends
ReferenceExpressionwith a conditional mode that models ternary-style values in connection strings and other reference expressions. This enables proper manifest publishing and polyglot code generation for conditional logic like TLSssl=truein Redis connection strings, replacing the closure-basedDeferredValueProvider.ReferenceExpression conditional mode
ReferenceExpression.CreateConditional(condition, conditionMatch, whenTrue, whenFalse)creates a conditional expression that evaluates anIValueProvidercondition against a given "truthy" string value and selects between twoReferenceExpressionbranches. Key design:ReferenceExpressionrather than a separate type.IsConditional,Condition,WhenTrue,WhenFalseproperties expose the state.ValueExpressionat construction time (e.g.,{redis.bindings.tcp.tlsEnabled}→cond-redis-bindings-tcp-tlsenabled), eliminating the need for explicit naming.value.v0entries in the manifest, referenced via{name.value}in connection strings.Unified ATS wire format
Conditional expressions use the same
$exprmarker as value-mode expressions. The presence of aconditionproperty distinguishes the two modes:{ "$expr": { "format": "...", "valueProviders": [...] } }{ "$expr": { "condition": <$handle>, "matchValue": <string>, "whenTrue": <$expr>, "whenFalse": <$expr> } }All 5 polyglot language base files (TypeScript, Go, Python, Java, Rust) support:
createConditional()factory method for client-side constructiontoJSON()serialization using the unified$exprformatReferenceExpressionRefinAtsMarshallerRedis TLS
Re-enables TLS by default for Redis container endpoints.
EndpointReference.GetTlsValuenow returns a conditionalReferenceExpressioninstead of using a closure-basedDeferredValueProvider, making it compatible with polyglot code generation.Example
Fixes #13645
Checklist
<para/>and<code/>elements on your triple slash comments?aspire.devissue: