imagebuild cleanup and fixes

Signed-off-by: Kyle Quest <kcq.public@gmail.com>

pkg/app/master/command/cliprompt.go

@@ -26,6 +26,7 @@ import (
 type InteractiveApp struct {
 	appPrompt     *prompt.Prompt
 	fpCompleter   completer.FilePathCompleter
+	dpCompleter   completer.FilePathCompleter
 	app           *cli.App
 	dclient       *dockerapi.Client
 	crtConnection string
@@ -38,6 +39,10 @@ func NewInteractiveApp(app *cli.App, gparams *GenericParams) *InteractiveApp {
 		fpCompleter: completer.FilePathCompleter{
 			IgnoreCase: true,
 		},
+		dpCompleter: completer.FilePathCompleter{
+			IgnoreCase: true,
+			Filter:     func(fi os.FileInfo) bool { return fi.IsDir() },
+		},
 	}
 
 	ia.appPrompt = prompt.New(
@@ -507,6 +512,10 @@ func CompleteFile(ia *InteractiveApp, token string, params prompt.Document) []pr
 	return ia.fpCompleter.Complete(params)
 }
 
+func CompleteDir(ia *InteractiveApp, token string, params prompt.Document) []prompt.Suggest {
+	return ia.dpCompleter.Complete(params)
+}
+
 // Runtime
 
 var runtimeValues = []prompt.Suggest{

pkg/app/master/command/imagebuild/cli.go

@@ -3,8 +3,10 @@ package imagebuild
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"strings"
 
+	log "github.com/sirupsen/logrus"
 	"github.com/urfave/cli/v2"
 
 	"github.com/mintoolkit/mint/pkg/app"
@@ -43,8 +45,11 @@ var CLI = &cli.Command{
 	Usage:   Usage,
 	Flags:   ImageBuildFlags,
 	Action: func(ctx *cli.Context) error {
+		logger := log.WithFields(log.Fields{"app": command.AppName, "cmd": Name, "op": "cli.Action"})
+
 		gcvalues, ok := command.CLIContextGet(ctx.Context, command.GlobalParams).(*command.GenericParams)
 		if !ok || gcvalues == nil {
+			logger.Error("no gcvalues")
 			return command.ErrNoGlobalParams
 		}
 
@@ -64,6 +69,7 @@ var CLI = &cli.Command{
 			ContextDir:       ctx.String(FlagContextDir),
 			Runtime:          ctx.String(FlagRuntimeLoad),
 			Architecture:     ctx.String(FlagArchitecture),
+			Labels:           map[string]string{},
 		}
 
 		cboBuildArgs := command.ParseKVParams(ctx.StringSlice(FlagBuildArg))
@@ -79,26 +85,39 @@ var CLI = &cli.Command{
 
 		engineProps, found := BuildEngines[cparams.Engine]
 		if !found {
+			logger.Errorf("engine not found - %s", cparams.Engine)
 			return command.ErrBadParamValue
 		}
 
 		if cparams.Dockerfile == "" {
 			cparams.Dockerfile = DefaultDockerfilePath
 		}
 
-		if !fsutil.Exists(cparams.Dockerfile) {
+		if !fsutil.DirExists(cparams.ContextDir) {
+			logger.Errorf("context dir not found - %s", cparams.ContextDir)
 			return command.ErrBadParamValue
 		}
 
-		if !fsutil.DirExists(cparams.ContextDir) {
-			return command.ErrBadParamValue
+		switch cparams.Engine {
+		case BuildkitBuildEngine, DepotBuildEngine:
+			if !fsutil.Exists(cparams.Dockerfile) {
+				logger.Errorf("Dockerfile not found - '%s'", cparams.Dockerfile)
+				return command.ErrBadParamValue
+			}
+		default:
+			fullDockerfilePath := filepath.Join(cparams.ContextDir, cparams.Dockerfile)
+			if !fsutil.Exists(fullDockerfilePath) {
+				logger.Errorf("Dockerfile not found - '%s' ('%s')", cparams.Dockerfile, fullDockerfilePath)
+				return command.ErrBadParamValue
+			}
 		}
 
 		if cparams.Architecture == "" {
 			cparams.Architecture = GetDefaultBuildArch()
 		}
 
 		if !IsArchValue(cparams.Architecture) {
+			logger.Errorf("architecture not supported - %s", cparams.Architecture)
 			return command.ErrBadParamValue
 		}
 

pkg/app/master/command/imagebuild/flags.go

@@ -31,7 +31,7 @@ const (
 	FlagImageArchiveFileUsage = "local file path for the image tar archive file"
 
 	FlagDockerfile      = "dockerfile"
-	FlagDockerfileUsage = "local Dockerfile path"
+	FlagDockerfileUsage = "local Dockerfile path (for buildkit and depot) or a relative to the build context directory (for docker or podman)"
 
 	FlagContextDir      = "context-dir"
 	FlagContextDirUsage = "local build context directory"

pkg/app/master/command/imagebuild/prompt.go

@@ -54,11 +54,13 @@ var CommandFlagSuggestions = &command.FlagSuggestions{
 		{Text: command.FullFlagName(FlagDockerfile), Description: FlagDockerfileUsage},
 		{Text: command.FullFlagName(FlagContextDir), Description: FlagContextDirUsage},
 		{Text: command.FullFlagName(FlagBuildArg), Description: FlagBuildArgUsage},
+		{Text: command.FullFlagName(FlagLabel), Description: FlagLabelUsage},
 		{Text: command.FullFlagName(FlagArchitecture), Description: FlagArchitectureUsage},
 	},
 	Values: map[string]command.CompleteValue{
 		command.FullFlagName(FlagEngine):       completeBuildEngine,
 		command.FullFlagName(FlagRuntimeLoad):  completeRuntimeLoad,
 		command.FullFlagName(FlagArchitecture): completeArchitecture,
+		command.FullFlagName(FlagContextDir):   command.CompleteDir,
 	},
 }

pkg/crt/docker/dockercrtclient/dockercrtclient.go

@@ -63,6 +63,10 @@ func (ref *Instance) BuildImage(options imagebuilder.DockerfileBuildOptions) err
 		options.Labels = labels
 	}
 
+	if options.BuildContext == "" {
+		options.BuildContext = "."
+	}
+
 	//not using options.CacheTo in this image builder...
 	buildOptions := docker.BuildImageOptions{
 		Dockerfile: options.Dockerfile,
@@ -90,21 +94,16 @@ func (ref *Instance) BuildImage(options imagebuilder.DockerfileBuildOptions) err
 	} else {
 		if exists := fsutil.DirExists(options.BuildContext); exists {
 			buildOptions.ContextDir = options.BuildContext
+			//Dockerfile path is expected to be relative to build context
+			fullDockerfileName := filepath.Join(buildOptions.ContextDir, buildOptions.Dockerfile)
+			if !fsutil.Exists(fullDockerfileName) || !fsutil.IsRegularFile(fullDockerfileName) {
+				return fmt.Errorf("invalid dockerfile reference (%s) - %s", buildOptions.Dockerfile, fullDockerfileName)
+			}
 		} else {
 			return imagebuilder.ErrInvalidContextDir
 		}
 	}
 
-	if !fsutil.Exists(buildOptions.Dockerfile) || !fsutil.IsRegularFile(buildOptions.Dockerfile) {
-		//a slightly hacky behavior using the build context directory if the dockerfile flag doesn't include a usable path
-		fullDockerfileName := filepath.Join(buildOptions.ContextDir, buildOptions.Dockerfile)
-		if !fsutil.Exists(fullDockerfileName) || !fsutil.IsRegularFile(fullDockerfileName) {
-			return fmt.Errorf("invalid dockerfile reference - %s", fullDockerfileName)
-		}
-
-		buildOptions.Dockerfile = fullDockerfileName
-	}
-
 	if options.OutputStream != nil {
 		buildOptions.OutputStream = options.OutputStream
 	} else if ref.showBuildLogs {

pkg/crt/podman/podmancrtclient/podmancrtclient.go

@@ -77,6 +77,10 @@ func (ref *Instance) BuildImage(options imagebuilder.DockerfileBuildOptions) err
 		}
 	}
 
+	if options.BuildContext == "" {
+		options.BuildContext = "."
+	}
+
 	var contextDir string
 	if strings.HasPrefix(options.BuildContext, "http://") ||
 		strings.HasPrefix(options.BuildContext, "https://") {
@@ -86,20 +90,16 @@ func (ref *Instance) BuildImage(options imagebuilder.DockerfileBuildOptions) err
 	} else {
 		if exists := fsutil.DirExists(options.BuildContext); exists {
 			contextDir = options.BuildContext
+			//Dockerfile path is expected to be relative to build context
+			fullDockerfileName := filepath.Join(contextDir, options.Dockerfile)
+			if !fsutil.Exists(fullDockerfileName) || !fsutil.IsRegularFile(fullDockerfileName) {
+				return fmt.Errorf("invalid dockerfile reference (%s) - %s", options.Dockerfile, fullDockerfileName)
+			}
 		} else {
 			return imagebuilder.ErrInvalidContextDir
 		}
 	}
 
-	fullDockerfileName := options.Dockerfile
-	if !fsutil.Exists(fullDockerfileName) || !fsutil.IsRegularFile(fullDockerfileName) {
-		//a slightly hacky behavior using the build context directory if the dockerfile flag doesn't include a usable path
-		fullDockerfileName = filepath.Join(contextDir, fullDockerfileName)
-		if !fsutil.Exists(fullDockerfileName) || !fsutil.IsRegularFile(fullDockerfileName) {
-			return fmt.Errorf("invalid dockerfile reference - %s", fullDockerfileName)
-		}
-	}
-
 	buildOptions := images.BuildOptions{
 		BuildOptions: buildahDefine.BuildOptions{
 			Output:                 options.ImagePath,
@@ -110,15 +110,18 @@ func (ref *Instance) BuildImage(options imagebuilder.DockerfileBuildOptions) err
 			RemoveIntermediateCtrs: true,
 			PullPolicy:             buildahDefine.PullIfMissing,
 			OutputFormat:           buildahDefine.Dockerv2ImageManifest, //buildah.OCIv1ImageManifest
-			CommonBuildOpts: &buildahDefine.CommonBuildOptions{
-				AddHost: strings.Split(options.ExtraHosts, ","),
+			CommonBuildOpts:        &buildahDefine.CommonBuildOptions{
+				//AddHost: strings.Split(options.ExtraHosts, ","),
 			},
-
-			//ConfigureNetwork: tbd <- options.NetworkMode
+			ConfigureNetwork: buildahDefine.NetworkDefault, //tbd <- options.NetworkMode
 			//CacheFrom: tbd <- options.CacheFrom
 			//CacheTo: tbd <- options.CacheFrom
 		},
-		ContainerFiles: []string{fullDockerfileName},
+		ContainerFiles: []string{options.Dockerfile},
+	}
+
+	if options.ExtraHosts != "" {
+		buildOptions.CommonBuildOpts.AddHost = strings.Split(options.ExtraHosts, ",")
 	}
 
 	if len(options.Platforms) > 0 {
@@ -149,8 +152,11 @@ func (ref *Instance) BuildImage(options imagebuilder.DockerfileBuildOptions) err
 		}
 	}
 
-	for _, nv := range options.BuildArgs {
-		buildOptions.Args[nv.Name] = nv.Value
+	if len(options.BuildArgs) > 0 {
+		buildOptions.Args = map[string]string{}
+		for _, nv := range options.BuildArgs {
+			buildOptions.Args[nv.Name] = nv.Value
+		}
 	}
 
 	if options.OutputStream != nil {

pkg/imagebuilder/standardbuilder/engine.go

@@ -1,16 +1,8 @@
 package standardbuilder
 
 import (
-	//"bytes"
-	//"fmt"
-	//"path/filepath"
-	//"strings"
-
-	//docker "github.com/fsouza/go-dockerclient"
-
 	"github.com/mintoolkit/mint/pkg/crt"
 	"github.com/mintoolkit/mint/pkg/imagebuilder"
-	//"github.com/mintoolkit/mint/pkg/util/fsutil"
 )
 
 const (
@@ -52,89 +44,9 @@ func (ref *Engine) Name() string {
 }
 
 func (ref *Engine) BuildLog() string {
-	return ref.pclient.BuildOutputLog() //ref.buildLog.String()
+	return ref.pclient.BuildOutputLog()
 }
 
 func (ref *Engine) Build(options imagebuilder.DockerfileBuildOptions) error {
 	return ref.pclient.BuildImage(options)
-	/*
-		if len(options.Labels) > 0 {
-			//Docker has a limit on how long the labels can be
-			labels := map[string]string{}
-			for k, v := range options.Labels {
-				lineLen := len(k) + len(v) + 7
-				if lineLen > 65535 {
-					//TODO: improve JSON data splitting
-					valueLen := len(v)
-					parts := valueLen / 50000
-					parts++
-					offset := 0
-					for i := 0; i < parts && offset < valueLen; i++ {
-						chunkSize := 50000
-						if (offset + chunkSize) > valueLen {
-							chunkSize = valueLen - offset
-						}
-						value := v[offset:(offset + chunkSize)]
-						offset += chunkSize
-						key := fmt.Sprintf("%s.%d", k, i)
-						labels[key] = value
-					}
-				} else {
-					labels[k] = v
-				}
-			}
-			options.Labels = labels
-		}
-
-		//not using options.CacheTo in this image builder...
-		buildOptions := docker.BuildImageOptions{
-			Dockerfile: options.Dockerfile,
-			Target:     options.Target,
-			Name:       options.ImagePath,
-
-			NetworkMode:    options.NetworkMode,
-			ExtraHosts:     options.ExtraHosts,
-			CacheFrom:      options.CacheFrom,
-			Labels:         options.Labels,
-			RmTmpContainer: true,
-		}
-
-		if len(options.Platforms) > 0 {
-			buildOptions.Platform = strings.Join(options.Platforms, ",")
-		}
-
-		for _, nv := range options.BuildArgs {
-			buildOptions.BuildArgs = append(buildOptions.BuildArgs, docker.BuildArg{Name: nv.Name, Value: nv.Value})
-		}
-
-		if strings.HasPrefix(options.BuildContext, "http://") ||
-			strings.HasPrefix(options.BuildContext, "https://") {
-			buildOptions.Remote = options.BuildContext
-		} else {
-			if exists := fsutil.DirExists(options.BuildContext); exists {
-				buildOptions.ContextDir = options.BuildContext
-			} else {
-				return imagebuilder.ErrInvalidContextDir
-			}
-		}
-
-		if !fsutil.Exists(buildOptions.Dockerfile) || !fsutil.IsRegularFile(buildOptions.Dockerfile) {
-			//a slightly hacky behavior using the build context directory if the dockerfile flag doesn't include a usable path
-			fullDockerfileName := filepath.Join(buildOptions.ContextDir, buildOptions.Dockerfile)
-			if !fsutil.Exists(fullDockerfileName) || !fsutil.IsRegularFile(fullDockerfileName) {
-				return fmt.Errorf("invalid dockerfile reference - %s", fullDockerfileName)
-			}
-
-			buildOptions.Dockerfile = fullDockerfileName
-		}
-
-		if options.OutputStream != nil {
-			buildOptions.OutputStream = options.OutputStream
-		} else if ref.showBuildLogs {
-			ref.buildLog.Reset()
-			buildOptions.OutputStream = &ref.buildLog
-		}
-
-		return ref.pclient.BuildImage(buildOptions)
-	*/
 }