Update build images

* Add swagger annotations for all the query and response parameters
  for buildimages
* Improve populating the BuildOptions struct
* Improve swagger.json generation, removing tags.xml and move tag
  definiation into the swagger:meta block
* Update Makefile to be more robust, added target for validation

* TODO once validation passes add that step to the generation step

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

5 files changed, 366 insertions, 116 deletions

diff --git a/pkg/api/Makefile b/pkg/api/Makefile
index 8a1556800..915f0b9b3 100644
--- a/pkg/api/Makefile
+++ b/pkg/api/Makefile
@@ -2,6 +2,12 @@ export GO111MODULE=off
 
 SWAGGER_OUT ?= swagger.yaml
 
-swagger:
+.PHONY: ${SWAGGER_OUT}
+${SWAGGER_OUT}:
+	# generate doesn't remove file on error
+	rm -f ${SWAGGER_OUT}
 	swagger generate spec -o ${SWAGGER_OUT} -w ./
-	cat tags.yaml >> swagger.yaml
+
+# TODO: when pass validation move it under swagger.
+validate:
+	swagger validate ${SWAGGER_OUT}
diff --git a/pkg/api/handlers/handler.go b/pkg/api/handlers/handler.go
index 2efeb1379..4f303f6ab 100644
--- a/pkg/api/handlers/handler.go
+++ b/pkg/api/handlers/handler.go
@@ -36,11 +36,11 @@ func getRuntime(r *http.Request) *libpod.Runtime {
 	return r.Context().Value("runtime").(*libpod.Runtime)
 }
 
-func getHeader(r *http.Request, k string) string {
-	return r.Header.Get(k)
-}
-
-func hasHeader(r *http.Request, k string) bool {
-	_, found := r.Header[k]
-	return found
-}
+// func getHeader(r *http.Request, k string) string {
+// 	return r.Header.Get(k)
+// }
+//
+// func hasHeader(r *http.Request, k string) bool {
+// 	_, found := r.Header[k]
+// 	return found
+// }
diff --git a/pkg/api/handlers/images_build.go b/pkg/api/handlers/images_build.go
index c7c746392..b29c45574 100644
--- a/pkg/api/handlers/images_build.go
+++ b/pkg/api/handlers/images_build.go
@@ -1,6 +1,7 @@
 package handlers
 
 import (
+	"bytes"
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
@@ -9,58 +10,66 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
 
 	"github.com/containers/buildah"
 	"github.com/containers/buildah/imagebuildah"
 	"github.com/containers/libpod/pkg/api/handlers/utils"
 	"github.com/containers/storage/pkg/archive"
-	log "github.com/sirupsen/logrus"
+	"github.com/gorilla/mux"
 )
 
 func BuildImage(w http.ResponseWriter, r *http.Request) {
 	authConfigs := map[string]AuthConfig{}
-	if hasHeader(r, "X-Registry-Config") {
-		registryHeader := getHeader(r, "X-Registry-Config")
-		authConfigsJSON := base64.NewDecoder(base64.URLEncoding, strings.NewReader(registryHeader))
+	if hdr, found := r.Header["X-Registry-Config"]; found && len(hdr) > 0 {
+		authConfigsJSON := base64.NewDecoder(base64.URLEncoding, strings.NewReader(hdr[0]))
 		if json.NewDecoder(authConfigsJSON).Decode(&authConfigs) != nil {
-			utils.BadRequest(w, "X-Registry-Config", registryHeader, json.NewDecoder(authConfigsJSON).Decode(&authConfigs))
+			utils.BadRequest(w, "X-Registry-Config", hdr[0], json.NewDecoder(authConfigsJSON).Decode(&authConfigs))
 			return
 		}
 	}
 
+	if hdr, found := r.Header["Content-Type"]; found && len(hdr) > 0 {
+		if hdr[0] != "application/x-tar" {
+			utils.BadRequest(w, "Content-Type", hdr[0],
+				fmt.Errorf("Content-Type: %s is not supported. Should be \"application/x-tar\"", hdr[0]))
+		}
+	}
+
 	anchorDir, err := extractTarFile(r, w)
 	if err != nil {
 		utils.InternalServerError(w, err)
 		return
 	}
-	// defer os.RemoveAll(anchorDir)
+	defer os.RemoveAll(anchorDir)
 
 	query := struct {
-		Dockerfile  string `json:"dockerfile"`
-		Tag         string `json:"t"`
-		ExtraHosts  string `json:"extrahosts"`
-		Remote      string `json:"remote"`
-		Quiet       bool   `json:"q"`
-		NoCache     bool   `json:"nocache"`
-		CacheFrom   string `json:"cachefrom"`
-		Pull        string `json:"pull"`
-		Rm          bool   `json:"rm"`
-		ForceRm     bool   `json:"forcerm"`
-		Memory      int    `json:"memory"`
-		MemSwap     int    `json:"memswap"`
-		CpuShares   int    `json:"cpushares"`
-		CpuSetCpus  string `json:"cpusetcpus"`
-		CpuPeriod   int    `json:"cpuperiod"`
-		CpuQuota    int    `json:"cpuquota"`
-		BuildArgs   string `json:"buildargs"`
-		ShmSize     int    `json:"shmsize"`
-		Squash      bool   `json:"squash"`
-		Labels      string `json:"labels"`
-		NetworkMode string `json:"networkmode"`
-		Platform    string `json:"platform"`
-		Target      string `json:"target"`
-		Outputs     string `json:"outputs"`
+		Dockerfile  string `schema:"dockerfile"`
+		Tag         string `schema:"t"`
+		ExtraHosts  string `schema:"extrahosts"`
+		Remote      string `schema:"remote"`
+		Quiet       bool   `schema:"q"`
+		NoCache     bool   `schema:"nocache"`
+		CacheFrom   string `schema:"cachefrom"`
+		Pull        bool   `schema:"pull"`
+		Rm          bool   `schema:"rm"`
+		ForceRm     bool   `schema:"forcerm"`
+		Memory      int64  `schema:"memory"`
+		MemSwap     int64  `schema:"memswap"`
+		CpuShares   uint64 `schema:"cpushares"`
+		CpuSetCpus  string `schema:"cpusetcpus"`
+		CpuPeriod   uint64 `schema:"cpuperiod"`
+		CpuQuota    int64  `schema:"cpuquota"`
+		BuildArgs   string `schema:"buildargs"`
+		ShmSize     int    `schema:"shmsize"`
+		Squash      bool   `schema:"squash"`
+		Labels      string `schema:"labels"`
+		NetworkMode string `schema:"networkmode"`
+		Platform    string `schema:"platform"`
+		Target      string `schema:"target"`
+		Outputs     string `schema:"outputs"`
+		Registry    string `schema:"registry"`
 	}{
 		Dockerfile:  "Dockerfile",
 		Tag:         "",
@@ -69,7 +78,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 		Quiet:       false,
 		NoCache:     false,
 		CacheFrom:   "",
-		Pull:        "",
+		Pull:        false,
 		Rm:          true,
 		ForceRm:     false,
 		Memory:      0,
@@ -86,6 +95,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 		Platform:    "",
 		Target:      "",
 		Outputs:     "",
+		Registry:    "docker.io",
 	}
 
 	if err := decodeQuery(r, &query); err != nil {
@@ -93,80 +103,121 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// Tag is the name with optional tag...
-	var name = query.Tag
-	var tag string
+	var (
+		// Tag is the name with optional tag...
+		name = query.Tag
+		tag  = "latest"
+	)
 	if strings.Contains(query.Tag, ":") {
 		tokens := strings.SplitN(query.Tag, ":", 2)
 		name = tokens[0]
 		tag = tokens[1]
 	}
 
+	if t, found := mux.Vars(r)["target"]; found {
+		name = t
+	}
+
 	var buildArgs = map[string]string{}
-	if found := hasVar(r, "buildargs"); found {
-		if err := json.Unmarshal([]byte(query.BuildArgs), &buildArgs); err != nil {
-			utils.BadRequest(w, "buildargs", query.BuildArgs, err)
+	if a, found := mux.Vars(r)["buildargs"]; found {
+		if err := json.Unmarshal([]byte(a), &buildArgs); err != nil {
+			utils.BadRequest(w, "buildargs", a, err)
 			return
 		}
 	}
 
 	// convert label formats
 	var labels = []string{}
-	if hasVar(r, "labels") {
+	if l, found := mux.Vars(r)["labels"]; found {
 		var m = map[string]string{}
-		if err := json.Unmarshal([]byte(query.Labels), &m); err != nil {
-			utils.BadRequest(w, "labels", query.Labels, err)
+		if err := json.Unmarshal([]byte(l), &m); err != nil {
+			utils.BadRequest(w, "labels", l, err)
 			return
 		}
 
 		for k, v := range m {
-			labels = append(labels, fmt.Sprintf("%s=%v", k, v))
+			labels = append(labels, k+"="+v)
+		}
+	}
+
+	pullPolicy := buildah.PullIfMissing
+	if _, found := mux.Vars(r)["pull"]; found {
+		if query.Pull {
+			pullPolicy = buildah.PullAlways
 		}
 	}
 
+	// build events will be recorded here
+	var (
+		buildEvents = []string{}
+		progress    = bytes.Buffer{}
+	)
+
 	buildOptions := imagebuildah.BuildOptions{
 		ContextDirectory:               filepath.Join(anchorDir, "build"),
-		PullPolicy:                     0,
-		Registry:                       "",
-		IgnoreUnrecognizedInstructions: false,
+		PullPolicy:                     pullPolicy,
+		Registry:                       query.Registry,
+		IgnoreUnrecognizedInstructions: true,
 		Quiet:                          query.Quiet,
-		Isolation:                      0,
+		Isolation:                      buildah.IsolationChroot,
 		Runtime:                        "",
 		RuntimeArgs:                    nil,
 		TransientMounts:                nil,
-		Compression:                    0,
+		Compression:                    archive.Gzip,
 		Args:                           buildArgs,
 		Output:                         name,
 		AdditionalTags:                 []string{tag},
-		Log:                            nil,
-		In:                             nil,
-		Out:                            nil,
-		Err:                            nil,
-		SignaturePolicyPath:            "",
-		ReportWriter:                   nil,
-		OutputFormat:                   "",
-		SystemContext:                  nil,
-		NamespaceOptions:               nil,
-		ConfigureNetwork:               0,
-		CNIPluginPath:                  "",
-		CNIConfigDir:                   "",
-		IDMappingOptions:               nil,
-		AddCapabilities:                nil,
-		DropCapabilities:               nil,
-		CommonBuildOpts:                &buildah.CommonBuildOptions{},
-		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,
+		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,
+		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,
+		},
+		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,
 	}
 
 	id, _, err := getRuntime(r).Build(r.Context(), buildOptions, query.Dockerfile)
@@ -179,17 +230,13 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 		struct {
 			Stream string `json:"stream"`
 		}{
-			Stream: fmt.Sprintf("Successfully built %s\n", id),
+			Stream: progress.String() + "\n" +
+				strings.Join(buildEvents, "\n") +
+				fmt.Sprintf("\nSuccessfully built %s\n", id),
 		})
 }
 
 func extractTarFile(r *http.Request, w http.ResponseWriter) (string, error) {
-	var (
-		// length  int64
-		// n       int64
-		copyErr error
-	)
-
 	// build a home for the request body
 	anchorDir, err := ioutil.TempDir("", "libpod_builder")
 	if err != nil {
@@ -204,26 +251,14 @@ func extractTarFile(r *http.Request, w http.ResponseWriter) (string, error) {
 	}
 	defer tarBall.Close()
 
-	// if hasHeader(r, "Content-Length") {
-	// 	length, err := strconv.ParseInt(getHeader(r, "Content-Length"), 10, 64)
-	// 	if err != nil {
-	// 		return "", errors.New(fmt.Sprintf("Failed request: unable to parse Content-Length of '%s'", getHeader(r, "Content-Length")))
-	// 	}
-	// 	n, copyErr = io.CopyN(tarBall, r.Body, length+1)
-	// } else {
-	_, copyErr = io.Copy(tarBall, r.Body)
-	// }
+	// Content-Length not used as too many existing API clients didn't honor it
+	_, err = io.Copy(tarBall, r.Body)
 	r.Body.Close()
 
-	if copyErr != nil {
+	if err != nil {
 		utils.InternalServerError(w,
 			fmt.Errorf("failed Request: Unable to copy tar file from request body %s", r.RequestURI))
 	}
-	log.Debugf("Content-Length: %s", getVar(r, "Content-Length"))
-
-	// if hasHeader(r, "Content-Length") && n != length {
-	// 	return "", errors.New(fmt.Sprintf("Failed request: Given Content-Length does not match file size %d != %d", n, length))
-	// }
 
 	_, _ = tarBall.Seek(0, 0)
 	if err := archive.Untar(tarBall, buildDir, &archive.TarOptions{}); err != nil {
diff --git a/pkg/api/server/register_images.go b/pkg/api/server/register_images.go
index cd42afe71..744bc1ace 100644
--- a/pkg/api/server/register_images.go
+++ b/pkg/api/server/register_images.go
@@ -342,6 +342,210 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error {
 	//      $ref: '#/responses/InternalError'
 	r.Handle(VersionedPath("/commit"), APIHandler(s.Context, generic.CommitContainer)).Methods(http.MethodPost)
 
+	// swagger:operation POST /build images buildImage
+	// ---
+	// tags:
+	//  - images
+	// summary: Create image
+	// description: Build an image from the given Dockerfile(s)
+	// parameters:
+	//  - in: query
+	//    name: dockerfile
+	//    type: string
+	//    default: Dockerfile
+	//    description: |
+	//      Path within the build context to the `Dockerfile`.
+	//      This is ignored if remote is specified and points to an external `Dockerfile`.
+	//  - in: query
+	//    name: t
+	//    type: string
+	//    default: latest
+	//    description: A name and optional tag to apply to the image in the `name:tag` format.
+	//  - in: query
+	//    name: extrahosts
+	//    type: string
+	//    default:
+	//    description: |
+	//      TBD Extra hosts to add to /etc/hosts
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: remote
+	//    type: string
+	//    default:
+	//    description: |
+	//      A Git repository URI or HTTP/HTTPS context URI.
+	//      If the URI points to a single text file, the file’s contents are placed
+	//      into a file called Dockerfile and the image is built from that file. If
+	//      the URI points to a tarball, the file is downloaded by the daemon and the
+	//      contents therein used as the context for the build. If the URI points to a
+	//      tarball and the dockerfile parameter is also specified, there must be a file
+	//      with the corresponding path inside the tarball.
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: q
+	//    type: boolean
+	//    default: false
+	//    description: |
+	//      Suppress verbose build output
+	//  - in: query
+	//    name: nocache
+	//    type: boolean
+	//    default: false
+	//    description: |
+	//      Do not use the cache when building the image
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: cachefrom
+	//    type: string
+	//    default:
+	//    description: |
+	//      JSON array of images used to build cache resolution
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: pull
+	//    type: boolean
+	//    default: false
+	//    description: |
+	//      Attempt to pull the image even if an older image exists locally
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: rm
+	//    type: boolean
+	//    default: true
+	//    description: |
+	//      Remove intermediate containers after a successful build
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: forcerm
+	//    type: boolean
+	//    default: false
+	//    description: |
+	//      Always remove intermediate containers, even upon failure
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: memory
+	//    type: integer
+	//    description: |
+	//      Memory is the upper limit (in bytes) on how much memory running containers can use
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: memswap
+	//    type: integer
+	//    description: |
+	//      MemorySwap limits the amount of memory and swap together
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: cpushares
+	//    type: integer
+	//    description: |
+	//      CPUShares (relative weight
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: cpusetcpus
+	//    type: string
+	//    description: |
+	//      CPUSetCPUs in which to allow execution (0-3, 0,1)
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: cpuperiod
+	//    type: integer
+	//    description: |
+	//      CPUPeriod limits the CPU CFS (Completely Fair Scheduler) period
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: cpuquota
+	//    type: integer
+	//    description: |
+	//      CPUQuota limits the CPU CFS (Completely Fair Scheduler) quota
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: buildargs
+	//    type: string
+	//    default:
+	//    description: |
+	//      JSON map of string pairs denoting build-time variables.
+	//      For example, the build argument `Foo` with the value of `bar` would be encoded in JSON as `["Foo":"bar"]`.
+	//
+	//      For example, buildargs={"Foo":"bar"}.
+	//
+	//      Note(s):
+	//      * This should not be used to pass secrets.
+	//      * The value of buildargs should be URI component encoded before being passed to the API.
+	//
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: shmsize
+	//    type: integer
+	//    default: 67108864
+	//    description: |
+	//      ShmSize is the "size" value to use when mounting an shmfs on the container's /dev/shm directory.
+	//      Default is 64MB
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: squash
+	//    type: boolean
+	//    default: false
+	//    description: |
+	//      Silently ignored.
+	//      Squash the resulting images layers into a single layer
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: labels
+	//    type: string
+	//    default:
+	//    description: |
+	//      JSON map of key, value pairs to set as labels on the new image
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: networkmode
+	//    type: string
+	//    default: bridge
+	//    description: |
+	//      Sets the networking mode for the run commands during build.
+	//      Supported standard values are:
+	//        * `bridge` limited to containers within a single host, port mapping required for external access
+	//        * `host` no isolation between host and containers on this network
+	//        * `none` disable all networking for this container
+	//        * container:<nameOrID> share networking with given container
+	//        ---All other values are assumed to be a custom network's name
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: platform
+	//    type: string
+	//    default:
+	//    description: |
+	//      Platform format os[/arch[/variant]]
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: target
+	//    type: string
+	//    default:
+	//    description: |
+	//      Target build stage
+	//      (As of version 1.xx)
+	//  - in: query
+	//    name: outputs
+	//    type: string
+	//    default:
+	//    description: |
+	//      output configuration TBD
+	//      (As of version 1.xx)
+	// produces:
+	// - application/json
+	// responses:
+	//   200:
+	//    description: OK (As of version 1.xx)
+	//    schema:
+	//      type: object
+	//      required:
+	//        - stream
+	//      properties:
+	//        stream:
+	//          type: string
+	//          example: |
+	//            (build details...)
+	//            Successfully built 8ba084515c724cbf90d447a63600c0a6
+	r.Handle(VersionedPath("/build"), APIHandler(s.Context, handlers.BuildImage)).Methods(http.MethodPost)
 	/*
 		libpod endpoints
 	*/
@@ -605,6 +809,5 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error {
 	//      $ref: '#/responses/InternalError'
 	r.Handle(VersionedPath("/libpod/images/{name:..*}/tag"), APIHandler(s.Context, handlers.TagImage)).Methods(http.MethodPost)
 
-	r.Handle(VersionedPath("/build"), APIHandler(s.Context, handlers.BuildImage)).Methods(http.MethodPost)
 	return nil
 }
diff --git a/pkg/api/server/server.go b/pkg/api/server/server.go
index 9abedb359..a64995a26 100644
--- a/pkg/api/server/server.go
+++ b/pkg/api/server/server.go
@@ -1,6 +1,6 @@
-// Package serviceapi Provides a Container compatible interface (EXPERIMENTAL)
+// Package api Provides a container compatible interface.
 //
-// This documentation describes the HTTP LibPod interface.  It is to be consider
+// This documentation describes the HTTP Libpod interface.  It is to be consider
 // only as experimental as this point.  The endpoints, parameters, inputs, and
 // return values can all change.
 //
@@ -25,12 +25,18 @@
 //     - text/html
 //
 //     tags:
-//     - name: "Containers"
-//       description: manage containers
-//     - name: "Images"
-//       description: manage images
-//     - name: "System"
-//       description: manage system resources
+//     - name: containers
+//       description: Actions related to containers
+//     - name: images
+//       description: Actions related to images
+//     - name: pods
+//       description: Actions related to pods
+//     - name: volumes
+//       description: Actions related to volumes
+//     - name: containers (compat)
+//       description: Actions related to containers for the compatibility endpoints
+//     - name: images (compat)
+//       description: Actions related to images for the compatibility endpoints
 //
 // swagger:meta
 package server