Fix parameter issues (#42070)

JoshLove-msft · web-flow · commit 910632f90638 · 2024-02-19T22:32:14.000-08:00
diff --git a/sdk/provisioning/Azure.Provisioning/src/Construct.cs b/sdk/provisioning/Azure.Provisioning/src/Construct.cs
@@ -279,13 +279,13 @@ internal void WriteOutputs(MemoryStream stream)
             foreach (var output in outputsToWrite)
             {
                 string value;
-                if (output.IsLiteral || ReferenceEquals(this, output.ModuleSource))
+                if (output.IsLiteral || ReferenceEquals(this, output.Resource.ModuleScope))
                 {
                     value = output.IsLiteral ? $"'{output.Value}'" : output.Value;
                 }
                 else
                 {
-                    value = $"{output.ModuleSource!.Name}.outputs.{output.Name}";
+                    value = $"{output.Resource.ModuleScope!.Name}.outputs.{output.Name}";
                 }
                 string name = output.Name;
                 stream.WriteLine($"output {name} string = {value}");
diff --git a/sdk/provisioning/Azure.Provisioning/src/ModuleInfrastructure.cs b/sdk/provisioning/Azure.Provisioning/src/ModuleInfrastructure.cs
@@ -47,7 +47,6 @@ private void AddOutputsToModules()
                 // ToList to avoid modifying the collection while iterating
                 foreach (var output in construct.GetOutputs(false).ToList())
                 {
-                    output.ModuleSource = output.Resource.ModuleScope!;
                     output.Resource.ModuleScope!.AddOutput(output);
                 }
             }
@@ -102,7 +101,6 @@ private void BuildModuleConstructs(Resource resource, Dictionary<Resource, List<
                 {
                     root = construct;
                     construct.IsRoot = true;
-                    parentScope = construct;
                 }
             }
 
@@ -112,6 +110,8 @@ private void BuildModuleConstructs(Resource resource, Dictionary<Resource, List<
                 resource.ModuleScope = parentScope;
             }
 
+            parentScope ??= construct;
+
             if (parentScope != null)
             {
                 foreach (var parameter in resource.Parameters)
@@ -131,7 +131,8 @@ private void BuildModuleConstructs(Resource resource, Dictionary<Resource, List<
                             parameterToCopy.DefaultValue,
                             parameterToCopy.IsSecure,
                             parentScope,
-                            parameterToCopy.Value);
+                            parameterToCopy.Value,
+                            parameterToCopy.Output);
                     }
                 }
             }
diff --git a/sdk/provisioning/Azure.Provisioning/src/Output.cs b/sdk/provisioning/Azure.Provisioning/src/Output.cs
@@ -27,7 +27,6 @@ public class Output
         /// </summary>
         public bool IsSecure { get; }
         internal IConstruct Source { get; }
-        internal IConstruct? ModuleSource { get; set; }
 
         internal Resource Resource { get; }
 
diff --git a/sdk/provisioning/Azure.Provisioning/src/Parameter.cs b/sdk/provisioning/Azure.Provisioning/src/Parameter.cs
@@ -29,8 +29,9 @@ public readonly struct Parameter
         /// Gets a value indicating whether the parameter is secure.
         /// </summary>
         public bool IsSecure { get; }
-        internal bool IsFromOutput { get; }
-        internal bool IsLiteral { get; }
+
+        internal bool IsFromOutput => Output != null;
+        internal bool IsLiteral => Output?.IsLiteral ?? false;
         internal string? Value { get; }
         internal IConstruct? Source { get; }
         internal Output? Output { get; }
@@ -43,21 +44,20 @@ public Parameter(Output output)
         {
             Name = output.Name;
             IsSecure = output.IsSecure;
-            IsFromOutput = true;
-            IsLiteral = output.IsLiteral;
             Value = output.Value;
             Source = output.Source;
             Output = output;
         }
 
-        internal Parameter(string name, string? description, object? defaultValue, bool isSecure, IConstruct source, string? value)
+        internal Parameter(string name, string? description, object? defaultValue, bool isSecure, IConstruct source, string? value, Output? output)
         {
             Name = name;
             Description = description;
             DefaultValue = defaultValue;
             IsSecure = isSecure;
             Source = source;
             Value = value;
+            Output = output;
         }
 
         /// <summary>
@@ -82,14 +82,21 @@ internal string GetParameterString(IConstruct parentScope)
             {
                 return Name;
             }
+
+            // If the parameter is from an output that is not in the current scope, use the parameter name.
+            if (!parentScope.GetOutputs().Contains(Output))
+            {
+                return Name;
+            }
+
             // If the parameter is an output from the current scope, use its Value.
-            if (ReferenceEquals(Output!.ModuleSource, parentScope))
+            if (ReferenceEquals(Output!.Resource.ModuleScope, parentScope))
             {
                 return Value!;
             }
 
             // Otherwise it is an output from a different scope, use the full reference.
-            return $"{Output!.ModuleSource!.Name}.outputs.{Name}";
+            return $"{Output!.Resource.ModuleScope!.Name}.outputs.{Name}";
         }
     }
 }
diff --git a/sdk/provisioning/Azure.Provisioning/src/keyvault/KeyVaultAddAccessPolicy.cs b/sdk/provisioning/Azure.Provisioning/src/keyvault/KeyVaultAddAccessPolicy.cs
@@ -33,6 +33,7 @@ public KeyVaultAddAccessPolicy(IConstruct scope, Parameter principalIdParameter,
                 }))
         {
             ParameterOverrides.Add(Properties.AccessPolicies[0], new Dictionary<string, Parameter> { { nameof(KeyVaultAccessPolicy.ObjectId), principalIdParameter } });
+            Parameters.Add(principalIdParameter);
         }
 
         private static string GetParamValue(Parameter principalIdParameter, IConstruct scope)
diff --git a/sdk/provisioning/Azure.Provisioning/tests/Infrastructure/OutputsSpanningModules/main.bicep b/sdk/provisioning/Azure.Provisioning/tests/Infrastructure/OutputsSpanningModules/main.bicep
@@ -28,6 +28,9 @@ resource resourceGroup_Q4i0lpa1h 'Microsoft.Resources/resourceGroups@2023-07-01'
 module rg1_TEST './resources/rg1_TEST/rg1_TEST.bicep' = {
   name: 'rg1_TEST'
   scope: resourceGroup_AVG5HpqPz
+  params: {
+    SERVICE_API_IDENTITY_PRINCIPAL_ID: rg3_TEST.outputs.SERVICE_API_IDENTITY_PRINCIPAL_ID
+  }
 }
 
 module rg2_TEST './resources/rg2_TEST/rg2_TEST.bicep' = {
diff --git a/sdk/provisioning/Azure.Provisioning/tests/Infrastructure/OutputsSpanningModules/resources/rg1_TEST/rg1_TEST.bicep b/sdk/provisioning/Azure.Provisioning/tests/Infrastructure/OutputsSpanningModules/resources/rg1_TEST/rg1_TEST.bicep
@@ -1,3 +1,7 @@
+@secure()
+@description('')
+param SERVICE_API_IDENTITY_PRINCIPAL_ID string
+
 
 resource appServicePlan_viooTTlOI 'Microsoft.Web/serverfarms@2021-02-01' = {
   name: 'appServicePlan-TEST'
@@ -44,7 +48,7 @@ resource applicationSettingsResource_MAMFSSuFs 'Microsoft.Web/sites/config@2021-
 
 resource keyVault_BRsYQF4qT 'Microsoft.KeyVault/vaults@2023-02-01' = {
   name: 'kv-TEST'
-  location: LOCATION
+  location: webSite_dOTaZfna6.location
   properties: {
     tenantId: '00000000-0000-0000-0000-000000000000'
     sku: {
diff --git a/sdk/provisioning/Azure.Provisioning/tests/Infrastructure/StorageBlobDefaults/resources/rg_TEST/rg_TEST.bicep b/sdk/provisioning/Azure.Provisioning/tests/Infrastructure/StorageBlobDefaults/resources/rg_TEST/rg_TEST.bicep
@@ -1,6 +1,6 @@
 
-resource storageAccount_o16OWzTQE 'Microsoft.Storage/storageAccounts@2022-09-01' = {
-  name: 'photoacct985209930ac24f6'
+resource storageAccount_7Spem00ph 'Microsoft.Storage/storageAccounts@2022-09-01' = {
+  name: 'photoacct4aa56e7da51149b'
   location: 'westus'
   sku: {
     name: 'Premium_LRS'
@@ -10,8 +10,8 @@ resource storageAccount_o16OWzTQE 'Microsoft.Storage/storageAccounts@2022-09-01'
   }
 }
 
-resource blobService_b1lTObtBZ 'Microsoft.Storage/storageAccounts/blobServices@2022-09-01' = {
-  parent: storageAccount_o16OWzTQE
+resource blobService_Al1mntjNG 'Microsoft.Storage/storageAccounts/blobServices@2022-09-01' = {
+  parent: storageAccount_7Spem00ph
   name: 'default'
   properties: {
     cors: {
diff --git a/sdk/provisioning/Azure.Provisioning/tests/Infrastructure/StorageBlobDropDown/resources/rg_TEST/rg_TEST.bicep b/sdk/provisioning/Azure.Provisioning/tests/Infrastructure/StorageBlobDropDown/resources/rg_TEST/rg_TEST.bicep
@@ -1,6 +1,6 @@
 
-resource storageAccount_xlURFCc5A 'Microsoft.Storage/storageAccounts@2022-09-01' = {
-  name: 'photoaccte6de904bc2d5415'
+resource storageAccount_xksvj6bLA 'Microsoft.Storage/storageAccounts@2022-09-01' = {
+  name: 'photoacct420f4454773442c'
   location: 'westus'
   sku: {
     name: 'Premium_LRS'
@@ -10,8 +10,8 @@ resource storageAccount_xlURFCc5A 'Microsoft.Storage/storageAccounts@2022-09-01'
   }
 }
 
-resource blobService_AhHMDPJzw 'Microsoft.Storage/storageAccounts/blobServices@2022-09-01' = {
-  parent: storageAccount_xlURFCc5A
+resource blobService_OX8Ox5p80 'Microsoft.Storage/storageAccounts/blobServices@2022-09-01' = {
+  parent: storageAccount_xksvj6bLA
   name: 'default'
   properties: {
     cors: {
diff --git a/sdk/provisioning/Azure.Provisioning/tests/Infrastructure/WebSiteUsingL1/resources/rg_TEST/rg_TEST.bicep b/sdk/provisioning/Azure.Provisioning/tests/Infrastructure/WebSiteUsingL1/resources/rg_TEST/rg_TEST.bicep
@@ -94,7 +94,7 @@ resource keyVaultAddAccessPolicy_NWCGclP20 'Microsoft.KeyVault/vaults/accessPoli
     accessPolicies: [
       {
         tenantId: '00000000-0000-0000-0000-000000000000'
-        objectId: SERVICE_API_IDENTITY_PRINCIPAL_ID
+        objectId: webSite_W5EweSXEq.identity.principalId
         permissions: {
           secrets: [
             'get'
diff --git a/sdk/provisioning/Azure.Provisioning/tests/Infrastructure/WebSiteUsingL2/resources/rg_TEST/rg_TEST.bicep b/sdk/provisioning/Azure.Provisioning/tests/Infrastructure/WebSiteUsingL2/resources/rg_TEST/rg_TEST.bicep
@@ -37,7 +37,7 @@ resource keyVaultAddAccessPolicy_NWCGclP20 'Microsoft.KeyVault/vaults/accessPoli
     accessPolicies: [
       {
         tenantId: '00000000-0000-0000-0000-000000000000'
-        objectId: SERVICE_API_IDENTITY_PRINCIPAL_ID
+        objectId: webSite_W5EweSXEq.identity.principalId
         permissions: {
           secrets: [
             'get'
diff --git a/sdk/provisioning/Azure.Provisioning/tests/Infrastructure/WebSiteUsingL3/resources/rg_TEST/rg_TEST.bicep b/sdk/provisioning/Azure.Provisioning/tests/Infrastructure/WebSiteUsingL3/resources/rg_TEST/rg_TEST.bicep
@@ -40,7 +40,7 @@ resource keyVaultAddAccessPolicy_NWCGclP20 'Microsoft.KeyVault/vaults/accessPoli
     accessPolicies: [
       {
         tenantId: '00000000-0000-0000-0000-000000000000'
-        objectId: SERVICE_API_IDENTITY_PRINCIPAL_ID
+        objectId: webSite_W5EweSXEq.identity.principalId
         permissions: {
           secrets: [
             'get'
diff --git a/sdk/provisioning/Azure.Provisioning/tests/ProvisioningTests.cs b/sdk/provisioning/Azure.Provisioning/tests/ProvisioningTests.cs
@@ -6,6 +6,7 @@
 using System;
 using System.IO;
 using System.Diagnostics;
+using System.Linq;
 using Azure.Core.TestFramework;
 using Azure.Core.Tests.TestFramework;
 using Azure.Provisioning.AppService;
@@ -165,8 +166,14 @@ public void OutputsSpanningModules()
             frontEnd2.AssignParameter(data => data.Identity.PrincipalId, new Parameter(output1));
             frontEnd2.AssignParameter(data => data.Location, new Parameter(output2));
 
-            _ = new TestFrontEndWebSite(infra, parent: rg3);
+            var testFrontEndWebSite = new TestFrontEndWebSite(infra, parent: rg3);
             infra.Build(GetOutputPath());
+
+            Assert.AreEqual(3, infra.GetParameters().Count());
+            Assert.AreEqual(3, infra.GetOutputs().Count());
+
+            Assert.AreEqual(0, testFrontEndWebSite.GetParameters().Count());
+            Assert.AreEqual(1, testFrontEndWebSite.GetOutputs().Count());
         }
 
         private static string GetGitRoot()

Original file line number	Diff line number	Diff line change
`@@ -279,13 +279,13 @@ internal void WriteOutputs(MemoryStream stream)`
`279`	`279`	`foreach (var output in outputsToWrite)`
`280`	`280`	`{`
`281`	`281`	`string value;`
`282`		`- if (output.IsLiteral \|\| ReferenceEquals(this, output.ModuleSource))`
	`282`	`+ if (output.IsLiteral \|\| ReferenceEquals(this, output.Resource.ModuleScope))`
`283`	`283`	`{`
`284`	`284`	`value = output.IsLiteral ? $"'{output.Value}'" : output.Value;`
`285`	`285`	`}`
`286`	`286`	`else`
`287`	`287`	`{`
`288`		`- value = $"{output.ModuleSource!.Name}.outputs.{output.Name}";`
	`288`	`+ value = $"{output.Resource.ModuleScope!.Name}.outputs.{output.Name}";`
`289`	`289`	`}`
`290`	`290`	`string name = output.Name;`
`291`	`291`	`stream.WriteLine($"output {name} string = {value}");`
Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,6 @@ private void AddOutputsToModules()`
`47`	`47`	`// ToList to avoid modifying the collection while iterating`
`48`	`48`	`foreach (var output in construct.GetOutputs(false).ToList())`
`49`	`49`	`{`
`50`		`- output.ModuleSource = output.Resource.ModuleScope!;`
`51`	`50`	`output.Resource.ModuleScope!.AddOutput(output);`
`52`	`51`	`}`
`53`	`52`	`}`
`@@ -102,7 +101,6 @@ private void BuildModuleConstructs(Resource resource, Dictionary<Resource, List<`
`102`	`101`	`{`
`103`	`102`	`root = construct;`
`104`	`103`	`construct.IsRoot = true;`
`105`		`- parentScope = construct;`
`106`	`104`	`}`
`107`	`105`	`}`
`108`	`106`
`@@ -112,6 +110,8 @@ private void BuildModuleConstructs(Resource resource, Dictionary<Resource, List<`
`112`	`110`	`resource.ModuleScope = parentScope;`
`113`	`111`	`}`
`114`	`112`
	`113`	`+ parentScope ??= construct;`
	`114`	`+`
`115`	`115`	`if (parentScope != null)`
`116`	`116`	`{`
`117`	`117`	`foreach (var parameter in resource.Parameters)`
`@@ -131,7 +131,8 @@ private void BuildModuleConstructs(Resource resource, Dictionary<Resource, List<`
`131`	`131`	`parameterToCopy.DefaultValue,`
`132`	`132`	`parameterToCopy.IsSecure,`
`133`	`133`	`parentScope,`
`134`		`- parameterToCopy.Value);`
	`134`	`+ parameterToCopy.Value,`
	`135`	`+ parameterToCopy.Output);`
`135`	`136`	`}`
`136`	`137`	`}`
`137`	`138`	`}`
Original file line number	Diff line number	Diff line change
`@@ -29,8 +29,9 @@ public readonly struct Parameter`
`29`	`29`	`/// Gets a value indicating whether the parameter is secure.`
`30`	`30`	`/// </summary>`
`31`	`31`	`public bool IsSecure { get; }`
`32`		`- internal bool IsFromOutput { get; }`
`33`		`- internal bool IsLiteral { get; }`
	`32`	`+`
	`33`	`+ internal bool IsFromOutput => Output != null;`
	`34`	`+ internal bool IsLiteral => Output?.IsLiteral ?? false;`
`34`	`35`	`internal string? Value { get; }`
`35`	`36`	`internal IConstruct? Source { get; }`
`36`	`37`	`internal Output? Output { get; }`
`@@ -43,21 +44,20 @@ public Parameter(Output output)`
`43`	`44`	`{`
`44`	`45`	`Name = output.Name;`
`45`	`46`	`IsSecure = output.IsSecure;`
`46`		`- IsFromOutput = true;`
`47`		`- IsLiteral = output.IsLiteral;`
`48`	`47`	`Value = output.Value;`
`49`	`48`	`Source = output.Source;`
`50`	`49`	`Output = output;`
`51`	`50`	`}`
`52`	`51`
`53`		`- internal Parameter(string name, string? description, object? defaultValue, bool isSecure, IConstruct source, string? value)`
	`52`	`+ internal Parameter(string name, string? description, object? defaultValue, bool isSecure, IConstruct source, string? value, Output? output)`
`54`	`53`	`{`
`55`	`54`	`Name = name;`
`56`	`55`	`Description = description;`
`57`	`56`	`DefaultValue = defaultValue;`
`58`	`57`	`IsSecure = isSecure;`
`59`	`58`	`Source = source;`
`60`	`59`	`Value = value;`
	`60`	`+ Output = output;`
`61`	`61`	`}`
`62`	`62`
`63`	`63`	`/// <summary>`
`@@ -82,14 +82,21 @@ internal string GetParameterString(IConstruct parentScope)`
`82`	`82`	`{`
`83`	`83`	`return Name;`
`84`	`84`	`}`
	`85`	`+`
	`86`	`+ // If the parameter is from an output that is not in the current scope, use the parameter name.`
	`87`	`+ if (!parentScope.GetOutputs().Contains(Output))`
	`88`	`+ {`
	`89`	`+ return Name;`
	`90`	`+ }`
	`91`	`+`
`85`	`92`	`// If the parameter is an output from the current scope, use its Value.`
`86`		`- if (ReferenceEquals(Output!.ModuleSource, parentScope))`
	`93`	`+ if (ReferenceEquals(Output!.Resource.ModuleScope, parentScope))`
`87`	`94`	`{`
`88`	`95`	`return Value!;`
`89`	`96`	`}`
`90`	`97`
`91`	`98`	`// Otherwise it is an output from a different scope, use the full reference.`
`92`		`- return $"{Output!.ModuleSource!.Name}.outputs.{Name}";`
	`99`	`+ return $"{Output!.Resource.ModuleScope!.Name}.outputs.{Name}";`
`93`	`100`	`}`
`94`	`101`	`}`
`95`	`102`	`}`
Original file line number	Diff line number	Diff line change
`@@ -33,6 +33,7 @@ public KeyVaultAddAccessPolicy(IConstruct scope, Parameter principalIdParameter,`
`33`	`33`	`}))`
`34`	`34`	`{`
`35`	`35`	`ParameterOverrides.Add(Properties.AccessPolicies[0], new Dictionary<string, Parameter> { { nameof(KeyVaultAccessPolicy.ObjectId), principalIdParameter } });`
	`36`	`+ Parameters.Add(principalIdParameter);`
`36`	`37`	`}`
`37`	`38`
`38`	`39`	`private static string GetParamValue(Parameter principalIdParameter, IConstruct scope)`
Original file line number	Diff line number	Diff line change
`@@ -28,6 +28,9 @@ resource resourceGroup_Q4i0lpa1h 'Microsoft.Resources/resourceGroups@2023-07-01'`
`28`	`28`	`module rg1_TEST './resources/rg1_TEST/rg1_TEST.bicep' = {`
`29`	`29`	`name: 'rg1_TEST'`
`30`	`30`	`scope: resourceGroup_AVG5HpqPz`
	`31`	`+ params: {`
	`32`	`+ SERVICE_API_IDENTITY_PRINCIPAL_ID: rg3_TEST.outputs.SERVICE_API_IDENTITY_PRINCIPAL_ID`
	`33`	`+ }`
`31`	`34`	`}`
`32`	`35`
`33`	`36`	`module rg2_TEST './resources/rg2_TEST/rg2_TEST.bicep' = {`
Original file line number	Diff line number	Diff line change
`@@ -94,7 +94,7 @@ resource keyVaultAddAccessPolicy_NWCGclP20 'Microsoft.KeyVault/vaults/accessPoli`
`94`	`94`	`accessPolicies: [`
`95`	`95`	`{`
`96`	`96`	`tenantId: '00000000-0000-0000-0000-000000000000'`
`97`		`- objectId: SERVICE_API_IDENTITY_PRINCIPAL_ID`
	`97`	`+ objectId: webSite_W5EweSXEq.identity.principalId`
`98`	`98`	`permissions: {`
`99`	`99`	`secrets: [`
`100`	`100`	`'get'`