Skip to content

Commit 6c06577

Browse files
committed
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 <[email protected]>
1 parent e6a3523 commit 6c06577

File tree

3 files changed

+31
-61
lines changed

3 files changed

+31
-61
lines changed

pkg/libartifact/artifact.go

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,17 @@ import (
1111
)
1212

1313
type Artifact struct {
14-
Manifests []manifest.OCI1
15-
Name string
14+
// Manifest is the OCI manifest for the artifact with the name.
15+
// In a valid artifact the Manifest is guaranteed to not be nil.
16+
Manifest *manifest.OCI1
17+
Name string
1618
}
1719

1820
// TotalSizeBytes returns the total bytes of the all the artifact layers
1921
func (a *Artifact) TotalSizeBytes() int64 {
2022
var s int64
21-
for _, artifact := range a.Manifests {
22-
for _, layer := range artifact.Layers {
23-
s += layer.Size
24-
}
23+
for _, layer := range a.Manifest.Layers {
24+
s += layer.Size
2525
}
2626
return s
2727
}
@@ -45,13 +45,7 @@ func (a *Artifact) SetName(name string) {
4545
}
4646

4747
func (a *Artifact) GetDigest() (*digest.Digest, error) {
48-
if len(a.Manifests) > 1 {
49-
return nil, fmt.Errorf("not supported: multiple manifests found in artifact")
50-
}
51-
if len(a.Manifests) < 1 {
52-
return nil, fmt.Errorf("not supported: no manifests found in artifact")
53-
}
54-
b, err := json.Marshal(a.Manifests[0])
48+
b, err := json.Marshal(a.Manifest)
5549
if err != nil {
5650
return nil, err
5751
}
@@ -72,17 +66,13 @@ func (al ArtifactList) GetByNameOrDigest(nameOrDigest string) (*Artifact, bool,
7266
}
7367
// Before giving up, check by digest
7468
for _, artifact := range al {
75-
// TODO Here we have to assume only a single manifest for the artifact; this will
76-
// need to evolve
77-
if len(artifact.Manifests) > 0 {
78-
artifactDigest, err := artifact.GetDigest()
79-
if err != nil {
80-
return nil, false, err
81-
}
82-
// If the artifact's digest matches or is a prefix of ...
83-
if artifactDigest.Encoded() == nameOrDigest || strings.HasPrefix(artifactDigest.Encoded(), nameOrDigest) {
84-
return artifact, true, nil
85-
}
69+
artifactDigest, err := artifact.GetDigest()
70+
if err != nil {
71+
return nil, false, err
72+
}
73+
// If the artifact's digest matches or is a prefix of ...
74+
if artifactDigest.Encoded() == nameOrDigest || strings.HasPrefix(artifactDigest.Encoded(), nameOrDigest) {
75+
return artifact, true, nil
8676
}
8777
}
8878
return nil, false, fmt.Errorf("no artifact found with name or digest of %s", nameOrDigest)

pkg/libartifact/store/store.go

Lines changed: 13 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -299,13 +299,13 @@ func (as ArtifactStore) getArtifacts(ctx context.Context, _ *libartTypes.GetArti
299299
if err != nil {
300300
return nil, err
301301
}
302-
manifests, err := getManifests(ctx, imgSrc, nil)
302+
manifest, err := getManifest(ctx, imgSrc)
303303
imgSrc.Close()
304304
if err != nil {
305305
return nil, err
306306
}
307307
artifact := libartifact.Artifact{
308-
Manifests: manifests,
308+
Manifest: manifest,
309309
}
310310
if val, ok := l.ManifestDescriptor.Annotations[specV1.AnnotationRefName]; ok {
311311
artifact.SetName(val)
@@ -316,41 +316,25 @@ func (as ArtifactStore) getArtifacts(ctx context.Context, _ *libartTypes.GetArti
316316
return al, nil
317317
}
318318

319-
// getManifests takes an imgSrc and starting digest (nil means "top") and collects all the manifests "under"
320-
// it. this func calls itself recursively with a new startingDigest assuming that we are dealing with
321-
// an index list
322-
func getManifests(ctx context.Context, imgSrc types.ImageSource, startingDigest *digest.Digest) ([]manifest.OCI1, error) {
323-
var (
324-
manifests []manifest.OCI1
325-
)
326-
b, manifestType, err := imgSrc.GetManifest(ctx, startingDigest)
319+
// getManifest takes an imgSrc and returns the manifest for the imgSrc.
320+
// A OCI index list is not supported and will return an error.
321+
func getManifest(ctx context.Context, imgSrc types.ImageSource) (*manifest.OCI1, error) {
322+
b, manifestType, err := imgSrc.GetManifest(ctx, nil)
327323
if err != nil {
328324
return nil, err
329325
}
330326

331-
// this assumes that there are only single, and multi-images
332-
if !manifest.MIMETypeIsMultiImage(manifestType) {
333-
// these are the keepers
334-
mani, err := manifest.OCI1FromManifest(b)
335-
if err != nil {
336-
return nil, err
337-
}
338-
manifests = append(manifests, *mani)
339-
return manifests, nil
327+
// We only support a single flat manifest and not an oci index list
328+
if manifest.MIMETypeIsMultiImage(manifestType) {
329+
return nil, fmt.Errorf("manifest %q is index list", imgSrc.Reference().StringWithinTransport())
340330
}
341-
// We are dealing with an oci index list
342-
maniList, err := manifest.OCI1IndexFromManifest(b)
331+
332+
// parse the single manifest
333+
mani, err := manifest.OCI1FromManifest(b)
343334
if err != nil {
344335
return nil, err
345336
}
346-
for _, m := range maniList.Manifests {
347-
iterManifests, err := getManifests(ctx, imgSrc, &m.Digest)
348-
if err != nil {
349-
return nil, err
350-
}
351-
manifests = append(manifests, iterManifests...)
352-
}
353-
return manifests, nil
337+
return mani, nil
354338
}
355339

356340
func createEmptyStanza(path string) error {

test/e2e/artifact_test.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,9 @@ var _ = Describe("Podman artifact", func() {
9494
err = json.Unmarshal([]byte(inspectSingleSession.OutputToString()), &a)
9595
Expect(err).ToNot(HaveOccurred())
9696
Expect(a.Name).To(Equal(artifact1Name))
97-
Expect(a.Manifests[0].ArtifactType).To(Equal(artifactType))
98-
Expect(a.Manifests[0].Layers[0].Annotations["color"]).To(Equal("blue"))
99-
Expect(a.Manifests[0].Layers[0].Annotations["flavor"]).To(Equal("lemon"))
97+
Expect(a.Manifest.ArtifactType).To(Equal(artifactType))
98+
Expect(a.Manifest.Layers[0].Annotations["color"]).To(Equal("blue"))
99+
Expect(a.Manifest.Layers[0].Annotations["flavor"]).To(Equal("lemon"))
100100

101101
failSession := podmanTest.Podman([]string{"artifact", "add", "--annotation", "org.opencontainers.image.title=foobar", "foobar", artifact1File})
102102
failSession.WaitWithDefaultTimeout()
@@ -122,11 +122,7 @@ var _ = Describe("Podman artifact", func() {
122122
Expect(err).ToNot(HaveOccurred())
123123
Expect(a.Name).To(Equal(artifact1Name))
124124

125-
var layerCount int
126-
for _, layer := range a.Manifests {
127-
layerCount += len(layer.Layers)
128-
}
129-
Expect(layerCount).To(Equal(2))
125+
Expect(a.Manifest.Layers).To(HaveLen(2))
130126
})
131127

132128
It("podman artifact push and pull", func() {

0 commit comments

Comments
 (0)