Skip to content

feat: add support for unzip argument in downloadArtifact #2095

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 101 additions & 0 deletions packages/artifact/__tests__/download-artifact.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,15 @@ const expectExtractedArchive = async (dir: string): Promise<void> => {
}
}

const expectArchive = async (dir: string): Promise<void> => {
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<void> => {
noopLogs()
await fs.promises.mkdir(testDir, {recursive: true})
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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
})
})
})
})
33 changes: 26 additions & 7 deletions packages/artifact/src/internal/download/download-artifact.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import fs from 'fs/promises'
import fsStream from 'fs'

import * as crypto from 'crypto'
import * as stream from 'stream'

Expand All @@ -9,6 +11,7 @@ import unzip from 'unzip-stream'
import {
DownloadArtifactOptions,
DownloadArtifactResponse,
StreamExtractOptions,
StreamExtractResponse
} from '../shared/interfaces'
import {getUserAgentString} from '../shared/user-agent'
Expand All @@ -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)
Expand All @@ -43,12 +47,13 @@ async function exists(path: string): Promise<boolean> {

async function streamExtract(
url: string,
directory: string
directory: string,
options?: StreamExtractOptions
): Promise<StreamExtractResponse> {
let retryCount = 0
while (retryCount < 5) {
try {
return await streamExtractExternal(url, directory)
return await streamExtractExternal(url, directory, options)
} catch (error) {
retryCount++
core.debug(
Expand All @@ -64,7 +69,8 @@ async function streamExtract(

export async function streamExtractExternal(
url: string,
directory: string
directory: string,
options?: StreamExtractOptions
): Promise<StreamExtractResponse> {
const client = new httpClient.HttpClient(getUserAgentString())
const response = await client.get(url)
Expand Down Expand Up @@ -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()
Expand All @@ -103,15 +118,15 @@ export async function streamExtractExternal(
clearTimeout(timer)
reject(error)
})
.pipe(unzip.Extract({path: directory}))
.pipe(outputStream)
.on('close', () => {
clearTimeout(timer)
if (hashStream) {
hashStream.end()
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)
Expand All @@ -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}'`
Expand Down Expand Up @@ -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) {
Expand All @@ -185,6 +202,8 @@ export async function downloadArtifactInternal(

const artifactClient = internalArtifactTwirpClient()

const { expectedHash, path, ...streamExtractOptions} = options ?? {}

let digestMismatch = false

const {workflowRunBackendId, workflowJobRunBackendId} =
Expand Down Expand Up @@ -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) {
Expand Down
22 changes: 21 additions & 1 deletion packages/artifact/src/internal/shared/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down