IsSupportedReference returns descriptive error (#233)

This can be useful to determine what they need to do to make a ko publish work.
ko-build · Oct 31, 2020 · d767708 · d767708
1 parent ff18e80
commit d767708
Show file tree

Hide file tree

Showing 13 changed files with 45 additions and 44 deletions.
diff --git a/pkg/build/build.go b/pkg/build/build.go
@@ -24,10 +24,11 @@ import (
 // Interface abstracts different methods for turning a supported importpath
 // reference into a v1.Image.
 type Interface interface {
-	// IsSupportedReference determines whether the given reference is to an importpath reference
-	// that Ko supports building.
+	// IsSupportedReference determines whether the given reference is to an
+	// importpath reference that Ko supports building, returning an error
+	// if it is not.
 	// TODO(mattmoor): Verify that some base repo: foo.io/bar can be suffixed with this reference and parsed.
-	IsSupportedReference(string) bool
+	IsSupportedReference(string) error
 
 	// Build turns the given importpath reference into a v1.Image containing the Go binary
 	// (or a set of images as a v1.ImageIndex).

diff --git a/pkg/build/gobuild.go b/pkg/build/gobuild.go
@@ -172,16 +172,19 @@ func NewGo(options ...Option) (Interface, error) {
 //
 // Only valid importpaths that provide commands (i.e., are "package main") are
 // supported.
-func (g *gobuild) IsSupportedReference(s string) bool {
+func (g *gobuild) IsSupportedReference(s string) error {
 	ref := newRef(s)
 	if !ref.IsStrict() {
-		return false
+		return errors.New("importpath does not start with ko://")
 	}
 	p, err := g.importPackage(ref)
 	if err != nil {
-		return false
+		return err
+	}
+	if !p.IsCommand() {
+		return errors.New("importpath is not `package main`")
 	}
-	return p.IsCommand()
+	return nil
 }
 
 // importPackage wraps go/build.Import to handle go modules.

diff --git a/pkg/build/gobuild_test.go b/pkg/build/gobuild_test.go
@@ -49,8 +49,8 @@ func TestGoBuildIsSupportedRef(t *testing.T) {
 		"ko://github.com/google/ko/cmd/ko", // ko can build itself.
 	} {
 		t.Run(importpath, func(t *testing.T) {
-			if !ng.IsSupportedReference(importpath) {
-				t.Errorf("IsSupportedReference(%q) = false, want true", importpath)
+			if err := ng.IsSupportedReference(importpath); err != nil {
+				t.Errorf("IsSupportedReference(%q) = (%v), want nil", importpath, err)
 			}
 		})
 	}
@@ -61,8 +61,8 @@ func TestGoBuildIsSupportedRef(t *testing.T) {
 		"ko://github.com/google/ko/pkg/nonexistent", // does not exist.
 	} {
 		t.Run(importpath, func(t *testing.T) {
-			if ng.IsSupportedReference(importpath) {
-				t.Errorf("IsSupportedReference(%v) = true, want false", importpath)
+			if err := ng.IsSupportedReference(importpath); err == nil {
+				t.Errorf("IsSupportedReference(%v) = nil, want error", importpath)
 			}
 		})
 	}
@@ -110,8 +110,8 @@ func TestGoBuildIsSupportedRefWithModules(t *testing.T) {
 		"ko://github.com/some/module/cmd",       // ko can build commands in dependent modules
 	} {
 		t.Run(importpath, func(t *testing.T) {
-			if !ng.IsSupportedReference(importpath) {
-				t.Errorf("IsSupportedReference(%q) = false, want true", importpath)
+			if err := ng.IsSupportedReference(importpath); err != nil {
+				t.Errorf("IsSupportedReference(%q) = (%v), want nil", err, importpath)
 			}
 		})
 	}
@@ -123,8 +123,8 @@ func TestGoBuildIsSupportedRefWithModules(t *testing.T) {
 		"ko://github.com/google/ko/cmd/ko",          // not in this module.
 	} {
 		t.Run(importpath, func(t *testing.T) {
-			if ng.IsSupportedReference(importpath) {
-				t.Errorf("IsSupportedReference(%v) = true, want false", importpath)
+			if err := ng.IsSupportedReference(importpath); err == nil {
+				t.Errorf("IsSupportedReference(%v) = nil, want error", importpath)
 			}
 		})
 	}

diff --git a/pkg/build/limit.go b/pkg/build/limit.go
@@ -30,7 +30,7 @@ type Limiter struct {
 var _ Interface = (*Recorder)(nil)
 
 // IsSupportedReference implements Interface
-func (l *Limiter) IsSupportedReference(ip string) bool {
+func (l *Limiter) IsSupportedReference(ip string) error {
 	return l.Builder.IsSupportedReference(ip)
 }
 

diff --git a/pkg/build/limit_test.go b/pkg/build/limit_test.go
@@ -27,8 +27,8 @@ type sleeper struct{}
 var _ Interface = (*sleeper)(nil)
 
 // IsSupportedReference implements Interface
-func (r *sleeper) IsSupportedReference(ip string) bool {
-	return true
+func (r *sleeper) IsSupportedReference(ip string) error {
+	return nil
 }
 
 // Build implements Interface

diff --git a/pkg/build/recorder.go b/pkg/build/recorder.go
@@ -30,7 +30,7 @@ type Recorder struct {
 var _ Interface = (*Recorder)(nil)
 
 // IsSupportedReference implements Interface
-func (r *Recorder) IsSupportedReference(ip string) bool {
+func (r *Recorder) IsSupportedReference(ip string) error {
 	return r.Builder.IsSupportedReference(ip)
 }
 

diff --git a/pkg/build/recorder_test.go b/pkg/build/recorder_test.go
@@ -22,21 +22,17 @@ import (
 )
 
 type fake struct {
-	isr func(string) bool
+	isr func(string) error
 	b   func(string) (Result, error)
 }
 
 var _ Interface = (*fake)(nil)
 
 // IsSupportedReference implements Interface
-func (r *fake) IsSupportedReference(ip string) bool {
-	return r.isr(ip)
-}
+func (r *fake) IsSupportedReference(ip string) error { return r.isr(ip) }
 
 // Build implements Interface
-func (r *fake) Build(_ context.Context, ip string) (Result, error) {
-	return r.b(ip)
-}
+func (r *fake) Build(_ context.Context, ip string) (Result, error) { return r.b(ip) }
 
 func TestISRPassThrough(t *testing.T) {
 	tests := []struct {
@@ -53,12 +49,12 @@ func TestISRPassThrough(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			called := false
 			inner := &fake{
-				isr: func(ip string) bool {
+				isr: func(ip string) error {
 					called = true
 					if ip != test.input {
 						t.Errorf("ISR = %v, wanted %v", ip, test.input)
 					}
-					return true
+					return nil
 				},
 			}
 			rec := &Recorder{

diff --git a/pkg/build/shared.go b/pkg/build/shared.go
@@ -65,7 +65,7 @@ func (c *Caching) Build(ctx context.Context, ip string) (Result, error) {
 }
 
 // IsSupportedReference implements Interface
-func (c *Caching) IsSupportedReference(ip string) bool {
+func (c *Caching) IsSupportedReference(ip string) error {
 	return c.inner.IsSupportedReference(ip)
 }
 

diff --git a/pkg/build/shared_test.go b/pkg/build/shared_test.go
@@ -29,9 +29,7 @@ type slowbuild struct {
 // slowbuild implements Interface
 var _ Interface = (*slowbuild)(nil)
 
-func (sb *slowbuild) IsSupportedReference(string) bool {
-	return true
-}
+func (sb *slowbuild) IsSupportedReference(string) error { return nil }
 
 func (sb *slowbuild) Build(context.Context, string) (Result, error) {
 	time.Sleep(sb.sleep)
@@ -45,8 +43,8 @@ func TestCaching(t *testing.T) {
 	sb := &slowbuild{duration}
 	cb, _ := NewCaching(sb)
 
-	if !cb.IsSupportedReference(ip) {
-		t.Errorf("ISR(%q) = false, wanted true", ip)
+	if err := cb.IsSupportedReference(ip); err != nil {
+		t.Errorf("ISR(%q) = (%v), wanted nil", err, ip)
 	}
 
 	previousDigest := "not-a-digest"

diff --git a/pkg/commands/publisher.go b/pkg/commands/publisher.go
@@ -55,8 +55,8 @@ func publishImages(ctx context.Context, importpaths []string, pub publish.Interf
 			importpath = build.StrictScheme + importpath
 		}
 
-		if !b.IsSupportedReference(importpath) {
-			return nil, fmt.Errorf("importpath %q is not supported", importpath)
+		if err := b.IsSupportedReference(importpath); err != nil {
+			return nil, fmt.Errorf("importpath %q is not supported: %v", importpath, err)
 		}
 
 		img, err := b.Build(ctx, importpath)

diff --git a/pkg/internal/testing/fixed.go b/pkg/internal/testing/fixed.go
@@ -16,6 +16,7 @@ package testing
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 
@@ -36,10 +37,12 @@ func NewFixedBuild(entries map[string]build.Result) build.Interface {
 }
 
 // IsSupportedReference implements build.Interface
-func (f *fixedBuild) IsSupportedReference(s string) bool {
+func (f *fixedBuild) IsSupportedReference(s string) error {
 	s = strings.TrimPrefix(s, build.StrictScheme)
-	_, ok := f.entries[s]
-	return ok
+	if _, ok := f.entries[s]; !ok {
+		return errors.New("importpath is not supported")
+	}
+	return nil
 }
 
 // Build implements build.Interface

diff --git a/pkg/internal/testing/fixed_test.go b/pkg/internal/testing/fixed_test.go
@@ -67,17 +67,17 @@ func TestFixedBuild(t *testing.T) {
 		"asdf": testImage,
 	})
 
-	if got, want := f.IsSupportedReference("asdf"), true; got != want {
-		t.Errorf("IsSupportedReference(asdf) = %v, want %v", got, want)
+	if got := f.IsSupportedReference("asdf"); got != nil {
+		t.Errorf("IsSupportedReference(asdf) = (%v), want nil", got)
 	}
 	if got, err := f.Build(context.Background(), "asdf"); err != nil {
 		t.Errorf("Build(asdf) = %v, want %v", err, testImage)
 	} else if got != testImage {
 		t.Errorf("Build(asdf) = %v, want %v", got, testImage)
 	}
 
-	if got, want := f.IsSupportedReference("blah"), false; got != want {
-		t.Errorf("IsSupportedReference(blah) = %v, want %v", got, want)
+	if got := f.IsSupportedReference("blah"); got == nil {
+		t.Error("IsSupportedReference(blah) = nil, want error")
 	}
 	if got, err := f.Build(context.Background(), "blah"); err == nil {
 		t.Errorf("Build(blah) = %v, want error", got)

diff --git a/pkg/resolve/resolve.go b/pkg/resolve/resolve.go
@@ -41,7 +41,7 @@ func ImageReferences(ctx context.Context, docs []*yaml.Node, strict bool, builde
 		for node, ok := it(); ok; node, ok = it() {
 			ref := strings.TrimSpace(node.Value)
 
-			if builder.IsSupportedReference(ref) {
+			if err := builder.IsSupportedReference(ref); err == nil {
 				refs[ref] = append(refs[ref], node)
 			} else if strict {
 				return fmt.Errorf("found strict reference but %s is not a valid import path", ref)