Skip to content

Commit

Permalink
Use relocation map during bundle fixup
Browse files Browse the repository at this point in the history 
I believe the relocation map was accidentally not used after
WithRelocationMap is set, and it news up an empty relocation map before
fixing the images in the bundle.

I have updated the fixup to use the relocation mapping set by the
caller, and fixed the mocks in the tests to account for the new code
flow.

Signed-off-by: Carolyn Van Slyck <[email protected]>
  • Loading branch information
@carolynvs
carolynvs committed Jun 7, 2021
1 parent c859986 commit 76007cc
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 14 deletions.
13 changes: 9 additions & 4 deletions remotes/fixup.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func FixupBundle(ctx context.Context, b *bundle.Bundle, ref reference.Named, res
return nil, fmt.Errorf("only one invocation image supported for bundle %q", ref)
}

relocationMap := relocation.ImageRelocationMap{}
relocationMap := cfg.relocationMap
if err := fixupImage(ctx, "InvocationImage", &b.InvocationImages[0].BaseImage, relocationMap, cfg, events, cfg.invocationImagePlatformFilter); err != nil {
return nil, err
}
Expand All @@ -72,13 +72,18 @@ func fixupImage(
events chan<- FixupEvent,
platformFilter platforms.Matcher) error {

// Fixup the base image, using the relocated base image if available
sourceImage := *baseImage
if relocatedBaseImage, ok := relocationMap[baseImage.Image]; ok {
sourceImage.Image = relocatedBaseImage
}

log.G(ctx).Debugf("Updating entry in relocation map for %q", baseImage.Image)
ctx = withMutedContext(ctx)
notifyEvent, progress := makeEventNotifier(events, baseImage.Image, cfg.targetRef)
notifyEvent, progress := makeEventNotifier(events, sourceImage.Image, cfg.targetRef)

notifyEvent(FixupEventTypeCopyImageStart, "", nil)
// Fixup Base image
fixupInfo, pushed, err := fixupBaseImage(ctx, name, baseImage, cfg)
fixupInfo, pushed, err := fixupBaseImage(ctx, name, &sourceImage, cfg)
if err != nil {
return notifyError(notifyEvent, err)
}
Expand Down
96 changes: 87 additions & 9 deletions remotes/fixup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/opencontainers/go-digest"
ocischemav1 "github.com/opencontainers/image-spec/specs-go/v1"
"gotest.tools/assert"
"gotest.tools/assert/cmp"
)

func TestFixupBundleWithAutoUpdate(t *testing.T) {
Expand Down Expand Up @@ -162,6 +163,7 @@ func TestFixupBundlePushImages(t *testing.T) {
imageClient := newMockImageClient()
ref, err := reference.ParseNamed("my.registry/namespace/my-app")
assert.NilError(t, err)

_, err = FixupBundle(context.TODO(), b, ref, resolver, WithAutoBundleUpdate(), WithPushImages(imageClient, os.Stdout))
assert.NilError(t, err)
// 2 images has been pushed
Expand Down Expand Up @@ -196,7 +198,7 @@ func TestFixupBundlePushImages(t *testing.T) {
assert.DeepEqual(t, b, expectedBundle)
}

func TestFixupBundleCheckResolveOrder(t *testing.T) {
func TestFixupRelocatedBundle(t *testing.T) {
index := ocischemav1.Manifest{}
bufManifest, err := json.Marshal(index)
assert.NilError(t, err)
Expand All @@ -205,6 +207,82 @@ func TestFixupBundleCheckResolveOrder(t *testing.T) {
bytes.NewBuffer(bufManifest),
}}
pusher := &mockPusher{}
resolver := &mockResolver{
pusher: pusher,
fetcher: fetcher,
resolvedDescriptors: []ocischemav1.Descriptor{
// Invocation image will not be resolved first, so push will occurs
{
// just a code to raise an error in the mock
Size: -1,
},
// Invocation image is resolved after push
{
MediaType: ocischemav1.MediaTypeImageManifest,
Size: 65,
Digest: "sha256:beef1aa7866258751a261bae525a1842c7ff0662d4f34a355d5f36826abc0343",
},
{ // Trigger a push for the referenced image
Size: -1,
},
// Referenced image will be resolved after push based on Digest
{
MediaType: ocischemav1.MediaTypeImageManifest,
Size: 65,
Digest: "sha256:beef1aa7866258751a261bae525a1842c7ff0662d4f34a355d5f36826abc0344",
},
},
}
b := &bundle.Bundle{
SchemaVersion: "v1.0.0",
InvocationImages: []bundle.InvocationImage{
{
BaseImage: bundle.BaseImage{
Image: "my.registry/namespace/my-app-invoc",
ImageType: "docker",
},
},
},
Images: map[string]bundle.Image{
"my-service": {
BaseImage: bundle.BaseImage{
Digest: "sha256:beef1aa7866258751a261bae525a1842c7ff0662d4f34a355d5f36826abc0344",
Image: "my.registry/namespace/my-service",
ImageType: "docker",
},
},
},
Name: "my-app",
Version: "0.1.0",
}
imageClient := newMockImageClient()
ref, err := reference.ParseNamed("my.registry/namespace/my-app")
assert.NilError(t, err)

rm := relocation.ImageRelocationMap{
"my.registry/namespace/my-app-invoc":"localhost:5000/my-app-invoc",
"my.registry/namespace/my-service":"localhost:5000/my-service",
}
_, err = FixupBundle(context.TODO(), b, ref, resolver, WithRelocationMap(rm), WithAutoBundleUpdate(), WithPushImages(imageClient, os.Stdout))
assert.NilError(t, err)

// Check that the relocated image was pushed and not the original
assert.Equal(t, 2, len(imageClient.taggedImages), "expected 2 images pushed")
assert.Assert(t, cmp.Contains(imageClient.taggedImages, "localhost:5000/my-app-invoc"), "expected the relocated invocation image to be pushed, not the original")
assert.Assert(t, cmp.Contains(imageClient.taggedImages, "localhost:5000/my-service"), "expected the relocated referenced image to be pushed, not the original")
}

func TestFixupBundleCheckResolveOrder(t *testing.T) {
index := ocischemav1.Manifest{}
bufManifest, err := json.Marshal(index)
assert.NilError(t, err)
fetcher := &mockFetcher{indexBuffers: []*bytes.Buffer{
// Manifest index for each relocated image
bytes.NewBuffer(bufManifest),
bytes.NewBuffer(bufManifest),
bytes.NewBuffer(bufManifest),
}}
pusher := &mockPusher{}

// Construct a test case
// - invocation map will be found in relocation map, resolved and copy
Expand Down Expand Up @@ -276,13 +354,13 @@ func TestFixupBundleCheckResolveOrder(t *testing.T) {
// Resolvable
{
MediaType: ocischemav1.MediaTypeImageManifest,
Size: 42,
Size: 65,
Digest: "sha256:beef1aa7866258751a261bae525a1842c7ff0662d4f34a355d5f36826abc0343",
},
// This one is from the copy task
{
MediaType: ocischemav1.MediaTypeImageManifest,
Size: 42,
Size: 65,
Digest: "sha256:beef1aa7866258751a261bae525a1842c7ff0662d4f34a355d5f36826abc0343",
},

Expand All @@ -294,13 +372,13 @@ func TestFixupBundleCheckResolveOrder(t *testing.T) {
// resolved by second pass, from the bundle
{
MediaType: ocischemav1.MediaTypeImageManifest,
Size: 42,
Size: 65,
Digest: "sha256:beef1aa7866258751a261bae525a1842c7ff0662d4f34a355d5f36826abc0343",
},
// copy task
{
MediaType: ocischemav1.MediaTypeImageManifest,
Size: 42,
Size: 65,
Digest: "sha256:beef1aa7866258751a261bae525a1842c7ff0662d4f34a355d5f36826abc0343",
},

Expand All @@ -316,21 +394,21 @@ func TestFixupBundleCheckResolveOrder(t *testing.T) {
// image is pushed, resolve is called at the end
{
MediaType: ocischemav1.MediaTypeImageManifest,
Size: 42,
Size: 65,
Digest: "sha256:beef1aa7866258751a261bae525a1842c7ff0662d4f34a355d5f36826abc0343",
},

// Third image "not-in-relocation"
// not in relocation map but resolvable
{
MediaType: ocischemav1.MediaTypeImageManifest,
Size: 42,
Size: 65,
Digest: "sha256:beef1aa7866258751a261bae525a1842c7ff0662d4f34a355d5f36826abc0343",
},
// copy task
{
MediaType: ocischemav1.MediaTypeImageManifest,
Size: 42,
Size: 65,
Digest: "sha256:beef1aa7866258751a261bae525a1842c7ff0662d4f34a355d5f36826abc0343",
},

Expand All @@ -342,7 +420,7 @@ func TestFixupBundleCheckResolveOrder(t *testing.T) {
// image is pushed, resolve is called at the end
{
MediaType: ocischemav1.MediaTypeImageManifest,
Size: 42,
Size: 65,
Digest: "sha256:beef1aa7866258751a261bae525a1842c7ff0662d4f34a355d5f36826abc0343",
},
},
Expand Down
4 changes: 3 additions & 1 deletion remotes/mocks_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,16 +109,18 @@ func (rc mockReadCloser) Close() error {

type mockImageClient struct {
pushedImages int
taggedImages map[string]string
}

func newMockImageClient() *mockImageClient {
return &mockImageClient{}
return &mockImageClient{ taggedImages: map[string]string{}}
}

func (c *mockImageClient) ImagePush(ctx context.Context, ref string, options types.ImagePushOptions) (io.ReadCloser, error) {
c.pushedImages++
return mockReadCloser{}, nil
}
func (c *mockImageClient) ImageTag(ctx context.Context, image, ref string) error {
c.taggedImages[image] = ref
return nil
}

0 comments on commit 76007cc

Please sign in to comment.