Skip to content

Commit

Permalink
Always use "strict" mode
Browse files Browse the repository at this point in the history
We dropped this flag, so we're always in non-strict mode by default,
so if there is a ko:// reference that fails to build, we don't error
out. This drops some guards so that we are always in strict mode and
only build ko:// things.
  • Loading branch information
jonjohnsonjr committed Dec 22, 2020
1 parent 78b7bed commit 6586a72
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 73 deletions.
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

0 comments on commit 6586a72

Please sign in to comment.