Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use relocation map during bundle fixup #109

Merged
merged 1 commit into from
Jun 14, 2021

Conversation

carolynvs
Copy link
Collaborator

@carolynvs carolynvs commented Jun 7, 2021

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. I've also tweaked the log messages so that you can see exactly which image is being pushed (i.e. pushing the relocated image instead always printing the original image name).

Fixes #108

@radu-matei
Copy link
Member

/brig run

@carolynvs carolynvs force-pushed the fixup-with-relocation-map branch 3 times, most recently from 76007cc to 4cf5358 Compare June 7, 2021 20:18
@carolynvs
Copy link
Collaborator Author

So the cnab-to-oci build is currently broken. It's on go 1.13 but using the latest of tools that no longer support 1.13. I played around with bumping to go 1.15 in this PR and ran into more issues. I'll try pinning the tools to older versions and see how that plays out, and then will open a separate PR with the fix for the build.

Copy link
Member

@vdice vdice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! Defer to maintainers @silvin-lubecki and @radu-matei for final approval.

Copy link
Member

@radu-matei radu-matei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs a rebase, then it's good to go.
Thank you!

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 <me@carolynvanslyck.com>
@carolynvs
Copy link
Collaborator Author

@radu-matei Rebased!

@radu-matei radu-matei merged commit e4d2bd5 into cnabio:main Jun 14, 2021
@carolynvs carolynvs deleted the fixup-with-relocation-map branch June 14, 2021 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Copy a bundle with a relocation map to another registry
3 participants