fix: consider base image media type when appending layers (#1437)

thesayyn · web-flow · commit 2b54510be740 · 2022-08-17T14:00:20.000-04:00
* fix: consider base image media type when appending layers

* fix docs

* fix unit test

* adress changes
diff --git a/cmd/crane/cmd/append.go b/cmd/crane/cmd/append.go
@@ -23,6 +23,7 @@ import (
 	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/types"
 	specsv1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/spf13/cobra"
 )
@@ -31,7 +32,7 @@ import (
 func NewCmdAppend(options *[]crane.Option) *cobra.Command {
 	var baseRef, newTag, outFile string
 	var newLayers []string
-	var annotate bool
+	var annotate, ociEmptyBase bool
 
 	appendCmd := &cobra.Command{
 		Use:   "append",
@@ -51,6 +52,10 @@ container image.`,
 			if baseRef == "" {
 				logs.Warn.Printf("base unspecified, using empty image")
 				base = empty.Image
+				if ociEmptyBase {
+					base = mutate.MediaType(base, types.OCIManifestSchema1)
+					base = mutate.ConfigMediaType(base, types.OCIConfigJSON)
+				}
 			} else {
 				base, err = crane.Pull(baseRef, *options...)
 				if err != nil {
@@ -108,7 +113,9 @@ container image.`,
 	appendCmd.Flags().StringSliceVarP(&newLayers, "new_layer", "f", []string{}, "Path to tarball to append to image")
 	appendCmd.Flags().StringVarP(&outFile, "output", "o", "", "Path to new tarball of resulting image")
 	appendCmd.Flags().BoolVar(&annotate, "set-base-image-annotations", false, "If true, annotate the resulting image as being based on the base image")
+	appendCmd.Flags().BoolVar(&ociEmptyBase, "oci-empty-base", false, "If true, empty base image will have OCI media types instead of Docker")
 
+	appendCmd.MarkFlagsMutuallyExclusive("oci-empty-base", "base")
 	appendCmd.MarkFlagRequired("new_tag")
 	appendCmd.MarkFlagRequired("new_layer")
 	return appendCmd
diff --git a/cmd/crane/doc/crane_append.md b/cmd/crane/doc/crane_append.md
diff --git a/pkg/crane/append.go b/pkg/crane/append.go
@@ -23,12 +23,10 @@ import (
 	"github.com/google/go-containerregistry/pkg/v1/mutate"
 	"github.com/google/go-containerregistry/pkg/v1/stream"
 	"github.com/google/go-containerregistry/pkg/v1/tarball"
+	"github.com/google/go-containerregistry/pkg/v1/types"
 )
 
 func isWindows(img v1.Image) (bool, error) {
-	if img == nil {
-		return false, nil
-	}
 	cfg, err := img.ConfigFile()
 	if err != nil {
 		return false, err
@@ -42,14 +40,30 @@ func isWindows(img v1.Image) (bool, error) {
 // "windows"), the contents of the tarballs will be modified to be suitable for
 // a Windows container image.`,
 func Append(base v1.Image, paths ...string) (v1.Image, error) {
+	if base == nil {
+		return nil, fmt.Errorf("invalid argument: base")
+	}
+
 	win, err := isWindows(base)
 	if err != nil {
 		return nil, fmt.Errorf("getting base image: %w", err)
 	}
 
+	baseMediaType, err := base.MediaType()
+
+	if err != nil {
+		return nil, fmt.Errorf("getting base image media type: %w", err)
+	}
+
+	layerType := types.DockerLayer
+
+	if baseMediaType == types.OCIManifestSchema1 {
+		layerType = types.OCILayer
+	}
+
 	layers := make([]v1.Layer, 0, len(paths))
 	for _, path := range paths {
-		layer, err := getLayer(path)
+		layer, err := getLayer(path, layerType)
 		if err != nil {
 			return nil, fmt.Errorf("reading layer %q: %w", path, err)
 		}
@@ -67,16 +81,16 @@ func Append(base v1.Image, paths ...string) (v1.Image, error) {
 	return mutate.AppendLayers(base, layers...)
 }
 
-func getLayer(path string) (v1.Layer, error) {
+func getLayer(path string, layerType types.MediaType) (v1.Layer, error) {
 	f, err := streamFile(path)
 	if err != nil {
 		return nil, err
 	}
 	if f != nil {
-		return stream.NewLayer(f), nil
+		return stream.NewLayer(f, stream.WithMediaType(layerType)), nil
 	}
 
-	return tarball.LayerFromFile(path)
+	return tarball.LayerFromFile(path, tarball.WithMediaType(layerType))
 }
 
 // If we're dealing with a named pipe, trying to open it multiple times will
diff --git a/pkg/crane/append_test.go b/pkg/crane/append_test.go
@@ -0,0 +1,73 @@
+// Copyright 2022 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 crane_test
+
+import (
+	"testing"
+
+	"github.com/google/go-containerregistry/pkg/crane"
+	"github.com/google/go-containerregistry/pkg/v1/empty"
+	"github.com/google/go-containerregistry/pkg/v1/mutate"
+	"github.com/google/go-containerregistry/pkg/v1/types"
+)
+
+func TestAppendWithOCIBaseImage(t *testing.T) {
+	base := mutate.MediaType(empty.Image, types.OCIManifestSchema1)
+	img, err := crane.Append(base, "testdata/content.tar")
+
+	if err != nil {
+		t.Fatalf("crane.Append(): %v", err)
+	}
+
+	layers, err := img.Layers()
+
+	if err != nil {
+		t.Fatalf("img.Layers(): %v", err)
+	}
+
+	mediaType, err := layers[0].MediaType()
+
+	if err != nil {
+		t.Fatalf("layers[0].MediaType(): %v", err)
+	}
+
+	if got, want := mediaType, types.OCILayer; got != want {
+		t.Errorf("MediaType(): want %q, got %q", want, got)
+	}
+}
+
+func TestAppendWithDockerBaseImage(t *testing.T) {
+	img, err := crane.Append(empty.Image, "testdata/content.tar")
+
+	if err != nil {
+		t.Fatalf("crane.Append(): %v", err)
+	}
+
+	layers, err := img.Layers()
+
+	if err != nil {
+		t.Fatalf("img.Layers(): %v", err)
+	}
+
+	mediaType, err := layers[0].MediaType()
+
+	if err != nil {
+		t.Fatalf("layers[0].MediaType(): %v", err)
+	}
+
+	if got, want := mediaType, types.DockerLayer; got != want {
+		t.Errorf("MediaType(): want %q, got %q", want, got)
+	}
+}
diff --git a/pkg/crane/testdata/content.tar b/pkg/crane/testdata/content.tar
diff --git a/pkg/v1/stream/layer.go b/pkg/v1/stream/layer.go
@@ -48,6 +48,7 @@ type Layer struct {
 	mu             sync.Mutex
 	digest, diffID *v1.Hash
 	size           int64
+	mediaType      types.MediaType
 }
 
 var _ v1.Layer = (*Layer)(nil)
@@ -62,11 +63,21 @@ func WithCompressionLevel(level int) LayerOption {
 	}
 }
 
+// WithMediaType is a functional option for overriding the layer's media type.
+func WithMediaType(mt types.MediaType) LayerOption {
+	return func(l *Layer) {
+		l.mediaType = mt
+	}
+}
+
 // NewLayer creates a Layer from an io.ReadCloser.
 func NewLayer(rc io.ReadCloser, opts ...LayerOption) *Layer {
 	layer := &Layer{
 		blob:        rc,
 		compression: gzip.BestSpeed,
+		// We use DockerLayer for now as uncompressed layers
+		// are unimplemented
+		mediaType: types.DockerLayer,
 	}
 
 	for _, opt := range opts {
@@ -108,9 +119,7 @@ func (l *Layer) Size() (int64, error) {
 
 // MediaType implements v1.Layer
 func (l *Layer) MediaType() (types.MediaType, error) {
-	// We return DockerLayer for now as uncompressed layers
-	// are unimplemented
-	return types.DockerLayer, nil
+	return l.mediaType, nil
 }
 
 // Uncompressed implements v1.Layer.
diff --git a/pkg/v1/stream/layer_test.go b/pkg/v1/stream/layer_test.go
@@ -284,3 +284,16 @@ func TestMediaType(t *testing.T) {
 		t.Errorf("MediaType(): want %q, got %q", want, got)
 	}
 }
+
+func TestMediaTypeOption(t *testing.T) {
+	l := NewLayer(ioutil.NopCloser(strings.NewReader("hello")), WithMediaType(types.OCILayer))
+	mediaType, err := l.MediaType()
+
+	if err != nil {
+		t.Fatalf("MediaType(): %v", err)
+	}
+
+	if got, want := mediaType, types.OCILayer; got != want {
+		t.Errorf("MediaType(): want %q, got %q", want, got)
+	}
+}
