Skip to content

Commit 0203cf2

Browse files
salmanmkcsalmanmkc
authored
Node 20 -> Node 24 migration feature flagging, opt-in and opt-out environment variables (#3948)
1 parent 5e74a4d commit 0203cf2

4 files changed

Lines changed: 293 additions & 3 deletions

File tree

‎src/Runner.Common/Constants.cs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,22 @@ public static class Features
170170
public static readonly string AddCheckRunIdToJobContext = "actions_add_check_run_id_to_job_context";
171171
public static readonly string DisplayHelpfulActionsDownloadErrors = "actions_display_helpful_actions_download_errors";
172172
}
173+
174+
// Node version migration related constants
175+
public static class NodeMigration
176+
{
177+
// Node versions
178+
public static readonly string Node20 = "node20";
179+
public static readonly string Node24 = "node24";
180+
181+
// Environment variables for controlling node version selection
182+
public static readonly string ForceNode24Variable = "FORCE_JAVASCRIPT_ACTIONS_TO_NODE24";
183+
public static readonly string AllowUnsecureNodeVersionVariable = "ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION";
184+
185+
// Feature flags for controlling the migration phases
186+
public static readonly string UseNode24ByDefaultFlag = "actions.runner.usenode24bydefault";
187+
public static readonly string RequireNode24Flag = "actions.runner.requirenode24";
188+
}
173189

174190
public static readonly string InternalTelemetryIssueDataKey = "_internal_telemetry";
175191
public static readonly Guid TelemetryRecordId = new Guid("11111111-1111-1111-1111-111111111111");

‎src/Runner.Common/Util/NodeUtil.cs‎

Lines changed: 125 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,33 @@
11
using System;
2+
using System.Collections.Generic;
23
using System.Collections.ObjectModel;
4+
using GitHub.Runner.Sdk;
35

46
namespace GitHub.Runner.Common.Util
57
{
68
public static class NodeUtil
79
{
10+
/// <summary>
11+
/// Represents details about an environment variable, including its value and source
12+
/// </summary>
13+
private class EnvironmentVariableInfo
14+
{
15+
/// <summary>
16+
/// Gets or sets whether the value evaluates to true
17+
/// </summary>
18+
public bool IsTrue { get; set; }
19+
20+
/// <summary>
21+
/// Gets or sets whether the value came from the workflow environment
22+
/// </summary>
23+
public bool FromWorkflow { get; set; }
24+
25+
/// <summary>
26+
/// Gets or sets whether the value came from the system environment
27+
/// </summary>
28+
public bool FromSystem { get; set; }
29+
}
30+
831
private const string _defaultNodeVersion = "node20";
932
public static readonly ReadOnlyCollection<string> BuiltInNodeVersions = new(new[] { "node20" });
1033
public static string GetInternalNodeVersion()
@@ -18,6 +41,70 @@ public static string GetInternalNodeVersion()
1841
}
1942
return _defaultNodeVersion;
2043
}
44+
/// <summary>
45+
/// Determines the appropriate Node version for Actions to use
46+
/// </summary>
47+
/// <param name="workflowEnvironment">Optional dictionary containing workflow-level environment variables</param>
48+
/// <param name="useNode24ByDefault">Feature flag indicating if Node 24 should be the default</param>
49+
/// <param name="requireNode24">Feature flag indicating if Node 24 is required</param>
50+
/// <returns>The Node version to use (node20 or node24) and warning message if both env vars are set</returns>
51+
public static (string nodeVersion, string warningMessage) DetermineActionsNodeVersion(
52+
IDictionary<string, string> workflowEnvironment = null,
53+
bool useNode24ByDefault = false,
54+
bool requireNode24 = false)
55+
{
56+
// Phase 3: Always use Node 24 regardless of environment variables
57+
if (requireNode24)
58+
{
59+
return (Constants.Runner.NodeMigration.Node24, null);
60+
}
61+
62+
// Get environment variable details with source information
63+
var forceNode24Details = GetEnvironmentVariableDetails(
64+
Constants.Runner.NodeMigration.ForceNode24Variable, workflowEnvironment);
65+
66+
var allowUnsecureNodeDetails = GetEnvironmentVariableDetails(
67+
Constants.Runner.NodeMigration.AllowUnsecureNodeVersionVariable, workflowEnvironment);
68+
69+
bool forceNode24 = forceNode24Details.IsTrue;
70+
bool allowUnsecureNode = allowUnsecureNodeDetails.IsTrue;
71+
string warningMessage = null;
72+
73+
// Check if both flags are set from the same source
74+
bool bothFromWorkflow = forceNode24Details.IsTrue && allowUnsecureNodeDetails.IsTrue &&
75+
forceNode24Details.FromWorkflow && allowUnsecureNodeDetails.FromWorkflow;
76+
77+
bool bothFromSystem = forceNode24Details.IsTrue && allowUnsecureNodeDetails.IsTrue &&
78+
forceNode24Details.FromSystem && allowUnsecureNodeDetails.FromSystem;
79+
80+
// Handle the case when both are set in the same source
81+
if (bothFromWorkflow || bothFromSystem)
82+
{
83+
string source = bothFromWorkflow ? "workflow" : "system";
84+
string defaultVersion = useNode24ByDefault ? Constants.Runner.NodeMigration.Node24 : Constants.Runner.NodeMigration.Node20;
85+
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}.";
86+
return (defaultVersion, warningMessage);
87+
}
88+
89+
// Phase 2: Node 24 is the default
90+
if (useNode24ByDefault)
91+
{
92+
if (allowUnsecureNode)
93+
{
94+
return (Constants.Runner.NodeMigration.Node20, null);
95+
}
96+
97+
return (Constants.Runner.NodeMigration.Node24, null);
98+
}
99+
100+
// Phase 1: Node 20 is the default
101+
if (forceNode24)
102+
{
103+
return (Constants.Runner.NodeMigration.Node24, null);
104+
}
105+
106+
return (Constants.Runner.NodeMigration.Node20, null);
107+
}
21108

22109
/// <summary>
23110
/// Checks if Node24 is requested but running on ARM32 Linux, and determines if fallback is needed.
@@ -26,14 +113,50 @@ public static string GetInternalNodeVersion()
26113
/// <returns>A tuple containing the adjusted node version and an optional warning message</returns>
27114
public static (string nodeVersion, string warningMessage) CheckNodeVersionForLinuxArm32(string preferredVersion)
28115
{
29-
if (string.Equals(preferredVersion, "node24", StringComparison.OrdinalIgnoreCase) &&
116+
if (string.Equals(preferredVersion, Constants.Runner.NodeMigration.Node24, StringComparison.OrdinalIgnoreCase) &&
30117
Constants.Runner.PlatformArchitecture.Equals(Constants.Architecture.Arm) &&
31118
Constants.Runner.Platform.Equals(Constants.OSPlatform.Linux))
32119
{
33-
return ("node20", "Node 24 is not supported on Linux ARM32 platforms. Falling back to Node 20.");
120+
return (Constants.Runner.NodeMigration.Node20, "Node 24 is not supported on Linux ARM32 platforms. Falling back to Node 20.");
34121
}
35122

36123
return (preferredVersion, null);
37124
}
125+
126+
/// <summary>
127+
/// Gets detailed information about an environment variable from both workflow and system environments
128+
/// </summary>
129+
/// <param name="variableName">The name of the environment variable</param>
130+
/// <param name="workflowEnvironment">Optional dictionary containing workflow-level environment variables</param>
131+
/// <returns>An EnvironmentVariableInfo object containing details about the variable from both sources</returns>
132+
private static EnvironmentVariableInfo GetEnvironmentVariableDetails(string variableName, IDictionary<string, string> workflowEnvironment)
133+
{
134+
var info = new EnvironmentVariableInfo();
135+
136+
// Check workflow environment
137+
bool foundInWorkflow = false;
138+
string workflowValue = null;
139+
140+
if (workflowEnvironment != null && workflowEnvironment.TryGetValue(variableName, out workflowValue))
141+
{
142+
foundInWorkflow = true;
143+
info.FromWorkflow = true;
144+
info.IsTrue = StringUtil.ConvertToBoolean(workflowValue); // Workflow value takes precedence for the boolean value
145+
}
146+
147+
// Also check system environment
148+
string systemValue = Environment.GetEnvironmentVariable(variableName);
149+
bool foundInSystem = !string.IsNullOrEmpty(systemValue);
150+
151+
info.FromSystem = foundInSystem;
152+
153+
// If not found in workflow, use system values
154+
if (!foundInWorkflow)
155+
{
156+
info.IsTrue = StringUtil.ConvertToBoolean(systemValue);
157+
}
158+
159+
return info;
160+
}
38161
}
39162
}

‎src/Runner.Worker/Handlers/HandlerFactory.cs‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,41 @@ public IHandler Create(
5858
var nodeData = data as NodeJSActionExecutionData;
5959

6060
// With node12 EoL in 04/2022 and node16 EoL in 09/23, we want to execute all JS actions using node20
61+
// With node20 EoL approaching, we're preparing to migrate to node24
6162
if (string.Equals(nodeData.NodeVersion, "node12", StringComparison.InvariantCultureIgnoreCase) ||
6263
string.Equals(nodeData.NodeVersion, "node16", StringComparison.InvariantCultureIgnoreCase))
6364
{
64-
nodeData.NodeVersion = "node20";
65+
nodeData.NodeVersion = Common.Constants.Runner.NodeMigration.Node20;
66+
}
67+
68+
// Check if node20 was explicitly specified in the action
69+
// We don't modify if node24 was explicitly specified
70+
if (string.Equals(nodeData.NodeVersion, Constants.Runner.NodeMigration.Node20, StringComparison.InvariantCultureIgnoreCase))
71+
{
72+
bool useNode24ByDefault = executionContext.Global.Variables?.GetBoolean(Constants.Runner.NodeMigration.UseNode24ByDefaultFlag) ?? false;
73+
bool requireNode24 = executionContext.Global.Variables?.GetBoolean(Constants.Runner.NodeMigration.RequireNode24Flag) ?? false;
74+
75+
var (nodeVersion, configWarningMessage) = NodeUtil.DetermineActionsNodeVersion(environment, useNode24ByDefault, requireNode24);
76+
var (finalNodeVersion, platformWarningMessage) = NodeUtil.CheckNodeVersionForLinuxArm32(nodeVersion);
77+
nodeData.NodeVersion = finalNodeVersion;
78+
79+
if (!string.IsNullOrEmpty(configWarningMessage))
80+
{
81+
executionContext.Warning(configWarningMessage);
82+
}
83+
84+
if (!string.IsNullOrEmpty(platformWarningMessage))
85+
{
86+
executionContext.Warning(platformWarningMessage);
87+
}
88+
89+
// Show information about Node 24 migration in Phase 2
90+
if (useNode24ByDefault && !requireNode24 && string.Equals(finalNodeVersion, Constants.Runner.NodeMigration.Node24, StringComparison.OrdinalIgnoreCase))
91+
{
92+
string infoMessage = "Node 20 is being deprecated. This workflow is running with Node 24 by default. " +
93+
"If you need to temporarily use Node 20, you can set the ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION=true environment variable.";
94+
executionContext.Output(infoMessage);
95+
}
6596
}
6697

6798
(handler as INodeScriptActionHandler).Data = nodeData;

‎src/Test/L0/Util/NodeUtilL0.cs‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using GitHub.Runner.Common;
4+
using GitHub.Runner.Common.Util;
5+
using Xunit;
6+
7+
namespace GitHub.Runner.Common.Tests.Util
8+
{
9+
public class NodeUtilL0
10+
{
11+
// We're testing the logic with feature flags
12+
[Theory]
13+
[InlineData(false, false, false, false, "node20", false)] // Phase 1: No env vars
14+
[InlineData(false, false, false, true, "node20", false)] // Phase 1: Allow unsecure (redundant)
15+
[InlineData(false, false, true, false, "node24", false)] // Phase 1: Force node24
16+
[InlineData(false, false, true, true, "node20", true)] // Phase 1: Both flags (use phase default + warning)
17+
[InlineData(false, true, false, false, "node24", false)] // Phase 2: No env vars
18+
[InlineData(false, true, false, true, "node20", false)] // Phase 2: Allow unsecure
19+
[InlineData(false, true, true, false, "node24", false)] // Phase 2: Force node24 (redundant)
20+
[InlineData(false, true, true, true, "node24", true)] // Phase 2: Both flags (use phase default + warning)
21+
[InlineData(true, false, false, false, "node24", false)] // Phase 3: Always Node 24 regardless of env vars
22+
[InlineData(true, false, false, true, "node24", false)] // Phase 3: Always Node 24 regardless of env vars
23+
[InlineData(true, false, true, false, "node24", false)] // Phase 3: Always Node 24 regardless of env vars
24+
[InlineData(true, false, true, true, "node24", false)] // Phase 3: Always Node 24 regardless of env vars, no warnings in Phase 3
25+
public void TestNodeVersionLogic(bool requireNode24, bool useNode24ByDefault, bool forceNode24, bool allowUnsecureNode, string expectedVersion, bool expectWarning)
26+
{
27+
try
28+
{
29+
Environment.SetEnvironmentVariable(Constants.Runner.NodeMigration.ForceNode24Variable, forceNode24 ? "true" : null);
30+
Environment.SetEnvironmentVariable(Constants.Runner.NodeMigration.AllowUnsecureNodeVersionVariable, allowUnsecureNode ? "true" : null);
31+
32+
// Call the actual method
33+
var (actualVersion, warningMessage) = NodeUtil.DetermineActionsNodeVersion(null, useNode24ByDefault, requireNode24);
34+
35+
// Assert
36+
Assert.Equal(expectedVersion, actualVersion);
37+
38+
if (expectWarning)
39+
{
40+
Assert.NotNull(warningMessage);
41+
Assert.Contains("Both", warningMessage);
42+
Assert.Contains("are set to true", warningMessage);
43+
}
44+
else
45+
{
46+
Assert.Null(warningMessage);
47+
}
48+
}
49+
finally
50+
{
51+
// Cleanup
52+
Environment.SetEnvironmentVariable(Constants.Runner.NodeMigration.ForceNode24Variable, null);
53+
Environment.SetEnvironmentVariable(Constants.Runner.NodeMigration.AllowUnsecureNodeVersionVariable, null);
54+
}
55+
}
56+
57+
[Theory]
58+
[InlineData(false, false, false, false, false, true, "node20", false)] // Phase 1: System env: none, Workflow env: allow=true
59+
[InlineData(false, false, true, false, false, false, "node24", false)] // Phase 1: System env: force node24, Workflow env: none
60+
[InlineData(false, true, false, false, true, false, "node24", false)] // Phase 1: System env: none, Workflow env: force node24
61+
[InlineData(false, false, false, true, false, true, "node20", false)] // Phase 1: System env: allow=true, Workflow env: allow=true (workflow takes precedence)
62+
[InlineData(false, false, true, true, false, false, "node20", true)] // Phase 1: System env: both true, Workflow env: none (use phase default + warning)
63+
[InlineData(false, false, false, false, true, true, "node20", true)] // Phase 1: System env: none, Workflow env: both (use phase default + warning)
64+
[InlineData(true, false, false, false, false, false, "node24", false)] // Phase 2: System env: none, Workflow env: none
65+
[InlineData(true, false, false, true, false, false, "node20", false)] // Phase 2: System env: allow=true, Workflow env: none
66+
[InlineData(true, false, false, false, false, true, "node20", false)] // Phase 2: System env: none, Workflow env: allow unsecure
67+
[InlineData(true, false, true, false, false, true, "node20", false)] // Phase 2: System env: force node24, Workflow env: allow unsecure
68+
[InlineData(true, false, true, true, false, false, "node24", true)] // Phase 2: System env: both true, Workflow env: none (use phase default + warning)
69+
[InlineData(true, false, false, false, true, true, "node24", true)] // Phase 2: System env: none, Workflow env: both (phase default + warning)
70+
[InlineData(false, true, false, false, false, true, "node24", false)] // Phase 3: System env: none, Workflow env: allow=true (always Node 24 in Phase 3)
71+
[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)
72+
[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)
73+
public void TestNodeVersionLogicWithWorkflowEnvironment(bool useNode24ByDefault, bool requireNode24,
74+
bool systemForceNode24, bool systemAllowUnsecure,
75+
bool workflowForceNode24, bool workflowAllowUnsecure,
76+
string expectedVersion, bool expectWarning)
77+
{
78+
try
79+
{
80+
// Set system environment variables
81+
Environment.SetEnvironmentVariable(Constants.Runner.NodeMigration.ForceNode24Variable, systemForceNode24 ? "true" : null);
82+
Environment.SetEnvironmentVariable(Constants.Runner.NodeMigration.AllowUnsecureNodeVersionVariable, systemAllowUnsecure ? "true" : null);
83+
84+
// Set workflow environment variables
85+
var workflowEnv = new Dictionary<string, string>();
86+
if (workflowForceNode24)
87+
{
88+
workflowEnv[Constants.Runner.NodeMigration.ForceNode24Variable] = "true";
89+
}
90+
if (workflowAllowUnsecure)
91+
{
92+
workflowEnv[Constants.Runner.NodeMigration.AllowUnsecureNodeVersionVariable] = "true";
93+
}
94+
95+
// Call the actual method with our test parameters
96+
var (actualVersion, warningMessage) = NodeUtil.DetermineActionsNodeVersion(workflowEnv, useNode24ByDefault, requireNode24);
97+
98+
// Assert
99+
Assert.Equal(expectedVersion, actualVersion);
100+
101+
if (expectWarning)
102+
{
103+
Assert.NotNull(warningMessage);
104+
Assert.Contains("Both", warningMessage);
105+
Assert.Contains("are set to true", warningMessage);
106+
}
107+
else
108+
{
109+
Assert.Null(warningMessage);
110+
}
111+
}
112+
finally
113+
{
114+
// Cleanup
115+
Environment.SetEnvironmentVariable(Constants.Runner.NodeMigration.ForceNode24Variable, null);
116+
Environment.SetEnvironmentVariable(Constants.Runner.NodeMigration.AllowUnsecureNodeVersionVariable, null);
117+
}
118+
}
119+
}
120+
}

0 commit comments

Comments
 (0)