Skip to content

Commit 1d96ac2

Browse files
davidfowldavidfowl
authored
Fix referencing existing keyvault resources as references (#10991)
* Fix referencing existing keyvault resources as references - 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 deploymetn time. - This change properly takes the existing annotation into account when generating the reference. - Added and updated tests * Enhance Key Vault reference handling in Azure App Service tests * Only add scope if the target module is not in the correct scope * Simplify the API * Rename parameter 'resource' to 'provisionableResource' in TryApplyExistingResourceNameAndScope method for clarity
1 parent e79c396 commit 1d96ac2

File tree

20 files changed

+373
-49
lines changed

20 files changed

+373
-49
lines changed

‎src/Aspire.Hosting.Azure.KeyVault/AzureKeyVaultResource.cs‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,18 +64,26 @@ public override ProvisionableResource AddAsExistingResource(AzureResourceInfrast
6464
{
6565
var bicepIdentifier = this.GetBicepIdentifier();
6666
var resources = infra.GetProvisionableResources();
67-
67+
6868
// Check if a KeyVaultService with the same identifier already exists
6969
var existingStore = resources.OfType<KeyVaultService>().SingleOrDefault(store => store.BicepIdentifier == bicepIdentifier);
70-
70+
7171
if (existingStore is not null)
7272
{
7373
return existingStore;
7474
}
75-
75+
7676
// Create and add new resource if it doesn't exist
7777
var store = KeyVaultService.FromExisting(bicepIdentifier);
78-
store.Name = NameOutputReference.AsProvisioningParameter(infra);
78+
79+
if (!TryApplyExistingResourceNameAndScope(
80+
this,
81+
infra,
82+
store))
83+
{
84+
store.Name = NameOutputReference.AsProvisioningParameter(infra);
85+
}
86+
7987
infra.Add(store);
8088
return store;
8189
}

‎src/Aspire.Hosting.Azure/AzureProvisioningResource.cs‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,79 @@ public static T CreateExistingOrNewProvisionableResource<T>(AzureResourceInfrast
145145
return provisionedResource;
146146
}
147147

148+
/// <summary>
149+
/// Attempts to apply the name and (optionally) the resource group scope for the <see cref="ProvisionableResource"/>
150+
/// from an <see cref="ExistingAzureResourceAnnotation"/> attached to <paramref name="aspireResource"/>.
151+
/// </summary>
152+
/// <param name="aspireResource">The Aspire resource that may have an <see cref="ExistingAzureResourceAnnotation"/>.</param>
153+
/// <param name="infra">The infrastructure used for converting parameters into provisioning expressions.</param>
154+
/// <param name="provisionableResource">The <see cref="ProvisionableResource"/> resource to configure.</param>
155+
/// <returns><see langword="true"/> if an <see cref="ExistingAzureResourceAnnotation"/> was present and applied; otherwise <see langword="false"/>.</returns>
156+
/// <remarks>
157+
/// When the annotation includes a resource group, a synthetic <c>scope</c> property is added to the resource's
158+
/// provisionable properties to correctly scope the existing resource in the generated Bicep.
159+
/// The caller is responsible for setting a generated name when the method returns <see langword="false"/>.
160+
/// </remarks>
161+
public static bool TryApplyExistingResourceNameAndScope(IAzureResource aspireResource, AzureResourceInfrastructure infra, ProvisionableResource provisionableResource)
162+
{
163+
ArgumentNullException.ThrowIfNull(aspireResource);
164+
ArgumentNullException.ThrowIfNull(infra);
165+
ArgumentNullException.ThrowIfNull(provisionableResource);
166+
167+
if (!aspireResource.TryGetLastAnnotation<ExistingAzureResourceAnnotation>(out var existingAnnotation))
168+
{
169+
return false;
170+
}
171+
172+
var existingResourceName = existingAnnotation.Name switch
173+
{
174+
ParameterResource nameParameter => nameParameter.AsProvisioningParameter(infra),
175+
string s => new BicepValue<string>(s),
176+
_ => throw new NotSupportedException($"Existing resource name type '{existingAnnotation.Name.GetType()}' is not supported.")
177+
};
178+
179+
((IBicepValue)existingResourceName).Self = new BicepValueReference(provisionableResource, "Name", ["name"]);
180+
provisionableResource.ProvisionableProperties["name"] = existingResourceName;
181+
182+
static bool ResourceGroupEquals(object existingResourceGroup, object? infraResourceGroup)
183+
{
184+
// We're in the resource group being created
185+
if (infraResourceGroup is null)
186+
{
187+
return false;
188+
}
189+
190+
// Compare the resource groups only if they are the same type (string or ParameterResource)
191+
if (infraResourceGroup.GetType() == existingResourceGroup.GetType())
192+
{
193+
return infraResourceGroup.Equals(existingResourceGroup);
194+
}
195+
196+
return false;
197+
}
198+
199+
// Apply resource group scope if the target infrastructure's resource group is different from the existing annotation's resource group
200+
if (existingAnnotation.ResourceGroup is not null &&
201+
!ResourceGroupEquals(existingAnnotation.ResourceGroup, infra.AspireResource.Scope?.ResourceGroup))
202+
{
203+
BicepValue<string> scope = existingAnnotation.ResourceGroup switch
204+
{
205+
string rgName => new FunctionCallExpression(new IdentifierExpression("resourceGroup"), new StringLiteralExpression(rgName)),
206+
ParameterResource p => new FunctionCallExpression(new IdentifierExpression("resourceGroup"), p.AsProvisioningParameter(infra).Value.Compile()),
207+
_ => throw new NotSupportedException($"Resource group type '{existingAnnotation.ResourceGroup.GetType()}' is not supported.")
208+
};
209+
210+
// HACK: This is a dance we do to set extra properties using Azure.Provisioning
211+
// will be resolved if we ever get https://github.com/Azure/azure-sdk-for-net/issues/47980
212+
var expression = scope.Compile();
213+
var value = new BicepValue<string>(expression);
214+
((IBicepValue)value).Self = new BicepValueReference(provisionableResource, "Scope", ["scope"]);
215+
provisionableResource.ProvisionableProperties["scope"] = value;
216+
}
217+
218+
return true;
219+
}
220+
148221
private void EnsureParametersAlign(AzureResourceInfrastructure infrastructure)
149222
{
150223
// WARNING: GetParameters currently returns more than one instance of the same

‎src/Aspire.Hosting.Azure/AzureProvisioningResourceExtensions.cs‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,22 @@ public static KeyVaultSecret AsKeyVaultSecret(this IAzureKeyVaultSecretReference
6666

6767
var resources = infrastructure.GetProvisionableResources();
6868

69-
var parameter = secretReference.Resource.NameOutputReference.AsProvisioningParameter(infrastructure);
70-
var kvName = Infrastructure.NormalizeBicepIdentifier($"{parameter.BicepIdentifier}_kv");
69+
var kvName = secretReference.Resource.GetBicepIdentifier();
7170

7271
var kv = resources.OfType<KeyVaultService>().SingleOrDefault(kv => kv.BicepIdentifier == kvName);
7372

7473
if (kv is null)
7574
{
7675
kv = KeyVaultService.FromExisting(kvName);
77-
kv.Name = parameter;
76+
77+
if (!AzureProvisioningResource.TryApplyExistingResourceNameAndScope(
78+
secretReference.Resource,
79+
infrastructure,
80+
kv))
81+
{
82+
kv.Name = secretReference.Resource.NameOutputReference.AsProvisioningParameter(infrastructure);
83+
}
84+
7885
infrastructure.Add(kv);
7986
}
8087

‎tests/Aspire.Hosting.Azure.Tests/AzureAppServiceTests.cs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,15 @@ public async Task KeyvaultReferenceHandling()
8181
var db = builder.AddAzureCosmosDB("mydb").WithAccessKeyAuthentication();
8282
db.AddCosmosDatabase("db");
8383

84+
var kvName = builder.AddParameter("kvName");
85+
var sharedRg = builder.AddParameter("sharedRg");
86+
87+
var existingKv = builder.AddAzureKeyVault("existingKv")
88+
.PublishAsExisting(kvName, sharedRg);
89+
8490
builder.AddProject<Project>("api", launchProfileName: null)
85-
.WithReference(db);
91+
.WithReference(db)
92+
.WithEnvironment("SECRET_VALUE", existingKv.GetSecret("secret"));
8693

8794
using var app = builder.Build();
8895

‎tests/Aspire.Hosting.Azure.Tests/AzureContainerAppsTests.cs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -423,10 +423,12 @@ public async Task AzureContainerAppsBicepGenerationIsIdempotent()
423423

424424
var secret = builder.AddParameter("secret", secret: true);
425425
var kv = builder.AddAzureKeyVault("kv");
426+
var existingKv = builder.AddAzureKeyVault("existingKv").PublishAsExisting("existingKvName", "existingRgName");
426427

427428
builder.AddContainer("api", "myimage")
428429
.WithEnvironment("TOP_SECRET", secret)
429-
.WithEnvironment("TOP_SECRET2", kv.Resource.GetSecret("secret"));
430+
.WithEnvironment("TOP_SECRET2", kv.GetSecret("secret"))
431+
.WithEnvironment("EXISTING_TOP_SECRET", existingKv.GetSecret("secret"));
430432

431433
using var app = builder.Build();
432434

@@ -670,8 +672,15 @@ public async Task KeyVaultReferenceHandling()
670672
var db = builder.AddAzureCosmosDB("mydb").WithAccessKeyAuthentication();
671673
db.AddCosmosDatabase("db");
672674

675+
var kvName = builder.AddParameter("kvName");
676+
var sharedRg = builder.AddParameter("sharedRg");
677+
678+
var existingKv = builder.AddAzureKeyVault("existingKv")
679+
.PublishAsExisting(kvName, sharedRg);
680+
673681
builder.AddContainer("api", "image")
674-
.WithReference(db);
682+
.WithReference(db)
683+
.WithEnvironment("SECRET_VALUE", existingKv.GetSecret("secret"));
675684

676685
using var app = builder.Build();
677686

‎tests/Aspire.Hosting.Azure.Tests/AzureKeyVaultTests.cs‎

Lines changed: 75 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public async Task AddKeyVaultViaRunMode()
2121

2222
await Verify(manifest.ToString(), "json")
2323
.AppendContentAsFile(bicep, "bicep");
24-
24+
2525
}
2626

2727
[Fact]
@@ -41,7 +41,7 @@ await Verify(manifest.ToString(), "json")
4141
.AppendContentAsFile(bicep, "bicep")
4242
.AppendContentAsFile(kvRolesBicep, "bicep")
4343
.AppendContentAsFile(kvRolesManifest.ToString(), "json");
44-
44+
4545
}
4646

4747
[Fact]
@@ -111,7 +111,75 @@ public async Task ConsumingAKeyVaultSecretInAnotherBicepModule()
111111

112112
await Verify(manifest.ToString(), "json")
113113
.AppendContentAsFile(bicep, "bicep");
114-
114+
115+
}
116+
117+
[Fact]
118+
public async Task ConsumingSecretsFromExistingKeyVaultInAnotherBicepModule_WithParameters()
119+
{
120+
using var builder = TestDistributedApplicationBuilder.Create(DistributedApplicationOperation.Publish);
121+
122+
var existingName = builder.AddParameter("existingKvName");
123+
var existingRg = builder.AddParameter("existingRgName");
124+
var kv = builder.AddAzureKeyVault("kv").PublishAsExisting(existingName, existingRg);
125+
126+
var secretReference = kv.Resource.GetSecret("mySecret");
127+
var secretReference2 = kv.Resource.GetSecret("mySecret2");
128+
129+
var module = builder.AddAzureInfrastructure("mymodule", infra =>
130+
{
131+
var secret = secretReference.AsKeyVaultSecret(infra);
132+
var secret2 = secretReference2.AsKeyVaultSecret(infra);
133+
_ = secretReference.AsKeyVaultSecret(infra); // idempotent
134+
135+
infra.Add(new ProvisioningOutput("secretUri1", typeof(string)) { Value = secret.Properties.SecretUri });
136+
infra.Add(new ProvisioningOutput("secretUri2", typeof(string)) { Value = secret2.Properties.SecretUri });
137+
});
138+
139+
var module2 = builder.AddAzureInfrastructure("mymodule2", infra =>
140+
{
141+
var secret = secretReference.AsKeyVaultSecret(infra);
142+
var secret2 = secretReference2.AsKeyVaultSecret(infra);
143+
144+
infra.Add(new ProvisioningOutput("secretUri1", typeof(string)) { Value = secret.Properties.SecretUri });
145+
infra.Add(new ProvisioningOutput("secretUri2", typeof(string)) { Value = secret2.Properties.SecretUri });
146+
});
147+
148+
module2.Resource.Scope = new(existingRg.Resource);
149+
150+
var (manifest, bicep) = await AzureManifestUtils.GetManifestWithBicep(module.Resource, skipPreparer: true);
151+
var (manifest2, bicep2) = await AzureManifestUtils.GetManifestWithBicep(module2.Resource, skipPreparer: true);
152+
153+
await Verify(manifest.ToString(), "json")
154+
.AppendContentAsFile(bicep, "bicep")
155+
.AppendContentAsFile(manifest.ToString(), "json")
156+
.AppendContentAsFile(bicep2, "bicep");
157+
}
158+
159+
[Fact]
160+
public async Task ConsumingSecretsFromExistingKeyVaultInAnotherBicepModule_WithLiterals()
161+
{
162+
using var builder = TestDistributedApplicationBuilder.Create(DistributedApplicationOperation.Publish);
163+
164+
var kv = builder.AddAzureKeyVault("kv").PublishAsExisting("literalKvName", "literalRgName");
165+
166+
var secretReference = kv.Resource.GetSecret("mySecret");
167+
var secretReference2 = kv.Resource.GetSecret("mySecret2");
168+
169+
var module = builder.AddAzureInfrastructure("mymodule", infra =>
170+
{
171+
var secret = secretReference.AsKeyVaultSecret(infra);
172+
var secret2 = secretReference2.AsKeyVaultSecret(infra);
173+
_ = secretReference.AsKeyVaultSecret(infra); // idempotent
174+
175+
infra.Add(new ProvisioningOutput("secretUri1", typeof(string)) { Value = secret.Properties.SecretUri });
176+
infra.Add(new ProvisioningOutput("secretUri2", typeof(string)) { Value = secret2.Properties.SecretUri });
177+
});
178+
179+
var (manifest, bicep) = await AzureManifestUtils.GetManifestWithBicep(module.Resource, skipPreparer: true);
180+
181+
await Verify(manifest.ToString(), "json")
182+
.AppendContentAsFile(bicep, "bicep");
115183
}
116184

117185
[Fact]
@@ -120,9 +188,9 @@ public void GetSecret_ReturnsSecretReference()
120188
using var builder = TestDistributedApplicationBuilder.Create();
121189

122190
var kv = builder.AddAzureKeyVault("myKeyVault");
123-
191+
124192
var secret = kv.GetSecret("mySecret");
125-
193+
126194
Assert.NotNull(secret);
127195
Assert.Equal("mySecret", secret.SecretName);
128196
Assert.Same(kv.Resource, secret.Resource);
@@ -135,9 +203,9 @@ public void AddSecret_ReturnsSecretResource()
135203

136204
var secretParam = builder.AddParameter("secretParam", secret: true);
137205
var kv = builder.AddAzureKeyVault("myKeyVault");
138-
206+
139207
var secretResource = kv.AddSecret("mySecret", secretParam);
140-
208+
141209
Assert.NotNull(secretResource);
142210
Assert.IsType<AzureKeyVaultSecretResource>(secretResource.Resource);
143211
Assert.Equal("mySecret", secretResource.Resource.Name);

‎tests/Aspire.Hosting.Azure.Tests/Snapshots/AzureAppServiceTests.KeyvaultReferenceHandling.verified.bicep‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ param api_containerimage string
1313

1414
param mydb_kv_outputs_name string
1515

16+
param kvName string
17+
18+
param sharedRg string
19+
1620
param api_identity_outputs_id string
1721

1822
param api_identity_outputs_clientid string
@@ -28,13 +32,23 @@ resource mainContainer 'Microsoft.Web/sites/sitecontainers@2024-11-01' = {
2832
parent: webapp
2933
}
3034

31-
resource mydb_kv_outputs_name_kv 'Microsoft.KeyVault/vaults@2024-11-01' existing = {
35+
resource mydb_kv 'Microsoft.KeyVault/vaults@2024-11-01' existing = {
3236
name: mydb_kv_outputs_name
3337
}
3438

35-
resource mydb_kv_outputs_name_kv_connectionstrings__mydb 'Microsoft.KeyVault/vaults/secrets@2024-11-01' existing = {
39+
resource mydb_kv_connectionstrings__mydb 'Microsoft.KeyVault/vaults/secrets@2024-11-01' existing = {
3640
name: 'connectionstrings--mydb'
37-
parent: mydb_kv_outputs_name_kv
41+
parent: mydb_kv
42+
}
43+
44+
resource existingKv 'Microsoft.KeyVault/vaults@2024-11-01' existing = {
45+
name: kvName
46+
scope: resourceGroup(sharedRg)
47+
}
48+
49+
resource existingKv_secret 'Microsoft.KeyVault/vaults/secrets@2024-11-01' existing = {
50+
name: 'secret'
51+
parent: existingKv
3852
}
3953

4054
resource webapp 'Microsoft.Web/sites@2024-11-01' = {
@@ -62,7 +76,11 @@ resource webapp 'Microsoft.Web/sites@2024-11-01' = {
6276
}
6377
{
6478
name: 'ConnectionStrings__mydb'
65-
value: '@Microsoft.KeyVault(SecretUri=${mydb_kv_outputs_name_kv_connectionstrings__mydb.properties.secretUri})'
79+
value: '@Microsoft.KeyVault(SecretUri=${mydb_kv_connectionstrings__mydb.properties.secretUri})'
80+
}
81+
{
82+
name: 'SECRET_VALUE'
83+
value: '@Microsoft.KeyVault(SecretUri=${existingKv_secret.properties.secretUri})'
6684
}
6785
{
6886
name: 'AZURE_CLIENT_ID'
@@ -78,4 +96,4 @@ resource webapp 'Microsoft.Web/sites@2024-11-01' = {
7896
'${api_identity_outputs_id}': { }
7997
}
8098
}
81-
}
99+
}

‎tests/Aspire.Hosting.Azure.Tests/Snapshots/AzureAppServiceTests.KeyvaultReferenceHandling.verified.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
"env_outputs_azure_container_registry_managed_identity_client_id": "{env.outputs.AZURE_CONTAINER_REGISTRY_MANAGED_IDENTITY_CLIENT_ID}",
99
"api_containerimage": "{api.containerImage}",
1010
"mydb_kv_outputs_name": "{mydb-kv.outputs.name}",
11+
"kvName": "{kvName.value}",
12+
"sharedRg": "{sharedRg.value}",
1113
"api_identity_outputs_id": "{api-identity.outputs.id}",
1214
"api_identity_outputs_clientid": "{api-identity.outputs.clientId}"
1315
}

0 commit comments

Comments
 (0)