From 20b7e86e4720eab3b81254e0a4f3b29ff883c4dd Mon Sep 17 00:00:00 2001 From: Nikola Jokic <97525037+nikola-jokic@users.noreply.github.com> Date: Thu, 28 Apr 2022 03:23:12 +0200 Subject: [PATCH] Added ability to run Dockerfile.SUFFIX ContainerAction (#1738) * Added ability to run Dockerfile.SUFFIX ContainerAction * Extracted IsDockerFile method * reformatted, moved from index to Last() * extracted IsDockerfile to DockerUtil with L0 * added check for IsDockerfile to account for docker:// * updated test to clearly show path/dockerfile:tag * fail if Data.Image is not Dockerfile or docker://[image] --- src/Runner.Worker/ActionManager.cs | 2 +- src/Runner.Worker/Container/DockerUtil.cs | 13 +++++- .../Handlers/ContainerActionHandler.cs | 7 ++- src/Test/L0/Container/DockerUtilL0.cs | 43 +++++++++++++++++++ 4 files changed, 61 insertions(+), 4 deletions(-) diff --git a/src/Runner.Worker/ActionManager.cs b/src/Runner.Worker/ActionManager.cs index 954d9a691..88af0347a 100644 --- a/src/Runner.Worker/ActionManager.cs +++ b/src/Runner.Worker/ActionManager.cs @@ -1004,7 +1004,7 @@ namespace GitHub.Runner.Worker if (actionDefinitionData.Execution.ExecutionType == ActionExecutionType.Container) { var containerAction = actionDefinitionData.Execution as ContainerActionExecutionData; - if (containerAction.Image.EndsWith("Dockerfile") || containerAction.Image.EndsWith("dockerfile")) + if (DockerUtil.IsDockerfile(containerAction.Image)) { var dockerFileFullPath = Path.Combine(actionEntryDirectory, containerAction.Image); executionContext.Debug($"Dockerfile for action: '{dockerFileFullPath}'."); diff --git a/src/Runner.Worker/Container/DockerUtil.cs b/src/Runner.Worker/Container/DockerUtil.cs index 02c2ece5b..af7de18df 100644 --- a/src/Runner.Worker/Container/DockerUtil.cs +++ b/src/Runner.Worker/Container/DockerUtil.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Text.RegularExpressions; namespace GitHub.Runner.Worker.Container @@ -17,7 +18,7 @@ namespace GitHub.Runner.Worker.Container string pattern = $"^(?<{targetPort}>\\d+)/(?<{proto}>\\w+) -> (?<{host}>.+):(?<{hostPort}>\\d+)$"; List portMappings = new List(); - foreach(var line in portMappingLines) + foreach (var line in portMappingLines) { Match m = Regex.Match(line, pattern, RegexOptions.None, TimeSpan.FromSeconds(1)); if (m.Success) @@ -61,5 +62,15 @@ namespace GitHub.Runner.Worker.Container } return ""; } + + public static bool IsDockerfile(string image) + { + if (image.StartsWith("docker://", StringComparison.OrdinalIgnoreCase)) + { + return false; + } + var imageWithoutPath = image.Split('/').Last(); + return imageWithoutPath.StartsWith("Dockerfile.") || imageWithoutPath.StartsWith("dockerfile.") || imageWithoutPath.EndsWith("Dockerfile") || imageWithoutPath.EndsWith("dockerfile"); + } } } diff --git a/src/Runner.Worker/Handlers/ContainerActionHandler.cs b/src/Runner.Worker/Handlers/ContainerActionHandler.cs index 3eaacb5a9..d6494f63f 100644 --- a/src/Runner.Worker/Handlers/ContainerActionHandler.cs +++ b/src/Runner.Worker/Handlers/ContainerActionHandler.cs @@ -44,9 +44,8 @@ namespace GitHub.Runner.Worker.Handlers { Data.Image = Data.Image.Substring("docker://".Length); } - else if (Data.Image.EndsWith("Dockerfile") || Data.Image.EndsWith("dockerfile")) + else if (DockerUtil.IsDockerfile(Data.Image)) { - // ensure docker file exist var dockerFile = Path.Combine(ActionDirectory, Data.Image); ArgUtil.File(dockerFile, nameof(Data.Image)); @@ -68,6 +67,10 @@ namespace GitHub.Runner.Worker.Handlers Data.Image = imageName; } + else + { + throw new InvalidOperationException($"'{Data.Image}' should be either '[path]/Dockerfile' or 'docker://image[:tag]'."); + } string type = Action.Type == Pipelines.ActionSourceType.Repository ? "Dockerfile" : "DockerHub"; // Set extra telemetry base on the current context. diff --git a/src/Test/L0/Container/DockerUtilL0.cs b/src/Test/L0/Container/DockerUtilL0.cs index c069255a8..c31e9aead 100644 --- a/src/Test/L0/Container/DockerUtilL0.cs +++ b/src/Test/L0/Container/DockerUtilL0.cs @@ -144,5 +144,48 @@ namespace GitHub.Runner.Common.Tests.Worker.Container var actual = DockerUtil.ParseRegistryHostnameFromImageName(input); Assert.Equal(expected, actual); } + + [Theory] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + [InlineData("dockerhub/repo", false)] + [InlineData("debian:latest", false)] + [InlineData("something/dockerfileimage", false)] + [InlineData("ghcr.io/docker/dockerfile", true)] // should be false but might break the current workflows + [InlineData("Dockerfile", true)] + [InlineData("Dockerfile.", true)] + [InlineData(".Dockerfile", true)] + [InlineData(".Dockerfile.", false)] + [InlineData("dockerfile", true)] + [InlineData("dockerfile.", true)] + [InlineData(".dockerfile", true)] + [InlineData(".dockerfile.", false)] + [InlineData("Dockerfile.test", true)] + [InlineData("test.Dockerfile", true)] + [InlineData("docker/dockerfile:latest", false)] + [InlineData("/some/path/dockerfile:latest", false)] + [InlineData("dockerfile:latest", false)] + [InlineData("Dockerfile:latest", false)] + [InlineData("dockerfile-latest", false)] + [InlineData("Dockerfile-latest", false)] + [InlineData("dockerfile.latest", true)] + [InlineData("Dockerfile.latest", true)] + [InlineData("../dockerfile/dockerfileone", false)] + [InlineData("../Dockerfile/dockerfileone", false)] + [InlineData("../dockerfile.test", true)] + [InlineData("../Dockerfile.test", true)] + [InlineData("./dockerfile/image", false)] + [InlineData("./Dockerfile/image", false)] + [InlineData("example/Dockerfile.test", true)] + [InlineData("./example/Dockerfile.test", true)] + [InlineData("example/test.dockerfile", true)] + [InlineData("./example/test.dockerfile", true)] + [InlineData("docker://Dockerfile", false)] + [InlineData("docker://ubuntu:latest", false)] + public void IsDockerfile(string input, bool expected) + { + var actual = DockerUtil.IsDockerfile(input); + Assert.Equal(expected, actual); + } } }