diff --git a/src/Runner.Common/Util/NodeUtil.cs b/src/Runner.Common/Util/NodeUtil.cs index 27e124cd9..903a42c43 100644 --- a/src/Runner.Common/Util/NodeUtil.cs +++ b/src/Runner.Common/Util/NodeUtil.cs @@ -1,141 +1,102 @@ using System; using System.Collections.Generic; -using System.Collections.ObjectModel; +using GitHub.DistributedTask.WebApi; +using GitHub.Runner.Common.Util; +using Pipelines = GitHub.DistributedTask.Pipelines; +using GitHub.Runner.Common; +using GitHub.Runner.Sdk; -namespace GitHub.Runner.Common.Util +namespace GitHub.Runner.Worker.Handlers { - public static class NodeUtil + [ServiceLocator(Default = typeof(HandlerFactory))] + public interface IHandlerFactory : IRunnerService { - private const string _defaultNodeVersion = "node20"; - public static readonly ReadOnlyCollection BuiltInNodeVersions = new(new[] { "node20" }); - public static string GetInternalNodeVersion() + IHandler Create( + IExecutionContext executionContext, + Pipelines.ActionStepDefinitionReference action, + IStepHost stepHost, + ActionExecutionData data, + Dictionary inputs, + Dictionary environment, + Variables runtimeVariables, + string actionDirectory, + List localActionContainerSetupSteps); + } + + public sealed class HandlerFactory : RunnerService, IHandlerFactory + { + public IHandler Create( + IExecutionContext executionContext, + Pipelines.ActionStepDefinitionReference action, + IStepHost stepHost, + ActionExecutionData data, + Dictionary inputs, + Dictionary environment, + Variables runtimeVariables, + string actionDirectory, + List localActionContainerSetupSteps) { - var forcedInternalNodeVersion = Environment.GetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion); - var isForcedInternalNodeVersion = !string.IsNullOrEmpty(forcedInternalNodeVersion) && BuiltInNodeVersions.Contains(forcedInternalNodeVersion); + // Validate args. + Trace.Entering(); + ArgUtil.NotNull(executionContext, nameof(executionContext)); + ArgUtil.NotNull(stepHost, nameof(stepHost)); + ArgUtil.NotNull(data, nameof(data)); + ArgUtil.NotNull(inputs, nameof(inputs)); + ArgUtil.NotNull(environment, nameof(environment)); + ArgUtil.NotNull(runtimeVariables, nameof(runtimeVariables)); - if (isForcedInternalNodeVersion) + // Create the handler. + IHandler handler; + if (data.ExecutionType == ActionExecutionType.Container) { - return forcedInternalNodeVersion; + handler = HostContext.CreateService(); + (handler as IContainerActionHandler).Data = data as ContainerActionExecutionData; } - return _defaultNodeVersion; - } - /// - /// Determines the appropriate Node version for Actions to use - /// - /// Optional dictionary containing workflow-level environment variables - /// Feature flag indicating if Node 24 should be the default - /// Feature flag indicating if Node 24 is required - /// Optional callback for emitting warnings - /// The Node version to use (node20 or node24) and warning message if both env vars are set - public static (string nodeVersion, string warningMessage) DetermineActionsNodeVersion( - IDictionary workflowEnvironment = null, - bool useNode24ByDefault = false, - bool requireNode24 = false) - { - // Get effective values for the flags, with workflow taking precedence over system - bool forceNode24 = IsEnvironmentVariableTrue(Constants.Runner.NodeMigration.ForceNode24Variable, workflowEnvironment); - bool allowUnsecureNode = IsEnvironmentVariableTrue(Constants.Runner.NodeMigration.AllowUnsecureNodeVersionVariable, workflowEnvironment); - - string warningMessage = null; - - // Phase 3: Always use Node 24 regardless of environment variables - if (requireNode24) + else if (data.ExecutionType == ActionExecutionType.NodeJS) { - return (Constants.Runner.NodeMigration.Node24, null); - } + handler = HostContext.CreateService(); + var nodeData = data as NodeJSActionExecutionData; - // Check if both flags are set from the same source - bool bothFromWorkflow = false; - bool bothFromSystem = false; - - if (workflowEnvironment != null) - { - bool workflowHasForce = workflowEnvironment.TryGetValue(Constants.Runner.NodeMigration.ForceNode24Variable, out string forceValue) && - !string.IsNullOrEmpty(forceValue); - bool workflowHasAllow = workflowEnvironment.TryGetValue(Constants.Runner.NodeMigration.AllowUnsecureNodeVersionVariable, out string allowValue) && - !string.IsNullOrEmpty(allowValue); - - bothFromWorkflow = workflowHasForce && workflowHasAllow && - string.Equals(forceValue, "true", StringComparison.OrdinalIgnoreCase) && - string.Equals(allowValue, "true", StringComparison.OrdinalIgnoreCase); - } - - // Check if both are set in system and neither is overridden by workflow - string sysForce = Environment.GetEnvironmentVariable(Constants.Runner.NodeMigration.ForceNode24Variable); - string sysAllow = Environment.GetEnvironmentVariable(Constants.Runner.NodeMigration.AllowUnsecureNodeVersionVariable); - - bool systemHasForce = !string.IsNullOrEmpty(sysForce) && string.Equals(sysForce, "true", StringComparison.OrdinalIgnoreCase); - bool systemHasAllow = !string.IsNullOrEmpty(sysAllow) && string.Equals(sysAllow, "true", StringComparison.OrdinalIgnoreCase); - - // Both are set in system and not overridden by workflow - bothFromSystem = systemHasForce && systemHasAllow && - (workflowEnvironment == null || - (!workflowEnvironment.ContainsKey(Constants.Runner.NodeMigration.ForceNode24Variable) && - !workflowEnvironment.ContainsKey(Constants.Runner.NodeMigration.AllowUnsecureNodeVersionVariable))); - - // Handle the case when both are set in the same source - if ((bothFromWorkflow || bothFromSystem) && !requireNode24) - { - string defaultVersion = useNode24ByDefault ? Constants.Runner.NodeMigration.Node24 : Constants.Runner.NodeMigration.Node20; - warningMessage = $"Both {Constants.Runner.NodeMigration.ForceNode24Variable} and {Constants.Runner.NodeMigration.AllowUnsecureNodeVersionVariable} environment variables are set to true. This is likely a configuration error. Using the default Node version: {defaultVersion}."; - return (defaultVersion, warningMessage); - } - - // Phase 2: Node 24 is the default - if (useNode24ByDefault) - { - if (allowUnsecureNode) + // With node12 EoL in 04/2022 and node16 EoL in 09/23, we want to execute all JS actions using node20 + if (string.Equals(nodeData.NodeVersion, "node12", StringComparison.InvariantCultureIgnoreCase) || + string.Equals(nodeData.NodeVersion, "node16", StringComparison.InvariantCultureIgnoreCase)) { - return (Constants.Runner.NodeMigration.Node20, null); + nodeData.NodeVersion = "node20"; } - return (Constants.Runner.NodeMigration.Node24, null); + (handler as INodeScriptActionHandler).Data = nodeData; } - - // Phase 1: Node 20 is the default - if (forceNode24) + else if (data.ExecutionType == ActionExecutionType.Script) { - return (Constants.Runner.NodeMigration.Node24, null); + handler = HostContext.CreateService(); + (handler as IScriptHandler).Data = data as ScriptActionExecutionData; } - - return (Constants.Runner.NodeMigration.Node20, null); - } - - /// - /// Checks if Node24 is requested but running on ARM32 Linux, and determines if fallback is needed. - /// - /// The preferred Node version - /// A tuple containing the adjusted node version and an optional warning message - public static (string nodeVersion, string warningMessage) CheckNodeVersionForLinuxArm32(string preferredVersion) - { - if (string.Equals(preferredVersion, Constants.Runner.NodeMigration.Node24, StringComparison.OrdinalIgnoreCase) && - Constants.Runner.PlatformArchitecture.Equals(Constants.Architecture.Arm) && - Constants.Runner.Platform.Equals(Constants.OSPlatform.Linux)) + else if (data.ExecutionType == ActionExecutionType.Plugin) { - return (Constants.Runner.NodeMigration.Node20, "Node 24 is not supported on Linux ARM32 platforms. Falling back to Node 20."); + // Runner plugin + handler = HostContext.CreateService(); + (handler as IRunnerPluginHandler).Data = data as PluginActionExecutionData; } - - return (preferredVersion, null); - } - - /// - /// Checks if an environment variable is set to "true" in either the workflow environment or system environment - /// - /// The name of the environment variable - /// Optional dictionary containing workflow-level environment variables - /// True if the variable is set to "true" in either environment - private static bool IsEnvironmentVariableTrue(string variableName, IDictionary workflowEnvironment) - { - // Workflow environment variables take precedence over system environment variables - // If the workflow explicitly sets the value (even to false), we respect that over the system value - if (workflowEnvironment != null && workflowEnvironment.TryGetValue(variableName, out string workflowValue)) + else if (data.ExecutionType == ActionExecutionType.Composite) { - return string.Equals(workflowValue, "true", StringComparison.OrdinalIgnoreCase); + handler = HostContext.CreateService(); + (handler as ICompositeActionHandler).Data = data as CompositeActionExecutionData; + } + else + { + // This should never happen. + throw new NotSupportedException(data.ExecutionType.ToString()); } - // Fall back to system environment variable only if workflow doesn't specify this variable - string systemValue = Environment.GetEnvironmentVariable(variableName); - return string.Equals(systemValue, "true", StringComparison.OrdinalIgnoreCase); + handler.Action = action; + handler.Environment = environment; + handler.RuntimeVariables = runtimeVariables; + handler.ExecutionContext = executionContext; + handler.StepHost = stepHost; + handler.Inputs = inputs; + handler.ActionDirectory = actionDirectory; + handler.LocalActionContainerSetupSteps = localActionContainerSetupSteps; + return handler; } } -} +} \ No newline at end of file diff --git a/src/Runner.Worker/Handlers/HandlerFactory.cs b/src/Runner.Worker/Handlers/HandlerFactory.cs index 0f92b9bba..5ea9361cd 100644 --- a/src/Runner.Worker/Handlers/HandlerFactory.cs +++ b/src/Runner.Worker/Handlers/HandlerFactory.cs @@ -58,33 +58,10 @@ namespace GitHub.Runner.Worker.Handlers var nodeData = data as NodeJSActionExecutionData; // With node12 EoL in 04/2022 and node16 EoL in 09/23, we want to execute all JS actions using node20 - // With node20 EoL approaching, we're preparing to migrate to node24 if (string.Equals(nodeData.NodeVersion, "node12", StringComparison.InvariantCultureIgnoreCase) || string.Equals(nodeData.NodeVersion, "node16", StringComparison.InvariantCultureIgnoreCase)) { - nodeData.NodeVersion = Common.Constants.Runner.NodeMigration.Node20; - } - - // Check if node20 was explicitly specified in the action - // We don't modify if node24 was explicitly specified - if (string.Equals(nodeData.NodeVersion, Constants.Runner.NodeMigration.Node20, StringComparison.InvariantCultureIgnoreCase)) - { - bool useNode24ByDefault = true; - bool requireNode24 = FeatureManager.IsFeatureEnabled(executionContext.Global.Variables, Constants.Runner.NodeMigration.RequireNode24Flag); - - var (nodeVersion, configWarningMessage) = NodeUtil.DetermineActionsNodeVersion(environment, useNode24ByDefault, requireNode24); - var (finalNodeVersion, platformWarningMessage) = NodeUtil.CheckNodeVersionForLinuxArm32(nodeVersion); - nodeData.NodeVersion = finalNodeVersion; - - if (!string.IsNullOrEmpty(configWarningMessage)) - { - executionContext.Warning(configWarningMessage); - } - - if (!string.IsNullOrEmpty(platformWarningMessage)) - { - executionContext.Warning(platformWarningMessage); - } + nodeData.NodeVersion = "node20"; } (handler as INodeScriptActionHandler).Data = nodeData;