[release/9.0] Experimental custom domain support for ACA. #6391
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
This one does make me a bit nervous as it is essentially adding a new feature as well here (while being added as experimental). Help me understand the impact and risk a bit more, How likely is it for someone to hit this regression and how confident are we with the fix in terms of risk? Also, is this fallout of the onboard to the new Azure.Provisioning? |
|
Risk is very low here, it's an additional API that isn't used by default and that customers will likely use instead of the code they were hacking together manually editing bicep in v8. |
|
The reason we had to do this was that in .NET Aspire 8.0 we achieved custom domain support by having a feature flag in If we don't ship this in 9.0, folks who want to customize their container apps (e.g. scale to zero) cannot use a custom domain because subsequent deployments would remove that binding and take their site offline. They could work around it by pasting in the blob of Azure Provisioning code - but that ultimately is what this extension method does. If they don't use the extension method there is literally zero impact because all it does is add some additional Bicep properties (if it is called). |
|
Got it, thanks for the explanation Mitch. This is approved for 9.0. One last question though, what is the plan for this long term? Is it to use this API and just remove the experimental attribute? If so, any idea when? |
|
@mitchdenny is this ready to go? If so I can merge. |
|
It's ready |
| /// <summary> | ||
| /// Provides extension methods for customizing Azure Container App resource. | ||
| /// </summary> | ||
| public static class ContainerAppExtensions |
There was a problem hiding this comment.
- Why is this in a new class?
- Why does this new class not follow the naming conventions in the rest of the library?
I think this method should go in the existing AzureContainerAppExtensions class.
There was a problem hiding this comment.
This was intentionally named I think because @mitchdenny was thinking it would eventually be a cdk method. But not sure
There was a problem hiding this comment.
Class named to match the type name.
#### AI description (iteration 1) #### PR Classification Merge branch `release/9.0` into `internal/release/9.0` to integrate recent changes and address specific work items. #### PR Summary This pull request integrates changes from the `release/9.0` branch into `internal/release/9.0`, addressing several work items related to Azure storage and network isolation. - `/tests/Aspire.Hosting.Azure.Tests/AzureContainerAppsTests.cs`: Added a new test `ConfigureCustomDomainsMutatesIngress` to validate custom domain configuration for Azure Container Apps. - `/src/Aspire.Hosting.Azure.AppContainers/ContainerAppExtensions.cs`: Introduced a new extension method `ConfigureCustomDomain` to simplify the process of assigning custom domains to Azure Container Apps. Related work items: dotnet#6368, dotnet#6391, dotnet#6433, dotnet#6458, dotnet#6467
Backport of #6275 to release/9.0
/cc @mitchdenny
Customer Impact
If a customer upgrades to .NET Aspire 9.0 and makes use of
PublishAsAzureContainerAppthenazdwill no longer preserve custom domains. This PR adds an experimental API that returns this capability which would unblock their adoption of .NET Aspire 9.0.Testing
Tested locally with AZD as well as unit tests to verify that valid Bicep is produced.
Risk
Low risk. Only comes into play when the
ConfigureCustomDomains(...)extension is used and does not influence any code paths unless it is used.Regression?
No.
Microsoft Reviewers: Open in CodeFlow