Repository navigation
Description
Background and Motivation
Follow-up from the 17.0 API review (thread). cc Matt Kotsenas (@MattKotsenas)
PipelineOutputDiffRule.Action (added in #21681) is a string that accepts Include or Exclude, matched case-insensitively (source). Its sibling, PipelineOutputDiffOptions.Mode, uses the PipelineOutputDiffMode enum, so an enum would make the valid values discoverable and keep the two consistent. The enum design guidelines say "DO favor using an enum instead of static constants." The API is [Experimental("ASPIREPIPELINES005")], so it can change in the next release.
The string type is deliberate (source):
// Keep Action bindable as text: invalid enum conversion can silently drop collection
// items. Options validation must see and reject malformed configuration instead.WithOutputDiffing binds the Pipeline:OutputDiff section directly (source) using the reflection-based configuration binder; Aspire.Hosting doesn't enable the binder source generator. The binder's BindCollection catches conversion failures and skips the item unless BinderOptions.ErrorOnUnknownConfiguration is set.
I checked this locally with Action changed to enum { Include, Exclude } (Microsoft.Extensions.Configuration.Binder 10.0.15 and 11.0.0 behaved the same). Today, options validation rejects every one of these inputs with OptionsValidationException:
Rules in configuration |
Bound rules with an enum Action |
|---|---|
[{ "Pattern": "**/*" }] |
Include **/* |
[{ "Action": null, "Pattern": "**/*" }] |
Include **/* |
[{ "Action": 1, "Pattern": "**/*" }] |
Exclude **/* |
[{ "Action": "Include,Exclude", "Pattern": "**/*" }] |
Exclude **/* |
[{ "Action": "", "Pattern": "**/*" }] |
None. The rule is dropped, so the default include of **/* applies. |
[{ "Action": "Drop", "Pattern": "**/*" }] |
None. The rule is dropped, so the default include of **/* applies. |
[{ "Action": "Include", "Pattern": "**/*" }, { "Action": "Exlude", "Pattern": "secrets/**" }] |
Include **/* only. The misspelled exclude is dropped. |
In every row, what's left is valid: each Action is a defined value, or the rule is gone. An Enum.IsDefined check can't catch any of these, and a misspelled Exclude silently widens the comparison instead of failing. All five malformed-Action cases in MalformedJsonRulesAreRejectedAtOptionsResolution would stop throwing.
Mode isn't affected because it isn't a collection item: a value such as "Gti" throws InvalidOperationException during binding.
Proposed API
namespace Aspire.Hosting.Pipelines;
+[Experimental("ASPIREPIPELINES005", UrlFormat = "https://aka.ms/aspire/diagnostics/{0}")]
+public enum PipelineOutputDiffRuleAction
+{
+ Include = 0,
+ Exclude = 1,
+}
+
[Experimental("ASPIREPIPELINES005", UrlFormat = "https://aka.ms/aspire/diagnostics/{0}")]
public sealed class PipelineOutputDiffRule
{
- public string Action { get; set; }
+ public PipelineOutputDiffRuleAction Action { get; set; }
public string Pattern { get; set; }
}Implementation requirements:
- Keep malformed configuration an error. Don't bind the enum directly. For example, bind each rule into an internal type with a string
Action, and map it in anIConfigureOptions<PipelineOutputDiffOptions>registered where the section binding is today. That keeps configuration ordered beforeConfigurecallbacks registered afterWithOutputDiffing(), and beforePostConfigure. Unparsable actions must still fail withOptionsValidationExceptionwhen options are resolved. Parsing must also stay lazy:CustomProviderOwnsPolicyWithoutGitOrValidNativeOptionsconfiguresRules:0:ActionasCustomPolicyfor a custom provider and must keep passing. - Decide which spellings are valid. Today only the two names are accepted, case-insensitively.
Enum.TryParseand the binder's converter also accept numeric strings and comma-separated lists ("Include,Exclude"becomesExclude), so neither is a drop-in replacement. For comparison,Modealready accepts"1". - Decide what an unset
Actionmeans. WithInclude = 0, an omitted action becomesInclude, both in configuration and in C# (new() { Pattern = "generated/**" }). Both are rejected today. Options include keeping configuration strict through the string mapping, makingActionrequired, or making it nullable. The guidelines recommend giving zero to the most common default when there's no naturalNonevalue. - Use the enum downstream. Validate with
Enum.IsDefined(rule.Action), and switch on the value inDefaultPipelineOutputSnapshotter, which currently treats every action other thanIncludeasExclude(source).
Usage Examples
The C# example in docs/specs/pipeline-output.md becomes:
builder.Services.Configure<PipelineOutputDiffOptions>(options =>
{
options.Mode = PipelineOutputDiffMode.FileSystem;
options.Rules.Add(new() { Action = PipelineOutputDiffRuleAction.Include, Pattern = "generated/**" });
});
builder.Services.PostConfigure<PipelineOutputDiffOptions>(options =>
options.Rules.Add(new() { Action = PipelineOutputDiffRuleAction.Exclude, Pattern = "**/*.tmp" }));Configuration is unchanged:
{
"Pipeline": {
"OutputDiff": {
"Mode": "FileSystem",
"Rules": [
{ "Action": "Include", "Pattern": "generated/**" },
{ "Action": "Exclude", "Pattern": "**/*.tmp" }
]
}
}
}Polyglot AppHosts aren't affected. The only exported WithOutputDiffing overload takes step names and a description, and no [AspireExport] signature uses these types, so polyglot AppHosts configure rules through configuration.
Alternative Designs
- Keep
stringand add constants, for example a staticPipelineOutputDiffRuleActionsclass. This is more discoverable, but any string still compiles, and the guidelines favor enums over static constants. - Add static factories
PipelineOutputDiffRule.Include(string pattern)andPipelineOutputDiffRule.Exclude(string pattern). These improve discoverability and work with either property type. - Keep the current design. Document the valid values and the reason for the string type in the
ActionXML docs, then close this issue. - Set
BinderOptions.ErrorOnUnknownConfiguration."","Drop", and"Exlude"then throwInvalidOperationException, but missing,null, numeric, and comma-separated actions still bind. It also rejects unknown keys, such as an extra"Note"property on a rule, which is a broader behavior change, and the failure isn't anOptionsValidationException. - Enable the configuration binder source generator for Aspire.Hosting. In a quick check it threw for
"Exlude", but it's a project-wide change, and a missingActionwould still bind as the zero value.
Risks
- This is a source and binary breaking change for C# code that sets or reads
Action, which is acceptable for an[Experimental]API. Configuration keeps working as long as the names are still accepted case-insensitively. - The main risk is weakening validation.
MalformedJsonRulesAreRejectedAtOptionsResolutionmust keep rejecting every malformedAction; change its numeric case only if numbers are deliberately allowed. Add a case with a valid rule followed by an invalid one, and keepCustomProviderOwnsPolicyWithoutGitOrValidNativeOptionspassing. - Files to update:
src/Aspire.Hosting/Pipelines/PipelineOutputDiffOptions.cs: the enum, theActiontype, and its XML docssrc/Aspire.Hosting/Pipelines/PipelineOutputDiffOptionsValidator.cs: theActioncheck and its commentsrc/Aspire.Hosting/Pipelines/DefaultPipelineOutputSnapshotter.cs: the include/exclude decisionsrc/Aspire.Hosting/PipelineOutputDiffingExtensions.cs: the configuration bindingtests/Aspire.Hosting.Tests/Pipelines/PipelineOutputSelectionTests.cs,PipelineOutputDiffingTests.cs, andPipelineOutputStrategyTests.cs: rules that setActionto a stringtests/Aspire.Hosting.Tests/Pipelines/PipelineOutputChangeTests.cs: add the enum to the exact type list inDiffingExportsOnlyConsumerContracts;DiffingConsumerContractsAreExperimentalthen requiresASPIREPIPELINES005on itdocs/specs/pipeline-output.md: the C# example and the note on accepted actions
Activity