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

Fix building from http or '-' options #7121

Merged
merged 1 commit into from
Jul 29, 2020
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
52 changes: 18 additions & 34 deletions cmd/podman/images/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,36 +138,9 @@ func build(cmd *cobra.Command, args []string) error {
return errors.New("cannot specify --squash, --squash-all and --layers options together")
}

contextDir, containerFiles, err := extractContextAndFiles(args, buildOpts.File)
if err != nil {
return err
}

ie, err := registry.NewImageEngine(cmd, args)
if err != nil {
return err
}

apiBuildOpts, err := buildFlagsWrapperToOptions(cmd, contextDir, &buildOpts)
if err != nil {
return err
}

_, err = ie.Build(registry.GetContext(), containerFiles, *apiBuildOpts)
return err
}

// extractContextAndFiles parses args and files to extract a context directory
// and {Container,Docker}files.
//
// TODO: this was copied and altered from the v1 client which in turn was
// copied and altered from the Buildah code. Ideally, all of this code should
// be cleanly consolidated into a package that is shared between Buildah and
// Podman.
func extractContextAndFiles(args, files []string) (string, []string, error) {
// Extract container files from the CLI (i.e., --file/-f) first.
var containerFiles []string
for _, f := range files {
for _, f := range buildOpts.File {
if f == "-" {
containerFiles = append(containerFiles, "/dev/stdin")
} else {
Expand All @@ -181,7 +154,7 @@ func extractContextAndFiles(args, files []string) (string, []string, error) {
// The context directory could be a URL. Try to handle that.
tempDir, subDir, err := imagebuildah.TempDirForURL("", "buildah", args[0])
if err != nil {
return "", nil, errors.Wrapf(err, "error prepping temporary context directory")
return errors.Wrapf(err, "error prepping temporary context directory")
}
if tempDir != "" {
// We had to download it to a temporary directory.
Expand All @@ -196,7 +169,7 @@ func extractContextAndFiles(args, files []string) (string, []string, error) {
// Nope, it was local. Use it as is.
absDir, err := filepath.Abs(args[0])
if err != nil {
return "", nil, errors.Wrapf(err, "error determining path to directory %q", args[0])
return errors.Wrapf(err, "error determining path to directory %q", args[0])
}
contextDir = absDir
}
Expand All @@ -212,18 +185,18 @@ func extractContextAndFiles(args, files []string) (string, []string, error) {
}
absFile, err := filepath.Abs(containerFiles[i])
if err != nil {
return "", nil, errors.Wrapf(err, "error determining path to file %q", containerFiles[i])
return errors.Wrapf(err, "error determining path to file %q", containerFiles[i])
}
contextDir = filepath.Dir(absFile)
break
}
}

if contextDir == "" {
return "", nil, errors.Errorf("no context directory and no Containerfile specified")
return errors.Errorf("no context directory and no Containerfile specified")
}
if !utils.IsDir(contextDir) {
return "", nil, errors.Errorf("context must be a directory: %q", contextDir)
return errors.Errorf("context must be a directory: %q", contextDir)
}
if len(containerFiles) == 0 {
if utils.FileExists(filepath.Join(contextDir, "Containerfile")) {
Expand All @@ -233,7 +206,18 @@ func extractContextAndFiles(args, files []string) (string, []string, error) {
}
}

return contextDir, containerFiles, nil
ie, err := registry.NewImageEngine(cmd, args)
if err != nil {
return err
}

apiBuildOpts, err := buildFlagsWrapperToOptions(cmd, contextDir, &buildOpts)
if err != nil {
return err
}

_, err = ie.Build(registry.GetContext(), containerFiles, *apiBuildOpts)
return err
}

// buildFlagsWrapperToOptions converts the local build flags to the build options used
Expand Down
21 changes: 21 additions & 0 deletions test/system/070-build.bats
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,27 @@ Labels.$label_name | $label_value
run_podman rmi -f build_test
}

@test "podman build - stdin test" {
if is_remote && is_rootless; then
skip "unreliable with podman-remote and rootless; #2972"
fi

# Random workdir, and multiple random strings to verify command & env
workdir=/$(random_string 10)
PODMAN_TIMEOUT=240 run_podman build -t build_test - << EOF
FROM $IMAGE
RUN mkdir $workdir
WORKDIR $workdir
RUN /bin/echo 'Test'
EOF
is "$output" ".*STEP 5: COMMIT" "COMMIT seen in log"

run_podman run --rm build_test pwd
is "$output" "$workdir" "pwd command in container"

Copy link
Member

Choose a reason for hiding this comment

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

Why not add a podman run test? Verify that is "$output" "Test" "blah blah"? Costs nothing, and could (maybe??) catch some weird future error.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added

run_podman rmi -f build_test
}

function teardown() {
# A timeout or other error in 'build' can leave behind stale images
# that podman can't even see and which will cascade into subsequent
Expand Down