Return better error messages (#33)

This hoists the publisher creation out of the loop for `ko publish` to
avoid needlessly resolving credentials multiple times.

This pulls the publisher creation outside of resolveFilesToWriter so we
can fail earlier if e.g. KO_DOCKER_REPO is unset, otherwise we start up
the kubectl goroutine and it would asynchronously fail with:

  error: no objects passed to apply

pkg/commands/apply.go

@@ -15,14 +15,14 @@
 package commands
 
 import (
-	"github.com/google/ko/pkg/commands/options"
-	"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
 	"log"
 	"os"
 	"os/exec"
 
+	"github.com/google/ko/pkg/commands/options"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
+	"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
 )
 
 // addApply augments our CLI surface with apply.
@@ -62,6 +62,14 @@ func addApply(topLevel *cobra.Command) {
   cat config.yaml | ko apply -f -`,
 		Args: cobra.NoArgs,
 		Run: func(cmd *cobra.Command, args []string) {
+			builder, err := makeBuilder(do)
+			if err != nil {
+				log.Fatalf("error creating builder: %v", err)
+			}
+			publisher, err := makePublisher(no, lo, ta)
+			if err != nil {
+				log.Fatalf("error creating publisher: %v", err)
+			}
 			// Create a set of ko-specific flags to ignore when passing through
 			// kubectl global flags.
 			ignoreSet := make(map[string]struct{})
@@ -107,7 +115,7 @@ func addApply(topLevel *cobra.Command) {
 					stdin.Write([]byte("---\n"))
 				}
 				// Once primed kick things off.
-				resolveFilesToWriter(fo, no, lo, ta, do, stdin)
+				resolveFilesToWriter(builder, publisher, fo, stdin)
 			}()
 
 			// Run it.

pkg/commands/config.go

@@ -21,12 +21,11 @@ import (
 	"strconv"
 	"time"
 
-	"github.com/spf13/viper"
-
 	"github.com/google/go-containerregistry/pkg/authn"
 	"github.com/google/go-containerregistry/pkg/name"
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/google/go-containerregistry/pkg/v1/remote"
+	"github.com/spf13/viper"
 )
 
 var (
@@ -51,7 +50,7 @@ func getCreationTime() (*v1.Time, error) {
 
 	seconds, err := strconv.ParseInt(epoch, 10, 64)
 	if err != nil {
-		return nil, fmt.Errorf("the environment variable SOURCE_DATE_EPOCH is invalid. It's must be a number of seconds since January 1st 1970, 00:00 UTC, got %v", err)
+		return nil, fmt.Errorf("the environment variable SOURCE_DATE_EPOCH should be the number of seconds since January 1st 1970, 00:00 UTC, got: %v", err)
 	}
 	return &v1.Time{time.Unix(seconds, 0)}, nil
 }

pkg/commands/create.go

@@ -15,14 +15,14 @@
 package commands
 
 import (
-	"github.com/google/ko/pkg/commands/options"
-	"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
 	"log"
 	"os"
 	"os/exec"
 
+	"github.com/google/ko/pkg/commands/options"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
+	"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
 )
 
 // addCreate augments our CLI surface with apply.
@@ -62,6 +62,14 @@ func addCreate(topLevel *cobra.Command) {
   cat config.yaml | ko create -f -`,
 		Args: cobra.NoArgs,
 		Run: func(cmd *cobra.Command, args []string) {
+			builder, err := makeBuilder(do)
+			if err != nil {
+				log.Fatalf("error creating builder: %v", err)
+			}
+			publisher, err := makePublisher(no, lo, ta)
+			if err != nil {
+				log.Fatalf("error creating publisher: %v", err)
+			}
 			// Create a set of ko-specific flags to ignore when passing through
 			// kubectl global flags.
 			ignoreSet := make(map[string]struct{})
@@ -107,7 +115,7 @@ func addCreate(topLevel *cobra.Command) {
 					stdin.Write([]byte("---\n"))
 				}
 				// Once primed kick things off.
-				resolveFilesToWriter(fo, no, lo, ta, do, stdin)
+				resolveFilesToWriter(builder, publisher, fo, stdin)
 			}()
 
 			// Run it.

pkg/commands/publish.go

@@ -15,11 +15,12 @@
 package commands
 
 import (
+	"log"
+
 	"github.com/google/ko/pkg/commands/options"
 	"github.com/spf13/cobra"
 )
 
-
 // addPublish augments our CLI surface with publish.
 func addPublish(topLevel *cobra.Command) {
 	lo := &options.LocalOptions{}
@@ -58,7 +59,17 @@ func addPublish(topLevel *cobra.Command) {
   ko publish --local github.com/foo/bar/cmd/baz github.com/foo/bar/cmd/blah`,
 		Args: cobra.MinimumNArgs(1),
 		Run: func(_ *cobra.Command, args []string) {
-			publishImages(args, no, lo, ta, do)
+			builder, err := makeBuilder(do)
+			if err != nil {
+				log.Fatalf("error creating builder: %v", err)
+			}
+			publisher, err := makePublisher(no, lo, ta)
+			if err != nil {
+				log.Fatalf("error creating publisher: %v", err)
+			}
+			if _, err := publishImages(args, publisher, builder); err != nil {
+				log.Fatalf("failed to publish images: %v", err)
+			}
 		},
 	}
 	options.AddLocalArg(publish, lo)

pkg/commands/publisher.go

@@ -16,14 +16,11 @@ package commands
 
 import (
 	"fmt"
-	"github.com/google/ko/pkg/commands/options"
 	gb "go/build"
-	"log"
 	"os"
 	"path/filepath"
 	"strings"
 
-	"github.com/google/go-containerregistry/pkg/authn"
 	"github.com/google/go-containerregistry/pkg/name"
 	"github.com/google/ko/pkg/build"
 	"github.com/google/ko/pkg/publish"
@@ -39,15 +36,7 @@ func qualifyLocalImport(importpath, gopathsrc, pwd string) (string, error) {
 	return filepath.Join(strings.TrimPrefix(pwd, gopathsrc+string(filepath.Separator)), importpath), nil
 }
 
-func publishImages(importpaths []string, no *options.NameOptions, lo *options.LocalOptions, ta *options.TagsOptions, do *options.DebugOptions) map[string]name.Reference {
+func publishImages(importpaths []string, pub publish.Interface, b build.Interface) (map[string]name.Reference, error) {
-	opt, err := gobuildOptions(do)
-	if err != nil {
-		log.Fatalf("error setting up builder options: %v", err)
-	}
-	b, err := build.NewGo(opt...)
-	if err != nil {
-		log.Fatalf("error creating go builder: %v", err)
-	}
 	imgs := make(map[string]name.Reference)
 	for _, importpath := range importpaths {
 		if gb.IsLocalImport(importpath) {
@@ -56,44 +45,27 @@ func publishImages(importpaths []string, no *options.NameOptions, lo *options.Lo
 			gopathsrc := filepath.Join(gb.Default.GOPATH, "src")
 			pwd, err := os.Getwd()
 			if err != nil {
-				log.Fatalf("error getting current working directory: %v", err)
+				return nil, fmt.Errorf("error getting current working directory: %v", err)
 			}
 			importpath, err = qualifyLocalImport(importpath, gopathsrc, pwd)
 			if err != nil {
-				log.Fatal(err)
+				return nil, err
 			}
 		}
 
 		if !b.IsSupportedReference(importpath) {
-			log.Fatalf("importpath %q is not supported", importpath)
+			return nil, fmt.Errorf("importpath %q is not supported", importpath)
 		}
 
 		img, err := b.Build(importpath)
 		if err != nil {
-			log.Fatalf("error building %q: %v", importpath, err)
+			return nil, fmt.Errorf("error building %q: %v", importpath, err)
-		}
-		var pub publish.Interface
-		repoName := os.Getenv("KO_DOCKER_REPO")
-
-		namer := options.MakeNamer(no)
-
-		if lo.Local || repoName == publish.LocalDomain {
-			pub = publish.NewDaemon(namer, ta.Tags)
-		} else {
-			if _, err := name.NewRepository(repoName, name.WeakValidation); err != nil {
-				log.Fatalf("the environment variable KO_DOCKER_REPO must be set to a valid docker repository, got %v", err)
-			}
-			opts := []publish.Option{publish.WithAuthFromKeychain(authn.DefaultKeychain), publish.WithNamer(namer), publish.WithTags(ta.Tags)}
-			pub, err = publish.NewDefault(repoName, opts...)
-			if err != nil {
-				log.Fatalf("error setting up default image publisher: %v", err)
-			}
 		}
 		ref, err := pub.Publish(img, importpath)
 		if err != nil {
-			log.Fatalf("error publishing %s: %v", importpath, err)
+			return nil, fmt.Errorf("error publishing %s: %v", importpath, err)
 		}
 		imgs[importpath] = ref
 	}
-	return imgs
+	return imgs, nil
 }

pkg/commands/resolve.go

@@ -15,9 +15,11 @@
 package commands
 
 import (
+	"log"
+	"os"
+
 	"github.com/google/ko/pkg/commands/options"
 	"github.com/spf13/cobra"
-	"os"
 )
 
 // addResolve augments our CLI surface with resolve.
@@ -55,7 +57,15 @@ func addResolve(topLevel *cobra.Command) {
   ko resolve --local -f config/`,
 		Args: cobra.NoArgs,
 		Run: func(cmd *cobra.Command, args []string) {
-			resolveFilesToWriter(fo, no, lo, ta, do, os.Stdout)
+			builder, err := makeBuilder(do)
+			if err != nil {
+				log.Fatalf("error creating builder: %v", err)
+			}
+			publisher, err := makePublisher(no, lo, ta)
+			if err != nil {
+				log.Fatalf("error creating publisher: %v", err)
+			}
+			resolveFilesToWriter(builder, publisher, fo, os.Stdout)
 		},
 	}
 	options.AddLocalArg(resolve, lo)

pkg/commands/resolver.go

@@ -15,8 +15,8 @@
 package commands
 
 import (
+	"errors"
 	"fmt"
-	"github.com/google/ko/pkg/commands/options"
 	"io"
 	"io/ioutil"
 	"log"
@@ -26,6 +26,7 @@ import (
 	"github.com/google/go-containerregistry/pkg/authn"
 	"github.com/google/go-containerregistry/pkg/name"
 	"github.com/google/ko/pkg/build"
+	"github.com/google/ko/pkg/commands/options"
 	"github.com/google/ko/pkg/publish"
 	"github.com/google/ko/pkg/resolve"
 	"github.com/mattmoor/dep-notify/pkg/graph"
@@ -87,9 +88,12 @@ func makePublisher(no *options.NameOptions, lo *options.LocalOptions, ta *option
 		if lo.Local || repoName == publish.LocalDomain {
 			return publish.NewDaemon(namer, ta.Tags), nil
 		}
+		if repoName == "" {
+			return nil, errors.New("KO_DOCKER_REPO environment variable is unset")
+		}
 		_, err := name.NewRepository(repoName, name.WeakValidation)
 		if err != nil {
-			return nil, fmt.Errorf("the environment variable KO_DOCKER_REPO must be set to a valid docker repository, got %v", err)
+			return nil, fmt.Errorf("failed to parse environment variable KO_DOCKER_REPO=%q as repository: %v", repoName, err)
 		}
 
 		return publish.NewDefault(repoName,
@@ -108,18 +112,8 @@ func makePublisher(no *options.NameOptions, lo *options.LocalOptions, ta *option
 // resolvedFuture represents a "future" for the bytes of a resolved file.
 type resolvedFuture chan []byte
 
-func resolveFilesToWriter(fo *options.FilenameOptions, no *options.NameOptions, lo *options.LocalOptions, ta *options.TagsOptions, do *options.DebugOptions, out io.WriteCloser) {
+func resolveFilesToWriter(builder *build.Caching, publisher publish.Interface, fo *options.FilenameOptions, out io.WriteCloser) {
 	defer out.Close()
-	builder, err := makeBuilder(do)
-	if err != nil {
-		log.Fatalf("error creating builder: %v", err)
-	}
-
-	// Wrap publisher in a memoizing publisher implementation.
-	publisher, err := makePublisher(no, lo, ta)
-	if err != nil {
-		log.Fatalf("error creating publisher: %v", err)
-	}
 
 	// By having this as a channel, we can hook this up to a filesystem
 	// watcher and leave `fs` open to stream the names of yaml files
@@ -132,6 +126,7 @@ func resolveFilesToWriter(fo *options.FilenameOptions, no *options.NameOptions,
 
 	var g graph.Interface
 	var errCh chan error
+	var err error
 	if fo.Watch {
 		// Start a dep-notify process that on notifications scans the
 		// file-to-recorded-build map and for each affected file resends

pkg/commands/run.go

@@ -15,11 +15,11 @@
 package commands
 
 import (
-	"github.com/google/ko/pkg/commands/options"
 	"log"
 	"os"
 	"os/exec"
 
+	"github.com/google/ko/pkg/commands/options"
 	"github.com/spf13/cobra"
 )
 
@@ -45,7 +45,18 @@ func addRun(topLevel *cobra.Command) {
   # This supports relative import paths as well.
   ko run foo --image=./cmd/baz`,
 		Run: func(cmd *cobra.Command, args []string) {
-			imgs := publishImages([]string{bo.Path}, no, lo, ta, do)
+			builder, err := makeBuilder(do)
+			if err != nil {
+				log.Fatalf("error creating builder: %v", err)
+			}
+			publisher, err := makePublisher(no, lo, ta)
+			if err != nil {
+				log.Fatalf("error creating publisher: %v", err)
+			}
+			imgs, err := publishImages([]string{bo.Path}, publisher, builder)
+			if err != nil {
+				log.Fatalf("failed to publish images: %v", err)
+			}
 
 			// There's only one, but this is the simple way to access the
 			// reference since the import path may have been qualified.