Repository: google/ko
Pull request #233 (2020-10-31 15:33:35)

IsSupportedReference returns descriptive error
This can be useful to determine what they need to do to make a ko
publish work.

codecov-io: # [Codecov](https://codecov.io/gh/google/ko/pull/233?src=pr&el=h1) Report
> Merging [#233](https://codecov.io/gh/google/ko/pull/233?src=pr&el=desc) into [master](https://codecov.io/gh/google/ko/commit/cde258294ff7be831199f61e6f6e8cc6ed17f564?el=desc) will **increase** coverage by `0.13%`.
> The diff coverage is `72.72%`.

[![Impacted file tree graph](https://codecov.io/gh/google/ko/pull/233/graphs/tree.svg?width=650&height=150&src=pr&token=UhqMbLrA3r)](https://codecov.io/gh/google/ko/pull/233?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #233      +/-   ##
==========================================
+ Coverage   36.64%   36.78%   +0.13%     
==========================================
  Files          33       33              
  Lines        1438     1441       +3     
==========================================
+ Hits          527      530       +3     
  Misses        822      822              
  Partials       89       89              
```


| [Impacted Files](https://codecov.io/gh/google/ko/pull/233?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/build/limit.go](https://codecov.io/gh/google/ko/pull/233/diff?src=pr&el=tree#diff-cGtnL2J1aWxkL2xpbWl0Lmdv) | `62.50% <ø> (ø)` | |
| [pkg/build/recorder.go](https://codecov.io/gh/google/ko/pull/233/diff?src=pr&el=tree#diff-cGtnL2J1aWxkL3JlY29yZGVyLmdv) | `100.00% <ø> (ø)` | |
| [pkg/build/shared.go](https://codecov.io/gh/google/ko/pull/233/diff?src=pr&el=tree#diff-cGtnL2J1aWxkL3NoYXJlZC5nbw==) | `100.00% <ø> (ø)` | |
| [pkg/commands/publisher.go](https://codecov.io/gh/google/ko/pull/233/diff?src=pr&el=tree#diff-cGtnL2NvbW1hbmRzL3B1Ymxpc2hlci5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/build/gobuild.go](https://codecov.io/gh/google/ko/pull/233/diff?src=pr&el=tree#diff-cGtnL2J1aWxkL2dvYnVpbGQuZ28=) | `58.15% <80.00%> (+0.29%)` | :arrow_up: |
| [pkg/internal/testing/fixed.go](https://codecov.io/gh/google/ko/pull/233/diff?src=pr&el=tree#diff-cGtnL2ludGVybmFsL3Rlc3RpbmcvZml4ZWQuZ28=) | `69.56% <100.00%> (+1.38%)` | :arrow_up: |
| [pkg/resolve/resolve.go](https://codecov.io/gh/google/ko/pull/233/diff?src=pr&el=tree#diff-cGtnL3Jlc29sdmUvcmVzb2x2ZS5nbw==) | `68.75% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/google/ko/pull/233?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/google/ko/pull/233?src=pr&el=footer). Last update [cde2582...0f4dfc8](https://codecov.io/gh/google/ko/pull/233?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/pkg/build/build.go b/pkg/build/build.go
index 692ceda5..3df5c605 100644
--- 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
index 7f6c33dc..545a2a30 100644
--- 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/limit.go b/pkg/build/limit.go
index 432cf680..8d67ec3a 100644
--- 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/recorder.go b/pkg/build/recorder.go
index 1a82de3b..847a0712 100644
--- 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/shared.go b/pkg/build/shared.go
index c6e70af5..6fab0202 100644
--- 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/commands/publisher.go b/pkg/commands/publisher.go
index 9b611d92..b40dadf5 100644
--- 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/resolve/resolve.go b/pkg/resolve/resolve.go
index 20c5e04e..d605c416 100644
--- 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)


Test patch:
diff --git a/pkg/build/gobuild_test.go b/pkg/build/gobuild_test.go
index 2eff3dc8..5ccbc2fa 100644
--- 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_test.go b/pkg/build/limit_test.go
index 1ec41ee7..d17a6ecd 100644
--- 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_test.go b/pkg/build/recorder_test.go
index 3ffcd841..b778ecc5 100644
--- 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_test.go b/pkg/build/shared_test.go
index 5c9577b2..d8e917dc 100644
--- 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/internal/testing/fixed.go b/pkg/internal/testing/fixed.go
index 0cad00d9..054caee4 100644
--- 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
index 4a0b1df9..4c7556a3 100644
--- a/pkg/internal/testing/fixed_test.go
+++ b/pkg/internal/testing/fixed_test.go
@@ -67,8 +67,8 @@ 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)
@@ -76,8 +76,8 @@ func TestFixedBuild(t *testing.T) {
 		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)