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 the publish.Namer, add --bare option for image naming #234

Merged
merged 3 commits into from
Nov 3, 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
27 changes: 19 additions & 8 deletions pkg/commands/options/publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,14 @@ type PublishOptions struct {
PreserveImportPaths bool
// BaseImportPaths uses the base path without MD5 hash after KO_DOCKER_REPO.
BaseImportPaths bool
// Base uses a tag on the KO_DOCKER_REPO without anything additional.
Bare bool
}

func AddPublishArg(cmd *cobra.Command, po *PublishOptions) {
cmd.Flags().StringSliceVarP(&po.Tags, "tags", "t", []string{"latest"},
"Which tags to use for the produced image instead of the default 'latest' tag.")
"Which tags to use for the produced image instead of the default 'latest' tag "+
"(may not work properly with --base-import-paths or --bare).")

cmd.Flags().BoolVar(&po.Push, "push", true, "Push images to KO_DOCKER_REPO")

Expand All @@ -60,28 +63,36 @@ func AddPublishArg(cmd *cobra.Command, po *PublishOptions) {
cmd.Flags().BoolVarP(&po.PreserveImportPaths, "preserve-import-paths", "P", po.PreserveImportPaths,
"Whether to preserve the full import path after KO_DOCKER_REPO.")
cmd.Flags().BoolVarP(&po.BaseImportPaths, "base-import-paths", "B", po.BaseImportPaths,
"Whether to use the base path without MD5 hash after KO_DOCKER_REPO.")
"Whether to use the base path without MD5 hash after KO_DOCKER_REPO (may not work properly with --tags).")
cmd.Flags().BoolVar(&po.Bare, "bare", po.Bare,
"Whether to just use KO_DOCKER_REPO without additional context (will not work properly with --tags).")
}

func packageWithMD5(importpath string) string {
func packageWithMD5(base, importpath string) string {
hasher := md5.New() //nolint: gosec // No strong cryptography needed.
hasher.Write([]byte(importpath))
return filepath.Base(importpath) + "-" + hex.EncodeToString(hasher.Sum(nil))
return filepath.Join(base, filepath.Base(importpath)+"-"+hex.EncodeToString(hasher.Sum(nil)))
}

func preserveImportPath(importpath string) string {
return importpath
func preserveImportPath(base, importpath string) string {
return filepath.Join(base, importpath)
}

func baseImportPaths(importpath string) string {
return filepath.Base(importpath)
func baseImportPaths(base, importpath string) string {
return filepath.Join(base, filepath.Base(importpath))
}

func bareDockerRepo(base, _ string) string {
return base
}

func MakeNamer(po *PublishOptions) publish.Namer {
if po.PreserveImportPaths {
return preserveImportPath
} else if po.BaseImportPaths {
return baseImportPaths
} else if po.Bare {
return bareDockerRepo
}
return packageWithMD5
}
2 changes: 1 addition & 1 deletion pkg/commands/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ func (n nopPublisher) Publish(br build.Result, s string) (name.Reference, error)
if err != nil {
return nil, err
}
return name.NewDigest(fmt.Sprintf("%s/%s@%s", n.repoName, n.namer(s), h))
return name.NewDigest(fmt.Sprintf("%s@%s", n.namer(n.repoName, s), h))
}

func (n nopPublisher) Close() error { return nil }
Expand Down
4 changes: 2 additions & 2 deletions pkg/publish/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (d *demon) Publish(br build.Result, s string) (name.Reference, error) {
return nil, err
}

digestTag, err := name.NewTag(fmt.Sprintf("%s/%s:%s", LocalDomain, d.namer(s), h.Hex))
digestTag, err := name.NewTag(fmt.Sprintf("%s:%s", d.namer(LocalDomain, s), h.Hex))
if err != nil {
return nil, err
}
Expand All @@ -106,7 +106,7 @@ func (d *demon) Publish(br build.Result, s string) (name.Reference, error) {

for _, tagName := range d.tags {
log.Printf("Adding tag %v", tagName)
tag, err := name.NewTag(fmt.Sprintf("%s/%s:%s", LocalDomain, d.namer(s), tagName))
tag, err := name.NewTag(fmt.Sprintf("%s:%s", d.namer(LocalDomain, s), tagName))
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/publish/daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func TestDaemon(t *testing.T) {
def := NewDaemon(md5Hash, []string{})
if d, err := def.Publish(img, importpath); err != nil {
t.Errorf("Publish() = %v", err)
} else if got, want := d.String(), "ko.local/"+md5Hash(importpath); !strings.HasPrefix(got, want) {
} else if got, want := d.String(), md5Hash("ko.local", importpath); !strings.HasPrefix(got, want) {
t.Errorf("Publish() = %v, wanted prefix %v", got, want)
}
}
Expand All @@ -74,7 +74,7 @@ func TestDaemonTags(t *testing.T) {
def := NewDaemon(md5Hash, []string{"v2.0.0", "v1.2.3", "production"})
if d, err := def.Publish(img, importpath); err != nil {
t.Errorf("Publish() = %v", err)
} else if got, want := d.String(), "ko.local/"+md5Hash(importpath); !strings.HasPrefix(got, want) {
} else if got, want := d.String(), md5Hash("ko.local", importpath); !strings.HasPrefix(got, want) {
t.Errorf("Publish() = %v, wanted prefix %v", got, want)
}

Expand Down
11 changes: 6 additions & 5 deletions pkg/publish/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"fmt"
"log"
"net/http"
"path/filepath"
"strings"

"github.com/google/go-containerregistry/pkg/authn"
Expand Down Expand Up @@ -52,13 +53,13 @@ type defaultOpener struct {

// Namer is a function from a supported import path to the portion of the resulting
// image name that follows the "base" repository name.
type Namer func(string) string
type Namer func(string, string) string

// identity is the default namer, so import paths are affixed as-is under the repository
// name for maximum clarity, e.g.
// gcr.io/foo/github.com/bar/baz/cmd/blah
// ^--base--^ ^-------import path-------^
func identity(in string) string { return in }
func identity(base, in string) string { return filepath.Join(base, in) }

// As some registries do not support pushing an image by digest, the default tag for pushing
// is the 'latest' tag.
Expand Down Expand Up @@ -132,7 +133,7 @@ func (d *defalt) Publish(br build.Result, s string) (name.Reference, error) {
}

for i, tagName := range d.tags {
tag, err := name.NewTag(fmt.Sprintf("%s/%s:%s", d.base, d.namer(s), tagName), no...)
tag, err := name.NewTag(fmt.Sprintf("%s:%s", d.namer(d.base, s), tagName), no...)
if err != nil {
return nil, err
}
Expand All @@ -154,11 +155,11 @@ func (d *defalt) Publish(br build.Result, s string) (name.Reference, error) {
if err != nil {
return nil, err
}
ref := fmt.Sprintf("%s/%s@%s", d.base, d.namer(s), h)
ref := fmt.Sprintf("%s@%s", d.namer(d.base, s), h)
if len(d.tags) == 1 && d.tags[0] != defaultTags[0] {
// If a single tag is explicitly set (not latest), then this
// is probably a release, so include the tag in the reference.
ref = fmt.Sprintf("%s/%s:%s@%s", d.base, d.namer(s), d.tags[0], h)
ref = fmt.Sprintf("%s:%s@%s", d.namer(d.base, s), d.tags[0], h)
}
dig, err := name.NewDigest(ref)
if err != nil {
Expand Down
13 changes: 7 additions & 6 deletions pkg/publish/default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"net/http"
"net/http/httptest"
"net/url"
"path/filepath"
"strings"
"testing"

Expand Down Expand Up @@ -66,11 +67,11 @@ func TestDefault(t *testing.T) {
}
}

func md5Hash(s string) string {
func md5Hash(base, s string) string {
// md5 as hex.
hasher := md5.New()
hasher.Write([]byte(s))
return hex.EncodeToString(hasher.Sum(nil))
return filepath.Join(base, hex.EncodeToString(hasher.Sum(nil)))
}

func TestDefaultWithCustomNamer(t *testing.T) {
Expand Down Expand Up @@ -100,8 +101,8 @@ func TestDefaultWithCustomNamer(t *testing.T) {
t.Errorf("Publish() = %v", err)
} else if !strings.HasPrefix(d.String(), repoName) {
t.Errorf("Publish() = %v, wanted prefix %v", d, tag.Repository)
} else if !strings.HasSuffix(d.Context().String(), md5Hash(strings.ToLower(importpath))) {
t.Errorf("Publish() = %v, wanted suffix %v", d.Context(), md5Hash(importpath))
} else if !strings.HasSuffix(d.Context().String(), md5Hash("", strings.ToLower(importpath))) {
t.Errorf("Publish() = %v, wanted suffix %v", d.Context(), md5Hash("", importpath))
}
}
}
Expand Down Expand Up @@ -133,7 +134,7 @@ func TestDefaultWithTags(t *testing.T) {
} else if !strings.HasPrefix(d.String(), repoName) {
t.Errorf("Publish() = %v, wanted prefix %v", d, tag.Repository)
} else if !strings.HasSuffix(d.Context().String(), strings.ToLower(importpath)) {
t.Errorf("Publish() = %v, wanted suffix %v", d.Context(), md5Hash(importpath))
t.Errorf("Publish() = %v, wanted suffix %v", d.Context(), md5Hash("", importpath))
}

otherTag := fmt.Sprintf("%s/%s:v1.2.3", u.Host, expectedRepo)
Expand Down Expand Up @@ -214,7 +215,7 @@ func TestDefaultWithReleaseTag(t *testing.T) {
} else if !strings.HasPrefix(d.String(), repoName) {
t.Errorf("Publish() = %v, wanted prefix %v", d, tag.Repository)
} else if !strings.HasSuffix(d.Context().String(), strings.ToLower(importpath)) {
t.Errorf("Publish() = %v, wanted suffix %v", d.Context(), md5Hash(importpath))
t.Errorf("Publish() = %v, wanted suffix %v", d.Context(), md5Hash("", importpath))
} else if !strings.Contains(d.String(), releaseTag) {
t.Errorf("Publish() = %v, wanted tag included: %v", d.String(), releaseTag)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/publish/kind.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (t *kindPublisher) Publish(br build.Result, s string) (name.Reference, erro
return nil, err
}

digestTag, err := name.NewTag(fmt.Sprintf("%s/%s:%s", KindDomain, t.namer(s), h.Hex))
digestTag, err := name.NewTag(fmt.Sprintf("%s:%s", t.namer(KindDomain, s), h.Hex))
if err != nil {
return nil, err
}
Expand All @@ -108,7 +108,7 @@ func (t *kindPublisher) Publish(br build.Result, s string) (name.Reference, erro

for _, tagName := range t.tags {
log.Printf("Adding tag %v", tagName)
tag, err := name.NewTag(fmt.Sprintf("%s/%s:%s", KindDomain, t.namer(s), tagName))
tag, err := name.NewTag(fmt.Sprintf("%s:%s", t.namer(KindDomain, s), tagName))
if err != nil {
return nil, err
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/publish/tarball.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (t *tar) Publish(br build.Result, s string) (name.Reference, error) {
}

for _, tagName := range t.tags {
tag, err := name.ParseReference(fmt.Sprintf("%s/%s:%s", t.base, t.namer(s), tagName))
tag, err := name.ParseReference(fmt.Sprintf("%s:%s", t.namer(t.base, s), tagName))
if err != nil {
return nil, err
}
Expand All @@ -70,18 +70,18 @@ func (t *tar) Publish(br build.Result, s string) (name.Reference, error) {
}

if len(t.tags) == 0 {
ref, err := name.ParseReference(fmt.Sprintf("%s/%s@%s", t.base, t.namer(s), h))
ref, err := name.ParseReference(fmt.Sprintf("%s@%s", t.namer(t.base, s), h))
if err != nil {
return nil, err
}
t.refs[ref] = img
}

ref := fmt.Sprintf("%s/%s@%s", t.base, t.namer(s), h)
ref := fmt.Sprintf("%s@%s", t.namer(t.base, s), h)
if len(t.tags) == 1 && t.tags[0] != defaultTags[0] {
// If a single tag is explicitly set (not latest), then this
// is probably a release, so include the tag in the reference.
ref = fmt.Sprintf("%s/%s:%s@%s", t.base, t.namer(s), t.tags[0], h)
ref = fmt.Sprintf("%s:%s@%s", t.namer(t.base, s), t.tags[0], h)
}
dig, err := name.NewDigest(ref)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/publish/tarball_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func TestTarball(t *testing.T) {
defer fp.Close()
defer os.Remove(fp.Name())

expectedRepo := fmt.Sprintf("%s/%s", base, md5Hash(strings.ToLower(importpath)))
expectedRepo := md5Hash(base, strings.ToLower(importpath))

tag, err := name.NewTag(fmt.Sprintf("%s/%s:latest", "example.com", expectedRepo))
if err != nil {
Expand Down