Skip to content

Commit

Permalink
Use fully qualified symbolic name as copy loop name (#15910)
Browse files Browse the repository at this point in the history
  • Loading branch information
jeskew authored Dec 20, 2024
1 parent 40f6174 commit 81e0d13
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 15 deletions.
48 changes: 48 additions & 0 deletions src/Bicep.Core.IntegrationTests/ScenarioTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6546,4 +6546,52 @@ public void Test_Issue15898()

result.ExcludingLinterDiagnostics().Should().NotHaveAnyDiagnostics();
}

[TestMethod]
public void Nested_copy_resources_should_be_assigned_a_fully_qualified_copy_loop_name()
{
var result = CompilationHelper.Compile("""
param location string = resourceGroup().location
param _resourceName string
param serviceAccountsApp1 string[]
param serviceAccountsApp2 string[]
param _aksOidcIssuerProfileURL string
param configAks { namespace: string }
resource aksWorkloadIdentityApp1 'Microsoft.ManagedIdentity/userAssignedIdentities@2023-01-31' = {
name: '${_resourceName}-id-app1'
location: location
@batchSize(1)
resource federation 'federatedIdentityCredentials' = [for item in serviceAccountsApp1: {
name: 'federated-credentials-${item}'
properties: {
audiences: [ 'api://AzureADTokenExchange' ]
issuer: _aksOidcIssuerProfileURL
subject: 'system:serviceaccount:${configAks.namespace}:${item}'
}
}]
}
resource aksWorkloadIdentityApp2 'Microsoft.ManagedIdentity/userAssignedIdentities@2023-01-31' = {
name: '${_resourceName}-id-app2'
location: location
@batchSize(1)
resource federation 'federatedIdentityCredentials' = [for item in serviceAccountsApp2: {
name: 'federated-credentials-${item}'
properties: {
audiences: [ 'api://AzureADTokenExchange' ]
issuer: _aksOidcIssuerProfileURL
subject: 'system:serviceaccount:${configAks.namespace}:${item}'
}
}]
}
""");

result.ExcludingLinterDiagnostics().Should().NotHaveAnyDiagnostics();
result.Template.Should().NotBeNull();
result.Template.Should().HaveValueAtPath("$.resources.aksWorkloadIdentityApp1::federation.copy.name", "aksWorkloadIdentityApp1::federation");
result.Template.Should().HaveValueAtPath("$.resources.aksWorkloadIdentityApp2::federation.copy.name", "aksWorkloadIdentityApp2::federation");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"_generator": {
"name": "bicep",
"version": "dev",
"templateHash": "13207151182888561919"
"templateHash": "7127796614903415778"
}
},
"parameters": {
Expand Down Expand Up @@ -98,7 +98,7 @@
},
{
"copy": {
"name": "loopChild",
"name": "loopParent::loopChild",
"count": "[length(variables('items'))]"
},
"type": "My.Rp/parentType/childType",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ resource loopParent 'My.Rp/parentType@2020-12-01' = {
resource loopChild 'childType' = [for item in items: {
//@ {
//@ "copy": {
//@ "name": "loopChild",
//@ "name": "loopParent::loopChild",
//@ "count": "[length(variables('items'))]"
//@ },
//@ "type": "My.Rp/parentType/childType",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"_generator": {
"name": "bicep",
"version": "dev",
"templateHash": "15846782372928243436"
"templateHash": "1582162446949403796"
}
},
"parameters": {
Expand Down Expand Up @@ -109,7 +109,7 @@
},
"loopParent::loopChild": {
"copy": {
"name": "loopChild",
"name": "loopParent::loopChild",
"count": "[length(variables('items'))]"
},
"type": "My.Rp/parentType/childType",
Expand Down Expand Up @@ -159,4 +159,4 @@
"value": "loopChild"
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"_generator": {
"name": "bicep",
"version": "dev",
"templateHash": "9129114772864237098"
"templateHash": "10727609760574024345"
}
},
"parameters": {
Expand Down Expand Up @@ -66,7 +66,7 @@
"resources": [
{
"copy": {
"name": "sqlDatabases",
"name": "sqlServer::sqlDatabases",
"count": "[length(variables('dbs'))]",
"mode": "serial",
"batchSize": 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1164,7 +1164,7 @@ resource sqlServer 'Microsoft.Sql/servers@2021-11-01' = {
resource sqlDatabases 'databases' = [for db in dbs: {
//@ {
//@ "copy": {
//@ "name": "sqlDatabases",
//@ "name": "sqlServer::sqlDatabases",
//@ "count": "[length(variables('dbs'))]",
//@ "mode": "serial",
//@ "batchSize": 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"_generator": {
"name": "bicep",
"version": "dev",
"templateHash": "5924434758202725836"
"templateHash": "17920750879435313448"
}
},
"parameters": {
Expand Down Expand Up @@ -76,7 +76,7 @@
},
"sqlServer::sqlDatabases": {
"copy": {
"name": "sqlDatabases",
"name": "sqlServer::sqlDatabases",
"count": "[length(variables('dbs'))]",
"mode": "serial",
"batchSize": 1
Expand Down
7 changes: 5 additions & 2 deletions src/Bicep.Core/Emit/ExpressionConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ [new JTokenExpression(parameter.Symbol.Name),],
DeclaredResourceMetadata declared when context.Settings.EnableSymbolicNames =>
GenerateSymbolicReference(declared, indexContext),
DeclaredResourceMetadata declared => declared.Symbol.IsCollection && indexContext == null
? new JTokenExpression(declared.Symbol.Name) // this is the copy name
? new JTokenExpression(GetSymbolicName(declared)) // this is the copy name
: GetFullyQualifiedResourceId(resource),

_ => throw new InvalidOperationException($"Unexpected resource metadata type: {resource.GetType()}"),
Expand Down Expand Up @@ -815,8 +815,11 @@ private LanguageExpression ConvertUnary(UnaryExpression unary)
}

public string GetSymbolicName(DeclaredResourceMetadata resource)
=> GetSymbolicName(context.SemanticModel.ResourceAncestors, resource);

public static string GetSymbolicName(ResourceAncestorGraph resourceAncestorGraph, DeclaredResourceMetadata resource)
{
var nestedHierarchy = this.context.SemanticModel.ResourceAncestors.GetAncestors(resource)
var nestedHierarchy = resourceAncestorGraph.GetAncestors(resource)
.Reverse()
.TakeWhile(x => x.AncestorType == ResourceAncestorGraph.ResourceAncestorType.Nested)
.Select(x => x.Resource)
Expand Down
10 changes: 8 additions & 2 deletions src/Bicep.Core/Intermediate/ExpressionBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -691,15 +691,21 @@ void AddLoop(LoopExpressionContext newLoop)
if (ancestor.AncestorType == ResourceAncestorGraph.ResourceAncestorType.Nested &&
ancestor.Resource.Symbol.DeclaringResource.Value is ForSyntax ancestorFor)
{
AddLoop(new(ancestor.Resource.Symbol.Name, ancestorFor, ConvertWithoutLowering(ancestorFor.Expression)));
AddLoop(new(
ExpressionConverter.GetSymbolicName(Context.SemanticModel.ResourceAncestors, ancestor.Resource),
ancestorFor,
ConvertWithoutLowering(ancestorFor.Expression)));
}
}

// Unwrap the 'real' resource body if there's a condition / for loop
var body = resource.Symbol.DeclaringResource.Value;
if (body is ForSyntax @for)
{
AddLoop(new(resource.Symbol.Name, @for, ConvertWithoutLowering(@for.Expression)));
AddLoop(new(
ExpressionConverter.GetSymbolicName(Context.SemanticModel.ResourceAncestors, resource),
@for,
ConvertWithoutLowering(@for.Expression)));
body = @for.Body;
}

Expand Down

0 comments on commit 81e0d13

Please sign in to comment.