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.

Sign up for GitHub

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

Jump to bottom

Replace grep base images parsing with dockerfile-json #1304

Merged
merged 1 commit into from
Sep 3, 2024
Merged

Replace grep base images parsing with dockerfile-json #1304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions task/buildah-oci-ta/0.2/buildah-oci-ta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,14 +288,12 @@ spec:

BUILDAH_ARGS=()

BASE_IMAGES=$(grep -i '^\s*FROM' "$dockerfile_path" | sed 's/--platform=\S*//' | awk '{print $2}' | (grep -v ^oci-archive: || true))
BASE_IMAGES=$(dockerfile-json "$dockerfile_path" | jq -r '.Stages[] | select(.From | .Stage or .Scratch | not) | .BaseName')
Copy link
Member

Choose a reason for hiding this comment

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

It is now possible to have an empty value here, right whereas before there was always going to be at least one value?

Will this break any functionality elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, previously there was always at least one value - e.g when it was FROM scratch, but now we are omitting that. That's why I could remove all those if conditions that are checking for "scratch".

I tested it with a couple of builds and the e2e tests are passing. And from the logic of the buildah it does not seem anything should break, since when we are passing that further (for example to the sbom), the "scratch" was omitted anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

An empty Dockerfile should seldom happen. However, if it happens accidentally, dockerfile-json will report an error like [dockerfile-json 1.0.8] error: parse "./Dockerfile": dockerfile/parser.Parse file with no instructions. And set -o pipefail is not set in the build step.

Copy link
Member

Choose a reason for hiding this comment

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

It is good that the e2e tests are passing but I don't have much faith in the robustness of the e2e tests lately (i.e. do they test an image which is just FROM scratch)? Would you be able to test this change in a pipeline that has a simple FROM scratch Containerfile?

If you need a file, you can customize this pipeline: https://github.com/konflux-ci/olm-operator-konflux-sample/blob/main/.tekton/gatekeeper-operator-bundle-pull-request.yaml#L35 and just modify the Dockerfile to remove the first stage (or better yet, test it with the first stage and without).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do have a FROM scratch test, see konflux-ci/e2e-tests@9c5c74c and 5573eb3. But I tested it anyway, once again, now also with multiple stages as has the dockerfile you linked.

if [ "${HERMETIC}" == "true" ]; then
BUILDAH_ARGS+=("--pull=never")
UNSHARE_ARGS="--net"
for image in $BASE_IMAGES; do
if [ "${image}" != "scratch" ]; then
unshare -Ufp --keep-caps -r --map-users 1,1,65536 --map-groups 1,1,65536 -- buildah pull $image
fi
unshare -Ufp --keep-caps -r --map-users 1,1,65536 --map-groups 1,1,65536 -- buildah pull $image
done
echo "Build will be executed with network isolation"
fi
Expand Down Expand Up @@ -415,9 +413,7 @@ spec:

touch /shared/base_images_digests
for image in $BASE_IMAGES; do
if [ "${image}" != "scratch" ]; then
buildah images --format '{{ .Name }}:{{ .Tag }}@{{ .Digest }}' --filter reference="$image" >>/shared/base_images_digests
fi
buildah images --format '{{ .Name }}:{{ .Tag }}@{{ .Digest }}' --filter reference="$image" >>/shared/base_images_digests
done

# Needed to generate base images SBOM
Expand Down
10 changes: 3 additions & 7 deletions task/buildah-remote-oci-ta/0.2/buildah-remote-oci-ta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,14 +323,12 @@ spec:

BUILDAH_ARGS=()

BASE_IMAGES=$(grep -i '^\s*FROM' "$dockerfile_path" | sed 's/--platform=\S*//' | awk '{print $2}' | (grep -v ^oci-archive: || true))
BASE_IMAGES=$(dockerfile-json "$dockerfile_path" | jq -r '.Stages[] | select(.From | .Stage or .Scratch | not) | .BaseName')
if [ "${HERMETIC}" == "true" ]; then
BUILDAH_ARGS+=("--pull=never")
UNSHARE_ARGS="--net"
for image in $BASE_IMAGES; do
if [ "${image}" != "scratch" ]; then
unshare -Ufp --keep-caps -r --map-users 1,1,65536 --map-groups 1,1,65536 -- buildah pull $image
fi
unshare -Ufp --keep-caps -r --map-users 1,1,65536 --map-groups 1,1,65536 -- buildah pull $image
done
echo "Build will be executed with network isolation"
fi
Expand Down Expand Up @@ -450,9 +448,7 @@ spec:

touch /shared/base_images_digests
for image in $BASE_IMAGES; do
if [ "${image}" != "scratch" ]; then
buildah images --format '{{ .Name }}:{{ .Tag }}@{{ .Digest }}' --filter reference="$image" >>/shared/base_images_digests
fi
buildah images --format '{{ .Name }}:{{ .Tag }}@{{ .Digest }}' --filter reference="$image" >>/shared/base_images_digests
done

# Needed to generate base images SBOM
Expand Down
10 changes: 3 additions & 7 deletions task/buildah-remote/0.2/buildah-remote.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,14 +305,12 @@ spec:

BUILDAH_ARGS=()

BASE_IMAGES=$(grep -i '^\s*FROM' "$dockerfile_path" | sed 's/--platform=\S*//' | awk '{print $2}' | (grep -v ^oci-archive: || true))
BASE_IMAGES=$(dockerfile-json "$dockerfile_path" | jq -r '.Stages[] | select(.From | .Stage or .Scratch | not) | .BaseName')
if [ "${HERMETIC}" == "true" ]; then
BUILDAH_ARGS+=("--pull=never")
UNSHARE_ARGS="--net"
for image in $BASE_IMAGES; do
if [ "${image}" != "scratch" ]; then
unshare -Ufp --keep-caps -r --map-users 1,1,65536 --map-groups 1,1,65536 -- buildah pull $image
fi
unshare -Ufp --keep-caps -r --map-users 1,1,65536 --map-groups 1,1,65536 -- buildah pull $image
done
echo "Build will be executed with network isolation"
fi
Expand Down Expand Up @@ -432,9 +430,7 @@ spec:

touch /shared/base_images_digests
for image in $BASE_IMAGES; do
if [ "${image}" != "scratch" ]; then
buildah images --format '{{ .Name }}:{{ .Tag }}@{{ .Digest }}' --filter reference="$image" >> /shared/base_images_digests
fi
buildah images --format '{{ .Name }}:{{ .Tag }}@{{ .Digest }}' --filter reference="$image" >> /shared/base_images_digests
done

# Needed to generate base images SBOM
Expand Down
10 changes: 3 additions & 7 deletions task/buildah/0.2/buildah.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,14 +225,12 @@ spec:

BUILDAH_ARGS=()

BASE_IMAGES=$(grep -i '^\s*FROM' "$dockerfile_path" | sed 's/--platform=\S*//' | awk '{print $2}' | (grep -v ^oci-archive: || true))
BASE_IMAGES=$(dockerfile-json "$dockerfile_path" | jq -r '.Stages[] | select(.From | .Stage or .Scratch | not) | .BaseName')
if [ "${HERMETIC}" == "true" ]; then
BUILDAH_ARGS+=("--pull=never")
UNSHARE_ARGS="--net"
for image in $BASE_IMAGES; do
if [ "${image}" != "scratch" ]; then
unshare -Ufp --keep-caps -r --map-users 1,1,65536 --map-groups 1,1,65536 -- buildah pull $image
fi
unshare -Ufp --keep-caps -r --map-users 1,1,65536 --map-groups 1,1,65536 -- buildah pull $image
done
echo "Build will be executed with network isolation"
fi
Expand Down Expand Up @@ -352,9 +350,7 @@ spec:

touch /shared/base_images_digests
for image in $BASE_IMAGES; do
if [ "${image}" != "scratch" ]; then
buildah images --format '{{ .Name }}:{{ .Tag }}@{{ .Digest }}' --filter reference="$image" >> /shared/base_images_digests
fi
buildah images --format '{{ .Name }}:{{ .Tag }}@{{ .Digest }}' --filter reference="$image" >> /shared/base_images_digests
done

# Needed to generate base images SBOM
Expand Down
Loading