Skip to content

Commit

Permalink
artifact: only allow single manifest
Browse files Browse the repository at this point in the history
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 <pholzing@redhat.com>
  • Loading branch information
Luap99 committed Feb 4, 2025
1 parent e6a3523 commit 6c06577
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 61 deletions.
38 changes: 14 additions & 24 deletions pkg/libartifact/artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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)
Expand Down
42 changes: 13 additions & 29 deletions pkg/libartifact/store/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
12 changes: 4 additions & 8 deletions test/e2e/artifact_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,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()
Expand All @@ -122,11 +122,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() {
Expand Down

0 comments on commit 6c06577

Please sign in to comment.