Skip to content

fix: make sure all whiteout files are processed#1887

Merged
github-actions[bot] merged 1 commit into
quay:mainfrom
knrc:fix_layer_mapping_files
Jun 10, 2026
Merged

fix: make sure all whiteout files are processed#1887
github-actions[bot] merged 1 commit into
quay:mainfrom
knrc:fix_layer_mapping_files

Conversation

@knrc

@knrc knrc commented May 20, 2026

Copy link
Copy Markdown
Contributor

This PR fixes the mapping for IndexReport files within a layer. The previous mapping was for one file per layer, with one consequence being the inability to handle more than one whiteout file per layer.

@knrc knrc requested a review from a team as a code owner May 20, 2026 01:14
@knrc knrc requested review from hdonnay and removed request for a team May 20, 2026 01:14
@knrc knrc force-pushed the fix_layer_mapping_files branch from 2ea420b to b3fe47d Compare May 20, 2026 01:16
@crozzy

crozzy commented May 20, 2026

Copy link
Copy Markdown
Contributor

f45f117 for reference

@knrc

knrc commented May 20, 2026

Copy link
Copy Markdown
Contributor Author

f45f117 for reference

Yeah, using a slice was my original approach from a year or so back. I changed the implementation when I looked at the way the tests were being written, assuming the intent was to coalesce the entries.

Any idea why that's not in main?

@crozzy

crozzy commented May 21, 2026

Copy link
Copy Markdown
Contributor

I believe it was a classic case of "fix a bug while working on a feature and then decide on a new approach for the feature and lose the bug-fix"

@crozzy crozzy self-requested a review May 21, 2026 22:24

@crozzy crozzy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a couple of comments, both seem kind of pre-existing but it'd be good to fold in if possible

Comment thread whiteout/resolver_test.go
Path: "a/path/to/some/different_file/site-packages/.wh.a_package",
Kind: claircore.FileKindWhiteout,
Files: map[string]map[string]claircore.FileKind{
secondLayerHash.String(): map[string]claircore.FileKind{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be inserting 3 different files into the same layer hash key rather than overwriting it. Existing bug but seems like a good time to address.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment thread whiteout/resolver.go Outdated
slog.DebugContext(ctx, "package determined to be deleted",
"package name", pkg.Name,
"package file", pkg.Filepath,
"whiteout file", path)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we bail out here rather than checking the rest of the files / layers?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I optimised this a bit more, collecting whiteout files first before going through the loop for each package. Let me know what you think.

@knrc knrc force-pushed the fix_layer_mapping_files branch 2 times, most recently from 9e3619a to 7a4787c Compare May 25, 2026 20:24
@knrc

knrc commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

Sorry, one more to rebase on the current main

@knrc knrc force-pushed the fix_layer_mapping_files branch from 7a4787c to 364bdab Compare May 28, 2026 22:54
@hdonnay

hdonnay commented Jun 10, 2026

Copy link
Copy Markdown
Member

This LGTM. Going to rebase this myself and have the CI run.

Signed-off-by: Kevin Conner <kev.conner@gmail.com>
@hdonnay hdonnay force-pushed the fix_layer_mapping_files branch from 364bdab to 8121da3 Compare June 10, 2026 22:17
@hdonnay

hdonnay commented Jun 10, 2026

Copy link
Copy Markdown
Member

/fast-forward

@github-actions github-actions Bot merged commit 8121da3 into quay:main Jun 10, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants