Merge pull request #2 from actions/nikola-jokic/computed-build-directory

Computed action build directory. Refactored tests and added docker build test

packages/docker/src/dockerCommands/container.ts

@@ -2,10 +2,11 @@ import * as core from '@actions/core'
 import * as fs from 'fs'
 import {
   ContainerInfo,
+  Registry,
   RunContainerStepArgs,
   ServiceContainerInfo
 } from 'hooklib/lib'
-import path from 'path'
+import * as path from 'path'
 import { env } from 'process'
 import { v4 as uuidv4 } from 'uuid'
 import { runDockerCommand, RunDockerCommandOptions } from '../utils'
@@ -41,13 +42,9 @@ export async function createContainer(
   }
 
   if (args.environmentVariables) {
-    for (const [key, value] of Object.entries(args.environmentVariables)) {
+    for (const [key] of Object.entries(args.environmentVariables)) {
       dockerArgs.push('-e')
-      if (!value) {
-        dockerArgs.push(`"${key}"`)
-      } else {
-        dockerArgs.push(`"${key}=${value}"`)
-      }
+      dockerArgs.push(key)
     }
   }
 
@@ -144,17 +141,41 @@ export async function containerBuild(
   args: RunContainerStepArgs,
   tag: string
 ): Promise<void> {
-  const context = path.dirname(`${env.GITHUB_WORKSPACE}/${args.dockerfile}`)
+  if (!args.dockerfile) {
+    throw new Error("Container build expects 'args.dockerfile' to be set")
+  }
+
   const dockerArgs: string[] = ['build']
   dockerArgs.push('-t', tag)
-  dockerArgs.push('-f', `${env.GITHUB_WORKSPACE}/${args.dockerfile}`)
-  dockerArgs.push(context)
+  dockerArgs.push('-f', args.dockerfile)
+  dockerArgs.push(getBuildContext(args.dockerfile))
   // TODO: figure out build working directory
   await runDockerCommand(dockerArgs, {
-    workingDir: args['buildWorkingDirectory']
+    workingDir: getWorkingDir(args.dockerfile)
   })
 }
 
+function getBuildContext(dockerfilePath: string): string {
+  return path.dirname(dockerfilePath)
+}
+
+function getWorkingDir(dockerfilePath: string): string {
+  const workspace = env.GITHUB_WORKSPACE as string
+  let workingDir = workspace
+  if (!dockerfilePath?.includes(workspace)) {
+    // This is container action
+    const pathSplit = dockerfilePath.split('/')
+    const actionIndex = pathSplit?.findIndex(d => d === '_actions')
+    if (actionIndex) {
+      const actionSubdirectoryDepth = 3 // handle + repo + [branch | tag]
+      pathSplit.splice(actionIndex + actionSubdirectoryDepth + 1)
+      workingDir = pathSplit.join('/')
+    }
+  }
+
+  return workingDir
+}
+
 export async function containerLogs(id: string): Promise<void> {
   const dockerArgs: string[] = ['logs']
   dockerArgs.push('--details')
@@ -248,22 +269,22 @@ export async function healthCheck({
 export async function containerPorts(id: string): Promise<string[]> {
   const dockerArgs = ['port', id]
   const portMappings = (await runDockerCommand(dockerArgs)).trim()
-  return portMappings.split('\n')
+  return portMappings.split('\n').filter(p => !!p)
 }
 
-export async function registryLogin(args): Promise<string> {
-  if (!args.registry) {
+export async function registryLogin(registry?: Registry): Promise<string> {
+  if (!registry) {
     return ''
   }
   const credentials = {
-    username: args.registry.username,
-    password: args.registry.password
+    username: registry.username,
+    password: registry.password
   }
 
   const configLocation = `${env.RUNNER_TEMP}/.docker_${uuidv4()}`
   fs.mkdirSync(configLocation)
   try {
-    await dockerLogin(configLocation, args.registry.serverUrl, credentials)
+    await dockerLogin(configLocation, registry.serverUrl, credentials)
   } catch (error) {
     fs.rmdirSync(configLocation, { recursive: true })
     throw error
@@ -281,7 +302,7 @@ export async function registryLogout(configLocation: string): Promise<void> {
 async function dockerLogin(
   configLocation: string,
   registry: string,
-  credentials: { username: string; password: string }
+  credentials: { username?: string; password?: string }
 ): Promise<void> {
   const credentialsArgs =
     credentials.username && credentials.password
@@ -317,13 +338,9 @@ export async function containerExecStep(
 ): Promise<void> {
   const dockerArgs: string[] = ['exec', '-i']
   dockerArgs.push(`--workdir=${args.workingDirectory}`)
-  for (const [key, value] of Object.entries(args['environmentVariables'])) {
+  for (const [key] of Object.entries(args['environmentVariables'])) {
     dockerArgs.push('-e')
-    if (!value) {
-      dockerArgs.push(`"${key}"`)
-    } else {
-      dockerArgs.push(`"${key}=${value}"`)
-    }
+    dockerArgs.push(key)
   }
 
   if (args.prependPath?.length) {
@@ -341,7 +358,7 @@ export async function containerExecStep(
 export async function containerRun(
   args: RunContainerStepArgs,
   name: string,
-  network: string
+  network?: string
 ): Promise<void> {
   if (!args.image) {
     throw new Error('expected image to be set')
@@ -351,7 +368,9 @@ export async function containerRun(
   dockerArgs.push('--name', name)
   dockerArgs.push(`--workdir=${args.workingDirectory}`)
   dockerArgs.push(`--label=${getRunnerLabel()}`)
-  dockerArgs.push(`--network=${network}`)
+  if (network) {
+    dockerArgs.push(`--network=${network}`)
+  }
 
   if (args.createOptions) {
     dockerArgs.push(...args.createOptions.split(' '))

packages/docker/src/hooks/prepare-job.ts

@@ -186,15 +186,15 @@ function transformDockerPortsToContextPorts(
   meta: ContainerMetadata
 ): ContextPorts {
   // ex: '80/tcp -> 0.0.0.0:80'
-  const re = /^(\d+)\/(\w+)? -> (.*):(\d+)$/
+  const re = /^(\d+)(\/\w+)? -> (.*):(\d+)$/
   const contextPorts: ContextPorts = {}
 
-  if (meta.ports) {
+  if (meta.ports?.length) {
     for (const port of meta.ports) {
       const matches = port.match(re)
       if (!matches) {
         throw new Error(
-          'Container ports could not match the regex: "^(\\d+)\\/(\\w+)? -> (.*):(\\d+)$"'
+          'Container ports could not match the regex: "^(\\d+)(\\/\\w+)? -> (.*):(\\d+)$"'
         )
       }
       contextPorts[matches[1]] = matches[matches.length - 1]

packages/docker/src/hooks/run-container-step.ts

@@ -1,13 +1,12 @@
+import { RunContainerStepArgs } from 'hooklib/lib'
+import { v4 as uuidv4 } from 'uuid'
 import {
   containerBuild,
-  registryLogin,
-  registryLogout,
   containerPull,
-  containerRun
+  containerRun,
+  registryLogin,
+  registryLogout
 } from '../dockerCommands'
-import { v4 as uuidv4 } from 'uuid'
-import * as core from '@actions/core'
-import { RunContainerStepArgs } from 'hooklib/lib'
 import { getRunnerLabel } from '../dockerCommands/constants'
 
 export async function runContainerStep(
@@ -15,23 +14,23 @@ export async function runContainerStep(
   state
 ): Promise<void> {
   const tag = generateBuildTag() // for docker build
-  if (!args.image) {
-    core.error('expected an image')
-  } else {
-    if (args.dockerfile) {
-      await containerBuild(args, tag)
-      args.image = tag
-    } else {
-      const configLocation = await registryLogin(args)
-      try {
-        await containerPull(args.image, configLocation)
-      } finally {
-        await registryLogout(configLocation)
-      }
+  if (args.image) {
+    const configLocation = await registryLogin(args.registry)
+    try {
+      await containerPull(args.image, configLocation)
+    } finally {
+      await registryLogout(configLocation)
     }
+  } else if (args.dockerfile) {
+    await containerBuild(args, tag)
+    args.image = tag
+  } else {
+    throw new Error(
+      'run container step should have image or dockerfile fields specified'
+    )
   }
   // container will get pruned at the end of the job based on the label, no need to cleanup here
-  await containerRun(args, tag.split(':')[1], state.network)
+  await containerRun(args, tag.split(':')[1], state?.network)
 }
 
 function generateBuildTag(): string {

packages/docker/src/index.ts

@@ -13,6 +13,7 @@ import {
   runContainerStep,
   runScriptStep
 } from './hooks'
+import { checkEnvironment } from './utils'
 
 async function run(): Promise<void> {
   const input = await getInputFromStdin()
@@ -23,6 +24,7 @@ async function run(): Promise<void> {
   const state = input['state']
 
   try {
+    checkEnvironment()
     switch (command) {
       case Command.PrepareJob:
         await prepareJob(args as PrepareJobArgs, responseFile)

packages/docker/src/utils.ts

@@ -2,6 +2,7 @@
 /* eslint-disable @typescript-eslint/no-require-imports */
 /* eslint-disable import/no-commonjs */
 import * as core from '@actions/core'
+import { env } from 'process'
 // Import this way otherwise typescript has errors
 const exec = require('@actions/exec')
 
@@ -42,6 +43,12 @@ export function sanitize(val: string): string {
   return newNameBuilder.join('')
 }
 
+export function checkEnvironment(): void {
+  if (!env.GITHUB_WORKSPACE) {
+    throw new Error('GITHUB_WORKSPACE is not set')
+  }
+}
+
 // isAlpha accepts single character and checks if
 // that character is [a-zA-Z]
 function isAlpha(val: string): boolean {

packages/docker/tests/cleanup-job-test.ts

@@ -21,6 +21,11 @@ describe('cleanup job', () => {
     const prepareJobOutput = testSetup.createOutputFile(
       'prepare-job-output.json'
     )
+
+    prepareJobDefinition.args.container.registry = null
+    prepareJobDefinition.args.services.forEach(s => {
+      s.registry = null
+    })
     await prepareJob(prepareJobDefinition.args, prepareJobOutput)
   })
 

packages/docker/tests/container-build-test.ts

@@ -0,0 +1,33 @@
+import * as fs from 'fs'
+import { containerBuild } from '../src/dockerCommands'
+import TestSetup from './test-setup'
+
+let testSetup
+let runContainerStepDefinition
+const runContainerStepInputPath = `${__dirname}/../../../examples/run-container-step.json`
+
+describe('container build', () => {
+  beforeEach(() => {
+    testSetup = new TestSetup()
+    testSetup.initialize()
+
+    let runContainerStepJson = fs.readFileSync(
+      runContainerStepInputPath,
+      'utf8'
+    )
+    runContainerStepDefinition = JSON.parse(runContainerStepJson.toString())
+    runContainerStepDefinition.image = ''
+    const actionPath = testSetup.initializeDockerAction()
+    runContainerStepDefinition.dockerfile = `${actionPath}/Dockerfile`
+  })
+
+  afterEach(() => {
+    testSetup.teardown()
+  })
+
+  it('should build container', async () => {
+    await expect(
+      containerBuild(runContainerStepDefinition, 'example-test-tag')
+    ).resolves.not.toThrow()
+  })
+})

packages/docker/tests/e2e-test.ts

@@ -39,6 +39,10 @@ describe('e2e', () => {
     testSetup.initialize()
     definitions.prepareJob.args.container.systemMountVolumes =
       testSetup.systemMountVolumes
+    definitions.prepareJob.args.container.registry = null
+    definitions.prepareJob.args.services.forEach(s => {
+      s.registry = null
+    })
   })
 
   afterEach(() => {

packages/docker/tests/prepare-job-test.ts

@@ -19,6 +19,10 @@ describe('prepare job', () => {
       testSetup.systemMountVolumes
     prepareJobDefinition.args.container.workingDirectory =
       testSetup.workingDirectory
+    prepareJobDefinition.args.container.registry = null
+    prepareJobDefinition.args.services.forEach(s => {
+      s.registry = null
+    })
   })
 
   afterEach(() => {

packages/docker/tests/run-script-step.ts

@@ -34,6 +34,10 @@ describe('run-script-step', () => {
     const prepareJobOutput = testSetup.createOutputFile(
       'prepare-job-output.json'
     )
+    definitions.prepareJob.args.container.registry = null
+    definitions.prepareJob.args.services.forEach(s => {
+      s.registry = null
+    })
     await prepareJob(definitions.prepareJob.args, prepareJobOutput)
 
     prepareJobResponse = JSON.parse(fs.readFileSync(prepareJobOutput, 'utf-8'))

packages/docker/tests/test-setup.ts

@@ -115,4 +115,29 @@ export default class TestSetup {
   public get containerWorkingDirectory(): string {
     return `/__w/${this.projectName}/${this.projectName}`
   }
+
+  public initializeDockerAction(): string {
+    const actionPath = `${this.testdir}/_actions/example-handle/example-repo/example-branch/mock-directory`
+    fs.mkdirSync(actionPath, { recursive: true })
+    this.writeDockerfile(actionPath)
+    this.writeEntrypoint(actionPath)
+    return actionPath
+  }
+
+  private writeDockerfile(actionPath: string) {
+    const content = `FROM alpine:3.10
+COPY entrypoint.sh /entrypoint.sh
+ENTRYPOINT ["/entrypoint.sh"]`
+    fs.writeFileSync(`${actionPath}/Dockerfile`, content)
+  }
+
+  private writeEntrypoint(actionPath) {
+    const content = `#!/bin/sh -l
+echo "Hello $1"
+time=$(date)
+echo "::set-output name=time::$time"`
+    const entryPointPath = `${actionPath}/entrypoint.sh`
+    fs.writeFileSync(entryPointPath, content)
+    fs.chmodSync(entryPointPath, 0o755)
+  }
 }