Revert "docker: escape key-value pair as -e KEY and VALUE being environment var (#2091)"

This reverts commit 95459dea5f.

docs/adrs/1891-container-hooks.md

@@ -16,7 +16,7 @@ We should give them that option, and publish examples how how they can create th
   - For example, the current runner overrides `HOME`, we can do that in the hook, but we shouldn't pass that hook as an ENV with the other env's the user has set, as that is not user input, it is how the runner invokes containers
 
 ## Interface
-- You will set the variable `ACTIONS_RUNNER_CONTAINER_HOOK=/Users/foo/runner/hooks.js` which is the entrypoint to your hook handler.
+- You will set the variable `ACTIONS_RUNNER_CONTAINER_HOOKS=/Users/foo/runner/hooks.js` which is the entrypoint to your hook handler.
   - There is no partial opt in, you must handle every hook 
 - We will pass a command and some args via `stdin`
 - An exit code of 0 is a success, every other exit code is a failure

src/Runner.Worker/Container/DockerCommandManager.cs

@@ -107,7 +107,6 @@ namespace GitHub.Runner.Worker.Container
         public async Task<string> DockerCreate(IExecutionContext context, ContainerInfo container)
         {
             IList<string> dockerOptions = new List<string>();
-            IDictionary<string, string> environment = new Dictionary<string, string>();
             // OPTIONS
             dockerOptions.Add($"--name {container.ContainerDisplayName}");
             dockerOptions.Add($"--label {DockerInstanceLabel}");
@@ -136,8 +135,7 @@ namespace GitHub.Runner.Worker.Container
                 }
                 else
                 {
-                    environment.Add(env.Key, env.Value);
-                    dockerOptions.Add(DockerUtil.CreateEscapedOption("-e", env.Key));
+                    dockerOptions.Add(DockerUtil.CreateEscapedOption("-e", env.Key, env.Value));
                 }
             }
 
@@ -185,7 +183,7 @@ namespace GitHub.Runner.Worker.Container
             dockerOptions.Add($"{container.ContainerEntryPointArgs}");
 
             var optionsString = string.Join(" ", dockerOptions);
-            List<string> outputStrings = await ExecuteDockerCommandAsync(context, "create", optionsString, environment);
+            List<string> outputStrings = await ExecuteDockerCommandAsync(context, "create", optionsString);
 
             return outputStrings.FirstOrDefault();
         }
@@ -445,11 +443,6 @@ namespace GitHub.Runner.Worker.Container
         }
 
         private async Task<List<string>> ExecuteDockerCommandAsync(IExecutionContext context, string command, string options)
-        {
-            return await ExecuteDockerCommandAsync(context, command, options, null);
-        }
-
-        private async Task<List<string>> ExecuteDockerCommandAsync(IExecutionContext context, string command, string options, IDictionary<string, string> environment)
         {
             string arg = $"{command} {options}".Trim();
             context.Command($"{DockerPath} {arg}");
@@ -477,7 +470,7 @@ namespace GitHub.Runner.Worker.Container
                             workingDirectory: context.GetGitHubContext("workspace"),
                             fileName: DockerPath,
                             arguments: arg,
-                            environment: environment,
+                            environment: null,
                             requireExitCodeZero: true,
                             outputEncoding: null,
                             cancellationToken: CancellationToken.None);

src/Runner.Worker/Container/DockerUtil.cs

@@ -71,6 +71,15 @@ namespace GitHub.Runner.Worker.Container
             return $"{flag} \"{EscapeString(key)}\"";
         }
 
+        public static string CreateEscapedOption(string flag, string key, string value)
+        {
+            if (String.IsNullOrEmpty(key))
+            {
+                return "";
+            }
+            return $"{flag} \"{EscapeString(key)}={EscapeString(value)}\"";
+        }
+
         private static string EscapeString(string value)
         {
             return value.Replace("\\", "\\\\").Replace("\"", "\\\"");

src/Runner.Worker/ContainerOperationProvider.cs

@@ -134,6 +134,7 @@ namespace GitHub.Runner.Worker
             ArgUtil.NotNull(executionContext, nameof(executionContext));
             ArgUtil.NotNull(container, nameof(container));
             ArgUtil.NotNullOrEmpty(container.ContainerImage, nameof(container.ContainerImage));
+
             Trace.Info($"Container name: {container.ContainerName}");
             Trace.Info($"Container image: {container.ContainerImage}");
             Trace.Info($"Container options: {container.ContainerCreateOptions}");
@@ -196,8 +197,6 @@ namespace GitHub.Runner.Worker
 
             container.ContainerId = await _dockerManager.DockerCreate(executionContext, container);
             ArgUtil.NotNullOrEmpty(container.ContainerId, nameof(container.ContainerId));
-            
-
 
             // Start container
             int startExitCode = await _dockerManager.DockerStart(executionContext, container.ContainerId);
@@ -208,37 +207,20 @@ namespace GitHub.Runner.Worker
 
             try
             {
-                
                 // Make sure container is up and running
                 var psOutputs = await _dockerManager.DockerPS(executionContext, $"--all --filter id={container.ContainerId} --filter status=running --no-trunc --format \"{{{{.ID}}}} {{{{.Status}}}}\"");
-                
                 if (psOutputs.FirstOrDefault(x => !string.IsNullOrEmpty(x))?.StartsWith(container.ContainerId) != true)
                 {
                     // container is not up and running, pull docker log for this container.
                     await _dockerManager.DockerPS(executionContext, $"--all --filter id={container.ContainerId} --no-trunc --format \"{{{{.ID}}}} {{{{.Status}}}}\"");
-                    
-                    
-                    //executionContext.Output("##[group]Getting docker logs..");
-
                     int logsExitCode = await _dockerManager.DockerLogs(executionContext, container.ContainerId);
-                    
                     if (logsExitCode != 0)
                     {
                         executionContext.Warning($"Docker logs fail with exit code {logsExitCode}");
                     }
-                    //executionContext.Output("##[endgroup]");
+
                     executionContext.Warning($"Docker container {container.ContainerId} is not in running state.");
                 }
-                else {
-                        executionContext.Output($"##[group]Container {container.ContainerId} is not running!");
-                        string opt = "--format=\'{{.State.ExitCode}}\'";
-                        await _dockerManager.DockerInspect(executionContext, container.ContainerId, opt);
-                        await _dockerManager.DockerLogs(executionContext, container.ContainerId);
-                        //if exit code is not 0 we can maybe print a message?
-                        //executionContext.Output("Container exit code: ");
-                
-                    executionContext.Output("##[endgroup]");
-                }
             }
             catch (Exception ex)
             {

src/Runner.Worker/JobExtension.cs

@@ -279,7 +279,7 @@ namespace GitHub.Runner.Worker
 
                         preJobSteps.Add(new JobExtensionRunner(runAsync: containerProvider.StartContainersAsync,
                                                                           condition: $"{PipelineTemplateConstants.Success}()",
-                                                                          displayName: "**Initialize containers**",
+                                                                          displayName: "Initialize containers",
                                                                           data: (object)containers));
                     }
 

src/Test/L0/Container/DockerUtilL0.cs

@@ -171,5 +171,32 @@ namespace GitHub.Runner.Common.Tests.Worker.Container
             }
             Assert.Equal(expected, actual);
         }
+
+        [Theory]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        [InlineData("HOME", "", "HOME", "")]
+        [InlineData("HOME alpine:3.8 sh -c id #", "HOME alpine:3.8 sh -c id #", "HOME alpine:3.8 sh -c id #", "HOME alpine:3.8 sh -c id #")]
+        [InlineData("HOME \"alpine:3.8 sh -c id #", "HOME \"alpine:3.8 sh -c id #", "HOME \\\"alpine:3.8 sh -c id #", "HOME \\\"alpine:3.8 sh -c id #")]
+        [InlineData("HOME \\\"alpine:3.8 sh -c id #", "HOME \\\"alpine:3.8 sh -c id #", "HOME \\\\\\\"alpine:3.8 sh -c id #", "HOME \\\\\\\"alpine:3.8 sh -c id #")]
+        [InlineData("HOME \\\\\"alpine:3.8 sh -c id #", "HOME \\\\\"alpine:3.8 sh -c id #", "HOME \\\\\\\\\\\"alpine:3.8 sh -c id #", "HOME \\\\\\\\\\\"alpine:3.8 sh -c id #")]
+        [InlineData("HOME \"\"alpine:3.8 sh -c id #", "HOME \"\"alpine:3.8 sh -c id #", "HOME \\\"\\\"alpine:3.8 sh -c id #", "HOME \\\"\\\"alpine:3.8 sh -c id #")]
+        [InlineData("HOME \\\"\"alpine:3.8 sh -c id #", "HOME \\\"\"alpine:3.8 sh -c id #", "HOME \\\\\\\"\\\"alpine:3.8 sh -c id #", "HOME \\\\\\\"\\\"alpine:3.8 sh -c id #")]
+        [InlineData("HOME \"\\\"alpine:3.8 sh -c id #", "HOME \"\\\"alpine:3.8 sh -c id #", "HOME \\\"\\\\\\\"alpine:3.8 sh -c id #", "HOME \\\"\\\\\\\"alpine:3.8 sh -c id #")]
+        public void CreateEscapedOption_keyValue(string keyInput, string valueInput, string escapedKey, string escapedValue)
+        {
+            var flag = "--example";
+            var actual = DockerUtil.CreateEscapedOption(flag, keyInput, valueInput);
+            string expected;
+            if (String.IsNullOrEmpty(keyInput))
+            {
+                expected = "";
+            }
+            else
+            {
+                expected = $"{flag} \"{escapedKey}={escapedValue}\"";
+            }
+            Assert.Equal(expected, actual);
+        }
     }
 }

src/runnerversion

@@ -1 +1 @@
-2.296.1
+2.296.1