Add deprecated field to chart metadata (Chart.yaml)

[prydonius]

We want to be able to add metadata in the chart that can be used to determine whether a particular chart is deprecated or not. This metadata should live in the Chart.yaml file, such as:

name: my-chart
version: 0.1.0
description: My deprecated chart
deprecated: true

The field should be optional, and default value false so that non-deprecated charts can simply omit the field.

One issue with this approach is that the Chart.yaml is tied to a specific version of the chart, and so there may be some confusion over whether just a particular version of the chart is deprecated, or all versions are. In my opinion, we need to make it clear that this field indicates that all versions of the chart are deprecated. For greater flexibility, we could say that if the latest version of a chart is marked as deprecated, then the whole chart is deprecated (the index is ordered, so you just need to look at the first entry for a particular chart). This would give us the ability to later un-deprecate a chart, by simply publishing a new version without the field.

With this field, the approach we will follow in the kubernetes/charts repositories to deprecate a chart will be to submit a PR that:

• updates the Chart.yaml to add the deprecated field
• add a notice at the top of the README
• add a notice in NOTES.txt

The human-readable notices will allow us to briefly explain why a chart was deprecated, and possibly point to a replacement.

After merging, we should follow up with a PR that removes the directory from the git repository.

cc @migmartri @viglesiasce @mgoodness @lachie83 @technosophos

[technosophos]

Should helm install/upgrade issue a warning if a user installs a deprecated chart?

[prydonius]

I think that would be great, and may even drop the need for adding a notice in NOTES.txt.

[technosophos]

@prydonius I was starting to work on this today, and had one other quick idea for you:

Would it be easier for you if we implemented it like this: What if we made it possible to specify whether the deprecation was for this version only, or if it marked the deprecation of the chart in its entirety?

Mark one version deprecated:

deprecated: version # This _particular version_ is deprecated.

This implies that this one particular version should not be used (e.g. for security reasons). ONLY chart versions that were considered "broken" would use this flag.

Mark the entire chart deprecated:

deprecated: chart # The chart is now marked deprecated.

/cc @migmartri @jackfrancis

[jackfrancis]

My thoughts:

• I think there is real value in being able to express version-specific deprecation (your general security example is on point).
• The current implementation for expressing common attributes of a chart (common across all versions, that is), e.g., name, is an implicit contract between chart repo consumer and provider that those property values will be the same for all chart versions. Is that correct?

An example of point 2 above is the name value in these two chart version entries:

  - apiVersion: v1
    created: 2017-03-02T19:33:28.166146825Z
    description: Scales worker nodes within autoscaling groups.
    digest: 291eaabbf54913e71cda39d42a6e9c4c493a3672a5b0b5183ea1991a76d6c317
    engine: gotpl
    icon: https://github.com/kubernetes/kubernetes/blob/master/logo/logo.png
    maintainers:
    - email: mgoodness@gmail.com
      name: Michael Goodness
    name: aws-cluster-autoscaler
    sources:
    - https://github.com/kubernetes/contrib/tree/master/cluster-autoscaler/cloudprovider/aws
    urls:
    - https://kubernetes-charts.storage.googleapis.com/aws-cluster-autoscaler-0.2.1.tgz
    version: 0.2.1
  - apiVersion: v1
    created: 2017-03-02T18:48:30.418071154Z
    description: Scales worker nodes within autoscaling groups.
    digest: 52ee58b51619f641d0f6df4c778bcd068242379a1a040a269f0fc93867b79b13
    engine: gotpl
    maintainers:
    - email: mgoodness@gmail.com
      name: Michael Goodness
    name: aws-cluster-autoscaler
    sources:
    - https://github.com/kubernetes/contrib/tree/master/cluster-autoscaler/cloudprovider/aws
    urls:
    - https://kubernetes-charts.storage.googleapis.com/aws-cluster-autoscaler-0.2.0.tgz
    version: 0.2.0

If my thinking is correct, then I think we can continue this implicit representation thusly:

• we mark deprecated chart versions with a boolean deprecated: true property representation
• a chart repo consumer marks an entire chart as being deprecated (implementation TBD) when it detects that all versions of a chart are marked as deprecated: true

[technosophos]

Retroactively changing charts after they are published does have some rather nasty consequences to the Helm security model (it changes the crytographic SHA, which will also prevent those charts from being installed). That might be a little heavy handed for the case where I just want to let people know that there will be no more releases.

Now, we could just treat deprecated: true as an indication that a chart is EOL, and later come up with some other field to mark a chart as insecure.

[jackfrancis]

Can we implement deprecated: true as a "side-car" property of the chart, and not an intrinsic property? In other words, we don't include it as input to the SHA calculation. It is a remark against the other, canonical chart metadata, not a member of that metadata.

Other ways of properly doing this would include (probably) overly complex implementations, like maintaining a distinct "deprecated" data store, which contained authoritative records of deprecated chart+version entries.

[technosophos]

There is no good way to take the SHA of a tarball, yet exclude specific lines in one of the files. So the only way we could do it was by having it as something additional outside of the chart.

I could see some possibilities for having a "deprecated.yaml" file like a certificate revocation list... but... it does seem a little overly complex for the use case.

[migmartri]

In my opinion, having deprecated: true for newer versions of a chart is enough to flag the chart in both the Github repository and in Monocular (since we use info from the latest published version).

My take would be that in order to deprecate a chart, we package a new version (increasing the version name) with this flag set to true.

Related to @technosophos idea. Something interesting about how Google cloud handles deprecated versions, is that you need to provide a replacement image in order to deprecate another one https://cloud.google.com/compute/docs/images/create-delete-deprecate-private-images.

This could be useful for the case of one chart replacing another and could be implemented by setting the deprecated flag with the name of the replacement chart.

In nginx-lego Chart.yaml

# We tell that nginx-lego has been deprecated in favor of nginx-ingress
deprecated: stable/nginx-ingress

[prydonius]

Just catching up on this, I think there are some important points to highlight:

Retroactively changing charts after they are published does have some rather nasty consequences to the Helm security model (it changes the crytographic SHA, which will also prevent those charts from being installed).

I don't think this is a workflow we really want to encourage, and I agree that this should be some sort of side-car property. Also, I don't see a big need for deprecating specific versions today. A deprecated version could just mean an older version that is superseded by the latest version -- going back and changing older Chart.yamls when we find out an older version is vulnerable is not very feasible.

I could see some possibilities for having a "deprecated.yaml" file like a certificate revocation list... but... it does seem a little overly complex for the use case.

A deprecated.yaml or .deprecated could work, but again I don't think we will ever go back and add this to older chart packages. The workflow will be to PR an update to mark the chart as deprecated, publish that in the chart repo, and then PR to delete the chart from the github repo.

Therefore, I think it makes sense for a deprecation mark in the latest chart version to indicate EOL for the whole chart. A chart name can later be re-used/un-deprecated by publishing a new chart version that is not deprecated.

# We tell that nginx-lego has been deprecated in favor of nginx-ingress
deprecated: stable/nginx-ingress

I like the idea of pointing to a chart that supersedes the deprecated chart, but I don't think it should be required. The proposed format is also a little confusing, as it could be read as "stable/nginx-ingress is deprecated". I would avoid having this semantic in the chart spec, and leave it up to the chart maintainer in NOTES.txt/README. To give another example, in the particular case of nginx-lego, we would need to mention that it is deprecated in favour of stable/nginx-ingress and stable/kube-lego.

[prydonius]

I created #2107 with the initial idea about using only the latest version as an indicator, but I can change it to whatever we decide we want in this discussion.