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.

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

startup-script: Adjust ca-certificates install to only copy certs #283

Conversation

learnitall
Copy link
Contributor

@learnitall learnitall commented Jun 27, 2024

This PR contains two commits, one for adjusting the way we install ca-certificates in the startup-script image, and one to enable image builds in PRs from forks. Please see individual commits for details.

I opened a draft PR in cilium/cilium to test the changes to this image, see cilium/cilium#33414. The helm-based checks are failing because I didn't update the generated documentation. All other checks that actually use the new startup-script image are passing.

This commit adjusts the method in which `ca-certificates` is installed
into the startup-script image. The `ca-certificates` package has a
dependency on OpenSSL, which unnecessarily increases the attack surface
of the image, since the image's only functionality is fully contained in
`manage-startup-script.sh` and the script makes no network connections.
The package could theoretically be removed, but as a precaution, the
root certificates from the `ca-certificates` package are copied in to
the image using the same method as is used in the Cilium Operator image.

Signed-off-by: Ryan Drew <[email protected]>
@learnitall learnitall force-pushed the pr/learnitall/startup-script-ca-certificates branch from a3156f2 to eb5e8f8 Compare June 27, 2024 20:39
This commit adds two missing environment variables that allow for
anonymous authentication to remote registries when pushing build results
is disabled. Pulls and remote digest checks are read-only operations
that do not require credentials, however however, if credentials are not
set during a non-push build, some steps may return an error that looks
like this:

```
sha256:0e59c8168a48aa9ed0573be2d38d40a95e1b1635877d6e3dd56dfd33e3597e56
pulling image moby/buildkit:buildx-stable-1 done
ERROR: error getting credentials - err: exit status 1, out: `ANY_REGISTRY_USERNAME is not set`
```

The above example was triggered while building the `image-maker` image
in the call to `docker buildx build`. This bug prevents PRs from forks
from running the "Build all images" workflow to test their changes, making
it more difficult for members of the community who don't have write access
to this repository to open PRs.

Signed-off-by: Ryan Drew <[email protected]>
@learnitall learnitall force-pushed the pr/learnitall/startup-script-ca-certificates branch from eb5e8f8 to 2577810 Compare June 27, 2024 20:46
@learnitall learnitall marked this pull request as ready for review June 27, 2024 20:49
@learnitall learnitall requested a review from a team as a code owner June 27, 2024 20:49
@learnitall learnitall requested a review from borkmann June 27, 2024 20:49
Copy link
Contributor

@michi-covalent michi-covalent left a comment

Choose a reason for hiding this comment

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

let's do it

@borkmann borkmann merged commit 8f8cd0b into cilium:master Jul 3, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants