Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Aspire] Set autoConfigureDataProtection on ACA apps #3538

Closed
ellismg opened this issue Mar 14, 2024 · 6 comments · Fixed by #3711 or #3731
Closed

[Aspire] Set autoConfigureDataProtection on ACA apps #3538

ellismg opened this issue Mar 14, 2024 · 6 comments · Fixed by #3711 or #3731
Assignees
Milestone

Comments

@ellismg
Copy link
Member

ellismg commented Mar 14, 2024

The ACA team is adding a new feature (https://github.com/Azure/azure-rest-api-specs/pull/28001/files) which exposes a autoConfigureDataProtection property on the ACA App type. We need to start setting this to true for Aspire Apps to ensure that when an app is scaled up the multiple instances all use the same data protection configuration.

The API change is in the process of being rolled out - should be in regions we can test in in late March or early April - and once it has made it's way to all regions, we want it to be the default. I think we need to do the following:

  1. We already support the end user having full control over the shape of the data sent to the ACA control plane when we provision or update an app via azd infra synth. This gives them a YAML file they can edit and set any of these properties. However today we use a fixed api version when we make our ACA create call (it's hard coded to the latest stable release). I suspect that we will first add the ability to in this yaml file customize the api version that will be used for our REST calls - so a user who is doing stuff "by hand" can opt into the new version and set the new flag. When the ACA feature is rolled out to the staging/EUAP we can have folks test it by hand. We can land these changes eagerly because they are opt in and don't change the defaults. Users who want to try out the feature will have to run azd infra synth and then make two small edits to a YAML file (to set the API version to 2024-02-02-preview and to set autoConfigureDataProtection to true.

  2. Once the ACA feature has rolled out everywhere - update azd to by default use this new API version and set autoConfigureDataProtection to true. This means it will work by default even if folks don't run infra synth. We can have that PR ready to go after (1) is done and just wait to merge it until the feature is supported in all regions.

@ellismg
Copy link
Member Author

ellismg commented Mar 14, 2024

/cc @mitchdenny as an FYI for our plan of attack here. It's possible that we end up not having to do (2) on the AZD side depending on how far we get with the CDK work and having the bicep for container apps be written from the Aspire side of the fence.

@ellismg ellismg added this to the April 2024 milestone Mar 14, 2024
@mitchdenny
Copy link

I think we should probably plan on needing to do this in azd. At this point in time the spike on generating the container app resources inside Aspire.Hosting.Azure is just that ... a spike. The GA plan should still be to have azd set this property.

Using infra synth and editing the container app templates just to do some initial validation is a fine click stop though.

@claudiaregio
Copy link

claudiaregio commented Mar 15, 2024

@ellismg I have a few questions & a suggestion based on the answers. @kristenwomack

Questions

  1. This change would show up in the YAML file, correct?
  2. I want to confirm that given Mitch's comment that this property would show up my default (not via opt in), right? We really would like this experience to work by default for customers and not have them think any longer about how they can make their app auto scale with ACA when data protection gets pulled in.
  3. Can you confirm if the configurations below are correct and that the first one will be the one automatically added?

TRUE:

"dotnet": {
     "autoConfigureDataProtection" : true
}
"dotnet": {
}

FALSE:

"dotnet": {
     "autoConfigureDataProtection" : false
}
"someOtherThingMaybeJava": {
}

Suggestion

I ask because in chatting with @amcasey, he had a good point that while we believe it is best to automatically add this, if a user sees something they didn't add themselves, they may be tempted to delete it. This would cause them to just break themselves when deploying to ACA. Assuming the answers to the questions above are yes, yes, and yes, I think we could mitigate this by adding a comment along with the configuration that ends up automatically being added that has a quick explanation and/or an aka.ms link that leads to docs giving many more details on what's happening here.

@ellismg
Copy link
Member Author

ellismg commented Mar 15, 2024

I want to confirm that given Mitch's comment that this property would show up my default (not via opt in), right? We really would like this experience to work by default for customers and not have them think any longer about how they can make their app auto scale with ACA when data protection gets pulled in.

Correct - and this is what step (2) is getting after. Once the API Version is deployed to all the regions in the Azure Public Cloud and we can take a dependency on it, we'll turn this on by default. Then folks will no longer need to infra synth and edit YAML to opt into the custom API version (which they must know is supported in their region) and set the new property.

This change would show up in the YAML file, correct?

Correct - but note that by default the user never sees these YAML files - they are an implementation detail that are created behind the scenes. They are not even present in the user's project by default.

I ask because in chatting with @amcasey, he had a good point that while we believe it is best to automatically add this, if a user sees something they didn't add themselves, they may be tempted to delete it. This would cause them to just break themselves when deploying to ACA. Assuming the answers to the questions above are yes, yes, and yes, I think we could mitigate this by adding a comment along with the configuration that ends up automatically being added that has a quick explanation and/or an aka.ms link that leads to docs giving many more details on what's happening here.

Not opposed to this - but do note that even looking at these files is an advanced user journey. Most developers won't even run infra synth. There's already a bunch of other stuff in the YAML file today (e.g. it is just the YAML rendering of the ARM Container App object).

ellismg added a commit to ellismg/azure-dev that referenced this issue Mar 15, 2024
This change does two things:

- Supports using a custom api-version of the ACA CreateOrUpdate
request by allowing the yaml deployment template to have an
`api-version` property at top level. When set, this value is used as
the `api-version` of the request. The request body is the object,
rendered as JSON, with the `api-version` key removed.

- Uses the above new feature to opt into the `2024-02-02-preview` in
Aspire and sets the
`configuration.runtime.dotnet.autoConfigureDataProtection` property to
`true`.

Fixes Azure#3538
ellismg added a commit to ellismg/azure-dev that referenced this issue Mar 15, 2024
This change does two things:

- Supports using a custom api-version of the ACA CreateOrUpdate
request by allowing the yaml deployment template to have an
`api-version` property at top level. When set, this value is used as
the `api-version` of the request. The request body is the object,
rendered as JSON, with the `api-version` key removed.

- Uses the above new feature to opt into the `2024-02-02-preview` in
Aspire and sets the
`configuration.runtime.dotnet.autoConfigureDataProtection` property to
`true`.

Fixes Azure#3538
ellismg added a commit to ellismg/azure-dev that referenced this issue Mar 18, 2024
This change does two things:

- Supports using a custom api-version of the ACA CreateOrUpdate
request by allowing the yaml deployment template to have an
`api-version` property at top level. When set, this value is used as
the `api-version` of the request. The request body is the object,
rendered as JSON, with the `api-version` key removed.

- Uses the above new feature to opt into the `2024-02-02-preview` in
Aspire and sets the
`configuration.runtime.dotnet.autoConfigureDataProtection` property to
`true`.

Fixes Azure#3538
@claudiaregio
Copy link

@ellismg Ah ok so it may not be worth putting it in there? Do you have any telemetry per chance on what % of users open that file/edit that file? That may help us decide. I don't want to create unnecessary work

@ellismg
Copy link
Member Author

ellismg commented Mar 25, 2024

Do you have any telemetry per chance on what % of users open that file/edit that file?

@claudiaregio The only thing we could get from our telemetry is the count of users that have run azd infra synth, to cause that file to get persisted, we don't really have a good way of understanding if they edit it or just run the command because they want to see the IaC for their container apps to better understand their system. Also note that azd infra synth (the thing that writes this file today) is gated behind an alpha feature flag, so an end user needs to run a specific gesture to even enable the command to write this file. I expect the number of users is low. In practice the reason you would edit this file is that you need to customize something about your ACA configuration because the defaults that azd provided were not suitable for you. We (the Aspire and AZD team) and working together to develop a way for users to customize their ACA application without having to resort to editing this file and instead via customization of an object inside their Aspire app host project.

ellismg added a commit to ellismg/azure-dev that referenced this issue Apr 1, 2024
This change does two things:

- Supports using a custom api-version of the ACA CreateOrUpdate
request by allowing the yaml deployment template to have an
`api-version` property at top level. When set, this value is used as
the `api-version` of the request. The request body is the object,
rendered as JSON, with the `api-version` key removed.

- Uses the above new feature to opt into the `2024-02-02-preview` in
Aspire and sets the
`configuration.runtime.dotnet.autoConfigureDataProtection` property to
`true`.

Fixes Azure#3538
ellismg added a commit to ellismg/azure-dev that referenced this issue Apr 2, 2024
This change does two things:

- Supports using a custom api-version of the ACA CreateOrUpdate
request by allowing the yaml deployment template to have an
`api-version` property at top level. When set, this value is used as
the `api-version` of the request. The request body is the object,
rendered as JSON, with the `api-version` key removed.

- Uses the above new feature to opt into the `2024-02-02-preview` in
Aspire and sets the
`configuration.runtime.dotnet.autoConfigureDataProtection` property to
`true`.

Fixes Azure#3538
ellismg added a commit to ellismg/azure-dev that referenced this issue Apr 15, 2024
This change does two things:

- Supports using a custom api-version of the ACA CreateOrUpdate
request by allowing the yaml deployment template to have an
`api-version` property at top level. When set, this value is used as
the `api-version` of the request. The request body is the object,
rendered as JSON, with the `api-version` key removed.

- Uses the above new feature to opt into the `2024-02-02-preview` in
Aspire and sets the
`configuration.runtime.dotnet.autoConfigureDataProtection` property to
`true`.

Fixes Azure#3538
ellismg added a commit that referenced this issue Apr 16, 2024
This change does two things:

- Supports using a custom api-version of the ACA CreateOrUpdate
request by allowing the yaml deployment template to have an
`api-version` property at top level. When set, this value is used as
the `api-version` of the request. The request body is the object,
rendered as JSON, with the `api-version` key removed.

- Uses the above new feature to opt into the `2024-02-02-preview` in
Aspire and sets the
`configuration.runtime.dotnet.autoConfigureDataProtection` property to
`true`.

Fixes #3538
ellismg added a commit that referenced this issue Apr 17, 2024
This change does two things:

- Supports using a custom api-version of the ACA CreateOrUpdate
request by allowing the yaml deployment template to have an
`api-version` property at top level. When set, this value is used as
the `api-version` of the request. The request body is the object,
rendered as JSON, with the `api-version` key removed.

- Uses the above new feature to opt into the `2024-02-02-preview` in
Aspire and sets the
`configuration.runtime.dotnet.autoConfigureDataProtection` property to
`true`, but only when the alpha feature
`aspire.autoConfigureDataProtection` is turned on.

We are gating turning this feature on behind a feature flag the user
must opt-into instead of always sending this request, as we had done
in #3711, because currently regions in Azure are rejecting the
`2024-02-02-preview` requests (even though the error message implies
this should work).

This allows partners to test the end to end by running:

`azd config set alpha.aspire.autoConfigureDataProtection on`

And testing in a supported region, like `centraluseuap` (which can be
explicitly selected via `azd env set AZURE_LOCATION centraluseuap`.

Fixes #3538
ellismg added a commit that referenced this issue Apr 17, 2024
This change does two things:

- Supports using a custom api-version of the ACA CreateOrUpdate
request by allowing the yaml deployment template to have an
`api-version` property at top level. When set, this value is used as
the `api-version` of the request. The request body is the object,
rendered as JSON, with the `api-version` key removed.

- Uses the above new feature to opt into the `2024-02-02-preview` in
Aspire and sets the
`configuration.runtime.dotnet.autoConfigureDataProtection` property to
`true`, but only when the alpha feature
`aspire.autoConfigureDataProtection` is turned on.

We are gating turning this feature on behind a feature flag the user
must opt-into instead of always sending this request, as we had done
in #3711, because currently regions in Azure are rejecting the
`2024-02-02-preview` requests (even though the error message implies
this should work).

This allows partners to test the end to end by running:

`azd config set alpha.aspire.autoConfigureDataProtection on`

And testing in a supported region, like `centraluseuap` (which can be
explicitly selected via `azd env set AZURE_LOCATION centraluseuap`.

Fixes #3538
ellismg added a commit that referenced this issue Apr 17, 2024
This change does two things:

- Supports using a custom api-version of the ACA CreateOrUpdate
request by allowing the yaml deployment template to have an
`api-version` property at top level. When set, this value is used as
the `api-version` of the request. The request body is the object,
rendered as JSON, with the `api-version` key removed.

- Uses the above new feature to opt into the `2024-02-02-preview` in
Aspire and sets the
`configuration.runtime.dotnet.autoConfigureDataProtection` property to
`true`, but only when the alpha feature
`aspire.autoConfigureDataProtection` is turned on.

We are gating turning this feature on behind a feature flag the user
must opt-into instead of always sending this request, as we had done
in #3711, because currently regions in Azure are rejecting the
`2024-02-02-preview` requests (even though the error message implies
this should work).

This allows partners to test the end to end by running:

`azd config set alpha.aspire.autoConfigureDataProtection on`

And testing in a supported region, like `centraluseuap` (which can be
explicitly selected via `azd env set AZURE_LOCATION centraluseuap`.

Fixes #3538
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment