* 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>
Adds a custom oxlint rule renovate/v8-ignore-no-count that flags the
count form '/* v8 ignore next <N> */'. The count is not honored by
vitest's ast-v8-to-istanbul hint parser, which captures only the
keyword (if/else/next/file); 'next' always exempts exactly the next
AST node. Trailing counts therefore suggest a line range that is never
applied and mislead readers.
The rule is autofixable (drops the count, preserving actual behavior)
and has been applied to all existing occurrences.
To prevent potential SSRF attempts, we can ignore a
misconfigured/malicious `next` link, and log a warning.
For self-hosted users, we can provide an experimental override if they
need to opt back in.
Co-authored-by: Claude Opus 4.8 <jamie.tanna+claude-code@mend.io>
Co-authored-by: Claude Sonnet 5 <jamie.tanna+claude-code@mend.io>
* refactor(oxlint): enable func-style
Enable the built-in func-style rule as ["warn", "declaration"] so that
named functions are written as function declarations instead of arrow or
function expressions assigned to variables, matching the convention in
docs/development/best-practices.md (Functions section). Inline arrow
callbacks are unaffected; prefer-arrow-callback continues to cover the
callback direction.
Severity is "warn" instead of "error" as a fallback: a repo-wide run
found 196 pre-existing violations (70 in spec files, 126 in production
code), exceeding the 80-violation fix threshold for this change, so no
violations were fixed here. Follow-up work can convert the offenders
(largest clusters: lib/modules/versioning/ruby, lib/util/cache/package,
lib/modules/versioning/maven, lib/modules/versioning/gradle) and then
raise the rule to "error". Note oxlint 1.72 also supports an
allowTypeAnnotation option if typed consts should remain arrows.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* ci(linter): enforce func-style at error
* refactor(func-style): convert const-arrow functions to declarations in with-cache spec and ruby versioning
* refactor(func-style): convert const-arrow functions to declarations in maven/gradle modules
* refactor(func-style): convert const-arrow functions to declarations in remaining versioning modules
* refactor(func-style): convert const-arrow functions to declarations in platform modules
Discriminated-union closures (ensureCommentRemoval byTopic/byContent
helpers in github, gitlab, bitbucket, bitbucket-server) needed the
narrowed field passed explicitly as a parameter, since function
declarations are hoisted and lose the outer if-narrowing that inline
arrow-function closures previously relied on.
* refactor(func-style): convert const-arrow functions to declarations across util/manager/datasource modules
One case (logger/renovate-logger.ts logOnceFn) is kept as an arrow
function with a documented oxlint-disable: it is stored on
this.logger.once[level] and relies on lexically-bound this, which a
hoisted function declaration would not preserve.
* refactor(func-style): convert remaining const-arrow test helpers to declarations
get-updated.spec.ts: hoisted expectPipCompilePackageAndLockFile to
module scope (it only closes over the module-level pipCompile mock)
and added a documented oxlint-disable for vitest/expect-expect on its
caller test, since oxlint's expect-expect check does not trace
assertions through an externally-defined helper function.
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Enable the unicorn/prefer-at rule as error in .oxlintrc.json and fix all
43 pre-existing violations (arr[arr.length - 1] -> arr.at(-1)) via oxlint
autofix. Where .at() widened the type to T | undefined, non-null
assertions were added at sites where surrounding logic guarantees the
element exists (length guards, split results), preserving the original
runtime behavior and typing.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Enable the type-aware typescript/no-unnecessary-type-arguments rule as
error and remove all 44 pre-existing redundant type arguments (type
arguments identical to the type parameter default), e.g.
HttpResponse<string>, ZodError<unknown>,
PackageDependency<Record<string, any>>.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(docker): replace getJsonUnchecked with validated getJson
Replace all getJsonUnchecked calls in the Docker datasource with
Zod-validated getJson calls. Add RegistryAuthToken, RegistryTagsList,
and QuayTagsResponse schemas to schema.ts and add unit tests for them.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* do not use getJson if we do not look at the body
* chore: default to false for safety
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(config/global): return default values for `GlobalConfig.get`
As noted in #39949, it can be awkward working with in `GlobalConfig.get`, where
we would previously need to specify the default value when calling `.get`,
which could lead to drift with the defaults for a given config option.
To reduce this risk, we can rely on a given `default` for a configuration
option, providing that as a default.
Because this now definitely returns a value that matches the config option, it
now allows us to remove `!` assertions.
* fixup! fix(config/global): return default values for `GlobalConfig.get`
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
* fixup! fix(config/global): return default values for `GlobalConfig.get`
* test(presets): remove impossible test
As it's now not possible to set a value as `undefined` (as
`GlobalConfig` will then use the default), this test no longer validates
anything.
* test(workers/repository): add mock
Now `platform` defaults to `github`, we need to mock out the call to
fetch config.
Co-authored-by: Claude Sonnet 4.6 <jamie.tanna+claude-code@mend.io>
* test: coverage
* test: coverage
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <jamie.tanna+claude-code@mend.io>
* test(noNewURL): add new rule enforcing parseUrl() from 'lib/util/url.ts'
* replace newURL with parseUrl
* remove non-null assertions marks if appropiate
* test: add additional tests
* test: add additional tests and ignore where it makes sense
* implement suggestions
* test: further improve coverage
* apply suggestion
Until the completion of #38659, we should make it clearer that digests
do not have a `releaseTimestamp` associated with them.
This will also be improved in #38656 to provide the correct
`releaseTimestamp` of a given digest.
As flagged in #38212, this can lead to digests looking newer than they
are, and possibly cause `minimumReleaseAge` not to work.
We should surface this to users, and point to the issue for
documentation purposes.
As this note right now is not super clear that this support only works
with Docker Hub's API, which is not uniform with other registries, hence
it being a separate branch of code.
Changes default Docker Hub lookups from index.docker.io to hub.docker.com, which is more efficient. If you are configuring a Docker Hub token for docker.io then you should now configure it for docker.com as well.
Closes#24666
BREAKING CHANGE: Docker Hub lookups prefer hub.docker.com over index.docker.io. Set RENOVATE_X_DOCKER_HUB_TAGS_DISABLE=true in env to revert behavior.
James Griffiths