diff --git a/packages/artifact/__tests__/download-artifact.test.ts b/packages/artifact/__tests__/download-artifact.test.ts index 9c7d7136e2..c8323df0bc 100644 --- a/packages/artifact/__tests__/download-artifact.test.ts +++ b/packages/artifact/__tests__/download-artifact.test.ts @@ -87,6 +87,15 @@ const expectExtractedArchive = async (dir: string): Promise => { } } +const expectArchive = async (dir: string): Promise => { + const filePath = path.join(dir, `${fixtures.artifactName}.zip`) + expect(fs.existsSync(filePath)).toBe(true) + const stats = fs.statSync(filePath) + expect(stats.isFile()).toBe(true) + expect(stats.isDirectory()).toBe(false) + expect(stats.size).toBeGreaterThan(0) +} + const setup = async (): Promise => { noopLogs() await fs.promises.mkdir(testDir, {recursive: true}) @@ -437,6 +446,51 @@ describe('download-artifact', () => { expect(mockGetArtifactSuccess).toHaveBeenCalledTimes(1) expect(response.downloadPath).toBe(fixtures.workspaceDir) }, 28000) + + it('should be able to keep an artifact zipped', async () => { + const downloadArtifactMock = github.getOctokit(fixtures.token).rest + .actions.downloadArtifact as MockedDownloadArtifact + downloadArtifactMock.mockResolvedValueOnce({ + headers: { + location: fixtures.blobStorageUrl + }, + status: 302, + url: '', + data: Buffer.from('') + }) + + const mockHttpClient = (HttpClient as jest.Mock).mockImplementation( + () => { + return { + get: mockGetArtifactSuccess + } + } + ) + + const response = await downloadArtifactPublic( + fixtures.artifactID, + fixtures.repositoryOwner, + fixtures.repositoryName, + fixtures.token, + { unzip: false, artifactName: fixtures.artifactName } + ) + + expect(downloadArtifactMock).toHaveBeenCalledWith({ + owner: fixtures.repositoryOwner, + repo: fixtures.repositoryName, + artifact_id: fixtures.artifactID, + archive_format: 'zip', + request: { + redirect: 'manual' + } + }) + expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) + expect(mockGetArtifactSuccess).toHaveBeenCalledWith( + fixtures.blobStorageUrl + ) + expectArchive(fixtures.workspaceDir) + expect(response.downloadPath).toBe(fixtures.workspaceDir) + }) }) describe('internal', () => { @@ -610,5 +664,52 @@ describe('download-artifact', () => { name: fixtures.artifactName }) }) + + it('should be able to keep an artifact zipped', async () => { + const mockListArtifacts = jest + .spyOn(ArtifactServiceClientJSON.prototype, 'ListArtifacts') + .mockResolvedValue({ + artifacts: [ + { + ...fixtures.backendIds, + databaseId: fixtures.artifactID.toString(), + name: fixtures.artifactName, + size: fixtures.artifactSize.toString() + } + ] + }) + + const mockGetSignedArtifactURL = jest + .spyOn(ArtifactServiceClientJSON.prototype, 'GetSignedArtifactURL') + .mockReturnValue( + Promise.resolve({ + signedUrl: fixtures.blobStorageUrl + }) + ) + + const mockHttpClient = (HttpClient as jest.Mock).mockImplementation( + () => { + return { + get: mockGetArtifactSuccess + } + } + ) + + const response = await downloadArtifactInternal(fixtures.artifactID, {unzip: false, artifactName: fixtures.artifactName}) + + expectArchive(fixtures.workspaceDir) + expect(response.downloadPath).toBe(fixtures.workspaceDir) + expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) + expect(mockListArtifacts).toHaveBeenCalledWith({ + idFilter: { + value: fixtures.artifactID.toString() + }, + ...fixtures.backendIds + }) + expect(mockGetSignedArtifactURL).toHaveBeenCalledWith({ + ...fixtures.backendIds, + name: fixtures.artifactName + }) + }) }) }) diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index 4a735bb8fa..e7ef1c28e8 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -1,4 +1,6 @@ import fs from 'fs/promises' +import fsStream from 'fs' + import * as crypto from 'crypto' import * as stream from 'stream' @@ -9,6 +11,7 @@ import unzip from 'unzip-stream' import { DownloadArtifactOptions, DownloadArtifactResponse, + StreamExtractOptions, StreamExtractResponse } from '../shared/interfaces' import {getUserAgentString} from '../shared/user-agent' @@ -21,6 +24,7 @@ import { } from '../../generated' import {getBackendIdsFromToken} from '../shared/util' import {ArtifactNotFoundError} from '../shared/errors' +import { basename, join } from 'path' const scrubQueryParameters = (url: string): string => { const parsed = new URL(url) @@ -43,12 +47,13 @@ async function exists(path: string): Promise { async function streamExtract( url: string, - directory: string + directory: string, + options?: StreamExtractOptions ): Promise { let retryCount = 0 while (retryCount < 5) { try { - return await streamExtractExternal(url, directory) + return await streamExtractExternal(url, directory, options) } catch (error) { retryCount++ core.debug( @@ -64,7 +69,8 @@ async function streamExtract( export async function streamExtractExternal( url: string, - directory: string + directory: string, + options?: StreamExtractOptions ): Promise { const client = new httpClient.HttpClient(getUserAgentString()) const response = await client.get(url) @@ -92,6 +98,15 @@ export async function streamExtractExternal( passThrough.pipe(hashStream) const extractStream = passThrough + let outputStream: NodeJS.WritableStream; + + if (options?.unzip ?? true) { + outputStream = unzip.Extract({ path: directory }); + } else { + const fileName = `${options?.artifactName}.zip`; + outputStream = fsStream.createWriteStream(`${directory}/${fileName}`); + } + extractStream .on('data', () => { timer.refresh() @@ -103,7 +118,7 @@ export async function streamExtractExternal( clearTimeout(timer) reject(error) }) - .pipe(unzip.Extract({path: directory})) + .pipe(outputStream) .on('close', () => { clearTimeout(timer) if (hashStream) { @@ -111,7 +126,7 @@ export async function streamExtractExternal( sha256Digest = hashStream.read() as string core.info(`SHA256 digest of downloaded artifact is ${sha256Digest}`) } - resolve({sha256Digest: `sha256:${sha256Digest}`}) + resolve({ sha256Digest: `sha256:${sha256Digest}` }) }) .on('error', (error: Error) => { reject(error) @@ -131,6 +146,8 @@ export async function downloadArtifactPublic( const api = github.getOctokit(token) let digestMismatch = false + + const { expectedHash, path, ...streamExtractOptions} = options || {} core.info( `Downloading artifact '${artifactId}' from '${repositoryOwner}/${repositoryName}'` @@ -161,7 +178,7 @@ export async function downloadArtifactPublic( try { core.info(`Starting download of artifact to: ${downloadPath}`) - const extractResponse = await streamExtract(location, downloadPath) + const extractResponse = await streamExtract(location, downloadPath, streamExtractOptions) core.info(`Artifact download completed successfully.`) if (options?.expectedHash) { if (options?.expectedHash !== extractResponse.sha256Digest) { @@ -185,6 +202,8 @@ export async function downloadArtifactInternal( const artifactClient = internalArtifactTwirpClient() + const { expectedHash, path, ...streamExtractOptions} = options ?? {} + let digestMismatch = false const {workflowRunBackendId, workflowJobRunBackendId} = @@ -222,7 +241,7 @@ export async function downloadArtifactInternal( try { core.info(`Starting download of artifact to: ${downloadPath}`) - const extractResponse = await streamExtract(signedUrl, downloadPath) + const extractResponse = await streamExtract(signedUrl, downloadPath, streamExtractOptions) core.info(`Artifact download completed successfully.`) if (options?.expectedHash) { if (options?.expectedHash !== extractResponse.sha256Digest) { diff --git a/packages/artifact/src/internal/shared/interfaces.ts b/packages/artifact/src/internal/shared/interfaces.ts index 9675d39a3e..1e5da2dc0f 100644 --- a/packages/artifact/src/internal/shared/interfaces.ts +++ b/packages/artifact/src/internal/shared/interfaces.ts @@ -112,7 +112,27 @@ export interface DownloadArtifactOptions { * will provide a digestMismatch property indicating whether the hash of the downloaded artifact * matches the expected hash. */ - expectedHash?: string + expectedHash?: string, + + /** + * Whenever to unzip the artifact after download. Default to true. + */ + unzip?: boolean + /** + * Artifact Name to download. Currently only used when `unzip` is set to false. + */ + artifactName?: string +} + +export interface StreamExtractOptions { + /** + * Whenever to unzip the artifact after download. Default to true. + */ + unzip?: boolean + /** + * Artifact Name to download. Currently only used when `unzip` is set to false. + */ + artifactName?: string } export interface StreamExtractResponse {