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

Helm 3: OCI - Move metadata to manifest annotations, expand layers #6141

Closed
jdolitsky opened this issue Aug 1, 2019 · 12 comments

Comments

@jdolitsky
Copy link
Member

commented Aug 1, 2019

Currently, charts stored in a registry are divided into the following 2 layers:

  • application/vnd.cncf.helm.chart.meta.layer.v1+json
  • application/vnd.cncf.helm.chart.content.layer.v1+tar

Ideally, metadata should be stored on the manifest config as annotations. Additionally, the charts layers should be a collections of the components that compose a chart, for example:

  • README.md -> application/vnd.cncf.helm.chart.readme.layer.v1+md
  • values.yaml -> application/vnd.cncf.helm.chart.values.layer.v1+yaml
  • templates/NOTES.txt -> application/vnd.cncf.helm.chart.notes.layer.v1+txt
  • templates/_helpers.tpl -> application/vnd.cncf.helm.chart.helpers.layer.v1+tpl
  • templates/*.yaml -> application/vnd.cncf.helm.chart.template.layer.v1+yaml
  • templates/tests/*.yaml -> application/vnd.cncf.helm.chart.test.layer.v1+yaml

A current Helm 3 manifest (alpha2) looks like the following:

{
  "schemaVersion": 2,
  "config": {
    "mediaType": "application/vnd.cncf.helm.config.v1+json",
    "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
    "size": 2
  },
  "layers": [
    {
      "mediaType": "application/vnd.cncf.helm.chart.meta.layer.v1+json",
      "digest": "sha256:4204a6f7d3dc3b6214a0b3af6d2977f517b5c2da70120c9ed1c4f4188d005eff",
      "size": 617,
      "annotations": {
        "org.opencontainers.image.title": "chart-meta.json"
      }
    },
    {
      "mediaType": "application/vnd.cncf.helm.chart.content.layer.v1+tar",
      "digest": "sha256:4d85513409ca6ac4657aa7d47708426606368c424216633b99057e93e9a84277",
      "size": 12448,
      "annotations": {
        "org.opencontainers.image.title": "chart-content.tgz",
        "sh.helm.chart.name": "wordpress",
        "sh.helm.chart.version": "7.0.1"
      }
    }
  ]
}

Using the following script to upload a sample wordpress chart with ORAS:

echo '{}' > config.json
ruby -ryaml -rjson -e 'puts JSON.pretty_generate({"$config" => YAML.load(ARGF)})' \
    < Chart.yaml > annotations.json

oras push \
    --manifest-config config.json:application/vnd.cncf.helm.config.v1+json \
    localhost:5000/stable/wordpress:7.0.1 \
    README.md:application/vnd.cncf.helm.chart.readme.layer.v1+md \
    values.yaml:application/vnd.cncf.helm.chart.values.layer.v1+yaml \
    templates/NOTES.txt:application/vnd.cncf.helm.chart.notes.layer.v1+txt \
    templates/_helpers.tpl:application/vnd.cncf.helm.chart.helpers.layer.v1+tpl \
    templates/deployment.yaml:application/vnd.cncf.helm.chart.template.layer.v1+yaml \
    templates/ingress.yaml:application/vnd.cncf.helm.chart.template.layer.v1+yaml \
    templates/pvc.yaml:application/vnd.cncf.helm.chart.template.layer.v1+yaml \
    templates/tests/test-mariadb-connection.yaml:application/vnd.cncf.helm.chart.test.layer.v1+yaml

Results in a manifest like the following:

{
  "schemaVersion": 2,
  "config": {
    "mediaType": "application/vnd.cncf.helm.config.v1+json",
    "digest": "sha256:ca3d163bab055381827226140568f3bef7eaac187cebd76878e0b63e9e442356",
    "size": 3,
    "annotations": {
      "apiVersion": "v1",
      "appVersion": "5.2.2",
      "description": "Web publishing platform for building blogs and websites.",
      "engine": "gotpl",
      "home": "http://www.wordpress.com/",
      "icon": "https://bitnami.com/assets/stacks/wordpress/img/wordpress-stack-220x234.png",
      "name": "wordpress",
      "version": "7.0.1"
    }
  },
  "layers": [
    {
      "mediaType": "application/vnd.cncf.helm.chart.readme.layer.v1+md",
      "digest": "sha256:3c0bb73e83ded64c77c31eb853326c60c6f224a44dab4b6bbed132101d00a419",
      "size": 28481,
      "annotations": {
        "org.opencontainers.image.title": "README.md"
      }
    },
    {
      "mediaType": "application/vnd.cncf.helm.chart.values.layer.v1+yaml",
      "digest": "sha256:22fa5a9164970e9a86eeda82148348d02462570b4d9e4e57836e2073db0d208c",
      "size": 12555,
      "annotations": {
        "org.opencontainers.image.title": "values.yaml"
      }
    },
    {
      "mediaType": "application/vnd.cncf.helm.chart.notes.layer.v1+txt",
      "digest": "sha256:f3ff0f21d57595bb9ee3e8af66de48982a78d921a8789c81bcd2db5cd0307c7a",
      "size": 2442,
      "annotations": {
        "org.opencontainers.image.title": "templates/NOTES.txt"
      }
    },
    {
      "mediaType": "application/vnd.cncf.helm.chart.helpers.layer.v1+tpl",
      "digest": "sha256:57c052d50d68e53bcf4b8e8d54f33371ca45ef3ec659dbb5c84ea60141f427c9",
      "size": 4204,
      "annotations": {
        "org.opencontainers.image.title": "templates/_helpers.tpl"
      }
    },
    {
      "mediaType": "application/vnd.cncf.helm.chart.template.layer.v1+yaml",
      "digest": "sha256:669e2db698e355fd5126e3e1ddb0233a9ccb86e4ab85c192689be2bb21028266",
      "size": 7765,
      "annotations": {
        "org.opencontainers.image.title": "templates/deployment.yaml"
      }
    },
    {
      "mediaType": "application/vnd.cncf.helm.chart.template.layer.v1+yaml",
      "digest": "sha256:c03a2f4731c1426be994b8cb07b1507f67e283d40aeb653df5122b1b110c1f08",
      "size": 911,
      "annotations": {
        "org.opencontainers.image.title": "templates/ingress.yaml"
      }
    },
    {
      "mediaType": "application/vnd.cncf.helm.chart.template.layer.v1+yaml",
      "digest": "sha256:dbc0b9b9cad1360a83cf799cf943b6172dbc0627c9d56f84e39061d12007fc32",
      "size": 752,
      "annotations": {
        "org.opencontainers.image.title": "templates/pvc.yaml"
      }
    },
    {
      "mediaType": "application/vnd.cncf.helm.chart.test.layer.v1+yaml",
      "digest": "sha256:8fd048e1787d60d2eee4de5027a6ce5a10ae7ee4f38e91518738af5e210c34dd",
      "size": 1033,
      "annotations": {
        "org.opencontainers.image.title": "templates/tests/test-mariadb-connection.yaml"
      }
    }
  ]
}

I'm not sure the best way to store dependent charts in charts/ directory... I'm also not sure if nested annotations are supported (for things such as "maintainers" array). We can stringify/flatten if necessary.

The alternative to all this is to store everything in a single layer, application/vnd.cncf.helm.chart.content.layer.v1+tar - but still push the meta into annotations.

The OPA project is also going in direction of storing individual bundle files as layers. Please see this issue for more details: open-policy-agent/opa#1413

cc @reasonerjt from Project Harbor - thoughts on this approach?

@jdolitsky jdolitsky added the v3 label Aug 1, 2019

@jdolitsky jdolitsky added this to the 3.0.0-beta.1 milestone Aug 1, 2019

@jdolitsky jdolitsky self-assigned this Aug 1, 2019

@mattfarina

This comment has been minimized.

Copy link
Collaborator

commented Aug 1, 2019

I said this in the meeting today but wanted to get it on the ticket.

How we break things up impacts signing and provenance. We should still support signing. Right now the the tar is what is signed and hashed. If things are broken up it impacts how and what is signed.

@reasonerjt

This comment has been minimized.

Copy link

commented Aug 2, 2019

Josh, thanks for engaging me in this topic.

I totally agree moving metadata of a chart to config, I slightly prefer putting it in a json and package into a layer, rather than putting the data in annotations and leave the entity an empty json. Such that it may contain more complicated structures.

I don't have a strong opinion in regards to whether the different components of a chart should go to different layers. What're the benefits to do that?

@jdolitsky

This comment has been minimized.

Copy link
Member Author

commented Aug 2, 2019

@reasonerjt thanks for yourinput. I think I need clarification by what you mean -

I slightly prefer putting it in a json and package into a layer

Do you mean expand the contents of the config object vs. annotations visible on manifest? For example:

ruby -ryaml -rjson -e 'puts JSON.pretty_generate(YAML.load(ARGF))' \
    < Chart.yaml > config.json

oras push \
    --manifest-config config.json:application/vnd.cncf.helm.config.v1+json \
    localhost:5000/stable/wordpress:7.0.1 \
    README.md:application/vnd.cncf.helm.chart.readme.layer.v1+md \
...

resulting manifest (notice size=613 vs. size=3):

{
  "schemaVersion": 2,
  "config": {
    "mediaType": "application/vnd.cncf.helm.config.v1+json",
    "digest": "sha256:72aeca0a544968d569c657d436db7bb734d74e087f55736ff2b9c27aa93f933f",
    "size": 613
  },
  "layers": [
    {
      "mediaType": "application/vnd.cncf.helm.chart.readme.layer.v1+md",
      "digest": "sha256:3c0bb73e83ded64c77c31eb853326c60c6f224a44dab4b6bbed132101d00a419",
      "size": 28481,
      "annotations": {
        "org.opencontainers.image.title": "README.md"
      }
    }
...

This would work fine as well. Is one method easier for a registry to reason over on the backend? cc @SteveLasker @sajayantony

I don't have a strong opinion in regards to whether the different components of a chart should go to different layers. What're the benefits to do that?

If each piece of the chart is separated into its own layer, then each would have its own digest/sha256. In the case that, for example, I am pulling a new version of the wordpress chart - it could be the case that service.yaml was the only file updated in this new version. Therefore, on helm chart pull, only this layer will be downloaded resulting in less overall transfer.

That being said, Helm charts are quite small, and the benefit may not outweigh the complexity. The ORAS library, however, gives us the ability to do this with ease.

@sajayantony

This comment has been minimized.

Copy link

commented Aug 3, 2019

From an OCI perspective - these decisions are upto the owners of the respective artifacts. For e.g. Singularity prefers the file to be a single verifiable file and uses the registry( the distribution project) as way for their runtime to acquire the image from.
So again no specific concern either way. - The only thing that make is easier is that the config.mediaType indicating the main artifact makes it easier for client to parse a well know field and version in a well-known manner and artifact categorization remains clear.

@jdolitsky

This comment has been minimized.

Copy link
Member Author

commented Aug 3, 2019

@sajayantony

This comment has been minimized.

Copy link

commented Aug 5, 2019

@SteveLasker has the last update from the OCI call. Distribution OSS implementation and Spec should be the driving factor here. Annotation are lose but can always be promoted to the config. Annotations AFAIK are comments in a structured form and hence doesn't yield itself to things like schema validations which is what config with a mime-type can give. So my vote would be for config enhancements provided the OCI community is OK with this.

@reasonerjt

This comment has been minimized.

Copy link

commented Aug 6, 2019

@jdolitsky

Do you mean expand the contents of the config object vs. annotations visible on manifest? For example:...

Yes.

@SteveLasker

This comment has been minimized.

Copy link

commented Aug 8, 2019

Sorry for the delay. Was enjoying some summer time - ok other house projects.
This had more thought than a quick comment, so I wrote this post for discussion: OCI Artifact Authoring: Annotations & Config.json

@jdolitsky

This comment has been minimized.

Copy link
Member Author

commented Aug 8, 2019

@SteveLasker good thoughtful post, thanks Steve.

Given that, and since Chart.yaml contains several nested fields, I think we will stick with just config for now. If the need for annotations arises we can add them later on. I am interested in how this will play into, for example, displaying chart name in a registry UI

@SteveLasker

This comment has been minimized.

Copy link

commented Aug 8, 2019

As we enable registry support, the question could be, is the repo name the chart name?
Do we need yet another name?
repo name = chart
tag = version

@bacongobbler

This comment has been minimized.

Copy link
Member

commented Aug 8, 2019

removing from the beta milestone as #6185 has been PR'd, which means this discussion can move forward without blocking the release.

Yes, a chart name should match a repository name, and the tag should equate to a version. We've taken steps to ensure that when saving a chart in #6070. We don't have anything in place for repository names, but that's the eventual goal.

You should expect that if you fetch a chart from repository foo:1.0.0, you get a chart named foo at version 1.0.0.

@bacongobbler bacongobbler removed this from the 3.0.0-beta.1 milestone Aug 8, 2019

jdolitsky added a commit to jdolitsky/helm that referenced this issue Aug 12, 2019

ref(internal/experimental/registry): pkg refactor
No more magic separating the metadata from chart tarball - charts are
pushed to registry as a single tarball layer with Chart.yaml in tact.

No more fragile custom symlink chart storage, now following
the OCI Image Layout Specification for chart filesystem cache.

Also:
- Update to ORAS 0.6.0
- Simplify registry client setup with NewClientWithDefaults()
- Remove needless annotations and constants

Fixes helm#6068
Fixes helm#6141

Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>

jdolitsky added a commit to jdolitsky/helm that referenced this issue Aug 12, 2019

ref(internal/experimental/registry): pkg refactor
No more magic separating the metadata from chart tarball - charts are
pushed to registry as a single tarball layer with Chart.yaml in tact.

No more fragile custom symlink chart storage, now following
the OCI Image Layout Specification for chart filesystem cache.

Also:
- Update to ORAS 0.6.0
- Simplify registry client setup with NewClientWithDefaults()
- Remove needless annotations and constants

Fixes helm#6068
Fixes helm#6141

Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>

jdolitsky added a commit to jdolitsky/helm that referenced this issue Aug 12, 2019

ref(internal/experimental/registry): pkg refactor
No more magic separating the metadata from chart tarball - charts are
pushed to registry as a single tarball layer with Chart.yaml in tact.

No more fragile custom symlink chart storage, now following
the OCI Image Layout Specification for chart filesystem cache.

Also:
- Update to ORAS 0.6.0
- Simplify registry client setup with NewClientWithDefaults()
- Remove needless annotations and constants

Fixes helm#6068
Fixes helm#6141

Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>

jdolitsky added a commit to jdolitsky/helm that referenced this issue Aug 12, 2019

ref(internal/experimental/registry): pkg refactor
No more magic separating the metadata from chart tarball - charts are
pushed to registry as a single tarball layer with Chart.yaml in tact.

No more fragile custom symlink chart storage, now following
the OCI Image Layout Specification for chart filesystem cache.

Also:
- Update to ORAS 0.6.0
- Simplify registry client setup with NewClientWithDefaults()
- Remove needless annotations and constants

Fixes helm#6068
Fixes helm#6141

Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>

jdolitsky added a commit to jdolitsky/helm that referenced this issue Aug 12, 2019

ref(internal/experimental/registry): pkg refactor
No more magic separating the metadata from chart tarball - charts are
pushed to registry as a single tarball layer with Chart.yaml in tact.

No more fragile custom symlink chart storage, now following
the OCI Image Layout Specification for chart filesystem cache.

Also:
- Update to ORAS 0.6.0
- Simplify registry client setup with NewClientWithDefaults()
- Remove needless annotations and constants

Fixes helm#6068
Fixes helm#6141

Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>

jdolitsky added a commit to jdolitsky/helm that referenced this issue Aug 12, 2019

ref(internal/experimental/registry): pkg refactor
No more magic separating the metadata from chart tarball - charts are
pushed to registry as a single tarball layer with Chart.yaml in tact.

No more fragile custom symlink chart storage, now following
the OCI Image Layout Specification for chart filesystem cache.

Also:
- Update to ORAS 0.6.0
- Simplify registry client setup with NewClientWithDefaults()
- Remove needless annotations and constants

Fixes helm#6068
Fixes helm#6141

Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>

jdolitsky added a commit to jdolitsky/helm that referenced this issue Aug 12, 2019

ref(internal/experimental/registry): pkg refactor
No more magic separating the metadata from chart tarball - charts are
pushed to registry as a single tarball layer with Chart.yaml in tact.

No more fragile custom symlink chart storage, now following
the OCI Image Layout Specification for chart filesystem cache.

Also:
- Update to ORAS 0.6.0
- Simplify registry client setup with NewClientWithDefaults()
- Remove needless annotations and constants

Fixes helm#6068
Fixes helm#6141

Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>

jdolitsky added a commit that referenced this issue Aug 12, 2019

ref(internal/experimental/registry): pkg refactor (#6205)
No more magic separating the metadata from chart tarball - charts are
pushed to registry as a single tarball layer with Chart.yaml in tact.

No more fragile custom symlink chart storage, now following
the OCI Image Layout Specification for chart filesystem cache.

Also:
- Update to ORAS 0.6.0
- Simplify registry client setup with NewClientWithDefaults()
- Remove needless annotations and constants

Fixes #6068
Fixes #6141

Signed-off-by: Josh Dolitsky <jdolitsky@gmail.com>
@jdolitsky

This comment has been minimized.

Copy link
Member Author

commented Aug 13, 2019

closed via #6205

@jdolitsky jdolitsky closed this Aug 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.