Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always use "strict" mode #281

Merged
merged 1 commit into from
Dec 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions pkg/commands/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 1 addition & 2 deletions pkg/commands/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 {
Expand Down
20 changes: 0 additions & 20 deletions pkg/commands/options/strict.go

This file was deleted.

3 changes: 1 addition & 2 deletions pkg/commands/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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)
}
},
Expand Down
8 changes: 3 additions & 5 deletions pkg/commands/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 != "" {
Expand Down Expand Up @@ -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)
}

Expand Down
8 changes: 3 additions & 5 deletions pkg/commands/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand All @@ -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)
Expand Down Expand Up @@ -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)
}
Expand Down
18 changes: 7 additions & 11 deletions pkg/resolve/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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))
}
51 changes: 25 additions & 26 deletions pkg/resolve/resolve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package resolve
import (
"bytes"
"context"
"strings"
"testing"

"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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),
},
Expand All @@ -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),
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)}}}}}},
}}

Expand All @@ -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)
}
Expand All @@ -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)
Expand All @@ -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")
}
}

Expand Down