From f2332aa14b4826e837fbaf9a4211e80899c5c0e8 Mon Sep 17 00:00:00 2001 From: Logan Price Date: Fri, 1 Sep 2023 14:10:22 +0000 Subject: [PATCH] feat: ensure images layers correspond with the image media type Ensure zstd compression only gets applied to oci images. When adding a layer to an image ensure that they are compatable if not convert them. Create function to convert mediatypes between oci and docker types. --- pkg/executor/build.go | 110 +++++++++++++++++++++++---- pkg/executor/build_test.go | 151 ++++++++++++++++++++++++++++++++++--- pkg/executor/fakes.go | 19 ++++- 3 files changed, 255 insertions(+), 25 deletions(-) diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 793af727db..be1c6f9caf 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -501,42 +501,122 @@ func (s *stageBuilder) saveSnapshotToLayer(tarPath string) (v1.Layer, error) { return nil, nil } + layerOpts := s.getLayerOptionFromOpts() + imageMediaType, err := s.image.MediaType() + if err != nil { + return nil, err + } + // Only appending MediaType for OCI images as the default is docker + if extractMediaTypeVendor(imageMediaType) == types.OCIVendorPrefix { + if s.opts.Compression == config.ZStd { + layerOpts = append(layerOpts, tarball.WithCompression("zstd"), tarball.WithMediaType(types.OCILayerZStd)) + } else { + layerOpts = append(layerOpts, tarball.WithMediaType(types.OCILayer)) + } + } + + layer, err := tarball.LayerFromFile(tarPath, layerOpts...) + if err != nil { + return nil, err + } + + return layer, nil +} + +func (s *stageBuilder) getLayerOptionFromOpts() []tarball.LayerOption { var layerOpts []tarball.LayerOption - if s.opts.CompressedCaching == true { + if s.opts.CompressedCaching { layerOpts = append(layerOpts, tarball.WithCompressedCaching) } if s.opts.CompressionLevel > 0 { layerOpts = append(layerOpts, tarball.WithCompressionLevel(s.opts.CompressionLevel)) } + return layerOpts +} - switch s.opts.Compression { - case config.ZStd: - layerOpts = append(layerOpts, tarball.WithCompression("zstd"), tarball.WithMediaType(types.OCILayerZStd)) - - case config.GZip: +func extractMediaTypeVendor(mt types.MediaType) string { + if strings.Contains(string(mt), types.OCIVendorPrefix) { + return types.OCIVendorPrefix + } + return types.DockerVendorPrefix +} - // layer already gzipped by default +// https://github.com/opencontainers/image-spec/blob/main/media-types.md#compatibility-matrix +func convertMediaType(mt types.MediaType) types.MediaType { + switch mt { + case types.DockerManifestSchema1, types.DockerManifestSchema2: + return types.OCIManifestSchema1 + case types.DockerManifestList: + return types.OCIImageIndex + case types.DockerLayer: + return types.OCILayer + case types.DockerConfigJSON: + return types.OCIConfigJSON + case types.DockerForeignLayer: + return types.OCIUncompressedRestrictedLayer + case types.DockerUncompressedLayer: + return types.OCIUncompressedLayer + case types.OCIImageIndex: + return types.DockerManifestList + case types.OCIManifestSchema1: + return types.DockerManifestSchema2 + case types.OCIConfigJSON: + return types.DockerConfigJSON + case types.OCILayer, types.OCILayerZStd: + return types.DockerLayer + case types.OCIRestrictedLayer: + return types.DockerForeignLayer + case types.OCIUncompressedLayer: + return types.DockerUncompressedLayer + case types.OCIContentDescriptor, types.OCIUncompressedRestrictedLayer, types.DockerManifestSchema1Signed, types.DockerPluginConfig: + return "" default: - mt, err := s.image.MediaType() - if err != nil { - return nil, err - } - if strings.Contains(string(mt), types.OCIVendorPrefix) { - layerOpts = append(layerOpts, tarball.WithMediaType(types.OCILayer)) - } + return "" } +} - layer, err := tarball.LayerFromFile(tarPath, layerOpts...) +func (s *stageBuilder) convertLayerMediaType(layer v1.Layer) (v1.Layer, error) { + layerMediaType, err := layer.MediaType() + if err != nil { + return nil, err + } + imageMediaType, err := s.image.MediaType() if err != nil { return nil, err } + if extractMediaTypeVendor(layerMediaType) != extractMediaTypeVendor(imageMediaType) { + layerOpts := s.getLayerOptionFromOpts() + targetMediaType := convertMediaType(layerMediaType) + + if extractMediaTypeVendor(imageMediaType) == types.OCIVendorPrefix { + if s.opts.Compression == config.ZStd { + targetMediaType = types.OCILayerZStd + layerOpts = append(layerOpts, tarball.WithCompression("zstd")) + } + } + + layerOpts = append(layerOpts, tarball.WithMediaType(targetMediaType)) + if targetMediaType != "" { + return tarball.LayerFromOpener(layer.Uncompressed, layerOpts...) + } + return nil, fmt.Errorf( + "layer with media type %v cannot be converted to a media type that matches %v", + layerMediaType, + imageMediaType, + ) + } return layer, nil } + func (s *stageBuilder) saveLayerToImage(layer v1.Layer, createdBy string) error { var err error + layer, err = s.convertLayerMediaType(layer) + if err != nil { + return err + } s.image, err = mutate.Append(s.image, mutate.Addendum{ Layer: layer, diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go index af3865d145..fc205dacee 100644 --- a/pkg/executor/build_test.go +++ b/pkg/executor/build_test.go @@ -1704,14 +1704,6 @@ func Test_ResolveCrossStageInstructions(t *testing.T) { } } -type ociFakeImage struct { - *fakeImage -} - -func (f ociFakeImage) MediaType() (types.MediaType, error) { - return types.OCIManifestSchema1, nil -} - func Test_stageBuilder_saveSnapshotToLayer(t *testing.T) { dir, files := tempDirAndFile(t) type fields struct { @@ -1788,7 +1780,7 @@ func Test_stageBuilder_saveSnapshotToLayer(t *testing.T) { { name: "oci image, zstd compression", fields: fields{ - image: fakeImage{}, + image: ociFakeImage{}, opts: &config.KanikoOptions{ ForceBuildMetadata: true, Compression: config.ZStd, @@ -1847,3 +1839,144 @@ func Test_stageBuilder_saveSnapshotToLayer(t *testing.T) { }) } } + +func Test_stageBuilder_convertLayerMediaType(t *testing.T) { + type fields struct { + stage config.KanikoStage + image v1.Image + cf *v1.ConfigFile + baseImageDigest string + finalCacheKey string + opts *config.KanikoOptions + fileContext util.FileContext + cmds []commands.DockerCommand + args *dockerfile.BuildArgs + crossStageDeps map[int][]string + digestToCacheKey map[string]string + stageIdxToDigest map[string]string + snapshotter snapShotter + layerCache cache.LayerCache + pushLayerToCache cachePusher + } + type args struct { + layer v1.Layer + } + tests := []struct { + name string + fields fields + args args + expectedMediaType types.MediaType + wantErr bool + }{ + { + name: "docker image w/ docker layer", + fields: fields{ + image: fakeImage{}, + }, + args: args{ + layer: fakeLayer{ + mediaType: types.DockerLayer, + }, + }, + expectedMediaType: types.DockerLayer, + }, + { + name: "oci image w/ oci layer", + fields: fields{ + image: ociFakeImage{}, + }, + args: args{ + layer: fakeLayer{ + mediaType: types.OCILayer, + }, + }, + expectedMediaType: types.OCILayer, + }, + { + name: "oci image w/ convertable docker layer", + fields: fields{ + image: ociFakeImage{}, + opts: &config.KanikoOptions{}, + }, + args: args{ + layer: fakeLayer{ + mediaType: types.DockerLayer, + }, + }, + expectedMediaType: types.OCILayer, + }, + { + name: "oci image w/ convertable docker layer and zstd compression", + fields: fields{ + image: ociFakeImage{}, + opts: &config.KanikoOptions{ + Compression: config.ZStd, + }, + }, + args: args{ + layer: fakeLayer{ + mediaType: types.DockerLayer, + }, + }, + expectedMediaType: types.OCILayerZStd, + }, + { + name: "docker image and oci zstd layer", + fields: fields{ + image: dockerFakeImage{}, + opts: &config.KanikoOptions{}, + }, + args: args{ + layer: fakeLayer{ + mediaType: types.OCILayerZStd, + }, + }, + expectedMediaType: types.DockerLayer, + }, + { + name: "docker image w/ uncovertable oci image", + fields: fields{ + image: dockerFakeImage{}, + opts: &config.KanikoOptions{}, + }, + args: args{ + layer: fakeLayer{ + mediaType: types.OCIUncompressedRestrictedLayer, + }, + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &stageBuilder{ + stage: tt.fields.stage, + image: tt.fields.image, + cf: tt.fields.cf, + baseImageDigest: tt.fields.baseImageDigest, + finalCacheKey: tt.fields.finalCacheKey, + opts: tt.fields.opts, + fileContext: tt.fields.fileContext, + cmds: tt.fields.cmds, + args: tt.fields.args, + crossStageDeps: tt.fields.crossStageDeps, + digestToCacheKey: tt.fields.digestToCacheKey, + stageIdxToDigest: tt.fields.stageIdxToDigest, + snapshotter: tt.fields.snapshotter, + layerCache: tt.fields.layerCache, + pushLayerToCache: tt.fields.pushLayerToCache, + } + got, err := s.convertLayerMediaType(tt.args.layer) + if (err != nil) != tt.wantErr { + t.Errorf("stageBuilder.convertLayerMediaType() error = %v, wantErr %v", err, tt.wantErr) + return + } + if err == nil { + mt, _ := got.MediaType() + if mt != tt.expectedMediaType { + t.Errorf("stageBuilder.convertLayerMediaType() = %v, want %v", mt, tt.expectedMediaType) + } + } + }) + } +} diff --git a/pkg/executor/fakes.go b/pkg/executor/fakes.go index 7bb7b383d7..1552123976 100644 --- a/pkg/executor/fakes.go +++ b/pkg/executor/fakes.go @@ -145,6 +145,7 @@ func (f *fakeLayerCache) RetrieveLayer(key string) (v1.Image, error) { type fakeLayer struct { TarContent []byte + mediaType types.MediaType } func (f fakeLayer) Digest() (v1.Hash, error) { @@ -163,7 +164,7 @@ func (f fakeLayer) Size() (int64, error) { return 0, nil } func (f fakeLayer) MediaType() (types.MediaType, error) { - return "", nil + return f.mediaType, nil } type fakeImage struct { @@ -203,3 +204,19 @@ func (f fakeImage) LayerByDigest(v1.Hash) (v1.Layer, error) { func (f fakeImage) LayerByDiffID(v1.Hash) (v1.Layer, error) { return fakeLayer{}, nil } + +type ociFakeImage struct { + *fakeImage +} + +func (f ociFakeImage) MediaType() (types.MediaType, error) { + return types.OCIManifestSchema1, nil +} + +type dockerFakeImage struct { + *fakeImage +} + +func (f dockerFakeImage) MediaType() (types.MediaType, error) { + return types.DockerManifestSchema2, nil +}