From 72f1c9320bf5734b376fa346cf117890206225f5 Mon Sep 17 00:00:00 2001 From: Francisco-Gamino Date: Thu, 7 Sep 2023 23:57:35 -0700 Subject: [PATCH 1/7] * Add support to parsing command-line arguments prefix with functions- * Ignore unknown arguments * Remove deprecated option grpcMaxMessageLength --- src/Worker.cs | 67 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 54 insertions(+), 13 deletions(-) diff --git a/src/Worker.cs b/src/Worker.cs index d580e79a..317fe61f 100644 --- a/src/Worker.cs +++ b/src/Worker.cs @@ -31,10 +31,34 @@ public async static Task Main(string[] args) LogLevel.Information, string.Format(PowerShellWorkerStrings.PowerShellWorkerVersion, typeof(Worker).Assembly.GetName().Version)); - WorkerArguments arguments = null; - Parser.Default.ParseArguments(args) - .WithParsed(ops => arguments = ops) - .WithNotParsed(err => Environment.Exit(1)); + var workerOptions = new WorkerOptions(); + + var parser = new Parser(settings => settings.EnableDashDash = true); + parser.ParseArguments(args) + .WithParsed(workerArgs => + { + workerOptions.WorkerId = workerArgs.FunctionsWorkerId ?? workerArgs.WorkerId; + workerOptions.RequestId = workerArgs.FunctionsRequestId ?? workerArgs.RequestId; + + if (!string.IsNullOrWhiteSpace(workerArgs.FunctionsUri)) + { + try + { + var uri = new Uri(workerArgs.FunctionsUri); + workerOptions.Host = uri.Host; + workerOptions.Port = uri.Port; + } + catch (UriFormatException) + { + throw new ArgumentException("Invalid URI format", nameof(workerArgs.FunctionsUri)); + } + } + else + { + workerOptions.Host = workerArgs.Host; + workerOptions.Port = workerArgs.Port; + } + }); // Create the very first Runspace so the debugger has the target to attach to. // This PowerShell instance is shared by the first PowerShellManager instance created in the pool, @@ -44,14 +68,14 @@ public async static Task Main(string[] args) LogPowerShellVersion(pwshVersion); WarmUpPowerShell(firstPowerShellInstance); - var msgStream = new MessagingStream(arguments.Host, arguments.Port); + var msgStream = new MessagingStream(workerOptions.Host, workerOptions.Port); var requestProcessor = new RequestProcessor(msgStream, firstPowerShellInstance, pwshVersion); // Send StartStream message var startedMessage = new StreamingMessage() { - RequestId = arguments.RequestId, - StartStream = new StartStream() { WorkerId = arguments.WorkerId } + RequestId = workerOptions.RequestId, + StartStream = new StartStream() { WorkerId = workerOptions.WorkerId } }; msgStream.Write(startedMessage); @@ -85,19 +109,36 @@ private static void LogPowerShellVersion(string pwshVersion) internal class WorkerArguments { - [Option("host", Required = true, HelpText = "IP Address used to connect to the Host via gRPC.")] + [Option("host", Required = false, HelpText = "IP Address used to connect to the Host via gRPC.")] public string Host { get; set; } - [Option("port", Required = true, HelpText = "Port used to connect to the Host via gRPC.")] + [Option("port", Required = false, HelpText = "Port used to connect to the Host via gRPC.")] public int Port { get; set; } - [Option("workerId", Required = true, HelpText = "Worker ID assigned to this language worker.")] + [Option("workerId", Required = false, HelpText = "Worker ID assigned to this language worker.")] public string WorkerId { get; set; } - [Option("requestId", Required = true, HelpText = "Request ID used for gRPC communication with the Host.")] + [Option("requestId", Required = false, HelpText = "Request ID used for gRPC communication with the Host.")] public string RequestId { get; set; } - [Option("grpcMaxMessageLength", Required = false, HelpText = "[Deprecated and ignored] gRPC Maximum message size.")] - public int MaxMessageLength { get; set; } + [Option("functions-uri", Required = false, HelpText = "URI with IP Address and Port used to connect to the Host via gRPC.")] + public string FunctionsUri { get; set; } + + [Option("functions-workerid", Required = false, HelpText = "Worker ID assigned to this language worker.")] + public string FunctionsWorkerId { get; set; } + + [Option("functions-requestid", Required = false, HelpText = "Request ID used for gRPC communication with the Host.")] + public string FunctionsRequestId { get; set; } + } + + internal class WorkerOptions + { + public string Host { get; set; } + + public int Port { get; set; } + + public string WorkerId { get; set; } + + public string RequestId { get; set; } } } From 7c3eab7c7fe537d3405945846169b8b1cb4e3a50 Mon Sep 17 00:00:00 2001 From: Francisco-Gamino Date: Fri, 8 Sep 2023 00:51:36 -0700 Subject: [PATCH 2/7] Add back deprecated parser options --- src/Worker.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Worker.cs b/src/Worker.cs index 317fe61f..89367f57 100644 --- a/src/Worker.cs +++ b/src/Worker.cs @@ -121,6 +121,9 @@ internal class WorkerArguments [Option("requestId", Required = false, HelpText = "Request ID used for gRPC communication with the Host.")] public string RequestId { get; set; } + [Option("grpcMaxMessageLength", Required = false, HelpText = "[Deprecated and ignored] gRPC Maximum message size.")] + public int MaxMessageLength { get; set; } + [Option("functions-uri", Required = false, HelpText = "URI with IP Address and Port used to connect to the Host via gRPC.")] public string FunctionsUri { get; set; } @@ -129,6 +132,9 @@ internal class WorkerArguments [Option("functions-requestid", Required = false, HelpText = "Request ID used for gRPC communication with the Host.")] public string FunctionsRequestId { get; set; } + + [Option("functions-grpcmaxmessagelength", Required = false, HelpText = "[Deprecated and ignored] gRPC Maximum message size.")] + public int FunctionsMaxMessageLength { get; set; } } internal class WorkerOptions From e881b56f42012c8a42595202e84b6c4d7be14a9e Mon Sep 17 00:00:00 2001 From: Francisco-Gamino Date: Fri, 8 Sep 2023 12:09:58 -0700 Subject: [PATCH 3/7] Addressed PR comments --- src/Worker.cs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/Worker.cs b/src/Worker.cs index 89367f57..8768ad3a 100644 --- a/src/Worker.cs +++ b/src/Worker.cs @@ -37,6 +37,8 @@ public async static Task Main(string[] args) parser.ParseArguments(args) .WithParsed(workerArgs => { + // TODO: Remove parsing old command-line arguments that are not prefixed with functions- + // for more information, see https://github.com/Azure/azure-functions-powershell-worker/issues/995 workerOptions.WorkerId = workerArgs.FunctionsWorkerId ?? workerArgs.WorkerId; workerOptions.RequestId = workerArgs.FunctionsRequestId ?? workerArgs.RequestId; @@ -44,13 +46,15 @@ public async static Task Main(string[] args) { try { + // TODO: Update WorkerOptions to have a URI property instead of host name and port number + // for more information, see https://github.com/Azure/azure-functions-powershell-worker/issues/994 var uri = new Uri(workerArgs.FunctionsUri); workerOptions.Host = uri.Host; workerOptions.Port = uri.Port; } catch (UriFormatException) { - throw new ArgumentException("Invalid URI format", nameof(workerArgs.FunctionsUri)); + throw new ArgumentException($"Invalid URI format: {workerArgs.FunctionsUri}", nameof(workerArgs.FunctionsUri)); } } else @@ -58,6 +62,16 @@ public async static Task Main(string[] args) workerOptions.Host = workerArgs.Host; workerOptions.Port = workerArgs.Port; } + + // Validate workerOptions + ValidateProperty(workerOptions.WorkerId, "WorkerId"); + ValidateProperty(workerOptions.RequestId, "RequestId"); + ValidateProperty(workerOptions.Host, "Host"); + + if (workerOptions.Port <= 0) + { + throw new ArgumentException("Port has not been initialized", nameof(workerOptions.Port)); + } }); // Create the very first Runspace so the debugger has the target to attach to. @@ -105,6 +119,14 @@ private static void LogPowerShellVersion(string pwshVersion) var message = string.Format(PowerShellWorkerStrings.PowerShellVersion, pwshVersion); RpcLogger.WriteSystemLog(LogLevel.Information, message); } + + private static void ValidateProperty(string value, string propertyName) + { + if (string.IsNullOrWhiteSpace(value)) + { + throw new ArgumentException($"{propertyName} is null or empty", propertyName); + } + } } internal class WorkerArguments From b10d78d3cef0a427f1eaae17a2b5324e45373c38 Mon Sep 17 00:00:00 2001 From: Francisco-Gamino Date: Fri, 8 Sep 2023 13:35:44 -0700 Subject: [PATCH 4/7] Configure parser to ignore unknown arguments --- src/Worker.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Worker.cs b/src/Worker.cs index 8768ad3a..cc7198f5 100644 --- a/src/Worker.cs +++ b/src/Worker.cs @@ -33,7 +33,11 @@ public async static Task Main(string[] args) var workerOptions = new WorkerOptions(); - var parser = new Parser(settings => settings.EnableDashDash = true); + var parser = new Parser(settings => + { + settings.EnableDashDash = true; + settings.IgnoreUnknownArguments = true; + }); parser.ParseArguments(args) .WithParsed(workerArgs => { From 61d07ff7db65580ea07acdb0d4d7ce397d3499a0 Mon Sep 17 00:00:00 2001 From: Francisco-Gamino Date: Fri, 8 Sep 2023 13:37:18 -0700 Subject: [PATCH 5/7] Remove deprecated option grpcMaxMessageLength --- src/Worker.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/Worker.cs b/src/Worker.cs index cc7198f5..83f1bda3 100644 --- a/src/Worker.cs +++ b/src/Worker.cs @@ -147,9 +147,6 @@ internal class WorkerArguments [Option("requestId", Required = false, HelpText = "Request ID used for gRPC communication with the Host.")] public string RequestId { get; set; } - [Option("grpcMaxMessageLength", Required = false, HelpText = "[Deprecated and ignored] gRPC Maximum message size.")] - public int MaxMessageLength { get; set; } - [Option("functions-uri", Required = false, HelpText = "URI with IP Address and Port used to connect to the Host via gRPC.")] public string FunctionsUri { get; set; } @@ -158,9 +155,6 @@ internal class WorkerArguments [Option("functions-requestid", Required = false, HelpText = "Request ID used for gRPC communication with the Host.")] public string FunctionsRequestId { get; set; } - - [Option("functions-grpcmaxmessagelength", Required = false, HelpText = "[Deprecated and ignored] gRPC Maximum message size.")] - public int FunctionsMaxMessageLength { get; set; } } internal class WorkerOptions From d51dac79ad5a757decccc36cc4c9e8f6721262ce Mon Sep 17 00:00:00 2001 From: Francisco-Gamino Date: Fri, 8 Sep 2023 23:51:56 -0700 Subject: [PATCH 6/7] Addressed more PR comments --- src/Worker.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Worker.cs b/src/Worker.cs index 83f1bda3..fd029c36 100644 --- a/src/Worker.cs +++ b/src/Worker.cs @@ -56,9 +56,10 @@ public async static Task Main(string[] args) workerOptions.Host = uri.Host; workerOptions.Port = uri.Port; } - catch (UriFormatException) + catch (UriFormatException ex) { - throw new ArgumentException($"Invalid URI format: {workerArgs.FunctionsUri}", nameof(workerArgs.FunctionsUri)); + var message = $"Invalid URI format: {workerArgs.FunctionsUri}. Error message: {ex.Message}"; + throw new ArgumentException(message, nameof(workerArgs.FunctionsUri)); } } else From afc376b4eccf887f72f55679cf594a93ac71c4f2 Mon Sep 17 00:00:00 2001 From: Francisco-Gamino Date: Sat, 9 Sep 2023 10:33:42 -0700 Subject: [PATCH 7/7] Update variables' name and error message --- src/Worker.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Worker.cs b/src/Worker.cs index fd029c36..e11b501c 100644 --- a/src/Worker.cs +++ b/src/Worker.cs @@ -56,9 +56,9 @@ public async static Task Main(string[] args) workerOptions.Host = uri.Host; workerOptions.Port = uri.Port; } - catch (UriFormatException ex) + catch (UriFormatException formatEx) { - var message = $"Invalid URI format: {workerArgs.FunctionsUri}. Error message: {ex.Message}"; + var message = $"Invalid URI format: {workerArgs.FunctionsUri}. Error message: {formatEx.Message}"; throw new ArgumentException(message, nameof(workerArgs.FunctionsUri)); } } @@ -69,13 +69,13 @@ public async static Task Main(string[] args) } // Validate workerOptions - ValidateProperty(workerOptions.WorkerId, "WorkerId"); - ValidateProperty(workerOptions.RequestId, "RequestId"); - ValidateProperty(workerOptions.Host, "Host"); + ValidateProperty("WorkerId", workerOptions.WorkerId); + ValidateProperty("RequestId", workerOptions.RequestId); + ValidateProperty("Host", workerOptions.Host); if (workerOptions.Port <= 0) { - throw new ArgumentException("Port has not been initialized", nameof(workerOptions.Port)); + throw new ArgumentException("Port number has not been initialized", nameof(workerOptions.Port)); } }); @@ -125,11 +125,11 @@ private static void LogPowerShellVersion(string pwshVersion) RpcLogger.WriteSystemLog(LogLevel.Information, message); } - private static void ValidateProperty(string value, string propertyName) + private static void ValidateProperty(string name, string value) { if (string.IsNullOrWhiteSpace(value)) { - throw new ArgumentException($"{propertyName} is null or empty", propertyName); + throw new ArgumentException($"{name} is null or empty", name); } } }