Skip to content

Commit

Permalink
Merge pull request #25200 from Luap99/artifact-single-manifest
Browse files Browse the repository at this point in the history
artifact: only allow single manifest
  • Loading branch information
openshift-merge-bot[bot] authored Feb 4, 2025
2 parents 538a6a3 + 6c06577 commit 16a72c8
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 @@ -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()
Expand All @@ -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() {
Expand Down

0 comments on commit 16a72c8

Please sign in to comment.