task/buildah: add SKIP_INJECTIONS parameter#2964
Conversation
Today, the buildah task automatically adds `COPY` directives into the Containerfile to inject dynamically-generated files (`content-sets.json` and `labels.json`). This kind of dynamic mutation of the build process makes it very hard to reproduce the build outside of Konflux (especially `labels.json`, which requires parsing the Containerfile). Really, all the information in those files already lives in the upstream repo, and it's not hard for a project to take over that resposibility. Add a new SKIP_INJECTIONS parameter which when set to `true` skips all those mutations. To reiterate more explicitly: all our images should have these files. They are consumed by scanners like Clair. The goal is NOT to ship images without them, but simply to allow projects to take over that responsibility. We will eventually have checks in place that verify the presence and contents of these files. This will block anyone using this parameter without the necessary work upstream to add them. Signed-off-by: Jonathan Lebon <jonathan@jlebon.com>
jlebon
force-pushed
the
pr/skip-injections
branch
from
3f54e37 to
b64b0c4
Compare
| # Conditionally write to the old location for backward compatibility | ||
| if [ "${ICM_KEEP_COMPAT_LOCATION}" = "true" ]; then | ||
| echo 'COPY labels.json /root/buildinfo/labels.json' >>"$dockerfile_copy" | ||
| if [ "${SKIP_INJECTIONS}" = "false" ]; then |
There was a problem hiding this comment.
I just conditionalized the final injection sites rather than also many of the preparatory steps before this one to make this patch easier to review. I could be more invasive if folks prefer.
There was a problem hiding this comment.
This seems reasonable, the preparatory work is not particularly costly 👍
|
/ok-to-test |
What kind of checks do you have in mind? |
|
I'm guessing that ECP will check if necessary labels are correctly set. |
@chmeliik There is ECP rule for this labels already: |
|
@brunoapimentel: The following test has Completed, say /retest to rerun failed tests.
Inspecting Test ArtifactsTo inspect your test artifacts, follow these steps:
mkdir -p oras-artifacts
cd oras-artifacts
oras pull quay.io/konflux-test-storage/konflux-team/build-definitions:b64b0c4a9d49add62d09955f5489f146c8edf458Test results analysis🚨 Error occurred while running the E2E tests, list of failed Spec(s): OCI Artifact Browser URL<not enabled> |
That's for the labels themselves, but not for the existence of labels.json in the filesystem or its content. May I ask you @p-rog or @jlebon to file a Conforma story to add the checks? |
5 participants
Today, the buildah task automatically adds
COPYdirectives into the Containerfile to inject dynamically-generated files (content-sets.jsonandlabels.json). This kind of dynamic mutation of the build process makes it very hard to reproduce the build outside of Konflux (especiallylabels.json, which requires parsing the Containerfile).Really, all the information in those files already lives in the upstream repo, and it's not hard for a project to take over that resposibility.
Add a new SKIP_INJECTIONS parameter which when set to
trueskips all those mutations.To reiterate more explicitly: all our images should have these files. They are consumed by scanners like Clair. The goal is NOT to ship images without them, but simply to allow projects to take over that responsibility. We will eventually have checks in place that verify the presence and contents of these files. This will block anyone using this parameter without the necessary work upstream to add them.