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

v4.4.1-rhel backports #20906

Merged
merged 3 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions pkg/api/handlers/compat/images_build.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/containers/podman/v4/pkg/auth"
"github.com/containers/podman/v4/pkg/channel"
"github.com/containers/podman/v4/pkg/rootless"
"github.com/containers/podman/v4/pkg/util"
"github.com/containers/storage/pkg/archive"
"github.com/docker/docker/pkg/jsonmessage"
"github.com/gorilla/schema"
Expand Down Expand Up @@ -627,6 +628,12 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
reporter := channel.NewWriter(make(chan []byte))
defer reporter.Close()

_, ignoreFile, err := util.ParseDockerignore(containerFiles, contextDirectory)
if err != nil {
utils.Error(w, http.StatusInternalServerError, fmt.Errorf("processing ignore file: %w", err))
return
}

runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
buildOptions := buildahDefine.BuildOptions{
AddCapabilities: addCaps,
Expand Down Expand Up @@ -676,6 +683,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
From: fromImage,
IDMappingOptions: &idMappingOptions,
IgnoreUnrecognizedInstructions: query.Ignore,
IgnoreFile: ignoreFile,
Isolation: isolation,
Jobs: &jobs,
Labels: labels,
Expand Down
43 changes: 15 additions & 28 deletions pkg/bindings/images/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/containers/podman/v4/pkg/auth"
"github.com/containers/podman/v4/pkg/bindings"
"github.com/containers/podman/v4/pkg/domain/entities"
"github.com/containers/podman/v4/pkg/util"
"github.com/containers/storage/pkg/fileutils"
"github.com/containers/storage/pkg/ioutils"
"github.com/containers/storage/pkg/regexp"
Expand Down Expand Up @@ -405,14 +406,6 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
stdout = options.Out
}

excludes := options.Excludes
if len(excludes) == 0 {
excludes, err = parseDockerignore(options.ContextDirectory)
if err != nil {
return nil, err
}
}

contextDir, err = filepath.Abs(options.ContextDirectory)
if err != nil {
logrus.Errorf("Cannot find absolute path of %v: %v", options.ContextDirectory, err)
Expand Down Expand Up @@ -458,20 +451,26 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
if strings.HasPrefix(containerfile, contextDir+string(filepath.Separator)) {
containerfile = strings.TrimPrefix(containerfile, contextDir+string(filepath.Separator))
dontexcludes = append(dontexcludes, "!"+containerfile)
dontexcludes = append(dontexcludes, "!"+containerfile+".dockerignore")
dontexcludes = append(dontexcludes, "!"+containerfile+".containerignore")
} else {
// If Containerfile does not exist, assume it is in context directory and do Not add to tarfile
if _, err := os.Lstat(containerfile); err != nil {
if !os.IsNotExist(err) {
return nil, err
}
containerfile = c
dontexcludes = append(dontexcludes, "!"+containerfile)
dontexcludes = append(dontexcludes, "!"+containerfile+".dockerignore")
dontexcludes = append(dontexcludes, "!"+containerfile+".containerignore")
} else {
// If Containerfile does exist and not in the context directory, add it to the tarfile
tarContent = append(tarContent, containerfile)
}
}
newContainerFiles = append(newContainerFiles, filepath.ToSlash(containerfile))
}

if len(newContainerFiles) > 0 {
cFileJSON, err := json.Marshal(newContainerFiles)
if err != nil {
Expand All @@ -480,6 +479,14 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
params.Set("dockerfile", string(cFileJSON))
}

excludes := options.Excludes
if len(excludes) == 0 {
excludes, _, err = util.ParseDockerignore(newContainerFiles, options.ContextDirectory)
if err != nil {
return nil, err
}
}

// build secrets are usually absolute host path or relative to context dir on host
// in any case move secret to current context and ship the tar.
if secrets := options.CommonBuildOpts.Secrets; len(secrets) > 0 {
Expand Down Expand Up @@ -776,23 +783,3 @@ func nTar(excludes []string, sources ...string) (io.ReadCloser, error) {
})
return rc, nil
}

func parseDockerignore(root string) ([]string, error) {
ignore, err := os.ReadFile(filepath.Join(root, ".containerignore"))
if err != nil {
var dockerIgnoreErr error
ignore, dockerIgnoreErr = os.ReadFile(filepath.Join(root, ".dockerignore"))
if dockerIgnoreErr != nil && !os.IsNotExist(dockerIgnoreErr) {
return nil, err
}
}
rawexcludes := strings.Split(string(ignore), "\n")
excludes := make([]string, 0, len(rawexcludes))
for _, e := range rawexcludes {
if len(e) == 0 || e[0] == '#' {
continue
}
excludes = append(excludes, e)
}
return excludes, nil
}
71 changes: 71 additions & 0 deletions pkg/util/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/containers/podman/v4/pkg/signal"
"github.com/containers/storage/pkg/idtools"
stypes "github.com/containers/storage/types"
securejoin "github.com/cyphar/filepath-securejoin"
"github.com/opencontainers/runtime-spec/specs-go"
"github.com/sirupsen/logrus"
"golang.org/x/term"
Expand Down Expand Up @@ -56,6 +57,76 @@ func parseCreds(creds string) (string, string) {
return up[0], up[1]
}

// Takes build context and validates `.containerignore` or `.dockerignore`
// if they are symlink outside of buildcontext. Returns list of files to be
// excluded and resolved path to the ignore files inside build context or error
func ParseDockerignore(containerfiles []string, root string) ([]string, string, error) {
ignoreFile := ""
path, err := securejoin.SecureJoin(root, ".containerignore")
if err != nil {
return nil, ignoreFile, err
}
// set resolved ignore file so imagebuildah
// does not attempts to re-resolve it
ignoreFile = path
ignore, err := os.ReadFile(path)
if err != nil {
var dockerIgnoreErr error
path, symlinkErr := securejoin.SecureJoin(root, ".dockerignore")
if symlinkErr != nil {
return nil, ignoreFile, symlinkErr
}
// set resolved ignore file so imagebuildah
// does not attempts to re-resolve it
ignoreFile = path
ignore, dockerIgnoreErr = os.ReadFile(path)
if os.IsNotExist(dockerIgnoreErr) {
// In this case either ignorefile was not found
// or it is a symlink to unexpected file in such
// case manually set ignorefile to `/dev/null` so
// internally imagebuildah does not attempts to re-resolve
// this invalid symlink and instead reads a blank file.
ignoreFile = "/dev/null"
}
// after https://github.com/containers/buildah/pull/4239 build supports
// <Containerfile>.containerignore or <Containerfile>.dockerignore as ignore file
// so remote must support parsing that.
if dockerIgnoreErr != nil {
for _, containerfile := range containerfiles {
if _, err := os.Stat(filepath.Join(root, containerfile+".containerignore")); err == nil {
path, symlinkErr = securejoin.SecureJoin(root, containerfile+".containerignore")
if symlinkErr == nil {
ignoreFile = path
ignore, dockerIgnoreErr = os.ReadFile(path)
}
}
if _, err := os.Stat(filepath.Join(root, containerfile+".dockerignore")); err == nil {
path, symlinkErr = securejoin.SecureJoin(root, containerfile+".dockerignore")
if symlinkErr == nil {
ignoreFile = path
ignore, dockerIgnoreErr = os.ReadFile(path)
}
}
if dockerIgnoreErr == nil {
break
}
}
}
if dockerIgnoreErr != nil && !os.IsNotExist(dockerIgnoreErr) {
return nil, ignoreFile, err
}
}
rawexcludes := strings.Split(string(ignore), "\n")
excludes := make([]string, 0, len(rawexcludes))
for _, e := range rawexcludes {
if len(e) == 0 || e[0] == '#' {
continue
}
excludes = append(excludes, e)
}
return excludes, ignoreFile, nil
}

// ParseRegistryCreds takes a credentials string in the form USERNAME:PASSWORD
// and returns a DockerAuthConfig
func ParseRegistryCreds(creds string) (*types.DockerAuthConfig, error) {
Expand Down
4 changes: 3 additions & 1 deletion test/buildah-bud/apply-podman-deltas
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,9 @@ skip_if_remote "Explicit request in buildah PR 4190 to skip this on remote" \
# BEGIN tests which are skipped due to actual podman or podman-remote bugs.

skip_if_remote "different error messages between podman & podman-remote" \
"bud with .dockerignore #2"
"bud with .dockerignore #2" \
"bud with .dockerignore #4" \
"bud with .dockerignore #6"

# END tests which are skipped due to actual podman or podman-remote bugs.
###############################################################################
Expand Down
2 changes: 2 additions & 0 deletions test/e2e/build/containerignore-symlink/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
FROM alpine
COPY / /dir
Empty file.
Empty file.
39 changes: 39 additions & 0 deletions test/e2e/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,45 @@ RUN find /test`, ALPINE)
Expect(session.OutputToString()).To(ContainSubstring("/test/dummy"))
})

It("podman remote build must not allow symlink for ignore files", func() {
Copy link
Member

Choose a reason for hiding this comment

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

By a curious coincidence, I've spent the morning working on this very test. As written, this test is completely useless. If for any reason you need to repush, like to disable the failing containerized test, you might want to cherrypick #20582

// Create a random file where symlink must be resolved
// but build should not be able to access it.
privateFile := filepath.Join("/tmp", "private_file")
f, err := os.Create(privateFile)
Expect(err).ToNot(HaveOccurred())
// Mark hello to be ignored in outerfile, but it should not be ignored.
_, err = f.WriteString("hello\n")
Expect(err).ToNot(HaveOccurred())
defer f.Close()

// Create .dockerignore which is a symlink to /tmp/private_file.
currentDir, err := os.Getwd()
Expect(err).ToNot(HaveOccurred())
ignoreFile := filepath.Join(currentDir, "build/containerignore-symlink/.dockerignore")
err = os.Symlink(privateFile, ignoreFile)
Expect(err).ToNot(HaveOccurred())
// Remove created .dockerignore for this test when test ends.
defer func() {
os.Remove(ignoreFile)
}()

if IsRemote() {
podmanTest.StopRemoteService()
podmanTest.StartRemoteService()
} else {
Skip("Only valid at remote test")
}

session := podmanTest.Podman([]string{"build", "--pull-never", "-t", "test", "build/containerignore-symlink/"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))

session = podmanTest.Podman([]string{"run", "--rm", "test", "ls", "/dir"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))
Expect(session.OutputToString()).To(ContainSubstring("hello"))
})

It("podman remote test container/docker file is not at root of context dir", func() {
if IsRemote() {
podmanTest.StopRemoteService()
Expand Down
Loading