From e2ae92971db356b68b961622cfa639d9040fad4f Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Fri, 17 Oct 2025 13:33:28 -0400 Subject: [PATCH 1/2] Move define.TempDirForURL() to pkg/tmpdir.ForURL() Move the generic "download a thing" function to a package that wasn't originally intended to only include type definitions, but leave a wrapper in the old location for compatibility's sake. Signed-off-by: Nalin Dahyabhai --- define/types.go | 199 +---------------- imagebuildah/stage_executor.go | 5 +- pkg/cli/build.go | 3 +- pkg/tmpdir/url.go | 206 ++++++++++++++++++ .../types_test.go => pkg/tmpdir/url_test.go | 2 +- 5 files changed, 218 insertions(+), 197 deletions(-) create mode 100644 pkg/tmpdir/url.go rename define/types_test.go => pkg/tmpdir/url_test.go (98%) diff --git a/define/types.go b/define/types.go index a1f3946f4d1..83660b7e40c 100644 --- a/define/types.go +++ b/define/types.go @@ -1,26 +1,11 @@ package define import ( - "bufio" - "bytes" - "errors" - "fmt" - "io" - "net/http" - urlpkg "net/url" - "os" - "os/exec" - "path" - "path/filepath" - "strings" - + "github.com/containers/buildah/pkg/tmpdir" v1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/opencontainers/runtime-spec/specs-go" - "github.com/sirupsen/logrus" "go.podman.io/image/v5/manifest" "go.podman.io/storage/pkg/archive" - "go.podman.io/storage/pkg/chrootarchive" - "go.podman.io/storage/pkg/ioutils" "go.podman.io/storage/types" ) @@ -173,182 +158,10 @@ type SBOMScanOptions struct { // is, TempDirForURL creates a temporary directory, arranges for its contents // to be the contents of that URL, and returns the temporary directory's path, // along with the relative name of a subdirectory which should be used as the -// build context (which may be empty or "."). Removal of the temporary -// directory is the responsibility of the caller. If the string doesn't look -// like a URL or "-", TempDirForURL returns empty strings and a nil error code. +// build context (which may be empty or "."). +// Removal of the temporary directory is the responsibility of the caller. +// If the string doesn't look like a URL or "-", TempDirForURL returns empty +// strings and a nil error code. func TempDirForURL(dir, prefix, url string) (name string, subdir string, err error) { - if !strings.HasPrefix(url, "http://") && - !strings.HasPrefix(url, "https://") && - !strings.HasPrefix(url, "git://") && - !strings.HasPrefix(url, "github.com/") && - url != "-" { - return "", "", nil - } - name, err = os.MkdirTemp(dir, prefix) - if err != nil { - return "", "", fmt.Errorf("creating temporary directory for %q: %w", url, err) - } - downloadDir := filepath.Join(name, "download") - if err = os.MkdirAll(downloadDir, 0o700); err != nil { - return "", "", fmt.Errorf("creating directory %q for %q: %w", downloadDir, url, err) - } - urlParsed, err := urlpkg.Parse(url) - if err != nil { - return "", "", fmt.Errorf("parsing url %q: %w", url, err) - } - if strings.HasPrefix(url, "git://") || strings.HasSuffix(urlParsed.Path, ".git") { - combinedOutput, gitSubDir, err := cloneToDirectory(url, downloadDir) - if err != nil { - if err2 := os.RemoveAll(name); err2 != nil { - logrus.Debugf("error removing temporary directory %q: %v", name, err2) - } - return "", "", fmt.Errorf("cloning %q to %q:\n%s: %w", url, name, string(combinedOutput), err) - } - logrus.Debugf("Build context is at %q", filepath.Join(downloadDir, gitSubDir)) - return name, filepath.Join(filepath.Base(downloadDir), gitSubDir), nil - } - if strings.HasPrefix(url, "github.com/") { - ghurl := url - url = fmt.Sprintf("https://%s/archive/master.tar.gz", ghurl) - logrus.Debugf("resolving url %q to %q", ghurl, url) - subdir = path.Base(ghurl) + "-master" - } - if strings.HasPrefix(url, "http://") || strings.HasPrefix(url, "https://") { - err = downloadToDirectory(url, downloadDir) - if err != nil { - if err2 := os.RemoveAll(name); err2 != nil { - logrus.Debugf("error removing temporary directory %q: %v", name, err2) - } - return "", "", err - } - logrus.Debugf("Build context is at %q", filepath.Join(downloadDir, subdir)) - return name, filepath.Join(filepath.Base(downloadDir), subdir), nil - } - if url == "-" { - err = stdinToDirectory(downloadDir) - if err != nil { - if err2 := os.RemoveAll(name); err2 != nil { - logrus.Debugf("error removing temporary directory %q: %v", name, err2) - } - return "", "", err - } - logrus.Debugf("Build context is at %q", filepath.Join(downloadDir, subdir)) - return name, filepath.Join(filepath.Base(downloadDir), subdir), nil - } - logrus.Debugf("don't know how to retrieve %q", url) - if err2 := os.RemoveAll(name); err2 != nil { - logrus.Debugf("error removing temporary directory %q: %v", name, err2) - } - return "", "", errors.New("unreachable code reached") -} - -// parseGitBuildContext parses git build context to `repo`, `sub-dir` -// `branch/commit`, accepts GitBuildContext in the format of -// `repourl.git[#[branch-or-commit]:subdir]`. -func parseGitBuildContext(url string) (string, string, string) { - gitSubdir := "" - gitBranch := "" - gitBranchPart := strings.Split(url, "#") - if len(gitBranchPart) > 1 { - // check if string contains path to a subdir - gitSubDirPart := strings.Split(gitBranchPart[1], ":") - if len(gitSubDirPart) > 1 { - gitSubdir = gitSubDirPart[1] - } - gitBranch = gitSubDirPart[0] - } - return gitBranchPart[0], gitSubdir, gitBranch -} - -func cloneToDirectory(url, dir string) ([]byte, string, error) { - var cmd *exec.Cmd - gitRepo, gitSubdir, gitRef := parseGitBuildContext(url) - // init repo - cmd = exec.Command("git", "init", dir) - combinedOutput, err := cmd.CombinedOutput() - if err != nil { - // Return err.Error() instead of err as we want buildah to override error code with more predictable - // value. - return combinedOutput, gitSubdir, fmt.Errorf("failed while performing `git init`: %s", err.Error()) - } - // add origin - cmd = exec.Command("git", "remote", "add", "origin", gitRepo) - cmd.Dir = dir - combinedOutput, err = cmd.CombinedOutput() - if err != nil { - // Return err.Error() instead of err as we want buildah to override error code with more predictable - // value. - return combinedOutput, gitSubdir, fmt.Errorf("failed while performing `git remote add`: %s", err.Error()) - } - - logrus.Debugf("fetching repo %q and branch (or commit ID) %q to %q", gitRepo, gitRef, dir) - args := []string{"fetch", "-u", "--depth=1", "origin", "--", gitRef} - cmd = exec.Command("git", args...) - cmd.Dir = dir - combinedOutput, err = cmd.CombinedOutput() - if err != nil { - // Return err.Error() instead of err as we want buildah to override error code with more predictable - // value. - return combinedOutput, gitSubdir, fmt.Errorf("failed while performing `git fetch`: %s", err.Error()) - } - - cmd = exec.Command("git", "checkout", "FETCH_HEAD") - cmd.Dir = dir - combinedOutput, err = cmd.CombinedOutput() - if err != nil { - // Return err.Error() instead of err as we want buildah to override error code with more predictable - // value. - return combinedOutput, gitSubdir, fmt.Errorf("failed while performing `git checkout`: %s", err.Error()) - } - return combinedOutput, gitSubdir, nil -} - -func downloadToDirectory(url, dir string) error { - logrus.Debugf("extracting %q to %q", url, dir) - resp, err := http.Get(url) - if err != nil { - return err - } - defer resp.Body.Close() - if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusBadRequest { - return fmt.Errorf("invalid response status %d", resp.StatusCode) - } - if resp.ContentLength == 0 { - return fmt.Errorf("no contents in %q", url) - } - if err := chrootarchive.Untar(resp.Body, dir, nil); err != nil { - resp1, err := http.Get(url) - if err != nil { - return err - } - defer resp1.Body.Close() - body, err := io.ReadAll(resp1.Body) - if err != nil { - return err - } - dockerfile := filepath.Join(dir, "Dockerfile") - // Assume this is a Dockerfile - if err := ioutils.AtomicWriteFile(dockerfile, body, 0o600); err != nil { - return fmt.Errorf("failed to write %q to %q: %w", url, dockerfile, err) - } - } - return nil -} - -func stdinToDirectory(dir string) error { - logrus.Debugf("extracting stdin to %q", dir) - r := bufio.NewReader(os.Stdin) - b, err := io.ReadAll(r) - if err != nil { - return fmt.Errorf("failed to read from stdin: %w", err) - } - reader := bytes.NewReader(b) - if err := chrootarchive.Untar(reader, dir, nil); err != nil { - dockerfile := filepath.Join(dir, "Dockerfile") - // Assume this is a Dockerfile - if err := ioutils.AtomicWriteFile(dockerfile, b, 0o600); err != nil { - return fmt.Errorf("failed to write bytes to %q: %w", dockerfile, err) - } - } - return nil + return tmpdir.ForURL(dir, prefix, url) } diff --git a/imagebuildah/stage_executor.go b/imagebuildah/stage_executor.go index 408b778f008..e23a775a338 100644 --- a/imagebuildah/stage_executor.go +++ b/imagebuildah/stage_executor.go @@ -24,6 +24,7 @@ import ( internalUtil "github.com/containers/buildah/internal/util" "github.com/containers/buildah/pkg/parse" "github.com/containers/buildah/pkg/rusage" + tmpdirpkg "github.com/containers/buildah/pkg/tmpdir" "github.com/containers/buildah/util" docker "github.com/fsouza/go-dockerclient" buildkitparser "github.com/moby/buildkit/frontend/dockerfile/parser" @@ -488,7 +489,7 @@ func (s *stageExecutor) performCopy(excludes []string, copies ...imagebuilder.Co // additional context contains a tar file // so download and explode tar to buildah // temp and point context to that. - path, subdir, err := define.TempDirForURL(tmpdir.GetTempDir(), internal.BuildahExternalArtifactsDir, additionalBuildContext.Value) + path, subdir, err := tmpdirpkg.ForURL(tmpdir.GetTempDir(), internal.BuildahExternalArtifactsDir, additionalBuildContext.Value) if err != nil { return fmt.Errorf("unable to download context from external source %q: %w", additionalBuildContext.Value, err) } @@ -681,7 +682,7 @@ func (s *stageExecutor) runStageMountPoints(mountList []string) (map[string]inte // additional context contains a tar file // so download and explode tar to buildah // temp and point context to that. - path, subdir, err := define.TempDirForURL(tmpdir.GetTempDir(), internal.BuildahExternalArtifactsDir, additionalBuildContext.Value) + path, subdir, err := tmpdirpkg.ForURL(tmpdir.GetTempDir(), internal.BuildahExternalArtifactsDir, additionalBuildContext.Value) if err != nil { return nil, fmt.Errorf("unable to download context from external source %q: %w", additionalBuildContext.Value, err) } diff --git a/pkg/cli/build.go b/pkg/cli/build.go index a07f2b6ff9e..1ffab89e9e8 100644 --- a/pkg/cli/build.go +++ b/pkg/cli/build.go @@ -19,6 +19,7 @@ import ( "github.com/containers/buildah/define" "github.com/containers/buildah/pkg/parse" + "github.com/containers/buildah/pkg/tmpdir" "github.com/containers/buildah/pkg/util" "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" @@ -145,7 +146,7 @@ func GenBuildOptions(c *cobra.Command, inputArgs []string, iopts BuildOptions) ( } } else { // The context directory could be a URL. Try to handle that. - tempDir, subDir, err := define.TempDirForURL("", "buildah", cliArgs[0]) + tempDir, subDir, err := tmpdir.ForURL("", "buildah", cliArgs[0]) if err != nil { return options, nil, nil, fmt.Errorf("prepping temporary context directory: %w", err) } diff --git a/pkg/tmpdir/url.go b/pkg/tmpdir/url.go new file mode 100644 index 00000000000..a75b21339fb --- /dev/null +++ b/pkg/tmpdir/url.go @@ -0,0 +1,206 @@ +package tmpdir + +import ( + "bufio" + "bytes" + "errors" + "fmt" + "io" + "net/http" + urlpkg "net/url" + "os" + "os/exec" + "path" + "path/filepath" + "strings" + + "github.com/sirupsen/logrus" + "go.podman.io/storage/pkg/chrootarchive" + "go.podman.io/storage/pkg/ioutils" +) + +// ForURL checks if the passed-in string looks like a URL or "-". If it is, +// ForURL creates a temporary directory, arranges for the contents of one of +// its subdirectories to be the contents of that URL, and returns the temporary +// directory's path, along with the relative name of a subdirectory (which may +// be empty or "."). When the URL string specifies a build context, the +// subdirectory should be used as the build context. +// Removal of the temporary directory is the responsibility of the caller. +// If the string doesn't look like a URL or "-", ForURL returns empty strings +// and a nil error code. +func ForURL(dir, prefix, url string) (name string, subdir string, err error) { + if !strings.HasPrefix(url, "http://") && + !strings.HasPrefix(url, "https://") && + !strings.HasPrefix(url, "git://") && + !strings.HasPrefix(url, "github.com/") && + url != "-" { + return "", "", nil + } + name, err = os.MkdirTemp(dir, prefix) + if err != nil { + return "", "", fmt.Errorf("creating temporary directory for %q: %w", url, err) + } + downloadDir := filepath.Join(name, "download") + if err = os.MkdirAll(downloadDir, 0o700); err != nil { + return "", "", fmt.Errorf("creating directory %q for %q: %w", downloadDir, url, err) + } + urlParsed, err := urlpkg.Parse(url) + if err != nil { + return "", "", fmt.Errorf("parsing url %q: %w", url, err) + } + if strings.HasPrefix(url, "git://") || strings.HasSuffix(urlParsed.Path, ".git") { + combinedOutput, gitSubDir, err := cloneToDirectory(url, downloadDir) + if err != nil { + if err2 := os.RemoveAll(name); err2 != nil { + logrus.Debugf("error removing temporary directory %q: %v", name, err2) + } + return "", "", fmt.Errorf("cloning %q to %q:\n%s: %w", url, name, string(combinedOutput), err) + } + logrus.Debugf("Build context is at %q", filepath.Join(downloadDir, gitSubDir)) + return name, filepath.Join(filepath.Base(downloadDir), gitSubDir), nil + } + if strings.HasPrefix(url, "github.com/") { + ghurl := url + url = fmt.Sprintf("https://%s/archive/master.tar.gz", ghurl) + logrus.Debugf("resolving url %q to %q", ghurl, url) + subdir = path.Base(ghurl) + "-master" + } + if strings.HasPrefix(url, "http://") || strings.HasPrefix(url, "https://") { + err = downloadToDirectory(url, downloadDir) + if err != nil { + if err2 := os.RemoveAll(name); err2 != nil { + logrus.Debugf("error removing temporary directory %q: %v", name, err2) + } + return "", "", err + } + logrus.Debugf("Build context is at %q", filepath.Join(downloadDir, subdir)) + return name, filepath.Join(filepath.Base(downloadDir), subdir), nil + } + if url == "-" { + err = stdinToDirectory(downloadDir) + if err != nil { + if err2 := os.RemoveAll(name); err2 != nil { + logrus.Debugf("error removing temporary directory %q: %v", name, err2) + } + return "", "", err + } + logrus.Debugf("Build context is at %q", filepath.Join(downloadDir, subdir)) + return name, filepath.Join(filepath.Base(downloadDir), subdir), nil + } + logrus.Debugf("don't know how to retrieve %q", url) + if err2 := os.RemoveAll(name); err2 != nil { + logrus.Debugf("error removing temporary directory %q: %v", name, err2) + } + return "", "", errors.New("unreachable code reached") +} + +// parseGitBuildContext parses git build context to `repo`, `sub-dir` +// `branch/commit`, accepts GitBuildContext in the format of +// `repourl.git[#[branch-or-commit]:subdir]`. +func parseGitBuildContext(url string) (string, string, string) { + gitSubdir := "" + gitBranch := "" + gitBranchPart := strings.Split(url, "#") + if len(gitBranchPart) > 1 { + // check if string contains path to a subdir + gitSubDirPart := strings.Split(gitBranchPart[1], ":") + if len(gitSubDirPart) > 1 { + gitSubdir = gitSubDirPart[1] + } + gitBranch = gitSubDirPart[0] + } + return gitBranchPart[0], gitSubdir, gitBranch +} + +func cloneToDirectory(url, dir string) ([]byte, string, error) { + var cmd *exec.Cmd + gitRepo, gitSubdir, gitRef := parseGitBuildContext(url) + // init repo + cmd = exec.Command("git", "init", dir) + combinedOutput, err := cmd.CombinedOutput() + if err != nil { + // Return err.Error() instead of err as we want buildah to override error code with more predictable + // value. + return combinedOutput, gitSubdir, fmt.Errorf("failed while performing `git init`: %s", err.Error()) + } + // add origin + cmd = exec.Command("git", "remote", "add", "origin", gitRepo) + cmd.Dir = dir + combinedOutput, err = cmd.CombinedOutput() + if err != nil { + // Return err.Error() instead of err as we want buildah to override error code with more predictable + // value. + return combinedOutput, gitSubdir, fmt.Errorf("failed while performing `git remote add`: %s", err.Error()) + } + + logrus.Debugf("fetching repo %q and branch (or commit ID) %q to %q", gitRepo, gitRef, dir) + args := []string{"fetch", "-u", "--depth=1", "origin", "--", gitRef} + cmd = exec.Command("git", args...) + cmd.Dir = dir + combinedOutput, err = cmd.CombinedOutput() + if err != nil { + // Return err.Error() instead of err as we want buildah to override error code with more predictable + // value. + return combinedOutput, gitSubdir, fmt.Errorf("failed while performing `git fetch`: %s", err.Error()) + } + + cmd = exec.Command("git", "checkout", "FETCH_HEAD") + cmd.Dir = dir + combinedOutput, err = cmd.CombinedOutput() + if err != nil { + // Return err.Error() instead of err as we want buildah to override error code with more predictable + // value. + return combinedOutput, gitSubdir, fmt.Errorf("failed while performing `git checkout`: %s", err.Error()) + } + return combinedOutput, gitSubdir, nil +} + +func downloadToDirectory(url, dir string) error { + logrus.Debugf("extracting %q to %q", url, dir) + resp, err := http.Get(url) + if err != nil { + return err + } + defer resp.Body.Close() + if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusBadRequest { + return fmt.Errorf("invalid response status %d", resp.StatusCode) + } + if resp.ContentLength == 0 { + return fmt.Errorf("no contents in %q", url) + } + if err := chrootarchive.Untar(resp.Body, dir, nil); err != nil { + resp1, err := http.Get(url) + if err != nil { + return err + } + defer resp1.Body.Close() + body, err := io.ReadAll(resp1.Body) + if err != nil { + return err + } + dockerfile := filepath.Join(dir, "Dockerfile") + // Assume this is a Dockerfile + if err := ioutils.AtomicWriteFile(dockerfile, body, 0o600); err != nil { + return fmt.Errorf("failed to write %q to %q: %w", url, dockerfile, err) + } + } + return nil +} + +func stdinToDirectory(dir string) error { + logrus.Debugf("extracting stdin to %q", dir) + r := bufio.NewReader(os.Stdin) + b, err := io.ReadAll(r) + if err != nil { + return fmt.Errorf("failed to read from stdin: %w", err) + } + reader := bytes.NewReader(b) + if err := chrootarchive.Untar(reader, dir, nil); err != nil { + dockerfile := filepath.Join(dir, "Dockerfile") + // Assume this is a Dockerfile + if err := ioutils.AtomicWriteFile(dockerfile, b, 0o600); err != nil { + return fmt.Errorf("failed to write bytes to %q: %w", dockerfile, err) + } + } + return nil +} diff --git a/define/types_test.go b/pkg/tmpdir/url_test.go similarity index 98% rename from define/types_test.go rename to pkg/tmpdir/url_test.go index 7318e0935b8..c9e2f6ff850 100644 --- a/define/types_test.go +++ b/pkg/tmpdir/url_test.go @@ -1,4 +1,4 @@ -package define +package tmpdir import ( "testing" From dd781381fb351099eec05bf2321fae3d164a407d Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Fri, 17 Oct 2025 14:58:24 -0400 Subject: [PATCH 2/2] Unify the tmpdir.ForURL() and Add() http client configurations Make sure that tmpdir.ForURL() and Add() use the same HTTP client configurations, both for TLS and proxies, so that we apply the same settings when fetching build contexts that we do for ADD instructions. Signed-off-by: Nalin Dahyabhai --- add.go | 44 +++++++++++++------------------ cmd/buildah/addcopy.go | 2 ++ define/build.go | 5 ++++ define/types.go | 4 +-- docs/buildah-build.1.md | 2 +- imagebuildah/executor.go | 4 +++ imagebuildah/stage_executor.go | 20 ++++++++++++-- internal/httpclient/httpclient.go | 43 ++++++++++++++++++++++++++++++ pkg/cli/build.go | 13 ++++++++- pkg/tmpdir/url.go | 19 ++++++++++--- tests/bud.bats | 19 +++++++++++++ 11 files changed, 140 insertions(+), 35 deletions(-) create mode 100644 internal/httpclient/httpclient.go diff --git a/add.go b/add.go index 88049015360..ac02de6529a 100644 --- a/add.go +++ b/add.go @@ -3,7 +3,6 @@ package buildah import ( "archive/tar" "context" - "crypto/tls" "errors" "fmt" "io" @@ -21,9 +20,10 @@ import ( "github.com/containers/buildah/copier" "github.com/containers/buildah/define" + "github.com/containers/buildah/internal/httpclient" "github.com/containers/buildah/internal/tmpdir" "github.com/containers/buildah/pkg/chrootuser" - "github.com/docker/go-connections/tlsconfig" + tmpdirpkg "github.com/containers/buildah/pkg/tmpdir" "github.com/hashicorp/go-multierror" "github.com/moby/sys/userns" digest "github.com/opencontainers/go-digest" @@ -31,7 +31,6 @@ import ( "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" "go.podman.io/common/pkg/retry" - "go.podman.io/image/v5/pkg/tlsclientconfig" "go.podman.io/image/v5/types" "go.podman.io/storage/pkg/fileutils" "go.podman.io/storage/pkg/idtools" @@ -112,6 +111,9 @@ type AddAndCopyOptions struct { // inheritAnnotations, newAnnotations). This field is internally managed and should // not be set by external API users. BuildMetadata string + // Callback which controls which, if any, proxy server to use when retrieving HTTP or + // HTTPS sources. Used to construct an http.Client's Transport. + Proxy func(*http.Request) (*url.URL, error) } // gitURLFragmentSuffix matches fragments to use as Git reference and build @@ -138,30 +140,12 @@ func sourceIsRemote(source string) bool { } // getURL writes a tar archive containing the named content -func getURL(src string, chown *idtools.IDPair, mountpoint, renameTarget string, writer io.Writer, chmod *os.FileMode, srcDigest digest.Digest, certPath string, insecureSkipTLSVerify types.OptionalBool, timestamp *time.Time) error { +func getURL(src string, chown *idtools.IDPair, mountpoint, renameTarget string, writer io.Writer, chmod *os.FileMode, srcDigest digest.Digest, timestamp *time.Time, client *http.Client) error { url, err := url.Parse(src) if err != nil { return err } - tlsClientConfig := &tls.Config{ - // As of 2025-08, tlsconfig.ClientDefault() differs from Go 1.23 defaults only in CipherSuites; - // so, limit us to only using that value. If go-connections/tlsconfig changes its policy, we - // will want to consider that and make a decision whether to follow suit. - // There is some chance that eventually the Go default will be to require TLS 1.3, and that point - // we might want to drop the dependency on go-connections entirely. - CipherSuites: tlsconfig.ClientDefault().CipherSuites, - } - if err := tlsclientconfig.SetupCertificates(certPath, tlsClientConfig); err != nil { - return err - } - tlsClientConfig.InsecureSkipVerify = insecureSkipTLSVerify == types.OptionalBoolTrue - - tr := &http.Transport{ - TLSClientConfig: tlsClientConfig, - Proxy: http.ProxyFromEnvironment, - } - httpClient := &http.Client{Transport: tr} - response, err := httpClient.Get(src) + response, err := client.Get(src) if err != nil { return err } @@ -584,6 +568,16 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption putDir = extractDirectory } + urlOptions := tmpdirpkg.URLOptions{ + CertPath: options.CertPath, + InsecureSkipTLSVerify: options.InsecureSkipTLSVerify, + Proxy: options.Proxy, + } + httpClient, err := httpclient.ForURLOptions(urlOptions) + if err != nil { + return fmt.Errorf("setting up http client options: %w", err) + } + // Copy each source in turn. for _, src := range sources { var multiErr *multierror.Error @@ -605,7 +599,7 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption defer wg.Done() defer pipeWriter.Close() var cloneDir, subdir string - cloneDir, subdir, getErr = define.TempDirForURL(tmpdir.GetTempDir(), "", src) + cloneDir, subdir, getErr = tmpdirpkg.ForURL(tmpdir.GetTempDir(), "", src, &urlOptions) if getErr != nil { return } @@ -630,7 +624,7 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption } else { go func() { getErr = retry.IfNecessary(context.TODO(), func() error { - return getURL(src, chownFiles, mountPoint, renameTarget, pipeWriter, chmodDirsFiles, srcDigest, options.CertPath, options.InsecureSkipTLSVerify, options.Timestamp) + return getURL(src, chownFiles, mountPoint, renameTarget, pipeWriter, chmodDirsFiles, srcDigest, options.Timestamp, httpClient) }, &retry.Options{ MaxRetry: options.MaxRetries, Delay: options.RetryDelay, diff --git a/cmd/buildah/addcopy.go b/cmd/buildah/addcopy.go index 1cd433896de..95ee97acf3f 100644 --- a/cmd/buildah/addcopy.go +++ b/cmd/buildah/addcopy.go @@ -3,6 +3,7 @@ package main import ( "errors" "fmt" + "net/http" "os" "path/filepath" "strconv" @@ -266,6 +267,7 @@ func addAndCopyCmd(c *cobra.Command, args []string, verb string, iopts addCopyRe Parents: iopts.parents, Timestamp: timestamp, Link: iopts.link, + Proxy: http.ProxyFromEnvironment, } if iopts.contextdir != "" { var excludes []string diff --git a/define/build.go b/define/build.go index b63a7cedf96..9680b712149 100644 --- a/define/build.go +++ b/define/build.go @@ -2,6 +2,8 @@ package define import ( "io" + "net/http" + "net/url" "time" encconfig "github.com/containers/ocicrypt/config" @@ -421,4 +423,7 @@ type BuildOptions struct { // MetadataFile is the name of a file to which the builder should write a JSON map // containing metadata about the built image. MetadataFile string + // Proxy controls how we retrieve HTTP or HTTPS build contexts and + // sources to ADD. + Proxy func(req *http.Request) (*url.URL, error) } diff --git a/define/types.go b/define/types.go index 83660b7e40c..496c397552d 100644 --- a/define/types.go +++ b/define/types.go @@ -162,6 +162,6 @@ type SBOMScanOptions struct { // Removal of the temporary directory is the responsibility of the caller. // If the string doesn't look like a URL or "-", TempDirForURL returns empty // strings and a nil error code. -func TempDirForURL(dir, prefix, url string) (name string, subdir string, err error) { - return tmpdir.ForURL(dir, prefix, url) +func TempDirForURL(dir, prefix, url string) (name, subdir string, err error) { + return tmpdir.ForURL(dir, prefix, url, nil) } diff --git a/docs/buildah-build.1.md b/docs/buildah-build.1.md index c5ed8f964eb..f0928aaf5b4 100644 --- a/docs/buildah-build.1.md +++ b/docs/buildah-build.1.md @@ -1102,7 +1102,7 @@ not affect the timestamps of layer contents. **--tls-verify** *bool-value* -Require HTTPS and verification of certificates when talking to container registries (defaults to true) and retrieving content from HTTPS locations for ADD instructions. TLS verification cannot be used when talking to an insecure registry. +Require HTTPS and verification of certificates when talking to container registries (defaults to true) and retrieving build contexts and content from HTTPS locations for ADD instructions. TLS verification cannot be used when talking to an insecure registry. **--ulimit** *type*=*soft-limit*[:*hard-limit*] diff --git a/imagebuildah/executor.go b/imagebuildah/executor.go index 444ddd039c9..b9a5ce41b53 100644 --- a/imagebuildah/executor.go +++ b/imagebuildah/executor.go @@ -6,6 +6,8 @@ import ( "errors" "fmt" "io" + "net/http" + "net/url" "os" "slices" "strconv" @@ -175,6 +177,7 @@ type executor struct { rewriteTimestamp bool createdAnnotation types.OptionalBool metadataFile string + proxy func(req *http.Request) (*url.URL, error) } type imageTypeAndHistoryAndDiffIDs struct { @@ -350,6 +353,7 @@ func newExecutor(logger *logrus.Logger, logPrefix string, store storage.Store, o rewriteTimestamp: options.RewriteTimestamp, createdAnnotation: options.CreatedAnnotation, metadataFile: options.MetadataFile, + proxy: options.Proxy, } // sort unsetAnnotations because we will later write these // values to the history of the image therefore we want to diff --git a/imagebuildah/stage_executor.go b/imagebuildah/stage_executor.go index e23a775a338..fbd2c92622c 100644 --- a/imagebuildah/stage_executor.go +++ b/imagebuildah/stage_executor.go @@ -19,6 +19,7 @@ import ( "github.com/containers/buildah/define" buildahdocker "github.com/containers/buildah/docker" "github.com/containers/buildah/internal" + "github.com/containers/buildah/internal/httpclient" "github.com/containers/buildah/internal/metadata" "github.com/containers/buildah/internal/tmpdir" internalUtil "github.com/containers/buildah/internal/util" @@ -379,6 +380,13 @@ func (s *stageExecutor) Copy(excludes []string, copies ...imagebuilder.Copy) err } func (s *stageExecutor) performCopy(excludes []string, copies ...imagebuilder.Copy) error { + urlOptions := &httpclient.URLOptions{ + // These next two fields are set based on command line flags + // with more generic-sounding names. + CertPath: s.systemContext.DockerCertPath, + InsecureSkipTLSVerify: s.systemContext.DockerInsecureSkipTLSVerify, + Proxy: s.executor.proxy, + } copiesExtend := []imagebuilder.Copy{} for _, copy := range copies { if err := s.volumeCacheInvalidate(copy.Dest); err != nil { @@ -489,7 +497,7 @@ func (s *stageExecutor) performCopy(excludes []string, copies ...imagebuilder.Co // additional context contains a tar file // so download and explode tar to buildah // temp and point context to that. - path, subdir, err := tmpdirpkg.ForURL(tmpdir.GetTempDir(), internal.BuildahExternalArtifactsDir, additionalBuildContext.Value) + path, subdir, err := tmpdirpkg.ForURL(tmpdir.GetTempDir(), internal.BuildahExternalArtifactsDir, additionalBuildContext.Value, urlOptions) if err != nil { return fmt.Errorf("unable to download context from external source %q: %w", additionalBuildContext.Value, err) } @@ -594,6 +602,7 @@ func (s *stageExecutor) performCopy(excludes []string, copies ...imagebuilder.Co Parents: copy.Parents, Link: s.hasLink, BuildMetadata: labelsAndAnnotations, + Proxy: s.executor.proxy, } if len(copy.Files) > 0 { // If we are copying heredoc files, we need to temporary place @@ -621,6 +630,13 @@ func (s *stageExecutor) performCopy(excludes []string, copies ...imagebuilder.Co // Returns a map of StageName/ImageName:internal.StageMountDetails for the // items in the passed-in mounts list which include a "from=" value. func (s *stageExecutor) runStageMountPoints(mountList []string) (map[string]internal.StageMountDetails, error) { + urlOptions := &httpclient.URLOptions{ + // These next two fields are set based on command line flags + // with more generic-sounding names. + CertPath: s.systemContext.DockerCertPath, + InsecureSkipTLSVerify: s.systemContext.DockerInsecureSkipTLSVerify, + Proxy: s.executor.proxy, + } stageMountPoints := make(map[string]internal.StageMountDetails) for _, flag := range mountList { if strings.Contains(flag, "from") { @@ -682,7 +698,7 @@ func (s *stageExecutor) runStageMountPoints(mountList []string) (map[string]inte // additional context contains a tar file // so download and explode tar to buildah // temp and point context to that. - path, subdir, err := tmpdirpkg.ForURL(tmpdir.GetTempDir(), internal.BuildahExternalArtifactsDir, additionalBuildContext.Value) + path, subdir, err := tmpdirpkg.ForURL(tmpdir.GetTempDir(), internal.BuildahExternalArtifactsDir, additionalBuildContext.Value, urlOptions) if err != nil { return nil, fmt.Errorf("unable to download context from external source %q: %w", additionalBuildContext.Value, err) } diff --git a/internal/httpclient/httpclient.go b/internal/httpclient/httpclient.go new file mode 100644 index 00000000000..b40ac53459a --- /dev/null +++ b/internal/httpclient/httpclient.go @@ -0,0 +1,43 @@ +package httpclient + +import ( + "crypto/tls" + "net/http" + "net/url" + + "github.com/docker/go-connections/tlsconfig" + "go.podman.io/image/v5/pkg/tlsclientconfig" + "go.podman.io/image/v5/types" +) + +// URLOptions provides client-side options used by ForURLOptions(). +type URLOptions struct { + CertPath string // location of CA certificates, if not the system default + InsecureSkipTLSVerify types.OptionalBool + Proxy func(*http.Request) (*url.URL, error) +} + +// ForURLOptions returns an http.Client with the settings from `options` +// applied to its mostly-default transport, configured to use proxy settings +// from the environment. +func ForURLOptions(options URLOptions) (*http.Client, error) { + tlsClientConfig := &tls.Config{ + // As of 2025-08, tlsconfig.ClientDefault() differs from Go 1.23 defaults only in CipherSuites; + // so, limit us to only using that value. If go-connections/tlsconfig changes its policy, we + // will want to consider that and make a decision whether to follow suit. + // There is some chance that eventually the Go default will be to require TLS 1.3, and that point + // we might want to drop the dependency on go-connections entirely. + CipherSuites: tlsconfig.ClientDefault().CipherSuites, + } + if err := tlsclientconfig.SetupCertificates(options.CertPath, tlsClientConfig); err != nil { + return nil, err + } + tlsClientConfig.InsecureSkipVerify = options.InsecureSkipTLSVerify == types.OptionalBoolTrue + + tr := &http.Transport{ + TLSClientConfig: tlsClientConfig, + Proxy: options.Proxy, + } + httpClient := &http.Client{Transport: tr} + return httpClient, nil +} diff --git a/pkg/cli/build.go b/pkg/cli/build.go index 1ffab89e9e8..c77f9d0ec69 100644 --- a/pkg/cli/build.go +++ b/pkg/cli/build.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "io" + "net/http" "os" "path/filepath" "slices" @@ -146,7 +147,16 @@ func GenBuildOptions(c *cobra.Command, inputArgs []string, iopts BuildOptions) ( } } else { // The context directory could be a URL. Try to handle that. - tempDir, subDir, err := tmpdir.ForURL("", "buildah", cliArgs[0]) + urlOptions := tmpdir.URLOptions{ + Proxy: http.ProxyFromEnvironment, + } + if c.Flag("cert-dir").Changed { + urlOptions.CertPath = iopts.CertDir + } + if c.Flag("tls-verify").Changed { + urlOptions.InsecureSkipTLSVerify = types.NewOptionalBool(!iopts.TLSVerify) + } + tempDir, subDir, err := tmpdir.ForURL("", "buildah", cliArgs[0], &urlOptions) if err != nil { return options, nil, nil, fmt.Errorf("prepping temporary context directory: %w", err) } @@ -431,6 +441,7 @@ func GenBuildOptions(c *cobra.Command, inputArgs []string, iopts BuildOptions) ( Output: output, OutputFormat: format, Platforms: platforms, + Proxy: http.ProxyFromEnvironment, PullPolicy: pullPolicy, Quiet: iopts.Quiet, RemoveIntermediateCtrs: iopts.Rm, diff --git a/pkg/tmpdir/url.go b/pkg/tmpdir/url.go index a75b21339fb..57f300c7bed 100644 --- a/pkg/tmpdir/url.go +++ b/pkg/tmpdir/url.go @@ -14,11 +14,14 @@ import ( "path/filepath" "strings" + "github.com/containers/buildah/internal/httpclient" "github.com/sirupsen/logrus" "go.podman.io/storage/pkg/chrootarchive" "go.podman.io/storage/pkg/ioutils" ) +type URLOptions = httpclient.URLOptions + // ForURL checks if the passed-in string looks like a URL or "-". If it is, // ForURL creates a temporary directory, arranges for the contents of one of // its subdirectories to be the contents of that URL, and returns the temporary @@ -28,7 +31,10 @@ import ( // Removal of the temporary directory is the responsibility of the caller. // If the string doesn't look like a URL or "-", ForURL returns empty strings // and a nil error code. -func ForURL(dir, prefix, url string) (name string, subdir string, err error) { +func ForURL(dir, prefix, url string, options *URLOptions) (name, subdir string, err error) { + if options == nil { + options = &URLOptions{} + } if !strings.HasPrefix(url, "http://") && !strings.HasPrefix(url, "https://") && !strings.HasPrefix(url, "git://") && @@ -66,7 +72,7 @@ func ForURL(dir, prefix, url string) (name string, subdir string, err error) { subdir = path.Base(ghurl) + "-master" } if strings.HasPrefix(url, "http://") || strings.HasPrefix(url, "https://") { - err = downloadToDirectory(url, downloadDir) + err = downloadToDirectory(*options, url, downloadDir) if err != nil { if err2 := os.RemoveAll(name); err2 != nil { logrus.Debugf("error removing temporary directory %q: %v", name, err2) @@ -155,9 +161,14 @@ func cloneToDirectory(url, dir string) ([]byte, string, error) { return combinedOutput, gitSubdir, nil } -func downloadToDirectory(url, dir string) error { +func downloadToDirectory(options URLOptions, url, dir string) error { logrus.Debugf("extracting %q to %q", url, dir) - resp, err := http.Get(url) + + httpClient, err := httpclient.ForURLOptions(options) + if err != nil { + return err + } + resp, err := httpClient.Get(url) if err != nil { return err } diff --git a/tests/bud.bats b/tests/bud.bats index d0353813f78..a847e4475fb 100644 --- a/tests/bud.bats +++ b/tests/bud.bats @@ -3393,6 +3393,25 @@ function validate_instance_compression { _test_http from-scratch Containerfile } +@test "bud-insecure-https-context" { + _prefetch busybox + target=target + cat > ${TEST_SCRATCH_DIR}/Containerfile <<- EOF + FROM busybox + COPY Containerfile / +EOF + mkdir -p ${TEST_SCRATCH_DIR}/subdir + tar -cv -C ${TEST_SCRATCH_DIR} -f ${TEST_SCRATCH_DIR}/subdir/context.tar.gz Containerfile + starthttpd "${TEST_SCRATCH_DIR}" "" "${TEST_SCRATCH_DIR}"/localhost.crt "${TEST_SCRATCH_DIR}"/localhost.key + run_buildah 125 build $WITH_POLICY_JSON -t ${target} https://0.0.0.0:${HTTP_SERVER_PORT}/subdir/context.tar.gz + assert "$output" =~ "tls: failed to verify certificate" + run_buildah 125 build $WITH_POLICY_JSON --tls-verify=true -t ${target} https://0.0.0.0:${HTTP_SERVER_PORT}/subdir/context.tar.gz + assert "$output" =~ "tls: failed to verify certificate" + run_buildah build $WITH_POLICY_JSON --tls-verify=false -t ${target} https://0.0.0.0:${HTTP_SERVER_PORT}/subdir/context.tar.gz + stophttpd + run_buildah from ${target} +} + @test "bud-http-context-with-Dockerfile" { _test_http http-context context.tar }