From e1d0e67c527e195b0b3aacfa9906f00dccc2ca96 Mon Sep 17 00:00:00 2001 From: Matt Hilton Date: Tue, 14 Jun 2022 16:48:55 +1000 Subject: [PATCH 1/9] Ensure HealthCheck respects configured proxy settings Fixes OctopusDeploy/Issues#7544 Updates the HealthCheck to use the new version of the Azure SDKs. It's in beta, so not touching the actual deployment code at this stage. --- source/Calamari/Azure/AzureClient.cs | 22 ++++++++++++++++++ .../Calamari/Azure/AzureKnownEnvironment.cs | 18 ++++++++++++--- source/Calamari/Calamari.csproj | 2 ++ source/Calamari/HealthCheckCommand.cs | 23 +++++++++++++------ 4 files changed, 55 insertions(+), 10 deletions(-) diff --git a/source/Calamari/Azure/AzureClient.cs b/source/Calamari/Azure/AzureClient.cs index 75e58216..3d208bfc 100644 --- a/source/Calamari/Azure/AzureClient.cs +++ b/source/Calamari/Azure/AzureClient.cs @@ -1,5 +1,8 @@ using System.Net; using System.Net.Http; +using Azure.Core.Pipeline; +using Azure.Identity; +using Azure.ResourceManager; using Microsoft.Azure.Management.Fluent; using Microsoft.Azure.Management.ResourceManager.Fluent; @@ -23,5 +26,24 @@ public static IAzure CreateAzureClient(this ServicePrincipalAccount servicePrinc .Authenticate(credentials) .WithSubscription(servicePrincipal.SubscriptionNumber); } + + /// + /// Creates an ArmClient for the new Azure SDK, which replaces the older fluent libraries. + /// We should migrate to this SDK once it stabilises. + /// + /// Service Principal Account to use when connecting to Azure + /// + public static ArmClient CreateArmClient(this ServicePrincipalAccount servicePrincipal) + { + var environment = new AzureKnownEnvironment(servicePrincipal.AzureEnvironment).AsAzureArmEnvironment(); + + var httpClientTransport = new HttpClientTransport(new HttpClientHandler { Proxy = WebRequest.DefaultWebProxy }); + + var tokenCredentialOptions = new TokenCredentialOptions { Transport = httpClientTransport }; + var credential = new ClientSecretCredential(servicePrincipal.TenantId, servicePrincipal.ClientId, servicePrincipal.Password, tokenCredentialOptions); + + var armClientOptions = new ArmClientOptions() { Transport = httpClientTransport, Environment = environment }; + return new ArmClient(credential, defaultSubscriptionId: servicePrincipal.SubscriptionNumber, armClientOptions); + } } } diff --git a/source/Calamari/Azure/AzureKnownEnvironment.cs b/source/Calamari/Azure/AzureKnownEnvironment.cs index 39cdac89..5fdd1212 100644 --- a/source/Calamari/Azure/AzureKnownEnvironment.cs +++ b/source/Calamari/Azure/AzureKnownEnvironment.cs @@ -1,4 +1,5 @@ using System; +using Azure.ResourceManager; using Microsoft.Azure.Management.ResourceManager.Fluent; namespace Calamari.Azure @@ -15,11 +16,11 @@ public AzureKnownEnvironment(string environment) if (string.IsNullOrEmpty(environment) || environment == "AzureCloud") // This environment name is defined in Sashimi.Azure.Accounts.AzureEnvironmentsListAction Value = Global.Value; // We interpret it as the normal Azure environment for historical reasons) - azureEnvironment = AzureEnvironment.FromName(Value) ?? + azureSdkEnvironment = AzureEnvironment.FromName(Value) ?? throw new InvalidOperationException($"Unknown environment name {Value}"); } - private readonly AzureEnvironment azureEnvironment; + private readonly AzureEnvironment azureSdkEnvironment; public string Value { get; } public static readonly AzureKnownEnvironment Global = new AzureKnownEnvironment("AzureGlobalCloud"); @@ -29,7 +30,18 @@ public AzureKnownEnvironment(string environment) public AzureEnvironment AsAzureSDKEnvironment() { - return azureEnvironment; + return azureSdkEnvironment; } + + public ArmEnvironment AsAzureArmEnvironment() => ToArmEnvironment(Value); + + private static ArmEnvironment ToArmEnvironment(string name) => name switch + { + "AzureGlobalCloud" => ArmEnvironment.AzurePublicCloud, + "AzureChinaCloud" => ArmEnvironment.AzureChina, + "AzureGermanCloud" => ArmEnvironment.AzureGermany, + "AzureUSGovernment" => ArmEnvironment.AzureGovernment, + _ => throw new InvalidOperationException($"ARM Environment {name} is not a known Azure Environment name.") + }; } } diff --git a/source/Calamari/Calamari.csproj b/source/Calamari/Calamari.csproj index eaa6d44a..4671f9c7 100644 --- a/source/Calamari/Calamari.csproj +++ b/source/Calamari/Calamari.csproj @@ -16,6 +16,8 @@ net5.0 + + diff --git a/source/Calamari/HealthCheckCommand.cs b/source/Calamari/HealthCheckCommand.cs index 7826298a..557a5d7c 100644 --- a/source/Calamari/HealthCheckCommand.cs +++ b/source/Calamari/HealthCheckCommand.cs @@ -1,10 +1,12 @@ using System; using System.Collections.Generic; using System.Threading.Tasks; +using Azure; +using Azure.ResourceManager.AppService; +using Azure.ResourceManager.Resources; using Calamari.Azure; using Calamari.Common.Commands; using Calamari.Common.Plumbing.Pipeline; -using Microsoft.Azure.Management.ResourceManager.Fluent; namespace Calamari.AzureAppService { @@ -34,14 +36,21 @@ public Task Execute(RunningDeployment context) return ConfirmWebAppExists(account, resourceGroupName, webAppName); } - async Task ConfirmWebAppExists(ServicePrincipalAccount servicePrincipal, string resourceGroupName, string siteAndSlotName) + private async Task ConfirmWebAppExists(ServicePrincipalAccount servicePrincipal, string resourceGroupName, string siteAndSlotName) { - var azureClient = servicePrincipal.CreateAzureClient(); - var webApp = await azureClient.WebApps.GetByResourceGroupAsync(resourceGroupName, siteAndSlotName); - if (webApp == null) + var client = servicePrincipal.CreateArmClient(); + var subscription = await client.GetDefaultSubscriptionAsync(); + var resourceGroups = subscription.GetResourceGroups(); + + try + { + ResourceGroupResource resourceGroup = await resourceGroups.GetAsync(resourceGroupName); + _ = await resourceGroup.GetWebSiteAsync(siteAndSlotName); + } + catch (RequestFailedException rfe) when (rfe.Status == 404) { - throw new Exception($"Could not find site {siteAndSlotName} in resource group {resourceGroupName}, using Service Principal with subscription {servicePrincipal.SubscriptionNumber}"); + throw new Exception($"Could not find site {siteAndSlotName} in resource group {resourceGroupName}, using Service Principal with subscription {servicePrincipal.SubscriptionNumber}", rfe); } } } -} \ No newline at end of file +} From 8ffb0ad7323d61c5db3804d66bb2057258a9fb64 Mon Sep 17 00:00:00 2001 From: Matt Hilton Date: Tue, 14 Jun 2022 17:14:44 +1000 Subject: [PATCH 2/9] Extract integration test base class to its own file --- .../AppServiceBehaviorFixture.cs | 103 ++--------------- .../AppServiceIntegrationTest.cs | 107 ++++++++++++++++++ 2 files changed, 114 insertions(+), 96 deletions(-) create mode 100644 source/Calamari.Tests/AppServiceIntegrationTest.cs diff --git a/source/Calamari.Tests/AppServiceBehaviorFixture.cs b/source/Calamari.Tests/AppServiceBehaviorFixture.cs index 8aa82536..375f6912 100644 --- a/source/Calamari.Tests/AppServiceBehaviorFixture.cs +++ b/source/Calamari.Tests/AppServiceBehaviorFixture.cs @@ -1,16 +1,12 @@ -using System; -using System.Collections.Generic; +using System.Collections.Generic; using System.IO; using System.IO.Compression; using System.Linq; -using System.Net.Http; using System.Reflection; using System.Text; using System.Threading.Tasks; using System.Xml; using System.Xml.Linq; -using Azure.Identity; -using Azure.ResourceManager.Resources; using Azure.ResourceManager.Resources.Models; using Calamari.Azure; using Calamari.Common.Plumbing.FileSystem; @@ -279,7 +275,7 @@ private static (string packagePath, string packageName, string packageVersion) P return packageInfo; } - private void AddVariables(CommandTestBuilderContext context) + private void AddAzureVariables(CommandTestBuilderContext context) { context.Variables.Add(AccountVariables.ClientId, clientId); context.Variables.Add(AccountVariables.Password, clientSecret); @@ -287,6 +283,11 @@ private void AddVariables(CommandTestBuilderContext context) context.Variables.Add(AccountVariables.SubscriptionId, subscriptionId); context.Variables.Add("Octopus.Action.Azure.ResourceGroupName", resourceGroupName); context.Variables.Add("Octopus.Action.Azure.WebAppName", site.Name); + } + + private void AddVariables(CommandTestBuilderContext context) + { + AddAzureVariables(context); context.Variables.Add("Greeting", greeting); context.Variables.Add(KnownVariables.Package.EnabledFeatures, KnownVariables.Features.SubstituteInFiles); context.Variables.Add(PackageVariables.SubstituteInFilesTargets, "index.html"); @@ -438,94 +439,4 @@ private void AddVariables(CommandTestBuilderContext context) } } } - - public abstract class AppServiceIntegrationTest - { - protected string clientId; - protected string clientSecret; - protected string tenantId; - protected string subscriptionId; - protected string resourceGroupName; - protected string resourceGroupLocation; - protected string greeting = "Calamari"; - protected string authToken; - protected WebSiteManagementClient webMgmtClient; - protected Site site; - - private ResourceGroupsOperations resourceGroupClient; - private readonly HttpClient client = new HttpClient(); - - [OneTimeSetUp] - public async Task Setup() - { - var resourceManagementEndpointBaseUri = - Environment.GetEnvironmentVariable(AccountVariables.ResourceManagementEndPoint) ?? - DefaultVariables.ResourceManagementEndpoint; - var activeDirectoryEndpointBaseUri = - Environment.GetEnvironmentVariable(AccountVariables.ActiveDirectoryEndPoint) ?? - DefaultVariables.ActiveDirectoryEndpoint; - - resourceGroupName = Guid.NewGuid().ToString(); - - clientId = ExternalVariables.Get(ExternalVariable.AzureSubscriptionClientId); - clientSecret = ExternalVariables.Get(ExternalVariable.AzureSubscriptionPassword); - tenantId = ExternalVariables.Get(ExternalVariable.AzureSubscriptionTenantId); - subscriptionId = ExternalVariables.Get(ExternalVariable.AzureSubscriptionId); - resourceGroupLocation = Environment.GetEnvironmentVariable("AZURE_NEW_RESOURCE_REGION") ?? "eastus"; - - authToken = await Auth.GetAuthTokenAsync(activeDirectoryEndpointBaseUri, resourceManagementEndpointBaseUri, - tenantId, clientId, clientSecret); - - var resourcesClient = new ResourcesManagementClient(subscriptionId, - new ClientSecretCredential(tenantId, clientId, clientSecret)); - - resourceGroupClient = resourcesClient.ResourceGroups; - - var resourceGroup = new ResourceGroup(resourceGroupLocation); - resourceGroup = await resourceGroupClient.CreateOrUpdateAsync(resourceGroupName, resourceGroup); - - webMgmtClient = new WebSiteManagementClient(new TokenCredentials(authToken)) - { - SubscriptionId = subscriptionId, - HttpClient = { BaseAddress = new Uri(DefaultVariables.ResourceManagementEndpoint) }, - }; - - await ConfigureTestResources(resourceGroup); - } - - protected abstract Task ConfigureTestResources(ResourceGroup resourceGroup); - - [OneTimeTearDown] - public async Task Cleanup() - { - if (resourceGroupClient != null) - await resourceGroupClient.StartDeleteAsync(resourceGroupName); - } - - protected async Task AssertContent(string hostName, string actualText, string rootPath = null) - { - var result = await client.GetStringAsync($"https://{hostName}/{rootPath}"); - - result.Should().Contain(actualText); - } - - protected static async Task DoWithRetries(int retries, Func action, int secondsBetweenRetries) - { - foreach (var retry in Enumerable.Range(1, retries)) - { - try - { - await action(); - break; - } - catch - { - if (retry == retries) - throw; - - await Task.Delay(secondsBetweenRetries * 1000); - } - } - } - } } diff --git a/source/Calamari.Tests/AppServiceIntegrationTest.cs b/source/Calamari.Tests/AppServiceIntegrationTest.cs new file mode 100644 index 00000000..f29b0277 --- /dev/null +++ b/source/Calamari.Tests/AppServiceIntegrationTest.cs @@ -0,0 +1,107 @@ +using System; +using System.Linq; +using System.Net.Http; +using System.Threading.Tasks; +using Azure.Identity; +using Azure.ResourceManager.Resources; +using Azure.ResourceManager.Resources.Models; +using Calamari.Azure; +using Calamari.Tests.Shared; +using FluentAssertions; +using Microsoft.Azure.Management.WebSites; +using Microsoft.Azure.Management.WebSites.Models; +using Microsoft.Rest; +using NUnit.Framework; + +namespace Calamari.AzureAppService.Tests +{ + public abstract class AppServiceIntegrationTest + { + protected string clientId; + protected string clientSecret; + protected string tenantId; + protected string subscriptionId; + protected string resourceGroupName; + protected string resourceGroupLocation; + protected string greeting = "Calamari"; + protected string authToken; + protected WebSiteManagementClient webMgmtClient; + protected Site site; + + private ResourceGroupsOperations resourceGroupClient; + private readonly HttpClient client = new HttpClient(); + + [OneTimeSetUp] + public async Task Setup() + { + var resourceManagementEndpointBaseUri = + Environment.GetEnvironmentVariable(AccountVariables.ResourceManagementEndPoint) ?? + DefaultVariables.ResourceManagementEndpoint; + var activeDirectoryEndpointBaseUri = + Environment.GetEnvironmentVariable(AccountVariables.ActiveDirectoryEndPoint) ?? + DefaultVariables.ActiveDirectoryEndpoint; + + resourceGroupName = Guid.NewGuid().ToString(); + + clientId = ExternalVariables.Get(ExternalVariable.AzureSubscriptionClientId); + clientSecret = ExternalVariables.Get(ExternalVariable.AzureSubscriptionPassword); + tenantId = ExternalVariables.Get(ExternalVariable.AzureSubscriptionTenantId); + subscriptionId = ExternalVariables.Get(ExternalVariable.AzureSubscriptionId); + resourceGroupLocation = Environment.GetEnvironmentVariable("AZURE_NEW_RESOURCE_REGION") ?? "eastus"; + + authToken = await Auth.GetAuthTokenAsync(activeDirectoryEndpointBaseUri, resourceManagementEndpointBaseUri, + tenantId, clientId, clientSecret); + + var resourcesClient = new ResourcesManagementClient(subscriptionId, + new ClientSecretCredential(tenantId, clientId, clientSecret)); + + resourceGroupClient = resourcesClient.ResourceGroups; + + var resourceGroup = new ResourceGroup(resourceGroupLocation); + resourceGroup = await resourceGroupClient.CreateOrUpdateAsync(resourceGroupName, resourceGroup); + + webMgmtClient = new WebSiteManagementClient(new TokenCredentials(authToken)) + { + SubscriptionId = subscriptionId, + HttpClient = { BaseAddress = new Uri(DefaultVariables.ResourceManagementEndpoint) }, + }; + + await ConfigureTestResources(resourceGroup); + } + + protected abstract Task ConfigureTestResources(ResourceGroup resourceGroup); + + [OneTimeTearDown] + public async Task Cleanup() + { + if (resourceGroupClient != null) + await resourceGroupClient.StartDeleteAsync(resourceGroupName); + } + + protected async Task AssertContent(string hostName, string actualText, string rootPath = null) + { + var result = await client.GetStringAsync($"https://{hostName}/{rootPath}"); + + result.Should().Contain(actualText); + } + + protected static async Task DoWithRetries(int retries, Func action, int secondsBetweenRetries) + { + foreach (var retry in Enumerable.Range(1, retries)) + { + try + { + await action(); + break; + } + catch + { + if (retry == retries) + throw; + + await Task.Delay(secondsBetweenRetries * 1000); + } + } + } + } +} \ No newline at end of file From 79ec1fa070834e545791094edf3c15ffe8c3d09b Mon Sep 17 00:00:00 2001 From: Matt Hilton Date: Tue, 14 Jun 2022 17:16:28 +1000 Subject: [PATCH 3/9] Pull common variable setup down to base class --- .../Calamari.Tests/AppServiceBehaviorFixture.cs | 17 +---------------- .../Calamari.Tests/AppServiceIntegrationTest.cs | 10 ++++++++++ 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/source/Calamari.Tests/AppServiceBehaviorFixture.cs b/source/Calamari.Tests/AppServiceBehaviorFixture.cs index 375f6912..b2b1a5e7 100644 --- a/source/Calamari.Tests/AppServiceBehaviorFixture.cs +++ b/source/Calamari.Tests/AppServiceBehaviorFixture.cs @@ -275,16 +275,6 @@ private static (string packagePath, string packageName, string packageVersion) P return packageInfo; } - private void AddAzureVariables(CommandTestBuilderContext context) - { - context.Variables.Add(AccountVariables.ClientId, clientId); - context.Variables.Add(AccountVariables.Password, clientSecret); - context.Variables.Add(AccountVariables.TenantId, tenantId); - context.Variables.Add(AccountVariables.SubscriptionId, subscriptionId); - context.Variables.Add("Octopus.Action.Azure.ResourceGroupName", resourceGroupName); - context.Variables.Add("Octopus.Action.Azure.WebAppName", site.Name); - } - private void AddVariables(CommandTestBuilderContext context) { AddAzureVariables(context); @@ -429,12 +419,7 @@ private static (string packagePath, string packageName, string packageVersion) P private void AddVariables(CommandTestBuilderContext context) { - context.Variables.Add(AccountVariables.ClientId, clientId); - context.Variables.Add(AccountVariables.Password, clientSecret); - context.Variables.Add(AccountVariables.TenantId, tenantId); - context.Variables.Add(AccountVariables.SubscriptionId, subscriptionId); - context.Variables.Add("Octopus.Action.Azure.ResourceGroupName", resourceGroupName); - context.Variables.Add("Octopus.Action.Azure.WebAppName", functionAppSiteName); + AddAzureVariables(context); context.Variables.Add(SpecialVariables.Action.Azure.DeploymentType, "ZipDeploy"); } } diff --git a/source/Calamari.Tests/AppServiceIntegrationTest.cs b/source/Calamari.Tests/AppServiceIntegrationTest.cs index f29b0277..e6bdba4e 100644 --- a/source/Calamari.Tests/AppServiceIntegrationTest.cs +++ b/source/Calamari.Tests/AppServiceIntegrationTest.cs @@ -103,5 +103,15 @@ protected static async Task DoWithRetries(int retries, Func action, int se } } } + + protected void AddAzureVariables(CommandTestBuilderContext context) + { + context.Variables.Add(AccountVariables.ClientId, clientId); + context.Variables.Add(AccountVariables.Password, clientSecret); + context.Variables.Add(AccountVariables.TenantId, tenantId); + context.Variables.Add(AccountVariables.SubscriptionId, subscriptionId); + context.Variables.Add("Octopus.Action.Azure.ResourceGroupName", resourceGroupName); + context.Variables.Add("Octopus.Action.Azure.WebAppName", site.Name); + } } } \ No newline at end of file From c8e62a9690cb1c74247f3186e3f049f2118adb2f Mon Sep 17 00:00:00 2001 From: Matt Hilton Date: Tue, 14 Jun 2022 18:53:41 +1000 Subject: [PATCH 4/9] Add test to ensure Health Check respects proxy settings --- ...eWebAppHealthCheckActionHandlerFixtures.cs | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs b/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs index 9fb1c74d..8864e1c9 100644 --- a/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs +++ b/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs @@ -1,7 +1,11 @@ #nullable disable +using System; +using System.IO; +using System.Net; using System.Threading.Tasks; using Calamari.Azure; using Calamari.AzureAppService; +using Calamari.Common.Plumbing.Proxies; using Calamari.Tests.Shared; using FluentAssertions; using Microsoft.Azure.Management.AppService.Fluent; @@ -9,7 +13,9 @@ using Microsoft.Azure.Management.ResourceManager.Fluent.Core; using NUnit.Framework; using Sashimi.Azure.Accounts; +using Sashimi.Server.Contracts.ActionHandlers; using Sashimi.Tests.Shared.Server; +using OperatingSystem = Microsoft.Azure.Management.AppService.Fluent.OperatingSystem; namespace Sashimi.AzureAppService.Tests { @@ -112,5 +118,97 @@ public void WebApp_Is_Not_Found() .WithAssert(result => result.WasSuccessful.Should().BeFalse()) .Execute(false); } + + /// + /// Configuring all the infrastructure required for a proper proxy test (with blocking certain addresses, proxy + /// server itself etc) is over the top for a test here. We can implicitly test that the proxy settings are being + /// picked up properly by setting a non-existent property, and ensuring that we fail with connectivity errors + /// *to the non-existent proxy* rather than a successful healthcheck directly to Azure. + /// + [Test] + public void ConfiguredProxy_IsUsedForHealthCheck() + { + // Arrange + var randomName = SdkContext.RandomResourceName(nameof(AzureWebAppHealthCheckActionHandlerFixtures), 60); + var clientId = ExternalVariables.Get(ExternalVariable.AzureSubscriptionClientId); + var clientSecret = ExternalVariables.Get(ExternalVariable.AzureSubscriptionPassword); + var tenantId = ExternalVariables.Get(ExternalVariable.AzureSubscriptionTenantId); + var subscriptionId = ExternalVariables.Get(ExternalVariable.AzureSubscriptionId); + + using var errorReader = new ConsoleErrorReader(); + using (new ProxySettings("non-existent-proxy.local", 3128)) + { + // Act + var result = ActionHandlerTestBuilder.CreateAsync() + .WithArrange(context => + { + context.Variables.Add(AccountVariables.SubscriptionId, subscriptionId); + context.Variables.Add(AccountVariables.TenantId, tenantId); + context.Variables.Add(AccountVariables.ClientId, clientId); + context.Variables.Add(AccountVariables.Password, clientSecret); + context.Variables.Add(SpecialVariables.Action.Azure.ResourceGroupName, randomName); + context.Variables.Add(SpecialVariables.Action.Azure.WebAppName, randomName); + context.Variables.Add(SpecialVariables.AccountType, AccountTypes.AzureServicePrincipalAccountType.ToString()); + }) + .Execute(false); + + // Assert + result.Outcome.Should().Be(ExecutionOutcome.Unsuccessful); + + var errors = errorReader.ReadErrors(); + errors.Should().Contain("No such host is known. (non-existent-proxy.local:3128)"); + } + } + + /// + /// Redirects StandardError to a local text reader for the lifetime of the ConsoleErrorReader object, + /// then allows reading what was written at any stage. + /// Original StandardError target is restored on dispose. + /// + class ConsoleErrorReader : IDisposable + { + private StringWriter stringWriter; + private TextWriter originalError; + + public ConsoleErrorReader() + { + originalError = Console.Error; + + stringWriter = new StringWriter(); + Console.SetError(stringWriter); + } + + public string ReadErrors() + { + return stringWriter.ToString(); + } + + public void Dispose() + { + Console.SetError(originalError); + } + } + + /// + /// Sets a DefaultWebProxy for the lifetime of the ProxySettings object. + /// Original DefaultWebProxy is restored on dispose. + /// + class ProxySettings : IDisposable + { + private IWebProxy? originalProxySettings; + + public ProxySettings(string hostname, int port) + { + originalProxySettings = WebRequest.DefaultWebProxy; + + var proxy = new UseCustomProxySettings(hostname, port, null, null); + WebRequest.DefaultWebProxy = proxy.CreateProxy().Value; + } + + public void Dispose() + { + WebRequest.DefaultWebProxy = originalProxySettings; + } + } } } \ No newline at end of file From b6673a193f23e7d7f35464d04ae7595281e2689d Mon Sep 17 00:00:00 2001 From: Matt Hilton Date: Wed, 15 Jun 2022 09:15:30 +1000 Subject: [PATCH 5/9] Ensure Proxy test works on both CI and locally --- ...eWebAppHealthCheckActionHandlerFixtures.cs | 106 ++++++++++++++---- 1 file changed, 85 insertions(+), 21 deletions(-) diff --git a/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs b/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs index 8864e1c9..33df6027 100644 --- a/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs +++ b/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs @@ -1,5 +1,6 @@ #nullable disable using System; +using System.Collections.Generic; using System.IO; using System.Net; using System.Threading.Tasks; @@ -135,8 +136,8 @@ public void ConfiguredProxy_IsUsedForHealthCheck() var tenantId = ExternalVariables.Get(ExternalVariable.AzureSubscriptionTenantId); var subscriptionId = ExternalVariables.Get(ExternalVariable.AzureSubscriptionId); - using var errorReader = new ConsoleErrorReader(); - using (new ProxySettings("non-existent-proxy.local", 3128)) + using (new ProxySettingsMemento("non-existent-proxy.local", 3128)) + using (var errorReader = new ConsoleErrorReaderMemento()) { // Act var result = ActionHandlerTestBuilder.CreateAsync() @@ -150,7 +151,7 @@ public void ConfiguredProxy_IsUsedForHealthCheck() context.Variables.Add(SpecialVariables.Action.Azure.WebAppName, randomName); context.Variables.Add(SpecialVariables.AccountType, AccountTypes.AzureServicePrincipalAccountType.ToString()); }) - .Execute(false); + .Execute(assertWasSuccess: false); // Assert result.Outcome.Should().Be(ExecutionOutcome.Unsuccessful); @@ -165,50 +166,113 @@ public void ConfiguredProxy_IsUsedForHealthCheck() /// then allows reading what was written at any stage. /// Original StandardError target is restored on dispose. /// - class ConsoleErrorReader : IDisposable + private class ConsoleErrorReaderMemento : AutoResetMemento { - private StringWriter stringWriter; - private TextWriter originalError; + private readonly StringWriter stringWriter; - public ConsoleErrorReader() + public ConsoleErrorReaderMemento() + : base(() => Console.Error, Console.SetError) { - originalError = Console.Error; - stringWriter = new StringWriter(); - Console.SetError(stringWriter); + base.SetValue(stringWriter); } public string ReadErrors() { return stringWriter.ToString(); } + } + + /// + /// Sets a Proxy Settings for the lifetime of the ProxySettingsMemento object. + /// Original settings are restored on dispose. + /// + /// + /// Calamari Tests operate differently on CI (out of process Calamari.exe) and locally (in-process code execution). + /// When in-process, the WebRequest.DefaultWebProxy is used. When out-of-process, the EnvVars are used. + /// + private class ProxySettingsMemento : IDisposable + { + private readonly List mementos; + + public ProxySettingsMemento(string hostname, int port) + { + mementos = new List(); + + SetLocalEnvironmentProxySettings(hostname, port); + SetCiEnvironmentProxySettings(hostname, port); + } + + private void SetLocalEnvironmentProxySettings(string hostname, int port) + { + var proxySettings = new UseCustomProxySettings(hostname, port, null!, null!).CreateProxy().Value; + mementos.Add(new AutoResetMemento(() => WebRequest.DefaultWebProxy, v => WebRequest.DefaultWebProxy = v, proxySettings)); + } + + private void SetCiEnvironmentProxySettings(string hostname, int port) + { + mementos.Add(new EnvironmentVariableMemento(EnvironmentVariables.TentacleProxyHost, hostname)); + mementos.Add(new EnvironmentVariableMemento(EnvironmentVariables.TentacleProxyPort, $"{port}")); + } public void Dispose() { - Console.SetError(originalError); + mementos.ForEach(m => m.Dispose()); + } + } + + /// + /// Sets an Environment Variable for the lifetime of the EnvironmentVariableMemento object. + /// Original setting is restored on dispose. + /// + private class EnvironmentVariableMemento : AutoResetMemento + { + public EnvironmentVariableMemento(string variableName, string testValue) + : base(() => Environment.GetEnvironmentVariable(variableName), + value => Environment.SetEnvironmentVariable(variableName, value), + testValue) + { } } /// - /// Sets a DefaultWebProxy for the lifetime of the ProxySettings object. - /// Original DefaultWebProxy is restored on dispose. + /// A not-quite implementation of the Memento pattern, which enables a single original value to be remembered, + /// temporarily overridden with a new value, and then automatically set back to the original value at the end + /// of the object's lifetime. + /// + /// Useful for managing potentially-shared state, though in a simplistic way. /// - class ProxySettings : IDisposable + /// + /// Does not attempt to handle concurrency: may produce unexpected results if multiple AutoResetMementos + /// are modifying the same shared state (eg Environment variables) across threads. + /// + /// + private class AutoResetMemento : IDisposable { - private IWebProxy? originalProxySettings; + private readonly TValue originalValue; + private readonly Action setter; - public ProxySettings(string hostname, int port) + public AutoResetMemento(Func getter, Action setter) { - originalProxySettings = WebRequest.DefaultWebProxy; + originalValue = getter(); + this.setter = setter; + } - var proxy = new UseCustomProxySettings(hostname, port, null, null); - WebRequest.DefaultWebProxy = proxy.CreateProxy().Value; + public AutoResetMemento(Func getter, Action setter, TValue newValue) + : this(getter, setter) + { + SetValue(newValue); + } + + public void SetValue(TValue value) + { + setter(value); } public void Dispose() { - WebRequest.DefaultWebProxy = originalProxySettings; + setter(originalValue); } } } -} \ No newline at end of file +} From 4cebf52143c0f5856c7e69ea213f1e11ffef41a5 Mon Sep 17 00:00:00 2001 From: Matt Hilton Date: Wed, 15 Jun 2022 09:35:18 +1000 Subject: [PATCH 6/9] Fixes another small difference in testing between CI and local --- .../AzureWebAppHealthCheckActionHandlerFixtures.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs b/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs index 33df6027..25c62028 100644 --- a/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs +++ b/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs @@ -156,8 +156,10 @@ public void ConfiguredProxy_IsUsedForHealthCheck() // Assert result.Outcome.Should().Be(ExecutionOutcome.Unsuccessful); - var errors = errorReader.ReadErrors(); - errors.Should().Contain("No such host is known. (non-existent-proxy.local:3128)"); + // This also operates differently locally vs on CI, so combine both StdErr and Calamari Log to get + // the full picture + var calamariOutput = result.FullLog + errorReader.ReadErrors(); + calamariOutput.Should().Contain("No such host is known. (non-existent-proxy.local:3128)"); } } From 25675f341e3b3f11a6cf41a4b88851a162d9d4d8 Mon Sep 17 00:00:00 2001 From: Matt Hilton Date: Wed, 15 Jun 2022 10:01:02 +1000 Subject: [PATCH 7/9] More tweaks to test for cross-OS assertion --- .../AzureWebAppHealthCheckActionHandlerFixtures.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs b/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs index 25c62028..283f14e4 100644 --- a/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs +++ b/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs @@ -159,7 +159,12 @@ public void ConfiguredProxy_IsUsedForHealthCheck() // This also operates differently locally vs on CI, so combine both StdErr and Calamari Log to get // the full picture var calamariOutput = result.FullLog + errorReader.ReadErrors(); - calamariOutput.Should().Contain("No such host is known. (non-existent-proxy.local:3128)"); + var windowsNetFxDnsError = "The remote name could not be resolved: 'non-existent-proxy.local'"; + var ubuntuDnsError = "Resource temporarily unavailable (non-existent-proxy.local:3128)"; + var generalLinuxDnsError = "Name or service not known (non-existent-proxy.local:3128)"; + var windowsDotNetDnsError = "No such host is known. (non-existent-proxy.local:3128)"; + + calamariOutput.Should().ContainAny(windowsDotNetDnsError, ubuntuDnsError,generalLinuxDnsError, windowsNetFxDnsError); } } From 0505497ebec342f93b3adf3f41805ed6593d3e0f Mon Sep 17 00:00:00 2001 From: Matt Hilton Date: Wed, 15 Jun 2022 10:49:41 +1000 Subject: [PATCH 8/9] Fix up refactor in AppServiceBehaviourFixture --- .../Calamari.Tests/AppServiceBehaviorFixture.cs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/source/Calamari.Tests/AppServiceBehaviorFixture.cs b/source/Calamari.Tests/AppServiceBehaviorFixture.cs index b2b1a5e7..946c484a 100644 --- a/source/Calamari.Tests/AppServiceBehaviorFixture.cs +++ b/source/Calamari.Tests/AppServiceBehaviorFixture.cs @@ -288,9 +288,6 @@ private void AddVariables(CommandTestBuilderContext context) [TestFixture] public class WhenUsingALinuxAppService : AppServiceIntegrationTest { - private string linuxServicePlanName; - private string functionAppSiteName; - protected override async Task ConfigureTestResources(ResourceGroup resourceGroup) { var storageClient = new StorageManagementClient(new TokenCredentials(authToken)) @@ -320,7 +317,7 @@ protected override async Task ConfigureTestResources(ResourceGroup resourceGroup } ); - var functionAppSite = await webMgmtClient.WebApps.BeginCreateOrUpdateAsync(resourceGroupName, + site = await webMgmtClient.WebApps.BeginCreateOrUpdateAsync(resourceGroupName, $"{resourceGroupName}-linux", new Site(resourceGroupLocation) { @@ -341,9 +338,6 @@ protected override async Task ConfigureTestResources(ResourceGroup resourceGroup } } ); - - linuxServicePlanName = linuxSvcPlan.Name; - functionAppSiteName = functionAppSite.Name; } [Test] @@ -362,7 +356,7 @@ await CommandTestBuilder.CreateAsync().Wi // Assert await DoWithRetries(10, async () => { - await AssertContent($"{functionAppSiteName}.azurewebsites.net", + await AssertContent($"{site.Name}.azurewebsites.net", rootPath: $"api/HttpExample?name={greeting}", actualText: $"Hello, {greeting}"); }, @@ -373,9 +367,9 @@ await AssertContent($"{functionAppSiteName}.azurewebsites.net", public async Task CanDeployZip_ToLinuxFunctionApp_WithRunFromPackageFlag() { // Arrange - var settings = await webMgmtClient.WebApps.ListApplicationSettingsAsync(resourceGroupName, functionAppSiteName); + var settings = await webMgmtClient.WebApps.ListApplicationSettingsAsync(resourceGroupName, site.Name); settings.Properties["WEBSITE_RUN_FROM_PACKAGE"] = "1"; - await webMgmtClient.WebApps.UpdateApplicationSettingsAsync(resourceGroupName, functionAppSiteName, settings); + await webMgmtClient.WebApps.UpdateApplicationSettingsAsync(resourceGroupName, site.Name, settings); var packageInfo = PrepareZipPackage(); @@ -389,7 +383,7 @@ await CommandTestBuilder.CreateAsync().Wi // Assert await DoWithRetries(10, async () => { - await AssertContent($"{functionAppSiteName}.azurewebsites.net", + await AssertContent($"{site.Name}.azurewebsites.net", rootPath: $"api/HttpExample?name={greeting}", actualText: $"Hello, {greeting}"); }, From af49e36cb71253145bb735b9f3ff91a3cdabaa62 Mon Sep 17 00:00:00 2001 From: Matt Hilton Date: Wed, 15 Jun 2022 13:56:07 +1000 Subject: [PATCH 9/9] Simplify Proxy test code --- .../AppServiceIntegrationTest.cs | 2 +- ...eWebAppHealthCheckActionHandlerFixtures.cs | 200 +++++++----------- 2 files changed, 77 insertions(+), 125 deletions(-) diff --git a/source/Calamari.Tests/AppServiceIntegrationTest.cs b/source/Calamari.Tests/AppServiceIntegrationTest.cs index e6bdba4e..c0d93769 100644 --- a/source/Calamari.Tests/AppServiceIntegrationTest.cs +++ b/source/Calamari.Tests/AppServiceIntegrationTest.cs @@ -114,4 +114,4 @@ protected void AddAzureVariables(CommandTestBuilderContext context) context.Variables.Add("Octopus.Action.Azure.WebAppName", site.Name); } } -} \ No newline at end of file +} diff --git a/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs b/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs index 283f14e4..bdb75ee1 100644 --- a/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs +++ b/source/Sashimi.Tests/AzureWebAppHealthCheckActionHandlerFixtures.cs @@ -119,6 +119,28 @@ public void WebApp_Is_Not_Found() .WithAssert(result => result.WasSuccessful.Should().BeFalse()) .Execute(false); } + } + + [TestFixture] + class AzureWebAppHealthCheckActionHandlerProxyFixture + { + private const string NonExistentProxyHostname = "non-existent-proxy.local"; + private const int NonExistentProxyPort = 3128; + + private IWebProxy? originalProxy; + private string originalProxyHost; + private string originalProxyPort; + + private StringWriter errorStream; + private TextWriter originalConsoleErrorOut; + + [SetUp] + public void SetUp() + { + SetLocalEnvironmentProxySettings(NonExistentProxyHostname, NonExistentProxyPort); + SetCiEnvironmentProxySettings(NonExistentProxyHostname, NonExistentProxyPort); + SetConsoleErrorOut(); + } /// /// Configuring all the infrastructure required for a proper proxy test (with blocking certain addresses, proxy @@ -136,150 +158,80 @@ public void ConfiguredProxy_IsUsedForHealthCheck() var tenantId = ExternalVariables.Get(ExternalVariable.AzureSubscriptionTenantId); var subscriptionId = ExternalVariables.Get(ExternalVariable.AzureSubscriptionId); - using (new ProxySettingsMemento("non-existent-proxy.local", 3128)) - using (var errorReader = new ConsoleErrorReaderMemento()) - { - // Act - var result = ActionHandlerTestBuilder.CreateAsync() - .WithArrange(context => - { - context.Variables.Add(AccountVariables.SubscriptionId, subscriptionId); - context.Variables.Add(AccountVariables.TenantId, tenantId); - context.Variables.Add(AccountVariables.ClientId, clientId); - context.Variables.Add(AccountVariables.Password, clientSecret); - context.Variables.Add(SpecialVariables.Action.Azure.ResourceGroupName, randomName); - context.Variables.Add(SpecialVariables.Action.Azure.WebAppName, randomName); - context.Variables.Add(SpecialVariables.AccountType, AccountTypes.AzureServicePrincipalAccountType.ToString()); - }) - .Execute(assertWasSuccess: false); + // Act + var result = ActionHandlerTestBuilder.CreateAsync() + .WithArrange(context => + { + context.Variables.Add(AccountVariables.SubscriptionId, subscriptionId); + context.Variables.Add(AccountVariables.TenantId, tenantId); + context.Variables.Add(AccountVariables.ClientId, clientId); + context.Variables.Add(AccountVariables.Password, clientSecret); + context.Variables.Add(SpecialVariables.Action.Azure.ResourceGroupName, randomName); + context.Variables.Add(SpecialVariables.Action.Azure.WebAppName, randomName); + context.Variables.Add(SpecialVariables.AccountType, AccountTypes.AzureServicePrincipalAccountType.ToString()); + }) + .Execute(assertWasSuccess: false); - // Assert - result.Outcome.Should().Be(ExecutionOutcome.Unsuccessful); + // Assert + result.Outcome.Should().Be(ExecutionOutcome.Unsuccessful); - // This also operates differently locally vs on CI, so combine both StdErr and Calamari Log to get - // the full picture - var calamariOutput = result.FullLog + errorReader.ReadErrors(); - var windowsNetFxDnsError = "The remote name could not be resolved: 'non-existent-proxy.local'"; - var ubuntuDnsError = "Resource temporarily unavailable (non-existent-proxy.local:3128)"; - var generalLinuxDnsError = "Name or service not known (non-existent-proxy.local:3128)"; - var windowsDotNetDnsError = "No such host is known. (non-existent-proxy.local:3128)"; + // This also operates differently locally vs on CI, so combine both StdErr and Calamari Log to get + // the full picture + var windowsNetFxDnsError = "The remote name could not be resolved: 'non-existent-proxy.local'"; + var ubuntuDnsError = "Resource temporarily unavailable (non-existent-proxy.local:3128)"; + var generalLinuxDnsError = "Name or service not known (non-existent-proxy.local:3128)"; + var windowsDotNetDnsError = "No such host is known. (non-existent-proxy.local:3128)"; - calamariOutput.Should().ContainAny(windowsDotNetDnsError, ubuntuDnsError,generalLinuxDnsError, windowsNetFxDnsError); - } + var calamariOutput = result.FullLog + errorStream; + calamariOutput.Should().ContainAny(windowsDotNetDnsError, ubuntuDnsError,generalLinuxDnsError, windowsNetFxDnsError); } - /// - /// Redirects StandardError to a local text reader for the lifetime of the ConsoleErrorReader object, - /// then allows reading what was written at any stage. - /// Original StandardError target is restored on dispose. - /// - private class ConsoleErrorReaderMemento : AutoResetMemento + [TearDown] + public void TearDown() { - private readonly StringWriter stringWriter; - - public ConsoleErrorReaderMemento() - : base(() => Console.Error, Console.SetError) - { - stringWriter = new StringWriter(); - base.SetValue(stringWriter); - } - - public string ReadErrors() - { - return stringWriter.ToString(); - } + RestoreLocalEnvironmentProxySettings(); + RestoreCiEnvironmentProxySettings(); + RestoreConsoleErrorOut(); } - /// - /// Sets a Proxy Settings for the lifetime of the ProxySettingsMemento object. - /// Original settings are restored on dispose. - /// - /// - /// Calamari Tests operate differently on CI (out of process Calamari.exe) and locally (in-process code execution). - /// When in-process, the WebRequest.DefaultWebProxy is used. When out-of-process, the EnvVars are used. - /// - private class ProxySettingsMemento : IDisposable + private void SetConsoleErrorOut() { - private readonly List mementos; - - public ProxySettingsMemento(string hostname, int port) - { - mementos = new List(); + originalConsoleErrorOut = Console.Error; + errorStream = new StringWriter(); + Console.SetError(errorStream); + } - SetLocalEnvironmentProxySettings(hostname, port); - SetCiEnvironmentProxySettings(hostname, port); - } + private void SetLocalEnvironmentProxySettings(string hostname, int port) + { + originalProxy = WebRequest.DefaultWebProxy; - private void SetLocalEnvironmentProxySettings(string hostname, int port) - { - var proxySettings = new UseCustomProxySettings(hostname, port, null!, null!).CreateProxy().Value; - mementos.Add(new AutoResetMemento(() => WebRequest.DefaultWebProxy, v => WebRequest.DefaultWebProxy = v, proxySettings)); - } + var proxySettings = new UseCustomProxySettings(hostname, port, null!, null!).CreateProxy().Value; + WebRequest.DefaultWebProxy = proxySettings; + } - private void SetCiEnvironmentProxySettings(string hostname, int port) - { - mementos.Add(new EnvironmentVariableMemento(EnvironmentVariables.TentacleProxyHost, hostname)); - mementos.Add(new EnvironmentVariableMemento(EnvironmentVariables.TentacleProxyPort, $"{port}")); - } + private void SetCiEnvironmentProxySettings(string hostname, int port) + { + originalProxyHost = Environment.GetEnvironmentVariable(EnvironmentVariables.TentacleProxyHost); + originalProxyPort = Environment.GetEnvironmentVariable(EnvironmentVariables.TentacleProxyPort); - public void Dispose() - { - mementos.ForEach(m => m.Dispose()); - } + Environment.SetEnvironmentVariable(EnvironmentVariables.TentacleProxyHost, hostname); + Environment.SetEnvironmentVariable(EnvironmentVariables.TentacleProxyPort, $"{port}"); } - /// - /// Sets an Environment Variable for the lifetime of the EnvironmentVariableMemento object. - /// Original setting is restored on dispose. - /// - private class EnvironmentVariableMemento : AutoResetMemento + private void RestoreConsoleErrorOut() { - public EnvironmentVariableMemento(string variableName, string testValue) - : base(() => Environment.GetEnvironmentVariable(variableName), - value => Environment.SetEnvironmentVariable(variableName, value), - testValue) - { - } + Console.SetError(originalConsoleErrorOut); } - /// - /// A not-quite implementation of the Memento pattern, which enables a single original value to be remembered, - /// temporarily overridden with a new value, and then automatically set back to the original value at the end - /// of the object's lifetime. - /// - /// Useful for managing potentially-shared state, though in a simplistic way. - /// - /// - /// Does not attempt to handle concurrency: may produce unexpected results if multiple AutoResetMementos - /// are modifying the same shared state (eg Environment variables) across threads. - /// - /// - private class AutoResetMemento : IDisposable + private void RestoreLocalEnvironmentProxySettings() { - private readonly TValue originalValue; - private readonly Action setter; - - public AutoResetMemento(Func getter, Action setter) - { - originalValue = getter(); - this.setter = setter; - } - - public AutoResetMemento(Func getter, Action setter, TValue newValue) - : this(getter, setter) - { - SetValue(newValue); - } - - public void SetValue(TValue value) - { - setter(value); - } + WebRequest.DefaultWebProxy = originalProxy; + } - public void Dispose() - { - setter(originalValue); - } + private void RestoreCiEnvironmentProxySettings() + { + Environment.SetEnvironmentVariable(EnvironmentVariables.TentacleProxyHost, originalProxyHost); + Environment.SetEnvironmentVariable(EnvironmentVariables.TentacleProxyPort, originalProxyPort); } } }