publish images: amd64/arm64 images containing cli only #2770

leppeK · 2024-08-20T05:49:26Z

build desired target with a buildargument

This way we are not diverging in multiple dockerfiles and we can choose between two targets:

cilium-cli-ci The original target based on ubuntu and the default
cilium-cli Only contains the cilium cli based on scratch

Fixes: #2755

leppeK · 2024-08-20T07:30:22Z

As there are changes to github ci i think these steps need to run from this branch to check if it builds correctly

Dockerfile-cli-only

.github/workflows/images.yaml

asauber

Thanks for the refactor, looks good to me except for one change.

leppeK · 2024-09-02T06:48:41Z

Thanks for the refactor, looks good to me except for one change.
@asauber I believe it is in order now, can you sign off the requested changes?

asauber

LGTM. Thanks for the updates.

viktor-kurchenko

Thanks!

leppeK · 2024-09-17T04:18:35Z

@michi-covalent Anything blocking this merge?

michi-covalent · 2024-09-17T14:22:21Z

hi @leppeK thanks for the PR 👋 . a couple of things:

please resolve the conflict in Dockerfile.
this wouldn't work as is because there is no quay.io/cilium/cilium-cli repository right now, and i'm somewhat hesitant to introduce a new image repository for this purpose. could we use a different tag suffix (maybe something like -scratch) instead? that's what we do with https://quay.io/repository/cilium/cilium-ci?tab=tags. we push 2 additional images -race and -unstripped. the workflow is defined in https://github.com/cilium/cilium/blob/main/.github/workflows/build-images-ci.yaml.

cc @tklauser what do you think? 💭

leppeK · 2024-09-17T14:46:36Z

@michi-covalent sure, the conflicts can be resolved by me.

The reason for the different repo is because the -ci suffix implies it is used for ci purposes instead of an official image. So on the perspective of an user it is more clear to have a seperate repo.

tklauser

If I understand this change correctly, we would be publishing these cli-only images on every PR and push to main. I think we should only publish these for releases. Or do you have a specific use case in mind where you'd need to publish these images on each PR/push?

leppeK · 2024-09-17T14:48:51Z

If I understand this change correctly, we would be publishing these cli-only images on every PR and push to main. I think we should only publish these for releases. Or do you have a specific use case in mind where you'd need to publish these images on each PR/push?

I think it is okay to only do releases for these images so we'd only have stable/latest and release tags.

Otherwise it doesn't really matter to rebuild all the time because the layer for amd64 rlshould already exist from the builder regardless and you'd probably still want to make sure arm64 can also build

There was no prebuilt image available for multiple architectures, this provides an image from scratch only containing cilium-cli binary. This ensures that the image can be used directly as a standalone solution or included within other containers example dockerfile --- FROM quay.io/cilium/cilium-cli:tag as cilium FROM exampleimage COPY --from=cilium /usr/local/bin/cilium /usr/local/bin/cilium --- Fixes: cilium#2755 Signed-off-by: Merijn Keppel <[email protected]>

This way we are not diverging in multiple dockerfiles and we can choose between two targets: - cilium-cli-ci The original target based on ubuntu and the default - cilium-cli Only contains the cilium cli based on scratch Fixes: cilium#2755 Signed-off-by: Merijn Keppel <[email protected]>

@asauber

@asauber pointed out i referenced a Dockerfile that does not exist anymore Fixes: cilium#2755 Signed-off-by: Merijn Keppel <[email protected]>

joestringer · 2024-10-04T17:59:33Z

.github/workflows/images.yaml

-            quay.io/${{ github.repository_owner }}/${{ matrix.name }}-ci:latest
-            quay.io/${{ github.repository_owner }}/${{ matrix.name }}-ci:${{ steps.tag.outputs.tag }}
+            quay.io/${{ github.repository_owner }}/${{ matrix.name }}:latest
+            quay.io/${{ github.repository_owner }}/${{ matrix.name }}:${{ steps.tag.outputs.tag }}


Reviewing on behalf of @cilium/security: We keep the CI workflows, builds, environment and secrets entirely separate from the production versions to reduce the risk to supply chain security.

I think it probably makes sense to split this overall effort into two steps:

Update the CI workflow to build arm64 builds and make any potential changes to the dockerfiles for development builds. Do not change the target repositories.

Create another separate PR, separate workflow for publishing production builds.

@michi-covalent / @tklauser then the other aspect is how we're validating these workflow changes. I believe that deliberately these changes to the workflows would not be tested from the fork, because that would mean that arbitrary contributors could hijack the workflows to do what they want. So we probably need to collaborate a little more closely to validate any changes like this separately. That might look like enabling this on a third party github repo and validating there, or at your discretion merging the commits and keeping an eye on them for any potential breakage in the tree.

this wouldn't work as is because there is no quay.io/cilium/cilium-cli repository right now, and i'm somewhat hesitant to introduce a new image repository for this purpose. could we use a different tag suffix (maybe something like -scratch) instead? that's what we do with https://quay.io/repository/cilium/cilium-ci?tab=tags. we push 2 additional images -race and -unstripped. the workflow is defined in https://github.com/cilium/cilium/blob/main/.github/workflows/build-images-ci.yaml.

@michi-covalent if we only need to publish untrusted CI versions of the workflows, something like this works. However, if we intend to publish trusted production builds that we can validate are definitely based only on input from vetted code merged into the tree, then I think it's essential to keep those in a dedicated different repository. With a different repository we can enforce stricter constraints around how code flows into that repository, access to secrets, maintainer approval of individual builds.

@joestringer

Update the CI workflow to build arm64 builds and make any potential changes to the dockerfiles for development builds. Do not change the target repositories.

cilium-cli/.github/workflows/images.yaml

Lines 27 to 33 in 8aec284

include:

- name: cilium-cli-ci

dockerfile: ./Dockerfile

platforms: linux/amd64

- name: cilium-cli

dockerfile: ./Dockerfile

platforms: linux/amd64,linux/arm64

Specifically this is also changed so we do not change the target repository for the cli-ci builds

Also adding the new -ci suffixless version, which could be a seperate PR however the Dockerfile changes are already merged

@joestringer
Specifically your second point, there is already such a specific step in this workflow:

cilium-cli/.github/workflows/images.yaml

Lines 61 to 75 in 8aec284

# main branch or tag pushes

- name: CI Build ${{ matrix.name }}

if: ${{ github.event_name != 'pull_request_target' }}

uses: docker/build-push-action@5cd11c3a4ced054e52742c5fd54dca954e0edd85 # v6.7.0

id: docker_build_ci_main

with:

context: .

file: ${{ matrix.dockerfile }}

push: true

platforms: ${{ matrix.platforms }}

build-args: |

FINAL_CONTAINER=${{ matrix.name }}

tags: |

quay.io/${{ github.repository_owner }}/${{ matrix.name }}:latest

quay.io/${{ github.repository_owner }}/${{ matrix.name }}:${{ steps.tag.outputs.tag }}

which only builds latest and the tagged release when main/tag is pushed

The ${{ secrets.QUAY_CI_TOKEN }} secrets do not have access to publish to quay.io/cilium/cilium-cli, only quay.io/cilium/cilium-cli-ci. The Cilium supply chain threat model considers these repositories to be in different threat categories due to the way that credentials are used in different organization workflows.

Typically the way we handle this in cilium/cilium is that there is an entirely different set of secrets for official builds vs. CI, then there is a dedicated GitHub environment for the official builds that applies strict limits on how builds can be created (for example, requiring maintainer approval), and finally there are different workflows for the different builds with different triggers.

joestringer · 2024-10-04T18:06:34Z

.github/workflows/images.yaml

As for the SBOM question from the community meeting: I think that it's not so important for CI builds (see also cilium/cilium#35116), though having SBOM on CI builds can help us to find out if the SBOM steps in the workflow are working or not before we go to do a release.

For releases for CLI, I think SBOM can be valuable and should be relatively straightforward (something like https://github.com/cilium/cilium/blob/475310f713fb25897b1b939668c73db7da359d08/.github/workflows/build-images-releases.yaml#L121-L134), but let's get the repos up and running first and worry about SBOM after.

joestringer

Marking as "request changes" until we've resolved the ongoing thread about supply chain / secrets.

leppeK requested review from a team as code owners August 20, 2024 05:49

leppeK requested review from viktor-kurchenko and asauber August 20, 2024 05:49

leppeK temporarily deployed to ci August 20, 2024 05:49 — with GitHub Actions Inactive

asauber reviewed Aug 21, 2024

View reviewed changes

Dockerfile-cli-only Outdated Show resolved Hide resolved

leppeK temporarily deployed to ci August 22, 2024 08:26 — with GitHub Actions Inactive

asauber self-requested a review August 22, 2024 22:50

asauber reviewed Aug 22, 2024

View reviewed changes

.github/workflows/images.yaml Outdated Show resolved Hide resolved

asauber requested changes Aug 22, 2024

View reviewed changes

leppeK temporarily deployed to ci August 23, 2024 09:35 — with GitHub Actions Inactive

leppeK force-pushed the fix2755 branch from 5dc0d10 to 5f96dce Compare August 23, 2024 18:21

leppeK temporarily deployed to ci August 23, 2024 18:21 — with GitHub Actions Inactive

leppeK requested a review from asauber August 23, 2024 18:23

asauber approved these changes Sep 3, 2024

View reviewed changes

viktor-kurchenko approved these changes Sep 10, 2024

View reviewed changes

tklauser requested changes Sep 17, 2024

View reviewed changes

leppeK added 3 commits September 18, 2024 14:38

reference the correct file.

8aec284

@asauber pointed out i referenced a Dockerfile that does not exist anymore Fixes: cilium#2755 Signed-off-by: Merijn Keppel <[email protected]>

leppeK force-pushed the fix2755 branch from 5f96dce to 8aec284 Compare September 18, 2024 12:46

leppeK temporarily deployed to ci September 18, 2024 12:46 — with GitHub Actions Inactive

joestringer reviewed Oct 4, 2024

View reviewed changes

leppeK mentioned this pull request Nov 1, 2024

Enable multi-arch build for cilium-cli image #2782

Draft

joestringer requested changes Nov 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

publish images: amd64/arm64 images containing cli only #2770

publish images: amd64/arm64 images containing cli only #2770

leppeK commented Aug 20, 2024 •

edited

Loading

leppeK commented Aug 20, 2024

asauber left a comment

leppeK commented Sep 2, 2024

asauber left a comment

viktor-kurchenko left a comment

leppeK commented Sep 17, 2024

michi-covalent commented Sep 17, 2024

leppeK commented Sep 17, 2024

tklauser left a comment

leppeK commented Sep 17, 2024 •

edited

Loading

joestringer Oct 4, 2024 •

edited

Loading

joestringer Oct 4, 2024

leppeK Nov 1, 2024 •

edited

Loading

leppeK Nov 1, 2024

joestringer Nov 5, 2024

joestringer Oct 4, 2024 •

edited

Loading

joestringer left a comment

	include:
	- name: cilium-cli-ci
	dockerfile: ./Dockerfile
	platforms: linux/amd64
	- name: cilium-cli
	dockerfile: ./Dockerfile
	platforms: linux/amd64,linux/arm64

	# main branch or tag pushes
	- name: CI Build ${{ matrix.name }}
	if: ${{ github.event_name != 'pull_request_target' }}
	uses: docker/build-push-action@5cd11c3a4ced054e52742c5fd54dca954e0edd85 # v6.7.0
	id: docker_build_ci_main
	with:
	context: .
	file: ${{ matrix.dockerfile }}
	push: true
	platforms: ${{ matrix.platforms }}
	build-args: \|
	FINAL_CONTAINER=${{ matrix.name }}
	tags: \|
	quay.io/${{ github.repository_owner }}/${{ matrix.name }}:latest
	quay.io/${{ github.repository_owner }}/${{ matrix.name }}:${{ steps.tag.outputs.tag }}

publish images: amd64/arm64 images containing cli only #2770

Are you sure you want to change the base?

publish images: amd64/arm64 images containing cli only #2770

Conversation

leppeK commented Aug 20, 2024 • edited Loading

leppeK commented Aug 20, 2024

asauber left a comment

Choose a reason for hiding this comment

leppeK commented Sep 2, 2024

asauber left a comment

Choose a reason for hiding this comment

viktor-kurchenko left a comment

Choose a reason for hiding this comment

leppeK commented Sep 17, 2024

michi-covalent commented Sep 17, 2024

leppeK commented Sep 17, 2024

tklauser left a comment

Choose a reason for hiding this comment

leppeK commented Sep 17, 2024 • edited Loading

joestringer Oct 4, 2024 • edited Loading

Choose a reason for hiding this comment

joestringer Oct 4, 2024

Choose a reason for hiding this comment

leppeK Nov 1, 2024 • edited Loading

Choose a reason for hiding this comment

leppeK Nov 1, 2024

Choose a reason for hiding this comment

joestringer Nov 5, 2024

Choose a reason for hiding this comment

joestringer Oct 4, 2024 • edited Loading

Choose a reason for hiding this comment

joestringer left a comment

Choose a reason for hiding this comment

leppeK commented Aug 20, 2024 •

edited

Loading

leppeK commented Sep 17, 2024 •

edited

Loading

joestringer Oct 4, 2024 •

edited

Loading

leppeK Nov 1, 2024 •

edited

Loading

joestringer Oct 4, 2024 •

edited

Loading