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

Publish from archive should use a relocation map #1819

Closed
carolynvs opened this issue Nov 8, 2021 · 4 comments
Closed

Publish from archive should use a relocation map #1819

carolynvs opened this issue Nov 8, 2021 · 4 comments
Assignees
Labels
2 - 🍕 Pizza should be eaten daily bug Oops, sorry! cnab Related to the CNAB spec
Milestone

Comments

@carolynvs
Copy link
Member

I was reviewing this PR and that we don't use a relocation mapping file and preserve the original bundle.

See this very wonderfully long function comment for context:

// publishFromArchive (re-)publishes a bundle, provided by the archive file, using the provided tag.
//
// After the bundle is extracted from the archive, we iterate through all of the images (invocation
// and application) listed in the bundle, grab their digests by parsing the extracted
// OCI Layout, rename each based on the registry/org values derived from the provided tag
// and then push each updated image with the original digests
//
// Finally, we generate a new bundle from the old, with all image names and digests updated, based
// on the newly copied images, and then push this new bundle using the provided tag.
// (Currently we use the docker/cnab-to-oci library for this logic.)
//
// In the generation of a new bundle, we therefore don't preserve content digests and can't maintain
// signature verification throughout the process. Once we wish to preserve content digest and such verification,
// this approach will need to be refactored, via preserving the original bundle and employing
// a relocation mapping approach to associate the bundle's (old) images with the newly copied images.

This PR is correcting a bug in Porter where when we copy a bundle from one registry to another, we aren't preserving the relocation map. I was checking all the code that I thought would use that function and wasn't clear on why we are essentially "re-publishing" the bundle from the archive instead of preserving the original.

Originally posted by @carolynvs in #1815 (comment)

Vaughn confirmed that we should update the implementation of publish from archive to preserve the original bundle and use a relocation mapping file.

@carolynvs carolynvs added 2 - 🍕 Pizza should be eaten daily gap We missed a spot labels Nov 8, 2021
@carolynvs carolynvs added this to Inbox in Porter and Mixins via automation Nov 8, 2021
@jasmdk
Copy link
Contributor

jasmdk commented Jan 10, 2022

I see a behavior which may be the same root cause. I have bundles which are airgapped (includes images from docker.io) and this bundle is pushed to my own registry.
During my continuous integration, I do "porter archive --reference ...." to archive the bundle into a tgz which I can move to my airgapped environment.
On my build agents where I am not authenticated against docker.io and therefore have a lower pull rate limit combined with the fact that all access to docker.io from our corporate location is represented by the same public ip, then I get pull rate limit errors during the "archive" action where I would suspect all the image information is already included, however something insists invoking the docker.io API.
This is hard to reproduce, but once I reached the limit I can not archive my bundle:

porter archive busy.tgz --reference host.docker.internal:5443/porter-hello:v0.1.0

Error: error preparing artifacts: TOOMANYREQUESTS: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit

@carolynvs carolynvs added this to the 1.0 milestone Jan 10, 2022
@carolynvs carolynvs added the cnab Related to the CNAB spec label Jan 10, 2022
@carolynvs
Copy link
Member Author

It very well could be related but I'm not confident. I'll try to reproduce and debug this week, as between this and your previous issue I think I have enough to go on.

@carolynvs carolynvs self-assigned this Jan 10, 2022
@carolynvs carolynvs added bug Oops, sorry! and removed gap We missed a spot labels Jan 10, 2022
@carolynvs carolynvs removed their assignment Feb 1, 2022
@carolynvs carolynvs moved this from Inbox to Backlog in Porter and Mixins Feb 22, 2022
@carolynvs
Copy link
Member Author

I found the original issue that Vaughn made to track this work: #739

@github-actions
Copy link

Closed by #2186.

Porter and Mixins automation moved this from Backlog to Done Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 - 🍕 Pizza should be eaten daily bug Oops, sorry! cnab Related to the CNAB spec
Projects
Development

No branches or pull requests

3 participants