From 3e5b6cba5ab490b17872cf634afd47d06afee49b Mon Sep 17 00:00:00 2001 From: stuartko <47676853+stuartko@users.noreply.github.com> Date: Mon, 30 Nov 2020 00:11:45 -0800 Subject: [PATCH] [Resources] Fix issue preventing deployment of Template Specs located outside of the current subscription context (#13483) * Template Specs: Fixing issue where template specs from subscriptions not in context could not be deployed * Adding mock test for covering recent cross-sub template spec deployment bug involving dynamic parameters. * Template Specs: Fixing issue with test on Unix based systems * Fix for test failure --- .../ResourceWithParameterCmdletBase.cs | 98 ++++++++++++++----- ...zureResourceGroupDeploymentCommandTests.cs | 87 +++++++++++++++- src/Resources/Resources/ChangeLog.md | 1 + 3 files changed, 163 insertions(+), 23 deletions(-) diff --git a/src/Resources/ResourceManager/Implementation/CmdletBase/ResourceWithParameterCmdletBase.cs b/src/Resources/ResourceManager/Implementation/CmdletBase/ResourceWithParameterCmdletBase.cs index 53c8e5dbf77e..b703d8655a9e 100644 --- a/src/Resources/ResourceManager/Implementation/CmdletBase/ResourceWithParameterCmdletBase.cs +++ b/src/Resources/ResourceManager/Implementation/CmdletBase/ResourceWithParameterCmdletBase.cs @@ -17,11 +17,11 @@ using System.Collections.Generic; using System.Linq; using System.Management.Automation; +using Microsoft.Azure.Commands.Common.Authentication; using Microsoft.Azure.Commands.Common.Authentication.Abstractions; using Microsoft.Azure.Commands.ResourceManager.Cmdlets.Components; -using Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkClient; -using Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkModels; using Microsoft.Azure.Commands.ResourceManager.Cmdlets.Utilities; +using Microsoft.Azure.Management.ResourceManager; using Microsoft.WindowsAzure.Commands.Utilities.Common; using Newtonsoft.Json.Linq; @@ -59,6 +59,8 @@ public abstract class ResourceWithParameterCmdletBase : ResourceManagerCmdletBas private string templateSpecId; + private ITemplateSpecsClient templateSpecsClient; + protected ResourceWithParameterCmdletBase() { dynamicParameters = new RuntimeDefinedParameterDictionary(); @@ -142,30 +144,35 @@ protected ResourceWithParameterCmdletBase() "to provide a better error message in the case where not all required parameters are satisfied.")] public SwitchParameter SkipTemplateParameterPrompt { get; set; } - private TemplateSpecsSdkClient templateSpecsSdkClient; - /// - /// Gets or sets the Template Specs Azure sdk client wrapper + /// Gets or sets the Template Specs Azure SDK client /// - public TemplateSpecsSdkClient TemplateSpecsSdkClient + public ITemplateSpecsClient TemplateSpecsClient { get { - if (this.templateSpecsSdkClient == null) + if (this.templateSpecsClient == null) { - this.templateSpecsSdkClient = new TemplateSpecsSdkClient(DefaultContext); + this.templateSpecsClient = + AzureSession.Instance.ClientFactory.CreateArmClient( + DefaultContext, + AzureEnvironment.Endpoint.ResourceManager + ); } - return this.templateSpecsSdkClient; + return this.templateSpecsClient; } - set { this.templateSpecsSdkClient = value; } + set { this.templateSpecsClient = value; } } public object GetDynamicParameters() { if (!this.IsParameterBound(c => c.SkipTemplateParameterPrompt)) { + // Resolve the static parameter names for this cmdlet: + string[] staticParameterNames = this.GetStaticParameterNames(); + if (TemplateObject != null && TemplateObject != templateObject) { templateObject = TemplateObject; @@ -175,7 +182,7 @@ public object GetDynamicParameters() TemplateObject, TemplateParameterObject, this.ResolvePath(TemplateParameterFile), - MyInvocation.MyCommand.Parameters.Keys.ToArray()); + staticParameterNames); } else { @@ -183,7 +190,7 @@ public object GetDynamicParameters() TemplateObject, TemplateParameterObject, TemplateParameterUri, - MyInvocation.MyCommand.Parameters.Keys.ToArray()); + staticParameterNames); } } else if (!string.IsNullOrEmpty(TemplateFile) && @@ -196,7 +203,7 @@ public object GetDynamicParameters() this.ResolvePath(TemplateFile), TemplateParameterObject, this.ResolvePath(TemplateParameterFile), - MyInvocation.MyCommand.Parameters.Keys.ToArray()); + staticParameterNames); } else { @@ -204,7 +211,7 @@ public object GetDynamicParameters() this.ResolvePath(TemplateFile), TemplateParameterObject, TemplateParameterUri, - MyInvocation.MyCommand.Parameters.Keys.ToArray()); + staticParameterNames); } } else if (!string.IsNullOrEmpty(TemplateUri) && @@ -217,7 +224,7 @@ public object GetDynamicParameters() TemplateUri, TemplateParameterObject, this.ResolvePath(TemplateParameterFile), - MyInvocation.MyCommand.Parameters.Keys.ToArray()); + staticParameterNames); } else { @@ -225,7 +232,7 @@ public object GetDynamicParameters() TemplateUri, TemplateParameterObject, TemplateParameterUri, - MyInvocation.MyCommand.Parameters.Keys.ToArray()); + staticParameterNames); } } else if (!string.IsNullOrEmpty(TemplateSpecId) && @@ -238,12 +245,25 @@ public object GetDynamicParameters() throw new PSArgumentException("No version found in Resource ID"); } - var templateSpecVersion = TemplateSpecsSdkClient.GetTemplateSpec( - ResourceIdUtility.GetResourceName(templateSpecId).Split('/')[0], + if (!string.IsNullOrEmpty(resourceIdentifier.Subscription) && + TemplateSpecsClient.SubscriptionId != resourceIdentifier.Subscription) + { + // The template spec is in a different subscription than our default + // context. Force the client to use that subscription: + TemplateSpecsClient.SubscriptionId = resourceIdentifier.Subscription; + } + + var templateSpecVersion = TemplateSpecsClient.TemplateSpecVersions.Get( ResourceIdUtility.GetResourceGroupName(templateSpecId), - resourceIdentifier.ResourceName).Versions.Single(); + ResourceIdUtility.GetResourceName(templateSpecId).Split('/')[0], + resourceIdentifier.ResourceName); + + if (!(templateSpecVersion.Template is JObject)) + { + throw new InvalidOperationException("Unexpected type."); // Sanity check + } - var templateObj = JObject.Parse(templateSpecVersion.Template); + JObject templateObj = (JObject)templateSpecVersion.Template; if (string.IsNullOrEmpty(TemplateParameterUri)) { @@ -251,7 +271,7 @@ public object GetDynamicParameters() templateObj, TemplateParameterObject, this.ResolvePath(TemplateParameterFile), - MyInvocation.MyCommand.Parameters.Keys.ToArray()); + staticParameterNames); } else { @@ -259,7 +279,7 @@ public object GetDynamicParameters() templateObj, TemplateParameterObject, TemplateParameterUri, - MyInvocation.MyCommand.Parameters.Keys.ToArray()); + staticParameterNames); } } } @@ -345,5 +365,39 @@ protected string GetDeploymentDebugLogLevel(string deploymentDebugLogLevel) return debugSetting; } + + /// + /// Gets the names of the static parameters defined for this cmdlet. + /// + protected string[] GetStaticParameterNames() + { + if (MyInvocation.MyCommand != null) + { + // We're running inside the shell... parameter information will already + // be resolved for us: + return MyInvocation.MyCommand.Parameters.Keys.ToArray(); + } + + // This invocation is internal (e.g: through a unit test), fallback to + // collecting the command/parameter info explicitly from our current type: + + CmdletAttribute cmdletAttribute = (CmdletAttribute)this.GetType() + .GetCustomAttributes(typeof(CmdletAttribute), true) + .FirstOrDefault(); + + if (cmdletAttribute == null) + { + throw new InvalidOperationException( + $"Expected type '{this.GetType().Name}' to have CmdletAttribute." + ); + } + + // The command name we provide for the temporary CmdletInfo isn't consumed + // anywhere other than instantiation, but let's resolve it anyway: + string commandName = $"{cmdletAttribute.VerbName}-{cmdletAttribute.NounName}"; + + CmdletInfo cmdletInfo = new CmdletInfo(commandName, this.GetType()); + return cmdletInfo.Parameters.Keys.ToArray(); + } } } diff --git a/src/Resources/Resources.Test/ResourceGroupDeployments/NewAzureResourceGroupDeploymentCommandTests.cs b/src/Resources/Resources.Test/ResourceGroupDeployments/NewAzureResourceGroupDeploymentCommandTests.cs index 71d669129276..4be4a82ba0c0 100644 --- a/src/Resources/Resources.Test/ResourceGroupDeployments/NewAzureResourceGroupDeploymentCommandTests.cs +++ b/src/Resources/Resources.Test/ResourceGroupDeployments/NewAzureResourceGroupDeploymentCommandTests.cs @@ -30,6 +30,12 @@ using System.Collections; using FluentAssertions; using ProvisioningState = Microsoft.Azure.Commands.ResourceManager.Cmdlets.Entities.ProvisioningState; +using Microsoft.Azure.Management.ResourceManager; +using Newtonsoft.Json.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Rest.Azure; +using Newtonsoft.Json; namespace Microsoft.Azure.Commands.Resources.Test { @@ -39,6 +45,10 @@ public class NewAzureResourceGroupDeploymentCommandTests : RMTestBase private Mock resourcesClientMock; + private Mock templateSpecsClientMock; + + private Mock templateSpecsVersionOperationsMock; + private Mock commandRuntimeMock; private string resourceGroupName = "myResourceGroup"; @@ -54,13 +64,20 @@ public class NewAzureResourceGroupDeploymentCommandTests : RMTestBase public NewAzureResourceGroupDeploymentCommandTests(ITestOutputHelper output) { resourcesClientMock = new Mock(); + + templateSpecsClientMock = new Mock(); + templateSpecsClientMock.SetupAllProperties(); + templateSpecsVersionOperationsMock = new Mock(); + templateSpecsClientMock.Setup(m => m.TemplateSpecVersions).Returns(templateSpecsVersionOperationsMock.Object); + XunitTracingInterceptor.AddToContext(new XunitTracingInterceptor(output)); commandRuntimeMock = new Mock(); SetupConfirmation(commandRuntimeMock); cmdlet = new NewAzureResourceGroupDeploymentCmdlet() { CommandRuntime = commandRuntimeMock.Object, - ResourceManagerSdkClient = resourcesClientMock.Object + ResourceManagerSdkClient = resourcesClientMock.Object, + TemplateSpecsClient = templateSpecsClientMock.Object }; } @@ -271,5 +288,73 @@ public void CreatesNewPSResourceGroupDeploymentWithUserTemplateEmptyRollback() commandRuntimeMock.Verify(f => f.WriteObject(expected), Times.Once()); } + + /// + /// When deployments are created using a template spec, the dynamic parameters are + /// resolved by reading the parameters from the template spec version's template body. + /// Previously a bug was present that prevented successful dynamic parameter resolution + /// if the template spec existed in a subscription outside the current subscription + /// context. This test validates dynamic parameter resolution works for deployments using + /// cross-subscription template specs. + /// + + [Fact] + [Trait(Category.AcceptanceType, Category.CheckIn)] + public void ResolvesDynamicParametersWithCrossSubTemplateSpec() + { + const string templateSpecSubscriptionId = "10000000-0000-0000-0000-000000000000"; + const string templateSpecRGName = "someRG"; + const string templateSpecName = "myTemplateSpec"; + const string templateSpecVersion = "v1"; + + string templateSpecId = $"/subscriptions/{templateSpecSubscriptionId}/" + + $"resourceGroups/{templateSpecRGName}/providers/Microsoft.Resources/" + + $"templateSpecs/{templateSpecName }/versions/{templateSpecVersion}"; + + // Ensure our template file path is normalized for the current system: + var normalizedTemplateFilePath = (Path.DirectorySeparatorChar != '\\') + ? templateFile.Replace('\\', Path.DirectorySeparatorChar) // Other/Unix based + : templateFile; // Windows based (already valid) + + var templateContentForTest = File.ReadAllText(normalizedTemplateFilePath); + var template = JsonConvert.DeserializeObject(templateContentForTest); + + templateSpecsVersionOperationsMock.Setup(f => f.GetWithHttpMessagesAsync( + templateSpecRGName, + templateSpecName, + templateSpecVersion, + null, + new CancellationToken())) + .Returns(() => { + + // We should only be getting this template spec from the expected subscription: + Assert.Equal(templateSpecSubscriptionId, templateSpecsClientMock.Object.SubscriptionId); + + var versionToReturn = new TemplateSpecVersion( + location: "westus2", + id: templateSpecId, + name: templateSpecVersion, + type: "Microsoft.Resources/templateSpecs/versions", + template: JObject.Parse(templateContentForTest) + ); + + return Task.Factory.StartNew(() => + new AzureOperationResponse() + { + Body = versionToReturn + } + ); + }); + + cmdlet.ResourceGroupName = resourceGroupName; + cmdlet.Name = deploymentName; + cmdlet.TemplateSpecId = templateSpecId; + + var dynamicParams = cmdlet.GetDynamicParameters() as RuntimeDefinedParameterDictionary; + + dynamicParams.Should().NotBeNull(); + dynamicParams.Count().Should().Be(template.Parameters.Count); + dynamicParams.Keys.Should().Contain(template.Parameters.Keys); + } } } diff --git a/src/Resources/Resources/ChangeLog.md b/src/Resources/Resources/ChangeLog.md index 4f4babacd9cd..56b742ee8ad1 100644 --- a/src/Resources/Resources/ChangeLog.md +++ b/src/Resources/Resources/ChangeLog.md @@ -23,6 +23,7 @@ * Fixed an issue where What-If shows two resource group scopes with different casing * Updated `Export-AzResourceGroup` to use the SDK. * Added culture info to parse methods +* Fixed issue where attempts to deploy template specs from a subscription outside of the current subscription context would fail * Changed Double parser for version parser ## Version 3.0.0