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

Skip("Waiting for VM images with Podman > 5.5.2 for multipart build context support") is in place until VM images are updated to podman 5.6.0

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

openshift-ci · 2025-07-14T02:47:07Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 2004joshua
Once this PR has been reviewed and has the lgtm label, please assign mheon for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

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.

Luap99 · 2025-07-14T10:08:35Z

pkg/api/handlers/compat/images_build.go

+	const maxMemory = 256 << 20
+
+	if err := r.ParseMultipartForm(maxMemory); err != nil {
+		return "", nil, fmt.Errorf("failed to parse multipart form: %w", err)
+	}


This read everything into memory which does not seem good and the limit is likely to low for some contexts

Luap99 · 2025-07-14T10:09:14Z

pkg/bindings/images/build.go

+		var body bytes.Buffer
+		writer := multipart.NewWriter(&body)


These contexts can be very large, much bigger than the available memory so reading everything into memory seems unacceptable to me

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.

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]>

flouthoc · 2025-07-14T21:02:32Z

pkg/bindings/images/build.go

+							if err != nil {
+								return err
+							}
+
+							header, err := tar.FileInfoHeader(info, "")
+							if err != nil {
+								return err
+							}
+
+							relPath, err := filepath.Rel(context.Value, path)
+							if err != nil {
+								return err
+							}
+							if relPath == "." {
+								relPath = filepath.Base(context.Value)
+							}
+							header.Name = relPath
+
+							if err := tarWriter.WriteHeader(header); err != nil {
+								return err


I think all the return err here should we wrapped with helper err message ?

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.

nalind · 2025-07-16T17:01:00Z

pkg/api/handlers/compat/images_build.go

+				return "", nil, fmt.Errorf("creating context directory: %w", err)
+			}
+
+			if err := archive.Untar(part, contextPath, nil); err != nil {


Should consider using extractTarFile() here, since it's what handles the default build context archive when the request isn't a multipart form.

nalind · 2025-07-16T17:03:12Z

pkg/api/handlers/compat/images_build.go

 		switch contentType {
 		case "application/tar":
 			logrus.Infof("tar file content type is  %s, should use \"application/x-tar\" content type", contentType)
 		case "application/x-tar":
 			break
+		case "multipart/form-data":
+			logrus.Debugf("Received multipart/form-data: %s", contentType)


Isn't this always going to log "Received multipart/form-data: multipart/form-data"?

nalind · 2025-07-16T17:08:22Z

pkg/api/handlers/compat/images_build.go

+			}
+			logrus.Debugf("Added image context %q with reference %q", contextName, imageRef)
+
+		case strings.HasPrefix(fieldName, "buildcontext-local-") && fileName != "":


Is fileName used at all?

nalind · 2025-07-16T17:13:40Z

pkg/api/handlers/compat/images_build.go

+
+		case strings.HasPrefix(fieldName, "buildcontext-local-") && fileName != "":
+			contextName := strings.TrimPrefix(fieldName, "buildcontext-local-")
+			additionalAnchor := filepath.Join(anchorDir, "additional", contextName)


Rule of thumb: when possible, avoid using values supplied by the client when constructing path names. os.MkdirTemp() would be fine here.

nalind · 2025-07-16T17:18:41Z

pkg/bindings/images/build.go

+				case context.IsImage:
+					metadataField := fmt.Sprintf("buildcontext-image-%s", name)
+					if err := writer.WriteField(metadataField, context.Value); err != nil {
+						pipeWriter.CloseWithError(fmt.Errorf("adding URL context %q: %w", name, err))


Suggested change

pipeWriter.CloseWithError(fmt.Errorf("adding URL context %q: %w", name, err))

pipeWriter.CloseWithError(fmt.Errorf("adding image context %q: %w", name, err))

nalind · 2025-07-16T17:25:14Z

pkg/bindings/images/build.go

+							return
+						}
+					} else {
+						tarWriter := tar.NewWriter(part)


Why isn't this calling nTar() instead? This is missing logic for handling symbolic and hard links, and compressing the archive.

nalind · 2025-07-16T17:28:08Z

pkg/bindings/images/build.go

+					}
+
+					// local context is already a tar
+					if strings.HasSuffix(context.Value, ".tar") {


Expecting the file name to correctly indicate things can lead to errors. Use github.com/containers/storage/pkg/archive.IsArchivePath().

nalind · 2025-07-16T17:29:11Z

pkg/bindings/images/build.go

+							if relPath == "." {
+								relPath = filepath.Base(context.Value)
+							}
+							header.Name = relPath


If my additional build context path is "/tmp/build-context", this adds an entry for a directory named "build-context" to the archive? Why?

nalind · 2025-07-16T17:33:55Z

pkg/api/handlers/compat/images_build.go

+				return "", nil, fmt.Errorf("creating additional context directory %q: %w", contextName, err)
+			}
+
+			if err := archive.Untar(part, additionalAnchor, nil); err != nil {


The archive content's coming from a client. Why isn't it extracted using chrootarchive.Untar(), or extractTarFile()?

nalind · 2025-07-16T17:37:17Z

pkg/api/handlers/compat/images_build.go

+		logrus.Debugf("Processing part: field=%q, file=%q", fieldName, fileName)
+
+		switch {
+		case fieldName == "context" && fileName != "":


Is fileName used at all?

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

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 2 times, most recently from d9d6872 to 5b62920 Compare July 14, 2025 19:40

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

flouthoc reviewed Jul 14, 2025

View reviewed changes

nalind suggested changes Jul 16, 2025

View reviewed changes

	pipeWriter.CloseWithError(fmt.Errorf("adding URL context %q: %w", name, err))
	pipeWriter.CloseWithError(fmt.Errorf("adding image context %q: %w", name, err))

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

Are you sure you want to change the base?

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!

openshift-ci bot commented Jul 14, 2025

Uh oh!

Luap99 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

flouthoc Jul 14, 2025 • 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!

nalind left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

2004joshua commented Jul 14, 2025 •

edited

Loading

flouthoc Jul 14, 2025 •

edited

Loading