From 141eae99b85181277a05ac586089cbd25052c698 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 3 Feb 2025 12:18:16 +0100 Subject: [PATCH] artifact: only allow single manifest Allowing for multiple manifest per artifact just makes the code and cli design harder to work with it. It is not clear how mounting, extracting or edit on a multi manifest artifact should have worked. A single manifest should make the code much easier to work with. Signed-off-by: Paul Holzinger --- pkg/libartifact/artifact.go | 38 ++++++++++++------------------ pkg/libartifact/store/store.go | 42 +++++++++++----------------------- test/e2e/artifact_test.go | 12 ++++------ 3 files changed, 31 insertions(+), 61 deletions(-) diff --git a/pkg/libartifact/artifact.go b/pkg/libartifact/artifact.go index 326ea72f2c..7d8304e533 100644 --- a/pkg/libartifact/artifact.go +++ b/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) diff --git a/pkg/libartifact/store/store.go b/pkg/libartifact/store/store.go index fff26cb10d..8a0364c247 100644 --- a/pkg/libartifact/store/store.go +++ b/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 { diff --git a/test/e2e/artifact_test.go b/test/e2e/artifact_test.go index eb9e9a6a10..c3cfe504eb 100644 --- a/test/e2e/artifact_test.go +++ b/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() {