Skip to content

Commit

Permalink
Add an option to specify retention days for artifacts (#575)
Browse files Browse the repository at this point in the history
* Add an option to specify retention days for artifacts

* Validate against settings exposed as env var to give early feedback

* Fix lint

* Add tests and addressing feedback

* Update packages/artifact/__tests__/upload.test.ts

Co-authored-by: Konrad Pabjan <konradpabjan@github.com>

* Update packages/artifact/README.md

Co-authored-by: Konrad Pabjan <konradpabjan@github.com>

* Update packages/artifact/src/internal/utils.ts

Co-authored-by: Konrad Pabjan <konradpabjan@github.com>

* Update packages/artifact/__tests__/util.test.ts

Co-authored-by: Konrad Pabjan <konradpabjan@github.com>

Co-authored-by: Konrad Pabjan <konradpabjan@github.com>
  • Loading branch information
yacaovsnc and konradpabjan authored Sep 18, 2020
1 parent 71b19c1 commit ace7a82
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 5 deletions.
5 changes: 5 additions & 0 deletions packages/artifact/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ Method Name: `uploadArtifact`
- If set to `false`, and an error is encountered, all other uploads will stop and any files that were queued will not be attempted to be uploaded. The partial artifact available will only include files up until the failure.
- If set to `true` and an error is encountered, the failed file will be skipped and ignored and all other queued files will be attempted to be uploaded. There will be an artifact available for download at the end with everything excluding the file that failed to upload
- Optional, defaults to `true` if not specified
- `retentionDays`
- Duration after which artifact will expire in days
- Minimum value: 1
- Maximum value: 90 unless changed by repository setting
- If this is set to a greater value than the retention settings allowed, the retention on artifacts will be reduced to match the max value allowed on the server, and the upload process will continue. An input of 0 assumes default retention value.

#### Example using Absolute File Paths

Expand Down
12 changes: 12 additions & 0 deletions packages/artifact/__tests__/upload.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
} from '../src/internal/contracts'
import {UploadSpecification} from '../src/internal/upload-specification'
import {getArtifactUrl} from '../src/internal/utils'
import {UploadOptions} from '../src/internal/upload-options'

const root = path.join(__dirname, '_temp', 'artifact-upload')
const file1Path = path.join(root, 'file1.txt')
Expand Down Expand Up @@ -106,6 +107,17 @@ describe('Upload Tests', () => {
)
})

it('Create Artifact - Retention Less Than Min Value Error', async () => {
const artifactName = 'valid-artifact-name'
const options: UploadOptions = {
retentionDays: -1
}
const uploadHttpClient = new UploadHttpClient()
expect(
uploadHttpClient.createArtifactInFileContainer(artifactName, options)
).rejects.toEqual(new Error('Invalid retention, minimum value is 1.'))
})

it('Create Artifact - Storage Quota Error', async () => {
const artifactName = 'storage-quota-hit'
const uploadHttpClient = new UploadHttpClient()
Expand Down
14 changes: 14 additions & 0 deletions packages/artifact/__tests__/util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,20 @@ describe('Utils', () => {
}
})

it('Test negative artifact retention throws', () => {
expect(() => {
utils.getProperRetention(-1, undefined)
}).toThrow()
})

it('Test no setting specified takes artifact retention input', () => {
expect(utils.getProperRetention(180, undefined)).toEqual(180)
})

it('Test artifact retention must conform to max allowed', () => {
expect(utils.getProperRetention(180, '45')).toEqual(45)
})

it('Test constructing artifact URL', () => {
const runtimeUrl = getRuntimeUrl()
const runId = getWorkFlowRunId()
Expand Down
4 changes: 4 additions & 0 deletions packages/artifact/src/internal/__mocks__/config-variables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,7 @@ export function getRuntimeUrl(): string {
export function getWorkFlowRunId(): string {
return '15'
}

export function getRetentionDays(): string | undefined {
return '45'
}
3 changes: 2 additions & 1 deletion packages/artifact/src/internal/artifact-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ export class DefaultArtifactClient implements ArtifactClient {
} else {
// Create an entry for the artifact in the file container
const response = await uploadHttpClient.createArtifactInFileContainer(
name
name,
options
)
if (!response.fileContainerResourceUrl) {
core.debug(response.toString())
Expand Down
4 changes: 4 additions & 0 deletions packages/artifact/src/internal/config-variables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,7 @@ export function getWorkSpaceDirectory(): string {
}
return workspaceDirectory
}

export function getRetentionDays(): string | undefined {
return process.env['GITHUB_RETENTION_DAYS']
}
1 change: 1 addition & 0 deletions packages/artifact/src/internal/contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export interface ArtifactResponse {
export interface CreateArtifactParameters {
Type: string
Name: string
RetentionDays?: number
}

export interface PatchArtifactSize {
Expand Down
19 changes: 16 additions & 3 deletions packages/artifact/src/internal/upload-http-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ import {
isForbiddenStatusCode,
displayHttpDiagnostics,
getExponentialRetryTimeInMilliseconds,
tryGetRetryAfterValueTimeInMilliseconds
tryGetRetryAfterValueTimeInMilliseconds,
getProperRetention
} from './utils'
import {
getUploadChunkSize,
getUploadFileConcurrency,
getRetryLimit
getRetryLimit,
getRetentionDays
} from './config-variables'
import {promisify} from 'util'
import {URL} from 'url'
Expand Down Expand Up @@ -55,12 +57,23 @@ export class UploadHttpClient {
* @returns The response from the Artifact Service if the file container was successfully created
*/
async createArtifactInFileContainer(
artifactName: string
artifactName: string,
options?: UploadOptions | undefined
): Promise<ArtifactResponse> {
const parameters: CreateArtifactParameters = {
Type: 'actions_storage',
Name: artifactName
}

// calculate retention period
if (options && options.retentionDays) {
const maxRetentionStr = getRetentionDays()
parameters.RetentionDays = getProperRetention(
options.retentionDays,
maxRetentionStr
)
}

const data: string = JSON.stringify(parameters, null, 2)
const artifactUrl = getArtifactUrl()

Expand Down
17 changes: 17 additions & 0 deletions packages/artifact/src/internal/upload-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,21 @@ export interface UploadOptions {
*
*/
continueOnError?: boolean

/**
* Duration after which artifact will expire in days.
*
* By default artifact expires after 90 days:
* https://docs.github.com/en/actions/configuring-and-managing-workflows/persisting-workflow-data-using-artifacts#downloading-and-deleting-artifacts-after-a-workflow-run-is-complete
*
* Use this option to override the default expiry.
*
* Min value: 1
* Max value: 90 unless changed by repository setting
*
* If this is set to a greater value than the retention settings allowed, the retention on artifacts
* will be reduced to match the max value allowed on server, and the upload process will continue. An
* input of 0 assumes default retention setting.
*/
retentionDays?: number
}
23 changes: 22 additions & 1 deletion packages/artifact/src/internal/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {debug, info} from '@actions/core'
import {debug, info, warning} from '@actions/core'
import {promises as fs} from 'fs'
import {HttpCodes, HttpClient} from '@actions/http-client'
import {BearerCredentialHandler} from '@actions/http-client/auth'
Expand Down Expand Up @@ -302,3 +302,24 @@ export async function createEmptyFilesForArtifact(
await (await fs.open(filePath, 'w')).close()
}
}

export function getProperRetention(
retentionInput: number,
retentionSetting: string | undefined
): number {
if (retentionInput < 0) {
throw new Error('Invalid retention, minimum value is 1.')
}

let retention = retentionInput
if (retentionSetting) {
const maxRetention = parseInt(retentionSetting)
if (!isNaN(maxRetention) && maxRetention < retention) {
warning(
`Retention days is greater than the max value allowed by the repository setting, reduce retention to ${maxRetention} days`
)
retention = maxRetention
}
}
return retention
}

0 comments on commit ace7a82

Please sign in to comment.