Rewrite container handling internals to drop graphdriver and gpgme dependencies [RHEL-56367] by achilleas-k · Pull Request #2607 · osbuild/image-builder

achilleas-k · 2026-08-20T18:38:33Z

This PR rewrites container-related things in order to drop our dependency on graphdrivers and gpgme. It finishes the work I started in #2346, where I changed the container resolver internals to use skopeo.

Changes in this PR, in order:

Rewrite testregistry to use github.com/distribution/distribution. This simplifies the implementation of the internal container test registry significantly, offloading all the work to distribution/distribution. It also gives us a more fully-featured test registry, so we can test pushing containers as well.
Test osbuild-upload-container with the new test registry. The upload code wasn't being tested at all before, so any testing here is an improvement.
Rewrite osbuild-upload-container to use skopeo copy internally.

With these rewrites, we rely on skopeo for almost all of our interactions with container registries, which means we don't depend on any of the filesystem libraries or gpgme.

supakeen · 2026-08-25T14:43:44Z


-	var manifestDigest digest.Digest
+	if dockerAuth := cl.sysCtx.DockerAuthConfig; dockerAuth != nil {
+		cmd.Args = append(cmd.Args, fmt.Sprintf("--dest-creds=%s:%s", dockerAuth.Username, dockerAuth.Password))


This leaks the credentials to any user running ps during a build. Is there no other way of passing these?

I had the same thought but the only reason we have the user:pass option is because the osbuild-upload-container supports --username and --password and in that case, the original call leaks them to begin with. Maybe we should get rid of that entirely and only support auth-file.

Let's turn this into an issue here.

This seems to also cause a leak in the osbuild-composer workers when uploading container images. They call (*Client).UploadImage() at osbuild-composer/cmd/osbuild-worker/jobimpl-osbuild.go:1319 which results in the same argv leak under discussion here.

Given that this is a new leak affecting the library API we may want to reconsider deferring to the future. I think it would make sense to update this pull request to only support auth-file.

Oh we do use it! Ok then, I'll take care of it in this PR.

lucasgarfield

Overall looks great!

Only thing I caught is that the skopeo argv leak is a bit worse than we thought. I'd request that we address that somehow (if we choose to defer, that's fine, but we should be intentional about it) before approval.

I would also be curious to hear why we decided to go with distribution/distribution for the registry. Did you consider google/go-containerregistry/pkg/registry? I have no strong opinion and can see good arguments for both, just curious about you decided. distribution/distribution has more features (which I'm not sure we're taking advantage of... yet?) but also more dependencies.

achilleas-k · 2026-09-01T14:00:11Z

I would also be curious to hear why we decided to go with distribution/distribution for the registry. Did you consider google/go-containerregistry/pkg/registry? I have no strong opinion and can see good arguments for both, just curious about you decided. distribution/distribution has more features (which I'm not sure we're taking advantage of... yet?) but also more dependencies.

I didn't spend too much time looking into the two but it seemed like it was less work to get a minimal registry running with distribution/distribution. Looking a bit more deeply though, it looks like I was wrong and go-containerregistry is closer to what we want.

I'll play around with it a bit more.

achilleas-k · 2026-09-01T16:14:26Z

Ok, moving everything to go-containerregistry is nicer actually. And it's a minimal change from what we have now. Pushing in a minute.

Rewrite the resolver tests to use the new test registry functionality.

Return the image IDs that were created by the PopulateWithManifestList() function alongside the digests for the new images.

Rewrite the container client tests to use the new test registry functionality.

Rewrite the blocking resolver tests to use the new test registry functionality.

Use the new testregistry to upload a container using osbuild-upload-container and verify that it was uploaded successfully by resolving the container using our own resolver. The main() function of the osbuild-upload-resolver is split into an upload() function that does the main work so it can be testable. The main() function now only handles command line flags and then calls into the new function.

Test uploading a container with credentials (username and password).

Rewrite the container UploadImage() function to shell out to skopeo copy. It takes care of all the options we currently support by converting them to command line options. The functionality is tested by the new osbuild-upload-container tests.

Drop the parseImageName() function and the policy property of the client. They're not needed anymore.

On RHEL and CentOS, the exclude_graphdriver_btrfs tag was needed to compile without support for the btrfs driver from the containers/image dependency. That dependency is now gone and we don't need to specify the tag anymore. The same goes for the other graphdrivers: devicemapper and overlay. Even though we could support those on the two distros, we sometimes specified them to avoid needing the devel package dependency installed.

The containers/image pgp dependency is also gone.

Remove btrfs-progs-devel, device-mapper-devel, and gpgme-devel from the dependency install script.

Remove mentions of btrfs-progs-devel and device-mapper-devel. Also remove mentions of cmd/build. This README should be replaced by the Image Builder readme soon. See HMS-10586

Remove all mentions of: - gpgme-devel - btrfs-progs-devel - device-mapper-devel

Specifically, skip tests that fail in containers because of the way they access the host container storage. These weren't an issue on CentOS before, but now appear to be failing the same way they do in Fedora containers.

We use systemd-escape to generate mount unit names. On installations, this is pulled in by osbuild itself, but tests will fail at build time without it.

achilleas-k · 2026-09-01T16:28:55Z

To make reviewing easier, I pushed a rebase on main first and then did a second force push with the changes addressing the reviews.

And another two to fix issues caught by the linter.

Instead of putting credentials in the arguments of the 'skopeo copy' call (with --dest-creds) when uploading a container, write a temporary auth file and use --authfile. This prevents leaking credentials through the shell call, which is visible to any user on the system.

Now that we have an auth file creation function, let's use it instead of depending on 'skopeo login' to generate it.

lucasgarfield

Looks good! Leak is fixed and I'm happy you ended up liking go-containerregistry/pkg/registry... the main thing that made me interested in it is that I'm now always on the lookout to minimize transitive dependencies. 😄

achilleas-k requested review from croissanne and supakeen August 20, 2026 18:38

achilleas-k requested review from a team and thozza as code owners August 20, 2026 18:38

achilleas-k force-pushed the drop-dep/containers-image/part2 branch 7 times, most recently from 5bf4948 to 5236abc Compare August 20, 2026 19:05

This comment was marked as outdated.

Sign in to view

achilleas-k force-pushed the drop-dep/containers-image/part2 branch 6 times, most recently from 31d3292 to e7e82a8 Compare August 21, 2026 11:55

This comment was marked as outdated.

Sign in to view

supakeen reviewed Aug 25, 2026

View reviewed changes

achilleas-k force-pushed the drop-dep/containers-image/part2 branch from e7e82a8 to bb681b7 Compare August 25, 2026 18:48

achilleas-k mentioned this pull request Aug 26, 2026

skopeo copy leaks the credentials to any user running ps during a build #2624

Closed

supakeen previously approved these changes Aug 26, 2026

View reviewed changes

achilleas-k requested a review from avitova August 31, 2026 12:47

lucasgarfield requested changes Sep 1, 2026

View reviewed changes

achilleas-k dismissed supakeen’s stale review via eb74836 September 1, 2026 16:15

achilleas-k force-pushed the drop-dep/containers-image/part2 branch from bb681b7 to eb74836 Compare September 1, 2026 16:15

achilleas-k added 16 commits September 1, 2026 18:24

container: rewrite resolver tests to use new registry

47147b9

Rewrite the resolver tests to use the new test registry functionality.

testregistry: return image IDs alongside digests

418349d

Return the image IDs that were created by the PopulateWithManifestList() function alongside the digests for the new images.

container: rewrite container client tests to use new registry

97e415e

Rewrite the container client tests to use the new test registry functionality.

container: rewrite blocking resolver tests to use new registry

33f8144

Rewrite the blocking resolver tests to use the new test registry functionality.

osbuild-upload-container: test with credentials

f1b15c0

Test uploading a container with credentials (username and password).

container: remove unused client code and properties

1b0f487

Drop the parseImageName() function and the policy property of the client. They're not needed anymore.

Remove all uses of the containers_image_openpgp compile tag

563bc0b

The containers/image pgp dependency is also gone.

test: drop dependencies from install script

1b3b35f

Remove btrfs-progs-devel, device-mapper-devel, and gpgme-devel from the dependency install script.

README: update dependency descriptions

03602e5

Remove mentions of btrfs-progs-devel and device-mapper-devel. Also remove mentions of cmd/build. This README should be replaced by the Image Builder readme soon. See HMS-10586

Remove all mentions of the dropped dependencies

44dad55

Remove all mentions of: - gpgme-devel - btrfs-progs-devel - device-mapper-devel

github: install git for CentOS Stream unit tests

5220a72

test: run unit tests on CentOS the same way we do on Fedora

afb07ef

Specifically, skip tests that fail in containers because of the way they access the host container storage. These weren't an issue on CentOS before, but now appear to be failing the same way they do in Fedora containers.

spec: add systemd to BuildRequires

917207e

We use systemd-escape to generate mount unit names. On installations, this is pulled in by osbuild itself, but tests will fail at build time without it.

achilleas-k force-pushed the drop-dep/containers-image/part2 branch from be9d96a to ac9961c Compare September 1, 2026 16:28

achilleas-k requested review from lucasgarfield and supakeen September 1, 2026 16:30

achilleas-k added 2 commits September 2, 2026 11:05

osbuild-upload-container: use the new function in the uploader test

73c0291

Now that we have an auth file creation function, let's use it instead of depending on 'skopeo login' to generate it.

achilleas-k force-pushed the drop-dep/containers-image/part2 branch from ac9961c to 73c0291 Compare September 2, 2026 09:05

lucasgarfield approved these changes Sep 2, 2026

View reviewed changes

supakeen approved these changes Sep 2, 2026

View reviewed changes

supakeen enabled auto-merge September 2, 2026 18:47

supakeen added this pull request to the merge queue Sep 2, 2026

Merged via the queue into osbuild:main with commit 100fbe5 Sep 2, 2026
55 of 63 checks passed

achilleas-k deleted the drop-dep/containers-image/part2 branch September 3, 2026 12:37

Uh oh!

Conversation

achilleas-k commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

supakeen Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

achilleas-k Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

supakeen Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

achilleas-k Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

lucasgarfield Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

achilleas-k Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lucasgarfield left a comment

Choose a reason for hiding this comment

Uh oh!

achilleas-k commented Sep 1, 2026

Uh oh!

achilleas-k commented Sep 1, 2026

Uh oh!

achilleas-k commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lucasgarfield left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

achilleas-k commented Aug 20, 2026 •

edited

Loading

achilleas-k Sep 1, 2026 •

edited

Loading

achilleas-k commented Sep 1, 2026 •

edited

Loading

lucasgarfield left a comment •

edited

Loading