From 898063bdddeb63e23de042e75b817d0d3f21a7c7 Mon Sep 17 00:00:00 2001 From: Nikola Jokic <97525037+nikola-jokic@users.noreply.github.com> Date: Thu, 16 Jun 2022 15:44:40 +0200 Subject: [PATCH] repaired docker PATH export and added tests both for docker and k8s (#17) * repaired docker PATH export and added tests both for docker and k8s * added todo comments about next major version and typeof prepend path --- .../docker/src/dockerCommands/container.ts | 25 +++++++++++++- packages/docker/tests/run-script-step-test.ts | 24 +++++++++++++ packages/k8s/src/k8s/utils.ts | 5 ++- packages/k8s/tests/run-script-step-test.ts | 34 +++++++++++++++++++ 4 files changed, 86 insertions(+), 2 deletions(-) diff --git a/packages/docker/src/dockerCommands/container.ts b/packages/docker/src/dockerCommands/container.ts index f3d9246..68c3f26 100644 --- a/packages/docker/src/dockerCommands/container.ts +++ b/packages/docker/src/dockerCommands/container.ts @@ -274,6 +274,20 @@ export async function containerPorts(id: string): Promise { return portMappings.split('\n').filter(p => !!p) } +export async function getContainerEnvValue( + id: string, + name: string +): Promise { + const dockerArgs = [ + 'inspect', + `--format='{{range $index, $value := .Config.Env}}{{if eq (index (split $value "=") 0) "${name}"}}{{index (split $value "=") 1}}{{end}}{{end}}'`, + id + ] + const value = (await runDockerCommand(dockerArgs)).trim() + const lines = value.split('\n') + return lines.length ? lines[0].replace(/^'/, '').replace(/'$/, '') : '' +} + export async function registryLogin(registry?: Registry): Promise { if (!registry) { return '' @@ -346,7 +360,16 @@ export async function containerExecStep( } if (args.prependPath?.length) { - dockerArgs.push('-e', `"PATH=${args.prependPath.join(':')}:$PATH"`) + // TODO: remove compatibility with typeof prependPath === 'string' as we bump to next major version, the hooks will lose PrependPath compat with runners 2.293.0 and older + const prependPath = + typeof args.prependPath === 'string' + ? args.prependPath + : args.prependPath.join(':') + + dockerArgs.push( + '-e', + `PATH=${prependPath}:${await getContainerEnvValue(containerId, 'PATH')}` + ) } dockerArgs.push(containerId) diff --git a/packages/docker/tests/run-script-step-test.ts b/packages/docker/tests/run-script-step-test.ts index 429ae3b..227657e 100644 --- a/packages/docker/tests/run-script-step-test.ts +++ b/packages/docker/tests/run-script-step-test.ts @@ -34,4 +34,28 @@ describe('run script step', () => { runScriptStep(definitions.runScriptStep.args, prepareJobResponse.state) ).resolves.not.toThrow() }) + + it('Should have path variable changed in container with prepend path string', async () => { + definitions.runScriptStep.args.prependPath = '/some/path' + definitions.runScriptStep.args.entryPoint = '/bin/bash' + definitions.runScriptStep.args.entryPointArgs = [ + '-c', + `if [[ ! $(env | grep "^PATH=") = "PATH=${definitions.runScriptStep.args.prependPath}:"* ]]; then exit 1; fi` + ] + await expect( + runScriptStep(definitions.runScriptStep.args, prepareJobResponse.state) + ).resolves.not.toThrow() + }) + + it('Should have path variable changed in container with prepend path string array', async () => { + definitions.runScriptStep.args.prependPath = ['/some/other/path'] + definitions.runScriptStep.args.entryPoint = '/bin/bash' + definitions.runScriptStep.args.entryPointArgs = [ + '-c', + `if [[ ! $(env | grep "^PATH=") = "PATH=${definitions.runScriptStep.args.prependPath}:"* ]]; then exit 1; fi` + ] + await expect( + runScriptStep(definitions.runScriptStep.args, prepareJobResponse.state) + ).resolves.not.toThrow() + }) }) diff --git a/packages/k8s/src/k8s/utils.ts b/packages/k8s/src/k8s/utils.ts index 2cf7d5f..9bdf5ec 100644 --- a/packages/k8s/src/k8s/utils.ts +++ b/packages/k8s/src/k8s/utils.ts @@ -100,7 +100,10 @@ export function writeEntryPointScript( ): { containerPath: string; runnerPath: string } { let exportPath = '' if (prependPath?.length) { - exportPath = `export PATH=${prependPath.join(':')}:$PATH` + // TODO: remove compatibility with typeof prependPath === 'string' as we bump to next major version, the hooks will lose PrependPath compat with runners 2.293.0 and older + const prepend = + typeof prependPath === 'string' ? prependPath : prependPath.join(':') + exportPath = `export PATH=${prepend}:$PATH` } let environmentPrefix = '' diff --git a/packages/k8s/tests/run-script-step-test.ts b/packages/k8s/tests/run-script-step-test.ts index a565b4f..70554ee 100644 --- a/packages/k8s/tests/run-script-step-test.ts +++ b/packages/k8s/tests/run-script-step-test.ts @@ -71,4 +71,38 @@ describe('Run script step', () => { ) ).resolves.not.toThrow() }) + + it('Should have path variable changed in container with prepend path string', async () => { + runScriptStepDefinition.args.prependPath = '/some/path' + runScriptStepDefinition.args.entryPoint = '/bin/bash' + runScriptStepDefinition.args.entryPointArgs = [ + '-c', + `'if [[ ! $(env | grep "^PATH=") = "PATH=${runScriptStepDefinition.args.prependPath}:"* ]]; then exit 1; fi'` + ] + + await expect( + runScriptStep( + runScriptStepDefinition.args, + prepareJobOutputData.state, + null + ) + ).resolves.not.toThrow() + }) + + it('Should have path variable changed in container with prepend path string array', async () => { + runScriptStepDefinition.args.prependPath = ['/some/other/path'] + runScriptStepDefinition.args.entryPoint = '/bin/bash' + runScriptStepDefinition.args.entryPointArgs = [ + '-c', + `'if [[ ! $(env | grep "^PATH=") = "PATH=${runScriptStepDefinition.args.prependPath}:"* ]]; then exit 1; fi'` + ] + + await expect( + runScriptStep( + runScriptStepDefinition.args, + prepareJobOutputData.state, + null + ) + ).resolves.not.toThrow() + }) })