Skip to content

Use image catalogers#37

Merged
jedevc merged 2 commits into
docker:masterfrom
cdupuis:use-image-catalogers
Apr 17, 2023
Merged

Use image catalogers#37
jedevc merged 2 commits into
docker:masterfrom
cdupuis:use-image-catalogers

Conversation

@cdupuis
Copy link
Copy Markdown
Contributor

@cdupuis cdupuis commented Feb 21, 2023

This pull request switches from the directory layout catalogers to explicitly use the image layout catalogers to align BuildKit SBOMs with ones created post build.

Additionally, this PR also records file relationships for package files that aren't recorded already so that we can keep track of layer information.

@cdupuis
Copy link
Copy Markdown
Contributor Author

cdupuis commented Feb 23, 2023

@jedevc could you take a look at this here, please?

Comment thread internal/target.go Outdated
Comment thread internal/target.go

// Enable all the image catalogers to mimic same cataloging behavior as syft running on an image
config := cataloger.DefaultConfig()
catalogers := cataloger.ImageCatalogers(config)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

So because we also support scanning directories in the form of the build context using BUILDKIT_SBOM_SCAN_CONTEXT (see https://docs.docker.com/build/attestations/sbom/#arguments), I wonder if we'd stop detecting things from there that we might have otherwise found.

Would AllCatalogers present problems? Curious if maybe the same packages might be duplicated or something like that?

Alternatively, we could make changes to try and indicate to the scanner which of the directories is the context, so we could apply a different scanner? Though this requires some more significant changes to buildkit.

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 can run some experiments and see what the results are with AllCatalogers

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

That would be super helpful 👍 We have some examples in the examples/ directory, that might be interesting to you.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ping @cdupuis did you manage to take a look at this?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As a concrete example of where ImageCataloguers causes a regression: if I have a local context that contains go.mod, ImageCatalogers won't detect any go packages, while DirectoryCataloguers and AllCataloguers do. IMO we probably want to be able to detect this case?

@cdupuis cdupuis force-pushed the use-image-catalogers branch from dd21216 to 783fcfc Compare February 23, 2023 10:53
@cdupuis cdupuis force-pushed the use-image-catalogers branch from 783fcfc to 0c30256 Compare March 28, 2023 23:47
@cdupuis cdupuis force-pushed the use-image-catalogers branch from 0c30256 to f435073 Compare April 17, 2023 07:47
@cdupuis
Copy link
Copy Markdown
Contributor Author

cdupuis commented Apr 17, 2023

@jedevc I've pushed another update to get to the latest version of Syft with all the PRs merged we need. With this, the SBOMs (e2e tests as well as manual tests) are looking good.

@jedevc
Copy link
Copy Markdown
Collaborator

jedevc commented Apr 17, 2023

Can you rebase this onto master? I've just merged #46 🙏

@cdupuis cdupuis force-pushed the use-image-catalogers branch from f435073 to c770900 Compare April 17, 2023 13:45
@jedevc jedevc merged commit afb730f into docker:master Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants