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

Proposal : Building tarred bundles without being dependent on a OCI registry #692

Closed
wants to merge 28 commits into from

Conversation

ashpect
Copy link
Contributor

@ashpect ashpect commented Oct 10, 2023

No description provided.

@netlify
Copy link

netlify bot commented Oct 10, 2023

Deploy Preview for carvel ready!

Name Link
🔨 Latest commit 0bb2ba1
🔍 Latest deploy log https://app.netlify.com/sites/carvel/deploys/65e4968d98969600081fdc78
😎 Deploy Preview https://deploy-preview-692--carvel.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@ashpect ashpect changed the title Proposal : Package Repository to tar Proposal : Building tarred bundles without being dependent on a OCI registry Oct 11, 2023

#### imgpkg
imgpkg save command will be added to imgpkg. This command will create a tar file from the bundle image. The command will be used as follows : <br>
`imgpkg save -f examples/basic-step-2 --to-tar /tmp/my-image.tar`<br>
Copy link
Member

Choose a reason for hiding this comment

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

I like the proposed UX in this comment carvel-dev/imgpkg#55 (comment). It is more in line with the rest of the commands that we already have in imgpkg.

For this proposal, we could also mention the idea of "inflating" the bundle with all the associated images, nevertheless, I do not think this is a priority to be implemented, but it would be good to have the proposal for it approved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've looked at the comment and also modified the proposal based on our slack conversation.
I've added the inflate option in the future goals as well explaining it briefly.

@ashpect ashpect force-pushed the develop branch 3 times, most recently from eaf7a2e to 0171447 Compare October 22, 2023 23:09

### Other Approaches Considered

The idea behind the usage of `imgpkg push -b my.registry.io/some-name -f some-folder --to-oci-tar local-oci-format.tar` is to concentrate in a single command all the options to create a new image, that being to the registry or directly to disk.
Copy link
Member

Choose a reason for hiding this comment

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

@ashpect @joaopapereira I was curious if we have a real use case for this? Since we have a --tar flag in imgpkg copy, it makes sense to add --oci-tar flag to it, but we don't have a --to-tar in imgpkg push so I was thinking if we should add the --to-oci-tar flag or not.
cc @100mik

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • The --to-oci-tar flag for the imgpkg push will create a oci spec tar file from the bundle image. The command will be used as follows :
    imgpkg push -b registry.example.com/xyz -f some-folder --to-oci-tar local-oci-format.tar
  • Making copy also save bundle to disk was not consistent with the already existing commands of copy where a registry is involved for copying to. If someone just wants to create a oci tar spec from bundle, with push it looked more in touch with existing commands.
  • https://kubernetes.slack.com/archives/CH8KCCKA5/p1697895845820659?thread_ts=1697604738.773009&cid=CH8KCCKA5 (Some conversation for the thought process regarding the save and push commands)

Copy link
Member

Choose a reason for hiding this comment

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

My main concern with this is that the -b option is required (because we want to add labels if it is bundle), and because of that the first issue that we are trying to solve in this problem still remains ("Dependent on using a registry to create a tar file from a bundle image.")

Copy link
Member

@joaopapereira joaopapereira left a comment

Choose a reason for hiding this comment

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

Thanks for the proposal I made some comments on it

proposals/repo-to-tar/README.md Outdated Show resolved Hide resolved
proposals/repo-to-tar/README.md Outdated Show resolved Hide resolved
proposals/repo-to-tar/README.md Outdated Show resolved Hide resolved
proposals/repo-to-tar/README.md Show resolved Hide resolved
proposals/repo-to-tar/README.md Outdated Show resolved Hide resolved
```
where one of the files in sha256 is the actual file tar and others 2 are configs and manifests.<br>

2. Add functionality to `imgpkg save` to create and persist a oci tar file mentioned in point 1 which in turn is compatible with `imgpkg copy` command.<br>
Copy link
Member

Choose a reason for hiding this comment

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

As I said above I think we can skip this goal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the save command from the proposal.

ashpect and others added 19 commits March 3, 2024 20:55
Signed-off-by: ashpect <[email protected]>
Signed-off-by: ashpect <[email protected]>
* Proposal - Signatures for Carvel Artifacts

Signed-off-by: Thomas Vitale <[email protected]>

* Update proposal metadata

Signed-off-by: Thomas Vitale <[email protected]>

* Improve formatting

Signed-off-by: Thomas Vitale <[email protected]>

* Update proposal status

Signed-off-by: Thomas Vitale <[email protected]>

* Improve proposal after review

Signed-off-by: Thomas Vitale <[email protected]>

* Refined proposal for binaries + examples

Signed-off-by: Thomas Vitale <[email protected]>

* Fix typo

Signed-off-by: Thomas Vitale <[email protected]>

* Introduce changes to Carvel GH Action

Signed-off-by: Thomas Vitale <[email protected]>

* Update proposal status to 'accepted'

Signed-off-by: Thomas Vitale <[email protected]>

---------

Signed-off-by: Thomas Vitale <[email protected]>
Signed-off-by: ashpect <[email protected]>
Signed-off-by: Fritz Duchardt <[email protected]>
Signed-off-by: ashpect <[email protected]>
Signed-off-by: Joao Pereira <[email protected]>
Signed-off-by: ashpect <[email protected]>
…arvel-dev#690)

* add optional flag in git configuration to skip ssl verification

Signed-off-by: Varsha Munishwar <[email protected]>

* update flag name to dangerousSkipTLSVerify

Signed-off-by: Varsha Munishwar <[email protected]>

---------

Signed-off-by: Varsha Munishwar <[email protected]>
Signed-off-by: ashpect <[email protected]>
Signed-off-by: Praveen Rewar <[email protected]>
Signed-off-by: ashpect <[email protected]>
Signed-off-by: Praveen Rewar <[email protected]>
Signed-off-by: ashpect <[email protected]>
Signed-off-by: Praveen Rewar <[email protected]>
Signed-off-by: ashpect <[email protected]>
Signed-off-by: Praveen Rewar <[email protected]>
Signed-off-by: ashpect <[email protected]>
Signed-off-by: Praveen Rewar <[email protected]>
Signed-off-by: ashpect <[email protected]>
Signed-off-by: Praveen Rewar <[email protected]>
Signed-off-by: ashpect <[email protected]>
* add optional flag in git configuration to skip SSL/TLS verification (carvel-dev#690)

* add optional flag in git configuration to skip ssl verification

Signed-off-by: Varsha Munishwar <[email protected]>

* update flag name to dangerousSkipTLSVerify

Signed-off-by: Varsha Munishwar <[email protected]>

---------

Signed-off-by: Varsha Munishwar <[email protected]>
Signed-off-by: renuy <[email protected]>

* RoadMap update October 2023

Signed-off-by: renuy <[email protected]>

* roadmap

Signed-off-by: renuy <[email protected]>

* Adding kcrtl items to roadmap

Signed-off-by: renuy <[email protected]>

---------

Signed-off-by: Varsha Munishwar <[email protected]>
Signed-off-by: renuy <[email protected]>
Co-authored-by: Varsha Munishwar <[email protected]>
Co-authored-by: renuy <[email protected]>
Signed-off-by: ashpect <[email protected]>
Signed-off-by: Praveen Rewar <[email protected]>
Signed-off-by: ashpect <[email protected]>
Revert unintended change from carvel-dev#676

Signed-off-by: Praveen Rewar <[email protected]>
Signed-off-by: ashpect <[email protected]>
Signed-off-by: Joao Pereira <[email protected]>
Signed-off-by: ashpect <[email protected]>
joaopapereira and others added 9 commits March 3, 2024 20:55
Signed-off-by: Joao Pereira <[email protected]>
Signed-off-by: ashpect <[email protected]>
Signed-off-by: ashpect <[email protected]>
Signed-off-by: ashpect <[email protected]>
Add approvers

Co-authored-by: João Pereira <[email protected]>
Signed-off-by: ashpect <[email protected]>
Co-authored-by: João Pereira <[email protected]>
Signed-off-by: ashpect <[email protected]>
Co-authored-by: João Pereira <[email protected]>
Signed-off-by: ashpect <[email protected]>
Co-authored-by: João Pereira <[email protected]>
Signed-off-by: ashpect <[email protected]>
Signed-off-by: ashpect <[email protected]>
@renuy
Copy link
Contributor

renuy commented Mar 5, 2024

@ashpect ,
Please rebase your changes.
I think we should be good if all review comments are addressed

@joaopapereira
Copy link
Member

Should we close this PR since looks like we have a newer one?

@ashpect
Copy link
Contributor Author

ashpect commented Apr 18, 2024

Yes, Please feel free to close this pr. The new pr is here

@joaopapereira
Copy link
Member

Closing PR since we have a newer one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

8 participants