Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve generation of base images SBOMs #1659

Merged
merged 2 commits into from
Dec 9, 2024

Conversation

mkosiarc
Copy link
Contributor

@mkosiarc mkosiarc commented Nov 28, 2024

most functional changes are in the related PR that updates the
base_images_sbom_script.py
konflux-ci/build-tasks-dockerfiles#191

Here, we are just updating on how we generate the inputs for this
script.
We are now passing the whole parsed Dockerfile in json format to that
script, which allows us to better parse/detect base images.

Also, the format of the /shared/base_images_digests file was changed.
Previously we could rely on the order of the image references with the
digests in the file. Now we need to provide a mapping from an image
reference as it was used in the Dockerfile to the full image reference
with digests that was used during build and generated by buildah.

The mapping is done as:

Also, the sbom utility image has to be updated together in the same
PR/commit, otherwise it would break konflux temporarily

KFLUXBUGS-1718

@mkosiarc mkosiarc force-pushed the improve-image-parsin branch 2 times, most recently from cef8f6c to 780dca1 Compare November 28, 2024 09:24
@mkosiarc mkosiarc requested a review from chmeliik November 28, 2024 11:35
@mkosiarc mkosiarc marked this pull request as ready for review November 28, 2024 12:22
@openshift-ci openshift-ci bot requested a review from mmorhun November 28, 2024 12:22
@mkosiarc
Copy link
Contributor Author

has to be merged only after konflux-ci/build-tasks-dockerfiles#191

Copy link
Contributor

@chmeliik chmeliik left a comment

Choose a reason for hiding this comment

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

<image-reference-used-in-dockerfile>|<full-image-reference-with-digest>
The character that creates the mapping "|" was chosen arbitrarily, just
because it cannot be a valid part of image reference.

It would be a bit more natural to just separate them by whitespace, but this is OK too

@mkosiarc
Copy link
Contributor Author

mkosiarc commented Dec 4, 2024

<image-reference-used-in-dockerfile>|<full-image-reference-with-digest>
The character that creates the mapping "|" was chosen arbitrarily, just
because it cannot be a valid part of image reference.

It would be a bit more natural to just separate them by whitespace, but this is OK too

I guess I don't mind using space instead of "|". I just used it for more visibility and because of an (possibly irrational) fear that additional empty space might find its way to the file, causing problems with the parsing.

@chmeliik
Copy link
Contributor

chmeliik commented Dec 4, 2024

<image-reference-used-in-dockerfile>|<full-image-reference-with-digest>
The character that creates the mapping "|" was chosen arbitrarily, just
because it cannot be a valid part of image reference.

It would be a bit more natural to just separate them by whitespace, but this is OK too

I guess I don't mind using space instead of "|". I just used it for more visibility and because of an (possibly irrational) fear that additional empty space might find its way to the file, causing problems with the parsing.

Python handles additional spaces without issues

In [1]: "   image1\t \t image2     \n".split()
Out[1]: ['image1', 'image2']

(and so does Bash, in fact)

$ read -r image1 image2 <<< $'   image1\t \t image2     \n'
$ echo "$image1; $image2;"
image1; image2;

@mkosiarc
Copy link
Contributor Author

mkosiarc commented Dec 4, 2024

Yeah, like I said, it was more irrational :D I can make the switch to using space.

most functional changes are in the related PR that updates the
base_images_sbom_script.py
konflux-ci/build-tasks-dockerfiles#191

Here, we are just updating on how we generate the inputs for this
script.
We are now passing the whole parsed Dockerfile in json format to that
script, which allows us to better parse/detect base images.

Also, the format of the /shared/base_images_digests file was changed.
Previously we could rely on the order of the image references with the
digests in the file. Now we need to provide a mapping from an image
reference as it was used in the Dockerfile to the full image reference
with digests that was used during build and generated by buildah.

The mapping is done as:
<image-reference-used-in-dockerfile> <full-image-reference-with-digest>

Also, the sbom utility image has to be updated together in the same
PR/commit, otherwise it would break konflux temporarily

KFLUXBUGS-1718

Signed-off-by: mkosiarc <[email protected]>
@mkosiarc mkosiarc force-pushed the improve-image-parsin branch from 3aa895f to eee3c4f Compare December 9, 2024 12:38
@mkosiarc mkosiarc mentioned this pull request Dec 9, 2024
1 task
@mkosiarc mkosiarc removed the hold label Dec 9, 2024
@mkosiarc mkosiarc enabled auto-merge December 9, 2024 13:16
@mkosiarc
Copy link
Contributor Author

mkosiarc commented Dec 9, 2024

/retest

@mkosiarc mkosiarc added this pull request to the merge queue Dec 9, 2024
Merged via the queue into konflux-ci:main with commit 9e81f3c Dec 9, 2024
16 checks passed
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.

3 participants