diff --git a/src/Runner.Common/Constants.cs b/src/Runner.Common/Constants.cs index 03d01b628..6c288eb2d 100644 --- a/src/Runner.Common/Constants.cs +++ b/src/Runner.Common/Constants.cs @@ -170,6 +170,22 @@ namespace GitHub.Runner.Common public static readonly string AddCheckRunIdToJobContext = "actions_add_check_run_id_to_job_context"; public static readonly string DisplayHelpfulActionsDownloadErrors = "actions_display_helpful_actions_download_errors"; } + + // Node version migration related constants + public static class NodeMigration + { + // Node versions + public static readonly string Node20 = "node20"; + public static readonly string Node24 = "node24"; + + // Environment variables for controlling node version selection + public static readonly string ForceNode24Variable = "FORCE_JAVASCRIPT_ACTIONS_TO_NODE24"; + public static readonly string AllowUnsecureNodeVersionVariable = "ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION"; + + // Feature flags for controlling the migration phases + public static readonly string UseNode24ByDefaultFlag = "actions.runner.usenode24bydefault"; + public static readonly string RequireNode24Flag = "actions.runner.requirenode24"; + } public static readonly string InternalTelemetryIssueDataKey = "_internal_telemetry"; public static readonly Guid TelemetryRecordId = new Guid("11111111-1111-1111-1111-111111111111"); diff --git a/src/Runner.Common/Util/NodeUtil.cs b/src/Runner.Common/Util/NodeUtil.cs index 65a3278fb..ff1a7a0af 100644 --- a/src/Runner.Common/Util/NodeUtil.cs +++ b/src/Runner.Common/Util/NodeUtil.cs @@ -1,10 +1,33 @@ using System; +using System.Collections.Generic; using System.Collections.ObjectModel; +using GitHub.Runner.Sdk; namespace GitHub.Runner.Common.Util { public static class NodeUtil { + /// + /// Represents details about an environment variable, including its value and source + /// + private class EnvironmentVariableInfo + { + /// + /// Gets or sets whether the value evaluates to true + /// + public bool IsTrue { get; set; } + + /// + /// Gets or sets whether the value came from the workflow environment + /// + public bool FromWorkflow { get; set; } + + /// + /// Gets or sets whether the value came from the system environment + /// + public bool FromSystem { get; set; } + } + private const string _defaultNodeVersion = "node20"; public static readonly ReadOnlyCollection BuiltInNodeVersions = new(new[] { "node20" }); public static string GetInternalNodeVersion() @@ -18,6 +41,70 @@ namespace GitHub.Runner.Common.Util } 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 + /// 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) + { + // Phase 3: Always use Node 24 regardless of environment variables + if (requireNode24) + { + return (Constants.Runner.NodeMigration.Node24, null); + } + + // Get environment variable details with source information + var forceNode24Details = GetEnvironmentVariableDetails( + Constants.Runner.NodeMigration.ForceNode24Variable, workflowEnvironment); + + var allowUnsecureNodeDetails = GetEnvironmentVariableDetails( + Constants.Runner.NodeMigration.AllowUnsecureNodeVersionVariable, workflowEnvironment); + + bool forceNode24 = forceNode24Details.IsTrue; + bool allowUnsecureNode = allowUnsecureNodeDetails.IsTrue; + string warningMessage = null; + + // Check if both flags are set from the same source + bool bothFromWorkflow = forceNode24Details.IsTrue && allowUnsecureNodeDetails.IsTrue && + forceNode24Details.FromWorkflow && allowUnsecureNodeDetails.FromWorkflow; + + bool bothFromSystem = forceNode24Details.IsTrue && allowUnsecureNodeDetails.IsTrue && + forceNode24Details.FromSystem && allowUnsecureNodeDetails.FromSystem; + + // Handle the case when both are set in the same source + if (bothFromWorkflow || bothFromSystem) + { + string source = bothFromWorkflow ? "workflow" : "system"; + 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 in the {source} environment. 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) + { + return (Constants.Runner.NodeMigration.Node20, null); + } + + return (Constants.Runner.NodeMigration.Node24, null); + } + + // Phase 1: Node 20 is the default + if (forceNode24) + { + return (Constants.Runner.NodeMigration.Node24, null); + } + + return (Constants.Runner.NodeMigration.Node20, null); + } /// /// Checks if Node24 is requested but running on ARM32 Linux, and determines if fallback is needed. @@ -26,14 +113,50 @@ namespace GitHub.Runner.Common.Util /// 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, "node24", StringComparison.OrdinalIgnoreCase) && + if (string.Equals(preferredVersion, Constants.Runner.NodeMigration.Node24, StringComparison.OrdinalIgnoreCase) && Constants.Runner.PlatformArchitecture.Equals(Constants.Architecture.Arm) && Constants.Runner.Platform.Equals(Constants.OSPlatform.Linux)) { - return ("node20", "Node 24 is not supported on Linux ARM32 platforms. Falling back to Node 20."); + return (Constants.Runner.NodeMigration.Node20, "Node 24 is not supported on Linux ARM32 platforms. Falling back to Node 20."); } return (preferredVersion, null); } + + /// + /// Gets detailed information about an environment variable from both workflow and system environments + /// + /// The name of the environment variable + /// Optional dictionary containing workflow-level environment variables + /// An EnvironmentVariableInfo object containing details about the variable from both sources + private static EnvironmentVariableInfo GetEnvironmentVariableDetails(string variableName, IDictionary workflowEnvironment) + { + var info = new EnvironmentVariableInfo(); + + // Check workflow environment + bool foundInWorkflow = false; + string workflowValue = null; + + if (workflowEnvironment != null && workflowEnvironment.TryGetValue(variableName, out workflowValue)) + { + foundInWorkflow = true; + info.FromWorkflow = true; + info.IsTrue = StringUtil.ConvertToBoolean(workflowValue); // Workflow value takes precedence for the boolean value + } + + // Also check system environment + string systemValue = Environment.GetEnvironmentVariable(variableName); + bool foundInSystem = !string.IsNullOrEmpty(systemValue); + + info.FromSystem = foundInSystem; + + // If not found in workflow, use system values + if (!foundInWorkflow) + { + info.IsTrue = StringUtil.ConvertToBoolean(systemValue); + } + + return info; + } } } diff --git a/src/Runner.Worker/Handlers/HandlerFactory.cs b/src/Runner.Worker/Handlers/HandlerFactory.cs index 5ea9361cd..ee022ec9d 100644 --- a/src/Runner.Worker/Handlers/HandlerFactory.cs +++ b/src/Runner.Worker/Handlers/HandlerFactory.cs @@ -58,10 +58,41 @@ 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 = "node20"; + 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 = executionContext.Global.Variables?.GetBoolean(Constants.Runner.NodeMigration.UseNode24ByDefaultFlag) ?? false; + bool requireNode24 = executionContext.Global.Variables?.GetBoolean(Constants.Runner.NodeMigration.RequireNode24Flag) ?? false; + + 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); + } + + // Show information about Node 24 migration in Phase 2 + if (useNode24ByDefault && !requireNode24 && string.Equals(finalNodeVersion, Constants.Runner.NodeMigration.Node24, StringComparison.OrdinalIgnoreCase)) + { + string infoMessage = "Node 20 is being deprecated. This workflow is running with Node 24 by default. " + + "If you need to temporarily use Node 20, you can set the ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION=true environment variable."; + executionContext.Output(infoMessage); + } } (handler as INodeScriptActionHandler).Data = nodeData; diff --git a/src/Test/L0/Util/NodeUtilL0.cs b/src/Test/L0/Util/NodeUtilL0.cs new file mode 100644 index 000000000..599d23514 --- /dev/null +++ b/src/Test/L0/Util/NodeUtilL0.cs @@ -0,0 +1,120 @@ +using System; +using System.Collections.Generic; +using GitHub.Runner.Common; +using GitHub.Runner.Common.Util; +using Xunit; + +namespace GitHub.Runner.Common.Tests.Util +{ + public class NodeUtilL0 + { + // We're testing the logic with feature flags + [Theory] + [InlineData(false, false, false, false, "node20", false)] // Phase 1: No env vars + [InlineData(false, false, false, true, "node20", false)] // Phase 1: Allow unsecure (redundant) + [InlineData(false, false, true, false, "node24", false)] // Phase 1: Force node24 + [InlineData(false, false, true, true, "node20", true)] // Phase 1: Both flags (use phase default + warning) + [InlineData(false, true, false, false, "node24", false)] // Phase 2: No env vars + [InlineData(false, true, false, true, "node20", false)] // Phase 2: Allow unsecure + [InlineData(false, true, true, false, "node24", false)] // Phase 2: Force node24 (redundant) + [InlineData(false, true, true, true, "node24", true)] // Phase 2: Both flags (use phase default + warning) + [InlineData(true, false, false, false, "node24", false)] // Phase 3: Always Node 24 regardless of env vars + [InlineData(true, false, false, true, "node24", false)] // Phase 3: Always Node 24 regardless of env vars + [InlineData(true, false, true, false, "node24", false)] // Phase 3: Always Node 24 regardless of env vars + [InlineData(true, false, true, true, "node24", false)] // Phase 3: Always Node 24 regardless of env vars, no warnings in Phase 3 + public void TestNodeVersionLogic(bool requireNode24, bool useNode24ByDefault, bool forceNode24, bool allowUnsecureNode, string expectedVersion, bool expectWarning) + { + try + { + Environment.SetEnvironmentVariable(Constants.Runner.NodeMigration.ForceNode24Variable, forceNode24 ? "true" : null); + Environment.SetEnvironmentVariable(Constants.Runner.NodeMigration.AllowUnsecureNodeVersionVariable, allowUnsecureNode ? "true" : null); + + // Call the actual method + var (actualVersion, warningMessage) = NodeUtil.DetermineActionsNodeVersion(null, useNode24ByDefault, requireNode24); + + // Assert + Assert.Equal(expectedVersion, actualVersion); + + if (expectWarning) + { + Assert.NotNull(warningMessage); + Assert.Contains("Both", warningMessage); + Assert.Contains("are set to true", warningMessage); + } + else + { + Assert.Null(warningMessage); + } + } + finally + { + // Cleanup + Environment.SetEnvironmentVariable(Constants.Runner.NodeMigration.ForceNode24Variable, null); + Environment.SetEnvironmentVariable(Constants.Runner.NodeMigration.AllowUnsecureNodeVersionVariable, null); + } + } + + [Theory] + [InlineData(false, false, false, false, false, true, "node20", false)] // Phase 1: System env: none, Workflow env: allow=true + [InlineData(false, false, true, false, false, false, "node24", false)] // Phase 1: System env: force node24, Workflow env: none + [InlineData(false, true, false, false, true, false, "node24", false)] // Phase 1: System env: none, Workflow env: force node24 + [InlineData(false, false, false, true, false, true, "node20", false)] // Phase 1: System env: allow=true, Workflow env: allow=true (workflow takes precedence) + [InlineData(false, false, true, true, false, false, "node20", true)] // Phase 1: System env: both true, Workflow env: none (use phase default + warning) + [InlineData(false, false, false, false, true, true, "node20", true)] // Phase 1: System env: none, Workflow env: both (use phase default + warning) + [InlineData(true, false, false, false, false, false, "node24", false)] // Phase 2: System env: none, Workflow env: none + [InlineData(true, false, false, true, false, false, "node20", false)] // Phase 2: System env: allow=true, Workflow env: none + [InlineData(true, false, false, false, false, true, "node20", false)] // Phase 2: System env: none, Workflow env: allow unsecure + [InlineData(true, false, true, false, false, true, "node20", false)] // Phase 2: System env: force node24, Workflow env: allow unsecure + [InlineData(true, false, true, true, false, false, "node24", true)] // Phase 2: System env: both true, Workflow env: none (use phase default + warning) + [InlineData(true, false, false, false, true, true, "node24", true)] // Phase 2: System env: none, Workflow env: both (phase default + warning) + [InlineData(false, true, false, false, false, true, "node24", false)] // Phase 3: System env: none, Workflow env: allow=true (always Node 24 in Phase 3) + [InlineData(false, true, true, true, false, false, "node24", false)] // Phase 3: System env: both true, Workflow env: none (always Node 24 in Phase 3, no warning) + [InlineData(false, true, false, false, true, true, "node24", false)] // Phase 3: System env: none, Workflow env: both (always Node 24 in Phase 3, no warning) + public void TestNodeVersionLogicWithWorkflowEnvironment(bool useNode24ByDefault, bool requireNode24, + bool systemForceNode24, bool systemAllowUnsecure, + bool workflowForceNode24, bool workflowAllowUnsecure, + string expectedVersion, bool expectWarning) + { + try + { + // Set system environment variables + Environment.SetEnvironmentVariable(Constants.Runner.NodeMigration.ForceNode24Variable, systemForceNode24 ? "true" : null); + Environment.SetEnvironmentVariable(Constants.Runner.NodeMigration.AllowUnsecureNodeVersionVariable, systemAllowUnsecure ? "true" : null); + + // Set workflow environment variables + var workflowEnv = new Dictionary(); + if (workflowForceNode24) + { + workflowEnv[Constants.Runner.NodeMigration.ForceNode24Variable] = "true"; + } + if (workflowAllowUnsecure) + { + workflowEnv[Constants.Runner.NodeMigration.AllowUnsecureNodeVersionVariable] = "true"; + } + + // Call the actual method with our test parameters + var (actualVersion, warningMessage) = NodeUtil.DetermineActionsNodeVersion(workflowEnv, useNode24ByDefault, requireNode24); + + // Assert + Assert.Equal(expectedVersion, actualVersion); + + if (expectWarning) + { + Assert.NotNull(warningMessage); + Assert.Contains("Both", warningMessage); + Assert.Contains("are set to true", warningMessage); + } + else + { + Assert.Null(warningMessage); + } + } + finally + { + // Cleanup + Environment.SetEnvironmentVariable(Constants.Runner.NodeMigration.ForceNode24Variable, null); + Environment.SetEnvironmentVariable(Constants.Runner.NodeMigration.AllowUnsecureNodeVersionVariable, null); + } + } + } +}