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

Allow for customization of the Azure ProvisioningContext + Better Azure resource name scheme #5809

Merged
merged 9 commits into from
Sep 24, 2024

Conversation

eerhardt
Copy link
Member

@eerhardt eerhardt commented Sep 20, 2024

Description

Add IOptions which contains a ProvisioningContext. Users can customize the ProvisioningContext by configuring the AzureProvisioningOptions like a typical IOptions.

This allows us to use the new CDK default naming scheme. Users can opt into the old scheme by customizing the ProvisioningContext.

Fix #5756
Fix #5341

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • 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?
Microsoft Reviewers: Open in CodeFlow

Add IOptions<AzureProvisioningOptions> which contains a ProvisioningContext. Users can customize the ProvisioningContext by configuring the AzureProvisioningOptions like a typical IOptions.
Use a property on AzureConstructResource instead of flowing the ProvisioningContext through to the GetBicep methods.
@@ -55,8 +56,7 @@ public override BicepTemplateFile GetBicepTemplateFile(string? directory = null,
var generationPath = Directory.CreateTempSubdirectory("aspire").FullName;
var moduleSourcePath = Path.Combine(generationPath, "main.bicep");

var provisioningContext = GetProvisioningContext();
var plan = resourceModuleConstruct.Build(provisioningContext);
var plan = resourceModuleConstruct.Build(ProvisioningContext);
Copy link
Member

@davidfowl davidfowl Sep 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You still need a fallback if this isn't set.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It allows null.

https://github.com/Azure/azure-sdk-for-net/blob/c8bdcfb11c5283c34da4693d5a700e565d80c384/sdk/provisioning/Azure.Provisioning/src/Infrastructure.cs#L193

It uses the context ??= ProvisioningContext.Provider.GetProvisioningContext();.

@tg-msft - is that going to change at all with the changes to remove the DefaultInfrastructure you were mentioning?

@eerhardt eerhardt marked this pull request as ready for review September 23, 2024 17:31
@eerhardt eerhardt added the breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. label Sep 23, 2024
@eerhardt
Copy link
Member Author

This PR is now ready for review.

@eerhardt eerhardt requested a review from mitchdenny September 23, 2024 22:49
@davidfowl davidfowl merged commit 9354539 into dotnet:main Sep 24, 2024
11 checks passed
@eerhardt eerhardt deleted the ProvisioningContextCustomization branch September 24, 2024 11:03
@davidfowl davidfowl added this to the 9.0 milestone Oct 17, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Nov 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking-change Issue or PR that represents a breaking API or functional change over a prerelease.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better Azure resource name scheme Invalid Cloud deployment of Azure Service Bus topics - name truncation
2 participants