Skip to content

Commit

Permalink
add a check on the position of the image in the index before deleting
Browse files Browse the repository at this point in the history
Signed-off-by: Philippe Vlérick <[email protected]>
  • Loading branch information
Pvlerick committed Oct 10, 2023
1 parent a56e265 commit 2807bce
Show file tree
Hide file tree
Showing 14 changed files with 211 additions and 43 deletions.
2 changes: 1 addition & 1 deletion oci/layout/fixtures/delete_image_only_one_image/index.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
{
"mediaType": "application/vnd.oci.image.manifest.v1+json",
"digest": "sha256:eaa95f3cfaac07c8a5153eb77c933269586ad0226c83405776be08547e4d2a18",
"size": 405,
"size": 476,
"annotations": {
"org.opencontainers.image.ref.name": "latest"
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
insert binary content here #9671
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"schemaVersion": 2,
"mediaType": "application/vnd.oci.image.manifest.v1+json",
"config": {
"mediaType": "application/vnd.oci.image.config.v1+json",
"digest": "sha256:a527179158cd5cebc11c152b8637b47ce96c838ba2aa0de66d14f45cedc11423",
"size": 740
},
"layers": [
{
"mediaType": "application/vnd.oci.image.layer.v1.tar+gzip",
"digest": "sha256:0c8b263642b51b5c1dc40fe402ae2e97119c6007b6e52146419985ec1f0092dc",
"size": 33
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"created": "2019-08-20T20:19:55.211423266Z",
"architecture": "amd64",
"os": "linux",
"config": {
"Env": [
"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
],
"Cmd": [
"/bin/sh"
]
},
"rootfs": {
"type": "layers",
"diff_ids": [
"sha256:03901b4a2ea88eeaad62dbe59b072b28b6efa00491962b8741081c5df50c65e0"
]
},
"history": [
{
"created": "2019-08-20T20:19:55.062606894Z",
"created_by": "/bin/sh -c #(nop) ADD file:fe64057fbb83dccb960efabbf1cd8777920ef279a7fa8dbca0a8801c651bdf7c in / "
},
{
"created": "2019-08-20T20:19:55.211423266Z",
"created_by": "/bin/sh -c #(nop) CMD [\"/bin/sh\"]",
"empty_layer": true
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"created": "2019-08-20T20:20:55.211423266Z",
"architecture": "amd64",
"os": "linux",
"config": {
"Env": [
"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
],
"Cmd": [
"/bin/sh"
]
},
"rootfs": {
"type": "layers",
"diff_ids": [
"sha256:03901b4a2ea88eeaad62dbe59b072b28b6efa00491962b8741081c5df50c65e0"
]
},
"history": [
{
"created": "2019-08-20T20:19:55.062606894Z",
"created_by": "/bin/sh -c #(nop) ADD file:fe64057fbb83dccb960efabbf1cd8777920ef279a7fa8dbca0a8801c651bdf7c in / "
},
{
"created": "2019-08-20T20:19:55.211423266Z",
"created_by": "/bin/sh -c #(nop) CMD [\"/bin/sh\"]",
"empty_layer": true
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"schemaVersion": 2,
"mediaType": "application/vnd.oci.image.manifest.v1+json",
"config": {
"mediaType": "application/vnd.oci.image.config.v1+json",
"digest": "sha256:ce229a4eb5797ecd3a3a1846613b6b49811f79e38b5b0ce666268ba4b6c68e41",
"size": 740
},
"layers": [
{
"mediaType": "application/vnd.oci.image.layer.v1.tar+gzip",
"digest": "sha256:fa00bb4e2adbc73a5da1fd54d2a840020592530a8d4e8de9888b9e9a533419d8",
"size": 33
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
insert binary content here 32515
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"schemaVersion": 2,
"manifests": [
{
"mediaType": "application/vnd.oci.image.manifest.v1+json",
"digest": "sha256:49d1584496c6e196f512c4a9f52b17b187642269d84c044538523c5b69a660b3",
"size": 476,
"annotations": {
"org.opencontainers.image.ref.name": "1.0.0"
}
},
{
"mediaType": "application/vnd.oci.image.manifest.v1+json",
"digest": "sha256:ecfa463536cb5472e238aadc4df81d4785d5d6373027c488a2db8a6e76fe88ed",
"size": 476,
"annotations": {
"org.opencontainers.image.ref.name": "1.0.0"
}
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"imageLayoutVersion": "1.0.0"}
11 changes: 6 additions & 5 deletions oci/layout/oci_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func (ref ociReference) DeleteImage(ctx context.Context, sys *types.SystemContex
sharedBlobsDir = sys.OCISharedBlobDirPath
}

descriptor, err := ref.getManifestDescriptor()
descriptor, descriptorIndex, err := ref.getManifestDescriptor()
if err != nil {
return err
}
Expand Down Expand Up @@ -50,7 +50,7 @@ func (ref ociReference) DeleteImage(ctx context.Context, sys *types.SystemContex
return err
}

return ref.deleteReferenceFromIndex()
return ref.deleteReferenceFromIndex(descriptorIndex)
}

func (ref ociReference) getBlobsUsedInSingleImage(descriptor *imgspecv1.Descriptor, sharedBlobsDir string) (map[digest.Digest]int, error) {
Expand Down Expand Up @@ -184,7 +184,7 @@ func deleteBlob(blobPath string) error {
}
}

func (ref ociReference) deleteReferenceFromIndex() error {
func (ref ociReference) deleteReferenceFromIndex(referenceIndex int) error {
index, err := ref.getIndex()
if err != nil {
return err
Expand All @@ -196,8 +196,9 @@ func (ref ociReference) deleteReferenceFromIndex() error {
}

newDescriptors := make([]imgspecv1.Descriptor, 0, len(index.Manifests)-1)
for _, descriptor := range index.Manifests {
if descriptor.Annotations[imgspecv1.AnnotationRefName] != ref.image {
for i, descriptor := range index.Manifests {
// Double check: same image reference name and same position in the index
if descriptor.Annotations[imgspecv1.AnnotationRefName] != ref.image || referenceIndex != i {

This comment has been minimized.

Copy link
@Pvlerick

Pvlerick Oct 10, 2023

Author Contributor

I would invert the condition here, I always found !(x && y) was clearer than !y || !y

newDescriptors = append(newDescriptors, descriptor)
}
}
Expand Down
35 changes: 31 additions & 4 deletions oci/layout/oci_delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func TestReferenceDeleteImage_multipleImages(t *testing.T) {
err = ref.DeleteImage(context.Background(), nil)
require.NoError(t, err)

// Check that the relevant blobs were deleted/preservend
// Check that the relevant blobs were deleted/preserved
blobsDir := filepath.Join(tmpDir, "blobs")
files, err := os.ReadDir(filepath.Join(blobsDir, "sha256"))
require.NoError(t, err)
Expand Down Expand Up @@ -136,7 +136,7 @@ func TestReferenceDeleteImage_multipleImages_blobsUsedByOtherImages(t *testing.T
err = ref.DeleteImage(context.Background(), nil)
require.NoError(t, err)

// Check that the relevant blobs were deleted/preservend
// Check that the relevant blobs were deleted/preserved
blobsDir := filepath.Join(tmpDir, "blobs")
files, err := os.ReadDir(filepath.Join(blobsDir, "sha256"))
require.NoError(t, err)
Expand Down Expand Up @@ -192,7 +192,7 @@ func TestReferenceDeleteImage_multipleImages_nestedIndexImage(t *testing.T) {
err = ref.DeleteImage(context.Background(), nil)
require.NoError(t, err)

// Check that the relevant blobs were deleted/preservend
// Check that the relevant blobs were deleted/preserved
blobsDir := filepath.Join(tmpDir, "blobs")
files, err := os.ReadDir(filepath.Join(blobsDir, "sha256"))
require.NoError(t, err)
Expand Down Expand Up @@ -232,7 +232,7 @@ func TestReferenceDeleteImage_multipleImages_nestedIndexImage_refWithSameContent
err = ref.DeleteImage(context.Background(), nil)
require.NoError(t, err)

// Check that the relevant blobs were deleted/preservend
// Check that the relevant blobs were deleted/preserved
blobsDir := filepath.Join(tmpDir, "blobs")
files, err := os.ReadDir(filepath.Join(blobsDir, "sha256"))
require.NoError(t, err)
Expand All @@ -247,6 +247,33 @@ func TestReferenceDeleteImage_multipleImages_nestedIndexImage_refWithSameContent
require.Equal(t, 6, len(index.Manifests))
}

func TestReferenceDeleteImage_multipleImages_twoIdenticalReferences(t *testing.T) {
tmpDir := loadFixture(t, "delete_image_two_identical_references")

ref, err := NewReference(tmpDir, "1.0.0")
require.NoError(t, err)

err = ref.DeleteImage(context.Background(), nil)
require.NoError(t, err)

// Check that the relevant blobs were deleted/preserved - in this case only the first reference should be deleted
blobsDir := filepath.Join(tmpDir, "blobs")
files, err := os.ReadDir(filepath.Join(blobsDir, "sha256"))
require.NoError(t, err)
require.Equal(t, 3, len(files))
assertBlobExists(t, blobsDir, "sha256:ecfa463536cb5472e238aadc4df81d4785d5d6373027c488a2db8a6e76fe88ed")
assertBlobExists(t, blobsDir, "sha256:ce229a4eb5797ecd3a3a1846613b6b49811f79e38b5b0ce666268ba4b6c68e41")
assertBlobExists(t, blobsDir, "sha256:fa00bb4e2adbc73a5da1fd54d2a840020592530a8d4e8de9888b9e9a533419d8")

// Check the index
ociRef, ok := ref.(ociReference)
require.True(t, ok)
// .. Check that the index has been reduced to the correct size
index, err := ociRef.getIndex()
require.NoError(t, err)
require.Equal(t, 1, len(index.Manifests))
}

func loadFixture(t *testing.T, fixtureName string) string {
tmpDir := t.TempDir()
err := cp.Copy(fmt.Sprintf("fixtures/%v/", fixtureName), tmpDir)
Expand Down
2 changes: 1 addition & 1 deletion oci/layout/oci_src.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func newImageSource(sys *types.SystemContext, ref ociReference) (private.ImageSo

client := &http.Client{}
client.Transport = tr
descriptor, err := ref.getManifestDescriptor()
descriptor, _, err := ref.getManifestDescriptor()
if err != nil {
return nil, err
}
Expand Down
19 changes: 10 additions & 9 deletions oci/layout/oci_transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,35 +181,35 @@ func parseJSON[T any](path string) (*T, error) {
return obj, nil
}

func (ref ociReference) getManifestDescriptor() (imgspecv1.Descriptor, error) {
func (ref ociReference) getManifestDescriptor() (imgspecv1.Descriptor, int, error) {
index, err := ref.getIndex()
if err != nil {
return imgspecv1.Descriptor{}, err
return imgspecv1.Descriptor{}, -1, err
}

if ref.image == "" {
// return manifest if only one image is in the oci directory
if len(index.Manifests) != 1 {
// ask user to choose image when more than one image in the oci directory
return imgspecv1.Descriptor{}, ErrMoreThanOneImage
return imgspecv1.Descriptor{}, -1, ErrMoreThanOneImage
}
return index.Manifests[0], nil
return index.Manifests[0], 0, nil
} else {
// if image specified, look through all manifests for a match
var unsupportedMIMETypes []string
for _, md := range index.Manifests {
for i, md := range index.Manifests {
if refName, ok := md.Annotations[imgspecv1.AnnotationRefName]; ok && refName == ref.image {
if md.MediaType == imgspecv1.MediaTypeImageManifest || md.MediaType == imgspecv1.MediaTypeImageIndex {
return md, nil
return md, i, nil
}
unsupportedMIMETypes = append(unsupportedMIMETypes, md.MediaType)
}
}
if len(unsupportedMIMETypes) != 0 {
return imgspecv1.Descriptor{}, fmt.Errorf("reference %q matches unsupported manifest MIME types %q", ref.image, unsupportedMIMETypes)
return imgspecv1.Descriptor{}, -1, fmt.Errorf("reference %q matches unsupported manifest MIME types %q", ref.image, unsupportedMIMETypes)
}
}
return imgspecv1.Descriptor{}, ImageNotFoundError{ref}
return imgspecv1.Descriptor{}, -1, ImageNotFoundError{ref}
}

// LoadManifestDescriptor loads the manifest descriptor to be used to retrieve the image name
Expand All @@ -219,7 +219,8 @@ func LoadManifestDescriptor(imgRef types.ImageReference) (imgspecv1.Descriptor,
if !ok {
return imgspecv1.Descriptor{}, errors.New("error typecasting, need type ociRef")
}
return ociRef.getManifestDescriptor()
md, _, err := ociRef.getManifestDescriptor()
return md, err
}

// NewImageSource returns a types.ImageSource for this reference.
Expand Down
Loading

0 comments on commit 2807bce

Please sign in to comment.