From eb06c2179487a20f7c19e16fc559670c4aa664bf Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Fri, 23 Dec 2022 12:44:35 +0100 Subject: [PATCH 1/6] Add cache restore dryRun option --- packages/cache/__tests__/options.test.ts | 11 ++++-- packages/cache/__tests__/restoreCache.test.ts | 36 +++++++++++++++++++ packages/cache/src/cache.ts | 5 +++ packages/cache/src/options.ts | 17 ++++++++- 4 files changed, 65 insertions(+), 4 deletions(-) diff --git a/packages/cache/__tests__/options.test.ts b/packages/cache/__tests__/options.test.ts index 83d9b227..46e540cb 100644 --- a/packages/cache/__tests__/options.test.ts +++ b/packages/cache/__tests__/options.test.ts @@ -9,6 +9,7 @@ const useAzureSdk = true const downloadConcurrency = 8 const timeoutInMs = 30000 const segmentTimeoutInMs = 3600000 +const dryRun = false const uploadConcurrency = 4 const uploadChunkSize = 32 * 1024 * 1024 @@ -19,7 +20,8 @@ test('getDownloadOptions sets defaults', async () => { useAzureSdk, downloadConcurrency, timeoutInMs, - segmentTimeoutInMs + segmentTimeoutInMs, + dryRun }) }) @@ -28,7 +30,8 @@ test('getDownloadOptions overrides all settings', async () => { useAzureSdk: false, downloadConcurrency: 14, timeoutInMs: 20000, - segmentTimeoutInMs: 3600000 + segmentTimeoutInMs: 3600000, + dryRun: true } const actualOptions = getDownloadOptions(expectedOptions) @@ -61,7 +64,8 @@ test('getDownloadOptions overrides download timeout minutes', async () => { useAzureSdk: false, downloadConcurrency: 14, timeoutInMs: 20000, - segmentTimeoutInMs: 3600000 + segmentTimeoutInMs: 3600000, + dryRun: true } process.env.SEGMENT_DOWNLOAD_TIMEOUT_MINS = '10' const actualOptions = getDownloadOptions(expectedOptions) @@ -72,4 +76,5 @@ test('getDownloadOptions overrides download timeout minutes', async () => { ) expect(actualOptions.timeoutInMs).toEqual(expectedOptions.timeoutInMs) expect(actualOptions.segmentTimeoutInMs).toEqual(600000) + expect(actualOptions.dryRun).toEqual(expectedOptions.dryRun) }) diff --git a/packages/cache/__tests__/restoreCache.test.ts b/packages/cache/__tests__/restoreCache.test.ts index 5318a007..c7499dc1 100644 --- a/packages/cache/__tests__/restoreCache.test.ts +++ b/packages/cache/__tests__/restoreCache.test.ts @@ -276,3 +276,39 @@ test('restore with cache found for restore key', async () => { expect(extractTarMock).toHaveBeenCalledWith(archivePath, compression) expect(getCompressionMock).toHaveBeenCalledTimes(1) }) + +test('restore with dry run', async () => { + const paths = ['node_modules'] + const key = 'node-test' + const options = {dryRun: true} + + const cacheEntry: ArtifactCacheEntry = { + cacheKey: key, + scope: 'refs/heads/main', + archiveLocation: 'www.actionscache.test/download' + } + const getCacheMock = jest.spyOn(cacheHttpClient, 'getCacheEntry') + getCacheMock.mockImplementation(async () => { + return Promise.resolve(cacheEntry) + }) + + const createTempDirectoryMock = jest.spyOn(cacheUtils, 'createTempDirectory') + const downloadCacheMock = jest.spyOn(cacheHttpClient, 'downloadCache') + + const compression = CompressionMethod.Gzip + const getCompressionMock = jest + .spyOn(cacheUtils, 'getCompressionMethod') + .mockReturnValue(Promise.resolve(compression)) + + const cacheKey = await restoreCache(paths, key, undefined, options) + + expect(cacheKey).toBe(key) + expect(getCompressionMock).toHaveBeenCalledTimes(1) + expect(getCacheMock).toHaveBeenCalledWith([key], paths, { + compressionMethod: compression, + enableCrossOsArchive: false + }) + // creating a tempDir and downloading the cache are skipped + expect(createTempDirectoryMock).toHaveBeenCalledTimes(0) + expect(downloadCacheMock).toHaveBeenCalledTimes(0) +}) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index ffa13e78..d464bf96 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -100,6 +100,11 @@ export async function restoreCache( return undefined } + if (options?.dryRun) { + core.info('Dry run - skipping download') + return cacheEntry.cacheKey + } + archivePath = path.join( await utils.createTempDirectory(), utils.getCacheFileName(compressionMethod) diff --git a/packages/cache/src/options.ts b/packages/cache/src/options.ts index 652899a2..49101572 100644 --- a/packages/cache/src/options.ts +++ b/packages/cache/src/options.ts @@ -53,6 +53,15 @@ export interface DownloadOptions { * @default 3600000 */ segmentTimeoutInMs?: number + + /** + * Weather to skip downloading the cache entry. + * If dryRun is set to true, the restore function will only check if + * a matching cache entry exists and return the cache key if it does. + * + * @default false + */ + dryRun?: boolean } /** @@ -92,7 +101,8 @@ export function getDownloadOptions(copy?: DownloadOptions): DownloadOptions { useAzureSdk: true, downloadConcurrency: 8, timeoutInMs: 30000, - segmentTimeoutInMs: 3600000 + segmentTimeoutInMs: 3600000, + dryRun: false } if (copy) { @@ -111,6 +121,10 @@ export function getDownloadOptions(copy?: DownloadOptions): DownloadOptions { if (typeof copy.segmentTimeoutInMs === 'number') { result.segmentTimeoutInMs = copy.segmentTimeoutInMs } + + if (typeof copy.dryRun === 'boolean') { + result.dryRun = copy.dryRun + } } const segmentDownloadTimeoutMins = process.env['SEGMENT_DOWNLOAD_TIMEOUT_MINS'] @@ -129,6 +143,7 @@ export function getDownloadOptions(copy?: DownloadOptions): DownloadOptions { `Cache segment download timeout mins env var: ${process.env['SEGMENT_DOWNLOAD_TIMEOUT_MINS']}` ) core.debug(`Segment download timeout (ms): ${result.segmentTimeoutInMs}`) + core.debug(`Dry run: ${result.dryRun}`) return result } From a3849b77aead53f9f89da9a7551f19dde8247f86 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Tue, 17 Jan 2023 17:12:42 +0100 Subject: [PATCH 2/6] Rename option to lookupOnly --- packages/cache/__tests__/options.test.ts | 10 +++++----- packages/cache/__tests__/restoreCache.test.ts | 2 +- packages/cache/src/cache.ts | 4 ++-- packages/cache/src/options.ts | 12 ++++++------ 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/cache/__tests__/options.test.ts b/packages/cache/__tests__/options.test.ts index 46e540cb..79f04241 100644 --- a/packages/cache/__tests__/options.test.ts +++ b/packages/cache/__tests__/options.test.ts @@ -9,7 +9,7 @@ const useAzureSdk = true const downloadConcurrency = 8 const timeoutInMs = 30000 const segmentTimeoutInMs = 3600000 -const dryRun = false +const lookupOnly = false const uploadConcurrency = 4 const uploadChunkSize = 32 * 1024 * 1024 @@ -21,7 +21,7 @@ test('getDownloadOptions sets defaults', async () => { downloadConcurrency, timeoutInMs, segmentTimeoutInMs, - dryRun + lookupOnly }) }) @@ -31,7 +31,7 @@ test('getDownloadOptions overrides all settings', async () => { downloadConcurrency: 14, timeoutInMs: 20000, segmentTimeoutInMs: 3600000, - dryRun: true + lookupOnly: true } const actualOptions = getDownloadOptions(expectedOptions) @@ -65,7 +65,7 @@ test('getDownloadOptions overrides download timeout minutes', async () => { downloadConcurrency: 14, timeoutInMs: 20000, segmentTimeoutInMs: 3600000, - dryRun: true + lookupOnly: true } process.env.SEGMENT_DOWNLOAD_TIMEOUT_MINS = '10' const actualOptions = getDownloadOptions(expectedOptions) @@ -76,5 +76,5 @@ test('getDownloadOptions overrides download timeout minutes', async () => { ) expect(actualOptions.timeoutInMs).toEqual(expectedOptions.timeoutInMs) expect(actualOptions.segmentTimeoutInMs).toEqual(600000) - expect(actualOptions.dryRun).toEqual(expectedOptions.dryRun) + expect(actualOptions.lookupOnly).toEqual(expectedOptions.lookupOnly) }) diff --git a/packages/cache/__tests__/restoreCache.test.ts b/packages/cache/__tests__/restoreCache.test.ts index c7499dc1..7992490e 100644 --- a/packages/cache/__tests__/restoreCache.test.ts +++ b/packages/cache/__tests__/restoreCache.test.ts @@ -280,7 +280,7 @@ test('restore with cache found for restore key', async () => { test('restore with dry run', async () => { const paths = ['node_modules'] const key = 'node-test' - const options = {dryRun: true} + const options = {lookupOnly: true} const cacheEntry: ArtifactCacheEntry = { cacheKey: key, diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index d464bf96..f7fadb6f 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -100,8 +100,8 @@ export async function restoreCache( return undefined } - if (options?.dryRun) { - core.info('Dry run - skipping download') + if (options?.lookupOnly) { + core.info('Lookup only - skipping download') return cacheEntry.cacheKey } diff --git a/packages/cache/src/options.ts b/packages/cache/src/options.ts index 49101572..cdddeeca 100644 --- a/packages/cache/src/options.ts +++ b/packages/cache/src/options.ts @@ -56,12 +56,12 @@ export interface DownloadOptions { /** * Weather to skip downloading the cache entry. - * If dryRun is set to true, the restore function will only check if + * If lookupOnly is set to true, the restore function will only check if * a matching cache entry exists and return the cache key if it does. * * @default false */ - dryRun?: boolean + lookupOnly?: boolean } /** @@ -102,7 +102,7 @@ export function getDownloadOptions(copy?: DownloadOptions): DownloadOptions { downloadConcurrency: 8, timeoutInMs: 30000, segmentTimeoutInMs: 3600000, - dryRun: false + lookupOnly: false } if (copy) { @@ -122,8 +122,8 @@ export function getDownloadOptions(copy?: DownloadOptions): DownloadOptions { result.segmentTimeoutInMs = copy.segmentTimeoutInMs } - if (typeof copy.dryRun === 'boolean') { - result.dryRun = copy.dryRun + if (typeof copy.lookupOnly === 'boolean') { + result.lookupOnly = copy.lookupOnly } } const segmentDownloadTimeoutMins = @@ -143,7 +143,7 @@ export function getDownloadOptions(copy?: DownloadOptions): DownloadOptions { `Cache segment download timeout mins env var: ${process.env['SEGMENT_DOWNLOAD_TIMEOUT_MINS']}` ) core.debug(`Segment download timeout (ms): ${result.segmentTimeoutInMs}`) - core.debug(`Dry run: ${result.dryRun}`) + core.debug(`Lookup only: ${result.lookupOnly}`) return result } From e45a26f77154a0a6b444ddf1424f34503a958a1c Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Tue, 21 Feb 2023 13:56:25 +0100 Subject: [PATCH 3/6] Update package version and changelog --- packages/cache/RELEASES.md | 5 ++++- packages/cache/package-lock.json | 4 ++-- packages/cache/package.json | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/cache/RELEASES.md b/packages/cache/RELEASES.md index 76bec600..424b7688 100644 --- a/packages/cache/RELEASES.md +++ b/packages/cache/RELEASES.md @@ -117,4 +117,7 @@ - Fix to prevent from setting MYSYS environement variable globally [#1329](https://github.com/actions/toolkit/pull/1329). ### 3.1.4 -- Fix zstd not being used due to `zstd --version` output change in zstd 1.5.4 release. See [#1353](https://github.com/actions/toolkit/pull/1353). \ No newline at end of file +- Fix zstd not being used due to `zstd --version` output change in zstd 1.5.4 release. See [#1353](https://github.com/actions/toolkit/pull/1353). + +### 3.2.0 +- Add `lookupOnly` to cache restore `DownloadOptions`. diff --git a/packages/cache/package-lock.json b/packages/cache/package-lock.json index 408660cb..0d568ff5 100644 --- a/packages/cache/package-lock.json +++ b/packages/cache/package-lock.json @@ -1,12 +1,12 @@ { "name": "@actions/cache", - "version": "3.1.4", + "version": "3.2.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@actions/cache", - "version": "3.1.4", + "version": "3.2.0", "license": "MIT", "dependencies": { "@actions/core": "^1.10.0", diff --git a/packages/cache/package.json b/packages/cache/package.json index 3d5dda6b..b411de1c 100644 --- a/packages/cache/package.json +++ b/packages/cache/package.json @@ -1,6 +1,6 @@ { "name": "@actions/cache", - "version": "3.1.4", + "version": "3.2.0", "preview": true, "description": "Actions cache lib", "keywords": [ From d47e0bac601c7f6e8af58074b75edac3d060d66f Mon Sep 17 00:00:00 2001 From: Ferenc Hammerl <31069338+fhammerl@users.noreply.github.com> Date: Mon, 6 Mar 2023 11:02:29 +0100 Subject: [PATCH 4/6] Support '*' wildcard (#1355) --- packages/http-client/__tests__/proxy.test.ts | 9 +++++++-- packages/http-client/src/proxy.ts | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/http-client/__tests__/proxy.test.ts b/packages/http-client/__tests__/proxy.test.ts index ccbceec2..60077a58 100644 --- a/packages/http-client/__tests__/proxy.test.ts +++ b/packages/http-client/__tests__/proxy.test.ts @@ -176,11 +176,16 @@ describe('proxy', () => { expect(bypass).toBeTruthy() }) - // Do not match wildcard ("*") as per https://github.com/actions/runner/blob/97195bad5870e2ad0915ebfef1616083aacf5818/docs/adrs/0263-proxy-support.md it('checkBypass returns true if no_proxy is "*"', () => { process.env['no_proxy'] = '*' const bypass = pm.checkBypass(new URL('https://anything.whatsoever.com')) - expect(bypass).toBeFalsy() + expect(bypass).toBeTruthy() + }) + + it('checkBypass returns true if no_proxy contains comma separated "*"', () => { + process.env['no_proxy'] = 'domain.com,* , example.com' + const bypass = pm.checkBypass(new URL('https://anything.whatsoever.com')) + expect(bypass).toBeTruthy() }) it('HttpClient does basic http get request through proxy', async () => { diff --git a/packages/http-client/src/proxy.ts b/packages/http-client/src/proxy.ts index e4e43a54..3c84c586 100644 --- a/packages/http-client/src/proxy.ts +++ b/packages/http-client/src/proxy.ts @@ -52,6 +52,7 @@ export function checkBypass(reqUrl: URL): boolean { .map(x => x.trim().toUpperCase()) .filter(x => x)) { if ( + upperNoProxyItem === '*' || upperReqHosts.some( x => x === upperNoProxyItem || From 94ab8de5f3856fbf78dcc8637bbd8c526c628e5a Mon Sep 17 00:00:00 2001 From: Ferenc Hammerl <31069338+fhammerl@users.noreply.github.com> Date: Mon, 6 Mar 2023 11:07:04 +0100 Subject: [PATCH 5/6] Bypass proxy on loopback IPs (localhost, 127.*, ::1 etc) (#1361) * Bypass proxy on loopback IPs * Expect empty array instead of undefined * Restore accidentally deleted test * Fix formatting * Fix linting * Update proxy.ts * Better ipv6 definitions * Fix linting * Update proxy.test.ts --- packages/http-client/__tests__/proxy.test.ts | 25 ++++++++++++++++++++ packages/http-client/src/proxy.ts | 15 ++++++++++++ 2 files changed, 40 insertions(+) diff --git a/packages/http-client/__tests__/proxy.test.ts b/packages/http-client/__tests__/proxy.test.ts index 60077a58..98d85c86 100644 --- a/packages/http-client/__tests__/proxy.test.ts +++ b/packages/http-client/__tests__/proxy.test.ts @@ -242,6 +242,31 @@ describe('proxy', () => { expect(_proxyConnects).toHaveLength(0) }) + it('HttpClient bypasses proxy for loopback addresses (localhost, ::1, 127.*)', async () => { + // setup a server listening on localhost:8091 + const server = http.createServer((request, response) => { + response.writeHead(200) + request.pipe(response) + }) + server.listen(8091) + try { + process.env['http_proxy'] = _proxyUrl + const httpClient = new httpm.HttpClient() + let res = await httpClient.get('http://localhost:8091') + expect(res.message.statusCode).toBe(200) + res = await httpClient.get('http://127.0.0.1:8091') + expect(res.message.statusCode).toBe(200) + + // no support for ipv6 for now + expect(httpClient.get('http://[::1]:8091')).rejects.toThrow() + + // proxy at _proxyUrl was ignored + expect(_proxyConnects).toEqual([]) + } finally { + server.close() + } + }) + it('proxyAuth not set in tunnel agent when authentication is not provided', async () => { process.env['https_proxy'] = 'http://127.0.0.1:8080' const httpClient = new httpm.HttpClient() diff --git a/packages/http-client/src/proxy.ts b/packages/http-client/src/proxy.ts index 3c84c586..1a967e34 100644 --- a/packages/http-client/src/proxy.ts +++ b/packages/http-client/src/proxy.ts @@ -25,6 +25,11 @@ export function checkBypass(reqUrl: URL): boolean { return false } + const reqHost = reqUrl.hostname + if (isLoopbackAddress(reqHost)) { + return true + } + const noProxy = process.env['no_proxy'] || process.env['NO_PROXY'] || '' if (!noProxy) { return false @@ -67,3 +72,13 @@ export function checkBypass(reqUrl: URL): boolean { return false } + +function isLoopbackAddress(host: string): boolean { + const hostLower = host.toLowerCase() + return ( + hostLower === 'localhost' || + hostLower.startsWith('127.') || + hostLower.startsWith('[::1]') || + hostLower.startsWith('[0:0:0:0:0:0:0:1]') + ) +} From 787b2cf270deaf96f0f7d166cc7fa6c9d3fb8ded Mon Sep 17 00:00:00 2001 From: Ferenc Hammerl <31069338+fhammerl@users.noreply.github.com> Date: Mon, 6 Mar 2023 17:03:09 +0100 Subject: [PATCH 6/6] Bump http-client to version 2.1.0 (#1364) * Update package.json * Update package.json * Update RELEASES.md --- packages/http-client/RELEASES.md | 4 ++++ packages/http-client/package.json | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/http-client/RELEASES.md b/packages/http-client/RELEASES.md index 344555f9..d3195080 100644 --- a/packages/http-client/RELEASES.md +++ b/packages/http-client/RELEASES.md @@ -1,5 +1,9 @@ ## Releases +## 2.1.0 +- Add support for `*` and subdomains in `no_proxy` [#1355](https://github.com/actions/toolkit/pull/1355) [#1223](https://github.com/actions/toolkit/pull/1223) +- Bypass proxy for loopback IP adresses [#1360](https://github.com/actions/toolkit/pull/1360)) + ## 2.0.1 - Fix an issue with missing `tunnel` dependency [#1085](https://github.com/actions/toolkit/pull/1085) diff --git a/packages/http-client/package.json b/packages/http-client/package.json index c1de2213..7f5c8ec3 100644 --- a/packages/http-client/package.json +++ b/packages/http-client/package.json @@ -1,6 +1,6 @@ { "name": "@actions/http-client", - "version": "2.0.1", + "version": "2.1.0", "description": "Actions Http Client", "keywords": [ "github",