Refactor API build endpoint to be more compliant

* Refactor/Rename channel.WriteCloser() to encapsulate the channel
* Refactor build endpoint to "live" stream buildah output channels
  over API rather then buffering output
* Refactor bindings/tunnel build because endpoint changes
  * building tar file now in bindings rather then depending on
    caller
* Cleanup initiating extra image engine
* Remove setting fields to zero values (less noise in code)
* Update tests to support remote builds

Fixes #7136
Fixes #7137

Signed-off-by: Jhon Honce <jhonce@redhat.com>

1 files changed, 148 insertions, 110 deletions

diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go
index 9601f5e18..fbaf8d10a 100644
--- a/pkg/api/handlers/compat/images_build.go
+++ b/pkg/api/handlers/compat/images_build.go
@@ -1,7 +1,7 @@
 package compat
 
 import (
-	"bytes"
+	"context"
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
@@ -18,8 +18,10 @@ import (
 	"github.com/containers/podman/v2/libpod"
 	"github.com/containers/podman/v2/pkg/api/handlers"
 	"github.com/containers/podman/v2/pkg/api/handlers/utils"
+	"github.com/containers/podman/v2/pkg/channel"
 	"github.com/containers/storage/pkg/archive"
 	"github.com/gorilla/schema"
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
 
@@ -47,12 +49,25 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	anchorDir, err := extractTarFile(r)
+	contextDirectory, err := extractTarFile(r)
 	if err != nil {
 		utils.InternalServerError(w, err)
 		return
 	}
-	defer os.RemoveAll(anchorDir)
+
+	defer func() {
+		if logrus.IsLevelEnabled(logrus.DebugLevel) {
+			if v, found := os.LookupEnv("PODMAN_RETAIN_BUILD_ARTIFACT"); found {
+				if keep, _ := strconv.ParseBool(v); keep {
+					return
+				}
+			}
+		}
+		err := os.RemoveAll(filepath.Dir(contextDirectory))
+		if err != nil {
+			logrus.Warn(errors.Wrapf(err, "failed to remove build scratch directory %q", filepath.Dir(contextDirectory)))
+		}
+	}()
 
 	query := struct {
 		Dockerfile  string   `schema:"dockerfile"`
@@ -67,10 +82,10 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 		ForceRm     bool     `schema:"forcerm"`
 		Memory      int64    `schema:"memory"`
 		MemSwap     int64    `schema:"memswap"`
-		CpuShares   uint64   `schema:"cpushares"`  //nolint
-		CpuSetCpus  string   `schema:"cpusetcpus"` //nolint
-		CpuPeriod   uint64   `schema:"cpuperiod"`  //nolint
-		CpuQuota    int64    `schema:"cpuquota"`   //nolint
+		CpuShares   uint64   `schema:"cpushares"`  // nolint
+		CpuSetCpus  string   `schema:"cpusetcpus"` // nolint
+		CpuPeriod   uint64   `schema:"cpuperiod"`  // nolint
+		CpuQuota    int64    `schema:"cpuquota"`   // nolint
 		BuildArgs   string   `schema:"buildargs"`
 		ShmSize     int      `schema:"shmsize"`
 		Squash      bool     `schema:"squash"`
@@ -81,52 +96,32 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 		Outputs     string   `schema:"outputs"`
 		Registry    string   `schema:"registry"`
 	}{
-		Dockerfile:  "Dockerfile",
-		Tag:         []string{},
-		ExtraHosts:  "",
-		Remote:      "",
-		Quiet:       false,
-		NoCache:     false,
-		CacheFrom:   "",
-		Pull:        false,
-		Rm:          true,
-		ForceRm:     false,
-		Memory:      0,
-		MemSwap:     0,
-		CpuShares:   0,
-		CpuSetCpus:  "",
-		CpuPeriod:   0,
-		CpuQuota:    0,
-		BuildArgs:   "",
-		ShmSize:     64 * 1024 * 1024,
-		Squash:      false,
-		Labels:      "",
-		NetworkMode: "",
-		Platform:    "",
-		Target:      "",
-		Outputs:     "",
-		Registry:    "docker.io",
+		Dockerfile: "Dockerfile",
+		Tag:        []string{},
+		Rm:         true,
+		ShmSize:    64 * 1024 * 1024,
+		Registry:   "docker.io",
 	}
+
 	decoder := r.Context().Value("decoder").(*schema.Decoder)
 	if err := decoder.Decode(&query, r.URL.Query()); err != nil {
 		utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, err)
 		return
 	}
 
-	var (
-		output          string
-		additionalNames []string
-	)
+	var output string
 	if len(query.Tag) > 0 {
 		output = query.Tag[0]
 	}
+	if _, found := r.URL.Query()["target"]; found {
+		output = query.Target
+	}
+
+	var additionalNames []string
 	if len(query.Tag) > 1 {
 		additionalNames = query.Tag[1:]
 	}
 
-	if _, found := r.URL.Query()["target"]; found {
-		output = query.Target
-	}
 	var buildArgs = map[string]string{}
 	if _, found := r.URL.Query()["buildargs"]; found {
 		if err := json.Unmarshal([]byte(query.BuildArgs), &buildArgs); err != nil {
@@ -156,95 +151,136 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	// build events will be recorded here
-	var (
-		buildEvents = []string{}
-		progress    = bytes.Buffer{}
-	)
+	// Channels all mux'ed in select{} below to follow API build protocol
+	stdout := channel.NewWriter(make(chan []byte, 1))
+	defer stdout.Close()
+
+	auxout := channel.NewWriter(make(chan []byte, 1))
+	defer auxout.Close()
+
+	stderr := channel.NewWriter(make(chan []byte, 1))
+	defer stderr.Close()
+
+	reporter := channel.NewWriter(make(chan []byte, 1))
+	defer reporter.Close()
 
 	buildOptions := imagebuildah.BuildOptions{
-		ContextDirectory:               filepath.Join(anchorDir, "build"),
+		ContextDirectory:               contextDirectory,
 		PullPolicy:                     pullPolicy,
 		Registry:                       query.Registry,
 		IgnoreUnrecognizedInstructions: true,
 		Quiet:                          query.Quiet,
 		Isolation:                      buildah.IsolationChroot,
-		Runtime:                        "",
-		RuntimeArgs:                    nil,
-		TransientMounts:                nil,
 		Compression:                    archive.Gzip,
 		Args:                           buildArgs,
 		Output:                         output,
 		AdditionalTags:                 additionalNames,
-		Log: func(format string, args ...interface{}) {
-			buildEvents = append(buildEvents, fmt.Sprintf(format, args...))
-		},
-		In:                  nil,
-		Out:                 &progress,
-		Err:                 &progress,
-		SignaturePolicyPath: "",
-		ReportWriter:        &progress,
-		OutputFormat:        buildah.Dockerv2ImageManifest,
-		SystemContext:       nil,
-		NamespaceOptions:    nil,
-		ConfigureNetwork:    0,
-		CNIPluginPath:       "",
-		CNIConfigDir:        "",
-		IDMappingOptions:    nil,
-		AddCapabilities:     nil,
-		DropCapabilities:    nil,
+		Out:                            stdout,
+		Err:                            auxout,
+		ReportWriter:                   reporter,
+		OutputFormat:                   buildah.Dockerv2ImageManifest,
 		CommonBuildOpts: &buildah.CommonBuildOptions{
-			AddHost:            nil,
-			CgroupParent:       "",
-			CPUPeriod:          query.CpuPeriod,
-			CPUQuota:           query.CpuQuota,
-			CPUShares:          query.CpuShares,
-			CPUSetCPUs:         query.CpuSetCpus,
-			CPUSetMems:         "",
-			HTTPProxy:          false,
-			Memory:             query.Memory,
-			DNSSearch:          nil,
-			DNSServers:         nil,
-			DNSOptions:         nil,
-			MemorySwap:         query.MemSwap,
-			LabelOpts:          nil,
-			SeccompProfilePath: "",
-			ApparmorProfile:    "",
-			ShmSize:            strconv.Itoa(query.ShmSize),
-			Ulimit:             nil,
-			Volumes:            nil,
+			CPUPeriod:  query.CpuPeriod,
+			CPUQuota:   query.CpuQuota,
+			CPUShares:  query.CpuShares,
+			CPUSetCPUs: query.CpuSetCpus,
+			Memory:     query.Memory,
+			MemorySwap: query.MemSwap,
+			ShmSize:    strconv.Itoa(query.ShmSize),
 		},
-		DefaultMountsFilePath:   "",
-		IIDFile:                 "",
 		Squash:                  query.Squash,
 		Labels:                  labels,
-		Annotations:             nil,
-		OnBuild:                 nil,
-		Layers:                  false,
 		NoCache:                 query.NoCache,
 		RemoveIntermediateCtrs:  query.Rm,
 		ForceRmIntermediateCtrs: query.ForceRm,
-		BlobDirectory:           "",
 		Target:                  query.Target,
-		Devices:                 nil,
 	}
 
 	runtime := r.Context().Value("runtime").(*libpod.Runtime)
-	id, _, err := runtime.Build(r.Context(), buildOptions, query.Dockerfile)
-	if err != nil {
-		utils.InternalServerError(w, err)
-		return
+	runCtx, cancel := context.WithCancel(context.Background())
+	var imageID string
+	go func() {
+		defer cancel()
+		imageID, _, err = runtime.Build(r.Context(), buildOptions, query.Dockerfile)
+		if err != nil {
+			stderr.Write([]byte(err.Error() + "\n"))
+		}
+	}()
+
+	flush := func() {
+		if flusher, ok := w.(http.Flusher); ok {
+			flusher.Flush()
+		}
 	}
 
-	// Find image ID that was built...
-	utils.WriteResponse(w, http.StatusOK,
-		struct {
-			Stream string `json:"stream"`
-		}{
-			Stream: progress.String() + "\n" +
-				strings.Join(buildEvents, "\n") +
-				fmt.Sprintf("\nSuccessfully built %s\n", id),
-		})
+	// Send headers and prime client for stream to come
+	w.WriteHeader(http.StatusOK)
+	w.Header().Add("Content-Type", "application/json")
+	flush()
+
+	var failed bool
+
+	body := w.(io.Writer)
+	if logrus.IsLevelEnabled(logrus.DebugLevel) {
+		if v, found := os.LookupEnv("PODMAN_RETAIN_BUILD_ARTIFACT"); found {
+			if keep, _ := strconv.ParseBool(v); keep {
+				t, _ := ioutil.TempFile("", "build_*_server")
+				defer t.Close()
+				body = io.MultiWriter(t, w)
+			}
+		}
+	}
+
+	enc := json.NewEncoder(body)
+	enc.SetEscapeHTML(true)
+loop:
+	for {
+		m := struct {
+			Stream string `json:"stream,omitempty"`
+			Error  string `json:"error,omitempty"`
+		}{}
+
+		select {
+		case e := <-stdout.Chan():
+			m.Stream = string(e)
+			if err := enc.Encode(m); err != nil {
+				stderr.Write([]byte(err.Error()))
+			}
+			flush()
+		case e := <-auxout.Chan():
+			m.Stream = string(e)
+			if err := enc.Encode(m); err != nil {
+				stderr.Write([]byte(err.Error()))
+			}
+			flush()
+		case e := <-reporter.Chan():
+			m.Stream = string(e)
+			if err := enc.Encode(m); err != nil {
+				stderr.Write([]byte(err.Error()))
+			}
+			flush()
+		case e := <-stderr.Chan():
+			failed = true
+			m.Error = string(e)
+			if err := enc.Encode(m); err != nil {
+				logrus.Warnf("Failed to json encode error %q", err.Error())
+			}
+			flush()
+		case <-runCtx.Done():
+			if !failed {
+				if utils.IsLibpodRequest(r) {
+					m.Stream = imageID
+				} else {
+					m.Stream = fmt.Sprintf("Successfully built %12.12s\n", imageID)
+				}
+				if err := enc.Encode(m); err != nil {
+					logrus.Warnf("Failed to json encode error %q", err.Error())
+				}
+				flush()
+			}
+			break loop
+		}
+	}
 }
 
 func extractTarFile(r *http.Request) (string, error) {
@@ -253,10 +289,9 @@ func extractTarFile(r *http.Request) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	buildDir := filepath.Join(anchorDir, "build")
 
 	path := filepath.Join(anchorDir, "tarBall")
-	tarBall, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0666)
+	tarBall, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600)
 	if err != nil {
 		return "", err
 	}
@@ -265,14 +300,17 @@ func extractTarFile(r *http.Request) (string, error) {
 	// Content-Length not used as too many existing API clients didn't honor it
 	_, err = io.Copy(tarBall, r.Body)
 	r.Body.Close()
-
 	if err != nil {
 		return "", fmt.Errorf("failed Request: Unable to copy tar file from request body %s", r.RequestURI)
 	}
 
-	_, _ = tarBall.Seek(0, 0)
-	if err := archive.Untar(tarBall, buildDir, &archive.TarOptions{}); err != nil {
+	buildDir := filepath.Join(anchorDir, "build")
+	err = os.Mkdir(buildDir, 0700)
+	if err != nil {
 		return "", err
 	}
-	return anchorDir, nil
+
+	_, _ = tarBall.Seek(0, 0)
+	err = archive.Untar(tarBall, buildDir, nil)
+	return buildDir, err
 }