From 92c287430881363c045753973e6ff91a82289272 Mon Sep 17 00:00:00 2001 From: Jurre Stender Date: Wed, 11 Aug 2021 22:01:00 +0200 Subject: [PATCH] Refactor proxy to a plain object without optionals --- __tests__/updater-integration.test.ts | 12 ++++- __tests__/updater.test.ts | 15 +++++- src/main.ts | 10 +++- src/proxy.ts | 51 +++++++++--------- src/updater.ts | 74 +++++++++++++++------------ 5 files changed, 101 insertions(+), 61 deletions(-) diff --git a/__tests__/updater-integration.test.ts b/__tests__/updater-integration.test.ts index fbd1b6a..9af1742 100644 --- a/__tests__/updater-integration.test.ts +++ b/__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 diff --git a/__tests__/updater.test.ts b/__tests__/updater.test.ts index addde06..09e2ad6 100644 --- a/__tests__/updater.test.ts +++ b/__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 () => { diff --git a/src/main.ts b/src/main.ts index af92ca8..0053479 100644 --- a/src/main.ts +++ b/src/main.ts @@ -26,7 +26,15 @@ async function run(): Promise { 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) diff --git a/src/proxy.ts b/src/proxy.ts index 4e4b567..d43392f 100644 --- a/src/proxy.ts +++ b/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 { + async run(details: JobDetails, credentials: Credential[]): Promise { 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 { @@ -139,7 +141,8 @@ export class Proxy { private async createContainer( jobID: string, - containerName: string + containerName: string, + networkName: string ): Promise { 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 } }) diff --git a/src/updater.ts b/src/updater.ts index d18e04f..1fe91b0 100644 --- a/src/updater.ts +++ b/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,15 +18,15 @@ 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) } /** @@ -34,44 +34,47 @@ export class Updater { */ async runUpdater(): Promise { 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 { - const container = await this.createContainer('fetch_files') + private async runFileFetcher(proxy: Proxy): Promise { + 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 { 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 { + private async createContainer( + proxy: Proxy, + updaterCommand: string + ): Promise { const container = await this.docker.createContainer({ Image: this.updaterImage, AttachStdout: true, @@ -136,10 +142,10 @@ 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', @@ -147,7 +153,7 @@ export class Updater { `/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`