Skip to content

Conversation

HadrienPatte
Copy link
Member

@HadrienPatte HadrienPatte commented Jul 15, 2025

Docker build checks are currently only being flagged on PRs by the docker/build-push-action action. With this PR, image builds will fail whenever there are docker build check failures.

See docker build checks.

There are two main ways to enable enforcement of docker build checks:

  • With a special # check=error=true comment at the top of each Dockerfile
  • With a special BUILDKIT_DOCKERFILE_CHECK=error=true build-arg in each docker build command

In general it's easier to add the build arg in the central place where image builds are configured, that way all future Dockerfiles are garanteed to be covered. However in the case of the cilium/cilium repository, it turns out that image builds are defined in a lot of places: in docker/build-push-action (10 individual occurences) workflows, but also in Makefile.docker and images/scripts/build-image.sh. As a result of this, it is more difficult to ensure continued coverage of all image build mechanisms, both those used locally and those used in CI. This is why I opted to add the check=error=true comment in Dockerfiles instead.

We can see an example of how a CI image build job now fails on check violations here:

 5 warnings found (use docker --debug to expand):
 - FromAsCasing: 'as' and 'FROM' keywords' casing do not match (line 93)
 - FromAsCasing: 'as' and 'FROM' keywords' casing do not match (line 100)
 - LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 58)
 - LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 59)
 - LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 60)
ERROR: failed to build: failed to solve: lint violation found for rules: FromAsCasing, LegacyKeyValueFormat

As part of this PR, the images/builder/Dockerfile, contrib/backporting/Dockerfile and contrib/coccinelle/Dockerfile Dockerfiles are getting updated to fix existing check failures.


Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Are you a user of Cilium? Please add yourself to the Users doc
  • Thanks for contributing!
build: Enforce docker build checks

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jul 15, 2025
@HadrienPatte HadrienPatte force-pushed the pr/HadrienPatte/docker-build-check branch 2 times, most recently from 23ff55d to 24f3e2c Compare July 15, 2025 12:02
@HadrienPatte HadrienPatte force-pushed the pr/HadrienPatte/docker-build-check branch from 24f3e2c to 56c5853 Compare July 15, 2025 13:11
@auto-committer auto-committer bot temporarily deployed to release-base-images July 15, 2025 13:34 Inactive
@HadrienPatte HadrienPatte marked this pull request as ready for review July 15, 2025 14:10
@HadrienPatte HadrienPatte requested review from a team as code owners July 15, 2025 14:10
@joestringer joestringer added the release-note/misc This PR makes changes that have no direct user impact. label Jul 15, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jul 15, 2025
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@joestringer joestringer removed the request for review from hemanthmalla July 15, 2025 18:00
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

Thanks

@joestringer
Copy link
Member

/test

@joestringer joestringer enabled auto-merge July 17, 2025 17:41
Docker build checks are currently only being flagged on PRs by the
`docker/build-push-action` action. With this PR, image builds will fail
whenever there are docker build check failures.

See [docker build checks](https://docs.docker.com/build/checks/).

Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>
@HadrienPatte HadrienPatte force-pushed the pr/HadrienPatte/docker-build-check branch from 4a2854e to af05964 Compare July 30, 2025 13:25
Signed-off-by: Cilium Imagebot <noreply@cilium.io>
@auto-committer auto-committer bot temporarily deployed to release-base-images July 30, 2025 13:57 Inactive
@HadrienPatte
Copy link
Member Author

@cilium/docs-structure can someone please approve the docs-builder deployment?

@HadrienPatte
Copy link
Member Author

@qmonnet Thanks! It looks like it needs an additionnal approval for the Update Pull Request with new image reference step (and likely a final approval on the noop workflow run for the new commit that this workflow will push to this branch 😅)

Signed-off-by: Cilium Imagebot <noreply@cilium.io>
@auto-committer auto-committer bot temporarily deployed to release-base-images July 30, 2025 14:54 Inactive
@qmonnet
Copy link
Member

qmonnet commented Jul 30, 2025

@qmonnet Thanks! It looks like it needs an additionnal approval for the Update Pull Request with new image reference step (and likely a final approval on the noop workflow run for the new commit that this workflow will push to this branch 😅)

Yes I meant to come back here after the first deployment had run, and got distracted while working on something else 🙂

@HadrienPatte
Copy link
Member Author

/test

@joestringer joestringer added this pull request to the merge queue Jul 30, 2025
Merged via the queue into main with commit 5a60ca4 Jul 30, 2025
373 of 375 checks passed
@joestringer joestringer deleted the pr/HadrienPatte/docker-build-check branch July 30, 2025 16:19
@maintainer-s-little-helper maintainer-s-little-helper bot added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Jul 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants