From dd26d269cb979d6e3b188b2d2e7bae8002d45480 Mon Sep 17 00:00:00 2001 From: Omar Tawfik <15987992+OmarTawfik@users.noreply.github.com> Date: Mon, 8 Jul 2024 00:02:23 -0700 Subject: [PATCH 1/5] add `inheritEnv` action parameter --- action.yml | 4 ++++ azdo-task/DevcontainersCi/src/main.ts | 3 ++- common/__tests__/envvars.test.ts | 19 ++++++++++++++++--- common/src/envvars.ts | 7 ++++++- docs/github-action.md | 1 + github-action/src/main.ts | 3 ++- 6 files changed, 31 insertions(+), 6 deletions(-) diff --git a/action.yml b/action.yml index 0a3aeee9d..35979b9f0 100644 --- a/action.yml +++ b/action.yml @@ -44,6 +44,10 @@ inputs: env: required: false description: Specify environment variables to pass to the docker run command + inheritEnv: + required: false + default: false + description: Inherit all environment variables of the runner CI machine. skipContainerUserIdUpdate: required: false default: false diff --git a/azdo-task/DevcontainersCi/src/main.ts b/azdo-task/DevcontainersCi/src/main.ts index 787871239..ba75f2114 100644 --- a/azdo-task/DevcontainersCi/src/main.ts +++ b/azdo-task/DevcontainersCi/src/main.ts @@ -44,7 +44,8 @@ export async function runMain(): Promise { const relativeConfigFile = task.getInput('configFile'); const runCommand = task.getInput('runCmd'); const envs = task.getInput('env')?.split('\n') ?? []; - const inputEnvsWithDefaults = populateDefaults(envs); + const inerhitEnv = (task.getInput('inerhitEnv') ?? 'false') === 'true'; + const inputEnvsWithDefaults = populateDefaults(envs, inerhitEnv); const cacheFrom = task.getInput('cacheFrom')?.split('\n') ?? []; const noCache = (task.getInput('noCache') ?? 'false') === 'true'; const skipContainerUserIdUpdate = diff --git a/common/__tests__/envvars.test.ts b/common/__tests__/envvars.test.ts index 6eb53b085..0a3a20b77 100644 --- a/common/__tests__/envvars.test.ts +++ b/common/__tests__/envvars.test.ts @@ -33,21 +33,34 @@ describe('substituteValues', () => { describe('populateDefaults', () => { test('returns original inputs when fully specified', () => { const input = ['TEST_ENV1=value1', 'TEST_ENV2=value2']; - const result = populateDefaults(input); + const result = populateDefaults(input, false); expect(result).toEqual(['TEST_ENV1=value1', 'TEST_ENV2=value2']); }); test('adds process env value when set and input value not provided', () => { const input = ['TEST_ENV1', 'TEST_ENV2=value2']; process.env.TEST_ENV1 = 'TestEnvValue1'; - const result = populateDefaults(input); + const result = populateDefaults(input, false); expect(result).toEqual(['TEST_ENV1=TestEnvValue1', 'TEST_ENV2=value2']); }); test('skips value when process env value not set and input value not provided', () => { const input = ['TEST_ENV1', 'TEST_ENV2=value2']; delete process.env.TEST_ENV1; - const result = populateDefaults(input); + const result = populateDefaults(input, false); expect(result).toEqual(['TEST_ENV2=value2']); }); + + test('inherits process env when asked', () => { + const originalEnv = structuredClone(process.env); + try { + process.env = {TEST_ENV1: 'value1'}; + const input = ['TEST_ENV2=value2']; + const result = populateDefaults(input, true); + expect(result).toEqual(['TEST_ENV1=value1', 'TEST_ENV2=value2']); + } + finally { + process.env = originalEnv; + } + }); }); diff --git a/common/src/envvars.ts b/common/src/envvars.ts index 5200152af..a216b3958 100644 --- a/common/src/envvars.ts +++ b/common/src/envvars.ts @@ -29,8 +29,13 @@ function getSubstitutionValue(regexMatch: string, placeholder: string): string { // In the latter case, the corresponding returned item would be "BAR=hi" // where the value is taken from the matching process env var. // In the case of values not set in the process, they are omitted -export function populateDefaults(envs: string[]): string[] { +export function populateDefaults(envs: string[], inerhitEnv: boolean): string[] { const result: string[] = []; + if (inerhitEnv) { + for (const [key, value] of Object.entries(process.env)) { + result.push(`${key}=${value}`); + } + } for (let i = 0; i < envs.length; i++) { const inputEnv = envs[i]; if (inputEnv.indexOf('=') >= 0) { diff --git a/docs/github-action.md b/docs/github-action.md index 8e5e69d95..4a1ff4e74 100644 --- a/docs/github-action.md +++ b/docs/github-action.md @@ -133,6 +133,7 @@ The [`devcontainers/ci` action](https://github.com/marketplace/actions/devcontai | configFile | false | Use this to specify the repo-relative path to the devcontainer.json file. Defaults to `./.devcontainer/devcontainer.json` and `./.devcontainer.json`. | | runCmd | true | The command to run after building the dev container image | | env | false | Specify environment variables to pass to the dev container when run | +| inheritEnv | false | Inherit all environment variables of the runner CI machine | | checkoutPath | false | Only used for development/testing | | push | false | Control when images are pushed. Options are `never`, `filter`, `always`. For `filter`, images are pushed if the `refFilterForPush` and `eventFilterForPush` conditions are met. Defaults to `filter` if `imageName` is set, `never` otherwise. | | refFilterForPush | false | Set the source branches (e.g. `refs/heads/main`) that are allowed to trigger a push of the dev container image. Leave empty to allow all (default) | diff --git a/github-action/src/main.ts b/github-action/src/main.ts index 3e98d59d7..0d3d3d2b2 100644 --- a/github-action/src/main.ts +++ b/github-action/src/main.ts @@ -53,7 +53,8 @@ export async function runMain(): Promise { ); const runCommand = core.getInput('runCmd'); const inputEnvs: string[] = core.getMultilineInput('env'); - const inputEnvsWithDefaults = populateDefaults(inputEnvs); + const inerhitEnv: boolean = core.getBooleanInput('inerhitEnv'); + const inputEnvsWithDefaults = populateDefaults(inputEnvs, inerhitEnv); const cacheFrom: string[] = core.getMultilineInput('cacheFrom'); const noCache: boolean = core.getBooleanInput('noCache'); const skipContainerUserIdUpdate = core.getBooleanInput( From 7fb3ac399261d3f64a405f43edbdb316bc33eb6f Mon Sep 17 00:00:00 2001 From: Omar Tawfik <15987992+OmarTawfik@users.noreply.github.com> Date: Tue, 9 Jul 2024 08:42:49 -0700 Subject: [PATCH 2/5] pr comments --- azdo-task/DevcontainersCi/task.json | 9 ++++++++- azdo-task/README.md | 1 + docs/azure-devops-task.md | 1 + 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/azdo-task/DevcontainersCi/task.json b/azdo-task/DevcontainersCi/task.json index 917090464..f8dd8bdde 100644 --- a/azdo-task/DevcontainersCi/task.json +++ b/azdo-task/DevcontainersCi/task.json @@ -64,6 +64,13 @@ "label": "Specify environment variables to pass to the docker run command", "required": false }, + { + "name": "inheritEnv", + "type": "boolean", + "label": "Inherit all environment variables of the runner CI machine", + "defaultValue": false, + "required": false + }, { "name": "push", "type": "pickList", @@ -133,4 +140,4 @@ "argumentFormat": "" } } -} \ No newline at end of file +} diff --git a/azdo-task/README.md b/azdo-task/README.md index 60a16b84c..9a602c4fd 100644 --- a/azdo-task/README.md +++ b/azdo-task/README.md @@ -74,6 +74,7 @@ In the example above, the devcontainer-build-run will perform the following step | configFile | false | Use this to specify the repo-relative path to the devcontainer.json file. Defaults to `./.devcontainer/devcontainer.json` and `./.devcontainer.json`. | | runCmd | true | The command to run after building the dev container image | | env | false | Specify environment variables to pass to the dev container when run | +| inheritEnv | false | Inherit all environment variables of the runner CI machine | | push | false | One of: `never`, `filter`, `always`. When set to `filter`, the image if pushed if the `sourceBranchFilterForPush`, `buildReasonsForPush`, and `pushOnFailedBuild` conditions are met. Defaults to `filter` if `imageName` is set, `never` otherwise. | | pushOnFailedBuild | false | If `false` (default), only push if the build is successful. Set to true to push on failed builds | | sourceBranchFilterForPush | false | Allows you to limit which branch's builds are pushed to the registry (only specified branches are allowed to push). If empty, all branches are allowed | diff --git a/docs/azure-devops-task.md b/docs/azure-devops-task.md index 60a16b84c..9a602c4fd 100644 --- a/docs/azure-devops-task.md +++ b/docs/azure-devops-task.md @@ -74,6 +74,7 @@ In the example above, the devcontainer-build-run will perform the following step | configFile | false | Use this to specify the repo-relative path to the devcontainer.json file. Defaults to `./.devcontainer/devcontainer.json` and `./.devcontainer.json`. | | runCmd | true | The command to run after building the dev container image | | env | false | Specify environment variables to pass to the dev container when run | +| inheritEnv | false | Inherit all environment variables of the runner CI machine | | push | false | One of: `never`, `filter`, `always`. When set to `filter`, the image if pushed if the `sourceBranchFilterForPush`, `buildReasonsForPush`, and `pushOnFailedBuild` conditions are met. Defaults to `filter` if `imageName` is set, `never` otherwise. | | pushOnFailedBuild | false | If `false` (default), only push if the build is successful. Set to true to push on failed builds | | sourceBranchFilterForPush | false | Allows you to limit which branch's builds are pushed to the registry (only specified branches are allowed to push). If empty, all branches are allowed | From e9a4cf086bc99929b969f58a6650410c229efdc0 Mon Sep 17 00:00:00 2001 From: Omar Tawfik <15987992+OmarTawfik@users.noreply.github.com> Date: Sat, 13 Jul 2024 01:20:12 -0700 Subject: [PATCH 3/5] fix typo --- azdo-task/DevcontainersCi/src/main.ts | 4 ++-- common/src/envvars.ts | 4 ++-- github-action/src/main.ts | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/azdo-task/DevcontainersCi/src/main.ts b/azdo-task/DevcontainersCi/src/main.ts index ba75f2114..9d0296365 100644 --- a/azdo-task/DevcontainersCi/src/main.ts +++ b/azdo-task/DevcontainersCi/src/main.ts @@ -44,8 +44,8 @@ export async function runMain(): Promise { const relativeConfigFile = task.getInput('configFile'); const runCommand = task.getInput('runCmd'); const envs = task.getInput('env')?.split('\n') ?? []; - const inerhitEnv = (task.getInput('inerhitEnv') ?? 'false') === 'true'; - const inputEnvsWithDefaults = populateDefaults(envs, inerhitEnv); + const inheritEnv = (task.getInput('inheritEnv') ?? 'false') === 'true'; + const inputEnvsWithDefaults = populateDefaults(envs, inheritEnv); const cacheFrom = task.getInput('cacheFrom')?.split('\n') ?? []; const noCache = (task.getInput('noCache') ?? 'false') === 'true'; const skipContainerUserIdUpdate = diff --git a/common/src/envvars.ts b/common/src/envvars.ts index a216b3958..ccd33ddb6 100644 --- a/common/src/envvars.ts +++ b/common/src/envvars.ts @@ -29,9 +29,9 @@ function getSubstitutionValue(regexMatch: string, placeholder: string): string { // In the latter case, the corresponding returned item would be "BAR=hi" // where the value is taken from the matching process env var. // In the case of values not set in the process, they are omitted -export function populateDefaults(envs: string[], inerhitEnv: boolean): string[] { +export function populateDefaults(envs: string[], inheritEnv: boolean): string[] { const result: string[] = []; - if (inerhitEnv) { + if (inheritEnv) { for (const [key, value] of Object.entries(process.env)) { result.push(`${key}=${value}`); } diff --git a/github-action/src/main.ts b/github-action/src/main.ts index 0d3d3d2b2..61a251357 100644 --- a/github-action/src/main.ts +++ b/github-action/src/main.ts @@ -53,8 +53,8 @@ export async function runMain(): Promise { ); const runCommand = core.getInput('runCmd'); const inputEnvs: string[] = core.getMultilineInput('env'); - const inerhitEnv: boolean = core.getBooleanInput('inerhitEnv'); - const inputEnvsWithDefaults = populateDefaults(inputEnvs, inerhitEnv); + const inheritEnv: boolean = core.getBooleanInput('inheritEnv'); + const inputEnvsWithDefaults = populateDefaults(inputEnvs, inheritEnv); const cacheFrom: string[] = core.getMultilineInput('cacheFrom'); const noCache: boolean = core.getBooleanInput('noCache'); const skipContainerUserIdUpdate = core.getBooleanInput( From 99d3db3b760acdf1d17fff96495888b46ce96588 Mon Sep 17 00:00:00 2001 From: Omar Tawfik <15987992+OmarTawfik@users.noreply.github.com> Date: Sat, 13 Jul 2024 01:25:44 -0700 Subject: [PATCH 4/5] skip copying `PATH` --- common/src/envvars.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/common/src/envvars.ts b/common/src/envvars.ts index ccd33ddb6..73b0f1e68 100644 --- a/common/src/envvars.ts +++ b/common/src/envvars.ts @@ -33,7 +33,14 @@ export function populateDefaults(envs: string[], inheritEnv: boolean): string[] const result: string[] = []; if (inheritEnv) { for (const [key, value] of Object.entries(process.env)) { - result.push(`${key}=${value}`); + switch (key) { + case 'PATH': + // don't copy these by default (user can still explicitly specify them). + break; + default: + result.push(`${key}=${value}`); + break; + } } } for (let i = 0; i < envs.length; i++) { From 3626859bf764fb08c126c79ee4644afe1c9cae81 Mon Sep 17 00:00:00 2001 From: OmarTawfik <15987992+OmarTawfik@users.noreply.github.com> Date: Wed, 17 Jul 2024 05:07:55 -0700 Subject: [PATCH 5/5] fix tests --- common/__tests__/envvars.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/__tests__/envvars.test.ts b/common/__tests__/envvars.test.ts index 0a3a20b77..e687e4bd2 100644 --- a/common/__tests__/envvars.test.ts +++ b/common/__tests__/envvars.test.ts @@ -52,7 +52,7 @@ describe('populateDefaults', () => { }); test('inherits process env when asked', () => { - const originalEnv = structuredClone(process.env); + const originalEnv = process.env; try { process.env = {TEST_ENV1: 'value1'}; const input = ['TEST_ENV2=value2'];