Fix referencing existing keyvault resources as references

[davidfowl]

Description

• Today when using referencing an azure resources marked with AsExisting, that is not taken into account when using AsKeyVaultSecret, nor AddAsExisting. This causes errors to happen at deployment time. This change introduces a new helper to make this easier TryApplyExistingResourceNameAndScope.
• This change properly takes the existing annotation into account when generating the reference.
• Added and updated tests

Fixes #10908

Checklist

• Is this feature complete?
  • Yes. Ready to ship.
• Are you including unit tests for the changes and scenario tests if relevant?
  • Yes
• Did you add public API?
  • Yes
• Does the change make any security assumptions or guarantees?
  • No
• Does the change require an update in our Aspire docs?
  • No

PS: There will be follow up fixes to apply this to every implementation of AddAsExistingResource

[Copilot]

Pull Request Overview

This PR fixes a bug where referencing existing Azure Key Vault resources using AsExisting wasn't properly handled when using AsKeyVaultSecret or AddAsExisting. The fix ensures that the existing resource annotation is properly considered when generating Bicep references, preventing deployment-time errors.

• Fixes improper handling of existing Azure resource annotations in Key Vault secret references
• Adds a new helper method TryApplyExistingResourceNameAndScope to handle existing resource configuration
• Updates Key Vault resource provisioning to use the new helper method

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/Aspire.Hosting.Azure/AzureProvisioningResourceExtensions.cs	Updates Key Vault secret reference logic to properly handle existing resources
src/Aspire.Hosting.Azure/AzureProvisioningResource.cs	Adds new helper method for applying existing resource configuration
src/Aspire.Hosting.Azure.KeyVault/AzureKeyVaultResource.cs	Updates Key Vault resource provisioning to use the new helper method
tests/Aspire.Hosting.Azure.Tests/AzureKeyVaultTests.cs	Adds new test cases for existing Key Vault scenarios
tests/Aspire.Hosting.Azure.Tests/AzureContainerAppsTests.cs	Updates tests to include existing Key Vault references
tests/Aspire.Hosting.Azure.Tests/Snapshots/*	Updated snapshot files reflecting the corrected Bicep generation

[davidfowl]

Still testing. When I try to deploy with this I get:

Error BCP139: A resource's scope must match the scope of the Bicep file for it to be deployable. You must use modules to deploy resources to a different scope

[davidfowl]

Turns out this implementation would need to get quite a bit more complex as its not possible to reference across modules like this.

[davidfowl]

OK the issue is the role assignment bicep:

@description('The location for the resource(s) to be deployed.')
param location string = resourceGroup().location

param principalId string

resource kv 'Microsoft.KeyVault/vaults@2024-11-01' existing = {
  name: 'davidfowlkv0'
  scope: resourceGroup('rg-shared-dev')
}

resource kv_KeyVaultSecretsUser 'Microsoft.Authorization/roleAssignments@2022-04-01' = {
  name: guid(kv.id, principalId, subscriptionResourceId('Microsoft.Authorization/roleDefinitions', '4633458b-17de-408a-b874-0445c86b69e6'))
  properties: {
    principalId: principalId
    roleDefinitionId: subscriptionResourceId('Microsoft.Authorization/roleDefinitions', '4633458b-17de-408a-b874-0445c86b69e6')
    principalType: 'ServicePrincipal'
  }
  scope: kv
}

We need to know if the module has the "right" scope so that we can elide the explicit resourceGroup(...), or maybe we can special case the role assignment logic.

[davidfowl]

OK resolved the issue by checking the scope of the target infrastructure. If that is different from the existing resource's scope it skips this part.

[eerhardt]

Do we need to do this for more Azure resources? Or is KeyVault the only one with this problem?

[davidfowl]

#10992, will throw copilot at it once this is done.

[captainsafia]

Why this constraint? Couldn't we evaluate the value on a ParameterResource and compare it against a string resource group in a different annotation?

[davidfowl]

That doesn't seem like a meaningful comparison.