From ac35f995fea643cbfd583dc9c9ace8da5c2f453a Mon Sep 17 00:00:00 2001 From: Grant Birkinbine Date: Thu, 17 Apr 2025 04:47:03 +0000 Subject: [PATCH 01/12] implement new `artifact-ids` input --- README.md | 32 ++++++++++++++++++++ __tests__/download.test.ts | 2 +- action.yml | 3 ++ dist/index.js | 45 +++++++++++++++++++++++++-- docs/MIGRATION.md | 36 ++++++++++++++++++++++ src/constants.ts | 3 +- src/download-artifact.ts | 62 ++++++++++++++++++++++++++++++++++++-- 7 files changed, 177 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 6ed23d8..aa71e83 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,7 @@ See also [upload-artifact](https://github.com/actions/upload-artifact). - [Outputs](#outputs) - [Examples](#examples) - [Download Single Artifact](#download-single-artifact) + - [Download Artifacts by ID](#download-artifacts-by-id) - [Download All Artifacts](#download-all-artifacts) - [Download multiple (filtered) Artifacts to the same directory](#download-multiple-filtered-artifacts-to-the-same-directory) - [Download Artifacts from other Workflow Runs or Repositories](#download-artifacts-from-other-workflow-runs-or-repositories) @@ -53,6 +54,11 @@ For assistance with breaking changes, see [MIGRATION.md](docs/MIGRATION.md). # Optional. name: + # IDs of the artifacts to download, comma-separated. + # Either inputs `artifact-ids` or `name` can be used, but not both. + # Optional. + artifact-ids: + # Destination path. Supports basic tilde expansion. # Optional. Default is $GITHUB_WORKSPACE path: @@ -117,6 +123,32 @@ steps: run: ls -R your/destination/dir ``` +### Download Artifacts by ID + +The `artifact-ids` input allows downloading artifacts using their unique ID rather than name. This is particularly useful when working with immutable artifacts from `actions/upload-artifact@v4` which assigns a unique ID to each artifact. + +```yaml +steps: +- uses: actions/download-artifact@v4 + with: + artifact-ids: 12345 +- name: Display structure of downloaded files + run: ls -R +``` + +Multiple artifacts can be downloaded by providing a comma-separated list of IDs: + +```yaml +steps: +- uses: actions/download-artifact@v4 + with: + artifact-ids: 12345,67890 + path: path/to/artifacts +- name: Display structure of downloaded files + run: ls -R path/to/artifacts +``` + +This will download multiple artifacts to separate directories (similar to downloading multiple artifacts by name). ### Download All Artifacts diff --git a/__tests__/download.test.ts b/__tests__/download.test.ts index ac79f4e..09b4573 100644 --- a/__tests__/download.test.ts +++ b/__tests__/download.test.ts @@ -112,7 +112,7 @@ describe('download', () => { await run() expect(core.info).toHaveBeenCalledWith( - 'No input name or pattern filtered specified, downloading all artifacts' + 'No input name, artifact-ids or pattern filtered specified, downloading all artifacts' ) expect(core.info).toHaveBeenCalledWith('Total of 2 artifact(s) downloaded') diff --git a/action.yml b/action.yml index 54a3eb6..7fc4fb5 100644 --- a/action.yml +++ b/action.yml @@ -5,6 +5,9 @@ inputs: name: description: 'Name of the artifact to download. If unspecified, all artifacts for the run are downloaded.' required: false + artifact-ids: + description: 'IDs of the artifacts to download, comma-separated. Either inputs `artifact-ids` or `name` can be used, but not both.' + required: false path: description: 'Destination path. Supports basic tilde expansion. Defaults to $GITHUB_WORKSPACE' required: false diff --git a/dist/index.js b/dist/index.js index d0a4ebc..4ce8c85 100644 --- a/dist/index.js +++ b/dist/index.js @@ -118710,6 +118710,7 @@ var Inputs; Inputs["RunID"] = "run-id"; Inputs["Pattern"] = "pattern"; Inputs["MergeMultiple"] = "merge-multiple"; + Inputs["ArtifactIds"] = "artifact-ids"; })(Inputs || (exports.Inputs = Inputs = {})); var Outputs; (function (Outputs) { @@ -118783,7 +118784,10 @@ function run() { repository: core.getInput(constants_1.Inputs.Repository, { required: false }), runID: parseInt(core.getInput(constants_1.Inputs.RunID, { required: false })), pattern: core.getInput(constants_1.Inputs.Pattern, { required: false }), - mergeMultiple: core.getBooleanInput(constants_1.Inputs.MergeMultiple, { required: false }) + mergeMultiple: core.getBooleanInput(constants_1.Inputs.MergeMultiple, { + required: false + }), + artifactIds: core.getInput(constants_1.Inputs.ArtifactIds, { required: false }) }; if (!inputs.path) { inputs.path = process.env['GITHUB_WORKSPACE'] || process.cwd(); @@ -118791,7 +118795,12 @@ function run() { if (inputs.path.startsWith(`~`)) { inputs.path = inputs.path.replace('~', os.homedir()); } + // Check for mutually exclusive inputs + if (inputs.name && inputs.artifactIds) { + throw new Error(`Inputs 'name' and 'artifact-ids' cannot be used together. Please specify only one.`); + } const isSingleArtifactDownload = !!inputs.name; + const isDownloadByIds = !!inputs.artifactIds; const resolvedPath = path.resolve(inputs.path); core.debug(`Resolved path is ${resolvedPath}`); const options = {}; @@ -118808,6 +118817,7 @@ function run() { }; } let artifacts = []; + let artifactIds = []; if (isSingleArtifactDownload) { core.info(`Downloading single artifact`); const { artifact: targetArtifact } = yield artifact_1.default.getArtifact(inputs.name, options); @@ -118817,6 +118827,37 @@ function run() { core.debug(`Found named artifact '${inputs.name}' (ID: ${targetArtifact.id}, Size: ${targetArtifact.size})`); artifacts = [targetArtifact]; } + else if (isDownloadByIds) { + core.info(`Downloading artifacts by ID`); + const artifactIdList = inputs.artifactIds + .split(',') + .map(id => id.trim()) + .filter(id => id !== ''); + if (artifactIdList.length === 0) { + throw new Error(`No valid artifact IDs provided in 'artifact-ids' input`); + } + core.debug(`Parsed artifact IDs: ${JSON.stringify(artifactIdList)}`); + // Parse the artifact IDs + artifactIds = artifactIdList.map(id => { + const numericId = parseInt(id); + if (isNaN(numericId)) { + throw new Error(`Invalid artifact ID: '${id}'. Must be a number.`); + } + return numericId; + }); + // We need to fetch all artifacts to get metadata for the specified IDs + const listArtifactResponse = yield artifact_1.default.listArtifacts(Object.assign({ latest: true }, options)); + artifacts = listArtifactResponse.artifacts.filter(artifact => artifactIds.includes(artifact.id)); + if (artifacts.length === 0) { + throw new Error(`None of the provided artifact IDs were found`); + } + if (artifacts.length < artifactIds.length) { + const foundIds = artifacts.map(a => a.id); + const missingIds = artifactIds.filter(id => !foundIds.includes(id)); + core.warning(`Could not find the following artifact IDs: ${missingIds.join(', ')}`); + } + core.debug(`Found ${artifacts.length} artifacts by ID`); + } else { const listArtifactResponse = yield artifact_1.default.listArtifacts(Object.assign({ latest: true }, options)); artifacts = listArtifactResponse.artifacts; @@ -118828,7 +118869,7 @@ function run() { core.debug(`Filtered from ${listArtifactResponse.artifacts.length} to ${artifacts.length} artifacts`); } else { - core.info('No input name or pattern filtered specified, downloading all artifacts'); + core.info('No input name, artifact-ids or pattern filtered specified, downloading all artifacts'); if (!inputs.mergeMultiple) { core.info('An extra directory with the artifact name will be created for each download'); } diff --git a/docs/MIGRATION.md b/docs/MIGRATION.md index d56279e..fd1781d 100644 --- a/docs/MIGRATION.md +++ b/docs/MIGRATION.md @@ -4,6 +4,7 @@ - [Multiple uploads to the same named Artifact](#multiple-uploads-to-the-same-named-artifact) - [Overwriting an Artifact](#overwriting-an-artifact) - [Merging multiple artifacts](#merging-multiple-artifacts) + - [Working with Immutable Artifacts](#working-with-immutable-artifacts) Several behavioral differences exist between Artifact actions `v3` and below vs `v4`. This document outlines common scenarios in `v3`, and how they would be handled in `v4`. @@ -207,3 +208,38 @@ jobs: ``` Note that this will download all artifacts to a temporary directory and reupload them as a single artifact. For more information on inputs and other use cases for `actions/upload-artifact/merge@v4`, see [the action documentation](https://github.com/actions/upload-artifact/blob/main/merge/README.md). + +## Working with Immutable Artifacts + +In `v4`, artifacts are immutable by default and each artifact gets a unique ID when uploaded. When an artifact with the same name is uploaded again (with or without `overwrite: true`), it gets a new artifact ID. + +To take advantage of this immutability for security purposes (to avoid potential TOCTOU issues where an artifact might be replaced between upload and download), the new `artifact-ids` input allows you to download artifacts by their specific ID rather than by name: + +```yaml +jobs: + upload: + runs-on: ubuntu-latest + steps: + - name: Create a file + run: echo "hello world" > my-file.txt + - name: Upload Artifact + id: upload + uses: actions/upload-artifact@v4 + with: + name: my-artifact + path: my-file.txt + # The upload step outputs the artifact ID + - name: Print Artifact ID + run: echo "Artifact ID is ${{ steps.upload.outputs.artifact-id }}" + download: + needs: upload + runs-on: ubuntu-latest + steps: + - name: Download Artifact by ID + uses: actions/download-artifact@v4 + with: + # Use the artifact ID directly, not the name, to ensure you get exactly the artifact you expect + artifact-ids: ${{ needs.upload.outputs.artifact-id }} +``` + +This approach provides stronger guarantees about which artifact version you're downloading compared to using just the artifact name. diff --git a/src/constants.ts b/src/constants.ts index 17c7d34..b58f4e6 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -5,7 +5,8 @@ export enum Inputs { Repository = 'repository', RunID = 'run-id', Pattern = 'pattern', - MergeMultiple = 'merge-multiple' + MergeMultiple = 'merge-multiple', + ArtifactIds = 'artifact-ids' } export enum Outputs { diff --git a/src/download-artifact.ts b/src/download-artifact.ts index 1beef4d..cbbf099 100644 --- a/src/download-artifact.ts +++ b/src/download-artifact.ts @@ -23,7 +23,10 @@ export async function run(): Promise { repository: core.getInput(Inputs.Repository, {required: false}), runID: parseInt(core.getInput(Inputs.RunID, {required: false})), pattern: core.getInput(Inputs.Pattern, {required: false}), - mergeMultiple: core.getBooleanInput(Inputs.MergeMultiple, {required: false}) + mergeMultiple: core.getBooleanInput(Inputs.MergeMultiple, { + required: false + }), + artifactIds: core.getInput(Inputs.ArtifactIds, {required: false}) } if (!inputs.path) { @@ -34,7 +37,15 @@ export async function run(): Promise { inputs.path = inputs.path.replace('~', os.homedir()) } + // Check for mutually exclusive inputs + if (inputs.name && inputs.artifactIds) { + throw new Error( + `Inputs 'name' and 'artifact-ids' cannot be used together. Please specify only one.` + ) + } + const isSingleArtifactDownload = !!inputs.name + const isDownloadByIds = !!inputs.artifactIds const resolvedPath = path.resolve(inputs.path) core.debug(`Resolved path is ${resolvedPath}`) @@ -56,6 +67,7 @@ export async function run(): Promise { } let artifacts: Artifact[] = [] + let artifactIds: number[] = [] if (isSingleArtifactDownload) { core.info(`Downloading single artifact`) @@ -74,6 +86,52 @@ export async function run(): Promise { ) artifacts = [targetArtifact] + } else if (isDownloadByIds) { + core.info(`Downloading artifacts by ID`) + + const artifactIdList = inputs.artifactIds + .split(',') + .map(id => id.trim()) + .filter(id => id !== '') + + if (artifactIdList.length === 0) { + throw new Error(`No valid artifact IDs provided in 'artifact-ids' input`) + } + + core.debug(`Parsed artifact IDs: ${JSON.stringify(artifactIdList)}`) + + // Parse the artifact IDs + artifactIds = artifactIdList.map(id => { + const numericId = parseInt(id) + if (isNaN(numericId)) { + throw new Error(`Invalid artifact ID: '${id}'. Must be a number.`) + } + return numericId + }) + + // We need to fetch all artifacts to get metadata for the specified IDs + const listArtifactResponse = await artifactClient.listArtifacts({ + latest: true, + ...options + }) + + artifacts = listArtifactResponse.artifacts.filter(artifact => + artifactIds.includes(artifact.id) + ) + + if (artifacts.length === 0) { + throw new Error(`None of the provided artifact IDs were found`) + } + + if (artifacts.length < artifactIds.length) { + const foundIds = artifacts.map(a => a.id) + const missingIds = artifactIds.filter(id => !foundIds.includes(id)) + core.warning( + `Could not find the following artifact IDs: ${missingIds.join(', ')}` + ) + } + + core.debug(`Found ${artifacts.length} artifacts by ID`) } else { const listArtifactResponse = await artifactClient.listArtifacts({ latest: true, @@ -92,7 +150,7 @@ export async function run(): Promise { ) } else { core.info( - 'No input name or pattern filtered specified, downloading all artifacts' + 'No input name, artifact-ids or pattern filtered specified, downloading all artifacts' ) if (!inputs.mergeMultiple) { core.info( From 42aef06f22940dcf3f2d3a655f07d553905547a4 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Thu, 17 Apr 2025 09:55:13 -0700 Subject: [PATCH 02/12] apply https://github.com/actions/download-artifact/pull/401#discussion_r2048225048 suggestion --- src/download-artifact.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/download-artifact.ts b/src/download-artifact.ts index cbbf099..5cc6b17 100644 --- a/src/download-artifact.ts +++ b/src/download-artifact.ts @@ -102,7 +102,7 @@ export async function run(): Promise { // Parse the artifact IDs artifactIds = artifactIdList.map(id => { - const numericId = parseInt(id) + const numericId = parseInt(id, 10) if (isNaN(numericId)) { throw new Error(`Invalid artifact ID: '${id}'. Must be a number.`) } From ec378bcca14b8289ac31be4e58f96edad70bd770 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Thu, 17 Apr 2025 11:48:44 -0700 Subject: [PATCH 03/12] when only one artifact-id is given, use `getArtifact` and check the resulting id returned --- __tests__/download.test.ts | 162 +++++++++++++++++++++++++++++++++++++ src/download-artifact.ts | 45 +++++++++-- 2 files changed, 199 insertions(+), 8 deletions(-) diff --git a/__tests__/download.test.ts b/__tests__/download.test.ts index 09b4573..888c4e7 100644 --- a/__tests__/download.test.ts +++ b/__tests__/download.test.ts @@ -25,6 +25,8 @@ const mockInputs = (overrides?: Partial<{[K in Inputs]?: any}>) => { [Inputs.Repository]: 'owner/some-repository', [Inputs.RunID]: 'some-run-id', [Inputs.Pattern]: 'some-pattern', + [Inputs.MergeMultiple]: false, + [Inputs.ArtifactIds]: '', ...overrides } @@ -221,4 +223,164 @@ describe('download', () => { expect.stringContaining('digest validation failed') ) }) + + test('throws error when both name and artifact-ids are provided', async () => { + mockInputs({ + [Inputs.Name]: 'artifact-name', + [Inputs.ArtifactIds]: '123' + }) + + await expect(run()).rejects.toThrow( + "Inputs 'name' and 'artifact-ids' cannot be used together. Please specify only one." + ) + }) + + test('throws error when artifact-ids is empty', async () => { + mockInputs({ + [Inputs.Name]: '', + [Inputs.ArtifactIds]: ' , ' + }) + + await expect(run()).rejects.toThrow( + "No valid artifact IDs provided in 'artifact-ids' input" + ) + }) + + test('throws error when artifact-id is not a number', async () => { + mockInputs({ + [Inputs.Name]: '', + [Inputs.ArtifactIds]: '123,abc,456' + }) + + await expect(run()).rejects.toThrow( + "Invalid artifact ID: 'abc'. Must be a number." + ) + }) + + test('downloads a single artifact by ID', async () => { + const mockArtifact = { + id: 123, + name: 'artifact-by-id', + size: 1024, + digest: 'def456' + } + + mockInputs({ + [Inputs.Name]: '', + [Inputs.ArtifactIds]: '123' + }) + + jest + .spyOn(artifact, 'getArtifact') + .mockImplementation(() => Promise.resolve({artifact: mockArtifact})) + + await run() + + expect(core.debug).toHaveBeenCalledWith( + 'Only one artifact ID provided. Fetching latest artifact by its name and checking the ID' + ) + expect(artifact.getArtifact).toHaveBeenCalled() + expect(artifact.downloadArtifact).toHaveBeenCalledWith( + mockArtifact.id, + expect.objectContaining({ + expectedHash: mockArtifact.digest + }) + ) + expect(artifact.listArtifacts).not.toHaveBeenCalled() + expect(core.info).toHaveBeenCalledWith('Total of 1 artifact(s) downloaded') + }) + + test('throws error when single artifact ID is not found', async () => { + mockInputs({ + [Inputs.Name]: '', + [Inputs.ArtifactIds]: '999' + }) + + jest.spyOn(artifact, 'getArtifact').mockImplementation(() => { + return Promise.resolve({artifact: null} as any) + }) + + await expect(run()).rejects.toThrow( + "Artifact with ID '999' not found. Please check the ID." + ) + }) + + test('downloads multiple artifacts by IDs', async () => { + const mockArtifacts = [ + {id: 123, name: 'artifact1', size: 1024, digest: 'abc123'}, + {id: 456, name: 'artifact2', size: 2048, digest: 'def456'}, + {id: 789, name: 'artifact3', size: 3072, digest: 'ghi789'} + ] + + mockInputs({ + [Inputs.Name]: '', + [Inputs.ArtifactIds]: '123, 456' + }) + + jest + .spyOn(artifact, 'listArtifacts') + .mockImplementation(() => Promise.resolve({artifacts: mockArtifacts})) + + await run() + + expect(core.info).toHaveBeenCalledWith( + 'Multiple artifact IDs provided. Fetching all artifacts to filter by ID' + ) + expect(artifact.getArtifact).not.toHaveBeenCalled() + expect(artifact.listArtifacts).toHaveBeenCalled() + expect(artifact.downloadArtifact).toHaveBeenCalledTimes(2) + expect(artifact.downloadArtifact).toHaveBeenCalledWith( + 123, + expect.anything() + ) + expect(artifact.downloadArtifact).toHaveBeenCalledWith( + 456, + expect.anything() + ) + }) + + test('warns when some artifact IDs are not found', async () => { + const mockArtifacts = [ + {id: 123, name: 'artifact1', size: 1024, digest: 'abc123'} + ] + + mockInputs({ + [Inputs.Name]: '', + [Inputs.ArtifactIds]: '123, 456, 789' + }) + + jest + .spyOn(artifact, 'listArtifacts') + .mockImplementation(() => Promise.resolve({artifacts: mockArtifacts})) + + await run() + + expect(core.warning).toHaveBeenCalledWith( + 'Could not find the following artifact IDs: 456, 789' + ) + expect(artifact.downloadArtifact).toHaveBeenCalledTimes(1) + expect(artifact.downloadArtifact).toHaveBeenCalledWith( + 123, + expect.anything() + ) + }) + + test('throws error when none of the provided artifact IDs are found', async () => { + const mockArtifacts = [ + {id: 999, name: 'other-artifact', size: 1024, digest: 'xyz999'} + ] + + mockInputs({ + [Inputs.Name]: '', + [Inputs.ArtifactIds]: '123, 456' + }) + + jest + .spyOn(artifact, 'listArtifacts') + .mockImplementation(() => Promise.resolve({artifacts: mockArtifacts})) + + await expect(run()).rejects.toThrow( + 'None of the provided artifact IDs were found' + ) + }) }) diff --git a/src/download-artifact.ts b/src/download-artifact.ts index 5cc6b17..693a17a 100644 --- a/src/download-artifact.ts +++ b/src/download-artifact.ts @@ -109,15 +109,44 @@ export async function run(): Promise { return numericId }) - // We need to fetch all artifacts to get metadata for the specified IDs - const listArtifactResponse = await artifactClient.listArtifacts({ - latest: true, - ...options - }) + // if the length of artifactIds exactly 1 fetch the latest artifact by its name and check the ID + if (artifactIds.length === 1) { + core.debug( + `Only one artifact ID provided. Fetching latest artifact by its name and checking the ID` + ) + const getArtifactResponse = await artifactClient.getArtifact( + inputs.name, + {...options} + ) - artifacts = listArtifactResponse.artifacts.filter(artifact => - artifactIds.includes(artifact.id) - ) + if (!getArtifactResponse || !getArtifactResponse.artifact) { + throw new Error( + `Artifact with ID '${artifactIds[0]}' not found. Please check the ID.` + ) + } + + const artifact = getArtifactResponse.artifact + + core.debug( + `Found artifact by ID '${artifact.name}' (ID: ${artifact.id}, Size: ${artifact.size})` + ) + + artifacts = [artifact] + } else { + core.info( + `Multiple artifact IDs provided. Fetching all artifacts to filter by ID` + ) + + // We need to fetch all artifacts to get metadata for the specified IDs + const listArtifactResponse = await artifactClient.listArtifacts({ + latest: true, + ...options + }) + + artifacts = listArtifactResponse.artifacts.filter(artifact => + artifactIds.includes(artifact.id) + ) + } if (artifacts.length === 0) { throw new Error(`None of the provided artifact IDs were found`) From e463631f66dff8341f3a8188088081c3e4bff512 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Thu, 17 Apr 2025 11:49:32 -0700 Subject: [PATCH 04/12] bundle --- dist/index.js | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/dist/index.js b/dist/index.js index 4ce8c85..3a8adea 100644 --- a/dist/index.js +++ b/dist/index.js @@ -118839,15 +118839,29 @@ function run() { core.debug(`Parsed artifact IDs: ${JSON.stringify(artifactIdList)}`); // Parse the artifact IDs artifactIds = artifactIdList.map(id => { - const numericId = parseInt(id); + const numericId = parseInt(id, 10); if (isNaN(numericId)) { throw new Error(`Invalid artifact ID: '${id}'. Must be a number.`); } return numericId; }); - // We need to fetch all artifacts to get metadata for the specified IDs - const listArtifactResponse = yield artifact_1.default.listArtifacts(Object.assign({ latest: true }, options)); - artifacts = listArtifactResponse.artifacts.filter(artifact => artifactIds.includes(artifact.id)); + // if the length of artifactIds exactly 1 fetch the latest artifact by its name and check the ID + if (artifactIds.length === 1) { + core.debug(`Only one artifact ID provided. Fetching latest artifact by its name and checking the ID`); + const getArtifactResponse = yield artifact_1.default.getArtifact(inputs.name, Object.assign({}, options)); + if (!getArtifactResponse || !getArtifactResponse.artifact) { + throw new Error(`Artifact with ID '${artifactIds[0]}' not found. Please check the ID.`); + } + const artifact = getArtifactResponse.artifact; + core.debug(`Found artifact by ID '${artifact.name}' (ID: ${artifact.id}, Size: ${artifact.size})`); + artifacts = [artifact]; + } + else { + core.info(`Multiple artifact IDs provided. Fetching all artifacts to filter by ID`); + // We need to fetch all artifacts to get metadata for the specified IDs + const listArtifactResponse = yield artifact_1.default.listArtifacts(Object.assign({ latest: true }, options)); + artifacts = listArtifactResponse.artifacts.filter(artifact => artifactIds.includes(artifact.id)); + } if (artifacts.length === 0) { throw new Error(`None of the provided artifact IDs were found`); } @@ -128958,4 +128972,4 @@ module.exports = JSON.parse('[[[0,44],"disallowed_STD3_valid"],[[45,46],"valid"] /******/ module.exports = __webpack_exports__; /******/ /******/ })() -; \ No newline at end of file +; From 171183c7dce98c3cf8a1fc842429d0a38ed21d33 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Thu, 17 Apr 2025 12:18:37 -0700 Subject: [PATCH 05/12] use the same `artifactClient.getArtifact` structure as seen above in `isSingleArtifactDownload` logic --- src/download-artifact.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/download-artifact.ts b/src/download-artifact.ts index 693a17a..5414f10 100644 --- a/src/download-artifact.ts +++ b/src/download-artifact.ts @@ -114,24 +114,22 @@ export async function run(): Promise { core.debug( `Only one artifact ID provided. Fetching latest artifact by its name and checking the ID` ) - const getArtifactResponse = await artifactClient.getArtifact( + const {artifact: targetArtifact} = await artifactClient.getArtifact( inputs.name, - {...options} + options ) - if (!getArtifactResponse || !getArtifactResponse.artifact) { + if (!targetArtifact) { throw new Error( `Artifact with ID '${artifactIds[0]}' not found. Please check the ID.` ) } - const artifact = getArtifactResponse.artifact - core.debug( - `Found artifact by ID '${artifact.name}' (ID: ${artifact.id}, Size: ${artifact.size})` + `Found artifact by ID '${targetArtifact.name}' (ID: ${targetArtifact.id}, Size: ${targetArtifact.size})` ) - artifacts = [artifact] + artifacts = [targetArtifact] } else { core.info( `Multiple artifact IDs provided. Fetching all artifacts to filter by ID` From b83057b90d3e218abf5c7b1906579eb6c598ae85 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Thu, 17 Apr 2025 12:20:46 -0700 Subject: [PATCH 06/12] bundle --- dist/index.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/dist/index.js b/dist/index.js index 3a8adea..61783a4 100644 --- a/dist/index.js +++ b/dist/index.js @@ -118848,13 +118848,12 @@ function run() { // if the length of artifactIds exactly 1 fetch the latest artifact by its name and check the ID if (artifactIds.length === 1) { core.debug(`Only one artifact ID provided. Fetching latest artifact by its name and checking the ID`); - const getArtifactResponse = yield artifact_1.default.getArtifact(inputs.name, Object.assign({}, options)); - if (!getArtifactResponse || !getArtifactResponse.artifact) { + const { artifact: targetArtifact } = yield artifact_1.default.getArtifact(inputs.name, options); + if (!targetArtifact) { throw new Error(`Artifact with ID '${artifactIds[0]}' not found. Please check the ID.`); } - const artifact = getArtifactResponse.artifact; - core.debug(`Found artifact by ID '${artifact.name}' (ID: ${artifact.id}, Size: ${artifact.size})`); - artifacts = [artifact]; + core.debug(`Found artifact by ID '${targetArtifact.name}' (ID: ${targetArtifact.id}, Size: ${targetArtifact.size})`); + artifacts = [targetArtifact]; } else { core.info(`Multiple artifact IDs provided. Fetching all artifacts to filter by ID`); From 54124fbd881f8ce794405a06896c93c49c17463e Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Thu, 17 Apr 2025 12:30:12 -0700 Subject: [PATCH 07/12] revert `getArtifact()` changes - for now we have to list and filter by artifact-ids until a `getArtifactById()` public method exists --- __tests__/download.test.ts | 162 ------------------------------------- dist/index.js | 19 +---- src/download-artifact.ts | 43 ++-------- 3 files changed, 11 insertions(+), 213 deletions(-) diff --git a/__tests__/download.test.ts b/__tests__/download.test.ts index 888c4e7..09b4573 100644 --- a/__tests__/download.test.ts +++ b/__tests__/download.test.ts @@ -25,8 +25,6 @@ const mockInputs = (overrides?: Partial<{[K in Inputs]?: any}>) => { [Inputs.Repository]: 'owner/some-repository', [Inputs.RunID]: 'some-run-id', [Inputs.Pattern]: 'some-pattern', - [Inputs.MergeMultiple]: false, - [Inputs.ArtifactIds]: '', ...overrides } @@ -223,164 +221,4 @@ describe('download', () => { expect.stringContaining('digest validation failed') ) }) - - test('throws error when both name and artifact-ids are provided', async () => { - mockInputs({ - [Inputs.Name]: 'artifact-name', - [Inputs.ArtifactIds]: '123' - }) - - await expect(run()).rejects.toThrow( - "Inputs 'name' and 'artifact-ids' cannot be used together. Please specify only one." - ) - }) - - test('throws error when artifact-ids is empty', async () => { - mockInputs({ - [Inputs.Name]: '', - [Inputs.ArtifactIds]: ' , ' - }) - - await expect(run()).rejects.toThrow( - "No valid artifact IDs provided in 'artifact-ids' input" - ) - }) - - test('throws error when artifact-id is not a number', async () => { - mockInputs({ - [Inputs.Name]: '', - [Inputs.ArtifactIds]: '123,abc,456' - }) - - await expect(run()).rejects.toThrow( - "Invalid artifact ID: 'abc'. Must be a number." - ) - }) - - test('downloads a single artifact by ID', async () => { - const mockArtifact = { - id: 123, - name: 'artifact-by-id', - size: 1024, - digest: 'def456' - } - - mockInputs({ - [Inputs.Name]: '', - [Inputs.ArtifactIds]: '123' - }) - - jest - .spyOn(artifact, 'getArtifact') - .mockImplementation(() => Promise.resolve({artifact: mockArtifact})) - - await run() - - expect(core.debug).toHaveBeenCalledWith( - 'Only one artifact ID provided. Fetching latest artifact by its name and checking the ID' - ) - expect(artifact.getArtifact).toHaveBeenCalled() - expect(artifact.downloadArtifact).toHaveBeenCalledWith( - mockArtifact.id, - expect.objectContaining({ - expectedHash: mockArtifact.digest - }) - ) - expect(artifact.listArtifacts).not.toHaveBeenCalled() - expect(core.info).toHaveBeenCalledWith('Total of 1 artifact(s) downloaded') - }) - - test('throws error when single artifact ID is not found', async () => { - mockInputs({ - [Inputs.Name]: '', - [Inputs.ArtifactIds]: '999' - }) - - jest.spyOn(artifact, 'getArtifact').mockImplementation(() => { - return Promise.resolve({artifact: null} as any) - }) - - await expect(run()).rejects.toThrow( - "Artifact with ID '999' not found. Please check the ID." - ) - }) - - test('downloads multiple artifacts by IDs', async () => { - const mockArtifacts = [ - {id: 123, name: 'artifact1', size: 1024, digest: 'abc123'}, - {id: 456, name: 'artifact2', size: 2048, digest: 'def456'}, - {id: 789, name: 'artifact3', size: 3072, digest: 'ghi789'} - ] - - mockInputs({ - [Inputs.Name]: '', - [Inputs.ArtifactIds]: '123, 456' - }) - - jest - .spyOn(artifact, 'listArtifacts') - .mockImplementation(() => Promise.resolve({artifacts: mockArtifacts})) - - await run() - - expect(core.info).toHaveBeenCalledWith( - 'Multiple artifact IDs provided. Fetching all artifacts to filter by ID' - ) - expect(artifact.getArtifact).not.toHaveBeenCalled() - expect(artifact.listArtifacts).toHaveBeenCalled() - expect(artifact.downloadArtifact).toHaveBeenCalledTimes(2) - expect(artifact.downloadArtifact).toHaveBeenCalledWith( - 123, - expect.anything() - ) - expect(artifact.downloadArtifact).toHaveBeenCalledWith( - 456, - expect.anything() - ) - }) - - test('warns when some artifact IDs are not found', async () => { - const mockArtifacts = [ - {id: 123, name: 'artifact1', size: 1024, digest: 'abc123'} - ] - - mockInputs({ - [Inputs.Name]: '', - [Inputs.ArtifactIds]: '123, 456, 789' - }) - - jest - .spyOn(artifact, 'listArtifacts') - .mockImplementation(() => Promise.resolve({artifacts: mockArtifacts})) - - await run() - - expect(core.warning).toHaveBeenCalledWith( - 'Could not find the following artifact IDs: 456, 789' - ) - expect(artifact.downloadArtifact).toHaveBeenCalledTimes(1) - expect(artifact.downloadArtifact).toHaveBeenCalledWith( - 123, - expect.anything() - ) - }) - - test('throws error when none of the provided artifact IDs are found', async () => { - const mockArtifacts = [ - {id: 999, name: 'other-artifact', size: 1024, digest: 'xyz999'} - ] - - mockInputs({ - [Inputs.Name]: '', - [Inputs.ArtifactIds]: '123, 456' - }) - - jest - .spyOn(artifact, 'listArtifacts') - .mockImplementation(() => Promise.resolve({artifacts: mockArtifacts})) - - await expect(run()).rejects.toThrow( - 'None of the provided artifact IDs were found' - ) - }) }) diff --git a/dist/index.js b/dist/index.js index 61783a4..374211f 100644 --- a/dist/index.js +++ b/dist/index.js @@ -118845,22 +118845,9 @@ function run() { } return numericId; }); - // if the length of artifactIds exactly 1 fetch the latest artifact by its name and check the ID - if (artifactIds.length === 1) { - core.debug(`Only one artifact ID provided. Fetching latest artifact by its name and checking the ID`); - const { artifact: targetArtifact } = yield artifact_1.default.getArtifact(inputs.name, options); - if (!targetArtifact) { - throw new Error(`Artifact with ID '${artifactIds[0]}' not found. Please check the ID.`); - } - core.debug(`Found artifact by ID '${targetArtifact.name}' (ID: ${targetArtifact.id}, Size: ${targetArtifact.size})`); - artifacts = [targetArtifact]; - } - else { - core.info(`Multiple artifact IDs provided. Fetching all artifacts to filter by ID`); - // We need to fetch all artifacts to get metadata for the specified IDs - const listArtifactResponse = yield artifact_1.default.listArtifacts(Object.assign({ latest: true }, options)); - artifacts = listArtifactResponse.artifacts.filter(artifact => artifactIds.includes(artifact.id)); - } + // We need to fetch all artifacts to get metadata for the specified IDs + const listArtifactResponse = yield artifact_1.default.listArtifacts(Object.assign({ latest: true }, options)); + artifacts = listArtifactResponse.artifacts.filter(artifact => artifactIds.includes(artifact.id)); if (artifacts.length === 0) { throw new Error(`None of the provided artifact IDs were found`); } diff --git a/src/download-artifact.ts b/src/download-artifact.ts index 5414f10..5cc6b17 100644 --- a/src/download-artifact.ts +++ b/src/download-artifact.ts @@ -109,42 +109,15 @@ export async function run(): Promise { return numericId }) - // if the length of artifactIds exactly 1 fetch the latest artifact by its name and check the ID - if (artifactIds.length === 1) { - core.debug( - `Only one artifact ID provided. Fetching latest artifact by its name and checking the ID` - ) - const {artifact: targetArtifact} = await artifactClient.getArtifact( - inputs.name, - options - ) + // We need to fetch all artifacts to get metadata for the specified IDs + const listArtifactResponse = await artifactClient.listArtifacts({ + latest: true, + ...options + }) - if (!targetArtifact) { - throw new Error( - `Artifact with ID '${artifactIds[0]}' not found. Please check the ID.` - ) - } - - core.debug( - `Found artifact by ID '${targetArtifact.name}' (ID: ${targetArtifact.id}, Size: ${targetArtifact.size})` - ) - - artifacts = [targetArtifact] - } else { - core.info( - `Multiple artifact IDs provided. Fetching all artifacts to filter by ID` - ) - - // We need to fetch all artifacts to get metadata for the specified IDs - const listArtifactResponse = await artifactClient.listArtifacts({ - latest: true, - ...options - }) - - artifacts = listArtifactResponse.artifacts.filter(artifact => - artifactIds.includes(artifact.id) - ) - } + artifacts = listArtifactResponse.artifacts.filter(artifact => + artifactIds.includes(artifact.id) + ) if (artifacts.length === 0) { throw new Error(`None of the provided artifact IDs were found`) From d219c630f65d8bd14366a9e2f731cbf854f62258 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Thu, 17 Apr 2025 13:14:21 -0700 Subject: [PATCH 08/12] add supporting unit tests for artifact downloads with ids --- __tests__/download.test.ts | 150 +++++++++++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+) diff --git a/__tests__/download.test.ts b/__tests__/download.test.ts index 09b4573..032d857 100644 --- a/__tests__/download.test.ts +++ b/__tests__/download.test.ts @@ -221,4 +221,154 @@ describe('download', () => { expect.stringContaining('digest validation failed') ) }) + + test('downloads a single artifact by ID', async () => { + const mockArtifact = { + id: 456, + name: 'artifact-by-id', + size: 1024, + digest: 'def456' + } + + mockInputs({ + [Inputs.Name]: '', + [Inputs.Pattern]: '', + [Inputs.ArtifactIds]: '456' + }) + + jest.spyOn(artifact, 'listArtifacts').mockImplementation(() => + Promise.resolve({ + artifacts: [mockArtifact] + }) + ) + + await run() + + expect(core.info).toHaveBeenCalledWith('Downloading artifacts by ID') + expect(core.debug).toHaveBeenCalledWith('Parsed artifact IDs: ["456"]') + expect(artifact.downloadArtifact).toHaveBeenCalledTimes(1) + expect(artifact.downloadArtifact).toHaveBeenCalledWith( + 456, + expect.objectContaining({ + expectedHash: mockArtifact.digest + }) + ) + expect(core.info).toHaveBeenCalledWith('Total of 1 artifact(s) downloaded') + }) + + test('downloads multiple artifacts by ID', async () => { + const mockArtifacts = [ + {id: 123, name: 'first-artifact', size: 1024, digest: 'abc123'}, + {id: 456, name: 'second-artifact', size: 2048, digest: 'def456'}, + {id: 789, name: 'third-artifact', size: 3072, digest: 'ghi789'} + ] + + mockInputs({ + [Inputs.Name]: '', + [Inputs.Pattern]: '', + [Inputs.ArtifactIds]: '123, 456, 789' + }) + + jest.spyOn(artifact, 'listArtifacts').mockImplementation(() => + Promise.resolve({ + artifacts: mockArtifacts + }) + ) + + await run() + + expect(core.info).toHaveBeenCalledWith('Downloading artifacts by ID') + expect(core.debug).toHaveBeenCalledWith( + 'Parsed artifact IDs: ["123","456","789"]' + ) + expect(artifact.downloadArtifact).toHaveBeenCalledTimes(3) + mockArtifacts.forEach(mockArtifact => { + expect(artifact.downloadArtifact).toHaveBeenCalledWith( + mockArtifact.id, + expect.objectContaining({ + expectedHash: mockArtifact.digest + }) + ) + }) + expect(core.info).toHaveBeenCalledWith('Total of 3 artifact(s) downloaded') + }) + + test('warns when some artifact IDs are not found', async () => { + const mockArtifacts = [ + {id: 123, name: 'found-artifact', size: 1024, digest: 'abc123'} + ] + + mockInputs({ + [Inputs.Name]: '', + [Inputs.Pattern]: '', + [Inputs.ArtifactIds]: '123, 456, 789' + }) + + jest.spyOn(artifact, 'listArtifacts').mockImplementation(() => + Promise.resolve({ + artifacts: mockArtifacts + }) + ) + + await run() + + expect(core.warning).toHaveBeenCalledWith( + 'Could not find the following artifact IDs: 456, 789' + ) + expect(core.debug).toHaveBeenCalledWith('Found 1 artifacts by ID') + expect(artifact.downloadArtifact).toHaveBeenCalledTimes(1) + }) + + test('throws error when no artifacts with requested IDs are found', async () => { + mockInputs({ + [Inputs.Name]: '', + [Inputs.Pattern]: '', + [Inputs.ArtifactIds]: '123, 456' + }) + + jest.spyOn(artifact, 'listArtifacts').mockImplementation(() => + Promise.resolve({ + artifacts: [] + }) + ) + + await expect(run()).rejects.toThrow( + 'None of the provided artifact IDs were found' + ) + }) + + test('throws error when artifact-ids input is empty', async () => { + mockInputs({ + [Inputs.Name]: '', + [Inputs.Pattern]: '', + [Inputs.ArtifactIds]: ' ' + }) + + await expect(run()).rejects.toThrow( + "No valid artifact IDs provided in 'artifact-ids' input" + ) + }) + + test('throws error when some artifact IDs are not valid numbers', async () => { + mockInputs({ + [Inputs.Name]: '', + [Inputs.Pattern]: '', + [Inputs.ArtifactIds]: '123, abc, 456' + }) + + await expect(run()).rejects.toThrow( + "Invalid artifact ID: 'abc'. Must be a number." + ) + }) + + test('throws error when both name and artifact-ids are provided', async () => { + mockInputs({ + [Inputs.Name]: 'some-artifact', + [Inputs.ArtifactIds]: '123' + }) + + await expect(run()).rejects.toThrow( + "Inputs 'name' and 'artifact-ids' cannot be used together. Please specify only one." + ) + }) }) From 67f2bc382f6ba5ba75812a05909e8c25a366b5fb Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Wed, 23 Apr 2025 10:27:20 -0400 Subject: [PATCH 09/12] Fix workflow example for downloading by artifact ID --- docs/MIGRATION.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/docs/MIGRATION.md b/docs/MIGRATION.md index fd1781d..b5ab8cc 100644 --- a/docs/MIGRATION.md +++ b/docs/MIGRATION.md @@ -219,21 +219,29 @@ To take advantage of this immutability for security purposes (to avoid potential jobs: upload: runs-on: ubuntu-latest + + # Make the artifact ID available to the download job + outputs: + artifact-id: ${{ steps.upload-step.outputs.artifact-id }} + steps: - name: Create a file run: echo "hello world" > my-file.txt - name: Upload Artifact - id: upload + id: upload-step uses: actions/upload-artifact@v4 with: name: my-artifact path: my-file.txt # The upload step outputs the artifact ID - name: Print Artifact ID - run: echo "Artifact ID is ${{ steps.upload.outputs.artifact-id }}" + run: echo "Artifact ID is ${{ steps.upload-step.outputs.artifact-id }}" + download: needs: upload + runs-on: ubuntu-latest + steps: - name: Download Artifact by ID uses: actions/download-artifact@v4 From 84fc7a0a358aabc7f97f7f590cbfc25f57e26c6a Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Wed, 23 Apr 2025 10:32:04 -0400 Subject: [PATCH 10/12] Remove path filters from Check dist workflow --- .github/workflows/check-dist.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/check-dist.yml b/.github/workflows/check-dist.yml index cd90053..452f175 100644 --- a/.github/workflows/check-dist.yml +++ b/.github/workflows/check-dist.yml @@ -10,11 +10,7 @@ on: push: branches: - main - paths-ignore: - - '**.md' pull_request: - paths-ignore: - - '**.md' workflow_dispatch: jobs: From fc02353415da80201a0da48ab47022efd7725d11 Mon Sep 17 00:00:00 2001 From: Rob Herley Date: Thu, 24 Apr 2025 11:21:41 -0400 Subject: [PATCH 11/12] prep for v4.3.0 release --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 4377954..165fa10 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "download-artifact", - "version": "4.2.0", + "version": "4.3.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "download-artifact", - "version": "4.2.0", + "version": "4.3.0", "license": "MIT", "dependencies": { "@actions/artifact": "^2.3.2", diff --git a/package.json b/package.json index 7c22cfe..676b9a7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "download-artifact", - "version": "4.2.0", + "version": "4.3.0", "description": "Download an Actions Artifact from a workflow run", "main": "dist/index.js", "scripts": { From 47225c44b359a5155efdbbbc352041b3e249fb1b Mon Sep 17 00:00:00 2001 From: Ben De St Paer-Gotch Date: Mon, 16 Jun 2025 10:11:52 +0100 Subject: [PATCH 12/12] Update README.md --- README.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/README.md b/README.md index aa71e83..ab6b701 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,24 @@ For more information, see the [`@actions/artifact`](https://github.com/actions/t For assistance with breaking changes, see [MIGRATION.md](docs/MIGRATION.md). +## Note + +Thank you for your interest in this GitHub repo, however, right now we are not taking contributions. + +We continue to focus our resources on strategic areas that help our customers be successful while making developers' lives easier. While GitHub Actions remains a key part of this vision, we are allocating resources towards other areas of Actions and are not taking contributions to this repository at this time. The GitHub public roadmap is the best place to follow along for any updates on features we’re working on and what stage they’re in. + +We are taking the following steps to better direct requests related to GitHub Actions, including: + +1. We will be directing questions and support requests to our [Community Discussions area](https://github.com/orgs/community/discussions/categories/actions) + +2. High Priority bugs can be reported through Community Discussions or you can report these to our support team https://support.github.com/contact/bug-report. + +3. Security Issues should be handled as per our [security.md](SECURITY.md). + +We will still provide security updates for this project and fix major breaking changes during this time. + +You are welcome to still raise bugs in this repo. + ## Usage ### Inputs