Skip to content
Draft
Show file tree
Hide file tree
Changes from all 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
67 changes: 62 additions & 5 deletions src/Runner.Worker/ActionManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public sealed class ActionManager : RunnerService, IActionManager
// Stack-local cache: same action (owner/repo@ref) is resolved only once,
// even if it appears at multiple depths in a composite tree.
var resolvedDownloadInfos = batchActionResolution
? new Dictionary<string, WebApi.ActionDownloadInfo>(StringComparer.Ordinal)
? new Dictionary<string, WebApi.ActionDownloadInfo>(ActionLookupKeyComparer.Instance)
: null;
var depth = 0;
// We are running at the start of a job
Expand Down Expand Up @@ -228,6 +228,9 @@ public sealed class ActionManager : RunnerService, IActionManager
{
throw new Exception($"Missing download info for {lookupKey}");
}
// Normalize the reference name to match the resolved download info so that
// directory paths are consistent on case-sensitive filesystems (Linux).
NormalizeRepositoryReference(action, downloadInfo);
await DownloadRepositoryActionAsync(executionContext, downloadInfo);
}

Expand Down Expand Up @@ -414,6 +417,9 @@ public sealed class ActionManager : RunnerService, IActionManager
throw new Exception($"Missing download info for {lookupKey}");
}

// Normalize the reference name to match the resolved download info so that
// directory paths are consistent on case-sensitive filesystems (Linux).
NormalizeRepositoryReference(action, downloadInfo);
await DownloadRepositoryActionAsync(executionContext, downloadInfo);
}

Expand Down Expand Up @@ -858,7 +864,7 @@ private async Task BuildActionContainerAsync(IExecutionContext executionContext,

// Convert to action reference
var actionReferences = actions
.GroupBy(x => GetDownloadInfoLookupKey(x))
.GroupBy(x => GetDownloadInfoLookupKey(x), ActionLookupKeyComparer.Instance)
.Where(x => !string.IsNullOrEmpty(x.Key))
.Select(x =>
{
Expand All @@ -877,7 +883,7 @@ private async Task BuildActionContainerAsync(IExecutionContext executionContext,
// Nothing to resolve?
if (actionReferences.Count == 0)
{
return new Dictionary<string, WebApi.ActionDownloadInfo>();
return new Dictionary<string, WebApi.ActionDownloadInfo>(ActionLookupKeyComparer.Instance);
}

// Resolve download info
Expand Down Expand Up @@ -953,7 +959,7 @@ private async Task BuildActionContainerAsync(IExecutionContext executionContext,
}
}

return actionDownloadInfos.Actions;
return new Dictionary<string, WebApi.ActionDownloadInfo>(actionDownloadInfos.Actions, ActionLookupKeyComparer.Instance);
}

/// <summary>
Expand All @@ -963,7 +969,7 @@ private async Task BuildActionContainerAsync(IExecutionContext executionContext,
private async Task ResolveNewActionsAsync(IExecutionContext executionContext, List<Pipelines.ActionStep> actions, Dictionary<string, WebApi.ActionDownloadInfo> resolvedDownloadInfos)
{
var actionsToResolve = new List<Pipelines.ActionStep>();
var pendingKeys = new HashSet<string>(StringComparer.Ordinal);
var pendingKeys = new HashSet<string>(ActionLookupKeyComparer.Instance);
foreach (var action in actions)
{
var lookupKey = GetDownloadInfoLookupKey(action);
Expand Down Expand Up @@ -1342,6 +1348,22 @@ private ActionSetupInfo PrepareRepositoryActionAsync(IExecutionContext execution
}
}

/// <summary>
/// After case-insensitive deduplication, the resolved download info may use
/// a different casing than the action step's reference. Normalize the reference
/// so that directory paths (used by DownloadRepositoryActionAsync,
/// PrepareRepositoryActionAsync, and LoadAction) are consistent on
/// case-sensitive filesystems.
/// </summary>
private static void NormalizeRepositoryReference(Pipelines.ActionStep action, WebApi.ActionDownloadInfo downloadInfo)
{
if (action.Reference is Pipelines.RepositoryPathReference repoRef &&
!string.Equals(repoRef.Name, downloadInfo.NameWithOwner, StringComparison.Ordinal))
{
repoRef.Name = downloadInfo.NameWithOwner;
}
}

private static string GetDownloadInfoLookupKey(Pipelines.ActionStep action)
{
if (action.Reference.Type != Pipelines.ActionSourceType.Repository)
Expand All @@ -1367,6 +1389,41 @@ private static string GetDownloadInfoLookupKey(Pipelines.ActionStep action)
return $"{repositoryReference.Name}@{repositoryReference.Ref}";
}

/// <summary>
/// Compares action lookup keys ("{owner/repo}@{ref}") with case-insensitive
/// owner/repo (GitHub names are case-insensitive) and case-sensitive ref
/// (git refs are case-sensitive).
/// </summary>
private sealed class ActionLookupKeyComparer : IEqualityComparer<string>
{
public static readonly ActionLookupKeyComparer Instance = new();

public bool Equals(string x, string y)
{
if (ReferenceEquals(x, y)) return true;
if (x is null || y is null) return false;

var xAt = x.LastIndexOf('@');
var yAt = y.LastIndexOf('@');
if (xAt < 0 || yAt < 0) return StringComparer.OrdinalIgnoreCase.Equals(x, y);

// Name (owner/repo) is case-insensitive; @ref is case-sensitive
return x.AsSpan(0, xAt).Equals(y.AsSpan(0, yAt), StringComparison.OrdinalIgnoreCase)
&& x.AsSpan(xAt).Equals(y.AsSpan(yAt), StringComparison.Ordinal);
}

public int GetHashCode(string obj)
{
if (obj is null) return 0;
var at = obj.LastIndexOf('@');
if (at < 0) return StringComparer.OrdinalIgnoreCase.GetHashCode(obj);

return HashCode.Combine(
StringComparer.OrdinalIgnoreCase.GetHashCode(obj.Substring(0, at)),
StringComparer.Ordinal.GetHashCode(obj.Substring(at)));
}
}

private AuthenticationHeaderValue CreateAuthHeader(IExecutionContext executionContext, string downloadUrl, string token)
{
if (string.IsNullOrEmpty(token))
Expand Down
107 changes: 107 additions & 0 deletions src/Test/L0/Worker/ActionManagerL0.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1449,6 +1449,113 @@ public async void PrepareActions_DeduplicatesResolutionAcrossDepthLevels()
}
}

[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Worker")]
public async void PrepareActions_DeduplicatesCaseInsensitiveActionReferences()
{
// Verifies that action references differing only by owner/repo casing
// are deduplicated and resolved only once.
// Regression test for https://github.com/actions/runner/issues/3731
Environment.SetEnvironmentVariable("ACTIONS_BATCH_ACTION_RESOLUTION", "true");
try
{
//Arrange
Setup();
// Each action step with pre+post needs 2 IActionRunner instances (pre + post)
for (int i = 0; i < 6; i++)
{
_hc.EnqueueInstance<IActionRunner>(new Mock<IActionRunner>().Object);
}

var allResolvedKeys = new List<string>();
_jobServer.Setup(x => x.ResolveActionDownloadInfoAsync(It.IsAny<Guid>(), It.IsAny<string>(), It.IsAny<Guid>(), It.IsAny<Guid>(), It.IsAny<ActionReferenceList>(), It.IsAny<CancellationToken>()))
.Returns((Guid scopeIdentifier, string hubName, Guid planId, Guid jobId, ActionReferenceList actions, CancellationToken cancellationToken) =>
{
var result = new ActionDownloadInfoCollection { Actions = new Dictionary<string, ActionDownloadInfo>() };
foreach (var action in actions.Actions)
{
var key = $"{action.NameWithOwner}@{action.Ref}";
allResolvedKeys.Add(key);
result.Actions[key] = new ActionDownloadInfo
{
NameWithOwner = action.NameWithOwner,
Ref = action.Ref,
ResolvedNameWithOwner = action.NameWithOwner,
ResolvedSha = $"{action.Ref}-sha",
TarballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/tarball/{action.Ref}",
ZipballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/zipball/{action.Ref}",
};
}
return Task.FromResult(result);
});

var actions = new List<Pipelines.ActionStep>
{
new Pipelines.ActionStep()
{
Name = "action1",
Id = Guid.NewGuid(),
Reference = new Pipelines.RepositoryPathReference()
{
Name = "TingluoHuang/runner_L0",
Ref = "RepositoryActionWithWrapperActionfile_Node",
RepositoryType = "GitHub"
}
},
new Pipelines.ActionStep()
{
Name = "action2",
Id = Guid.NewGuid(),
Reference = new Pipelines.RepositoryPathReference()
{
Name = "tingluohuang/RUNNER_L0",
Ref = "RepositoryActionWithWrapperActionfile_Node",
RepositoryType = "GitHub"
}
},
new Pipelines.ActionStep()
{
Name = "action3",
Id = Guid.NewGuid(),
Reference = new Pipelines.RepositoryPathReference()
{
Name = "TINGLUOHUANG/Runner_L0",
Ref = "RepositoryActionWithWrapperActionfile_Node",
RepositoryType = "GitHub"
}
}
};

//Act
await _actionManager.PrepareActionsAsync(_ec.Object, actions);

//Assert
// All three references should deduplicate to a single resolve call
Assert.Equal(1, allResolvedKeys.Count);

// Verify all actions are usable: the download went to one directory and
// reference normalization ensures all three steps find the action there.
// The completed watermark proves the download + prepare succeeded.
Assert.True(File.Exists(Path.Combine(
_hc.GetDirectory(WellKnownDirectory.Actions),
"TingluoHuang/runner_L0",
"RepositoryActionWithWrapperActionfile_Node.completed")));

// Verify the references were normalized to the canonical casing
foreach (var action in actions)
{
var repoRef = action.Reference as Pipelines.RepositoryPathReference;
Assert.Equal("TingluoHuang/runner_L0", repoRef.Name);
}
}
finally
{
Environment.SetEnvironmentVariable("ACTIONS_BATCH_ACTION_RESOLUTION", null);
Teardown();
}
}

[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Worker")]
Expand Down