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

Refactor how/where ko:// is handled. #153

Merged
merged 1 commit into from
Apr 30, 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
48 changes: 28 additions & 20 deletions pkg/build/gobuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,32 +122,38 @@ 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 {
p, err := g.importPackage(s)
if err != nil {
ref := newRef(s)
if p, err := g.importPackage(ref); err != nil {
if ref.IsStrict() {
log.Fatalf("%q is not supported: %v", ref.String(), err)
}
return false
} else if p.IsCommand() {
return true
} else if ref.IsStrict() {
log.Fatalf(`%q does not have "package main"`, ref.String())
}
return p.IsCommand()
return false
}

var moduleErr = errors.New("unmatched importPackage with gomodules")

// importPackage wraps go/build.Import to handle go modules.
//
// Note that we will fall back to GOPATH if the project isn't using go modules.
func (g *gobuild) importPackage(s string) (*gb.Package, error) {
func (g *gobuild) importPackage(ref reference) (*gb.Package, error) {
if g.mod == nil {
return gb.Import(s, gb.Default.GOPATH, gb.ImportComment)
return gb.Import(ref.Path(), gb.Default.GOPATH, gb.ImportComment)
}

// If we're inside a go modules project, try to use the module's directory
// as our source root to import:
// * any strict reference we get
// * paths that match module path prefix (they should be in this project)
// * relative paths (they should also be in this project)
if strings.HasPrefix(s, g.mod.Path) || gb.IsLocalImport(s) {
return gb.Import(s, g.mod.Dir, gb.ImportComment)
if ref.IsStrict() || strings.HasPrefix(ref.Path(), g.mod.Path) || gb.IsLocalImport(ref.Path()) {
return gb.Import(ref.Path(), g.mod.Dir, gb.ImportComment)
}

return nil, moduleErr
return nil, fmt.Errorf("unmatched importPackage %q with gomodules", ref.String())
}

func build(ctx context.Context, ip string, platform v1.Platform, disableOptimizations bool) (string, error) {
Expand Down Expand Up @@ -273,8 +279,8 @@ func tarBinary(name, binary string) (*bytes.Buffer, error) {
return buf, nil
}

func (g *gobuild) kodataPath(s string) (string, error) {
p, err := g.importPackage(s)
func (g *gobuild) kodataPath(ref reference) (string, error) {
p, err := g.importPackage(ref)
if err != nil {
return "", err
}
Expand Down Expand Up @@ -348,7 +354,7 @@ func walkRecursive(tw *tar.Writer, root, chroot string) error {
})
}

func (g *gobuild) tarKoData(importpath string) (*bytes.Buffer, error) {
func (g *gobuild) tarKoData(ref reference) (*bytes.Buffer, error) {
buf := bytes.NewBuffer(nil)
// Compress this before calling tarball.LayerFromOpener, since it eagerly
// calculates digests and diffids. This prevents us from double compressing
Expand All @@ -360,7 +366,7 @@ func (g *gobuild) tarKoData(importpath string) (*bytes.Buffer, error) {
tw := tar.NewWriter(gw)
defer tw.Close()

root, err := g.kodataPath(importpath)
root, err := g.kodataPath(ref)
if err != nil {
return nil, err
}
Expand All @@ -370,8 +376,10 @@ func (g *gobuild) tarKoData(importpath string) (*bytes.Buffer, error) {

// Build implements build.Interface
func (gb *gobuild) Build(ctx context.Context, s string) (v1.Image, error) {
ref := newRef(s)

// Determine the appropriate base image for this import path.
base, err := gb.getBase(s)
base, err := gb.getBase(ref.Path())
if err != nil {
return nil, err
}
Expand All @@ -385,15 +393,15 @@ func (gb *gobuild) Build(ctx context.Context, s string) (v1.Image, error) {
}

// Do the build into a temporary file.
file, err := gb.build(ctx, s, platform, gb.disableOptimizations)
file, err := gb.build(ctx, ref.Path(), platform, gb.disableOptimizations)
if err != nil {
return nil, err
}
defer os.RemoveAll(filepath.Dir(file))

var layers []mutate.Addendum
// Create a layer from the kodata directory under this import path.
dataLayerBuf, err := gb.tarKoData(s)
dataLayerBuf, err := gb.tarKoData(ref)
if err != nil {
return nil, err
}
Expand All @@ -408,12 +416,12 @@ func (gb *gobuild) Build(ctx context.Context, s string) (v1.Image, error) {
Layer: dataLayer,
History: v1.History{
Author: "ko",
CreatedBy: "ko publish " + s,
CreatedBy: "ko publish " + ref.String(),
Comment: "kodata contents, at $KO_DATA_PATH",
},
})

appPath := path.Join(appDir, appFilename(s))
appPath := path.Join(appDir, appFilename(ref.Path()))

// Construct a tarball with the binary and produce a layer.
binaryLayerBuf, err := tarBinary(appPath, file)
Expand All @@ -431,7 +439,7 @@ func (gb *gobuild) Build(ctx context.Context, s string) (v1.Image, error) {
Layer: binaryLayer,
History: v1.History{
Author: "ko",
CreatedBy: "ko publish " + s,
CreatedBy: "ko publish " + ref.String(),
Comment: "go build output, at " + appPath,
},
})
Expand Down
49 changes: 49 additions & 0 deletions pkg/build/strict.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2020 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 build

import "strings"

// StrictScheme is a prefix that can be placed on import paths that users
// think MUST be supported references.
const StrictScheme = "ko://"

type reference struct {
strict bool
path string
}

func newRef(s string) reference {
return reference{
strict: strings.HasPrefix(s, StrictScheme),
path: strings.TrimPrefix(s, StrictScheme),
}
}

func (r reference) IsStrict() bool {
return r.strict
}

func (r reference) Path() string {
return r.path
}

func (r reference) String() string {
if r.IsStrict() {
return StrictScheme + r.Path()
} else {
return r.Path()
}
}
51 changes: 51 additions & 0 deletions pkg/build/strict_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright 2020 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 build

import "testing"

func TestStrictReference(t *testing.T) {
tests := []struct {
name string
input string
strict bool
path string
}{{
name: "loose",
input: "github.com/foo/bar",
strict: false,
path: "github.com/foo/bar",
}, {
name: "strict",
input: "ko://github.com/foo/bar",
strict: true,
path: "github.com/foo/bar",
}}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
ref := newRef(test.input)
if got, want := ref.IsStrict(), test.strict; got != want {
t.Errorf("got: %v, want: %v", got, want)
}
if got, want := ref.Path(), test.path; got != want {
t.Errorf("got: %v, want: %v", got, want)
}
if got, want := ref.String(), test.input; got != want {
t.Errorf("got: %v, want: %v", got, want)
}
})
}
}
4 changes: 4 additions & 0 deletions pkg/internal/testing/fixed.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package testing
import (
"context"
"fmt"
"strings"

"github.com/google/go-containerregistry/pkg/name"
v1 "github.com/google/go-containerregistry/pkg/v1"
Expand All @@ -36,12 +37,14 @@ func NewFixedBuild(entries map[string]v1.Image) build.Interface {

// IsSupportedReference implements build.Interface
func (f *fixedBuild) IsSupportedReference(s string) bool {
s = strings.TrimPrefix(s, build.StrictScheme)
_, ok := f.entries[s]
return ok
}

// Build implements build.Interface
func (f *fixedBuild) Build(_ context.Context, s string) (v1.Image, error) {
s = strings.TrimPrefix(s, build.StrictScheme)
if img, ok := f.entries[s]; ok {
return img, nil
}
Expand All @@ -61,6 +64,7 @@ func NewFixedPublish(base name.Repository, entries map[string]v1.Hash) publish.I

// Publish implements publish.Interface
func (f *fixedPublish) Publish(_ v1.Image, s string) (name.Reference, error) {
s = strings.TrimPrefix(s, build.StrictScheme)
h, ok := f.entries[s]
if !ok {
return nil, fmt.Errorf("unsupported importpath: %q", s)
Expand Down
9 changes: 3 additions & 6 deletions pkg/resolve/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ import (
"gopkg.in/yaml.v3"
)

const koPrefix = "ko://"

// ImageReferences resolves supported references to images within the input yaml
// to published image digests.
//
Expand All @@ -42,10 +40,9 @@ func ImageReferences(ctx context.Context, docs []*yaml.Node, strict bool, builde

for node, ok := it(); ok; node, ok = it() {
ref := strings.TrimSpace(node.Value)
tref := strings.TrimPrefix(ref, koPrefix)

if builder.IsSupportedReference(tref) {
refs[tref] = append(refs[tref], node)
if builder.IsSupportedReference(ref) {
refs[ref] = append(refs[ref], node)
} else if strict {
return fmt.Errorf("found strict reference but %s is not a valid import path", ref)
}
Expand Down Expand Up @@ -96,7 +93,7 @@ func refsFromDoc(doc *yaml.Node, strict bool) yit.Iterator {
Filter(yit.StringValue)

if strict {
return it.Filter(yit.WithPrefix(koPrefix))
return it.Filter(yit.WithPrefix(build.StrictScheme))
}

return it
Expand Down
7 changes: 4 additions & 3 deletions pkg/resolve/resolve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/google/go-containerregistry/pkg/name"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/random"
"github.com/google/ko/pkg/build"
kotesting "github.com/google/ko/pkg/internal/testing"
"gopkg.in/yaml.v3"
)
Expand Down Expand Up @@ -249,8 +250,8 @@ func TestYAMLObject(t *testing.T) {

func TestStrict(t *testing.T) {
refs := []string{
"ko://" + fooRef,
"ko://" + barRef,
build.StrictScheme + fooRef,
build.StrictScheme + barRef,
}
buf := bytes.NewBuffer(nil)
encoder := yaml.NewEncoder(buf)
Expand All @@ -270,7 +271,7 @@ func TestStrict(t *testing.T) {
}

func TestNoStrictKoPrefixRemains(t *testing.T) {
ref := "ko://" + fooRef
ref := build.StrictScheme + fooRef

buf := bytes.NewBuffer(nil)
encoder := yaml.NewEncoder(buf)
Expand Down