Refactor proxy to a plain object without optionals

__tests__/updater-integration.test.ts

@@ -40,7 +40,6 @@ describe('Updater', () => {
 
   const client = axios.create({baseURL: externalDependabotApiUrl})
   const apiClient = new APIClient(client, params)
-  const updater = new Updater(UPDATER_IMAGE_NAME, PROXY_IMAGE_NAME, apiClient)
 
   beforeAll(async () => {
     // Skip the test when we haven't preloaded the updater image
@@ -68,6 +67,17 @@ describe('Updater', () => {
       return
     }
 
+    const details = await apiClient.getJobDetails()
+    const credentials = await apiClient.getCredentials()
+
+    const updater = new Updater(
+      UPDATER_IMAGE_NAME,
+      PROXY_IMAGE_NAME,
+      apiClient,
+      details,
+      credentials
+    )
+
     await updater.runUpdater()
 
     // NOTE: This will not work when running against the actual dependabot-api

__tests__/updater.test.ts

@@ -1,5 +1,6 @@
 import {UPDATER_IMAGE_NAME, PROXY_IMAGE_NAME} from '../src/main'
 import {Updater} from '../src/updater'
+import {PackageManager} from '../src/api-client'
 
 describe('Updater', () => {
   const mockAPIClient: any = {
@@ -12,10 +13,22 @@ describe('Updater', () => {
       dependabotAPIURL: 'http://host.docker.internal:3001'
     }
   }
+  const mockJobDetails: any = {
+    id: '1',
+    'allowed-updates': [
+      {
+        'dependency-type': 'all'
+      }
+    ],
+    'package-manage': PackageManager.NpmAndYarn
+  }
+
   const updater = new Updater(
     UPDATER_IMAGE_NAME,
     PROXY_IMAGE_NAME,
-    mockAPIClient
+    mockAPIClient,
+    mockJobDetails,
+    []
   )
 
   it('should fetch job details', async () => {

src/main.ts

@@ -26,7 +26,15 @@ async function run(): Promise<void> {
 
     const client = axios.create({baseURL: params.dependabotAPIURL})
     const apiClient = new APIClient(client, params)
-    const updater = new Updater(UPDATER_IMAGE_NAME, PROXY_IMAGE_NAME, apiClient)
+    const details = await apiClient.getJobDetails()
+    const credentials = await apiClient.getCredentials()
+    const updater = new Updater(
+      UPDATER_IMAGE_NAME,
+      PROXY_IMAGE_NAME,
+      apiClient,
+      details,
+      credentials
+    )
     await ImageService.pull(UPDATER_IMAGE_NAME)
     await ImageService.pull(PROXY_IMAGE_NAME)
 

src/proxy.ts

@@ -41,51 +41,53 @@ const CERT_SUBJECT = [
   }
 ]
 
-export class Proxy {
-  container?: Container
+export type Proxy = {
+  container: Container
+  network: Network
   networkName: string
   url: string
   cert: string
-  network?: Network
+}
 
+export class ProxyBuilder {
   constructor(
     private readonly docker: Docker,
     private readonly proxyImage: string
-  ) {
-    // TODO: this is obviously gnarly, decouple some things so we don't need to
-    // initialize these as empty strings
-    this.networkName = ''
-    this.url = ''
-    this.cert = ''
-  }
+  ) {}
 
-  async run(details: JobDetails, credentials: Credential[]): Promise<void> {
+  async run(details: JobDetails, credentials: Credential[]): Promise<Proxy> {
     const name = `job-${details.id}-proxy`
     const config = this.buildProxyConfig(credentials, details.id)
-    this.cert = config.ca.cert
+    const cert = config.ca.cert
 
-    this.networkName = `job-${details.id}-network`
-    this.network = await this.ensureNetwork(this.networkName)
+    const networkName = `job-${details.id}-network`
+    const network = await this.ensureNetwork(networkName)
 
-    this.container = await this.createContainer(details.id, name)
+    const container = await this.createContainer(details.id, name, networkName)
 
     await ContainerService.storeInput(
       CONFIG_FILE_NAME,
       CONFIG_FILE_PATH,
-      this.container,
+      container,
       config
     )
 
-    const stream = await this.container.attach({
+    const stream = await container.attach({
       stream: true,
       stdout: true,
       stderr: true
     })
-    this.container.modem.demuxStream(stream, process.stdout, process.stderr)
-
-    this.container.start()
-    this.url = `http://${config.proxy_auth.username}:${config.proxy_auth.password}@${name}:1080`
-    core.info(this.url)
+    container.modem.demuxStream(stream, process.stdout, process.stderr)
+
+    container.start()
+    const url = `http://${config.proxy_auth.username}:${config.proxy_auth.password}@${name}:1080`
+    return {
+      container,
+      network,
+      networkName,
+      url,
+      cert
+    }
   }
 
   private async ensureNetwork(name: string): Promise<Network> {
@@ -139,7 +141,8 @@ export class Proxy {
 
   private async createContainer(
     jobID: string,
-    containerName: string
+    containerName: string,
+    networkName: string
   ): Promise<Container> {
     const container = await this.docker.createContainer({
       Image: this.proxyImage,
@@ -148,7 +151,7 @@ export class Proxy {
       AttachStderr: true,
       Env: [`JOB_ID=${jobID}`],
       HostConfig: {
-        NetworkMode: this.networkName
+        NetworkMode: networkName
       }
     })
 

src/updater.ts

@@ -2,11 +2,11 @@ import * as core from '@actions/core'
 import Docker, {Container} from 'dockerode'
 import path from 'path'
 import fs from 'fs'
-import {JobDetails, APIClient} from './api-client'
+import {JobDetails, APIClient, Credential} from './api-client'
 import {ContainerService} from './container-service'
 import {base64DecodeDependencyFile} from './utils'
 import {DependencyFile, FetchedFiles, FileUpdaterInput} from './file-types'
-import {Proxy} from './proxy'
+import {ProxyBuilder, Proxy} from './proxy'
 
 const JOB_INPUT_FILENAME = 'job.json'
 const JOB_INPUT_PATH = `/home/dependabot/dependabot-updater`
@@ -18,60 +18,63 @@ const CA_CERT_FILENAME = 'dbot-ca.crt'
 
 export class Updater {
   docker: Docker
-  proxy: Proxy
 
   constructor(
     private readonly updaterImage: string,
     private readonly proxyImage: string,
-    private readonly apiClient: APIClient
+    private readonly apiClient: APIClient,
+    private readonly details: JobDetails,
+    private readonly credentials: Credential[]
   ) {
     this.docker = new Docker()
-    this.proxy = new Proxy(this.docker, proxyImage)
   }
 
   /**
    * Execute an update job and report the result to Dependabot API.
    */
   async runUpdater(): Promise<void> {
     try {
-      const details = await this.apiClient.getJobDetails()
-      const credentials = await this.apiClient.getCredentials()
+      const proxy = await new ProxyBuilder(this.docker, this.proxyImage).run(
+        this.details,
+        this.credentials
+      )
 
-      await this.proxy.run(details, credentials)
+      try {
+        const files = await this.runFileFetcher(proxy)
+        if (!files) {
+          core.error(`failed during fetch, skipping updater`)
+          // TODO: report job runner_error?
+          return
+        }
 
-      const files = await this.runFileFetcher(details)
-      if (!files) {
-        core.error(`failed during fetch, skipping updater`)
+        await this.runFileUpdater(proxy, files)
+      } catch (e) {
         // TODO: report job runner_error?
-        return
+        core.error(`Error ${e}`)
+      } finally {
+        await proxy.container.stop()
+        await proxy.container.remove()
+        await proxy.network.remove()
       }
-
-      await this.runFileUpdater(details, files)
     } catch (e) {
       // TODO: report job runner_error?
       core.error(`Error ${e}`)
-    } finally {
-      await this.proxy.container?.stop()
-      await this.proxy.container?.remove()
-      await this.proxy.network?.remove()
     }
   }
 
-  private async runFileFetcher(
-    details: JobDetails
-  ): Promise<void | FetchedFiles> {
-    const container = await this.createContainer('fetch_files')
+  private async runFileFetcher(proxy: Proxy): Promise<void | FetchedFiles> {
+    const container = await this.createContainer(proxy, 'fetch_files')
     await ContainerService.storeInput(
       JOB_INPUT_FILENAME,
       JOB_INPUT_PATH,
       container,
-      {job: details}
+      {job: this.details}
     )
     await ContainerService.storeCert(
       CA_CERT_FILENAME,
       CA_CERT_INPUT_PATH,
       container,
-      this.proxy.cert
+      proxy.cert
     )
 
     await ContainerService.run(container)
@@ -96,16 +99,16 @@ export class Updater {
   }
 
   private async runFileUpdater(
-    details: JobDetails,
+    proxy: Proxy,
     files: FetchedFiles
   ): Promise<void> {
     core.info(`Running update job ${this.apiClient.params.jobID}`)
-    const container = await this.createContainer('update_files')
+    const container = await this.createContainer(proxy, 'update_files')
     const containerInput: FileUpdaterInput = {
       base_commit_sha: files.base_commit_sha,
       base64_dependency_files: files.base64_dependency_files,
       dependency_files: files.dependency_files,
-      job: details
+      job: this.details
     }
     await ContainerService.storeInput(
       JOB_INPUT_FILENAME,
@@ -117,13 +120,16 @@ export class Updater {
       CA_CERT_FILENAME,
       CA_CERT_INPUT_PATH,
       container,
-      this.proxy.cert
+      proxy.cert
     )
 
     await ContainerService.run(container)
   }
 
-  private async createContainer(updaterCommand: string): Promise<Container> {
+  private async createContainer(
+    proxy: Proxy,
+    updaterCommand: string
+  ): Promise<Container> {
     const container = await this.docker.createContainer({
       Image: this.updaterImage,
       AttachStdout: true,
@@ -136,18 +142,18 @@ export class Updater {
         `DEPENDABOT_REPO_CONTENTS_PATH=${REPO_CONTENTS_PATH}`,
         `DEPENDABOT_API_URL=${this.apiClient.params.dependabotAPIURL}`,
         `SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt`,
-        `http_proxy=${this.proxy.url}`,
-        `HTTP_PROXY=${this.proxy.url}`,
-        `https_proxy=${this.proxy.url}`,
-        `HTTPS_PROXY=${this.proxy.url}`
+        `http_proxy=${proxy.url}`,
+        `HTTP_PROXY=${proxy.url}`,
+        `https_proxy=${proxy.url}`,
+        `HTTPS_PROXY=${proxy.url}`
       ],
       Cmd: [
         'sh',
         '-c',
         `/usr/sbin/update-ca-certificates && $DEPENDABOT_HOME/dependabot-updater/bin/run ${updaterCommand}`
       ],
       HostConfig: {
-        NetworkMode: this.proxy.networkName,
+        NetworkMode: proxy.networkName,
         Binds: [
           `${path.join(__dirname, '../output')}:${JOB_OUTPUT_PATH}:rw`,
           `${path.join(__dirname, '../repo')}:${REPO_CONTENTS_PATH}:rw`