Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/Runner.Common/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,22 @@ public static class Features
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";
Comment thread
TingluoHuang marked this conversation as resolved.
Resolved
}

public static readonly string InternalTelemetryIssueDataKey = "_internal_telemetry";
public static readonly Guid TelemetryRecordId = new Guid("11111111-1111-1111-1111-111111111111");
Expand Down
105 changes: 103 additions & 2 deletions src/Runner.Common/Util/NodeUtil.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;

namespace GitHub.Runner.Common.Util
Expand All @@ -18,6 +19,86 @@ public static string GetInternalNodeVersion()
}
return _defaultNodeVersion;
}
/// <summary>
/// Determines the appropriate Node version for Actions to use
/// </summary>
/// <param name="workflowEnvironment">Optional dictionary containing workflow-level environment variables</param>
/// <param name="useNode24ByDefault">Feature flag indicating if Node 24 should be the default</param>
/// <param name="requireNode24">Feature flag indicating if Node 24 is required</param>
/// <returns>The Node version to use (node20 or node24) and warning message if both env vars are set</returns>
public static (string nodeVersion, string warningMessage) DetermineActionsNodeVersion(
IDictionary<string, string> 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
Comment thread
TingluoHuang marked this conversation as resolved.
Resolved
if (requireNode24)
{
return (Constants.Runner.NodeMigration.Node24, null);
}

// 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) &&
Comment thread
TingluoHuang marked this conversation as resolved.
Resolved
!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);
Comment thread
TingluoHuang marked this conversation as resolved.
Resolved
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)
{
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);
}

/// <summary>
/// Checks if Node24 is requested but running on ARM32 Linux, and determines if fallback is needed.
Expand All @@ -26,14 +107,34 @@ public static string GetInternalNodeVersion()
/// <returns>A tuple containing the adjusted node version and an optional warning message</returns>
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);
}

/// <summary>
/// Checks if an environment variable is set to "true" in either the workflow environment or system environment
/// </summary>
/// <param name="variableName">The name of the environment variable</param>
/// <param name="workflowEnvironment">Optional dictionary containing workflow-level environment variables</param>
/// <returns>True if the variable is set to "true" in either environment</returns>
private static bool IsEnvironmentVariableTrue(string variableName, IDictionary<string, string> 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))
{
return string.Equals(workflowValue, "true", StringComparison.OrdinalIgnoreCase);
Comment thread
salmanmkc marked this conversation as resolved.
Resolved
}

// 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);
}
}
}
15 changes: 15 additions & 0 deletions src/Runner.Worker/FeatureManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,20 @@ public static bool IsContainerHooksEnabled(Variables variables)
var isContainerHooksPathSet = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable(Constants.Hooks.ContainerHooksPath));
return isContainerHookFeatureFlagSet && isContainerHooksPathSet;
}

public static bool IsFeatureEnabled(Variables variables, string featureFlag)
{
return variables?.GetBoolean(featureFlag) ?? false;
}

public static bool IsUseNode24ByDefaultEnabled(Variables variables)
Comment thread
TingluoHuang marked this conversation as resolved.
Resolved
{
return IsFeatureEnabled(variables, Constants.Runner.NodeMigration.UseNode24ByDefaultFlag);
}

public static bool IsRequireNode24Enabled(Variables variables)
{
return IsFeatureEnabled(variables, Constants.Runner.NodeMigration.RequireNode24Flag);
}
}
}
33 changes: 32 additions & 1 deletion src/Runner.Worker/Handlers/HandlerFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,41 @@ public IHandler Create(
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 = FeatureManager.IsFeatureEnabled(executionContext.Global.Variables, Constants.Runner.NodeMigration.UseNode24ByDefaultFlag);
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);
}

// Show information about Node 24 migration in Phase 2
if (useNode24ByDefault && !requireNode24 && string.Equals(finalNodeVersion, Constants.Runner.NodeMigration.Node24, StringComparison.OrdinalIgnoreCase))
Comment thread
TingluoHuang marked this conversation as resolved.
Resolved
{
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;
Expand Down
Loading