Updater now raises on container failures or empty output

Co-authored-by: Barry Gordon <896971+brrygrdn@users.noreply.github.com>

__fixtures__/output/happy_path/output.json

@@ -0,0 +1 @@
+{"base64_dependency_files":[{"name":"go.mod","content":"bW9kdWxlIGdpdGh1Yi5jb20vZGVwZW5kYWJvdC92Z290ZXN0CgpnbyAxLjEy\nCgpyZXF1aXJlIHJzYy5pby9xciB2MC4xLjAKCg==\n","directory":"/","type":"file","support_file":false,"content_encoding":"utf-8","deleted":false,"operation":"update"},{"name":"go.sum","content":"cnNjLmlvL3FyIHYwLjEuMCBoMTpNL3NBeHNVMko1bWxRNFc4NEJ4Z2EyRWdk\nUXFPYUFsaWlwY2pQbU1VTTVRPQpyc2MuaW8vcXIgdjAuMS4wL2dvLm1vZCBo\nMTpJRit1WmprYjlmcXllRi80dGxCb3lucW1ReFVvUGZXRUtoOTIxY29PdVhz\nPQo=\n","directory":"/","type":"file","support_file":false,"content_encoding":"utf-8","deleted":false,"operation":"update"}],"base_commit_sha":"818a3756444cb0d997a1e11820563105db1c24c4"}

__fixtures__/output/malformed/output.json

@@ -0,0 +1 @@
+C'est ne pas un json

__tests__/container-service.test.ts

@@ -31,7 +31,7 @@ describe('ContainerService', () => {
         Image: 'alpine',
         AttachStdout: true,
         AttachStderr: true,
-        Cmd: ['/bin/sh', '-c']
+        Cmd: ['/bin/sh', '-c', 'nosuchccommand']
       })
     })
 

__tests__/updater.test.ts

@@ -1,15 +1,22 @@
-import {UPDATER_IMAGE_NAME, PROXY_IMAGE_NAME} from '../src/main'
 import {Updater} from '../src/updater'
+import Docker from 'dockerode'
+import {ContainerService} from '../src/container-service'
+import {ProxyBuilder} from '../src/proxy'
+
+// We do not need to build actual containers or run updates for this test.)
+jest.mock('dockerode')
+jest.mock('../src/container-service')
+jest.mock('../src/proxy')
 
 describe('Updater', () => {
   const mockApiClient: any = {
     getJobDetails: jest.fn(),
     getCredentials: jest.fn(),
     params: {
       jobId: 1,
-      jobToken: process.env.JOB_TOKEN,
-      credentialsToken: process.env.CREDENTIALS_TOKEN,
-      dependabotApiUrl: 'http://host.docker.internal:3001'
+      jobToken: 'job-token',
+      credentialsToken: 'job-credentials-token',
+      dependabotApiUrl: 'http://localhost:3001'
     }
   }
 
@@ -23,18 +30,163 @@ describe('Updater', () => {
     'package-manager': 'npm-and-yarn'
   }
 
-  const updater = new Updater(
-    UPDATER_IMAGE_NAME,
-    PROXY_IMAGE_NAME,
-    mockApiClient,
-    mockJobDetails,
-    []
-  )
+  const mockProxy: any = {
+    container: {
+      start: jest.fn()
+    },
+    network: jest.fn(),
+    networkName: 'mockNetworkName',
+    url: 'http://localhost',
+    cert: 'mockCertificate',
+    shutdown: jest.fn()
+  }
+
+  const mockContainer: any = {
+    id: 1
+  }
+
+  afterEach(async () => {
+    jest.clearAllMocks() // Reset any mocked classes
+  })
+
+  describe('when there is a happy path update', () => {
+    const updater = new Updater(
+      'MOCK_UPDATER_IMAGE_NAME',
+      'MOCK_PROXY_IMAGE_NAME',
+      mockApiClient,
+      mockJobDetails,
+      [],
+      '../__fixtures__/output/happy_path/output.json'
+    )
+
+    beforeEach(async () => {
+      jest
+        .spyOn(Docker.prototype, 'createContainer')
+        .mockResolvedValue(mockContainer)
+
+      jest.spyOn(ProxyBuilder.prototype, 'run').mockResolvedValue(mockProxy)
+      jest.spyOn(ContainerService, 'run').mockImplementation(jest.fn())
+    })
+
+    it('should be successful', async () => {
+      expect(await updater.runUpdater()).toBe(true)
+    })
+  })
+
+  describe('when the file fetch container fails', () => {
+    const updater = new Updater(
+      'MOCK_UPDATER_IMAGE_NAME',
+      'MOCK_PROXY_IMAGE_NAME',
+      mockApiClient,
+      mockJobDetails,
+      [],
+      '../__fixtures__/output/happy_path/output.json'
+    )
+
+    beforeEach(async () => {
+      jest
+        .spyOn(Docker.prototype, 'createContainer')
+        .mockResolvedValue(mockContainer)
+
+      jest.spyOn(ProxyBuilder.prototype, 'run').mockResolvedValue(mockProxy)
+
+      jest
+        .spyOn(ContainerService, 'run')
+        .mockImplementationOnce(
+          jest.fn(async () =>
+            Promise.reject(new Error('First call to container service errored'))
+          )
+        )
+    })
+
+    it('should raise an error', async () => {
+      await expect(updater.runUpdater()).rejects.toThrow()
+    })
+  })
+
+  describe('when file updater container fails', () => {
+    const updater = new Updater(
+      'MOCK_UPDATER_IMAGE_NAME',
+      'MOCK_PROXY_IMAGE_NAME',
+      mockApiClient,
+      mockJobDetails,
+      [],
+      '../__fixtures__/output/happy_path/output.json'
+    )
+
+    beforeEach(async () => {
+      jest
+        .spyOn(Docker.prototype, 'createContainer')
+        .mockResolvedValue(mockContainer)
+
+      jest.spyOn(ProxyBuilder.prototype, 'run').mockResolvedValue(mockProxy)
+
+      jest
+        .spyOn(ContainerService, 'run')
+        .mockImplementationOnce(jest.fn())
+        .mockImplementationOnce(
+          jest.fn(async () =>
+            Promise.reject(
+              new Error('Second call to container service errored')
+            )
+          )
+        )
+    })
+
+    it('should raise an error', async () => {
+      await expect(updater.runUpdater()).rejects.toThrow()
+    })
+  })
+
+  describe('when the file fetch step results in empty output', () => {
+    const updater = new Updater(
+      'MOCK_UPDATER_IMAGE_NAME',
+      'MOCK_PROXY_IMAGE_NAME',
+      mockApiClient,
+      mockJobDetails,
+      [],
+      '../__fixtures__/output/empty/output.json'
+    )
+
+    beforeEach(async () => {
+      jest
+        .spyOn(Docker.prototype, 'createContainer')
+        .mockResolvedValue(mockContainer)
+
+      jest.spyOn(ProxyBuilder.prototype, 'run').mockResolvedValue(mockProxy)
+
+      jest.spyOn(ContainerService, 'run').mockImplementation(jest.fn())
+    })
+
+    it('should raise an error', async () => {
+      await expect(updater.runUpdater()).rejects.toThrow(
+        new Error('No output.json created by the fetcher container')
+      )
+    })
+  })
+
+  describe('when the file fetch step results in malformed output', () => {
+    const updater = new Updater(
+      'MOCK_UPDATER_IMAGE_NAME',
+      'MOCK_PROXY_IMAGE_NAME',
+      mockApiClient,
+      mockJobDetails,
+      [],
+      '../__fixtures__/output/malformed/output.json'
+    )
+
+    beforeEach(async () => {
+      jest
+        .spyOn(Docker.prototype, 'createContainer')
+        .mockResolvedValue(mockContainer)
+
+      jest.spyOn(ProxyBuilder.prototype, 'run').mockResolvedValue(mockProxy)
+
+      jest.spyOn(ContainerService, 'run').mockImplementation(jest.fn())
+    })
 
-  it('should fetch job details', async () => {
-    mockApiClient.getJobDetails.mockImplementation(() => {
-      throw new Error('kaboom')
+    it('should raise an error', async () => {
+      await expect(updater.runUpdater()).rejects.toThrow()
     })
-    updater.runUpdater()
   })
 })

src/api-client.ts

@@ -41,7 +41,7 @@ export class ApiClient {
     readonly params: JobParameters
   ) {}
 
-  // We use a static unknown SHA when making a job as complete from the action
+  // We use a static unknown SHA when marking a job as complete from the action
   // to remain in parity with the existing runner.
   UnknownSha = {
     'base-commit-sha': 'unknown'

src/updater.ts

@@ -24,44 +24,32 @@ export class Updater {
     private readonly proxyImage: string,
     private readonly apiClient: ApiClient,
     private readonly details: JobDetails,
-    private readonly credentials: Credential[]
+    private readonly credentials: Credential[],
+    private readonly outputPath = '../output/output.json'
   ) {
     this.docker = new Docker()
   }
 
   /**
    * Execute an update job and report the result to Dependabot API.
    */
-  async runUpdater(): Promise<void> {
+  async runUpdater(): Promise<boolean> {
+    const proxy = await new ProxyBuilder(this.docker, this.proxyImage).run(
+      this.details,
+      this.credentials
+    )
+    proxy.container.start()
+
     try {
-      const proxy = await new ProxyBuilder(this.docker, this.proxyImage).run(
-        this.details,
-        this.credentials
-      )
-      proxy.container.start()
-
-      try {
-        const files = await this.runFileFetcher(proxy)
-        if (!files) {
-          core.error(`failed during fetch, skipping updater`)
-          // TODO: report job runner_error?
-          return
-        }
-
-        await this.runFileUpdater(proxy, files)
-      } catch (e) {
-        // TODO: report job runner_error?
-        core.error(`Error ${e}`)
-      } finally {
-        await this.cleanup(proxy)
-      }
-    } catch (e) {
-      // TODO: report job runner_error?
-      core.error(`Error ${e}`)
+      const files = await this.runFileFetcher(proxy)
+      await this.runFileUpdater(proxy, files)
+      return true
+    } finally {
+      await this.cleanup(proxy)
     }
   }
 
-  private async runFileFetcher(proxy: Proxy): Promise<void | FetchedFiles> {
+  private async runFileFetcher(proxy: Proxy): Promise<FetchedFiles> {
     const container = await this.createContainer(proxy, 'fetch_files')
     await ContainerService.storeInput(
       JOB_INPUT_FILENAME,
@@ -78,9 +66,9 @@ export class Updater {
 
     await ContainerService.run(container)
 
-    const outputPath = path.join(__dirname, '../output/output.json')
+    const outputPath = path.join(__dirname, this.outputPath)
     if (!fs.existsSync(outputPath)) {
-      return
+      throw new Error('No output.json created by the fetcher container')
     }
 
     const fileFetcherSync = fs.readFileSync(outputPath).toString()