diff --git a/pkg/commands/apply.go b/pkg/commands/apply.go index 55f521ac5a..1365bfdd3e 100644 --- a/pkg/commands/apply.go +++ b/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 { diff --git a/pkg/commands/create.go b/pkg/commands/create.go index defb9e1cdc..e91fb6a8ec 100644 --- a/pkg/commands/create.go +++ b/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 { diff --git a/pkg/commands/options/strict.go b/pkg/commands/options/strict.go deleted file mode 100644 index 2aaf2f3521..0000000000 --- a/pkg/commands/options/strict.go +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright 2019 Google LLC All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package options - -// StrictOptions holds options to require strict references. -type StrictOptions struct { - Strict bool -} diff --git a/pkg/commands/resolve.go b/pkg/commands/resolve.go index 8b81e27416..b53c18c684 100644 --- a/pkg/commands/resolve.go +++ b/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) } }, diff --git a/pkg/commands/resolver.go b/pkg/commands/resolver.go index 4bb7a1fb14..4798f28f8c 100644 --- a/pkg/commands/resolver.go +++ b/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) } diff --git a/pkg/commands/resolver_test.go b/pkg/commands/resolver_test.go index 554dfeacc4..76ad033f8e 100644 --- a/pkg/commands/resolver_test.go +++ b/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) } diff --git a/pkg/resolve/resolve.go b/pkg/resolve/resolve.go index e4c66bb7e9..0b4c503d8a 100644 --- a/pkg/resolve/resolve.go +++ b/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)) } diff --git a/pkg/resolve/resolve_test.go b/pkg/resolve/resolve_test.go index 0cbfb0416b..7602b219cd 100644 --- a/pkg/resolve/resolve_test.go +++ b/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") } }