Merge pull request #25200 from Luap99/artifact-single-manifest

artifact: only allow single manifest

Author: openshift-merge-bot[bot]

pkg/libartifact/artifact.go

@@ -11,17 +11,17 @@ import (
 )
 
 type Artifact struct {
-	Manifests []manifest.OCI1
-	Name      string
+	// Manifest is the OCI manifest for the artifact with the name.
+	// In a valid artifact the Manifest is guaranteed to not be nil.
+	Manifest *manifest.OCI1
+	Name     string
 }
 
 // TotalSizeBytes returns the total bytes of the all the artifact layers
 func (a *Artifact) TotalSizeBytes() int64 {
 	var s int64
-	for _, artifact := range a.Manifests {
-		for _, layer := range artifact.Layers {
-			s += layer.Size
-		}
+	for _, layer := range a.Manifest.Layers {
+		s += layer.Size
 	}
 	return s
 }
@@ -45,13 +45,7 @@ func (a *Artifact) SetName(name string) {
 }
 
 func (a *Artifact) GetDigest() (*digest.Digest, error) {
-	if len(a.Manifests) > 1 {
-		return nil, fmt.Errorf("not supported: multiple manifests found in artifact")
-	}
-	if len(a.Manifests) < 1 {
-		return nil, fmt.Errorf("not supported: no manifests found in artifact")
-	}
-	b, err := json.Marshal(a.Manifests[0])
+	b, err := json.Marshal(a.Manifest)
 	if err != nil {
 		return nil, err
 	}
@@ -72,17 +66,13 @@ func (al ArtifactList) GetByNameOrDigest(nameOrDigest string) (*Artifact, bool,
 	}
 	// Before giving up, check by digest
 	for _, artifact := range al {
-		// TODO Here we have to assume only a single manifest for the artifact; this will
-		// need to evolve
-		if len(artifact.Manifests) > 0 {
-			artifactDigest, err := artifact.GetDigest()
-			if err != nil {
-				return nil, false, err
-			}
-			// If the artifact's digest matches or is a prefix of ...
-			if artifactDigest.Encoded() == nameOrDigest || strings.HasPrefix(artifactDigest.Encoded(), nameOrDigest) {
-				return artifact, true, nil
-			}
+		artifactDigest, err := artifact.GetDigest()
+		if err != nil {
+			return nil, false, err
+		}
+		// If the artifact's digest matches or is a prefix of ...
+		if artifactDigest.Encoded() == nameOrDigest || strings.HasPrefix(artifactDigest.Encoded(), nameOrDigest) {
+			return artifact, true, nil
 		}
 	}
 	return nil, false, fmt.Errorf("no artifact found with name or digest of %s", nameOrDigest)

pkg/libartifact/store/store.go

@@ -299,13 +299,13 @@ func (as ArtifactStore) getArtifacts(ctx context.Context, _ *libartTypes.GetArti
 		if err != nil {
 			return nil, err
 		}
-		manifests, err := getManifests(ctx, imgSrc, nil)
+		manifest, err := getManifest(ctx, imgSrc)
 		imgSrc.Close()
 		if err != nil {
 			return nil, err
 		}
 		artifact := libartifact.Artifact{
-			Manifests: manifests,
+			Manifest: manifest,
 		}
 		if val, ok := l.ManifestDescriptor.Annotations[specV1.AnnotationRefName]; ok {
 			artifact.SetName(val)
@@ -316,41 +316,25 @@ func (as ArtifactStore) getArtifacts(ctx context.Context, _ *libartTypes.GetArti
 	return al, nil
 }
 
-// getManifests takes an imgSrc and starting digest (nil means "top") and collects all the manifests "under"
-// it.  this func calls itself recursively with a new startingDigest assuming that we are dealing with
-// an index list
-func getManifests(ctx context.Context, imgSrc types.ImageSource, startingDigest *digest.Digest) ([]manifest.OCI1, error) {
-	var (
-		manifests []manifest.OCI1
-	)
-	b, manifestType, err := imgSrc.GetManifest(ctx, startingDigest)
+// getManifest takes an imgSrc and returns the manifest for the imgSrc.
+// A OCI index list is not supported and will return an error.
+func getManifest(ctx context.Context, imgSrc types.ImageSource) (*manifest.OCI1, error) {
+	b, manifestType, err := imgSrc.GetManifest(ctx, nil)
 	if err != nil {
 		return nil, err
 	}
 
-	// this assumes that there are only single, and multi-images
-	if !manifest.MIMETypeIsMultiImage(manifestType) {
-		// these are the keepers
-		mani, err := manifest.OCI1FromManifest(b)
-		if err != nil {
-			return nil, err
-		}
-		manifests = append(manifests, *mani)
-		return manifests, nil
+	// We only support a single flat manifest and not an oci index list
+	if manifest.MIMETypeIsMultiImage(manifestType) {
+		return nil, fmt.Errorf("manifest %q is index list", imgSrc.Reference().StringWithinTransport())
 	}
-	// We are dealing with an oci index list
-	maniList, err := manifest.OCI1IndexFromManifest(b)
+
+	// parse the single manifest
+	mani, err := manifest.OCI1FromManifest(b)
 	if err != nil {
 		return nil, err
 	}
-	for _, m := range maniList.Manifests {
-		iterManifests, err := getManifests(ctx, imgSrc, &m.Digest)
-		if err != nil {
-			return nil, err
-		}
-		manifests = append(manifests, iterManifests...)
-	}
-	return manifests, nil
+	return mani, nil
 }
 
 func createEmptyStanza(path string) error {

test/e2e/artifact_test.go

@@ -100,9 +100,9 @@ var _ = Describe("Podman artifact", func() {
 		err = json.Unmarshal([]byte(inspectSingleSession.OutputToString()), &a)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(a.Name).To(Equal(artifact1Name))
-		Expect(a.Manifests[0].ArtifactType).To(Equal(artifactType))
-		Expect(a.Manifests[0].Layers[0].Annotations["color"]).To(Equal("blue"))
-		Expect(a.Manifests[0].Layers[0].Annotations["flavor"]).To(Equal("lemon"))
+		Expect(a.Manifest.ArtifactType).To(Equal(artifactType))
+		Expect(a.Manifest.Layers[0].Annotations["color"]).To(Equal("blue"))
+		Expect(a.Manifest.Layers[0].Annotations["flavor"]).To(Equal("lemon"))
 
 		failSession := podmanTest.Podman([]string{"artifact", "add", "--annotation", "org.opencontainers.image.title=foobar", "foobar", artifact1File})
 		failSession.WaitWithDefaultTimeout()
@@ -128,11 +128,7 @@ var _ = Describe("Podman artifact", func() {
 		Expect(err).ToNot(HaveOccurred())
 		Expect(a.Name).To(Equal(artifact1Name))
 
-		var layerCount int
-		for _, layer := range a.Manifests {
-			layerCount += len(layer.Layers)
-		}
-		Expect(layerCount).To(Equal(2))
+		Expect(a.Manifest.Layers).To(HaveLen(2))
 	})
 
 	It("podman artifact push and pull", func() {