From 6b2ef2822c9042c305c34c7a0562635c2048455e Mon Sep 17 00:00:00 2001
From: Sergio Zharinov <zharinov@users.noreply.github.com>
Date: Fri, 24 Jan 2020 13:42:09 +0400
Subject: [PATCH] =?UTF-8?q?refactor(exec):=20Explicit=20`extraEnv`=20defau?=
 =?UTF-8?q?lts=20and=20nullable=20docke=E2=80=A6=20(#5219)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 lib/util/exec/docker/index.ts | 17 ++++------
 lib/util/exec/index.ts        | 11 +++++--
 test/util/exec.spec.ts        | 58 +++++++++++++++++++++++------------
 3 files changed, 53 insertions(+), 33 deletions(-)

diff --git a/lib/util/exec/docker/index.ts b/lib/util/exec/docker/index.ts
index 94e4d9f5e9..f48d3df761 100644
--- a/lib/util/exec/docker/index.ts
+++ b/lib/util/exec/docker/index.ts
@@ -3,7 +3,7 @@ import {
   VolumesPair,
   DockerOptions,
   ExecConfig,
-  DockerExtraCommands,
+  Opt,
   rawExec,
 } from '../common';
 import { logger } from '../../../logger';
@@ -57,7 +57,7 @@ function prepareVolumes(volumes: VolumeOption[] = []): string[] {
   });
 }
 
-function prepareCommands(commands: DockerExtraCommands = []): string[] {
+function prepareCommands(commands: Opt<string>[]): string[] {
   return commands.filter(command => command && typeof command === 'string');
 }
 
@@ -66,15 +66,10 @@ export async function generateDockerCommand(
   options: DockerOptions,
   config: ExecConfig
 ): Promise<string> {
-  const {
-    image,
-    tag,
-    envVars,
-    cwd,
-    volumes = [],
-    preCommands,
-    postCommands,
-  } = options;
+  const { image, tag, envVars, cwd } = options;
+  const volumes = options.volumes || [];
+  const preCommands = options.preCommands || [];
+  const postCommands = options.postCommands || [];
   const { localDir, cacheDir, dockerUser } = config;
 
   const result = ['docker run --rm'];
diff --git a/lib/util/exec/index.ts b/lib/util/exec/index.ts
index a47b6e9323..2b5973ae8a 100644
--- a/lib/util/exec/index.ts
+++ b/lib/util/exec/index.ts
@@ -41,7 +41,12 @@ function createChildEnv(
   env: NodeJS.ProcessEnv,
   extraEnv: ExtraEnv
 ): ExtraEnv<string> {
-  const extraEnvKeys = Object.keys(extraEnv || {});
+  const extraEnvEntries = Object.entries({ ...extraEnv }).filter(([_, val]) => {
+    if (val === null) return false;
+    if (val === undefined) return false;
+    return true;
+  });
+  const extraEnvKeys = Object.keys(extraEnvEntries);
 
   const childEnv =
     env || extraEnv
@@ -66,7 +71,9 @@ function dockerEnvVars(
   childEnv: ExtraEnv<string>
 ): string[] {
   const extraEnvKeys = Object.keys(extraEnv || {});
-  return extraEnvKeys.filter(key => typeof childEnv[key] !== 'undefined');
+  return extraEnvKeys.filter(
+    key => typeof childEnv[key] === 'string' && childEnv[key].length > 0
+  );
 }
 
 export async function exec(
diff --git a/test/util/exec.spec.ts b/test/util/exec.spec.ts
index 453f819358..14f8a0f69e 100644
--- a/test/util/exec.spec.ts
+++ b/test/util/exec.spec.ts
@@ -168,7 +168,14 @@ describe(`Child process execution wrapper`, () => {
         execConfig,
         processEnv,
         inCmd,
-        inOpts: { extraEnv: { SELECTED_ENV_VAR: 'Default value' } },
+        inOpts: {
+          extraEnv: {
+            SELECTED_ENV_VAR: envMock.full.SELECTED_ENV_VAR,
+            FILTERED_ENV_VAR: null,
+            FOO: null,
+            BAR: undefined,
+          },
+        },
         outCmd,
         outOpts: [{ cwd, encoding, env: envMock.filtered }],
       },
@@ -182,7 +189,12 @@ describe(`Child process execution wrapper`, () => {
         inCmd,
         inOpts: {
           docker,
-          extraEnv: { SELECTED_ENV_VAR: 'Default value' },
+          extraEnv: {
+            SELECTED_ENV_VAR: envMock.full.SELECTED_ENV_VAR,
+            FILTERED_ENV_VAR: null,
+            FOO: null,
+            BAR: undefined,
+          },
           cwd,
         },
         outCmd: [
@@ -193,24 +205,6 @@ describe(`Child process execution wrapper`, () => {
       },
     ],
 
-    [
-      'Extra env vars with empty values',
-      {
-        execConfig,
-        processEnv,
-        inCmd,
-        inOpts: {
-          extraEnv: {
-            SELECTED_ENV_VAR: null, // pick from process.env
-            FOO: null,
-            BAR: undefined,
-          },
-        },
-        outCmd,
-        outOpts: [{ cwd, encoding, env: envMock.filtered }],
-      },
-    ],
-
     [
       'Extra env vars defaults',
       {
@@ -327,6 +321,30 @@ describe(`Child process execution wrapper`, () => {
         outOpts: [dockerPullOpts, { cwd, encoding, env: envMock.basic }],
       },
     ],
+
+    [
+      'Docker commands are nullable',
+      {
+        execConfig: {
+          ...execConfig,
+          binarySource: BinarySource.Docker,
+        },
+        processEnv,
+        inCmd,
+        inOpts: {
+          docker: {
+            image,
+            preCommands: null,
+            postCommands: undefined,
+          },
+        },
+        outCmd: [
+          dockerPullCmd,
+          `docker run --rm ${defaultVolumes} -w "${cwd}" ${image} bash -l -c "${inCmd}"`,
+        ],
+        outOpts: [dockerPullOpts, { cwd, encoding, env: envMock.basic }],
+      },
+    ],
   ];
 
   test.each(testInputs)('%s', async (_msg, testOpts) => {
-- 
GitLab