From e928fa3252e29a6e001d9212f384e2f2175f4f77 Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Mon, 6 Jun 2022 18:43:57 -0400 Subject: [PATCH] Pass secrets more securely for container action --- packages/k8s/src/hooks/cleanup-job.ts | 5 +- packages/k8s/src/hooks/constants.ts | 9 ++- packages/k8s/src/hooks/prepare-job.ts | 8 +-- packages/k8s/src/hooks/run-container-step.ts | 29 ++++++---- packages/k8s/src/k8s/index.ts | 58 ++++++++++++++++--- packages/k8s/tests/run-container-step-test.ts | 2 - 6 files changed, 84 insertions(+), 27 deletions(-) diff --git a/packages/k8s/src/hooks/cleanup-job.ts b/packages/k8s/src/hooks/cleanup-job.ts index cecb88e..b36dd57 100644 --- a/packages/k8s/src/hooks/cleanup-job.ts +++ b/packages/k8s/src/hooks/cleanup-job.ts @@ -1,5 +1,6 @@ -import { podPrune } from '../k8s' +import { pruneSecrets, prunePods } from '../k8s' export async function cleanupJob(): Promise { - await podPrune() + await prunePods() + await pruneSecrets() } diff --git a/packages/k8s/src/hooks/constants.ts b/packages/k8s/src/hooks/constants.ts index 8218293..5df2257 100644 --- a/packages/k8s/src/hooks/constants.ts +++ b/packages/k8s/src/hooks/constants.ts @@ -20,7 +20,7 @@ export function getJobPodName(): string { export function getStepPodName(): string { return `${getRunnerPodName().substring( 0, - MAX_POD_NAME_LENGTH - ('-step'.length + STEP_POD_NAME_SUFFIX_LENGTH) + MAX_POD_NAME_LENGTH - ('-step-'.length + STEP_POD_NAME_SUFFIX_LENGTH) )}-step-${uuidv4().substring(0, STEP_POD_NAME_SUFFIX_LENGTH)}` } @@ -34,6 +34,13 @@ export function getVolumeClaimName(): string { return name } +export function getSecretName(): string { + return `${getRunnerPodName().substring( + 0, + MAX_POD_NAME_LENGTH - ('-secret-'.length + STEP_POD_NAME_SUFFIX_LENGTH) + )}-secret-${uuidv4().substring(0, STEP_POD_NAME_SUFFIX_LENGTH)}` +} + const MAX_POD_NAME_LENGTH = 63 const STEP_POD_NAME_SUFFIX_LENGTH = 8 export const JOB_CONTAINER_NAME = 'job' diff --git a/packages/k8s/src/hooks/prepare-job.ts b/packages/k8s/src/hooks/prepare-job.ts index 9a62f93..0549563 100644 --- a/packages/k8s/src/hooks/prepare-job.ts +++ b/packages/k8s/src/hooks/prepare-job.ts @@ -14,7 +14,7 @@ import { isAuthPermissionsOK, isPodContainerAlpine, namespace, - podPrune, + prunePods, requiredPermissions, waitForPodPhases } from '../k8s' @@ -29,7 +29,7 @@ export async function prepareJob( args: prepareJobArgs, responseFile ): Promise { - await podPrune() + await prunePods() if (!(await isAuthPermissionsOK())) { throw new Error( `The Service account needs the following permissions ${JSON.stringify( @@ -58,7 +58,7 @@ export async function prepareJob( try { createdPod = await createPod(container, services, args.registry) } catch (err) { - await podPrune() + await prunePods() throw new Error(`failed to create job pod: ${JSON.stringify(err)}`) } @@ -73,7 +73,7 @@ export async function prepareJob( new Set([PodPhase.PENDING]) ) } catch (err) { - await podPrune() + await prunePods() throw new Error(`Pod failed to come online with error: ${err}`) } diff --git a/packages/k8s/src/hooks/run-container-step.ts b/packages/k8s/src/hooks/run-container-step.ts index 244f438..7de4dfa 100644 --- a/packages/k8s/src/hooks/run-container-step.ts +++ b/packages/k8s/src/hooks/run-container-step.ts @@ -3,6 +3,7 @@ import * as core from '@actions/core' import { PodPhase } from 'hooklib' import { createJob, + createSecretForEnvs, getContainerJobPodName, getPodLogs, getPodStatus, @@ -16,7 +17,13 @@ export async function runContainerStep(stepContainer): Promise { if (stepContainer.dockerfile) { throw new Error('Building container actions is not currently supported') } - const container = createPodSpec(stepContainer) + let secretName: string | undefined = undefined + if (stepContainer['environmentVariables']) { + secretName = await createSecretForEnvs( + stepContainer['environmentVariables'] + ) + } + const container = createPodSpec(stepContainer, secretName) const job = await createJob(container) if (!job.metadata?.name) { throw new Error( @@ -39,28 +46,28 @@ export async function runContainerStep(stepContainer): Promise { core.warning(`Can't determine container status`) return 0 } - const exitCode = status.containerStatuses[status.containerStatuses.length - 1].state ?.terminated?.exitCode return Number(exitCode) || 0 } -function createPodSpec(container): k8s.V1Container { +function createPodSpec(container, secretName?: string): k8s.V1Container { const podContainer = new k8s.V1Container() podContainer.name = JOB_CONTAINER_NAME podContainer.image = container.image if (container.entryPoint) { podContainer.command = [container.entryPoint, ...container.entryPointArgs] } - - podContainer.env = [] - for (const [key, value] of Object.entries( - container['environmentVariables'] - )) { - if (value && key !== 'HOME') { - podContainer.env.push({ name: key, value: value as string }) - } + if (secretName) { + podContainer.envFrom = [ + { + secretRef: { + name: secretName, + optional: false + } + } + ] } podContainer.volumeMounts = containerVolumes() diff --git a/packages/k8s/src/k8s/index.ts b/packages/k8s/src/k8s/index.ts index fef66ab..d5175de 100644 --- a/packages/k8s/src/k8s/index.ts +++ b/packages/k8s/src/k8s/index.ts @@ -1,10 +1,10 @@ import * as k8s from '@kubernetes/client-node' import { ContainerInfo, PodPhase, Registry } from 'hooklib' import * as stream from 'stream' -import { v4 as uuidv4 } from 'uuid' import { getJobPodName, getRunnerPodName, + getSecretName, getStepPodName, getVolumeClaimName, RunnerInstanceLabel @@ -251,12 +251,13 @@ export async function createDockerSecret( } } } - const secretName = generateSecretName() + const secretName = getSecretName() const secret = new k8s.V1Secret() secret.immutable = true secret.apiVersion = 'v1' secret.metadata = new k8s.V1ObjectMeta() secret.metadata.name = secretName + secret.metadata.labels = { 'runner-pod': getRunnerPodName() } secret.kind = 'Secret' secret.data = { '.dockerconfigjson': Buffer.from( @@ -269,6 +270,53 @@ export async function createDockerSecret( return body } +export async function createSecretForEnvs(envs: { + [key: string]: string +}): Promise { + const secret = new k8s.V1Secret() + const secretName = getSecretName() + secret.immutable = true + secret.apiVersion = 'v1' + secret.metadata = new k8s.V1ObjectMeta() + secret.metadata.name = secretName + secret.metadata.labels = { 'runner-pod': getRunnerPodName() } + secret.kind = 'Secret' + secret.data = {} + for (const [key, value] of Object.entries(envs)) { + secret.data[key] = Buffer.from(value).toString('base64') + } + try { + await k8sApi.createNamespacedSecret(namespace(), secret) + } catch (e) { + throw e + } + return secretName +} + +export async function deleteSecret(secretName: string): Promise { + await k8sApi.deleteNamespacedSecret(secretName, namespace()) +} + +export async function pruneSecrets(): Promise { + const secretList = await k8sApi.listNamespacedSecret( + namespace(), + undefined, + undefined, + undefined, + undefined, + new RunnerInstanceLabel().toString() + ) + if (!secretList.body.items.length) { + return + } + + await Promise.all( + secretList.body.items.map( + secret => secret.metadata?.name && deleteSecret(secret.metadata.name) + ) + ) +} + export async function waitForPodPhases( podName: string, awaitingPhases: Set, @@ -346,7 +394,7 @@ export async function getPodLogs( await new Promise(resolve => r.on('close', () => resolve(null))) } -export async function podPrune(): Promise { +export async function prunePods(): Promise { const podList = await k8sApi.listNamespacedPod( namespace(), undefined, @@ -460,10 +508,6 @@ export function namespace(): string { return context.namespace } -function generateSecretName(): string { - return `github-secret-${uuidv4()}` -} - function runnerName(): string { const name = process.env.ACTIONS_RUNNER_POD_NAME if (!name) { diff --git a/packages/k8s/tests/run-container-step-test.ts b/packages/k8s/tests/run-container-step-test.ts index d92984f..a28512a 100644 --- a/packages/k8s/tests/run-container-step-test.ts +++ b/packages/k8s/tests/run-container-step-test.ts @@ -27,7 +27,5 @@ describe('Run container step', () => { }) afterEach(async () => { await testHelper.cleanup() - // wait for the job cleanup - await new Promise(resolve => setTimeout(resolve, 300 * 1000)) }) })