1. Parse repos without metadata in ParseStorage
The timestamp check in ParseStorage was skipping repos that exist in
storage but don't have metadata. When GetRepoLastUpdated returns zero
time (no metadata), we should always parse the repo to create its
metadata. Check if metaLastUpdated is zero before comparing timestamps.
If zero, always parse regardless of storageLastUpdated.
2. Change the logic of how LastUpdated is computed in RepoSummary
It is not the latest tagged timestamp from the available images or
the last updated image created timestamp, based on whichever is the
latest.
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
feat(meta): add TaggedTimestamp field and preserve during re-parsing
Add TaggedTimestamp field to track when image tags were created, exposed
through GraphQL API. Previously, when zot restarted and re-parsed storage,
ResetRepoReferences would clear all tags, causing timestamp information to
be lost and reset to the service restart time for existing images.
This change adds TaggedTimestamp support and modifies ResetRepoReferences to
selectively preserve tags that still exist in storage, maintaining their
TaggedTimestamp values. Tags that no longer exist in storage are removed as
before.
Changes:
- Add TaggedTimestamp field to GraphQL ImageSummary schema
- Update GraphQL conversion functions to populate TaggedTimestamp with
fallback to PushTimestamp when unavailable
- Updated ResetRepoReferences interface to accept tagsToKeep parameter
- Modified ParseRepo to collect tags from storage before resetting
- Updated all backend implementations (Redis, DynamoDB, BoltDB) to preserve
tags in tagsToKeep instead of clearing all tags
- Updated tests and mocks to match new signature
This ensures TaggedTimestamp accurately reflects when tags were originally
created, and exposes this information through the GraphQL API.
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
* fix: prevent nil pointer dereference in RemoveImageFromRepoMeta
This commit fixes a critical bug where RemoveImageFromRepoMeta crashes with
a nil pointer dereference during retention policy execution and GC operations.
Root Cause:
The function was accessing blob metadata without checking if it exists first.
During GC/retention operations, the metadata database might have stale
references to blobs that no longer exist, causing runtime panics.
Changes:
- Added nil check for descriptorBlobInfo before accessing LastUpdated field
- Added nil check for blobInfo before dereferencing Size, Vendors, Platforms, and SubBlobs
- Made the function consistent with recalculateAggregateFields which already had these checks
Impact:
- Fixes crashes during retention policy execution
- Fixes crashes during GC manifest removal
- Fixes image deletion failures via API
- Eliminates need for dryRun: true workaround in retention config
The fix gracefully handles missing blob metadata by skipping those entries
instead of crashing.
Signed-off-by: Gianluca Boiano <morf3089@gmail.com>
* test: add comprehensive tests for RemoveImageFromRepoMeta nil checks
Add test coverage for the nil pointer dereference fixes in RemoveImageFromRepoMeta.
These tests ensure the function handles missing blob metadata gracefully during
GC and retention operations.
Test cases:
- Handle nil blob info for descriptor digest (line 280 check)
- Handle nil blob info in queue traversal (line 297 check)
- Verify correct behavior with valid blob info
- Handle empty tags edge case
- Skip tags with empty digest
Coverage: RemoveImageFromRepoMeta now has 100% test coverage
Signed-off-by: Gianluca Boiano <morf3089@gmail.com>
* test: fix RemoveImageFromRepoMeta tests to match actual usage
Address review feedback:
- Delete tag from repoMeta.Tags before calling RemoveImageFromRepoMeta
- Fix blob count expectations after tag removal
- Add assertion to verify tag was removed from metadata
- Update comments to clarify expected behavior
Signed-off-by: Gianluca Boiano <morf3089@gmail.com>
* test: add tag removal assertion to second test case
Add missing assertion to verify tag1 was removed from resultMeta.Tags
in the 'should handle nil blob info in queue traversal' test.
Signed-off-by: Gianluca Boiano <morf3089@gmail.com>
* refactor: improve nil blob handling documentation and test coverage
Address Copilot review feedback:
- Expand comment at line 278 to explain implications of skipping tags
with missing blob info, clarifying that metadata inconsistency is
acceptable in GC/cleanup scenarios
- Revise 'should handle nil blob info for descriptor digest' test to
cover more realistic scenario: remove tag1 while tag2 has missing
blob info, demonstrating graceful handling of data inconsistencies
in remaining tags during removal operations
All tests pass with 49 total assertions.
Signed-off-by: Gianluca Boiano <morf3089@gmail.com>
* fix: prevent nil pointer in GetCandidates when statistics missing
Add defensive check in pkg/retention/candidate.go to handle cases where
a tag exists in repoMeta.Tags but has no corresponding entry in
repoMeta.Statistics. This prevents incorrect retention decisions based
on zero-value timestamps.
Changes:
- Check statistics existence before creating candidates
- Skip tags with missing statistics (retained by GetRetainedTagsFromMetaDB)
- Improve performance from O(n*m) to O(n) by using direct map lookup
- Add comprehensive test coverage for missing statistics scenarios
This addresses the concern raised in PR #3658 about metadata
inconsistencies due to non-transactional writes to blob store and metaDB.
Related: #3658
Signed-off-by: Gianluca Boiano <morf3089@gmail.com>
* test: achieve 100% coverage for RemoveImageFromRepoMeta nil checks
Enhance test coverage for RemoveImageFromRepoMeta to address codecov failures
by adding comprehensive test cases that exercise all code paths including nil
pointer checks and continue statements.
Changes:
- Enhanced 'nil blob info for descriptor digest' test to verify processing
continues with other valid tags after skipping nil entries
- Enhanced 'nil blob info in queue traversal' test to handle mixed valid/nil
sub-blobs and verify correct processing continuation
- Added 'multiple nil blobs in deeply nested structure' test to cover complex
scenarios with multiple missing blobs at various nesting levels
- Enhanced 'skip tags with empty digest' test to verify processing continues
with valid tags after skipping empty digest entries
- Added 'combined edge cases' test to verify all edge cases work together:
empty digest, nil descriptor blob, and nil queue blob
Coverage Results:
- RemoveImageFromRepoMeta: 100.0% line coverage (was 87.50%)
- All 7 test scenarios pass with 75 total assertions
- All nil check code paths fully exercised
- All continue statement behaviors validated
Fixes codecov/patch failure on PR #3658 where 2 lines were missing coverage.
Signed-off-by: Gianluca Boiano <morf3089@gmail.com>
---------
Signed-off-by: Gianluca Boiano <morf3089@gmail.com>
fix: error handling: return nil explicitly on successful completion
Several functions in pkg/meta/redis/redis.go were returning 'err' at the
end of successful execution paths, which could lead to incorrect error
handling when 'err' was overwritten in loops or conditionals.
Changed the following functions to return nil explicitly when all
operations succeed:
- SearchRepos: return nil instead of err after successful loop
- SearchTags: return nil instead of err after successful loop
- GetRepoMeta: return nil instead of err after successful operations
- GetImageMeta: return nil instead of err after successful operations
- GetReferrersInfo: return nil instead of err after successful loop
This ensures that when functions complete successfully, they explicitly
return nil rather than relying on the last value of err, which may have
been overwritten during execution. This fixes TestRedisUnreachable which
was failing because SearchRepos was incorrectly returning nil error when
Redis was unreachable.
See failure in: https://github.com/project-zot/zot/actions/runs/19729927463/job/56528529923?pr=3599
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
fix(meta): handle cases where repositories when substores are nested
Note this is a case of bad configuration: having multiple stores
in the same tree structure. Guard against it in parse.go.
Fix getAllRepos to prevent duplicate repositories in metaDB when substore
directories are nested under the default store root directory.
The fix processes substores first, then the default store, using a
map-based deduplication approach to skip repositories that have already
been added. This ensures that when both the default store and substores
contain repositories with the same name (e.g., when a substore is nested
within the default store), only one instance is added to the repository
list.
Add test TestNoDuplicateReposWithSubstoresAndNestedRepoNames to verify
the deduplication logic works correctly with nested substores.
Also update the other tests to avoid these issues in the future
this is not a vali configuration.
This is not the intended use case for substores, and it may have caused:
https://github.com/project-zot/zot/actions/runs/19665302669/job/56320640980
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
fix: make sure metadb statistics are initialized on image download, and minor metadb fixes for Docker v2 manifest compatibility
Looking into potential causes of https://github.com/project-zot/zot/issues/3163
1. One possible reason is the statistics were not properly initialized in the first place because of (unknown and/or unavoidable) errors on image push.
To workaround this add logic to initialize the statistics on the call to download them.
2. Some images have the download statistics while others dont, one cause could be a bug in the logic handling manifest mediatypes in the search extension.
Add compatibility checks for Docker v2 manifest types in metadb convert functions, and more tests for covering the Docker mediatype use case.
Side fixes:
- Ensure PushedBy Statistics entries are properly initialized in SetRepoReference
- Fix and issue in the image upload test functions, they were uploading docker images with oci mediatypes in call headers
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
GC and scrub should not stop if a manifest or index is missing from storage.
Other similar changes are also included.
WRT metadb, the missing manifests cannot be added, and the results returned from metadb
do not include the descriptors for these manifests.
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
See: https://github.com/project-zot/zot/issues/2506
Note we are not loosing anything functionality-wise by making this change.
Initially we considered the tags are in the annotations present in the referrers
but the only annotations we set on referrers are the ones inside the manifests themselves,
not the ones in the manifest descriptors, so the tags were not presetn anyway.
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
* fix: migrate to Go module v2 for proper semantic versioning
This change updates the module path from 'zotregistry.dev/zot' to
'zotregistry.dev/zot/v2' to comply with Go's semantic versioning rules.
According to Go's module versioning requirements, major version v2+
must include the major version in the module path. The current
module path 'zotregistry.dev/zot' only supports v0.x.x and v1.x.x
versions, making existing v2.x.x tags (like v2.1.8) unusable.
Changes:
- Updated go.mod module path to zotregistry.dev/zot/v2
- Updated all internal import paths across 280+ Go source files
- Updated configuration files (golangcilint.yaml, gqlgen.yml)
- Updated README.md Go reference badge
This fix enables proper use of existing v2.x.x Git tags and allows
external packages to import zot v2+ versions without compatibility
errors.
Resolves: Go module import compatibility for v2+ versions
Fixes: #3071
Signed-off-by: Luca Muscariello <muscariello@ieee.org>
* fix: regenerate GraphQL files with updated v2 import paths
The gqlgen tool needs to regenerate the GraphQL schema files after
the module path change to use the new v2 imports.
Signed-off-by: Luca Muscariello <muscariello@ieee.org>
---------
Signed-off-by: Luca Muscariello <muscariello@ieee.org>
* fix: migrate from github.com/rs/zerolog to golang-native log/slog
We have been using zerolog for a really long time.
golang now has structured logging using slog.
Best to move to this in interests of long-term support.
This is a tech debt item.
Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com>
* fix: a few changes on top
Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com>
* fix: address comments
Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com>
---------
Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com>
* fix: update download counters for docker media types
closes#2929
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
* fix: handle docker config mediatype in MetaDB
The OS/Arch/Layer History information was not written to MetaDB
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
---------
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
* feat: add redis cache support
https://github.com/project-zot/zot/pull/2005
Fixes https://github.com/project-zot/zot/issues/2004
* feat: add redis cache support
Currently, we have dynamoDB as the remote shared cache but ideal only
for the cloud use case.
For on-prem use case, add support for redis.
Signed-off-by: Ramkumar Chinchani <rchincha@cisco.com>
* feat(redis): added blackbox tests for redis
Signed-off-by: Petu Eusebiu <peusebiu@cisco.com>
* feat(redis): dummy implementation of MetaDB interface for redis cache
Signed-off-by: Alexei Dodon <adodon@cisco.com>
* feat: check validity of driver configuration on metadb instantiation
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
* feat: multiple fixes for redis cache driver implementation
- add missing method GetAllBlobs
- add redis cache tests, with and without mocking
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
* feat(redis): redis implementation for MetaDB
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
* feat(redis): use redsync to block concurrent write access to the redis DB
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
* feat(redis): update .github/workflows/cluster.yaml to also test redis
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
* feat(metadb): add keyPrefix parameter for redis and remove unneeded method meta.Crate()
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
* feat(redis): support RedisCluster configuration and add unit tests
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
* feat(redis): more tests for redis metadb implementation
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
* feat(redis): add more examples and update examples/README.md
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
* feat(redis): move option parsing and redis client initialization under pkg/api/config/redis
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
* chore(cachedb): move Cache interface to pkg/storage/types
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
* feat(redis): reorganize code in pkg/storage/cache.go
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
* feat(redis): call redis.SetLogger() with the zot logger as parameter
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
* feat(redis): rename pkg/meta/redisdb to pkg/meta/redis
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
---------
Signed-off-by: Ramkumar Chinchani <rchincha@cisco.com>
Signed-off-by: Petu Eusebiu <peusebiu@cisco.com>
Signed-off-by: Alexei Dodon <adodon@cisco.com>
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
Co-authored-by: a <a@tuxpa.in>
Co-authored-by: Ramkumar Chinchani <rchincha@cisco.com>
Co-authored-by: Petu Eusebiu <peusebiu@cisco.com>
Co-authored-by: Alexei Dodon <adodon@cisco.com>
GHSA-c9p4-xwr9-rfhx
authN/authZ creds are added to the request context so that they can be
tracked and enforced in the various subsystems. However, it was
previously a appended list (incorrectly); consequently, even if the user
has been removed from the group configuration, the user could still
log in.
Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com>
* chore(dynamodb): refactor multiple apikey metadb calls into a single one
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
* fix(metadb): wrong error message in PatchDB() implementation
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
---------
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
* feat: add support for docker images
Issue #724
A new config section under "HTTP" called "Compat" is added which
currently takes a list of possible compatible legacy media-types.
https://github.com/opencontainers/image-spec/blob/main/media-types.md#compatibility-matrix
Only "docker2s2" (Docker Manifest V2 Schema V2) is currently supported.
Garbage collection also needs to be made aware of non-OCI compatible
layer types.
feat: add cve support for non-OCI compatible layer types
Signed-off-by: Ramkumar Chinchani <rchincha@cisco.com>
*
Signed-off-by: Ramkumar Chinchani <rchincha@cisco.com>
* test: add more docker compat tests
Signed-off-by: Ramkumar Chinchani <rchincha@cisco.com>
* feat: add additional validation checks for non-OCI images
Signed-off-by: Ramkumar Chinchani <rchincha@cisco.com>
* ci: make "full" images docker-compatible
Signed-off-by: Ramkumar Chinchani <rchincha@cisco.com>
---------
Signed-off-by: Ramkumar Chinchani <rchincha@cisco.com>
when a UI client tries to view image details
for an image with multiple tags pointing to the same digest
we make a query to dynamodb having duplicate keys (same digest)
resulting in an error and the client is redirect back to image
overview.
closes: #2464
Signed-off-by: Petu Eusebiu <peusebiu@cisco.com>
- update cve tests
- update scrub tests
- update tests for parsing storage and loading into meta DB
- update controller tests
Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
* fix(scheduler): data race when pushing new tasks
the problem here is that scheduler can be closed in two ways:
- canceling the context given as argument to scheduler.RunScheduler()
- running scheduler.Shutdown()
because of this shutdown can trigger a data race between calling scheduler.inShutdown()
and actually pushing tasks into the pool workers
solved that by keeping a quit channel and listening on both quit channel and ctx.Done()
and closing the worker chan and scheduler afterwards.
Signed-off-by: Petu Eusebiu <peusebiu@cisco.com>
* refactor(scheduler): refactor into a single shutdown
before this we could stop scheduler either by closing the context
provided to RunScheduler(ctx) or by running Shutdown().
simplify things by getting rid of the external context in RunScheduler().
keep an internal context in the scheduler itself and pass it down to all tasks.
Signed-off-by: Petu Eusebiu <peusebiu@cisco.com>
---------
Signed-off-by: Petu Eusebiu <peusebiu@cisco.com>
wait for workers to finish before exiting
should fix tests reporting they couldn't remove rootDir because it's being
written by tasks
Signed-off-by: Petu Eusebiu <peusebiu@cisco.com>
- MetaDB stores the time of the last update of a repo
- During startup we check if the layout has been updated after the last recorded change in the db
- If this is the case, the repo is parsed and updated in the DB otherwise it's skipped
Signed-off-by: Laurentiu Niculae <niculae.laurentiu1@gmail.com>
in the case of an already existing meta db without pushTimestamp field
its value would be 0 until image is updated, check for zero values and update them
with time.Now() so that retention logic won't remove them.
Signed-off-by: Petu Eusebiu <peusebiu@cisco.com>
Andrei Aaron