Skip to content

Commit

Permalink
Merge pull request #1847 from nalind/multiple-artifacts
Browse files Browse the repository at this point in the history 
Handle multiple artifacts correctly
  • Loading branch information
@openshift-merge-bot
openshift-merge-bot[bot] authored Feb 7, 2024
2 parents c7ab1e8 + f0557f0 commit 2e565c6
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 14 deletions.
22 changes: 14 additions & 8 deletions libimage/manifests/manifests.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"net/http"
"os"
"path/filepath"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -284,10 +285,8 @@ func (l *list) Reference(store storage.Store, multiple cp.ImageListSelection, in
}
case cp.CopySpecificImages:
for instance := range l.instances {
for _, allowed := range instances {
if instance == allowed {
whichInstances = append(whichInstances, instance)
}
if slices.Contains(instances, instance) {
whichInstances = append(whichInstances, instance)
}
}
}
Expand All @@ -304,8 +303,11 @@ func (l *list) Reference(store storage.Store, multiple cp.ImageListSelection, in
if err != nil {
return nil, err
}
subdir := 0
for artifactManifestDigest, contents := range l.artifacts.Manifests {
// create the blobs directory
subdir++
tmp := filepath.Join(tmp, strconv.Itoa(subdir))
blobsDir := filepath.Join(tmp, "blobs", artifactManifestDigest.Algorithm().String())
if err := os.MkdirAll(blobsDir, 0o700); err != nil {
return nil, fmt.Errorf("creating directory for blobs: %w", err)
Expand Down Expand Up @@ -891,20 +893,24 @@ func (l *list) AddArtifact(ctx context.Context, sys *types.SystemContext, option
}
l.artifacts.Manifests[artifactManifestDigest] = string(artifactManifestBytes)
l.artifacts.Layers[artifactManifestDigest] = nil
l.artifacts.Configs[artifactManifestDigest] = artifactManifest.Config.Digest
if configFilePath != "" {
l.artifacts.Configs[artifactManifestDigest] = artifactManifest.Config.Digest
l.artifacts.Detached[artifactManifest.Config.Digest] = configFilePath
l.artifacts.Files[artifactManifestDigest] = append(l.artifacts.Files[artifactManifestDigest], configFilePath)
}
if len(artifactManifest.Config.Data) != 0 {
l.artifacts.Configs[artifactManifestDigest] = artifactManifest.Config.Digest
} else {
l.artifacts.Blobs[artifactManifest.Config.Digest] = slices.Clone(artifactManifest.Config.Data)
}
for filePath, fileDigest := range fileDigests {
l.artifacts.Layers[artifactManifestDigest] = append(l.artifacts.Layers[artifactManifestDigest], fileDigest)
l.artifacts.Detached[fileDigest] = filePath
l.artifacts.Files[artifactManifestDigest] = append(l.artifacts.Files[artifactManifestDigest], filePath)
}
for _, layer := range layers {
if len(layer.Data) != 0 {
l.artifacts.Blobs[layer.Digest] = slices.Clone(layer.Data)
l.artifacts.Layers[artifactManifestDigest] = append(l.artifacts.Layers[artifactManifestDigest], layer.Digest)
}
}
// Add this artifact manifest to the image index.
if err := l.AddInstance(artifactManifestDigest, int64(len(artifactManifestBytes)), artifactManifest.MediaType, options.Platform.OS, options.Platform.Architecture, options.Platform.OSVersion, options.Platform.OSFeatures, options.Platform.Variant, nil, nil); err != nil {
return "", fmt.Errorf("adding artifact manifest for %q to image index: %w", files, err)
Expand Down
88 changes: 84 additions & 4 deletions libimage/manifests/manifests_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"os"
Expand All @@ -17,8 +18,10 @@ import (
"github.com/containerd/containerd/platforms"
"github.com/containers/common/pkg/manifests"
cp "github.com/containers/image/v5/copy"
"github.com/containers/image/v5/directory"
"github.com/containers/image/v5/manifest"
"github.com/containers/image/v5/pkg/compression"
"github.com/containers/image/v5/signature"
"github.com/containers/image/v5/transports"
"github.com/containers/image/v5/transports/alltransports"
"github.com/containers/image/v5/types"
Expand Down Expand Up @@ -471,29 +474,60 @@ func TestReference(t *testing.T) {
}
}()

policy, err := signature.NewPolicyFromFile(sys.SignaturePolicyPath)
assert.NoErrorf(t, err, "NewPolicyFromFile()")
policyContext, err := signature.NewPolicyContext(policy)
assert.NoErrorf(t, err, "NewPolicyContext()")
destRef, err := directory.NewReference(filepath.Join(dir, "directory"))
assert.NoErrorf(t, err, "NewReference()")
checkCopy := func(ref types.ImageReference, selection cp.ImageListSelection, instances []digest.Digest) error {
if ref == nil {
return errors.New("no reference")
}
copyOptions := &cp.Options{
ImageListSelection: selection,
SourceCtx: sys,
DestinationCtx: sys,
Instances: instances,
}
// t.Helper()
_, err := cp.Image(ctx, policyContext, destRef, ref, copyOptions)
return err
}
ref, err := alltransports.ParseImageName(otherListImage)
assert.NoErrorf(t, err, "ParseImageName(%q)", otherListImage)

list := Create()
_, err = list.Add(ctx, ppc64sys, ref, false)
assert.NoError(t, err, "list.Add(all=false)")

smallJSON := filepath.Join(dir, "small.json")
err = os.WriteFile(smallJSON, []byte(`{"slice":[1, 2, 3]}`), 0o600)
assert.NoError(t, err)

minimumJSON := filepath.Join(dir, "minimum.json")
err = os.WriteFile(minimumJSON, []byte("{}"), 0o600)
assert.NoError(t, err)

emptyJSON := filepath.Join(dir, "empty.json")
err = os.WriteFile(emptyJSON, []byte(""), 0o600)
assert.NoError(t, err)

artifactOptions := AddArtifactOptions{
ConfigFile: emptyJSON,
}
_, err = list.AddArtifact(ctx, &types.SystemContext{}, artifactOptions)
assert.NoErrorf(t, err, "list.AddArtifact(file=%s)", emptyJSON)

artifactOptions = AddArtifactOptions{
ConfigDescriptor: &v1.DescriptorEmptyJSON,
}
_, err = list.AddArtifact(ctx, &types.SystemContext{}, artifactOptions)
assert.NoError(t, err, "list.AddArtifact(empty descriptor)")
minimumArtifactDigest, err := list.AddArtifact(ctx, &types.SystemContext{}, artifactOptions, minimumJSON)
assert.NoError(t, err, "list.AddArtifact(file=%s)", minimumJSON)

artifactOptions = AddArtifactOptions{}
_, err = list.AddArtifact(ctx, &types.SystemContext{}, artifactOptions)
assert.NoError(t, err, "list.AddArtifact(no descriptor, no file)")
smallArtifactDigest, err := list.AddArtifact(ctx, &types.SystemContext{}, artifactOptions, smallJSON)
assert.NoError(t, err, "list.AddArtifact(file=%s)", smallJSON)

listRef, err := list.Reference(store, cp.CopyAllImages, nil)
assert.Error(t, err, "list.Reference(never saved)")
Expand All @@ -515,6 +549,18 @@ func TestReference(t *testing.T) {
assert.Error(t, err, "list.Reference(never saved)")
assert.Nilf(t, listRef, "list.Reference(never saved)")

listRef, err = list.Reference(store, cp.CopySpecificImages, []digest.Digest{otherListAmd64Digest, otherListArm64Digest, minimumArtifactDigest})
assert.Error(t, err, "list.Reference(never saved)")
assert.Nilf(t, listRef, "list.Reference(never saved)")
err = checkCopy(listRef, cp.CopySpecificImages, []digest.Digest{minimumArtifactDigest})
assert.Error(t, err, "list.Reference(saved)")

listRef, err = list.Reference(store, cp.CopySpecificImages, []digest.Digest{otherListAmd64Digest, otherListArm64Digest, minimumArtifactDigest, smallArtifactDigest})
assert.Error(t, err, "list.Reference(never saved)")
assert.Nilf(t, listRef, "list.Reference(never saved)")
err = checkCopy(listRef, cp.CopySpecificImages, []digest.Digest{minimumArtifactDigest, smallArtifactDigest})
assert.Error(t, err, "list.Reference(saved)")

_, err = list.SaveToImage(store, "", []string{listImageName}, "")
assert.NoError(t, err, "SaveToImage")

Expand All @@ -538,28 +584,62 @@ func TestReference(t *testing.T) {
assert.NoError(t, err, "list.Reference(saved)")
assert.NotNilf(t, listRef, "list.Reference(saved)")

listRef, err = list.Reference(store, cp.CopySpecificImages, []digest.Digest{otherListAmd64Digest, otherListArm64Digest, minimumArtifactDigest})
assert.NoError(t, err, "list.Reference(saved)")
assert.NotNilf(t, listRef, "list.Reference(saved)")
err = checkCopy(listRef, cp.CopySpecificImages, []digest.Digest{minimumArtifactDigest})
assert.NoError(t, err, "list.Reference(saved)")

listRef, err = list.Reference(store, cp.CopySpecificImages, []digest.Digest{otherListAmd64Digest, otherListArm64Digest, minimumArtifactDigest, smallArtifactDigest})
assert.NoError(t, err, "list.Reference(saved)")
assert.NotNilf(t, listRef, "list.Reference(saved)")
err = checkCopy(listRef, cp.CopySpecificImages, []digest.Digest{minimumArtifactDigest, smallArtifactDigest})
assert.NoError(t, err, "list.Reference(saved)")

_, err = list.Add(ctx, sys, ref, true)
assert.NoError(t, err, "list.Add(all=true)")

listRef, err = list.Reference(store, cp.CopyAllImages, nil)
assert.NoError(t, err, "list.Reference(saved)")
assert.NotNilf(t, listRef, "list.Reference(saved)")
err = checkCopy(listRef, cp.CopyAllImages, nil)
assert.NoError(t, err, "list.Reference(saved)")

listRef, err = list.Reference(store, cp.CopySystemImage, nil)
assert.NoError(t, err, "list.Reference(saved)")
assert.NotNilf(t, listRef, "list.Reference(saved)")
err = checkCopy(listRef, cp.CopySystemImage, nil)
assert.NoError(t, err, "list.Reference(saved)")

listRef, err = list.Reference(store, cp.CopySpecificImages, nil)
assert.NoError(t, err, "list.Reference(saved)")
assert.NotNilf(t, listRef, "list.Reference(saved)")
err = checkCopy(listRef, cp.CopySpecificImages, nil)
assert.NoError(t, err, "list.Reference(saved)")

listRef, err = list.Reference(store, cp.CopySpecificImages, []digest.Digest{otherListAmd64Digest})
assert.NoError(t, err, "list.Reference(saved)")
assert.NotNilf(t, listRef, "list.Reference(saved)")
err = checkCopy(listRef, cp.CopySpecificImages, []digest.Digest{otherListAmd64Digest})
assert.NoError(t, err, "list.Reference(saved)")

listRef, err = list.Reference(store, cp.CopySpecificImages, []digest.Digest{otherListAmd64Digest, otherListArm64Digest})
assert.NoError(t, err, "list.Reference(saved)")
assert.NotNilf(t, listRef, "list.Reference(saved)")
err = checkCopy(listRef, cp.CopySpecificImages, []digest.Digest{otherListAmd64Digest, otherListArm64Digest})
assert.NoError(t, err, "list.Reference(saved)")

listRef, err = list.Reference(store, cp.CopySpecificImages, []digest.Digest{otherListAmd64Digest, otherListArm64Digest, minimumArtifactDigest})
assert.NoError(t, err, "list.Reference(saved)")
assert.NotNilf(t, listRef, "list.Reference(saved)")
err = checkCopy(listRef, cp.CopySpecificImages, []digest.Digest{otherListAmd64Digest, otherListArm64Digest, minimumArtifactDigest})
assert.NoError(t, err, "list.Reference(saved)")

listRef, err = list.Reference(store, cp.CopySpecificImages, []digest.Digest{otherListAmd64Digest, otherListArm64Digest, minimumArtifactDigest, smallArtifactDigest})
assert.NoError(t, err, "list.Reference(saved)")
assert.NotNilf(t, listRef, "list.Reference(saved)")
err = checkCopy(listRef, cp.CopySpecificImages, []digest.Digest{otherListAmd64Digest, otherListArm64Digest, minimumArtifactDigest, smallArtifactDigest})
assert.NoError(t, err, "list.Reference(saved)")
}

func TestPushManifest(t *testing.T) {
Expand Down
9 changes: 7 additions & 2 deletions pkg/supplemented/supplemented.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
multierror "github.com/hashicorp/go-multierror"
digest "github.com/opencontainers/go-digest"
"github.com/sirupsen/logrus"
"golang.org/x/exp/slices"
)

// supplementedImageReference groups multiple references together.
Expand Down Expand Up @@ -139,7 +140,7 @@ func (s *supplementedImageReference) NewImageSource(ctx context.Context, sys *ty
}
sources[manifestDigest] = src

// Parse the manifest as a list of images.
// Parse the manifest as a list of images and artifacts.
list, err := manifest.ListFromBlob(manifestBytes, manifestType)
if err != nil {
return fmt.Errorf("parsing manifest blob %q as a %q: %w", string(manifestBytes), manifestType, err)
Expand All @@ -155,7 +156,11 @@ func (s *supplementedImageReference) NewImageSource(ctx context.Context, sys *ty
}
chaseInstances = []digest.Digest{instance}
case cp.CopySpecificImages:
chaseInstances = s.instances
for _, instance := range list.Instances() {
if slices.Contains(s.instances, instance) {
chaseInstances = append(chaseInstances, instance)
}
}
case cp.CopyAllImages:
chaseInstances = list.Instances()
}
Expand Down

0 comments on commit 2e565c6

Please sign in to comment.