diff --git a/__fixtures__/output/empty/.keep b/__fixtures__/output/empty/.keep deleted file mode 100644 index e69de29..0000000 diff --git a/__fixtures__/output/happy_path/output.json b/__fixtures__/output/happy_path/output.json deleted file mode 100644 index 50e114e..0000000 --- a/__fixtures__/output/happy_path/output.json +++ /dev/null @@ -1 +0,0 @@ -{"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"} \ No newline at end of file diff --git a/__fixtures__/output/malformed/output.json b/__fixtures__/output/malformed/output.json deleted file mode 100644 index ea9e681..0000000 --- a/__fixtures__/output/malformed/output.json +++ /dev/null @@ -1 +0,0 @@ -C'est ne pas un json diff --git a/__tests__/container-service.test.ts b/__tests__/container-service.test.ts index 698ad41..64bdc38 100644 --- a/__tests__/container-service.test.ts +++ b/__tests__/container-service.test.ts @@ -7,36 +7,18 @@ describe('ContainerService', () => { const docker = new Docker() let container: any - describe('when a container runs successfully', () => { - beforeEach(async () => { - await ImageService.pull('alpine') - container = await docker.createContainer({ - Image: 'alpine', - AttachStdout: true, - AttachStderr: true, - Cmd: ['/bin/sh', '-c', 'echo $VAR'], - Env: ['VAR=env-var'] - }) - }) - - test('it returns true', async () => { - expect(await ContainerService.run(container)).toBe(true) + beforeAll(async () => { + await ImageService.pull('alpine') + container = await docker.createContainer({ + Image: 'alpine', + AttachStdout: true, + AttachStderr: true, + Cmd: ['/bin/sh', '-c', 'echo $VAR'], + Env: ['VAR=env-var'] }) }) - describe('when a container runs unsuccessfully', () => { - beforeEach(async () => { - await ImageService.pull('alpine') - container = await docker.createContainer({ - Image: 'alpine', - AttachStdout: true, - AttachStderr: true, - Cmd: ['/bin/sh', '-c', 'nosuchccommand'] - }) - }) - - test('raises an exception', async () => { - await expect(ContainerService.run(container)).rejects.toThrow() - }) + test('runs containers', async () => { + await ContainerService.run(container) }) }) diff --git a/__tests__/main.test.ts b/__tests__/main.test.ts index eec35ca..b9ef95e 100644 --- a/__tests__/main.test.ts +++ b/__tests__/main.test.ts @@ -139,9 +139,7 @@ describe('run', () => { expect(reportJobErrorSpy).toHaveBeenCalledWith({ 'error-type': 'actions_workflow_unknown', - 'error-details': { - 'action-error': 'error getting job details' - } + 'error-detail': 'error getting job details' }) expect(markJobAsProcessedSpy).toHaveBeenCalled() }) @@ -175,9 +173,7 @@ describe('run', () => { expect(reportJobErrorSpy).toHaveBeenCalledWith({ 'error-type': 'actions_workflow_unknown', - 'error-details': { - 'action-error': 'error getting credentials' - } + 'error-detail': 'error getting credentials' }) expect(markJobAsProcessedSpy).toHaveBeenCalled() }) @@ -211,9 +207,7 @@ describe('run', () => { expect(reportJobErrorSpy).toHaveBeenCalledWith({ 'error-type': 'actions_workflow_image', - 'error-details': { - 'action-error': 'error pulling an image' - } + 'error-detail': 'error pulling an image' }) expect(markJobAsProcessedSpy).toHaveBeenCalled() }) @@ -247,9 +241,7 @@ describe('run', () => { expect(reportJobErrorSpy).toHaveBeenCalledWith({ 'error-type': 'actions_workflow_updater', - 'error-details': { - 'action-error': 'error running the update' - } + 'error-detail': 'error running the update' }) expect(markJobAsProcessedSpy).toHaveBeenCalled() }) diff --git a/__tests__/updater.test.ts b/__tests__/updater.test.ts index 98fe9a8..591e312 100644 --- a/__tests__/updater.test.ts +++ b/__tests__/updater.test.ts @@ -1,12 +1,5 @@ +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 = { @@ -14,9 +7,9 @@ describe('Updater', () => { getCredentials: jest.fn(), params: { jobId: 1, - jobToken: 'job-token', - credentialsToken: 'job-credentials-token', - dependabotApiUrl: 'http://localhost:3001' + jobToken: process.env.JOB_TOKEN, + credentialsToken: process.env.CREDENTIALS_TOKEN, + dependabotApiUrl: 'http://host.docker.internal:3001' } } @@ -30,163 +23,18 @@ describe('Updater', () => { 'package-manager': 'npm-and-yarn' } - 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' - ) - - 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' - ) - - 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' - ) - - 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' - ) - - 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' - ) - - beforeEach(async () => { - jest - .spyOn(Docker.prototype, 'createContainer') - .mockResolvedValue(mockContainer) - - jest.spyOn(ProxyBuilder.prototype, 'run').mockResolvedValue(mockProxy) - - jest.spyOn(ContainerService, 'run').mockImplementation(jest.fn()) - }) + const updater = new Updater( + UPDATER_IMAGE_NAME, + PROXY_IMAGE_NAME, + mockApiClient, + mockJobDetails, + [] + ) - it('should raise an error', async () => { - await expect(updater.runUpdater()).rejects.toThrow() + it('should fetch job details', async () => { + mockApiClient.getJobDetails.mockImplementation(() => { + throw new Error('kaboom') }) + updater.runUpdater() }) }) diff --git a/src/api-client.ts b/src/api-client.ts index 1f639ee..9f1186d 100644 --- a/src/api-client.ts +++ b/src/api-client.ts @@ -22,9 +22,7 @@ export type JobDetails = { export type JobError = { 'error-type': string - 'error-details': { - 'action-error': string - } + 'error-detail': any } export type Credential = { @@ -41,12 +39,6 @@ export class ApiClient { readonly params: JobParameters ) {} - // 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' - } - async getJobDetails(): Promise { const detailsURL = `/update_jobs/${this.params.jobId}/details` const res = await this.client.get(detailsURL, { @@ -73,29 +65,25 @@ export class ApiClient { async reportJobError(error: JobError): Promise { const recordErrorURL = `/update_jobs/${this.params.jobId}/record_update_job_error` - const res = await this.client.post( - recordErrorURL, - {data: error}, - { - headers: {Authorization: this.params.jobToken} - } - ) - if (res.status !== 204) { + const res = await this.client.post(recordErrorURL, error, { + headers: {Authorization: this.params.jobToken} + }) + if (res.status !== 200) { throw new Error(`Unexpected status code: ${res.status}`) } + + return res.data.data.attributes } async markJobAsProcessed(): Promise { const markAsProcessedURL = `/update_jobs/${this.params.jobId}/mark_as_processed` - const res = await this.client.patch( - markAsProcessedURL, - {data: this.UnknownSha}, - { - headers: {Authorization: this.params.jobToken} - } - ) - if (res.status !== 204) { + const res = await this.client.get(markAsProcessedURL, { + headers: {Authorization: this.params.credentialsToken} + }) + if (res.status !== 200) { throw new Error(`Unexpected status code: ${res.status}`) } + + return res.data.data.attributes } } diff --git a/src/container-service.ts b/src/container-service.ts index b6bf4c1..9984e93 100644 --- a/src/container-service.ts +++ b/src/container-service.ts @@ -4,8 +4,6 @@ import {pack} from 'tar-stream' import {FileFetcherInput, FileUpdaterInput, ProxyConfig} from './config-types' import {outStream, errStream} from './utils' -class ContainerRuntimeError extends Error {} - export const ContainerService = { async storeInput( name: string, @@ -31,7 +29,7 @@ export const ContainerService = { await container.putArchive(tar, {path}) }, - async run(container: Container): Promise { + async run(container: Container): Promise { try { const stream = await container.attach({ stream: true, @@ -45,15 +43,7 @@ export const ContainerService = { ) await container.start() - const outcome = await container.wait() - - if (outcome.StatusCode === 0) { - return true - } else { - throw new ContainerRuntimeError( - `Failure running container ${container.id}` - ) - } + await container.wait() } finally { await container.remove() core.info(`Cleaned up container ${container.id}`) diff --git a/src/main.ts b/src/main.ts index f8d339a..62e6693 100644 --- a/src/main.ts +++ b/src/main.ts @@ -92,9 +92,7 @@ async function failJob( ): Promise { await apiClient.reportJobError({ 'error-type': errorType, - 'error-details': { - 'action-error': error.message - } + 'error-detail': error.message }) await apiClient.markJobAsProcessed() core.setFailed(error.message) diff --git a/src/updater.ts b/src/updater.ts index c1e8553..ec40007 100644 --- a/src/updater.ts +++ b/src/updater.ts @@ -24,8 +24,7 @@ export class Updater { private readonly proxyImage: string, private readonly apiClient: ApiClient, private readonly details: JobDetails, - private readonly credentials: Credential[], - private readonly outputFolder = 'output/' + private readonly credentials: Credential[] ) { this.docker = new Docker() } @@ -33,24 +32,37 @@ export class Updater { /** * Execute an update job and report the result to Dependabot API. */ - async runUpdater(): Promise { - const proxy = await new ProxyBuilder(this.docker, this.proxyImage).run( - this.details, - this.credentials - ) - proxy.container.start() - + async runUpdater(): Promise { try { - const files = await this.runFileFetcher(proxy) - await this.runFileUpdater(proxy, files) - return true - } finally { - await proxy.shutdown() - await this.docker.pruneNetworks() + 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 proxy.shutdown() + await this.docker.pruneNetworks() + } + } catch (e) { + // TODO: report job runner_error? + core.error(`Error ${e}`) } } - private async runFileFetcher(proxy: Proxy): Promise { + private async runFileFetcher(proxy: Proxy): Promise { const container = await this.createContainer(proxy, 'fetch_files') await ContainerService.storeInput( JOB_INPUT_FILENAME, @@ -67,14 +79,9 @@ export class Updater { await ContainerService.run(container) - const outputPath = path.join( - __dirname, - '../', - this.outputFolder, - 'output.json' - ) + const outputPath = path.join(__dirname, '../output/output.json') if (!fs.existsSync(outputPath)) { - throw new Error('No output.json created by the fetcher container') + return } const fileFetcherSync = fs.readFileSync(outputPath).toString()