mirror of
https://github.com/renovatebot/renovate.git
synced 2026-10-25 06:51:11 +00:00
1afa293dda
* fix(docker): adjust tag caching for private registries Tag caching for private registries currently has two issues, observed in issues #45249 and #40734: 1. `getTags` has no `cacheable` guard (unlike `getReleases`) 2. `getTags` caches `null` tag results These two combined mean that regardless of `cachePrivatePackages` settings, private registry tag lookups which fail due to transient/auth errors are cached. This results in the cache being "poisoned" by misconfigured jobs, with well-configured jobs unable to recover. This is especially relevant for the hosted app where caching is shared by different tenants. This commit: 1. Aligns `getTags` caching with `getReleases`, ensuring caching behaviour is driven by `cachePrivatePackages`. 2. Prevents `null` tag results being cached, allowing for recovery regardless of `cachePrivatePackages` configuration. Note that I have _not_ applied (2) to `getReleases`, because 1. `getReleases` has a slightly different contract in that it sets `fallback: true` to intentionally cache failures 2. I have not observed any actual failures in `getReleases` due to (20, so it hasn't been empirically proven as requiring a fix 3. `getTags` is internal so is a lower-risk change Fixes - #45249 Relates to - #45249 - #40734 * refactor(docker): undefined for no-auth Previously `getAuthHeaders()` used `null` as an auth-failed sentinel, which was inconsistent with `withCache()`, and led to the original "fix" where nulls were explicitly omitted from the cache. Instead this implements the suggestion from https://github.com/renovatebot/renovate/pull/45348#issuecomment-5340524967 to uniformly return undefined for `getTags` auth errors, which allows the cache to recognise failures itself. Then cacheability ends up being conditional on dockerhub as always. This also rectifies a mismatch between auth-failure signals, where other functions like `_getImageConfig` were returning `undefined` when headers were unavailable. IMPORTANT: once this lands, since there aren't any cache invalidations, the existing issue will still be observed until the cache TTL expires and old `null` entries are evicted. * fix: whoops, sort out the vacuous tests I failed to realise caching isn't automatically orchestrated in these tests. As an eagle-eyed observer [pointed out][1], this meant the tests weren't actually exercising anything. This commit fixes that by adding a new test block which does the cache orchestration using a temp dir cache. I've also opted to add assertions against the mock server's http trace counter directly to ensure the mock really is/isn't being hit. This should also serve to make the tests fail fast if the underlying impl flow (auth probe -> get tags) is adjusted in the future. I've also brought in some caching dependencies into the test file, but those are used elsewhere too (e.g. `withCache` tests). This time, I have actually verified the tests which we'd expect to fail do fail against main. Note that the first 'caches tags for Docker Hub' is expected to pass on both branches (the control test). ``` ❯ getTags caching (3) ✓ caches tags for Docker Hub 21ms 249 MB heap used × does not cache tags for a private registry 20ms 212 MB heap used × does not cache failed lookups when cachePrivatePackages is enabled 15ms 215 MB heap used ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Tests 2 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ FAIL lib/modules/datasource/docker/index.spec.ts > modules/datasource/docker/index > getTags caching > does not cache tags for a private registry AssertionError: expected [ '1.0.0' ] to deeply equal [ '1.0.0', '2.0.0' ] - Expected + Received [ "1.0.0", - "2.0.0", ] ❯ lib/modules/datasource/docker/index.spec.ts:3694:72 3692| expect(httpMock.getTrace()).toHaveLength(2); 3693| 3694| expect(await ds.getTags('https://registry.company.com', 'node')).toEqual([ | ^ 3695| '1.0.0', 3696| '2.0.0', ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/4]⎯ FAIL lib/modules/datasource/docker/index.spec.ts > modules/datasource/docker/index > getTags caching > does not cache tags for a private registry Error: *** Unused HTTP mocks *** - GET https://registry.company.com/v2/node/tags/list Requests done: - GET https://registry.company.com/v2/node/tags/list?n=10000 [200] - GET https://registry.company.com/v2/node/tags/list?n=10000 [200] --- Renovate testing strategy requires that every HTTP request has a corresponding mock. This error occurs because some of the created mocks are unused. In most cases, you simply need to remove them. +++ ❯ <test> lib/modules/datasource/docker/index.spec.ts:3675:9 3673| }); 3674| 3675| it('does not cache tags for a private registry', async () => { | ^ 3676| httpMock 3677| .scope('https://registry.company.com/v2') ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[2/4]⎯ FAIL lib/modules/datasource/docker/index.spec.ts > modules/datasource/docker/index > getTags caching > does not cache failed lookups when cachePrivatePackages is enabled AssertionError: expected null to be undefined - Expected: undefined + Received: null ❯ lib/modules/datasource/docker/index.spec.ts:3717:9 3715| expect( 3716| await ds.getTags('https://registry.company.com', 'node'), 3717| ).toBeUndefined(); | ^ 3718| expect(httpMock.getTrace()).toHaveLength(1); 3719| ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[3/4]⎯ FAIL lib/modules/datasource/docker/index.spec.ts > modules/datasource/docker/index > getTags caching > does not cache failed lookups when cachePrivatePackages is enabled Error: *** Unused HTTP mocks *** - GET https://registry.company.com/v2/node/tags/list Requests done: - GET https://registry.company.com/v2/node/tags/list?n=10000 [403] --- Renovate testing strategy requires that every HTTP request has a corresponding mock. This error occurs because some of the created mocks are unused. In most cases, you simply need to remove them. +++ ❯ <test> lib/modules/datasource/docker/index.spec.ts:3701:9 3699| }); 3700| 3701| it('does not cache failed lookups when cachePrivatePackages is enabled', async () => { | ^ 3702| GlobalConfig.set({ cachePrivatePackages: true }); 3703| ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[4/4]⎯ Test Files 1 failed (1) Tests 2 failed | 313 passed (315) ``` [1]: https://github.com/renovatebot/renovate/pull/45348#pullrequestreview-5092478059 * oxlint-fix --------- Co-authored-by: Jamie Tanna <jamie.tanna@mend.io>
373 lines
11 KiB
TypeScript
373 lines
11 KiB
TypeScript
import { hostRules } from '~test/host-rules.ts';
|
|
import * as httpMock from '~test/http-mock.ts';
|
|
import { partial } from '~test/util.ts';
|
|
import { PAGE_NOT_FOUND_ERROR } from '../../../constants/error-messages.ts';
|
|
import { Http } from '../../../util/http/index.ts';
|
|
import {
|
|
dockerDatasourceId,
|
|
findHelmSourceUrl,
|
|
findLatestStable,
|
|
getAuthHeaders,
|
|
getRegistryRepository,
|
|
} from './common.ts';
|
|
import type { OciHelmConfig } from './schema.ts';
|
|
|
|
const http = new Http(dockerDatasourceId);
|
|
|
|
describe('modules/datasource/docker/common', () => {
|
|
describe('getRegistryRepository', () => {
|
|
it('handles local registries', () => {
|
|
const res = getRegistryRepository(
|
|
'registry:5000/org/package',
|
|
'https://index.docker.io',
|
|
);
|
|
expect(res).toStrictEqual({
|
|
dockerRepository: 'org/package',
|
|
registryHost: 'https://registry:5000',
|
|
});
|
|
});
|
|
|
|
it('supports registryUrls', () => {
|
|
const res = getRegistryRepository(
|
|
'my.local.registry/prefix/image',
|
|
'https://my.local.registry/prefix',
|
|
);
|
|
expect(res).toStrictEqual({
|
|
dockerRepository: 'prefix/image',
|
|
registryHost: 'https://my.local.registry',
|
|
});
|
|
});
|
|
|
|
it('supports http registryUrls', () => {
|
|
const res = getRegistryRepository(
|
|
'my.local.registry/prefix/image',
|
|
'http://my.local.registry/prefix',
|
|
);
|
|
expect(res).toStrictEqual({
|
|
dockerRepository: 'prefix/image',
|
|
registryHost: 'http://my.local.registry',
|
|
});
|
|
});
|
|
|
|
it('supports schemeless registryUrls', () => {
|
|
const res = getRegistryRepository(
|
|
'my.local.registry/prefix/image',
|
|
'my.local.registry/prefix',
|
|
);
|
|
expect(res).toStrictEqual({
|
|
dockerRepository: 'prefix/image',
|
|
registryHost: 'https://my.local.registry',
|
|
});
|
|
});
|
|
|
|
it('supports insecure registryUrls', () => {
|
|
hostRules.add({ insecureRegistry: true });
|
|
const res = getRegistryRepository(
|
|
'prefix/image',
|
|
'my.local.registry/prefix',
|
|
);
|
|
expect(res).toStrictEqual({
|
|
dockerRepository: 'prefix/prefix/image',
|
|
registryHost: 'http://my.local.registry',
|
|
});
|
|
});
|
|
|
|
it.each([
|
|
{
|
|
name: 'strimzi-kafka-operator',
|
|
url: 'https://quay.io/strimzi-helm/',
|
|
res: {
|
|
dockerRepository: 'strimzi-helm/strimzi-kafka-operator',
|
|
registryHost: 'https://quay.io',
|
|
},
|
|
},
|
|
{
|
|
name: 'strimzi-kafka-operator',
|
|
url: 'https://docker.io/strimzi-helm/',
|
|
res: {
|
|
dockerRepository: 'strimzi-helm/strimzi-kafka-operator',
|
|
registryHost: 'https://index.docker.io',
|
|
},
|
|
},
|
|
{
|
|
name: 'nginx',
|
|
url: 'https://docker.io',
|
|
res: {
|
|
dockerRepository: 'library/nginx',
|
|
registryHost: 'https://index.docker.io',
|
|
},
|
|
},
|
|
{
|
|
name: 'registry-1.docker.io/bitnamicharts/cert-manager',
|
|
url: 'https://index.docker.io',
|
|
res: {
|
|
dockerRepository: 'bitnamicharts/cert-manager',
|
|
registryHost: 'https://index.docker.io',
|
|
},
|
|
},
|
|
])('($name, $url)', ({ name, url, res }) => {
|
|
expect(getRegistryRepository(name, url)).toStrictEqual(res);
|
|
});
|
|
|
|
it('returns raw registryHost and dockerRepository when fullUrl is invalid', () => {
|
|
// 'https://[/prefix' is a syntactically invalid URL (unclosed bracket)
|
|
const res = getRegistryRepository('[/prefix/image', 'https://[/prefix');
|
|
expect(res).toStrictEqual({
|
|
registryHost: 'https://[/prefix',
|
|
dockerRepository: 'image',
|
|
});
|
|
});
|
|
});
|
|
|
|
describe('getAuthHeaders', () => {
|
|
beforeEach(() => {
|
|
hostRules.add({
|
|
username: 'some-username',
|
|
password: 'some-password',
|
|
});
|
|
});
|
|
|
|
it('throw page not found exception', async () => {
|
|
httpMock
|
|
.scope('https://my.local.registry')
|
|
.get('/v2/repo/tags/list?n=1000')
|
|
.reply(404, {});
|
|
|
|
await expect(
|
|
getAuthHeaders(
|
|
http,
|
|
'https://my.local.registry',
|
|
'repo',
|
|
'https://my.local.registry/v2/repo/tags/list?n=1000',
|
|
),
|
|
).rejects.toThrow(PAGE_NOT_FOUND_ERROR);
|
|
});
|
|
|
|
it('falls back to base /v2/ auth probe when tags URL returns 405 ECR maxResults error', async () => {
|
|
httpMock
|
|
.scope('https://my.local.registry')
|
|
.get('/v2/repo/tags/list?n=10000')
|
|
.reply(
|
|
405,
|
|
{
|
|
errors: [
|
|
{
|
|
code: 'UNSUPPORTED',
|
|
message:
|
|
"Invalid parameter at 'maxResults' failed to satisfy constraint: 'Member must have value less than or equal to 1000'",
|
|
},
|
|
],
|
|
},
|
|
{ 'docker-distribution-api-version': 'registry/2.0' },
|
|
)
|
|
.get('/v2/')
|
|
.reply(200, '');
|
|
|
|
const headers = await getAuthHeaders(
|
|
http,
|
|
'https://my.local.registry',
|
|
'repo',
|
|
'https://my.local.registry/v2/repo/tags/list?n=10000',
|
|
);
|
|
|
|
expect(headers).toEqual({});
|
|
});
|
|
|
|
it('falls back to base /v2/ auth probe and negotiates token when 405 ECR maxResults error is received', async () => {
|
|
httpMock
|
|
.scope('https://my.local.registry')
|
|
.get('/v2/repo/tags/list?n=10000')
|
|
.reply(
|
|
405,
|
|
{
|
|
errors: [
|
|
{
|
|
code: 'UNSUPPORTED',
|
|
message:
|
|
"Invalid parameter at 'maxResults' failed to satisfy constraint: 'Member must have value less than or equal to 1000'",
|
|
},
|
|
],
|
|
},
|
|
{ 'docker-distribution-api-version': 'registry/2.0' },
|
|
)
|
|
.get('/v2/', undefined, { badheaders: ['authorization'] })
|
|
.reply(401, '', {
|
|
'www-authenticate':
|
|
'Bearer realm="https://my.local.registry/oauth2/token",service="my.local.registry"',
|
|
})
|
|
.get(
|
|
'/oauth2/token?service=my.local.registry&scope=repository:repo:pull',
|
|
)
|
|
.reply(200, { token: 'abc' });
|
|
|
|
const headers = await getAuthHeaders(
|
|
http,
|
|
'https://my.local.registry',
|
|
'repo',
|
|
'https://my.local.registry/v2/repo/tags/list?n=10000',
|
|
);
|
|
|
|
expect(headers).toEqual({ authorization: 'Bearer abc' });
|
|
});
|
|
|
|
it('returns "authType token" if both provided', async () => {
|
|
httpMock
|
|
.scope('https://my.local.registry')
|
|
.get('/v2/', undefined, { badheaders: ['authorization'] })
|
|
.reply(401, '', { 'www-authenticate': 'Authenticate you must' });
|
|
hostRules.clear();
|
|
hostRules.add({
|
|
authType: 'some-authType',
|
|
token: 'some-token',
|
|
});
|
|
|
|
const headers = await getAuthHeaders(
|
|
http,
|
|
'https://my.local.registry',
|
|
'https://my.local.registry/prefix',
|
|
);
|
|
|
|
// do not inline, otherwise we get false positive from codeql
|
|
expect(headers).toMatchInlineSnapshot(`
|
|
{
|
|
"authorization": "some-authType some-token",
|
|
}
|
|
`);
|
|
});
|
|
|
|
it('returns "Bearer token" if only token provided', async () => {
|
|
httpMock
|
|
.scope('https://my.local.registry')
|
|
.get('/v2/', undefined, { badheaders: ['authorization'] })
|
|
.reply(401, '', { 'www-authenticate': 'Authenticate you must' });
|
|
hostRules.clear();
|
|
hostRules.add({
|
|
token: 'some-token',
|
|
});
|
|
|
|
const headers = await getAuthHeaders(
|
|
http,
|
|
'https://my.local.registry',
|
|
'https://my.local.registry/prefix',
|
|
);
|
|
|
|
// do not inline, otherwise we get false positive from codeql
|
|
expect(headers).toMatchInlineSnapshot(`
|
|
{
|
|
"authorization": "Bearer some-token",
|
|
}
|
|
`);
|
|
});
|
|
|
|
it('fails', async () => {
|
|
httpMock
|
|
.scope('https://my.local.registry')
|
|
.get('/v2/', undefined, { badheaders: ['authorization'] })
|
|
.reply(401, '', { 'www-authenticate': 'Authenticate you must' });
|
|
httpMock.clear(false);
|
|
|
|
httpMock
|
|
.scope('https://my.local.registry')
|
|
.get('/v2/', undefined, { badheaders: ['authorization'] })
|
|
.reply(401, '', {});
|
|
|
|
const headers = await getAuthHeaders(
|
|
http,
|
|
'https://my.local.registry',
|
|
'https://my.local.registry/prefix',
|
|
);
|
|
|
|
expect(headers).toBeUndefined();
|
|
});
|
|
|
|
it('use resources URL and resolve scope in www-authenticate header', async () => {
|
|
httpMock
|
|
.scope('https://my.local.registry')
|
|
.get('/v2/my/node/resource')
|
|
.reply(401, '', {
|
|
'www-authenticate':
|
|
'Bearer realm="https://my.local.registry/oauth2/token",service="my.local.registry",scope="repository:my/node:whatever"',
|
|
})
|
|
.get(
|
|
'/oauth2/token?service=my.local.registry&scope=repository:my/node:whatever',
|
|
)
|
|
.reply(200, { token: 'some-token' });
|
|
|
|
const headers = await getAuthHeaders(
|
|
http,
|
|
'https://my.local.registry',
|
|
'my/node/prefix',
|
|
'https://my.local.registry/v2/my/node/resource',
|
|
);
|
|
|
|
// do not inline, otherwise we get false positive from codeql
|
|
expect(headers).toMatchInlineSnapshot(`
|
|
{
|
|
"authorization": "Bearer some-token",
|
|
}
|
|
`);
|
|
});
|
|
|
|
it('supports multiple challenges in www-authenticate header', async () => {
|
|
httpMock
|
|
.scope('https://codeberg.org')
|
|
.get('/v2/')
|
|
.reply(401, '', {
|
|
'www-authenticate':
|
|
'Bearer realm="https://codeberg.org/v2/token",service="container_registry",scope="*",Basic realm="https://codeberg.org/v2",service="container_registry",scope="*"',
|
|
})
|
|
.get(
|
|
'/v2/token?service=container_registry&scope=repository:my/node/prefix:pull',
|
|
)
|
|
.reply(200, { token: 'abc' });
|
|
|
|
const headers = await getAuthHeaders(
|
|
http,
|
|
'https://codeberg.org',
|
|
'my/node/prefix',
|
|
);
|
|
|
|
// do not inline, otherwise we get false positive from codeql
|
|
expect(headers).toMatchInlineSnapshot(`
|
|
{
|
|
"authorization": "Bearer abc",
|
|
}
|
|
`);
|
|
});
|
|
});
|
|
|
|
it('findLatestStable works', () => {
|
|
expect(findLatestStable([])).toBeNull();
|
|
});
|
|
|
|
it('findHelmSourceUrl works', () => {
|
|
expect(
|
|
findHelmSourceUrl(
|
|
partial<OciHelmConfig>({
|
|
home: 'https://github.com/bitnami/charts/tree/main/bitnami/harbor',
|
|
}),
|
|
),
|
|
).toBe('https://github.com/bitnami/charts/tree/main/bitnami/harbor');
|
|
|
|
expect(findHelmSourceUrl(partial<OciHelmConfig>({}))).toBeNull();
|
|
|
|
expect(
|
|
findHelmSourceUrl(
|
|
partial<OciHelmConfig>({
|
|
sources: [
|
|
'https://github.com/bitnami/charts/tree/main/bitnami/harbor',
|
|
],
|
|
}),
|
|
),
|
|
).toBe('https://github.com/bitnami/charts/tree/main/bitnami/harbor');
|
|
|
|
expect(
|
|
findHelmSourceUrl(
|
|
partial<OciHelmConfig>({
|
|
sources: ['https://some.test'],
|
|
}),
|
|
),
|
|
).toBe('https://some.test');
|
|
});
|
|
});
|