Implement yaml extensions overwriting the default pod/container spec (#75)

* Implement yaml extensions overwriting the default pod/container spec

* format files

* Extend specs for container job and include docker and k8s tests in k8s

* Create table tests for docker tests

* included warnings and extracted append logic as generic

* updated merge to allow for file read

* reverted back examples and k8s/tests

* reverted back docker tests

* Tests for extension prepare-job

* Fix lint and format and merge error

* Added basic test for container step

* revert hooklib since new definition for container options is received from a file

* revert docker options since create options are a string

* Fix revert

* Update package locks and deps

* included example of extension.yaml. Added side-car container that was missing

* Ignore spec modification for the service containers, change selector to

* fix lint error

* Add missing image override

* Add comment explaining merge object meta with job and pod

* fix test

packages/k8s/tests/k8s-utils-test.ts

@@ -1,10 +1,15 @@
-import * as fs from 'fs'
+import * as fs from 'fs'
 import { containerPorts, POD_VOLUME_NAME } from '../src/k8s'
 import {
   containerVolumes,
   generateContainerName,
-  writeEntryPointScript
+  writeEntryPointScript,
+  mergePodSpecWithOptions,
+  mergeContainerWithOptions,
+  readExtensionFromFile,
+  ENV_HOOK_TEMPLATE_PATH
 } from '../src/k8s/utils'
+import * as k8s from '@kubernetes/client-node'
 import { TestHelper } from './test-setup'
 
 let testHelper: TestHelper
@@ -328,4 +333,183 @@ describe('k8s utils', () => {
       expect(() => generateContainerName(':latest')).toThrow()
     })
   })
+
+  describe('read extension', () => {
+    beforeEach(async () => {
+      testHelper = new TestHelper()
+      await testHelper.initialize()
+    })
+
+    afterEach(async () => {
+      await testHelper.cleanup()
+    })
+
+    it('should throw if env variable is set but file does not exist', () => {
+      process.env[ENV_HOOK_TEMPLATE_PATH] =
+        '/path/that/does/not/exist/data.yaml'
+      expect(() => readExtensionFromFile()).toThrow()
+    })
+
+    it('should return undefined if env variable is not set', () => {
+      delete process.env[ENV_HOOK_TEMPLATE_PATH]
+      expect(readExtensionFromFile()).toBeUndefined()
+    })
+
+    it('should throw if file is empty', () => {
+      let filePath = testHelper.createFile('data.yaml')
+      process.env[ENV_HOOK_TEMPLATE_PATH] = filePath
+      expect(() => readExtensionFromFile()).toThrow()
+    })
+
+    it('should throw if file is not valid yaml', () => {
+      let filePath = testHelper.createFile('data.yaml')
+      fs.writeFileSync(filePath, 'invalid yaml')
+      process.env[ENV_HOOK_TEMPLATE_PATH] = filePath
+      expect(() => readExtensionFromFile()).toThrow()
+    })
+
+    it('should return object if file is valid', () => {
+      let filePath = testHelper.createFile('data.yaml')
+      fs.writeFileSync(
+        filePath,
+        `
+metadata:
+  labels:
+    label-name: label-value
+  annotations:
+    annotation-name: annotation-value
+spec:
+  containers:
+    - name: test
+      image: node:14.16
+    - name: job
+      image: ubuntu:latest`
+      )
+
+      process.env[ENV_HOOK_TEMPLATE_PATH] = filePath
+      const extension = readExtensionFromFile()
+      expect(extension).toBeDefined()
+    })
+  })
+
+  it('should merge container spec', () => {
+    const base = {
+      image: 'node:14.16',
+      name: 'test',
+      env: [
+        {
+          name: 'TEST',
+          value: 'TEST'
+        }
+      ],
+      ports: [
+        {
+          containerPort: 8080,
+          hostPort: 8080,
+          protocol: 'TCP'
+        }
+      ]
+    } as k8s.V1Container
+
+    const from = {
+      ports: [
+        {
+          containerPort: 9090,
+          hostPort: 9090,
+          protocol: 'TCP'
+        }
+      ],
+      env: [
+        {
+          name: 'TEST_TWO',
+          value: 'TEST_TWO'
+        }
+      ],
+      image: 'ubuntu:latest',
+      name: 'overwrite'
+    } as k8s.V1Container
+
+    const expectContainer = {
+      name: base.name,
+      image: base.image,
+      ports: [
+        ...(base.ports as k8s.V1ContainerPort[]),
+        ...(from.ports as k8s.V1ContainerPort[])
+      ],
+      env: [...(base.env as k8s.V1EnvVar[]), ...(from.env as k8s.V1EnvVar[])]
+    }
+
+    const expectJobContainer = JSON.parse(JSON.stringify(expectContainer))
+    expectJobContainer.name = base.name
+    mergeContainerWithOptions(base, from)
+    expect(base).toStrictEqual(expectContainer)
+  })
+
+  it('should merge pod spec', () => {
+    const base = {
+      containers: [
+        {
+          image: 'node:14.16',
+          name: 'test',
+          env: [
+            {
+              name: 'TEST',
+              value: 'TEST'
+            }
+          ],
+          ports: [
+            {
+              containerPort: 8080,
+              hostPort: 8080,
+              protocol: 'TCP'
+            }
+          ]
+        }
+      ],
+      restartPolicy: 'Never'
+    } as k8s.V1PodSpec
+
+    const from = {
+      securityContext: {
+        runAsUser: 1000,
+        fsGroup: 2000
+      },
+      restartPolicy: 'Always',
+      volumes: [
+        {
+          name: 'work',
+          emptyDir: {}
+        }
+      ],
+      containers: [
+        {
+          image: 'ubuntu:latest',
+          name: 'side-car',
+          env: [
+            {
+              name: 'TEST',
+              value: 'TEST'
+            }
+          ],
+          ports: [
+            {
+              containerPort: 8080,
+              hostPort: 8080,
+              protocol: 'TCP'
+            }
+          ]
+        }
+      ]
+    } as k8s.V1PodSpec
+
+    const expected = JSON.parse(JSON.stringify(base))
+    expected.securityContext = from.securityContext
+    expected.restartPolicy = from.restartPolicy
+    expected.volumes = from.volumes
+    expected.containers.push(from.containers[0])
+
+    mergePodSpecWithOptions(base, from)
+
+    expect(base).toStrictEqual(expected)
+  })
 })

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

@@ -3,8 +3,15 @@ import * as path from 'path'
 import { cleanupJob } from '../src/hooks'
 import { createContainerSpec, prepareJob } from '../src/hooks/prepare-job'
 import { TestHelper } from './test-setup'
-import { generateContainerName } from '../src/k8s/utils'
+import {
+  ENV_HOOK_TEMPLATE_PATH,
+  generateContainerName,
+  readExtensionFromFile
+} from '../src/k8s/utils'
+import { getPodByName } from '../src/k8s'
 import { V1Container } from '@kubernetes/client-node'
+import * as yaml from 'js-yaml'
+import { JOB_CONTAINER_NAME } from '../src/hooks/constants'
 
 jest.useRealTimers()
 
@@ -83,6 +90,46 @@ describe('Prepare job', () => {
     expect(services[0].args).toBe(undefined)
   })
 
+  it('should run pod with extensions applied', async () => {
+    process.env[ENV_HOOK_TEMPLATE_PATH] = path.join(
+      __dirname,
+      '../../../examples/extension.yaml'
+    )
+
+    await expect(
+      prepareJob(prepareJobData.args, prepareJobOutputFilePath)
+    ).resolves.not.toThrow()
+
+    delete process.env[ENV_HOOK_TEMPLATE_PATH]
+
+    const content = JSON.parse(
+      fs.readFileSync(prepareJobOutputFilePath).toString()
+    )
+
+    const got = await getPodByName(content.state.jobPod)
+
+    expect(got.metadata?.annotations?.['annotated-by']).toBe('extension')
+    expect(got.metadata?.labels?.['labeled-by']).toBe('extension')
+    expect(got.spec?.securityContext?.runAsUser).toBe(1000)
+    expect(got.spec?.securityContext?.runAsGroup).toBe(3000)
+
+    // job container
+    expect(got.spec?.containers[0].name).toBe(JOB_CONTAINER_NAME)
+    expect(got.spec?.containers[0].image).toBe('node:14.16')
+    expect(got.spec?.containers[0].command).toEqual(['sh'])
+    expect(got.spec?.containers[0].args).toEqual(['-c', 'sleep 50'])
+
+    // service container
+    expect(got.spec?.containers[1].image).toBe('redis')
+    expect(got.spec?.containers[1].command).toBeFalsy()
+    expect(got.spec?.containers[1].args).toBeFalsy()
+    // side-car
+    expect(got.spec?.containers[2].name).toBe('side-car')
+    expect(got.spec?.containers[2].image).toBe('ubuntu:latest')
+    expect(got.spec?.containers[2].command).toEqual(['sh'])
+    expect(got.spec?.containers[2].args).toEqual(['-c', 'sleep 60'])
+  })
+
   test.each([undefined, null, []])(
     'should not throw exception when portMapping=%p',
     async pm => {

packages/k8s/tests/run-container-step-test.ts

@@ -1,5 +1,9 @@
 import { runContainerStep } from '../src/hooks'
 import { TestHelper } from './test-setup'
+import { ENV_HOOK_TEMPLATE_PATH } from '../src/k8s/utils'
+import * as fs from 'fs'
+import * as yaml from 'js-yaml'
+import { JOB_CONTAINER_EXTENSION_NAME } from '../src/hooks/constants'
 
 jest.useRealTimers()
 
@@ -23,6 +27,47 @@ describe('Run container step', () => {
     expect(exitCode).toBe(0)
   })
 
+  it('should run pod with extensions applied', async () => {
+    const extension = {
+      metadata: {
+        annotations: {
+          foo: 'bar'
+        },
+        labels: {
+          bar: 'baz'
+        }
+      },
+      spec: {
+        containers: [
+          {
+            name: JOB_CONTAINER_EXTENSION_NAME,
+            command: ['sh'],
+            args: ['-c', 'echo test']
+          },
+          {
+            name: 'side-container',
+            image: 'ubuntu:latest',
+            command: ['sh'],
+            args: ['-c', 'echo test']
+          }
+        ],
+        restartPolicy: 'Never',
+        securityContext: {
+          runAsUser: 1000,
+          runAsGroup: 3000
+        }
+      }
+    }
+
+    let filePath = testHelper.createFile()
+    fs.writeFileSync(filePath, yaml.dump(extension))
+    process.env[ENV_HOOK_TEMPLATE_PATH] = filePath
+    await expect(
+      runContainerStep(runContainerStepData.args)
+    ).resolves.not.toThrow()
+    delete process.env[ENV_HOOK_TEMPLATE_PATH]
+  })
+
   it('should shold have env variables available', async () => {
     runContainerStepData.args.entryPoint = 'bash'
     runContainerStepData.args.entryPointArgs = [