New buildah task for RHTAP #729

brunoapimentel · 2024-01-08T15:18:20Z

This PR proposes a new simplified and improved buildah task to be used for the new RHTAP docker build pipeline.

Compared to the current Konflux buildah task, this task:

Removes all the Konflux-specific features that are not needed in RHTAP
Uses a productized version of the buildah image
Removes root access from all steps
Sets the correct Syft catalogers when scanning the built image
Pins the output CycloneDX version
Avoids littering the source code workspace with temporary files
Improves the SBOM merge algorithm (dedup by PURL)
Add a the SBOM OCI artifact URL as a task result
Changes the tags

task/buildah-dance/0.1/buildah-dance.yaml

arewm · 2024-01-08T20:06:09Z

task/buildah-dance/0.1/buildah-dance.yaml

+      name: varlibcontainers
+    workingDir: $(workspaces.source.path)
+
+  - name: upload-sbom


@lcarva, do you have thoughts on whether we should change the process for uploading SBOMs to the registry for customer utilization? Is cosign attach sbom still the best option?

Funny thing is that cosign attach sbom actually uploads the sbom to the registry as a blob which is great. what we do need is to record the digest of the sbom so we can ensure it has not changed post-build. We had this at some point but had to revert because that was the tipping point for exceeding the size of the results in the buildah task.

In konflux, we can get around this because the sbom is always embedded in the image itself, so EC can just use that.

We are removing the embedded SBOM for RHTAP in this PR, so we won't be able to rely on that in case we need to. 😬
(no changes for Konflux though)

Oh... Can we introduce the SBOM_BLOB_URL for this task, then?

I'm happy to do so as a follow up PR.

I found this is already a requirement for RHTAP (related comment). It is supposed to be renamed to LINK_TO_SBOM, but I believe it has the same purpose.

Although, I'm confused on how to implement this now. I entered the deprecate cosign attachment rabbit hole once I saw the deprecation message in the CLI, and the task currently still is dependent on attach.

My suggestion is to keep using the cosign attach command as it is done today. It is deprecated, but right now we don't really have a better alternative. The main thing here is that that command uploads the SBOM to the registry as a "blob". So just record the sh256 digest of the SBOM and we should be able to find it in the registry, e.g. quay.io/foo/bar@sha256:abc...

I can help you with this. Feel free to ping me.

So, @lcarva, I took the same approach to generate the digest as in the PR you linked. Thing is, taking the sha256sum sbom-cyclonedx.json will give me the digest of the layer, not of the SBOM image that is pushed to the registry.

You can see the test I did in https://quay.io/repository/bpimente/test-images:

the SBOM image digest is ed244f35bd7959a1149836997cd568361dbae6e496ffa560821e0d29c46c94fc

the only layer existing has a digest of 57a8c881a24e554cecd66dce96645b55b331138c9613586c3507482b09f0e3c4

I can compute the latter if I

cosign download sbom quay.io/bpimente/test-images:test-rhtap > sbom.json sha256sum sbom.json

Maybe I'm doing something wrong?

I'll move this to a separate PR so we can wrap up the work on this one here.

Bruno and I chatted offline, summarizing it here for posterity/transparency.

We want to record the digest of the layer (which should match the digest of the SBOM file). This is what EC will fetch. An SBOM image could, in theory, contain multiple SBOMs. Adding an SBOM changes the image digest, but not the digest of the other SBOMs (as layers).

task/buildah-dance/0.1/buildah-dance.yaml

task/buildah-dance/OWNERS

chmeliik

Oh no, taking a good look at our buildah task did not end well. Congratulations @brunoapimentel, you get a review for the whole task 😁

Some of the comments are definitely not blockers, we can probably just file new stories or say it's acceptable

task/buildah-rhtap/0.1/buildah-rhtap.yaml

task/buildah-rhtap/OWNERS

pipelines/docker-build-dance/patch.yaml

brunoapimentel · 2024-01-17T02:27:46Z

Newer pushes are addressing some of the comments, but there are still quite a few comments to address.

The idea of using the default buildah task from the catalog proved not very promising, at least not for the current phase. I think the way the catalog handles tasks is very different from what we currently do, which makes the current approach of cleaning the current buildah task still easier.

The catalog keeps a separate repository for scripts and templates, and generates the actual tasks with the scripts files added as base64 encoded strings. My initial idea was to simply add the decoded scripts to our task, but it proved to be very verbose in the end, not to mention we lose the ability to reuse functions among different steps, since the script files can be imported whenever needed.

task/buildah-rhtap/0.1/buildah-rhtap.yaml

chmeliik

Mostly LGTM for the first iteration 👍

task/buildah-rhtap/0.1/buildah-rhtap.yaml

chmeliik · 2024-01-24T14:43:19Z

task/buildah-rhtap/0.1/buildah-rhtap.yaml

+    name: build
+    script: |
+      # Check if the Dockerfile exists
+      SOURCE_CODE_DIR=source


[follow-up] We will probably want to remove this, I don't expect that the RHTAP clone task would still clone the source to the source subdirectory of the workspace.

IIRC we want to use the official git task, which makes the subdirectory configurable https://github.com/openshift-pipelines/tektoncd-catalog/tree/p/tasks/task-git/0.2.0#git-tekton-task (but clones to the root of the workspace by default)

I will add this to the list of follow up stories.

task/buildah-rhtap/0.1/buildah-rhtap.yaml

chmeliik · 2024-01-24T14:57:26Z

task/buildah-rhtap/0.1/buildah-rhtap.yaml

+
+      for component in source_sbom.get("components", []):
+        if "purl" in component:
+          if component["purl"] not in existing_purls:


I think component["purl"] is a string, while existing_purls is a list[dict] (so this condition is never true)

You're absolutely right, I stopped halfway through that filter thing. It should be right now (this part at least... deduplication will need major rework)

task/buildah-rhtap/0.1/buildah-rhtap.yaml

mmorhun

I think we can start from current state of the new Buildah task and fix found problems in follow up PRs.

The new buildah task that will be used in RHTAP will be closely derived from the current buildah task used in Konflux. This commit simply copies the current code, so it is easier to follow the diff in the following commits. Signed-off-by: Bruno Pimentel <bpimente@redhat.com>

The removed parts were: - References to the JVM build service - References to Cachi2 prefetched content - Mechanism to make build hermetic - Generation of PURL SBOM - Copying of the SBOM into the built container image - Setting quay.io expiration date - Setting labels to the build image - Fetching a Dockerfile/.dockerignore from a remote location - Setting compute resources - Unused task params - Adding annotations to the built image - Enabling short name resolution Signed-off-by: Bruno Pimentel <bpimente@redhat.com>

The image present in Red Hat's catalog is properly productized and should be preferred instead of the current quay.io image. We also don't want to allow the user to set any arbitrary builder image to be used in this task. Signed-off-by: Bruno Pimentel <bpimente@redhat.com>

To avoid the need of root access, the SBOMs needed to be written in a separate volume instead of the "source" workspace. This also brings the benefit of not littering the cloned source code with files. The creation of "/workspace/container_name" was also removed since it was unused. Signed-off-by: Bruno Pimentel <bpimente@redhat.com>

The built image was being mounted as a directory, which makes Syft use a different set of catalogers than it would use in case it was scanning an image. This patch instead saves the image content as an oci-dir, which uses the same sets of catalogers than scanning an image from a registry or from a container daemon. Signed-off-by: Bruno Pimentel <bpimente@redhat.com>

Newer versions of Syft will default to the latest supported CycloneDX version, which can potentially bring breaking changes. Signed-off-by: Bruno Pimentel <bpimente@redhat.com>

Signed-off-by: Bruno Pimentel <bpimente@redhat.com>

Since we don't want to make the digest available to any other tasks through the workspace (it is emitted as a task result), this patch changes the behavior so it is only saved in a temporary volume. Signed-off-by: Bruno Pimentel <bpimente@redhat.com>

The combination of name+version is not a good enough to guarantee uniqueness, since there could be cases that components from different ecosystems (e.g. npm, rpm, golang) share the same name+version. The purl, on the other hand, takes in consideration not only name and version, but also the ecosystem, and potentially other things. There are cases in which the purl is absent, though, namely for components of type operating-system. Since they are only reported once in when scanning the image (not the source code), this case alone won't result in duplication. We still adding all components that lack a purl (at the risk of duplication) to avoid errors in scenarios we aren't aware of right now. Signed-off-by: Bruno Pimentel <bpimente@redhat.com>

The new buildah-rhtap task should replace the current existing buildah task, as that one has Konflux-specific features. Signed-off-by: Bruno Pimentel <bpimente@redhat.com>

sonarqubecloud · 2024-01-24T19:21:04Z

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

brunoapimentel · 2024-01-25T02:36:45Z

The newer push is just rebasing the branch. We still need to create the repository for the new task's image, since the CI is failing because of it.

chmeliik · 2024-01-25T14:31:45Z

/retest

chmeliik · 2024-01-25T15:41:44Z

@arewm it looks like your change request is blocking the merge button

task/buildah-rhtap/0.1/buildah-rhtap.yaml

arewm · 2024-01-25T16:26:53Z

task/buildah-rhtap/0.1/buildah-rhtap.yaml

+  - name: upload-sbom
+    image: quay.io/redhat-appstudio/cosign:v2.1.1@sha256:c883d6f8d39148f2cea71bff4622d196d89df3e510f36c140c097b932f0dd5d5


I think we should be able to modify this to the RHTAS images

Do you have a link for them?

https://catalog.redhat.com/software/containers/rhtas-tech-preview/cosign-rhel9/652971cd584d77e9d922883b?architecture=amd64&image=65c5506e19b6942d637e5603

brunoapimentel changed the title ~~Buildah dance~~ New buildah task for RHTAP Jan 8, 2024

brunoapimentel requested review from mmorhun, arewm and chmeliik January 8, 2024 15:19

arewm requested changes Jan 8, 2024

View reviewed changes

brunoapimentel force-pushed the buildah-dance branch from 3705b80 to bb8079c Compare January 8, 2024 22:20

chmeliik reviewed Jan 11, 2024

View reviewed changes

lkolacek reviewed Jan 15, 2024

View reviewed changes

pipelines/docker-build-dance/patch.yaml Outdated Show resolved Hide resolved

brunoapimentel force-pushed the buildah-dance branch from bb8079c to c39cef1 Compare January 17, 2024 00:50

brunoapimentel force-pushed the buildah-dance branch 5 times, most recently from b62bc4b to 86c8513 Compare January 22, 2024 22:55

mmorhun reviewed Jan 24, 2024

View reviewed changes

mmorhun requested review from arewm and chmeliik January 24, 2024 13:08

chmeliik reviewed Jan 24, 2024

View reviewed changes

mmorhun approved these changes Jan 24, 2024

View reviewed changes

brunoapimentel force-pushed the buildah-dance branch 2 times, most recently from b23b2ac to 9d6f2d3 Compare January 24, 2024 16:41

chmeliik approved these changes Jan 24, 2024

View reviewed changes

brunoapimentel marked this pull request as ready for review January 24, 2024 18:50

brunoapimentel added 6 commits January 24, 2024 16:20

buildah-rhtap: force the CycloneDX version to be 1.5

a607227

Newer versions of Syft will default to the latest supported CycloneDX version, which can potentially bring breaking changes. Signed-off-by: Bruno Pimentel <bpimente@redhat.com>

brunoapimentel added 4 commits January 24, 2024 16:20

buildah-rhtap: push the image in the build step

3f0150e

Signed-off-by: Bruno Pimentel <bpimente@redhat.com>

Change the docker-build-dance pipeline to use the buildah-rhtap task

1437d89

The new buildah-rhtap task should replace the current existing buildah task, as that one has Konflux-specific features. Signed-off-by: Bruno Pimentel <bpimente@redhat.com>

brunoapimentel force-pushed the buildah-dance branch from 9d6f2d3 to 1437d89 Compare January 24, 2024 19:20

arewm approved these changes Jan 25, 2024

View reviewed changes

mmorhun merged commit 39bc0e1 into konflux-ci:main Jan 26, 2024
7 checks passed

rh-tap-build-team bot mentioned this pull request Jan 26, 2024

build-definitions update redhat-appstudio/infra-deployments#3080

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New buildah task for RHTAP #729

New buildah task for RHTAP #729

brunoapimentel commented Jan 8, 2024 •

edited

Loading

arewm Jan 8, 2024

lcarva Jan 8, 2024

brunoapimentel Jan 8, 2024

lcarva Jan 9, 2024

brunoapimentel Jan 10, 2024

lcarva Jan 10, 2024

brunoapimentel Jan 23, 2024

brunoapimentel Jan 24, 2024

lcarva Jan 24, 2024

chmeliik left a comment •

edited

Loading

brunoapimentel commented Jan 17, 2024 •

edited

Loading

chmeliik left a comment

chmeliik Jan 24, 2024

brunoapimentel Jan 24, 2024

chmeliik Jan 24, 2024

brunoapimentel Jan 24, 2024

mmorhun left a comment

sonarqubecloud bot commented Jan 24, 2024

brunoapimentel commented Jan 25, 2024

chmeliik commented Jan 25, 2024

chmeliik commented Jan 25, 2024

arewm Jan 25, 2024

brunoapimentel Feb 12, 2024

arewm Feb 14, 2024

		- name: upload-sbom
		image: quay.io/redhat-appstudio/cosign:v2.1.1@sha256:c883d6f8d39148f2cea71bff4622d196d89df3e510f36c140c097b932f0dd5d5

New buildah task for RHTAP #729

New buildah task for RHTAP #729

Conversation

brunoapimentel commented Jan 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chmeliik left a comment • edited Loading

Choose a reason for hiding this comment

brunoapimentel commented Jan 17, 2024 • edited Loading

chmeliik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmorhun left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Jan 24, 2024

Quality Gate passed

brunoapimentel commented Jan 25, 2024

chmeliik commented Jan 25, 2024

chmeliik commented Jan 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brunoapimentel commented Jan 8, 2024 •

edited

Loading

chmeliik left a comment •

edited

Loading

brunoapimentel commented Jan 17, 2024 •

edited

Loading