Feat: send additional build contexts as tar files for remote builds #26628

2004joshua · 2025-07-14T02:47:01Z

Fixed the --build-context flag to properly send files for remote builds. Previously only the main context was sent over as a tar while additional contexts were passed as local paths and this would cause builds to fail since the files wouldn't exist.

New changes modifies the Build API to use multipart HTTP requests allowing multiple build contexts to be sent as tar files. Each additional context is packaged and transferred based on its type:

Local Directories: Sent as tar archives
Git Repositories: URL sent to the server, which then clones it
Container Images: Image reference sent to the server, which pulls the image
URLs/archives: URL sent to the server, which handles the download

Version check is in place to make sure that the server is updated to at least the upcoming 5.6.0 update or else the fix wont be usable.

Fixes: #23433

Does this PR introduce a user-facing change?

Added support for --build-context for podman-remote

Luap99

Not a full review, but reading into memory does not seem viable to me.
Have a look at ManifestModify() and the bindings manifests Modify() functions which AFAICT to such a multipart request without reading all into memory

Git Repositories: cloned locally then sent as a tar
URLs/archives: downloaded locally then sent as a tar

It would be best if just the server downloads them instead of the client, sending them as tar via the API just sounds slower.

pkg/api/handlers/compat/images_build.go

pkg/bindings/images/build.go

packit-as-a-service · 2025-07-14T15:44:54Z

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

pkg/bindings/images/build.go

nalind

Some unnecessary duplication of archive/extracting logic on both ends, probably still have an issue where the top-level directory of an additional build context has a different timestamp between otherwise-identical build requests made at different times.

pkg/api/handlers/compat/images_build.go

pkg/bindings/images/build.go

pkg/api/handlers/compat/images_build.go

Luap99 · 2025-07-17T14:21:56Z

I think for me this goes a bit to much into the breaking chnage territory, if caching gets broken on remote then this is a bit problematic. Also the copy now makes it much slower compared to the local file thing before.

I think this PR is overall the right thing to do but I think I would prefer for this to wait for 6.0. Or maybe this is something where the work @Honny1 is doing with a local build API could help here. Where we can have the local and remote way working so users could take advantage of either one.

Honny1 · 2025-07-17T16:07:14Z

I think this PR is overall the right thing to do but I think I would prefer for this to wait for 6.0. Or maybe this is something where the work @Honny1 is doing with a local build API could help here. Where we can have the local and remote way working so users could take advantage of either one.

Yes, a local API will help with performance issues, but only on systems with a podman machine. Probably also on remote systems, but that would have some security implications.

pkg/bindings/images/build.go

test/e2e/build_test.go

Luap99 · 2025-07-17T15:59:50Z

pkg/machine/e2e/basic_test.go

@@ -249,8 +249,8 @@ var _ = Describe("run basic podman commands", func() {
 	})

 	It("podman build contexts", func() {
+		Skip("Waiting for VM images with Podman > 5.5.2 for multipart build context support")


this is all of the sudden skipping for all tests, I would think this test must still pass. IF this doesn't pass it means we break backwards compatibility.

For the qemu skip you can switch the test to say unskip when machine image contain podman with this patch.

pkg/api/handlers/compat/images_build.go

pkg/bindings/images/build.go

Honny1

After a quick review of the code, I believe it would be more straightforward if additional build contexts were handled within the query, and the files to be transferred were part of the incoming tar. This could simplify the code significantly by removing the need to parse multipart/form-data.

pkg/api/handlers/compat/images_build.go

pkg/machine/e2e/basic_test.go

Luap99 · 2025-07-21T17:04:08Z

Also if we add a new content type then the new format must be document in our swagger file in pkg/api/server/register_images.go as well.

nalind · 2025-07-22T15:53:52Z

pkg/api/handlers/compat/images_build.go

 	buildDir := filepath.Join(anchorDir, "build")
 	err := os.Mkdir(buildDir, 0o700)
 	if err != nil {
 		return "", err
 	}

-	err = archive.Untar(r.Body, buildDir, nil)
+	err = archive.Untar(r, buildDir, nil)


Any reason this isn't chrootarchive.Untar()?

That function already had it like that

Sure, but why?

pkg/api/handlers/compat/images_build.go

nalind · 2025-07-22T16:45:51Z

pkg/api/server/register_images.go

+	//      Additional build contexts for builds that require more than one context.
+	//      We send both URL and image links through query parameters where we then setup the additional build contexts
+	//      struct. Local additional build contexts are sent as tar files via multipart/form-data.
+	//      (As of version 5.6.0)


This is the compat endpoint. Do other clients submit this information this way?

nalind · 2025-07-22T16:47:53Z

pkg/api/server/register_images.go

+	//      Additional build contexts for builds that require more than one context.
+	//      We send both URL and image links through query parameters where we then setup the additional build contexts
+	//      struct. Local additional build contexts are sent as tar files via multipart/form-data.
+	//      (As of version 5.6.0)


I don't think you need to mention data types used by the implementation, which a client wouldn't care about. A client would care about how they're expected to describe contexts that are URLs or images, so it would probably be good to describe what the strings should look like here.

Fixed the --build-context flag to properly send files for remote builds. Previously only the main context was sent over as a tar while additional contexts were passed as local paths and this would cause builds to fail since the files wouldn't exist. New changes modifies the Build API to use multipart HTTP requests allowing multiple build contexts to be used. Each additional context is packaged and transferred based on its type: - Local Directories: Sent as tar archives - Git Repositories: link sent to the server where its then cloned - Container Images: Image reference sent to the server, it then pulls the image there - URLs/archives: URL sent to the server, which handles the download Fixes: containers#23433 Signed-off-by: Joshua Arrevillaga <[email protected]>

mheon · 2025-07-23T19:04:39Z

LGTM

Luap99

/lgtm

openshift-ci · 2025-07-24T11:00:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 2004joshua, Luap99

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [Luap99]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci bot added the release-note label Jul 14, 2025

github-actions bot added the kind/api-change Change to remote API; merits scrutiny label Jul 14, 2025

Luap99 reviewed Jul 14, 2025

View reviewed changes

pkg/api/handlers/compat/images_build.go Outdated Show resolved Hide resolved

pkg/bindings/images/build.go Outdated Show resolved Hide resolved

2004joshua force-pushed the build_context branch 2 times, most recently from 7ea602e to b2db9e2 Compare July 14, 2025 15:32

2004joshua force-pushed the build_context branch from b2db9e2 to 71f5764 Compare July 14, 2025 16:43

github-actions bot added the machine label Jul 14, 2025

2004joshua force-pushed the build_context branch 3 times, most recently from 5b62920 to d56b2a1 Compare July 14, 2025 20:39

flouthoc reviewed Jul 14, 2025

View reviewed changes

pkg/bindings/images/build.go Outdated Show resolved Hide resolved

nalind suggested changes Jul 16, 2025

View reviewed changes

Luap99 reviewed Jul 17, 2025

View reviewed changes

Honny1 reviewed Jul 17, 2025

View reviewed changes

2004joshua force-pushed the build_context branch 6 times, most recently from e9f7827 to 3ccd11a Compare July 21, 2025 16:35

Luap99 reviewed Jul 21, 2025

View reviewed changes

pkg/api/handlers/compat/images_build.go Outdated Show resolved Hide resolved

pkg/api/handlers/compat/images_build.go Outdated Show resolved Hide resolved

pkg/api/handlers/compat/images_build.go Outdated Show resolved Hide resolved

Luap99 reviewed Jul 21, 2025

View reviewed changes

pkg/machine/e2e/basic_test.go Outdated Show resolved Hide resolved

nalind reviewed Jul 22, 2025

View reviewed changes

2004joshua force-pushed the build_context branch from 3ccd11a to 84d6ce0 Compare July 22, 2025 15:57

nalind reviewed Jul 22, 2025

View reviewed changes

pkg/api/handlers/compat/images_build.go Outdated Show resolved Hide resolved

2004joshua force-pushed the build_context branch from 84d6ce0 to a3778e8 Compare July 22, 2025 16:02

2004joshua force-pushed the build_context branch from a3778e8 to 6c04df1 Compare July 22, 2025 16:26

nalind reviewed Jul 22, 2025

View reviewed changes

2004joshua force-pushed the build_context branch 3 times, most recently from 1c4f355 to 0467ad7 Compare July 23, 2025 01:58

2004joshua force-pushed the build_context branch from 0467ad7 to 73f3e98 Compare July 23, 2025 15:35

Luap99 approved these changes Jul 24, 2025

View reviewed changes

openshift-ci bot assigned Luap99 Jul 24, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 24, 2025

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 24, 2025

openshift-merge-bot bot merged commit 12b7a33 into containers:main Jul 24, 2025
82 of 85 checks passed

Feat: send additional build contexts as tar files for remote builds #26628

Feat: send additional build contexts as tar files for remote builds #26628

Conversation

2004joshua commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Does this PR introduce a user-facing change?

Uh oh!

Luap99 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

packit-as-a-service bot commented Jul 14, 2025

Uh oh!

Uh oh!

nalind left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Luap99 commented Jul 17, 2025

Uh oh!

Honny1 commented Jul 17, 2025

Uh oh!

Uh oh!

Uh oh!

Luap99 Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Honny1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Luap99 commented Jul 21, 2025

Uh oh!

nalind Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

2004joshua Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

nalind Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nalind Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

nalind Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

mheon commented Jul 23, 2025

Uh oh!

Luap99 left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Jul 24, 2025

Uh oh!

Uh oh!

Uh oh!

2004joshua commented Jul 14, 2025 •

edited

Loading