Merge pull request #281 from jonjohnsonjr/strict-always

Always use "strict" mode

pkg/commands/apply.go

@@ -33,7 +33,6 @@ func addApply(topLevel *cobra.Command) {
 	po := &options.PublishOptions{}
 	fo := &options.FilenameOptions{}
 	so := &options.SelectorOptions{}
-	sto := &options.StrictOptions{}
 	bo := &options.BuildOptions{}
 	apply := &cobra.Command{
 		Use:   "apply -f FILENAME",
@@ -128,7 +127,7 @@ func addApply(topLevel *cobra.Command) {
 					stdin.Write([]byte("---\n"))
 				}
 				// Once primed kick things off.
-				return resolveFilesToWriter(ctx, builder, publisher, fo, so, sto, stdin)
+				return resolveFilesToWriter(ctx, builder, publisher, fo, so, stdin)
 			})
 
 			g.Go(func() error {

pkg/commands/create.go

@@ -33,7 +33,6 @@ func addCreate(topLevel *cobra.Command) {
 	po := &options.PublishOptions{}
 	fo := &options.FilenameOptions{}
 	so := &options.SelectorOptions{}
-	sto := &options.StrictOptions{}
 	bo := &options.BuildOptions{}
 	create := &cobra.Command{
 		Use:   "create -f FILENAME",
@@ -128,7 +127,7 @@ func addCreate(topLevel *cobra.Command) {
 					stdin.Write([]byte("---\n"))
 				}
 				// Once primed kick things off.
-				return resolveFilesToWriter(ctx, builder, publisher, fo, so, sto, stdin)
+				return resolveFilesToWriter(ctx, builder, publisher, fo, so, stdin)
 			})
 
 			g.Go(func() error {

pkg/commands/resolve.go

@@ -27,7 +27,6 @@ func addResolve(topLevel *cobra.Command) {
 	po := &options.PublishOptions{}
 	fo := &options.FilenameOptions{}
 	so := &options.SelectorOptions{}
-	sto := &options.StrictOptions{}
 	bo := &options.BuildOptions{}
 
 	resolve := &cobra.Command{
@@ -66,7 +65,7 @@ func addResolve(topLevel *cobra.Command) {
 				log.Fatalf("error creating publisher: %v", err)
 			}
 			defer publisher.Close()
-			if err := resolveFilesToWriter(ctx, builder, publisher, fo, so, sto, os.Stdout); err != nil {
+			if err := resolveFilesToWriter(ctx, builder, publisher, fo, so, os.Stdout); err != nil {
 				log.Fatal(err)
 			}
 		},

pkg/commands/resolver.go

@@ -234,7 +234,6 @@ func resolveFilesToWriter(
 	publisher publish.Interface,
 	fo *options.FilenameOptions,
 	so *options.SelectorOptions,
-	sto *options.StrictOptions,
 	out io.WriteCloser) error {
 	defer out.Close()
 
@@ -324,7 +323,7 @@ func resolveFilesToWriter(
 				recordingBuilder := &build.Recorder{
 					Builder: builder,
 				}
-				b, err := resolveFile(ctx, f, recordingBuilder, publisher, so, sto)
+				b, err := resolveFile(ctx, f, recordingBuilder, publisher, so)
 				if err != nil {
 					// This error is sometimes expected during watch mode, so this
 					// isn't fatal. Just print it and keep the watch open.
@@ -386,8 +385,7 @@ func resolveFile(
 	f string,
 	builder build.Interface,
 	pub publish.Interface,
-	so *options.SelectorOptions,
-	sto *options.StrictOptions) (b []byte, err error) {
+	so *options.SelectorOptions) (b []byte, err error) {
 
 	var selector labels.Selector
 	if so.Selector != "" {
@@ -435,7 +433,7 @@ func resolveFile(
 
 	}
 
-	if err := resolve.ImageReferences(ctx, docNodes, sto.Strict, builder, pub); err != nil {
+	if err := resolve.ImageReferences(ctx, docNodes, builder, pub); err != nil {
 		return nil, fmt.Errorf("error resolving image references: %v", err)
 	}
 

pkg/commands/resolver_test.go

@@ -58,7 +58,7 @@ func TestResolveMultiDocumentYAMLs(t *testing.T) {
 	buf := bytes.NewBuffer(nil)
 	encoder := yaml.NewEncoder(buf)
 	for _, input := range refs {
-		if err := encoder.Encode(input); err != nil {
+		if err := encoder.Encode(build.StrictScheme + input); err != nil {
 			t.Fatalf("Encode(%v) = %v", input, err)
 		}
 	}
@@ -70,8 +70,7 @@ func TestResolveMultiDocumentYAMLs(t *testing.T) {
 		yamlToTmpFile(t, buf.Bytes()),
 		testBuilder,
 		kotesting.NewFixedPublish(base, testHashes),
-		&options.SelectorOptions{},
-		&options.StrictOptions{})
+		&options.SelectorOptions{})
 
 	if err != nil {
 		t.Fatalf("ImageReferences(%v) = %v", string(inputYAML), err)
@@ -126,8 +125,7 @@ kind: Bar
 		kotesting.NewFixedPublish(base, testHashes),
 		&options.SelectorOptions{
 			Selector: "qux=baz",
-		},
-		&options.StrictOptions{})
+		})
 	if err != nil {
 		t.Fatalf("ImageReferences(%v) = %v", string(inputYAML), err)
 	}

pkg/resolve/resolve.go

@@ -31,21 +31,21 @@ import (
 // to published image digests.
 //
 // If a reference can be built and pushed, its yaml.Node will be mutated.
-func ImageReferences(ctx context.Context, docs []*yaml.Node, strict bool, builder build.Interface, publisher publish.Interface) error {
+func ImageReferences(ctx context.Context, docs []*yaml.Node, builder build.Interface, publisher publish.Interface) error {
 	// First, walk the input objects and collect a list of supported references
 	refs := make(map[string][]*yaml.Node)
 
 	for _, doc := range docs {
-		it := refsFromDoc(doc, strict)
+		it := refsFromDoc(doc)
 
 		for node, ok := it(); ok; node, ok = it() {
 			ref := strings.TrimSpace(node.Value)
 
-			if err := builder.IsSupportedReference(ref); err == nil {
-				refs[ref] = append(refs[ref], node)
-			} else if strict {
+			if err := builder.IsSupportedReference(ref); err != nil {
 				return fmt.Errorf("found strict reference but %s is not a valid import path: %v", ref, err)
 			}
+
+			refs[ref] = append(refs[ref], node)
 		}
 	}
 
@@ -87,14 +87,10 @@ func ImageReferences(ctx context.Context, docs []*yaml.Node, strict bool, builde
 	return nil
 }
 
-func refsFromDoc(doc *yaml.Node, strict bool) yit.Iterator {
+func refsFromDoc(doc *yaml.Node) yit.Iterator {
 	it := yit.FromNode(doc).
 		RecurseNodes().
 		Filter(yit.StringValue)
 
-	if strict {
-		return it.Filter(yit.WithPrefix(build.StrictScheme))
-	}
-
-	return it
+	return it.Filter(yit.WithPrefix(build.StrictScheme))
 }

pkg/resolve/resolve_test.go

@@ -17,7 +17,6 @@ package resolve
 import (
 	"bytes"
 	"context"
-	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -82,14 +81,17 @@ func TestYAMLArrays(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.desc, func(t *testing.T) {
-			inputStructured := test.refs
+			inputStructured := []string{}
+			for _, ref := range test.refs {
+				inputStructured = append(inputStructured, build.StrictScheme+ref)
+			}
 			inputYAML, err := yaml.Marshal(inputStructured)
 			if err != nil {
 				t.Fatalf("yaml.Marshal(%v) = %v", inputStructured, err)
 			}
 
 			doc := strToYAML(t, string(inputYAML))
-			err = ImageReferences(context.Background(), []*yaml.Node{doc}, false, testBuilder, kotesting.NewFixedPublish(test.base, testHashes))
+			err = ImageReferences(context.Background(), []*yaml.Node{doc}, testBuilder, kotesting.NewFixedPublish(test.base, testHashes))
 			if err != nil {
 				t.Fatalf("ImageReferences(%v) = %v", string(inputYAML), err)
 			}
@@ -124,17 +126,17 @@ func TestYAMLMaps(t *testing.T) {
 		expected map[string]string
 	}{{
 		desc:     "simple value",
-		input:    map[string]string{"image": fooRef},
+		input:    map[string]string{"image": build.StrictScheme + fooRef},
 		expected: map[string]string{"image": kotesting.ComputeDigest(base, fooRef, fooHash)},
 	}, {
 		desc:  "simple key",
-		input: map[string]string{bazRef: "blah"},
+		input: map[string]string{build.StrictScheme + bazRef: "blah"},
 		expected: map[string]string{
 			kotesting.ComputeDigest(base, bazRef, bazHash): "blah",
 		},
 	}, {
 		desc:  "key and value",
-		input: map[string]string{fooRef: barRef},
+		input: map[string]string{build.StrictScheme + fooRef: build.StrictScheme + barRef},
 		expected: map[string]string{
 			kotesting.ComputeDigest(base, fooRef, fooHash): kotesting.ComputeDigest(base, barRef, barHash),
 		},
@@ -145,8 +147,8 @@ func TestYAMLMaps(t *testing.T) {
 	}, {
 		desc: "multiple values",
 		input: map[string]string{
-			"arg1": fooRef,
-			"arg2": barRef,
+			"arg1": build.StrictScheme + fooRef,
+			"arg2": build.StrictScheme + barRef,
 		},
 		expected: map[string]string{
 			"arg1": kotesting.ComputeDigest(base, fooRef, fooHash),
@@ -163,7 +165,7 @@ func TestYAMLMaps(t *testing.T) {
 			}
 
 			doc := strToYAML(t, string(inputYAML))
-			err = ImageReferences(context.Background(), []*yaml.Node{doc}, false, testBuilder, kotesting.NewFixedPublish(base, testHashes))
+			err = ImageReferences(context.Background(), []*yaml.Node{doc}, testBuilder, kotesting.NewFixedPublish(base, testHashes))
 			if err != nil {
 				t.Fatalf("ImageReferences(%v) = %v", string(inputYAML), err)
 			}
@@ -203,23 +205,23 @@ func TestYAMLObject(t *testing.T) {
 		expected: &object{},
 	}, {
 		desc:     "string field",
-		input:    &object{S: fooRef},
+		input:    &object{S: build.StrictScheme + fooRef},
 		expected: &object{S: kotesting.ComputeDigest(base, fooRef, fooHash)},
 	}, {
 		desc:     "map field",
-		input:    &object{M: map[string]object{"blah": {S: fooRef}}},
+		input:    &object{M: map[string]object{"blah": {S: build.StrictScheme + fooRef}}},
 		expected: &object{M: map[string]object{"blah": {S: kotesting.ComputeDigest(base, fooRef, fooHash)}}},
 	}, {
 		desc:     "array field",
-		input:    &object{A: []object{{S: fooRef}}},
+		input:    &object{A: []object{{S: build.StrictScheme + fooRef}}},
 		expected: &object{A: []object{{S: kotesting.ComputeDigest(base, fooRef, fooHash)}}},
 	}, {
 		desc:     "pointer field",
-		input:    &object{P: &object{S: fooRef}},
+		input:    &object{P: &object{S: build.StrictScheme + fooRef}},
 		expected: &object{P: &object{S: kotesting.ComputeDigest(base, fooRef, fooHash)}},
 	}, {
 		desc:     "deep field",
-		input:    &object{M: map[string]object{"blah": {A: []object{{P: &object{S: fooRef}}}}}},
+		input:    &object{M: map[string]object{"blah": {A: []object{{P: &object{S: build.StrictScheme + fooRef}}}}}},
 		expected: &object{M: map[string]object{"blah": {A: []object{{P: &object{S: kotesting.ComputeDigest(base, fooRef, fooHash)}}}}}},
 	}}
 
@@ -232,7 +234,7 @@ func TestYAMLObject(t *testing.T) {
 			}
 
 			doc := strToYAML(t, string(inputYAML))
-			err = ImageReferences(context.Background(), []*yaml.Node{doc}, false, testBuilder, kotesting.NewFixedPublish(base, testHashes))
+			err = ImageReferences(context.Background(), []*yaml.Node{doc}, testBuilder, kotesting.NewFixedPublish(base, testHashes))
 			if err != nil {
 				t.Fatalf("ImageReferences(%v) = %v", string(inputYAML), err)
 			}
@@ -250,27 +252,27 @@ func TestYAMLObject(t *testing.T) {
 
 func TestStrict(t *testing.T) {
 	refs := []string{
-		build.StrictScheme + fooRef,
-		build.StrictScheme + barRef,
+		fooRef,
+		barRef,
 	}
 	buf := bytes.NewBuffer(nil)
 	encoder := yaml.NewEncoder(buf)
 	for _, input := range refs {
-		if err := encoder.Encode(input); err != nil {
+		if err := encoder.Encode(build.StrictScheme + input); err != nil {
 			t.Fatalf("Encode(%v) = %v", input, err)
 		}
 	}
 	base := mustRepository("gcr.io/multi-pass")
 	doc := strToYAML(t, buf.String())
 
-	err := ImageReferences(context.Background(), []*yaml.Node{doc}, true, testBuilder, kotesting.NewFixedPublish(base, testHashes))
+	err := ImageReferences(context.Background(), []*yaml.Node{doc}, testBuilder, kotesting.NewFixedPublish(base, testHashes))
 	if err != nil {
 		t.Fatalf("ImageReferences: %v", err)
 	}
 	t.Log(yamlToStr(t, doc))
 }
 
-func TestNoStrictKoPrefixRemains(t *testing.T) {
+func TestIsSupportedReferenceError(t *testing.T) {
 	ref := build.StrictScheme + fooRef
 
 	buf := bytes.NewBuffer(nil)
@@ -284,12 +286,9 @@ func TestNoStrictKoPrefixRemains(t *testing.T) {
 
 	noMatchBuilder := kotesting.NewFixedBuild(nil)
 
-	err := ImageReferences(context.Background(), []*yaml.Node{doc}, false, noMatchBuilder, kotesting.NewFixedPublish(base, testHashes))
-	if err != nil {
-		t.Fatalf("ImageReferences: %v", err)
-	}
-	if diff := cmp.Diff(ref, strings.TrimSpace(yamlToStr(t, doc))); diff != "" {
-		t.Errorf("expected the ko prefix to remain (-want,+got): %v", diff)
+	err := ImageReferences(context.Background(), []*yaml.Node{doc}, noMatchBuilder, kotesting.NewFixedPublish(base, testHashes))
+	if err == nil {
+		t.Fatal("ImageReferences should err, got nil")
 	}
 }