From 5b7f4c7c5d3d25296a6d31f1b1be38a8a0b6e9e4 Mon Sep 17 00:00:00 2001 From: Fabio Cavalcante Date: Tue, 1 Oct 2024 14:46:18 -0700 Subject: [PATCH] Small refactor and new tests (#3835) --- .../Actions/HostActions/StartHostAction.cs | 134 +++++++----------- src/Azure.Functions.Cli/Common/Constants.cs | 2 +- .../Common/DotnetConstants.cs | 2 + .../ActionsTests/StartHostActionTests.cs | 63 +++++++- .../E2E/StartTests.cs | 20 +-- 5 files changed, 127 insertions(+), 94 deletions(-) diff --git a/src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs b/src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs index 5e31d9023..7377f1aea 100644 --- a/src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs +++ b/src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs @@ -23,7 +23,6 @@ using Microsoft.Azure.WebJobs.Script.Configuration; using Microsoft.Azure.WebJobs.Script.Description; using Microsoft.Azure.WebJobs.Script.WebHost; -using Microsoft.Azure.WebJobs.Script.Workers; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; @@ -357,42 +356,21 @@ private void UpdateEnvironmentVariables(IDictionary secrets) /// /// Check local.settings.json to determine whether in-proc .NET8 is enabled. /// - private async Task IsInProcNet8Enabled() + private async Task IsInProcDotNet8Enabled() { var localSettingsJObject = await GetLocalSettingsJsonAsJObjectAsync(); - var inProcNet8EnabledSettingValue = localSettingsJObject?["Values"]?[Constants.FunctionsInProcNet8Enabled]?.Value(); - var isInProcNet8Enabled = string.Equals("1", inProcNet8EnabledSettingValue); + var inProcDotNet8EnabledValue = localSettingsJObject?["Values"]?[Constants.InProcDotNet8EnabledSetting]?.Value(); + var isInProcDotNet8Enabled = string.Equals("1", inProcDotNet8EnabledValue, StringComparison.Ordinal); if (VerboseLogging == true) { - var message = isInProcNet8Enabled - ? $"{Constants.FunctionsInProcNet8Enabled} app setting enabled in local.settings.json" - : $"{Constants.FunctionsInProcNet8Enabled} app setting is not enabled in local.settings.json"; + var message = isInProcDotNet8Enabled + ? $"{Constants.InProcDotNet8EnabledSetting} app setting enabled in local.settings.json" + : $"{Constants.InProcDotNet8EnabledSetting} app setting is not enabled in local.settings.json"; ColoredConsole.WriteLine(VerboseColor(message)); } - return isInProcNet8Enabled; - } - - // We launch the in-proc .NET8 application as a child process only if the SkipInProcessHost conditional compilation symbol is not defined. - // During build, we pass SkipInProcessHost=True only for artifacts used by our feed (we don't want to launch child process in that case). - private bool ShouldLaunchInProcNet8AsChildProcess() - { -#if SkipInProcessHost - if (VerboseLogging == true) - { - ColoredConsole.WriteLine(VerboseColor("SkipInProcessHost compilation symbol is defined.")); - } - - return false; -#else - if (VerboseLogging == true) - { - ColoredConsole.WriteLine(VerboseColor("SkipInProcessHost compilation symbol is not defined.")); - } - - return true; -#endif + return isInProcDotNet8Enabled; } private async Task GetLocalSettingsJsonAsJObjectAsync() @@ -423,8 +401,8 @@ public override async Task RunAsync() await PreRunConditions(); var isVerbose = VerboseLogging.HasValue && VerboseLogging.Value; - // Return if we have already started a child process - if (await ShouldExitAfterDeterminingHostRuntime(isVerbose)) + // Return if running is delegated to another version of Core Tools + if (await TryHandleInProcDotNetLaunchAsync()) { return; } @@ -482,15 +460,13 @@ public override async Task RunAsync() await runTask; } - private async Task ShouldExitAfterDeterminingHostRuntime(bool isVerbose) + private async Task TryHandleInProcDotNetLaunchAsync() { - var isInProc = GlobalCoreToolsSettings.CurrentWorkerRuntime == WorkerRuntime.dotnet; - // If --runtime param is set, handle runtime param logic; otherwise we infer the host to launch on startup if (HostRuntime is not null) { // Validate host runtime passed in - await ValidateHostRuntime(isInProc, isVerbose); + await ValidateHostRuntimeAsync(GlobalCoreToolsSettings.CurrentWorkerRuntime); if (!string.Equals(HostRuntime, "default", StringComparison.OrdinalIgnoreCase)) { @@ -498,82 +474,76 @@ private async Task ShouldExitAfterDeterminingHostRuntime(bool isVerbose) StartHostAsChildProcess(isNet8InProcSpecified); return true; } + + return false; } - else if (isInProc) + else if (GlobalCoreToolsSettings.CurrentWorkerRuntime == WorkerRuntime.dotnet) { // Infer host runtime by checking if .NET 8 is enabled - var shouldNet8InProcBeLaunched = await IsInProcNet8Enabled(); + var isDotNet8Project = await IsInProcDotNet8Enabled(); - if (shouldNet8InProcBeLaunched) - { - PrintVerboseForHostSelection(isVerbose, DotnetConstants.InProc8HostRuntime); - } - // Otherwise start .NET 6 child process since we are running an inproc app - else - { - PrintVerboseForHostSelection(isVerbose, DotnetConstants.InProc6HostRuntime); - } + var selectedRuntime = isDotNet8Project + ? DotnetConstants.InProc8HostRuntime + : DotnetConstants.InProc6HostRuntime; + + PrintVerboseForHostSelection(selectedRuntime); - StartHostAsChildProcess(shouldNet8InProcBeLaunched); - return true; + StartHostAsChildProcess(isDotNet8Project); + return true; } + // If the host runtime parameter is not set and the app is not in-proc, we should default to out-of-process host - PrintVerboseForHostSelection(isVerbose, "out-of-process"); + PrintVerboseForHostSelection("out-of-process"); return false; } - private async Task ValidateHostRuntime(bool isInProc, bool isVerbose) + internal async Task ValidateHostRuntimeAsync(WorkerRuntime currentWorkerRuntime, + Func> validateDotNet8ProjectEnablement = null) { - var isOutOfProc = GlobalCoreToolsSettings.CurrentWorkerRuntime == WorkerRuntime.dotnetIsolated; - var isInProc6 = string.Equals(HostRuntime, DotnetConstants.InProc6HostRuntime, StringComparison.OrdinalIgnoreCase); - var isInProc8 = string.Equals(HostRuntime, DotnetConstants.InProc8HostRuntime, StringComparison.OrdinalIgnoreCase); + validateDotNet8ProjectEnablement ??= IsInProcDotNet8Enabled; - // If we are running an .NET isolated app and the user specifies inproc6 or inproc8, throw an error - if (isOutOfProc && (isInProc8 || isInProc6)) + void ThrowCliException(string suffix) { - throw new CliException($"The runtime host value passed in, {HostRuntime}, is not a valid host version for your project. The host runtime is only valid for the worker runtime {WorkerRuntime.dotnet}"); + throw new CliException($"The runtime argument value provided, '{HostRuntime}', is invalid. {suffix}"); } - // If we are not running a .NET app and the user specifies inproc6 or inproc8, throw an error - if (!isOutOfProc && !isInProc && (isInProc8 || isInProc6)) + + if (DotnetConstants.ValidRuntimeValues.Contains(HostRuntime, StringComparer.OrdinalIgnoreCase) == false) { - throw new CliException($"The runtime host value passed in, {HostRuntime}, is not a valid host version for your project. The runtime is only valid for {WorkerRuntime.dotnetIsolated} and {WorkerRuntime.dotnet}"); + ThrowCliException($"Valid values are '{string.Join("', '", DotnetConstants.ValidRuntimeValues)}'."); } - if (string.Equals(HostRuntime, "default", StringComparison.OrdinalIgnoreCase)) + var isInproc6ArgumentValue = string.Equals(HostRuntime, DotnetConstants.InProc6HostRuntime, StringComparison.OrdinalIgnoreCase); + var isInproc8ArgumentValue = string.Equals(HostRuntime, DotnetConstants.InProc8HostRuntime, StringComparison.OrdinalIgnoreCase); + + if (currentWorkerRuntime == WorkerRuntime.dotnet) { - if (isInProc) + if (string.Equals(HostRuntime, "default", StringComparison.OrdinalIgnoreCase)) { - throw new CliException($"The runtime host value passed in, default, is not a valid host version for your project. For the default host runtime, the worker runtime must be set to {WorkerRuntime.dotnetIsolated}."); + ThrowCliException($"The provided value is only valid for the worker runtime '{WorkerRuntime.dotnetIsolated}'."); } - PrintVerboseForHostSelection(isVerbose, "out-of-process"); - return; - } - else if (isInProc8) - { - if (!await IsInProcNet8Enabled()) + + if (isInproc8ArgumentValue && !await validateDotNet8ProjectEnablement()) { - throw new CliException($"The runtime host value passed in, {DotnetConstants.InProc8HostRuntime}, is not a valid host version for your project. For the {DotnetConstants.InProc8HostRuntime} runtime, the {Constants.FunctionsInProcNet8Enabled} variable must be set while running a .NET 8 in-proc app."); + ThrowCliException($"For the '{DotnetConstants.InProc8HostRuntime}' runtime, the '{Constants.InProcDotNet8EnabledSetting}' environment variable must be set. See https://aka.ms/azure-functions/dotnet/net8-in-process."); } - PrintVerboseForHostSelection(isVerbose, DotnetConstants.InProc8HostRuntime); - return; - } - else if (isInProc6) - { - if (await IsInProcNet8Enabled()) + else if (isInproc6ArgumentValue && await validateDotNet8ProjectEnablement()) { - throw new CliException($"The runtime host value passed in, {DotnetConstants.InProc6HostRuntime}, is not a valid host version for your project. For the {DotnetConstants.InProc6HostRuntime} runtime, the {Constants.FunctionsInProcNet8Enabled} variable must not be set while running a .NET 6 in-proc app."); + ThrowCliException($"For the '{DotnetConstants.InProc6HostRuntime}' runtime, the '{Constants.InProcDotNet8EnabledSetting}' environment variable cannot be be set. See https://aka.ms/azure-functions/dotnet/net8-in-process."); } - PrintVerboseForHostSelection(isVerbose, DotnetConstants.InProc6HostRuntime); - return; + + } + else if (isInproc8ArgumentValue || isInproc6ArgumentValue) + { + ThrowCliException($"The provided value is only valid for the worker runtime '{WorkerRuntime.dotnet}'."); } - // Throw an exception if HostRuntime is set to none of the expected values - throw new CliException($"Invalid host runtime '{HostRuntime}'. Valid values are 'default', '{DotnetConstants.InProc8HostRuntime}', '{DotnetConstants.InProc6HostRuntime}'."); + + PrintVerboseForHostSelection(HostRuntime); } - private void PrintVerboseForHostSelection(bool isVerbose, string hostRuntime) + private void PrintVerboseForHostSelection(string hostRuntime) { - if (isVerbose) + if (VerboseLogging.GetValueOrDefault()) { ColoredConsole.WriteLine(VerboseColor($"Selected {hostRuntime} host.")); } diff --git a/src/Azure.Functions.Cli/Common/Constants.cs b/src/Azure.Functions.Cli/Common/Constants.cs index 52eb0b096..03c755c94 100644 --- a/src/Azure.Functions.Cli/Common/Constants.cs +++ b/src/Azure.Functions.Cli/Common/Constants.cs @@ -90,7 +90,7 @@ internal static class Constants public const string LocalSettingsJsonFileName = "local.settings.json"; public const string EnableWorkerIndexEnvironmentVariableName = "FunctionsHostingConfig__WORKER_INDEXING_ENABLED"; public const string Dotnet = "dotnet"; - public const string FunctionsInProcNet8Enabled = "FUNCTIONS_INPROC_NET8_ENABLED"; + public const string InProcDotNet8EnabledSetting = "FUNCTIONS_INPROC_NET8_ENABLED"; public const string AzureDevSessionsRemoteHostName = "AzureDevSessionsRemoteHostName"; public const string AzureDevSessionsPortSuffixPlaceholder = ""; // Sample format https://n12abc3t-.asse.devtunnels.ms/ diff --git a/src/Azure.Functions.Cli/Common/DotnetConstants.cs b/src/Azure.Functions.Cli/Common/DotnetConstants.cs index 8575ee976..d46022bdb 100644 --- a/src/Azure.Functions.Cli/Common/DotnetConstants.cs +++ b/src/Azure.Functions.Cli/Common/DotnetConstants.cs @@ -14,5 +14,7 @@ internal static class DotnetConstants public const string InProc6DirectoryName = "in-proc6"; public const string InProc8HostRuntime = "inproc8"; public const string InProc6HostRuntime = "inproc6"; + + public static readonly string[] ValidRuntimeValues = [InProc8HostRuntime, InProc6HostRuntime, "default"]; } } diff --git a/test/Azure.Functions.Cli.Tests/ActionsTests/StartHostActionTests.cs b/test/Azure.Functions.Cli.Tests/ActionsTests/StartHostActionTests.cs index 596274c84..70443e9c9 100644 --- a/test/Azure.Functions.Cli.Tests/ActionsTests/StartHostActionTests.cs +++ b/test/Azure.Functions.Cli.Tests/ActionsTests/StartHostActionTests.cs @@ -8,10 +8,14 @@ using System.Threading.Tasks; using Azure.Functions.Cli.Actions.HostActions; using Azure.Functions.Cli.Common; +using Azure.Functions.Cli.Helpers; +using Azure.Functions.Cli.Interfaces; using Colors.Net; using FluentAssertions; +using Moq; using NSubstitute; using Xunit; +using YamlDotNet.Core; namespace Azure.Functions.Cli.Tests.ActionsTests { @@ -44,7 +48,7 @@ public async Task CheckNonOptionalSettingsThrowsOnMissingAzureWebJobsStorageAndM exception.Should().NotBeNull(); exception.Should().BeOfType(); exception.Message.Should().Contain($"Missing value for AzureWebJobsStorage in local.settings.json. " + - $"This is required for all triggers other than {string.Join(", ", Constants.TriggersWithoutStorage)}."); + $"This is required for all triggers other than {string.Join(", ", Common.Constants.TriggersWithoutStorage)}."); } [Fact] @@ -186,6 +190,63 @@ private IFileSystem GetFakeFileSystem(IEnumerable<(string folder, string functio return fileSystem; } + [Theory] + // In-proc target, in-proc 8 argument, project configured for .NET 8. Succeeds. + [InlineData(WorkerRuntime.dotnet, DotnetConstants.InProc8HostRuntime, true, false)] + // In-proc target, in-proc 8 argument, project NOT configured for .NET 8. Fails. + [InlineData(WorkerRuntime.dotnet, DotnetConstants.InProc8HostRuntime, false, true)] + // In-proc target, in-proc 6 argument, project NOT configured for .NET 8. Succeeds. + [InlineData(WorkerRuntime.dotnet, DotnetConstants.InProc6HostRuntime, false, false)] + // In-proc target,'default' argument, project configured for .NET 8. Fails. + [InlineData(WorkerRuntime.dotnet, "default", true, true)] + // Isolated target,'default' argument, project NOT configured for .NET 8. Succeeds. + [InlineData(WorkerRuntime.dotnetIsolated, "default", true, false)] + // Isolated target,'default' argument, project configured for .NET 8. Succeeds. + [InlineData(WorkerRuntime.dotnetIsolated, "default", false, false)] + // Isolated target,in-proc 8 argument, project configured for .NET 8. Fails. + [InlineData(WorkerRuntime.dotnetIsolated, DotnetConstants.InProc8HostRuntime, true, true)] + // Isolated target,in-proc 6 argument, project not configured for .NET 8. Fails. + [InlineData(WorkerRuntime.dotnetIsolated, DotnetConstants.InProc6HostRuntime, false, true)] + // Unsupported runtime targets. + [InlineData(WorkerRuntime.dotnetIsolated, "somevalue", false, true)] + [InlineData(WorkerRuntime.dotnet, "somevalue", false, true)] + // Non .NET worker runtimes. + [InlineData(WorkerRuntime.python, "default", false, false)] + [InlineData(WorkerRuntime.java, "default", false, false)] + [InlineData(WorkerRuntime.node, "default", false, false)] + [InlineData(WorkerRuntime.python, DotnetConstants.InProc6HostRuntime, false, true)] + [InlineData(WorkerRuntime.java, DotnetConstants.InProc6HostRuntime, false, true)] + [InlineData(WorkerRuntime.node, DotnetConstants.InProc6HostRuntime, false, true)] + [InlineData(WorkerRuntime.python, DotnetConstants.InProc8HostRuntime, false, true)] + [InlineData(WorkerRuntime.java, DotnetConstants.InProc8HostRuntime, false, true)] + [InlineData(WorkerRuntime.node, DotnetConstants.InProc8HostRuntime, false, true)] + public async Task ValidateHostRuntimeAsync_MatchesExpectedResults(WorkerRuntime currentRuntime, string hostRuntimeArgument, bool validNet8Configuration, bool expectException) + { + try + { + Mock processManager = new(); + Mock secretsManager = new(); + + var startHostAction = new StartHostAction(secretsManager.Object, processManager.Object) + { + HostRuntime = hostRuntimeArgument + }; + + await startHostAction.ValidateHostRuntimeAsync(currentRuntime, () => Task.FromResult(validNet8Configuration)); + } + catch (CliException) + { + if (!expectException) + { + throw; + } + + return; + } + + Assert.False(expectException, "Expected validation failure."); + } + public void Dispose() { FileSystemHelpers.Instance = null; diff --git a/test/Azure.Functions.Cli.Tests/E2E/StartTests.cs b/test/Azure.Functions.Cli.Tests/E2E/StartTests.cs index 272d9d67a..e252f2cdc 100644 --- a/test/Azure.Functions.Cli.Tests/E2E/StartTests.cs +++ b/test/Azure.Functions.Cli.Tests/E2E/StartTests.cs @@ -635,7 +635,7 @@ await CliTester.Run(new RunConfiguration "start --port 7073 --verbose --runtime inproc6" }, ExpectExit = false, - ErrorContains = ["The runtime host value passed in, inproc6, is not a valid host version for your project. The host runtime is only valid for the worker runtime dotnet"], + ErrorContains = ["The runtime argument value provided, 'inproc6', is invalid. The provided value is only valid for the worker runtime 'dotnet'."], Test = async (workingDir, p) => { using (var client = new HttpClient() { BaseAddress = new Uri("http://localhost:7073") }) @@ -659,7 +659,7 @@ await CliTester.Run(new RunConfiguration "start --port 7073 --verbose --runtime inproc8" }, ExpectExit = false, - ErrorContains = ["The runtime host value passed in, inproc8, is not a valid host version for your project. The host runtime is only valid for the worker runtime dotnet"], + ErrorContains = ["The runtime argument value provided, 'inproc8', is invalid. The provided value is only valid for the worker runtime 'dotnet'."], Test = async (workingDir, p) => { using (var client = new HttpClient() { BaseAddress = new Uri("http://localhost:7073") }) @@ -683,7 +683,7 @@ await CliTester.Run(new RunConfiguration "start --port 7073 --verbose --runtime inproc8" }, ExpectExit = false, - ErrorContains = ["The runtime host value passed in, inproc8, is not a valid host version for your project. For the inproc8 runtime, the FUNCTIONS_INPROC_NET8_ENABLED variable must be set while running a .NET 8 in-proc app."], + ErrorContains = ["The runtime argument value provided, 'inproc8', is invalid. For the 'inproc8' runtime, the 'FUNCTIONS_INPROC_NET8_ENABLED' environment variable must be set. See https://aka.ms/azure-functions/dotnet/net8-in-process."], Test = async (workingDir, p) => { using (var client = new HttpClient() { BaseAddress = new Uri("http://localhost:7073") }) @@ -707,7 +707,7 @@ await CliTester.Run(new RunConfiguration "start --port 7073 --verbose --runtime default" }, ExpectExit = false, - ErrorContains = ["The runtime host value passed in, default, is not a valid host version for your project. For the default host runtime, the worker runtime must be set to dotnetIsolated."], + ErrorContains = ["The runtime argument value provided, 'default', is invalid. The provided value is only valid for the worker runtime 'dotnetIsolated'."], Test = async (workingDir, p) => { using (var client = new HttpClient() { BaseAddress = new Uri("http://localhost:7073") }) @@ -731,7 +731,7 @@ await CliTester.Run(new RunConfiguration "start --port 7073 --verbose --runtime default" }, ExpectExit = false, - ErrorContains = ["The runtime host value passed in, default, is not a valid host version for your project. For the default host runtime, the worker runtime must be set to dotnetIsolated."], + ErrorContains = ["The runtime argument value provided, 'default', is invalid. The provided value is only valid for the worker runtime 'dotnetIsolated'."], Test = async (workingDir, p) => { using (var client = new HttpClient() { BaseAddress = new Uri("http://localhost:7073") }) @@ -755,7 +755,7 @@ await CliTester.Run(new RunConfiguration "start --port 7073 --verbose --runtime inproc6" }, ExpectExit = false, - ErrorContains = ["The runtime host value passed in, inproc6, is not a valid host version for your project. For the inproc6 runtime, the FUNCTIONS_INPROC_NET8_ENABLED variable must not be set while running a .NET 6 in-proc app."], + ErrorContains = ["The runtime argument value provided, 'inproc6', is invalid. For the 'inproc6' runtime, the 'FUNCTIONS_INPROC_NET8_ENABLED' environment variable cannot be be set. See https://aka.ms/azure-functions/dotnet/net8-in-process."], Test = async (workingDir, p) => { using (var client = new HttpClient() { BaseAddress = new Uri("http://localhost:7073") }) @@ -779,7 +779,7 @@ await CliTester.Run(new RunConfiguration "start --port 7073 --verbose --runtime inproc6" }, ExpectExit = false, - ErrorContains = ["The runtime host value passed in, inproc6, is not a valid host version for your project. The runtime is only valid for dotnetIsolated and dotnet"], + ErrorContains = ["The runtime argument value provided, 'inproc6', is invalid. The provided value is only valid for the worker runtime 'dotnet'."], Test = async (workingDir, p) => { using (var client = new HttpClient() { BaseAddress = new Uri("http://localhost:7073") }) @@ -803,7 +803,7 @@ await CliTester.Run(new RunConfiguration "start --port 7073 --verbose --runtime inproc8" }, ExpectExit = false, - ErrorContains = ["The runtime host value passed in, inproc8, is not a valid host version for your project. The runtime is only valid for dotnetIsolated and dotnet"], + ErrorContains = ["The runtime argument value provided, 'inproc8', is invalid. The provided value is only valid for the worker runtime 'dotnet'."], Test = async (workingDir, p) => { using (var client = new HttpClient() { BaseAddress = new Uri("http://localhost:7073") }) @@ -866,7 +866,7 @@ await CliTester.Run(new RunConfiguration if (_output is Xunit.Sdk.TestOutputHelper testOutputHelper) { testOutputHelper.Output.Should().Contain("4.10"); - testOutputHelper.Output.Should().Contain("Selected out-of-process host."); + testOutputHelper.Output.Should().Contain("Selected default host."); } } }, @@ -898,7 +898,7 @@ await CliTester.Run(new RunConfiguration if (_output is Xunit.Sdk.TestOutputHelper testOutputHelper) { testOutputHelper.Output.Should().Contain("4.10"); - testOutputHelper.Output.Should().Contain("Selected out-of-process host."); + testOutputHelper.Output.Should().Contain("Selected default host."); } } },