Skip to content

Commit

Permalink
Address feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
jonjohnsonjr committed Sep 24, 2020
1 parent 03a8e82 commit 1b78f95
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 3 deletions.
4 changes: 4 additions & 0 deletions pkg/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,7 @@ type Result interface {
Digest() (v1.Hash, error)
RawManifest() ([]byte, error)
}

// Assert that Image and ImageIndex implement Result.
var _ Result = (v1.Image)(nil)
var _ Result = (v1.ImageIndex)(nil)
9 changes: 7 additions & 2 deletions pkg/build/gobuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ func build(ctx context.Context, ip string, platform v1.Platform, disableOptimiza
cmd.Stderr = &output
cmd.Stdout = &output

log.Printf("Building %s", ip)
log.Printf("Building %s for %s", ip, platform.Architecture)
if err := cmd.Run(); err != nil {
os.RemoveAll(tmpDir)
log.Printf("Unexpected error running \"go build\": %v\n%v", err, output.String())
Expand Down Expand Up @@ -593,6 +593,7 @@ func (gb *gobuild) Build(ctx context.Context, s string) (Result, error) {
}
}

// TODO(#192): Do these in parallel?
func (gb *gobuild) buildAll(ctx context.Context, s string, base v1.ImageIndex) (v1.ImageIndex, error) {
im, err := base.IndexManifest()
if err != nil {
Expand All @@ -602,7 +603,11 @@ func (gb *gobuild) buildAll(ctx context.Context, s string, base v1.ImageIndex) (
// Build an image for each child from the base and append it to a new index to produce the result.
adds := []mutate.IndexAddendum{}
for _, desc := range im.Manifests {
// TODO(jonjohnsonjr): This would fail for a nested index, which is exceedingly rare.
// Nested index is pretty rare. We could support this in theory, but return an error for now.
if desc.MediaType != types.OCIManifestSchema1 && desc.MediaType != types.DockerManifestSchema2 {
return nil, fmt.Errorf("%q has unexpected mediaType %q in base for %q", desc.Digest, desc.MediaType, s)
}

base, err := base.Image(desc.Digest)
if err != nil {
return nil, err
Expand Down
33 changes: 33 additions & 0 deletions pkg/build/gobuild_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (
"time"

v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/empty"
"github.com/google/go-containerregistry/pkg/v1/mutate"
"github.com/google/go-containerregistry/pkg/v1/random"
)

Expand Down Expand Up @@ -500,3 +502,34 @@ func TestGoBuildIndex(t *testing.T) {
}
})
}

func TestNestedIndex(t *testing.T) {
baseLayers := int64(3)
images := int64(2)
base, err := random.Index(1024, baseLayers, images)
if err != nil {
t.Fatalf("random.Image() = %v", err)
}
importpath := "github.com/google/ko"

nestedBase := mutate.AppendManifests(empty.Index, mutate.IndexAddendum{Add: base})

creationTime := v1.Time{time.Unix(5000, 0)}
ng, err := NewGo(
WithCreationTime(creationTime),
WithBaseImages(func(string) (Result, error) { return nestedBase, nil }),
withBuilder(writeTempFile),
)
if err != nil {
t.Fatalf("NewGo() = %v", err)
}

_, err = ng.Build(context.Background(), filepath.Join(importpath, "cmd", "ko", "test"))
if err == nil {
t.Fatal("Build() expected err")
}

if !strings.Contains(err.Error(), "unexpected mediaType") {
t.Errorf("Build() expected unexpected mediaType error, got: %s", err)
}
}
1 change: 1 addition & 0 deletions pkg/build/recorder.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ func (r *Recorder) IsSupportedReference(ip string) bool {
return r.Builder.IsSupportedReference(ip)
}

// Build implements Interface
func (r *Recorder) Build(ctx context.Context, ip string) (Result, error) {
func() {
r.m.Lock()
Expand Down
14 changes: 14 additions & 0 deletions pkg/commands/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"log"
"os"
"os/signal"
"path"
"strconv"
"strings"
"syscall"
Expand All @@ -40,6 +41,16 @@ var (
)

func getBaseImage(platform string) build.GetBase {
// Default to linux/amd64 unless GOOS and GOARCH are set.
if platform == "" {
platform = "linux/amd64"

goos, goarch := os.Getenv("GOOS"), os.Getenv("GOARCH")
if goos != "" && goarch != "" {
platform = path.Join(goos, goarch)
}
}

return func(s string) (build.Result, error) {
// Viper configuration file keys are case insensitive, and are
// returned as all lowercase. This means that import paths with
Expand Down Expand Up @@ -70,6 +81,9 @@ func getBaseImage(platform string) build.GetBase {
if len(parts) > 2 {
p.Variant = parts[2]
}
if len(parts) > 3 {
return nil, fmt.Errorf("too many slashes in platform spec: %s", platform)
}
ropt = append(ropt, remote.WithPlatform(p))
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/options/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func AddBuildOptions(cmd *cobra.Command, bo *BuildOptions) {
"The maximum number of concurrent builds")
cmd.Flags().BoolVar(&bo.DisableOptimizations, "disable-optimizations", bo.DisableOptimizations,
"Disable optimizations when building Go code. Useful when you want to interactively debug the created container.")
cmd.Flags().StringVar(&bo.Platform, "platform", "linux/amd64",
cmd.Flags().StringVar(&bo.Platform, "platform", "",
"Which platform to use when pulling a multi-platform base. Format: all | <os>[/<arch>[/<variant>]]")

}

0 comments on commit 1b78f95

Please sign in to comment.