Support callback, custom config key and publishing for parameters.

[davidebbo]

Description

See #4429 for details.

Fixes #4429

Checklist

• Is this feature complete?
  • Yes. Ready to ship.
  • No. Follow-up changes expected to support new manifest value
• Are you including unit tests for the changes and scenario tests if relevant?
  • Yes
  • No
• Did you add public API?
  • Yes
    • If yes, did you have an API Review for it?
      • Yes
      • No
    • Did you add <remarks /> and <code /> elements on your triple slash comments?
      • Yes
      • No
  • No
• Does the change make any security assumptions or guarantees?
  • Yes
    • If yes, have you done a threat model and had a security review?
      • Yes
      • No
  • No
• Does the change require an update in our Aspire docs?
  • Yes
    • Link to aspire-docs issue: none yet
  • No

Microsoft Reviewers: Open in CodeFlow

[mitchdenny]

Any thoughts on supporting a Func<IServiceProvder, Task> variant? Although just looking at it now I am not sure how we do it without breaking API. I guess if someone really wanted to asynchrously load a parameter value they could by using a config extensibility and then referencing a config value.

[mitchdenny]

Thanks once again for the contribution @davidebbo

[davidebbo]

@mitchdenny per discussion with @davidfowl, we decided not to tackle async at this time, though this could be a future thing (it does look hard!).

Note that per #4429 (comment) and discord chat, we'll be making some design changes for publishing the value, so I'll be evolving the PR.

[davidfowl]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[davidfowl]

@mitchdenny This is new schema for parameters to specify a default value. We should update the schema and file an issue on azd to implement this.

@vhvb1989 Thoughts on implementing this idea of no prompting for parameters with defaults? Or being able to suppress them somehow?

[davidebbo]

@vhvb1989 Thoughts on implementing this idea of no prompting for parameters with defaults? Or being able to suppress them somehow?

If it's a 'default', there needs to be a way to override them. Maybe there are two modes (e.g. controlled by switches):

1. Use all manifest defaults without prompting
2. Prompt, but prefill the field with the manifest default

[vhvb1989]

FYI, azd suports a generate field in default, like

Azd uses the gereate config to produce the value instead of prompt. If you need a static default value, we could add a rule to the generate config to just use the provided value

@eerhardt

When we added support for this, we considered a case where azd could ask forlk if they wanted to auto-generate the value (based on the rules) or enter a value manually. This is supported by azd already (just not mapped to any configuration from the Aspire Manifest).

[davidebbo]

To me, it feels a little strange to have this in generate (because there is no generation), which is why I went with an alternative value property, e.g.

              "inputs": {
                "value": {
                  "type": "string",
                  "default": {
                    "value": "DefaultValue"
                  }
                }
              }

Either way, whoever owns the aspire manifest schema should decide where to put that. If we want it under generate that's easy to do.

[davidfowl]

I agree that generate is strange. I like the above proposal of having a "default" section.

[davidfowl]

Yep

[davidebbo]

Ok, I pushed the schema change. I also added test coverage for "generate", as it was missing. @mitchdenny PTAL

[davidebbo]

@mitchdenny Also, how does the schema make its way from this repo to https://github.com/dotnet/dotnet/blob/main/src/aspire/src/Schema/aspire-8.0.json? Automatic or we need to push it?

[davidfowl]

Manually I think

[mitchdenny]

Yeah all manual.

[davidfowl]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[davidfowl]

cc @vhvb1989 @ellismg

[davidfowl]

Filed Azure/azure-dev#4359

[vhvb1989]

@davidebbo @davidfowl @mitchdenny is it safe to asume that a default.value will never be set as secure ?, like:

"param-with-value": {
      "type": "parameter.v0",
      "value": "{param-with-value.inputs.value}",
      "inputs": {
        "value": {
          "type": "string",
          "secret": true,
          "default":{
            "value": "default value for param"
          }
        }
      }
    },

See the "secret": true, there.

I am working on making azd to fail on this case as per: https://learn.microsoft.com/azure/azure-resource-manager/bicep/linter-rule-secure-parameter-default

But I am wondering if the AppHost is even allowing this or not? (maybe is blocked in AppHost so I don't even need to worry)?

[davidebbo]

@vhvb1989 we do not block it today, but I agree that we should. So I think we should allow:

var foo = builder.AddParameter("foo", value: "somesecret", secret: true);

as this only uses the value at runtime, but not:

var foo = builder.AddParameter("foo", value: "somesecret", publishValueAsDefault: true, secret: true);

which results in having the secret in the manifest.

@davidfowl @mitchdenny do you agree? If so, I can make a follow-up change to prevent this. e.g. an error like the 'publishValueAsDefault' and 'secret' flags are mutually exclusive, to prevent saving a secret to the manifest

[davidebbo]

I sent #5891 to make this change.

[davidebbo]

@vhvb1989 This is now blocked in AppHost, so you should not encounter this unless people mess with their manifest by hand,