From 5e86eb8abeb0e06c97fdb099795e2d1caed9fdd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 16 Jun 2022 19:05:08 +0200 Subject: [PATCH 01/14] Clean up PreserveOriginal comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One of the is fine, but better placed closer to the reference. The other doesn't relate to that code path at all, and is already documented in types.BlobInfo. Signed-off-by: Miloslav Trmač --- copy/compression.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/copy/compression.go b/copy/compression.go index ddfe679828..3951cd8a04 100644 --- a/copy/compression.go +++ b/copy/compression.go @@ -87,8 +87,8 @@ func (c *copier) blobPipelineCompressionStep(stream *sourceStream, canModifyBlob // bpcPreserveEncrypted checks if the input is encrypted, and returns a *bpCompressionStepData if so. func (c *copier) bpcPreserveEncrypted(stream *sourceStream, _ bpDetectCompressionStepData) (*bpCompressionStepData, error) { if isOciEncrypted(stream.info.MediaType) { - // PreserveOriginal due to any compression not being able to be done on an encrypted blob unless decrypted logrus.Debugf("Using original blob without modification for encrypted blob") + // PreserveOriginal due to any compression not being able to be done on an encrypted blob unless decrypted return &bpCompressionStepData{ operation: types.PreserveOriginal, uploadedAlgorithm: nil, @@ -195,7 +195,6 @@ func (c *copier) bpcDecompressCompressed(stream *sourceStream, detected bpDetect // bpcPreserveOriginal returns a *bpCompressionStepData for not changing the original blob. func (c *copier) bpcPreserveOriginal(stream *sourceStream, detected bpDetectCompressionStepData) *bpCompressionStepData { - // PreserveOriginal might also need to recompress the original blob if the desired compression format is different. logrus.Debugf("Using original blob without modification") // Remember if the original blob was compressed, and if so how, so that if // LayerInfosForCopy() returned something that differs from what was in the From ef4d0e22effde8b7c30726221f697e3c773646ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 10 Jun 2022 17:25:03 +0200 Subject: [PATCH 02/14] Reformat TestSupported*MediaType MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Having each test case streched across four lines doesn't make it any more readable; seeing the test cases on the same screen as the test body is more valuable. Should not change (test) behavior. Signed-off-by: Miloslav Trmač --- manifest/docker_schema2_test.go | 40 +++++------------------- manifest/oci_test.go | 55 +++++++-------------------------- 2 files changed, 19 insertions(+), 76 deletions(-) diff --git a/manifest/docker_schema2_test.go b/manifest/docker_schema2_test.go index a161447052..e2f75ff885 100644 --- a/manifest/docker_schema2_test.go +++ b/manifest/docker_schema2_test.go @@ -17,38 +17,14 @@ func TestSupportedSchema2MediaType(t *testing.T) { mustFail bool } data := []testData{ - { - DockerV2Schema2MediaType, - false, - }, - { - DockerV2Schema2ConfigMediaType, - false, - }, - { - DockerV2Schema2LayerMediaType, - false, - }, - { - DockerV2SchemaLayerMediaTypeUncompressed, - false, - }, - { - DockerV2ListMediaType, - false, - }, - { - DockerV2Schema2ForeignLayerMediaType, - false, - }, - { - DockerV2Schema2ForeignLayerMediaTypeGzip, - false, - }, - { - "application/vnd.docker.image.rootfs.foreign.diff.unknown", - true, - }, + {DockerV2Schema2MediaType, false}, + {DockerV2Schema2ConfigMediaType, false}, + {DockerV2Schema2LayerMediaType, false}, + {DockerV2SchemaLayerMediaTypeUncompressed, false}, + {DockerV2ListMediaType, false}, + {DockerV2Schema2ForeignLayerMediaType, false}, + {DockerV2Schema2ForeignLayerMediaTypeGzip, false}, + {"application/vnd.docker.image.rootfs.foreign.diff.unknown", true}, } for _, d := range data { err := SupportedSchema2MediaType(d.m) diff --git a/manifest/oci_test.go b/manifest/oci_test.go index 94fcb6662d..716227e991 100644 --- a/manifest/oci_test.go +++ b/manifest/oci_test.go @@ -18,50 +18,17 @@ func TestSupportedOCI1MediaType(t *testing.T) { mustFail bool } data := []testData{ - { - imgspecv1.MediaTypeDescriptor, - false, - }, - { - imgspecv1.MediaTypeImageConfig, - false, - }, - { - imgspecv1.MediaTypeImageLayer, - false, - }, - { - imgspecv1.MediaTypeImageLayerGzip, - false, - }, - { - imgspecv1.MediaTypeImageLayerNonDistributable, - false, - }, - { - imgspecv1.MediaTypeImageLayerNonDistributableGzip, - false, - }, - { - imgspecv1.MediaTypeImageLayerNonDistributableZstd, - false, - }, - { - imgspecv1.MediaTypeImageLayerZstd, - false, - }, - { - imgspecv1.MediaTypeImageManifest, - false, - }, - { - imgspecv1.MediaTypeLayoutHeader, - false, - }, - { - "application/vnd.oci.image.layer.nondistributable.v1.tar+unknown", - true, - }, + {imgspecv1.MediaTypeDescriptor, false}, + {imgspecv1.MediaTypeImageConfig, false}, + {imgspecv1.MediaTypeImageLayer, false}, + {imgspecv1.MediaTypeImageLayerGzip, false}, + {imgspecv1.MediaTypeImageLayerNonDistributable, false}, + {imgspecv1.MediaTypeImageLayerNonDistributableGzip, false}, + {imgspecv1.MediaTypeImageLayerNonDistributableZstd, false}, + {imgspecv1.MediaTypeImageLayerZstd, false}, + {imgspecv1.MediaTypeImageManifest, false}, + {imgspecv1.MediaTypeLayoutHeader, false}, + {"application/vnd.oci.image.layer.nondistributable.v1.tar+unknown", true}, } for _, d := range data { err := SupportedOCI1MediaType(d.m) From 38b1d742f3fb1da22b07f26d869923c74d812d1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 10 Jun 2022 18:00:40 +0200 Subject: [PATCH 03/14] Simplify repetitive code in manifest tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add manifestSchema2FromFixture and manifestOCI1FromFixture instead of open-coding the read+decode steps over and over. The tests also fail immediately if they can't use the fixture, instead of somehow trying to continue (and probably crashing). Signed-off-by: Miloslav Trmač --- manifest/docker_schema2_test.go | 66 +++++----------- manifest/oci_test.go | 131 +++++++++----------------------- 2 files changed, 55 insertions(+), 142 deletions(-) diff --git a/manifest/docker_schema2_test.go b/manifest/docker_schema2_test.go index e2f75ff885..b540019bde 100644 --- a/manifest/docker_schema2_test.go +++ b/manifest/docker_schema2_test.go @@ -11,6 +11,15 @@ import ( "github.com/stretchr/testify/require" ) +func manifestSchema2FromFixture(t *testing.T, fixture string) *Schema2 { + manifest, err := os.ReadFile(filepath.Join("fixtures", fixture)) + require.NoError(t, err) + + m, err := Schema2FromManifest(manifest) + require.NoError(t, err) + return m +} + func TestSupportedSchema2MediaType(t *testing.T) { type testData struct { m string @@ -55,13 +64,8 @@ func TestSchema2FromManifest(t *testing.T) { } func TestUpdateLayerInfosV2S2GzipToZstd(t *testing.T) { - bytes, err := os.ReadFile("fixtures/v2s2.manifest.json") - assert.Nil(t, err) - - origManifest, err := Schema2FromManifest(bytes) - assert.Nil(t, err) - - err = origManifest.UpdateLayerInfos([]types.BlobInfo{ + origManifest := manifestSchema2FromFixture(t, "v2s2.manifest.json") + err := origManifest.UpdateLayerInfos([]types.BlobInfo{ { Digest: "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", Size: 32654, @@ -88,13 +92,8 @@ func TestUpdateLayerInfosV2S2GzipToZstd(t *testing.T) { } func TestUpdateLayerInfosV2S2InvalidCompressionOperation(t *testing.T) { - bytes, err := os.ReadFile("fixtures/v2s2.manifest.json") - assert.Nil(t, err) - - origManifest, err := Schema2FromManifest(bytes) - assert.Nil(t, err) - - err = origManifest.UpdateLayerInfos([]types.BlobInfo{ + origManifest := manifestSchema2FromFixture(t, "v2s2.manifest.json") + err := origManifest.UpdateLayerInfos([]types.BlobInfo{ { Digest: "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", Size: 32654, @@ -118,13 +117,8 @@ func TestUpdateLayerInfosV2S2InvalidCompressionOperation(t *testing.T) { } func TestUpdateLayerInfosV2S2InvalidCompressionAlgorithm(t *testing.T) { - bytes, err := os.ReadFile("fixtures/v2s2.manifest.json") - assert.Nil(t, err) - - origManifest, err := Schema2FromManifest(bytes) - assert.Nil(t, err) - - err = origManifest.UpdateLayerInfos([]types.BlobInfo{ + origManifest := manifestSchema2FromFixture(t, "v2s2.manifest.json") + err := origManifest.UpdateLayerInfos([]types.BlobInfo{ { Digest: "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", Size: 32654, @@ -151,13 +145,9 @@ func TestUpdateLayerInfosV2S2InvalidCompressionAlgorithm(t *testing.T) { } func TestUpdateLayerInfosV2S2NondistributableToGzip(t *testing.T) { - bytes, err := os.ReadFile("fixtures/v2s2.nondistributable.manifest.json") - assert.Nil(t, err) - - origManifest, err := Schema2FromManifest(bytes) - assert.Nil(t, err) + origManifest := manifestSchema2FromFixture(t, "v2s2.nondistributable.manifest.json") - err = origManifest.UpdateLayerInfos([]types.BlobInfo{ + err := origManifest.UpdateLayerInfos([]types.BlobInfo{ { Digest: "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", Size: 32654, @@ -171,12 +161,7 @@ func TestUpdateLayerInfosV2S2NondistributableToGzip(t *testing.T) { updatedManifestBytes, err := origManifest.Serialize() assert.Nil(t, err) - bytes, err = os.ReadFile("fixtures/v2s2.nondistributable.gzip.manifest.json") - assert.Nil(t, err) - - expectedManifest, err := Schema2FromManifest(bytes) - assert.Nil(t, err) - + expectedManifest := manifestSchema2FromFixture(t, "v2s2.nondistributable.gzip.manifest.json") expectedManifestBytes, err := expectedManifest.Serialize() assert.Nil(t, err) @@ -184,13 +169,9 @@ func TestUpdateLayerInfosV2S2NondistributableToGzip(t *testing.T) { } func TestUpdateLayerInfosV2S2NondistributableGzipToUncompressed(t *testing.T) { - bytes, err := os.ReadFile("fixtures/v2s2.nondistributable.gzip.manifest.json") - assert.Nil(t, err) - - origManifest, err := Schema2FromManifest(bytes) - assert.Nil(t, err) + origManifest := manifestSchema2FromFixture(t, "v2s2.nondistributable.gzip.manifest.json") - err = origManifest.UpdateLayerInfos([]types.BlobInfo{ + err := origManifest.UpdateLayerInfos([]types.BlobInfo{ { Digest: "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", Size: 32654, @@ -203,12 +184,7 @@ func TestUpdateLayerInfosV2S2NondistributableGzipToUncompressed(t *testing.T) { updatedManifestBytes, err := origManifest.Serialize() assert.Nil(t, err) - bytes, err = os.ReadFile("fixtures/v2s2.nondistributable.manifest.json") - assert.Nil(t, err) - - expectedManifest, err := Schema2FromManifest(bytes) - assert.Nil(t, err) - + expectedManifest := manifestSchema2FromFixture(t, "v2s2.nondistributable.manifest.json") expectedManifestBytes, err := expectedManifest.Serialize() assert.Nil(t, err) diff --git a/manifest/oci_test.go b/manifest/oci_test.go index 716227e991..0aa70d60f0 100644 --- a/manifest/oci_test.go +++ b/manifest/oci_test.go @@ -12,6 +12,15 @@ import ( "github.com/stretchr/testify/require" ) +func manifestOCI1FromFixture(t *testing.T, fixture string) *OCI1 { + manifest, err := os.ReadFile(filepath.Join("fixtures", fixture)) + require.NoError(t, err) + + m, err := OCI1FromManifest(manifest) + require.NoError(t, err) + return m +} + func TestSupportedOCI1MediaType(t *testing.T) { type testData struct { m string @@ -60,13 +69,9 @@ func TestOCI1FromManifest(t *testing.T) { } func TestUpdateLayerInfosOCIGzipToZstd(t *testing.T) { - bytes, err := os.ReadFile("fixtures/ociv1.manifest.json") - assert.Nil(t, err) - - manifest, err := OCI1FromManifest(bytes) - assert.Nil(t, err) + manifest := manifestOCI1FromFixture(t, "ociv1.manifest.json") - err = manifest.UpdateLayerInfos([]types.BlobInfo{ + err := manifest.UpdateLayerInfos([]types.BlobInfo{ { Digest: "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", Size: 32654, @@ -94,12 +99,7 @@ func TestUpdateLayerInfosOCIGzipToZstd(t *testing.T) { updatedManifestBytes, err := manifest.Serialize() assert.Nil(t, err) - bytes, err = os.ReadFile("fixtures/ociv1.zstd.manifest.json") - assert.Nil(t, err) - - expectedManifest, err := OCI1FromManifest(bytes) - assert.Nil(t, err) - + expectedManifest := manifestOCI1FromFixture(t, "ociv1.zstd.manifest.json") expectedManifestBytes, err := expectedManifest.Serialize() assert.Nil(t, err) @@ -107,13 +107,9 @@ func TestUpdateLayerInfosOCIGzipToZstd(t *testing.T) { } func TestUpdateLayerInfosOCIZstdToGzip(t *testing.T) { - bytes, err := os.ReadFile("fixtures/ociv1.zstd.manifest.json") - assert.Nil(t, err) - - manifest, err := OCI1FromManifest(bytes) - assert.Nil(t, err) + manifest := manifestOCI1FromFixture(t, "ociv1.zstd.manifest.json") - err = manifest.UpdateLayerInfos([]types.BlobInfo{ + err := manifest.UpdateLayerInfos([]types.BlobInfo{ { Digest: "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", Size: 32654, @@ -141,12 +137,7 @@ func TestUpdateLayerInfosOCIZstdToGzip(t *testing.T) { updatedManifestBytes, err := manifest.Serialize() assert.Nil(t, err) - bytes, err = os.ReadFile("fixtures/ociv1.manifest.json") - assert.Nil(t, err) - - expectedManifest, err := OCI1FromManifest(bytes) - assert.Nil(t, err) - + expectedManifest := manifestOCI1FromFixture(t, "ociv1.manifest.json") expectedManifestBytes, err := expectedManifest.Serialize() assert.Nil(t, err) @@ -154,13 +145,9 @@ func TestUpdateLayerInfosOCIZstdToGzip(t *testing.T) { } func TestUpdateLayerInfosOCIZstdToUncompressed(t *testing.T) { - bytes, err := os.ReadFile("fixtures/ociv1.zstd.manifest.json") - assert.Nil(t, err) + manifest := manifestOCI1FromFixture(t, "ociv1.zstd.manifest.json") - manifest, err := OCI1FromManifest(bytes) - assert.Nil(t, err) - - err = manifest.UpdateLayerInfos([]types.BlobInfo{ + err := manifest.UpdateLayerInfos([]types.BlobInfo{ { Digest: "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", Size: 32654, @@ -185,12 +172,7 @@ func TestUpdateLayerInfosOCIZstdToUncompressed(t *testing.T) { updatedManifestBytes, err := manifest.Serialize() assert.Nil(t, err) - bytes, err = os.ReadFile("fixtures/ociv1.uncompressed.manifest.json") - assert.Nil(t, err) - - expectedManifest, err := OCI1FromManifest(bytes) - assert.Nil(t, err) - + expectedManifest := manifestOCI1FromFixture(t, "ociv1.uncompressed.manifest.json") expectedManifestBytes, err := expectedManifest.Serialize() assert.Nil(t, err) @@ -198,13 +180,8 @@ func TestUpdateLayerInfosOCIZstdToUncompressed(t *testing.T) { } func TestUpdateLayerInfosInvalidCompressionOperation(t *testing.T) { - bytes, err := os.ReadFile("fixtures/ociv1.zstd.manifest.json") - assert.Nil(t, err) - - manifest, err := OCI1FromManifest(bytes) - assert.Nil(t, err) - - err = manifest.UpdateLayerInfos([]types.BlobInfo{ + manifest := manifestOCI1FromFixture(t, "ociv1.zstd.manifest.json") + err := manifest.UpdateLayerInfos([]types.BlobInfo{ { Digest: "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", Size: 32654, @@ -231,14 +208,10 @@ func TestUpdateLayerInfosInvalidCompressionOperation(t *testing.T) { } func TestUpdateLayerInfosInvalidCompressionAlgorithm(t *testing.T) { - bytes, err := os.ReadFile("fixtures/ociv1.zstd.manifest.json") - assert.Nil(t, err) - - manifest, err := OCI1FromManifest(bytes) - assert.Nil(t, err) + manifest := manifestOCI1FromFixture(t, "ociv1.zstd.manifest.json") customCompression := compression.Algorithm{} - err = manifest.UpdateLayerInfos([]types.BlobInfo{ + err := manifest.UpdateLayerInfos([]types.BlobInfo{ { Digest: "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", Size: 32654, @@ -265,13 +238,9 @@ func TestUpdateLayerInfosInvalidCompressionAlgorithm(t *testing.T) { } func TestUpdateLayerInfosOCIGzipToUncompressed(t *testing.T) { - bytes, err := os.ReadFile("fixtures/ociv1.manifest.json") - assert.Nil(t, err) - - manifest, err := OCI1FromManifest(bytes) - assert.Nil(t, err) + manifest := manifestOCI1FromFixture(t, "ociv1.manifest.json") - err = manifest.UpdateLayerInfos([]types.BlobInfo{ + err := manifest.UpdateLayerInfos([]types.BlobInfo{ { Digest: "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", Size: 32654, @@ -296,12 +265,7 @@ func TestUpdateLayerInfosOCIGzipToUncompressed(t *testing.T) { updatedManifestBytes, err := manifest.Serialize() assert.Nil(t, err) - bytes, err = os.ReadFile("fixtures/ociv1.uncompressed.manifest.json") - assert.Nil(t, err) - - expectedManifest, err := OCI1FromManifest(bytes) - assert.Nil(t, err) - + expectedManifest := manifestOCI1FromFixture(t, "ociv1.uncompressed.manifest.json") expectedManifestBytes, err := expectedManifest.Serialize() assert.Nil(t, err) @@ -309,13 +273,9 @@ func TestUpdateLayerInfosOCIGzipToUncompressed(t *testing.T) { } func TestUpdateLayerInfosOCINondistributableToGzip(t *testing.T) { - bytes, err := os.ReadFile("fixtures/ociv1.nondistributable.manifest.json") - assert.Nil(t, err) - - manifest, err := OCI1FromManifest(bytes) - assert.Nil(t, err) + manifest := manifestOCI1FromFixture(t, "ociv1.nondistributable.manifest.json") - err = manifest.UpdateLayerInfos([]types.BlobInfo{ + err := manifest.UpdateLayerInfos([]types.BlobInfo{ { Digest: "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", Size: 32654, @@ -329,12 +289,7 @@ func TestUpdateLayerInfosOCINondistributableToGzip(t *testing.T) { updatedManifestBytes, err := manifest.Serialize() assert.Nil(t, err) - bytes, err = os.ReadFile("fixtures/ociv1.nondistributable.gzip.manifest.json") - assert.Nil(t, err) - - expectedManifest, err := OCI1FromManifest(bytes) - assert.Nil(t, err) - + expectedManifest := manifestOCI1FromFixture(t, "ociv1.nondistributable.gzip.manifest.json") expectedManifestBytes, err := expectedManifest.Serialize() assert.Nil(t, err) @@ -342,13 +297,9 @@ func TestUpdateLayerInfosOCINondistributableToGzip(t *testing.T) { } func TestUpdateLayerInfosOCINondistributableToZstd(t *testing.T) { - bytes, err := os.ReadFile("fixtures/ociv1.nondistributable.manifest.json") - assert.Nil(t, err) - - manifest, err := OCI1FromManifest(bytes) - assert.Nil(t, err) + manifest := manifestOCI1FromFixture(t, "ociv1.nondistributable.manifest.json") - err = manifest.UpdateLayerInfos([]types.BlobInfo{ + err := manifest.UpdateLayerInfos([]types.BlobInfo{ { Digest: "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", Size: 32654, @@ -362,12 +313,7 @@ func TestUpdateLayerInfosOCINondistributableToZstd(t *testing.T) { updatedManifestBytes, err := manifest.Serialize() assert.Nil(t, err) - bytes, err = os.ReadFile("fixtures/ociv1.nondistributable.zstd.manifest.json") - assert.Nil(t, err) - - expectedManifest, err := OCI1FromManifest(bytes) - assert.Nil(t, err) - + expectedManifest := manifestOCI1FromFixture(t, "ociv1.nondistributable.zstd.manifest.json") expectedManifestBytes, err := expectedManifest.Serialize() assert.Nil(t, err) @@ -375,13 +321,9 @@ func TestUpdateLayerInfosOCINondistributableToZstd(t *testing.T) { } func TestUpdateLayerInfosOCINondistributableGzipToUncompressed(t *testing.T) { - bytes, err := os.ReadFile("fixtures/ociv1.nondistributable.gzip.manifest.json") - assert.Nil(t, err) - - manifest, err := OCI1FromManifest(bytes) - assert.Nil(t, err) + manifest := manifestOCI1FromFixture(t, "ociv1.nondistributable.gzip.manifest.json") - err = manifest.UpdateLayerInfos([]types.BlobInfo{ + err := manifest.UpdateLayerInfos([]types.BlobInfo{ { Digest: "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", Size: 32654, @@ -394,12 +336,7 @@ func TestUpdateLayerInfosOCINondistributableGzipToUncompressed(t *testing.T) { updatedManifestBytes, err := manifest.Serialize() assert.Nil(t, err) - bytes, err = os.ReadFile("fixtures/ociv1.nondistributable.manifest.json") - assert.Nil(t, err) - - expectedManifest, err := OCI1FromManifest(bytes) - assert.Nil(t, err) - + expectedManifest := manifestOCI1FromFixture(t, "ociv1.nondistributable.manifest.json") expectedManifestBytes, err := expectedManifest.Serialize() assert.Nil(t, err) From bf597f630226777637f27e33248f67758a20cb08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 10 Jun 2022 20:03:25 +0200 Subject: [PATCH 04/14] Move getEncryptedMediaType and getDecryptedMediaType closer to the only caller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... to make the code a tiny bit easier to follow. Only moves unchanged code, should not change behavior. Signed-off-by: Miloslav Trmač --- manifest/oci.go | 54 ++++++++++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/manifest/oci.go b/manifest/oci.go index 5892184df1..2a595cb424 100644 --- a/manifest/oci.go +++ b/manifest/oci.go @@ -151,6 +151,33 @@ func (m *OCI1) UpdateLayerInfos(layerInfos []types.BlobInfo) error { return nil } +// getEncryptedMediaType will return the mediatype to its encrypted counterpart and return +// an error if the mediatype does not support encryption +func getEncryptedMediaType(mediatype string) (string, error) { + for _, s := range strings.Split(mediatype, "+")[1:] { + if s == "encrypted" { + return "", errors.Errorf("unsupportedmediatype: %v already encrypted", mediatype) + } + } + unsuffixedMediatype := strings.Split(mediatype, "+")[0] + switch unsuffixedMediatype { + case DockerV2Schema2LayerMediaType, imgspecv1.MediaTypeImageLayer, imgspecv1.MediaTypeImageLayerNonDistributable: + return mediatype + "+encrypted", nil + } + + return "", errors.Errorf("unsupported mediatype to encrypt: %v", mediatype) +} + +// getEncryptedMediaType will return the mediatype to its encrypted counterpart and return +// an error if the mediatype does not support decryption +func getDecryptedMediaType(mediatype string) (string, error) { + if !strings.HasSuffix(mediatype, "+encrypted") { + return "", errors.Errorf("unsupported mediatype to decrypt %v:", mediatype) + } + + return strings.TrimSuffix(mediatype, "+encrypted"), nil +} + // Serialize returns the manifest in a blob format. // NOTE: Serialize() does not in general reproduce the original blob if this object was loaded from one, even if no modifications were made! func (m *OCI1) Serialize() ([]byte, error) { @@ -191,30 +218,3 @@ func (m *OCI1) ImageID([]digest.Digest) (string, error) { } return m.Config.Digest.Hex(), nil } - -// getEncryptedMediaType will return the mediatype to its encrypted counterpart and return -// an error if the mediatype does not support encryption -func getEncryptedMediaType(mediatype string) (string, error) { - for _, s := range strings.Split(mediatype, "+")[1:] { - if s == "encrypted" { - return "", errors.Errorf("unsupportedmediatype: %v already encrypted", mediatype) - } - } - unsuffixedMediatype := strings.Split(mediatype, "+")[0] - switch unsuffixedMediatype { - case DockerV2Schema2LayerMediaType, imgspecv1.MediaTypeImageLayer, imgspecv1.MediaTypeImageLayerNonDistributable: - return mediatype + "+encrypted", nil - } - - return "", errors.Errorf("unsupported mediatype to encrypt: %v", mediatype) -} - -// getEncryptedMediaType will return the mediatype to its encrypted counterpart and return -// an error if the mediatype does not support decryption -func getDecryptedMediaType(mediatype string) (string, error) { - if !strings.HasSuffix(mediatype, "+encrypted") { - return "", errors.Errorf("unsupported mediatype to decrypt %v:", mediatype) - } - - return strings.TrimSuffix(mediatype, "+encrypted"), nil -} From 83aee8f797fe8d63ab0b988759d6b7ebc39370dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 10 Jun 2022 19:05:24 +0200 Subject: [PATCH 05/14] Reject OCI artifacts in manifest.OCI1.ImageID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... and add tests for the ImageID implementations. Signed-off-by: Miloslav Trmač --- internal/manifest/errors.go | 32 ++++++++++++++++++++++++++++++++ manifest/docker_schema1_test.go | 19 +++++++++++++++++++ manifest/docker_schema2_test.go | 13 +++++++++++++ manifest/manifest.go | 5 +++++ manifest/oci.go | 21 +++++++++++++++++++++ manifest/oci_test.go | 18 ++++++++++++++++++ 6 files changed, 108 insertions(+) create mode 100644 internal/manifest/errors.go diff --git a/internal/manifest/errors.go b/internal/manifest/errors.go new file mode 100644 index 0000000000..e5732a8c43 --- /dev/null +++ b/internal/manifest/errors.go @@ -0,0 +1,32 @@ +package manifest + +import "fmt" + +// NonImageArtifactError (detected via errors.As) is used when asking for an image-specific operation +// on an object which is not a “container image” in the standard sense (e.g. an OCI artifact) +// +// This is publicly visible as c/image/manifest.NonImageArtifactError (but we don’t provide a public constructor) +type NonImageArtifactError struct { + // Callers should not be blindly calling image-specific operations and only checking MIME types + // on failure; if they care about the artifact type, they should check before using it. + // If they blindly assume an image, they don’t really need this value; just a type check + // is sufficient for basic "we can only pull images" UI. + // + // Also, there are fairly widespread “artifacts” which nevertheless use imgspecv1.MediaTypeImageConfig, + // e.g. https://github.com/sigstore/cosign/blob/main/specs/SIGNATURE_SPEC.md , which could cause the callers + // to complain about a non-image artifact with the correct MIME type; we should probably add some other kind of + // type discrimination, _and_ somehow make it available in the API, if we expect API callers to make decisions + // based on that kind of data. + // + // So, let’s not expose this until a specific need is identified. + mimeType string +} + +// NewNonImageArtifactError returns a NonImageArtifactError about an artifact with mimeType. +func NewNonImageArtifactError(mimeType string) error { + return NonImageArtifactError{mimeType: mimeType} +} + +func (e NonImageArtifactError) Error() string { + return fmt.Sprintf("unsupported image-specific operation on artifact with type %q", e.mimeType) +} diff --git a/manifest/docker_schema1_test.go b/manifest/docker_schema1_test.go index 93accdfffd..390c8816a0 100644 --- a/manifest/docker_schema1_test.go +++ b/manifest/docker_schema1_test.go @@ -7,10 +7,20 @@ import ( "time" "github.com/containers/image/v5/types" + "github.com/opencontainers/go-digest" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +// DiffID values corresponding to layers of schema2-to-schema1-by-docker.json +var schema1FixtureLayerDiffIDs = []digest.Digest{ + "sha256:142a601d97936307e75220c35dde0348971a9584c21e7cb42e1f7004005432ab", + "sha256:90fcc66ad3be9f1757f954b750deb37032f208428aa12599fcb02182b9065a9c", + "sha256:5a8624bb7e76d1e6829f9c64c43185e02bc07f97a2189eb048609a8914e72c56", + "sha256:d349ff6b3afc6a2800054768c82bfbf4289c9aa5da55c1290f802943dcd4d1e9", + "sha256:8c064bb1f60e84fa8cc6079b6d2e76e0423389fd6aeb7e497dfdae5e05b2b25b", +} + func manifestSchema1FromFixture(t *testing.T, fixture string) *Schema1 { manifest, err := os.ReadFile(filepath.Join("fixtures", fixture)) require.NoError(t, err) @@ -166,3 +176,12 @@ func TestSchema1LayerInfos(t *testing.T) { {BlobInfo: types.BlobInfo{Digest: "sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4", Size: -1}, EmptyLayer: true}, }, m.LayerInfos()) } + +func TestSchema1ImageID(t *testing.T) { + m := manifestSchema1FromFixture(t, "schema2-to-schema1-by-docker.json") + id, err := m.ImageID(schema1FixtureLayerDiffIDs) + require.NoError(t, err) + // NOTE: This value is dependent on the Schema1.ToSchema2Config implementation, and not necessarily stable over time. + // This is mostly a smoke-test; it’s fine to just update this value if that implementation changes. + assert.Equal(t, "9ca4bda0a6b3727a6ffcc43e981cad0f24e2ec79d338f6ba325b4dfd0756fb8f", id) +} diff --git a/manifest/docker_schema2_test.go b/manifest/docker_schema2_test.go index b540019bde..cd89b97016 100644 --- a/manifest/docker_schema2_test.go +++ b/manifest/docker_schema2_test.go @@ -7,6 +7,7 @@ import ( "github.com/containers/image/v5/pkg/compression" "github.com/containers/image/v5/types" + "github.com/opencontainers/go-digest" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -190,3 +191,15 @@ func TestUpdateLayerInfosV2S2NondistributableGzipToUncompressed(t *testing.T) { assert.Equal(t, string(expectedManifestBytes), string(updatedManifestBytes)) } + +func TestSchema2ImageID(t *testing.T) { + m := manifestSchema2FromFixture(t, "v2s2.manifest.json") + // These are not the real DiffID values, but they don’t actually matter in our implementation. + id, err := m.ImageID([]digest.Digest{ + "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc", + }) + require.NoError(t, err) + assert.Equal(t, "b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7", id) +} diff --git a/manifest/manifest.go b/manifest/manifest.go index 2e3e5da15e..53fc866a78 100644 --- a/manifest/manifest.go +++ b/manifest/manifest.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" + internalManifest "github.com/containers/image/v5/internal/manifest" "github.com/containers/image/v5/types" "github.com/containers/libtrust" digest "github.com/opencontainers/go-digest" @@ -34,6 +35,10 @@ const ( DockerV2Schema2ForeignLayerMediaTypeGzip = "application/vnd.docker.image.rootfs.foreign.diff.tar.gzip" ) +// NonImageArtifactError (detected via errors.As) is used when asking for an image-specific operation +// on an object which is not a “container image” in the standard sense (e.g. an OCI artifact) +type NonImageArtifactError = internalManifest.NonImageArtifactError + // SupportedSchema2MediaType checks if the specified string is a supported Docker v2s2 media type. func SupportedSchema2MediaType(m string) error { switch m { diff --git a/manifest/oci.go b/manifest/oci.go index 2a595cb424..6332e79823 100644 --- a/manifest/oci.go +++ b/manifest/oci.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + internalManifest "github.com/containers/image/v5/internal/manifest" compressiontypes "github.com/containers/image/v5/pkg/compression/types" "github.com/containers/image/v5/types" ociencspec "github.com/containers/ocicrypt/spec" @@ -213,6 +214,26 @@ func (m *OCI1) Inspect(configGetter func(types.BlobInfo) ([]byte, error)) (*type // ImageID computes an ID which can uniquely identify this image by its contents. func (m *OCI1) ImageID([]digest.Digest) (string, error) { + // The way m.Config.Digest “uniquely identifies” an image is + // by containing RootFS.DiffIDs, which identify the layers of the image. + // For non-image artifacts, the we can’t expect the config to change + // any time the other layers (semantically) change, so this approach of + // distinguishing objects only by m.Config.Digest doesn’t work in general. + // + // Any caller of this method presumably wants to disambiguate the same + // images with a different representation, but doesn’t want to disambiguate + // representations (by using a manifest digest). So, submitting a non-image + // artifact to such a caller indicates an expectation mismatch. + // So, we just fail here instead of inventing some other ID value (e.g. + // by combining the config and blob layer digests). That still + // gives us the option to not fail, and return some value, in the future, + // without committing to that approach now. + // (The only known caller of ImageID is storage/storageImageDestination.computeID, + // which can’t work with non-image artifacts.) + if m.Config.MediaType != imgspecv1.MediaTypeImageConfig { + return "", internalManifest.NewNonImageArtifactError(m.Config.MediaType) + } + if err := m.Config.Digest.Validate(); err != nil { return "", err } diff --git a/manifest/oci_test.go b/manifest/oci_test.go index 0aa70d60f0..273ef7267a 100644 --- a/manifest/oci_test.go +++ b/manifest/oci_test.go @@ -7,6 +7,7 @@ import ( "github.com/containers/image/v5/pkg/compression" "github.com/containers/image/v5/types" + "github.com/opencontainers/go-digest" imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -342,3 +343,20 @@ func TestUpdateLayerInfosOCINondistributableGzipToUncompressed(t *testing.T) { assert.Equal(t, string(expectedManifestBytes), string(updatedManifestBytes)) } + +func TestOCI1ImageID(t *testing.T) { + m := manifestOCI1FromFixture(t, "ociv1.manifest.json") + // These are not the real DiffID values, but they don’t actually matter in our implementation. + id, err := m.ImageID([]digest.Digest{ + "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc", + }) + require.NoError(t, err) + assert.Equal(t, "b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7", id) + + m = manifestOCI1FromFixture(t, "ociv1.artifact.json") + _, err = m.ImageID([]digest.Digest{}) + var expected NonImageArtifactError + assert.ErrorAs(t, err, &expected) +} From ec3d50b1cc1f635835d3d88b4796041a67745997 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 10 Jun 2022 19:20:16 +0200 Subject: [PATCH 06/14] Reject OCI artifacts in manifest.OCI1.Inspect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- manifest/oci.go | 8 ++++++++ manifest/oci_test.go | 13 +++++++++++++ 2 files changed, 21 insertions(+) diff --git a/manifest/oci.go b/manifest/oci.go index 6332e79823..2fa23e69cb 100644 --- a/manifest/oci.go +++ b/manifest/oci.go @@ -187,6 +187,14 @@ func (m *OCI1) Serialize() ([]byte, error) { // Inspect returns various information for (skopeo inspect) parsed from the manifest and configuration. func (m *OCI1) Inspect(configGetter func(types.BlobInfo) ([]byte, error)) (*types.ImageInspectInfo, error) { + if m.Config.MediaType != imgspecv1.MediaTypeImageConfig { + // We could return at least the layers, but that’s already available in a better format via types.Image.LayerInfos. + // Most software calling this without human intervention is going to expect the values to be realistic and relevant, + // and is probably better served by failing; we can always re-visit that later if we fail now, but + // if we started returning some data for OCI artifacts now, we couldn’t start failing in this function later. + return nil, internalManifest.NewNonImageArtifactError(m.Config.MediaType) + } + config, err := configGetter(m.ConfigInfo()) if err != nil { return nil, err diff --git a/manifest/oci_test.go b/manifest/oci_test.go index 273ef7267a..b752051490 100644 --- a/manifest/oci_test.go +++ b/manifest/oci_test.go @@ -344,6 +344,19 @@ func TestUpdateLayerInfosOCINondistributableGzipToUncompressed(t *testing.T) { assert.Equal(t, string(expectedManifestBytes), string(updatedManifestBytes)) } +func TestOCI1Inspect(t *testing.T) { + // Success is tested in image.TestManifestOCI1Inspect . + m := manifestOCI1FromFixture(t, "ociv1.artifact.json") + _, err := m.Inspect(func(info types.BlobInfo) ([]byte, error) { + require.Equal(t, m.Config.Digest, info.Digest) + // This just-enough-artifact contains a zero-byte config, sanity-check that’s till the case. + require.Equal(t, int64(0), m.Config.Size) + return []byte{}, nil + }) + var expected NonImageArtifactError + assert.ErrorAs(t, err, &expected) +} + func TestOCI1ImageID(t *testing.T) { m := manifestOCI1FromFixture(t, "ociv1.manifest.json") // These are not the real DiffID values, but they don’t actually matter in our implementation. From e6e1a4f4d7c2bb7d0fc4f31b16b16206fa7b55d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 10 Jun 2022 20:55:17 +0200 Subject: [PATCH 07/14] Refuse to convert non-image OCI artifacts to Docker formats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- internal/image/fixtures/oci1-artifact.json | 43 ++++++++++++++++++++++ internal/image/oci.go | 14 ++++++- internal/image/oci_test.go | 23 +++++++++++- 3 files changed, 76 insertions(+), 4 deletions(-) create mode 100644 internal/image/fixtures/oci1-artifact.json diff --git a/internal/image/fixtures/oci1-artifact.json b/internal/image/fixtures/oci1-artifact.json new file mode 100644 index 0000000000..9e54409a31 --- /dev/null +++ b/internal/image/fixtures/oci1-artifact.json @@ -0,0 +1,43 @@ +{ + "schemaVersion": 2, + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "config": { + "mediaType": "application/vnd.oci.custom.artifact.config.v1+json", + "size": 5940, + "digest": "sha256:9ca4bda0a6b3727a6ffcc43e981cad0f24e2ec79d338f6ba325b4dfd0756fb8f", + "annotations": { + "test-annotation-1": "one" + } + }, + "layers": [ + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 51354364, + "digest": "sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5bb" + }, + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 150, + "digest": "sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680c" + }, + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 11739507, + "digest": "sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a9", + "urls": ["https://layer.url"] + }, + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 8841833, + "digest": "sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25909", + "annotations": { + "test-annotation-2": "two" + } + }, + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 291, + "digest": "sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fa" + } + ] +} diff --git a/internal/image/oci.go b/internal/image/oci.go index 58e9c03ba3..49d58f278c 100644 --- a/internal/image/oci.go +++ b/internal/image/oci.go @@ -7,6 +7,7 @@ import ( "github.com/containers/image/v5/docker/reference" "github.com/containers/image/v5/internal/iolimits" + internalManifest "github.com/containers/image/v5/internal/manifest" "github.com/containers/image/v5/manifest" "github.com/containers/image/v5/pkg/blobinfocache/none" "github.com/containers/image/v5/types" @@ -194,10 +195,15 @@ func (m *manifestOCI1) convertToManifestSchema2Generic(ctx context.Context, opti // value. // This does not change the state of the original manifestOCI1 object. func (m *manifestOCI1) convertToManifestSchema2(_ context.Context, _ *types.ManifestUpdateOptions) (*manifestSchema2, error) { + if m.m.Config.MediaType != imgspecv1.MediaTypeImageConfig { + return nil, internalManifest.NewNonImageArtifactError(m.m.Config.MediaType) + } + // Create a copy of the descriptor. config := schema2DescriptorFromOCI1Descriptor(m.m.Config) - // The only difference between OCI and DockerSchema2 is the mediatypes. The + // Above, we have already checked that this manifest refers to an image, not an OCI artifact, + // so the only difference between OCI and DockerSchema2 is the mediatypes. The // media type of the manifest is handled by manifestSchema2FromComponents. config.MediaType = manifest.DockerV2Schema2ConfigMediaType @@ -233,7 +239,11 @@ func (m *manifestOCI1) convertToManifestSchema2(_ context.Context, _ *types.Mani // value. // This does not change the state of the original manifestOCI1 object. func (m *manifestOCI1) convertToManifestSchema1(ctx context.Context, options *types.ManifestUpdateOptions) (genericManifest, error) { - // We can't directly convert to V1, but we can transitively convert via a V2 image + if m.m.Config.MediaType != imgspecv1.MediaTypeImageConfig { + return nil, internalManifest.NewNonImageArtifactError(m.m.Config.MediaType) + } + + // We can't directly convert images to V1, but we can transitively convert via a V2 image m2, err := m.convertToManifestSchema2(ctx, options) if err != nil { return nil, err diff --git a/internal/image/oci_test.go b/internal/image/oci_test.go index bd183f17c1..ed83b948c7 100644 --- a/internal/image/oci_test.go +++ b/internal/image/oci_test.go @@ -454,7 +454,18 @@ func TestManifestOCI1ConvertToManifestSchema1(t *testing.T) { {Digest: GzippedEmptyLayerDigest, Size: -1}, }, s1Manifest.LayerInfos()) - // FIXME? Test also the various failure cases, if only to see that we don't crash? + // This can share originalSrc because the config digest is the same between oci1-artifact.json and oci1.json + artifact := manifestOCI1FromFixture(t, originalSrc, "oci1-artifact.json") + _, err = artifact.UpdatedImage(context.Background(), types.ManifestUpdateOptions{ + ManifestMIMEType: manifest.DockerV2Schema1SignedMediaType, + InformationOnly: types.ManifestUpdateInformation{ + Destination: memoryDest, + }, + }) + var expected manifest.NonImageArtifactError + assert.ErrorAs(t, err, &expected) + + // FIXME? Test also the other failure cases, if only to see that we don't crash? } func TestConvertToManifestSchema2(t *testing.T) { @@ -478,7 +489,15 @@ func TestConvertToManifestSchema2(t *testing.T) { require.NoError(t, err) assert.Equal(t, byHand, converted) - // FIXME? Test also the various failure cases, if only to see that we don't crash? + // This can share originalSrc because the config digest is the same between oci1-artifact.json and oci1.json + artifact := manifestOCI1FromFixture(t, originalSrc, "oci1-artifact.json") + _, err = artifact.UpdatedImage(context.Background(), types.ManifestUpdateOptions{ + ManifestMIMEType: manifest.DockerV2Schema2MediaType, + }) + var expected manifest.NonImageArtifactError + assert.ErrorAs(t, err, &expected) + + // FIXME? Test also the other failure cases, if only to see that we don't crash? } func TestConvertToManifestSchema2AllMediaTypes(t *testing.T) { From d24d259821ffef1112b21ec7969b99bee3fc2c76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 10 Jun 2022 21:17:34 +0200 Subject: [PATCH 08/14] Reject OCI artifacts in image.manifestOCI1.OCIConfig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- internal/image/oci.go | 4 ++++ internal/image/oci_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/internal/image/oci.go b/internal/image/oci.go index 49d58f278c..b6b2e52147 100644 --- a/internal/image/oci.go +++ b/internal/image/oci.go @@ -85,6 +85,10 @@ func (m *manifestOCI1) ConfigBlob(ctx context.Context) ([]byte, error) { // layers in the resulting configuration isn't guaranteed to be returned to due how // old image manifests work (docker v2s1 especially). func (m *manifestOCI1) OCIConfig(ctx context.Context) (*imgspecv1.Image, error) { + if m.m.Config.MediaType != imgspecv1.MediaTypeImageConfig { + return nil, internalManifest.NewNonImageArtifactError(m.m.Config.MediaType) + } + cb, err := m.ConfigBlob(ctx) if err != nil { return nil, err diff --git a/internal/image/oci_test.go b/internal/image/oci_test.go index ed83b948c7..bafc81d327 100644 --- a/internal/image/oci_test.go +++ b/internal/image/oci_test.go @@ -193,6 +193,31 @@ func TestManifestOCI1ConfigBlob(t *testing.T) { assert.Equal(t, configBlob, cb) } +func TestManifestOCI1OCIConfig(t *testing.T) { + // Just a smoke-test that the code can read the data… + configJSON, err := os.ReadFile("fixtures/oci1-config.json") + require.NoError(t, err) + expectedConfig := imgspecv1.Image{} + err = json.Unmarshal(configJSON, &expectedConfig) + require.NoError(t, err) + + originalSrc := newOCI1ImageSource(t, "httpd:latest") + for _, m := range []genericManifest{ + manifestOCI1FromFixture(t, originalSrc, "oci1.json"), + manifestOCI1FromComponentsLikeFixture(configJSON), + } { + config, err := m.OCIConfig(context.Background()) + require.NoError(t, err) + assert.Equal(t, &expectedConfig, config) + } + + // This can share originalSrc because the config digest is the same between oci1-artifact.json and oci1.json + artifact := manifestOCI1FromFixture(t, originalSrc, "oci1-artifact.json") + _, err = artifact.OCIConfig(context.Background()) + var expected manifest.NonImageArtifactError + assert.ErrorAs(t, err, &expected) +} + func TestManifestOCI1LayerInfo(t *testing.T) { for _, m := range []genericManifest{ manifestOCI1FromFixture(t, mocks.ForbiddenImageSource{}, "oci1.json"), From 2e6896f1563eec7c24fd860b7f27fc2106a2d9c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 10 Jun 2022 22:36:44 +0200 Subject: [PATCH 09/14] Pass imageCopier to copyBlobFromStream and blobPipelineCompressionStep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will allow us to refer to ic.c.cannotModifyManifestReason and image-specific properties; it just replaces the objects without changing the logic. Should not change behavior. Signed-off-by: Miloslav Trmač --- copy/blob.go | 22 +++++++++++----------- copy/compression.go | 42 +++++++++++++++++++++--------------------- copy/copy.go | 16 ++++++++-------- 3 files changed, 40 insertions(+), 40 deletions(-) diff --git a/copy/blob.go b/copy/blob.go index 4aea636c6f..234192f2ca 100644 --- a/copy/blob.go +++ b/copy/blob.go @@ -15,7 +15,7 @@ import ( // perhaps sending a copy to an io.Writer if getOriginalLayerCopyWriter != nil, // perhaps (de/re/)compressing it if canModifyBlob, // and returns a complete blobInfo of the copied blob. -func (c *copier) copyBlobFromStream(ctx context.Context, srcReader io.Reader, srcInfo types.BlobInfo, +func (ic *imageCopier) copyBlobFromStream(ctx context.Context, srcReader io.Reader, srcInfo types.BlobInfo, getOriginalLayerCopyWriter func(decompressor compressiontypes.DecompressorFunc) io.Writer, canModifyBlob bool, isConfig bool, toEncrypt bool, bar *progressBar, layerIndex int, emptyLayer bool) (types.BlobInfo, error) { if isConfig { // This is guaranteed by the caller, but set it here to be explicit. @@ -46,7 +46,7 @@ func (c *copier) copyBlobFromStream(ctx context.Context, srcReader io.Reader, sr stream.reader = bar.ProxyReader(stream.reader) // === Decrypt the stream, if required. - decryptionStep, err := c.blobPipelineDecryptionStep(&stream, srcInfo) + decryptionStep, err := ic.c.blobPipelineDecryptionStep(&stream, srcInfo) if err != nil { return types.BlobInfo{}, err } @@ -68,7 +68,7 @@ func (c *copier) copyBlobFromStream(ctx context.Context, srcReader io.Reader, sr // === Deal with layer compression/decompression if necessary // WARNING: If you are adding new reasons to change the blob, update also the OptimizeDestinationImageAlreadyExists // short-circuit conditions - compressionStep, err := c.blobPipelineCompressionStep(&stream, canModifyBlob, detectedCompression) + compressionStep, err := ic.blobPipelineCompressionStep(&stream, canModifyBlob, detectedCompression) if err != nil { return types.BlobInfo{}, err } @@ -80,17 +80,17 @@ func (c *copier) copyBlobFromStream(ctx context.Context, srcReader io.Reader, sr // Before relaxing this, see the original pull request’s review if there are other reasons to reject this. return types.BlobInfo{}, errors.New("Unable to support both decryption and encryption in the same copy") } - encryptionStep, err := c.blobPipelineEncryptionStep(&stream, toEncrypt, srcInfo, decryptionStep) + encryptionStep, err := ic.c.blobPipelineEncryptionStep(&stream, toEncrypt, srcInfo, decryptionStep) if err != nil { return types.BlobInfo{}, err } - // === Report progress using the c.progress channel, if required. - if c.progress != nil && c.progressInterval > 0 { + // === Report progress using the ic.c.progress channel, if required. + if ic.c.progress != nil && ic.c.progressInterval > 0 { progressReader := newProgressReader( stream.reader, - c.progress, - c.progressInterval, + ic.c.progress, + ic.c.progressInterval, srcInfo, ) defer progressReader.reportDone() @@ -99,14 +99,14 @@ func (c *copier) copyBlobFromStream(ctx context.Context, srcReader io.Reader, sr // === Finally, send the layer stream to dest. options := private.PutBlobOptions{ - Cache: c.blobInfoCache, + Cache: ic.c.blobInfoCache, IsConfig: isConfig, EmptyLayer: emptyLayer, } if !isConfig { options.LayerIndex = &layerIndex } - uploadedInfo, err := c.dest.PutBlobWithOptions(ctx, &errorAnnotationReader{stream.reader}, stream.info, options) + uploadedInfo, err := ic.c.dest.PutBlobWithOptions(ctx, &errorAnnotationReader{stream.reader}, stream.info, options) if err != nil { return types.BlobInfo{}, errors.Wrap(err, "writing blob") } @@ -138,7 +138,7 @@ func (c *copier) copyBlobFromStream(ctx context.Context, srcReader io.Reader, sr return types.BlobInfo{}, errors.Errorf("Internal error writing blob %s, blob with digest %s saved with digest %s", srcInfo.Digest, stream.info.Digest, uploadedInfo.Digest) } if digestingReader.validationSucceeded { - if err := compressionStep.recordValidatedDigestData(c, uploadedInfo, srcInfo, encryptionStep, decryptionStep); err != nil { + if err := compressionStep.recordValidatedDigestData(ic.c, uploadedInfo, srcInfo, encryptionStep, decryptionStep); err != nil { return types.BlobInfo{}, err } } diff --git a/copy/compression.go b/copy/compression.go index 3951cd8a04..85f608bef0 100644 --- a/copy/compression.go +++ b/copy/compression.go @@ -61,16 +61,16 @@ type bpCompressionStepData struct { // srcInfo is only used for error messages. // Returns data for other steps; the caller should eventually call updateCompressionEdits and perhaps recordValidatedBlobData, // and must eventually call close. -func (c *copier) blobPipelineCompressionStep(stream *sourceStream, canModifyBlob bool, +func (ic *imageCopier) blobPipelineCompressionStep(stream *sourceStream, canModifyBlob bool, detected bpDetectCompressionStepData) (*bpCompressionStepData, error) { // WARNING: If you are adding new reasons to change the blob, update also the OptimizeDestinationImageAlreadyExists // short-circuit conditions if canModifyBlob { for _, fn := range []func(*sourceStream, bpDetectCompressionStepData) (*bpCompressionStepData, error){ - c.bpcPreserveEncrypted, - c.bpcCompressUncompressed, - c.bpcRecompressCompressed, - c.bpcDecompressCompressed, + ic.bpcPreserveEncrypted, + ic.bpcCompressUncompressed, + ic.bpcRecompressCompressed, + ic.bpcDecompressCompressed, } { res, err := fn(stream, detected) if err != nil { @@ -81,11 +81,11 @@ func (c *copier) blobPipelineCompressionStep(stream *sourceStream, canModifyBlob } } } - return c.bpcPreserveOriginal(stream, detected), nil + return ic.bpcPreserveOriginal(stream, detected), nil } // bpcPreserveEncrypted checks if the input is encrypted, and returns a *bpCompressionStepData if so. -func (c *copier) bpcPreserveEncrypted(stream *sourceStream, _ bpDetectCompressionStepData) (*bpCompressionStepData, error) { +func (ic *imageCopier) bpcPreserveEncrypted(stream *sourceStream, _ bpDetectCompressionStepData) (*bpCompressionStepData, error) { if isOciEncrypted(stream.info.MediaType) { logrus.Debugf("Using original blob without modification for encrypted blob") // PreserveOriginal due to any compression not being able to be done on an encrypted blob unless decrypted @@ -100,17 +100,17 @@ func (c *copier) bpcPreserveEncrypted(stream *sourceStream, _ bpDetectCompressio } // bpcCompressUncompressed checks if we should be compressing an uncompressed input, and returns a *bpCompressionStepData if so. -func (c *copier) bpcCompressUncompressed(stream *sourceStream, detected bpDetectCompressionStepData) (*bpCompressionStepData, error) { - if c.dest.DesiredLayerCompression() == types.Compress && !detected.isCompressed { +func (ic *imageCopier) bpcCompressUncompressed(stream *sourceStream, detected bpDetectCompressionStepData) (*bpCompressionStepData, error) { + if ic.c.dest.DesiredLayerCompression() == types.Compress && !detected.isCompressed { logrus.Debugf("Compressing blob on the fly") var uploadedAlgorithm *compressiontypes.Algorithm - if c.compressionFormat != nil { - uploadedAlgorithm = c.compressionFormat + if ic.c.compressionFormat != nil { + uploadedAlgorithm = ic.c.compressionFormat } else { uploadedAlgorithm = defaultCompressionFormat } - reader, annotations := c.compressedStream(stream.reader, *uploadedAlgorithm) + reader, annotations := ic.c.compressedStream(stream.reader, *uploadedAlgorithm) // Note: reader must be closed on all return paths. stream.reader = reader stream.info = types.BlobInfo{ // FIXME? Should we preserve more data in src.info? @@ -130,9 +130,9 @@ func (c *copier) bpcCompressUncompressed(stream *sourceStream, detected bpDetect } // bpcRecompressCompressed checks if we should be recompressing a compressed input to another format, and returns a *bpCompressionStepData if so. -func (c *copier) bpcRecompressCompressed(stream *sourceStream, detected bpDetectCompressionStepData) (*bpCompressionStepData, error) { - if c.dest.DesiredLayerCompression() == types.Compress && detected.isCompressed && - c.compressionFormat != nil && c.compressionFormat.Name() != detected.format.Name() { +func (ic *imageCopier) bpcRecompressCompressed(stream *sourceStream, detected bpDetectCompressionStepData) (*bpCompressionStepData, error) { + if ic.c.dest.DesiredLayerCompression() == types.Compress && detected.isCompressed && + ic.c.compressionFormat != nil && ic.c.compressionFormat.Name() != detected.format.Name() { // When the blob is compressed, but the desired format is different, it first needs to be decompressed and finally // re-compressed using the desired format. logrus.Debugf("Blob will be converted") @@ -148,7 +148,7 @@ func (c *copier) bpcRecompressCompressed(stream *sourceStream, detected bpDetect } }() - recompressed, annotations := c.compressedStream(decompressed, *c.compressionFormat) + recompressed, annotations := ic.c.compressedStream(decompressed, *ic.c.compressionFormat) // Note: recompressed must be closed on all return paths. stream.reader = recompressed stream.info = types.BlobInfo{ // FIXME? Should we preserve more data in src.info? @@ -158,10 +158,10 @@ func (c *copier) bpcRecompressCompressed(stream *sourceStream, detected bpDetect succeeded = true return &bpCompressionStepData{ operation: types.PreserveOriginal, - uploadedAlgorithm: c.compressionFormat, + uploadedAlgorithm: ic.c.compressionFormat, uploadedAnnotations: annotations, srcCompressorName: detected.srcCompressorName, - uploadedCompressorName: c.compressionFormat.Name(), + uploadedCompressorName: ic.c.compressionFormat.Name(), closers: []io.Closer{decompressed, recompressed}, }, nil } @@ -169,8 +169,8 @@ func (c *copier) bpcRecompressCompressed(stream *sourceStream, detected bpDetect } // bpcDecompressCompressed checks if we should be decompressing a compressed input, and returns a *bpCompressionStepData if so. -func (c *copier) bpcDecompressCompressed(stream *sourceStream, detected bpDetectCompressionStepData) (*bpCompressionStepData, error) { - if c.dest.DesiredLayerCompression() == types.Decompress && detected.isCompressed { +func (ic *imageCopier) bpcDecompressCompressed(stream *sourceStream, detected bpDetectCompressionStepData) (*bpCompressionStepData, error) { + if ic.c.dest.DesiredLayerCompression() == types.Decompress && detected.isCompressed { logrus.Debugf("Blob will be decompressed") s, err := detected.decompressor(stream.reader) if err != nil { @@ -194,7 +194,7 @@ func (c *copier) bpcDecompressCompressed(stream *sourceStream, detected bpDetect } // bpcPreserveOriginal returns a *bpCompressionStepData for not changing the original blob. -func (c *copier) bpcPreserveOriginal(stream *sourceStream, detected bpDetectCompressionStepData) *bpCompressionStepData { +func (ic *imageCopier) bpcPreserveOriginal(stream *sourceStream, detected bpDetectCompressionStepData) *bpCompressionStepData { logrus.Debugf("Using original blob without modification") // Remember if the original blob was compressed, and if so how, so that if // LayerInfosForCopy() returned something that differs from what was in the diff --git a/copy/copy.go b/copy/copy.go index d54859239b..5f515e2d7c 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -1044,7 +1044,7 @@ func (ic *imageCopier) copyUpdatedConfigAndManifest(ctx context.Context, instanc return nil, "", errors.Wrap(err, "reading manifest") } - if err := ic.c.copyConfig(ctx, pendingImage); err != nil { + if err := ic.copyConfig(ctx, pendingImage); err != nil { return nil, "", err } @@ -1064,19 +1064,19 @@ func (ic *imageCopier) copyUpdatedConfigAndManifest(ctx context.Context, instanc } // copyConfig copies config.json, if any, from src to dest. -func (c *copier) copyConfig(ctx context.Context, src types.Image) error { +func (ic *imageCopier) copyConfig(ctx context.Context, src types.Image) error { srcInfo := src.ConfigInfo() if srcInfo.Digest != "" { - if err := c.concurrentBlobCopiesSemaphore.Acquire(ctx, 1); err != nil { + if err := ic.c.concurrentBlobCopiesSemaphore.Acquire(ctx, 1); err != nil { // This can only fail with ctx.Err(), so no need to blame acquiring the semaphore. return fmt.Errorf("copying config: %w", err) } - defer c.concurrentBlobCopiesSemaphore.Release(1) + defer ic.c.concurrentBlobCopiesSemaphore.Release(1) destInfo, err := func() (types.BlobInfo, error) { // A scope for defer - progressPool := c.newProgressPool() + progressPool := ic.c.newProgressPool() defer progressPool.Wait() - bar := c.createProgressBar(progressPool, false, srcInfo, "config", "done") + bar := ic.c.createProgressBar(progressPool, false, srcInfo, "config", "done") defer bar.Abort(false) configBlob, err := src.ConfigBlob(ctx) @@ -1084,7 +1084,7 @@ func (c *copier) copyConfig(ctx context.Context, src types.Image) error { return types.BlobInfo{}, errors.Wrapf(err, "reading config blob %s", srcInfo.Digest) } - destInfo, err := c.copyBlobFromStream(ctx, bytes.NewReader(configBlob), srcInfo, nil, false, true, false, bar, -1, false) + destInfo, err := ic.copyBlobFromStream(ctx, bytes.NewReader(configBlob), srcInfo, nil, false, true, false, bar, -1, false) if err != nil { return types.BlobInfo{}, err } @@ -1300,7 +1300,7 @@ func (ic *imageCopier) copyLayerFromStream(ctx context.Context, srcStream io.Rea } } - blobInfo, err := ic.c.copyBlobFromStream(ctx, srcStream, srcInfo, getDiffIDRecorder, ic.cannotModifyManifestReason == "", false, toEncrypt, bar, layerIndex, emptyLayer) // Sets err to nil on success + blobInfo, err := ic.copyBlobFromStream(ctx, srcStream, srcInfo, getDiffIDRecorder, ic.cannotModifyManifestReason == "", false, toEncrypt, bar, layerIndex, emptyLayer) // Sets err to nil on success return blobInfo, diffIDChan, err // We need the defer … pipeWriter.CloseWithError() to happen HERE so that the caller can block on reading from diffIDChan } From 59bc35e1b37be4733201b99b5782cddca8a12b3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 10 Jun 2022 22:40:26 +0200 Subject: [PATCH 10/14] Move the canModifyBlob decision inside copyBlobFromStream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... to eliminate at least one of the many parameters. Should not change behavior. Signed-off-by: Miloslav Trmač --- copy/blob.go | 6 ++---- copy/copy.go | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/copy/blob.go b/copy/blob.go index 234192f2ca..79374aa069 100644 --- a/copy/blob.go +++ b/copy/blob.go @@ -17,10 +17,8 @@ import ( // and returns a complete blobInfo of the copied blob. func (ic *imageCopier) copyBlobFromStream(ctx context.Context, srcReader io.Reader, srcInfo types.BlobInfo, getOriginalLayerCopyWriter func(decompressor compressiontypes.DecompressorFunc) io.Writer, - canModifyBlob bool, isConfig bool, toEncrypt bool, bar *progressBar, layerIndex int, emptyLayer bool) (types.BlobInfo, error) { - if isConfig { // This is guaranteed by the caller, but set it here to be explicit. - canModifyBlob = false - } + isConfig bool, toEncrypt bool, bar *progressBar, layerIndex int, emptyLayer bool) (types.BlobInfo, error) { + canModifyBlob := !isConfig && ic.cannotModifyManifestReason == "" // The copying happens through a pipeline of connected io.Readers; // that pipeline is built by updating stream. diff --git a/copy/copy.go b/copy/copy.go index 5f515e2d7c..81d4597e27 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -1084,7 +1084,7 @@ func (ic *imageCopier) copyConfig(ctx context.Context, src types.Image) error { return types.BlobInfo{}, errors.Wrapf(err, "reading config blob %s", srcInfo.Digest) } - destInfo, err := ic.copyBlobFromStream(ctx, bytes.NewReader(configBlob), srcInfo, nil, false, true, false, bar, -1, false) + destInfo, err := ic.copyBlobFromStream(ctx, bytes.NewReader(configBlob), srcInfo, nil, true, false, bar, -1, false) if err != nil { return types.BlobInfo{}, err } @@ -1300,7 +1300,7 @@ func (ic *imageCopier) copyLayerFromStream(ctx context.Context, srcStream io.Rea } } - blobInfo, err := ic.copyBlobFromStream(ctx, srcStream, srcInfo, getDiffIDRecorder, ic.cannotModifyManifestReason == "", false, toEncrypt, bar, layerIndex, emptyLayer) // Sets err to nil on success + blobInfo, err := ic.copyBlobFromStream(ctx, srcStream, srcInfo, getDiffIDRecorder, false, toEncrypt, bar, layerIndex, emptyLayer) // Sets err to nil on success return blobInfo, diffIDChan, err // We need the defer … pipeWriter.CloseWithError() to happen HERE so that the caller can block on reading from diffIDChan } From 00e7d0feee750d95c72e96217bef271ea2cecebb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 10 Jun 2022 22:43:02 +0200 Subject: [PATCH 11/14] Reorganize copyBlobFromStream a bit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change behavior. Signed-off-by: Miloslav Trmač --- copy/blob.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/copy/blob.go b/copy/blob.go index 79374aa069..dacff33d41 100644 --- a/copy/blob.go +++ b/copy/blob.go @@ -18,8 +18,6 @@ import ( func (ic *imageCopier) copyBlobFromStream(ctx context.Context, srcReader io.Reader, srcInfo types.BlobInfo, getOriginalLayerCopyWriter func(decompressor compressiontypes.DecompressorFunc) io.Writer, isConfig bool, toEncrypt bool, bar *progressBar, layerIndex int, emptyLayer bool) (types.BlobInfo, error) { - canModifyBlob := !isConfig && ic.cannotModifyManifestReason == "" - // The copying happens through a pipeline of connected io.Readers; // that pipeline is built by updating stream. // === Input: srcReader @@ -63,9 +61,10 @@ func (ic *imageCopier) copyBlobFromStream(ctx context.Context, srcReader io.Read originalLayerReader = stream.reader } - // === Deal with layer compression/decompression if necessary // WARNING: If you are adding new reasons to change the blob, update also the OptimizeDestinationImageAlreadyExists // short-circuit conditions + canModifyBlob := !isConfig && ic.cannotModifyManifestReason == "" + // === Deal with layer compression/decompression if necessary compressionStep, err := ic.blobPipelineCompressionStep(&stream, canModifyBlob, detectedCompression) if err != nil { return types.BlobInfo{}, err From 83034b5d15aabc1dc6e8984ed1ddcf76a2426533 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 16 Jun 2022 19:02:22 +0200 Subject: [PATCH 12/14] Add a srcInfo parameter to blobPipelineCompressionStep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We will use it for debug logs. Signed-off-by: Miloslav Trmač --- copy/blob.go | 2 +- copy/compression.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/copy/blob.go b/copy/blob.go index dacff33d41..020e703e81 100644 --- a/copy/blob.go +++ b/copy/blob.go @@ -65,7 +65,7 @@ func (ic *imageCopier) copyBlobFromStream(ctx context.Context, srcReader io.Read // short-circuit conditions canModifyBlob := !isConfig && ic.cannotModifyManifestReason == "" // === Deal with layer compression/decompression if necessary - compressionStep, err := ic.blobPipelineCompressionStep(&stream, canModifyBlob, detectedCompression) + compressionStep, err := ic.blobPipelineCompressionStep(&stream, canModifyBlob, srcInfo, detectedCompression) if err != nil { return types.BlobInfo{}, err } diff --git a/copy/compression.go b/copy/compression.go index 85f608bef0..f243838e13 100644 --- a/copy/compression.go +++ b/copy/compression.go @@ -58,10 +58,10 @@ type bpCompressionStepData struct { } // blobPipelineCompressionStep updates *stream to compress and/or decompress it. -// srcInfo is only used for error messages. +// srcInfo is primarily used for error messages. // Returns data for other steps; the caller should eventually call updateCompressionEdits and perhaps recordValidatedBlobData, // and must eventually call close. -func (ic *imageCopier) blobPipelineCompressionStep(stream *sourceStream, canModifyBlob bool, +func (ic *imageCopier) blobPipelineCompressionStep(stream *sourceStream, canModifyBlob bool, srcInfo types.BlobInfo, detected bpDetectCompressionStepData) (*bpCompressionStepData, error) { // WARNING: If you are adding new reasons to change the blob, update also the OptimizeDestinationImageAlreadyExists // short-circuit conditions From af70b53bbc59a59426ae35b3d17ca922fe0747d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 16 Jun 2022 19:07:13 +0200 Subject: [PATCH 13/14] Split findCompressionMIMETypeSet from compressionVariantMIMEType MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We will add another user of the lookup code. Erorr messages now use mimeType instead of mt, which were required required to be equal on that code path, now that mt is not visible. Should not change behavior. Signed-off-by: Miloslav Trmač --- manifest/common.go | 51 +++++++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/manifest/common.go b/manifest/common.go index 20955ab7ff..2b2014831b 100644 --- a/manifest/common.go +++ b/manifest/common.go @@ -118,6 +118,18 @@ type compressionMIMETypeSet map[string]string const mtsUncompressed = "" // A key in compressionMIMETypeSet for the uncompressed variant const mtsUnsupportedMIMEType = "" // A value in compressionMIMETypeSet that means “recognized but unsupported” +// findCompressionMIMETypeSet returns a pointer to a compressionMIMETypeSet in variantTable that contains a value of mimeType, or nil if not found +func findCompressionMIMETypeSet(variantTable []compressionMIMETypeSet, mimeType string) compressionMIMETypeSet { + for _, variants := range variantTable { + for _, mt := range variants { + if mt == mimeType { + return variants + } + } + } + return nil +} + // compressionVariantMIMEType returns a variant of mimeType for the specified algorithm (which may be nil // to mean "no compression"), based on variantTable. // The returned error will be a ManifestLayerCompressionIncompatibilityError if mimeType has variants @@ -130,29 +142,26 @@ func compressionVariantMIMEType(variantTable []compressionMIMETypeSet, mimeType if mimeType == mtsUnsupportedMIMEType { // Prevent matching against the {algo:mtsUnsupportedMIMEType} entries return "", fmt.Errorf("cannot update unknown MIME type") } - for _, variants := range variantTable { - for _, mt := range variants { - if mt == mimeType { // Found the variant - name := mtsUncompressed - if algorithm != nil { - name = algorithm.InternalUnstableUndocumentedMIMEQuestionMark() - } - if res, ok := variants[name]; ok { - if res != mtsUnsupportedMIMEType { - return res, nil - } - if name != mtsUncompressed { - return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("%s compression is not supported for type %q", name, mt)} - } - return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("uncompressed variant is not supported for type %q", mt)} - } - if name != mtsUncompressed { - return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("unknown compressed with algorithm %s variant for type %s", name, mt)} - } - // We can't very well say “the idea of no compression is unknown” - return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("uncompressed variant is not supported for type %q", mt)} + variants := findCompressionMIMETypeSet(variantTable, mimeType) + if variants != nil { + name := mtsUncompressed + if algorithm != nil { + name = algorithm.InternalUnstableUndocumentedMIMEQuestionMark() + } + if res, ok := variants[name]; ok { + if res != mtsUnsupportedMIMEType { + return res, nil + } + if name != mtsUncompressed { + return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("%s compression is not supported for type %q", name, mimeType)} } + return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("uncompressed variant is not supported for type %q", mimeType)} + } + if name != mtsUncompressed { + return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("unknown compressed with algorithm %s variant for type %s", name, mimeType)} } + // We can't very well say “the idea of no compression is unknown” + return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("uncompressed variant is not supported for type %q", mimeType)} } if algorithm != nil { return "", fmt.Errorf("unsupported MIME type for compression: %s", mimeType) From 6d458ba211ae8d14dd97128a50b9ccd1efb27c09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 10 Jun 2022 21:17:50 +0200 Subject: [PATCH 14/14] Introduce SourcedImage.CanChangeLayerCompression, use it in copy.Image MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Don't compress/decompress layers with unknown MIME types, and layers in OCI artifacts. - Don't even change manifest MIME types in these situations, whatever happens. - Don't substitute compressed/uncompressed variants (via TryReusingBlobWithOptions) for OCI artifacts, if we discover the same variants when copying images that refer to the same blobs. Note that this does _not_ restrict compression to algorithms supported by the SourcedImage, because that would prohibit a single-pass conversion from v2s2 to OCI while compressing to zstd [1], and that's a feature we currently exercise in tests. So, this prevents us from failing to copy OCI artifacts, but users of zstd still need to be careful about choosing OCI manually. [1] We would need to ask the _destination_ format handler about zstd, not the source-format SourcedImage, and we don't currently have that infrastructure. It's also not immediately clear how to combine this with the sequence of alternative manifest formats returned by determineManifestConversion. Signed-off-by: Miloslav Trmač --- copy/compression.go | 17 +++++++++++++---- copy/copy.go | 20 +++++++++++++------- internal/image/docker_schema1.go | 9 +++++++++ internal/image/docker_schema1_test.go | 9 +++++++++ internal/image/docker_schema2.go | 9 +++++++++ internal/image/docker_schema2_test.go | 11 +++++++++++ internal/image/manifest.go | 15 ++++++++++++--- internal/image/oci.go | 9 +++++++++ internal/image/oci_test.go | 14 ++++++++++++++ manifest/common.go | 10 ++++++++++ manifest/docker_schema2.go | 8 ++++++++ manifest/docker_schema2_test.go | 8 ++++++++ manifest/oci.go | 17 +++++++++++++++++ manifest/oci_test.go | 11 +++++++++++ 14 files changed, 153 insertions(+), 14 deletions(-) diff --git a/copy/compression.go b/copy/compression.go index f243838e13..99305a0398 100644 --- a/copy/compression.go +++ b/copy/compression.go @@ -65,7 +65,11 @@ func (ic *imageCopier) blobPipelineCompressionStep(stream *sourceStream, canModi detected bpDetectCompressionStepData) (*bpCompressionStepData, error) { // WARNING: If you are adding new reasons to change the blob, update also the OptimizeDestinationImageAlreadyExists // short-circuit conditions - if canModifyBlob { + layerCompressionChangeSupported := ic.src.CanChangeLayerCompression(stream.info.MediaType) + if !layerCompressionChangeSupported { + logrus.Debugf("Compression change for blob %s (%q) not supported", srcInfo.Digest, stream.info.MediaType) + } + if canModifyBlob && layerCompressionChangeSupported { for _, fn := range []func(*sourceStream, bpDetectCompressionStepData) (*bpCompressionStepData, error){ ic.bpcPreserveEncrypted, ic.bpcCompressUncompressed, @@ -81,7 +85,7 @@ func (ic *imageCopier) blobPipelineCompressionStep(stream *sourceStream, canModi } } } - return ic.bpcPreserveOriginal(stream, detected), nil + return ic.bpcPreserveOriginal(stream, detected, layerCompressionChangeSupported), nil } // bpcPreserveEncrypted checks if the input is encrypted, and returns a *bpCompressionStepData if so. @@ -194,14 +198,19 @@ func (ic *imageCopier) bpcDecompressCompressed(stream *sourceStream, detected bp } // bpcPreserveOriginal returns a *bpCompressionStepData for not changing the original blob. -func (ic *imageCopier) bpcPreserveOriginal(stream *sourceStream, detected bpDetectCompressionStepData) *bpCompressionStepData { +func (ic *imageCopier) bpcPreserveOriginal(stream *sourceStream, detected bpDetectCompressionStepData, + layerCompressionChangeSupported bool) *bpCompressionStepData { logrus.Debugf("Using original blob without modification") // Remember if the original blob was compressed, and if so how, so that if // LayerInfosForCopy() returned something that differs from what was in the // source's manifest, and UpdatedImage() needs to call UpdateLayerInfos(), // it will be able to correctly derive the MediaType for the copied blob. + // + // But don’t touch blobs in objects where we can’t change compression, + // so that src.UpdatedImage() doesn’t fail; assume that for such blobs + // LayerInfosForCopy() should not be making any changes in the first place. var algorithm *compressiontypes.Algorithm - if detected.isCompressed { + if layerCompressionChangeSupported && detected.isCompressed { algorithm = &detected.format } else { algorithm = nil diff --git a/copy/copy.go b/copy/copy.go index 81d4597e27..0df5952375 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -678,12 +678,14 @@ func (c *copier) copyOneImage(ctx context.Context, policyContext *signature.Poli cannotModifyManifestReason: cannotModifyManifestReason, ociEncryptLayers: options.OciEncryptLayers, } - // Ensure _this_ copy sees exactly the intended data when either processing a signed image or signing it. - // This may be too conservative, but for now, better safe than sorry, _especially_ on the SignBy path: - // The signature makes the content non-repudiable, so it very much matters that the signature is made over exactly what the user intended. - // We do intend the RecordDigestUncompressedPair calls to only work with reliable data, but at least there’s a risk - // that the compressed version coming from a third party may be designed to attack some other decompressor implementation, - // and we would reuse and sign it. + // Decide whether we can substitute blobs with semantic equivalents: + // - Don’t do that if we can’t modify the manifest at all + // - Ensure _this_ copy sees exactly the intended data when either processing a signed image or signing it. + // This may be too conservative, but for now, better safe than sorry, _especially_ on the SignBy path: + // The signature makes the content non-repudiable, so it very much matters that the signature is made over exactly what the user intended. + // We do intend the RecordDigestUncompressedPair calls to only work with reliable data, but at least there’s a risk + // that the compressed version coming from a third party may be designed to attack some other decompressor implementation, + // and we would reuse and sign it. ic.canSubstituteBlobs = ic.cannotModifyManifestReason == "" && options.SignBy == "" if err := ic.updateEmbeddedDockerReference(); err != nil { @@ -1143,6 +1145,10 @@ func (ic *imageCopier) copyLayer(ctx context.Context, srcInfo types.BlobInfo, to // Don’t read the layer from the source if we already have the blob, and optimizations are acceptable. if canAvoidProcessingCompleteLayer { + canChangeLayerCompression := ic.src.CanChangeLayerCompression(srcInfo.MediaType) + logrus.Debugf("Checking if we can reuse blob %s: general substitution = %v, compression for MIME type %q = %v", + srcInfo.Digest, ic.canSubstituteBlobs, srcInfo.MediaType, canChangeLayerCompression) + canSubstitute := ic.canSubstituteBlobs && ic.src.CanChangeLayerCompression(srcInfo.MediaType) // TODO: at this point we don't know whether or not a blob we end up reusing is compressed using an algorithm // that is acceptable for use on layers in the manifest that we'll be writing later, so if we end up reusing // a blob that's compressed with e.g. zstd, but we're only allowed to write a v2s2 manifest, this will cause @@ -1151,7 +1157,7 @@ func (ic *imageCopier) copyLayer(ctx context.Context, srcInfo types.BlobInfo, to // the ImageDestination interface lets us pass in. reused, blobInfo, err := ic.c.dest.TryReusingBlobWithOptions(ctx, srcInfo, private.TryReusingBlobOptions{ Cache: ic.c.blobInfoCache, - CanSubstitute: ic.canSubstituteBlobs, + CanSubstitute: canSubstitute, EmptyLayer: emptyLayer, LayerIndex: &layerIndex, SrcRef: srcRef, diff --git a/internal/image/docker_schema1.go b/internal/image/docker_schema1.go index 5f24970c37..94f776224e 100644 --- a/internal/image/docker_schema1.go +++ b/internal/image/docker_schema1.go @@ -246,3 +246,12 @@ func (m *manifestSchema1) convertToManifestOCI1(ctx context.Context, options *ty func (m *manifestSchema1) SupportsEncryption(context.Context) bool { return false } + +// CanChangeLayerCompression returns true if we can compress/decompress layers with mimeType in the current image +// (and the code can handle that). +// NOTE: Even if this returns true, the relevant format might not accept all compression algorithms; the set of accepted +// algorithms depends not on the current format, but possibly on the target of a conversion (if UpdatedImage converts +// to a different manifest format). +func (m *manifestSchema1) CanChangeLayerCompression(mimeType string) bool { + return true // There are no MIME types in the manifest, so we must assume a valid image. +} diff --git a/internal/image/docker_schema1_test.go b/internal/image/docker_schema1_test.go index 6aa7405e08..58f222b3c7 100644 --- a/internal/image/docker_schema1_test.go +++ b/internal/image/docker_schema1_test.go @@ -667,3 +667,12 @@ func TestConvertSchema1ToManifestOCIWithAnnotations(t *testing.T) { require.NoError(t, err) assert.NotEqual(t, res.LayerInfos(), layerInfoOverwrites) } + +func TestManifestSchema1CanChangeLayerCompression(t *testing.T) { + for _, m := range []genericManifest{ + manifestSchema1FromFixture(t, "schema1.json"), + manifestSchema1FromComponentsLikeFixture(t), + } { + assert.True(t, m.CanChangeLayerCompression("")) + } +} diff --git a/internal/image/docker_schema2.go b/internal/image/docker_schema2.go index ca55d96c2c..7dfd3c5d8c 100644 --- a/internal/image/docker_schema2.go +++ b/internal/image/docker_schema2.go @@ -402,3 +402,12 @@ func v1ConfigFromConfigJSON(configJSON []byte, v1ID, parentV1ID string, throwawa func (m *manifestSchema2) SupportsEncryption(context.Context) bool { return false } + +// CanChangeLayerCompression returns true if we can compress/decompress layers with mimeType in the current image +// (and the code can handle that). +// NOTE: Even if this returns true, the relevant format might not accept all compression algorithms; the set of accepted +// algorithms depends not on the current format, but possibly on the target of a conversion (if UpdatedImage converts +// to a different manifest format). +func (m *manifestSchema2) CanChangeLayerCompression(mimeType string) bool { + return m.m.CanChangeLayerCompression(mimeType) +} diff --git a/internal/image/docker_schema2_test.go b/internal/image/docker_schema2_test.go index b20c99d4b2..a6dc014406 100644 --- a/internal/image/docker_schema2_test.go +++ b/internal/image/docker_schema2_test.go @@ -653,3 +653,14 @@ func TestConvertSchema2ToManifestOCIWithAnnotations(t *testing.T) { require.NoError(t, err) assert.NotEqual(t, res.LayerInfos(), layerInfoOverwrites) } + +func TestManifestSchema2CanChangeLayerCompression(t *testing.T) { + for _, m := range []genericManifest{ + manifestSchema2FromFixture(t, mocks.ForbiddenImageSource{}, "schema2.json", false), + manifestSchema2FromComponentsLikeFixture(nil), + } { + assert.True(t, m.CanChangeLayerCompression(manifest.DockerV2Schema2LayerMediaType)) + // Some projects like to use squashfs and other unspecified formats for layers; don’t touch those. + assert.False(t, m.CanChangeLayerCompression("a completely unknown and quite possibly invalid MIME type")) + } +} diff --git a/internal/image/manifest.go b/internal/image/manifest.go index 36d70b5c23..6b5f345388 100644 --- a/internal/image/manifest.go +++ b/internal/image/manifest.go @@ -12,9 +12,8 @@ import ( ) // genericManifest is an interface for parsing, modifying image manifests and related data. -// Note that the public methods are intended to be a subset of types.Image -// so that embedding a genericManifest into structs works. -// will support v1 one day... +// The public methods are related to types.Image so that embedding a genericManifest implements most of it, +// but there are also public methods that are only visible by packages that can import c/image/internal/image. type genericManifest interface { serialize() ([]byte, error) manifestMIMEType() string @@ -51,6 +50,16 @@ type genericManifest interface { // the process of updating a manifest between different manifest types was to update then convert. // This resulted in some fields in the update being lost. This has been fixed by: https://github.com/containers/image/pull/836 SupportsEncryption(ctx context.Context) bool + + // The following methods are not a part of types.Image: + // === + + // CanChangeLayerCompression returns true if we can compress/decompress layers with mimeType in the current image + // (and the code can handle that). + // NOTE: Even if this returns true, the relevant format might not accept all compression algorithms; the set of accepted + // algorithms depends not on the current format, but possibly on the target of a conversion (if UpdatedImage converts + // to a different manifest format). + CanChangeLayerCompression(mimeType string) bool } // manifestInstanceFromBlob returns a genericManifest implementation for (manblob, mt) in src. diff --git a/internal/image/oci.go b/internal/image/oci.go index b6b2e52147..af1a90e82d 100644 --- a/internal/image/oci.go +++ b/internal/image/oci.go @@ -260,3 +260,12 @@ func (m *manifestOCI1) convertToManifestSchema1(ctx context.Context, options *ty func (m *manifestOCI1) SupportsEncryption(context.Context) bool { return true } + +// CanChangeLayerCompression returns true if we can compress/decompress layers with mimeType in the current image +// (and the code can handle that). +// NOTE: Even if this returns true, the relevant format might not accept all compression algorithms; the set of accepted +// algorithms depends not on the current format, but possibly on the target of a conversion (if UpdatedImage converts +// to a different manifest format). +func (m *manifestOCI1) CanChangeLayerCompression(mimeType string) bool { + return m.m.CanChangeLayerCompression(mimeType) +} diff --git a/internal/image/oci_test.go b/internal/image/oci_test.go index bafc81d327..bb72601a87 100644 --- a/internal/image/oci_test.go +++ b/internal/image/oci_test.go @@ -542,3 +542,17 @@ func TestConvertToV2S2WithInvalidMIMEType(t *testing.T) { _, err = manifestOCI1FromManifest(originalSrc, manifest) require.NoError(t, err) } + +func TestManifestOCI1CanChangeLayerCompression(t *testing.T) { + for _, m := range []genericManifest{ + manifestOCI1FromFixture(t, mocks.ForbiddenImageSource{}, "oci1.json"), + manifestOCI1FromComponentsLikeFixture(nil), + } { + assert.True(t, m.CanChangeLayerCompression(imgspecv1.MediaTypeImageLayerGzip)) + // Some projects like to use squashfs and other unspecified formats for layers; don’t touch those. + assert.False(t, m.CanChangeLayerCompression("a completely unknown and quite possibly invalid MIME type")) + } + + artifact := manifestOCI1FromFixture(t, mocks.ForbiddenImageSource{}, "oci1-artifact.json") + assert.False(t, artifact.CanChangeLayerCompression(imgspecv1.MediaTypeImageLayerGzip)) +} diff --git a/manifest/common.go b/manifest/common.go index 2b2014831b..9cf7dd3a94 100644 --- a/manifest/common.go +++ b/manifest/common.go @@ -218,3 +218,13 @@ type ManifestLayerCompressionIncompatibilityError struct { func (m ManifestLayerCompressionIncompatibilityError) Error() string { return m.text } + +// compressionVariantsRecognizeMIMEType returns true if variantTable contains data about compressing/decompressing layers with mimeType +// Note that the caller still needs to worry about a specific algorithm not being supported. +func compressionVariantsRecognizeMIMEType(variantTable []compressionMIMETypeSet, mimeType string) bool { + if mimeType == mtsUnsupportedMIMEType { // Prevent matching against the {algo:mtsUnsupportedMIMEType} entries + return false + } + variants := findCompressionMIMETypeSet(variantTable, mimeType) + return variants != nil // Alternatively, this could be len(variants) > 1, but really the caller should ask about a specific algorithm. +} diff --git a/manifest/docker_schema2.go b/manifest/docker_schema2.go index 1f4db54eed..8b3fbdd399 100644 --- a/manifest/docker_schema2.go +++ b/manifest/docker_schema2.go @@ -295,3 +295,11 @@ func (m *Schema2) ImageID([]digest.Digest) (string, error) { } return m.ConfigDescriptor.Digest.Hex(), nil } + +// CanChangeLayerCompression returns true if we can compress/decompress layers with mimeType in the current image +// (and the code can handle that). +// NOTE: Even if this returns true, the relevant format might not accept all compression algorithms; the set of accepted +// algorithms depends not on the current format, but possibly on the target of a conversion. +func (m *Schema2) CanChangeLayerCompression(mimeType string) bool { + return compressionVariantsRecognizeMIMEType(schema2CompressionMIMETypeSets, mimeType) +} diff --git a/manifest/docker_schema2_test.go b/manifest/docker_schema2_test.go index cd89b97016..bdd10d0987 100644 --- a/manifest/docker_schema2_test.go +++ b/manifest/docker_schema2_test.go @@ -203,3 +203,11 @@ func TestSchema2ImageID(t *testing.T) { require.NoError(t, err) assert.Equal(t, "b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7", id) } + +func TestSchema2CanChangeLayerCompression(t *testing.T) { + m := manifestSchema2FromFixture(t, "v2s2.manifest.json") + + assert.True(t, m.CanChangeLayerCompression(DockerV2Schema2LayerMediaType)) + // Some projects like to use squashfs and other unspecified formats for layers; don’t touch those. + assert.False(t, m.CanChangeLayerCompression("a completely unknown and quite possibly invalid MIME type")) +} diff --git a/manifest/oci.go b/manifest/oci.go index 2fa23e69cb..11927ab5ec 100644 --- a/manifest/oci.go +++ b/manifest/oci.go @@ -116,6 +116,12 @@ var oci1CompressionMIMETypeSets = []compressionMIMETypeSet{ // UpdateLayerInfos replaces the original layers with the specified BlobInfos (size+digest+urls+mediatype), in order (the root layer first, and then successive layered layers) // The returned error will be a manifest.ManifestLayerCompressionIncompatibilityError if any of the layerInfos includes a combination of CompressionOperation and // CompressionAlgorithm that isn't supported by OCI. +// +// It’s generally the caller’s responsibility to determine whether a particular edit is acceptable, rather than relying on +// failures of this function, because the layer is typically created _before_ UpdateLayerInfos is called, because UpdateLayerInfos needs +// to know the final digest). See OCI1.CanChangeLayerCompression for some help in determining this; other aspects like compression +// algorithms that might not be supported by a format, or the limited set of MIME types accepted for encryption, are not currently +// handled — that logic should eventually also be provided as OCI1 methods, not hard-coded in callers. func (m *OCI1) UpdateLayerInfos(layerInfos []types.BlobInfo) error { if len(m.Layers) != len(layerInfos) { return errors.Errorf("Error preparing updated manifest: layer count changed from %d to %d", len(m.Layers), len(layerInfos)) @@ -247,3 +253,14 @@ func (m *OCI1) ImageID([]digest.Digest) (string, error) { } return m.Config.Digest.Hex(), nil } + +// CanChangeLayerCompression returns true if we can compress/decompress layers with mimeType in the current image +// (and the code can handle that). +// NOTE: Even if this returns true, the relevant format might not accept all compression algorithms; the set of accepted +// algorithms depends not on the current format, but possibly on the target of a conversion. +func (m *OCI1) CanChangeLayerCompression(mimeType string) bool { + if m.Config.MediaType != imgspecv1.MediaTypeImageConfig { + return false + } + return compressionVariantsRecognizeMIMEType(oci1CompressionMIMETypeSets, mimeType) +} diff --git a/manifest/oci_test.go b/manifest/oci_test.go index b752051490..4eabc0545f 100644 --- a/manifest/oci_test.go +++ b/manifest/oci_test.go @@ -373,3 +373,14 @@ func TestOCI1ImageID(t *testing.T) { var expected NonImageArtifactError assert.ErrorAs(t, err, &expected) } + +func TestOCI1CanChangeLayerCompression(t *testing.T) { + m := manifestOCI1FromFixture(t, "ociv1.manifest.json") + + assert.True(t, m.CanChangeLayerCompression(imgspecv1.MediaTypeImageLayerGzip)) + // Some projects like to use squashfs and other unspecified formats for layers; don’t touch those. + assert.False(t, m.CanChangeLayerCompression("a completely unknown and quite possibly invalid MIME type")) + + artifact := manifestOCI1FromFixture(t, "ociv1.artifact.json") + assert.False(t, artifact.CanChangeLayerCompression(imgspecv1.MediaTypeImageLayerGzip)) +}