-
Notifications
You must be signed in to change notification settings - Fork 99
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
Add initial draft for registry specification #362
Conversation
|
||
Runtimes and registries MAY choose to _relocate_ the images and invocation images referenced in the bundle to registry where the bundle is pushed, but in its simplest form, a CNAB bundle MAY be represented in an OCI registry by the canonical `bundle.json` file descriptor, referenced in an OCI index (or manifest list). | ||
|
||
![](https://i.imgur.com/PfTcKOm.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting! Would like to discuss this at the next Security/Registry meeting to make sure I fully understand the proposal here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM once @trishankatdatadog's comments have been addressed. We can follow up with annotations and air-gapped image storage once this is merged in master.
@radu-matei LMK if you'd like me to push to your branch with fixes for these issues if you don't have the bandwidth
7a1078a
to
eca0d78
Compare
Thanks a lot for the feedback! Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had a few notes/comments while reading through. None blocking, of course, since I'm not a part of the working group. Reads great!
eca0d78
to
165aac3
Compare
Addressed feedback, cleaned up the unused comments, moved the known implementation document to the non-normative section, and rebased. |
- References to the invocation images | ||
- References to the component images | ||
|
||
### Example |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, sorry, I'm confused. The example doesn't seem to match the preceding prescription, but I might just be missing something. Could someone please explain?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is happening because the example itself only contains the reference to bundle.json
and an invocation image, but it does follow this structure.
We could probably update this later with a more complete example.
|
||
### `bundle.json` manifest | ||
|
||
The `bundle.json` MUST be serialized as canonical JSON and MUST be stored in the registry as a blob. This blob MUST be referenced by its digest in an OCI manifest. The manifest media type SHOULD be `application/vnd.cnab.bundle.config.v1+json` but MAY be a standard container image config type (`application/vnd.oci.image.config.v1+json`) if the target registry does not support the CNAB media type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit/question: is application/vnd.cnab.bundle.config.v1+json
redundant? We've been using application/vnd.cnab.config.v1+json
internally. I can see the former being nice if we add more cnab-scoped artifact types in the future though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think at this point we should follow the guidance in the OCI Artifacts repository.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened #381 to track this.
e5064ab
to
846581f
Compare
Addressed the new round of feedback and rebased, PTAL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏
Signed-off-by: Radu M <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In our management meeting this morning, @chris-crone and I agreed that this PR is at the right point to be merged. At this point, continued work on this PR may actually be blocking other people from getting their PRs opened. And the outstanding issues seem to be covered well in the issue queue.
This is based on the work @chris-crone, @silvin-lubecki, and I have done in this HackMD document.
There are still sections that we need to add, but hopefully this gives everyone in the community better visibility in the process.