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

Add initial draft for registry specification #362

Merged
merged 1 commit into from
Jul 29, 2020

Conversation

radu-matei
Copy link
Member

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.

@radu-matei radu-matei changed the title [WIP] Add initial draft for registry specification Add initial draft for registry specification Apr 22, 2020
@radu-matei radu-matei marked this pull request as ready for review April 22, 2020 17:36
200-CNAB-registries.md Outdated Show resolved Hide resolved
200-CNAB-registries.md Outdated Show resolved Hide resolved
200-CNAB-registries.md Outdated Show resolved Hide resolved

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)
Copy link
Member

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.

Copy link
Contributor

@chris-crone chris-crone left a 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

@radu-matei radu-matei force-pushed the registry-spec branch 2 times, most recently from 7a1078a to eca0d78 Compare June 10, 2020 05:26
@radu-matei
Copy link
Member Author

Thanks a lot for the feedback!
I addressed it and rebased, it should be ready for another review.

Thanks!

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.

Had a few notes/comments while reading through. None blocking, of course, since I'm not a part of the working group. Reads great!

200-CNAB-registries.md Outdated Show resolved Hide resolved
200-CNAB-registries.md Outdated Show resolved Hide resolved
200-CNAB-registries.md Show resolved Hide resolved
202-implementations.md Outdated Show resolved Hide resolved
201-representing-CNAB-in-OCI.md Outdated Show resolved Hide resolved
201-representing-CNAB-in-OCI.md Outdated Show resolved Hide resolved
201-representing-CNAB-in-OCI.md Outdated Show resolved Hide resolved
201-representing-CNAB-in-OCI.md Outdated Show resolved Hide resolved
@radu-matei
Copy link
Member Author

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
Copy link
Member

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?

Copy link
Member Author

@radu-matei radu-matei Jul 1, 2020

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.
Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member Author

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.

000-index.md Outdated Show resolved Hide resolved
200-CNAB-registries.md Outdated Show resolved Hide resolved
200-CNAB-registries.md Outdated Show resolved Hide resolved
201-representing-CNAB-in-OCI.md Outdated Show resolved Hide resolved
201-representing-CNAB-in-OCI.md Show resolved Hide resolved
201-representing-CNAB-in-OCI.md Show resolved Hide resolved
201-representing-CNAB-in-OCI.md Outdated Show resolved Hide resolved
201-representing-CNAB-in-OCI.md Show resolved Hide resolved
210-on-disk-representation.md Show resolved Hide resolved
807-registries-known-implementations.md Outdated Show resolved Hide resolved
@radu-matei radu-matei force-pushed the registry-spec branch 2 times, most recently from e5064ab to 846581f Compare July 1, 2020 17:35
@radu-matei
Copy link
Member Author

Addressed the new round of feedback and rebased, PTAL.

Copy link
Contributor

@carolynvs carolynvs left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@trishankatdatadog trishankatdatadog left a comment

Choose a reason for hiding this comment

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

👏

200-CNAB-registries.md Outdated Show resolved Hide resolved
Signed-off-by: Radu M <root@radu.sh>
Copy link
Member

@technosophos technosophos left a 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.

@radu-matei radu-matei merged commit 7c5957c into cnabio:master Jul 29, 2020
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.

None yet

8 participants