-
Notifications
You must be signed in to change notification settings - Fork 745
Fix referencing existing keyvault resources as references #10991
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
Changes from all commits
a1cf345
0c296c8
a14f33f
961a4c6
6751a3b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -145,6 +145,79 @@ public static T CreateExistingOrNewProvisionableResource<T>(AzureResourceInfrast | |
| return provisionedResource; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Attempts to apply the name and (optionally) the resource group scope for the <see cref="ProvisionableResource"/> | ||
| /// from an <see cref="ExistingAzureResourceAnnotation"/> attached to <paramref name="aspireResource"/>. | ||
| /// </summary> | ||
| /// <param name="aspireResource">The Aspire resource that may have an <see cref="ExistingAzureResourceAnnotation"/>.</param> | ||
| /// <param name="infra">The infrastructure used for converting parameters into provisioning expressions.</param> | ||
| /// <param name="provisionableResource">The <see cref="ProvisionableResource"/> resource to configure.</param> | ||
| /// <returns><see langword="true"/> if an <see cref="ExistingAzureResourceAnnotation"/> was present and applied; otherwise <see langword="false"/>.</returns> | ||
| /// <remarks> | ||
| /// When the annotation includes a resource group, a synthetic <c>scope</c> property is added to the resource's | ||
| /// provisionable properties to correctly scope the existing resource in the generated Bicep. | ||
| /// The caller is responsible for setting a generated name when the method returns <see langword="false"/>. | ||
| /// </remarks> | ||
| public static bool TryApplyExistingResourceNameAndScope(IAzureResource aspireResource, AzureResourceInfrastructure infra, ProvisionableResource provisionableResource) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(aspireResource); | ||
| ArgumentNullException.ThrowIfNull(infra); | ||
| ArgumentNullException.ThrowIfNull(provisionableResource); | ||
|
|
||
| if (!aspireResource.TryGetLastAnnotation<ExistingAzureResourceAnnotation>(out var existingAnnotation)) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| var existingResourceName = existingAnnotation.Name switch | ||
| { | ||
| ParameterResource nameParameter => nameParameter.AsProvisioningParameter(infra), | ||
| string s => new BicepValue<string>(s), | ||
| _ => throw new NotSupportedException($"Existing resource name type '{existingAnnotation.Name.GetType()}' is not supported.") | ||
| }; | ||
|
|
||
| ((IBicepValue)existingResourceName).Self = new BicepValueReference(provisionableResource, "Name", ["name"]); | ||
| provisionableResource.ProvisionableProperties["name"] = existingResourceName; | ||
|
|
||
| static bool ResourceGroupEquals(object existingResourceGroup, object? infraResourceGroup) | ||
| { | ||
| // We're in the resource group being created | ||
| if (infraResourceGroup is null) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| // Compare the resource groups only if they are the same type (string or ParameterResource) | ||
| if (infraResourceGroup.GetType() == existingResourceGroup.GetType()) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this constraint? Couldn't we evaluate the value on a ParameterResource and compare it against a string resource group in a different annotation?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That doesn't seem like a meaningful comparison. |
||
| { | ||
| return infraResourceGroup.Equals(existingResourceGroup); | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| // Apply resource group scope if the target infrastructure's resource group is different from the existing annotation's resource group | ||
| if (existingAnnotation.ResourceGroup is not null && | ||
| !ResourceGroupEquals(existingAnnotation.ResourceGroup, infra.AspireResource.Scope?.ResourceGroup)) | ||
| { | ||
| BicepValue<string> scope = existingAnnotation.ResourceGroup switch | ||
| { | ||
| string rgName => new FunctionCallExpression(new IdentifierExpression("resourceGroup"), new StringLiteralExpression(rgName)), | ||
| ParameterResource p => new FunctionCallExpression(new IdentifierExpression("resourceGroup"), p.AsProvisioningParameter(infra).Value.Compile()), | ||
| _ => throw new NotSupportedException($"Resource group type '{existingAnnotation.ResourceGroup.GetType()}' is not supported.") | ||
| }; | ||
|
|
||
davidfowl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // HACK: This is a dance we do to set extra properties using Azure.Provisioning | ||
| // will be resolved if we ever get https://github.com/Azure/azure-sdk-for-net/issues/47980 | ||
| var expression = scope.Compile(); | ||
| var value = new BicepValue<string>(expression); | ||
| ((IBicepValue)value).Self = new BicepValueReference(provisionableResource, "Scope", ["scope"]); | ||
| provisionableResource.ProvisionableProperties["scope"] = value; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| private void EnsureParametersAlign(AzureResourceInfrastructure infrastructure) | ||
| { | ||
| // WARNING: GetParameters currently returns more than one instance of the same | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to do this for more Azure resources? Or is KeyVault the only one with this problem?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#10992, will throw copilot at it once this is done.