Add subscription ID support to existing Azure resource scope - #11150
Add subscription ID support to existing Azure resource scope#11150David Fowler (davidfowl) with Copilot wants to merge 6 commits into
Conversation
…sion methods Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
|
Copilot check anywhere that uses AzureBicepResourceScope that may need to be updated. Also make sure the newly added tests pass, we're missing the snapshot files. |
…e usage sites Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
…tests Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
I've addressed both requests in commit 5269430:
The implementation now properly handles three scenarios:
All changes maintain backward compatibility and the API surface has been updated accordingly. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| /// </summary> | ||
| /// <param name="subscription">The subscription identifier for subscription-level resources.</param> | ||
| /// <param name="subscriptionOnly">Must be true to indicate this is a subscription-only scope.</param> | ||
| public AzureBicepResourceScope(object subscription, bool subscriptionOnly) |
There was a problem hiding this comment.
This API is kinda weird. I understand we got here because both the resourceGroup and subscription parameters are weakly-typed but it feels like this should be:
| public AzureBicepResourceScope(object subscription, bool subscriptionOnly) | |
| public AzureBicepResourceScope(object subscription, bool isSubscriptionScope) |
There was a problem hiding this comment.
Is this even a thing?
There was a problem hiding this comment.
Is this even a thing?
Bicep resources that are only subscription scoped are a thing. I dunno if we need to model it this directly in the app model. I could see it being useful if you wanted to create your own "main.bicep" and declare multiple resource groups per environment or something but maybe the pattern we'd use there is having you initiate different ResourceGroupBicepResources for each environment.
There was a problem hiding this comment.
the module can be subscription scoped, but resources are are always in a resource group no?
There was a problem hiding this comment.
Renamed parameter from subscriptionOnly to isSubscriptionScope as suggested and updated all usage sites in commit 7c35c67.
| @@ -0,0 +1,6 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
There was a problem hiding this comment.
Remove this file
| { | ||
| public AzureBicepResourceScope(object resourceGroup) { } | ||
|
|
||
| public object ResourceGroup { get { throw null; } } |
There was a problem hiding this comment.
Do not update this file, it will be auto updated.
There was a problem hiding this comment.
API file reverted to original state in commit 7c35c67.
| @@ -0,0 +1,33 @@ | |||
| using System.Text.Json.Nodes; | |||
There was a problem hiding this comment.
This test file should be deleted.
|
Copilot implement the review feedback |
…arameter Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
All review feedback implemented in commit 7c35c67:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| }, | ||
| "scope": { | ||
| "resourceGroup": "", | ||
| "subscription": "{existingSubscriptionId.value}" |
There was a problem hiding this comment.
Victor Vazquez (@vhvb1989) This will require azd changes.
There was a problem hiding this comment.
Did you close this PR at the end? nothing else to do?
There was a problem hiding this comment.
There’s a new one
|
🚀 Dogfood this PR with: curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 11150Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 11150" |
subscriptionOnlyparameter toisSubscriptionScopein AzureBicepResourceScope constructorImplementation Summary
The PR successfully adds subscription ID support to existing Azure resource scoping with the following key features:
Enhanced ExistingAzureResourceAnnotation
ExistingAzureResourceAnnotation(object name, object? resourceGroup, object subscription)Subscriptionproperty for subscription informationExtended Extension Methods
RunAsExisting(name, resourceGroup, subscription),PublishAsExisting(nameParam, rgParam, subParam),AsExisting("name", "rg", "subscription")Enhanced AzureBicepResourceScope
AzureBicepResourceScope(object resourceGroup)AzureBicepResourceScope(object resourceGroup, object subscription)AzureBicepResourceScope(object subscription, bool isSubscriptionScope)Enhanced Bicep Scope Generation
resourceGroup(subscriptionId, resourceGroupName)subscription(subscriptionId)resourceGroup(resourceGroupName)(backward compatible)Updated Manifest Schema
All changes maintain full backward compatibility and the build completes successfully.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.