Add support for ordering of resources within a chart for Custom Resources

[ashokponkumar]

Background

Custom Resources need a way to be ordered when deployed in certain cases. For example, When creating a SCC and service account as part of a helm chart, since it is not ordered to be created before deployment and other resources, the scc does not have an effect on the pods.

Solution 1

~~The simplest fix might be to just add kind "SecurityContextConstraints" just after "ServiceAccount" in the kind_sorter.go file. https://github.com/helm/helm/blob/release-3.0/pkg/releaseutil/kind_sorter.go~~

Solution 2 - Generic for all Custom Resources

Similar to how we have an ordering requirement for known resources in the InstallOrder, users want the capability to order custom resources too. For example a custom resource like SCC need to be applied before creation of any pods.

To enable this capability we can follow the same approach we use for hooks. Essentially add a annotation "helm.sh/order-weight" which can take an integer value (+ve or -ve). This will behave the same way as "helm.sh/hook-weight", but for resources not covered by hooks.

Here is how it will work:

The first resources to be installed will be all resources that have negative weights, and then those with zero or no weights, and then those with positive weights. All in ascending order. For resources which have same weight, Install order is preserved for known resources.

PR here showing the solution.

[hickeyma]

@ashokponkumar The kind sorter is for Kubernetes resources only. Is "SecurityContextConstraints" an Open Shift object?

[ashokponkumar]

@hickeyma I am not sure of the difference between a kubernetes resource and an Openshift object. Isn't everything a Kubernetes resource, even if it is added as a CRD. Each flavour/version of kubernetes has its own set of kinds depending on the operators that are installed in it.

For example, kubectl api-resources does lists SecurityContextConstraints along with every other resource supported in that cluster.

[liuming-dev]

cc #8292 , and this need reviewers.

[liuming-dev]

whether this can be solved via using hook?

[ashokponkumar]

hooks don't solve this issue for couple of reasons:

1. Conceptually, hooks are things that need to be done outside the chart context. SCC's and many other CRs are part of the chart.
2. If a scc is created as part of the chart, then it should be deleted when the chart is deleted. Hooks don't get deleted as far as I understand.

Technically speaking, the right way to handle any CR would be by using the "weight" concept that is used in the helm-hooks. That way it would be very flexible.

@liuming-dev I looked at #8292, and if I read it right, it is using alphabetical ordering or ordering in the manifest. I would assume both are of no significance when it comes to Kind names. Isn't it? Why can't we use weight annotation similar to that used by hooks?

[mattfarina]

@ashokponkumar There is a difference between conformant Kubernetes and Kubernetes with customizations. Kubernetes conformance is something handled by the CNCF and something that can install to a conformant Kubernetes should be able to be installed into any conformant Kubernetes system.

SecurityContextConstraints is an OpenShift addition that's not part of the conformance.

Helm sorting targets conformant Kubernetes. If the sorting targeted every possible CRD variation it would be a fair amount of work to just handle the public ones. There are many private and proprietary ones that Helm could not handle because it does not have the option of knowing about them.

The best way would be for Kubernetes to hold an ordering but the Kubernetes project considers things to be declarative so that order should not matter. I would not expect to get one from the Kubernetes project.

So, how can Helm handle this in a manner that does not hard code CRD ordering and is backwards compatible in v3? This is an honest question.

[ashokponkumar]

Thanks @mattfarina. I completely agree with you. Just for my own understanding, I had been trying to understand the Kubernetes flavors, and it would really help me, if you can point to the components that are part of the conformant Kubernetes distribution.

If I were to propose a generic solution that covers all CRs, then it would go as follows:

1. Have an annotation by the name "helm.sh/weight", which takes an Integer value (-ve or +ve). This a concept that Helm anyway uses for hooks ("helm.sh/hook-weight").
2. We retain the InstallOrder. We install CRs which have a negative weight in ascending order before those in InstallOrder. Then those with InstallOrder, and then Install those with +ve weights in ascending order.
3. If anyone specifies a weight for a resource in Install order, which is +ve or -ve, it goes with that batch.
4. If the weight is 0, then it happens just before those in the install order. So if there is a resource which in InstallOrder, but has a weight of 0, it will be applied before any other resource.

@mattfarina What do you think about this proposal?

[ashokponkumar]

The best way would be for Kubernetes to hold an ordering but the Kubernetes project considers things to be declarative so that order should not matter. I would not expect to get one from the Kubernetes project.

Technically speaking I would agree with you that the order should not matter. But not all objects were implemented so cleanly for the order to not matter, and I would assume that is the reason we even have InstallOrder in Helm. Kubernetes can not have InstallOrder, since for it each artifact is a discrete artifact. But when it comes to helm, we have the advantage of seeing the application as a whole. So we can technically enforce some order.

For aspects like scc, which influences the roles applied to a ServiceName. Lets say a pod/deployment/statefulset is created before a scc is applied, that pod would not have the right roles assigned by the scc. And the pod till it gets restarted at some later point in time, will never see the changes in the roles.

[hickeyma]

@ashokponkumar Would it be possible to use a separate chart for the SCC and deploy it prior to the app chart?

[ashokponkumar]

@ashokponkumar Would it be possible to use a separate chart for the SCC and deploy it prior to the app chart?

Technically yes, but it is not ideal for two reasons.

1. If I have 3 such resources which are dependent on order, then I would need to use 3 charts, which is not scalable.
2. I would always love to have all the resources in a single folder for a single application for context, and this would make it a bit more messy.

[paulczar]

Regardless of Helm's ability to determine when to apply an SCC I would have some concerns about putting them in the same chart as deployment. Clearly I am not you and don't know your full context and reasoning ... but these are my thoughts on wrapping stuff like SCC or PSP up in a helm chart.

Reading the documentation for Security Context Constraints, it seems to me they're designed to be set by the cluster administration level, not the application deployer. Setting SCC as part of a Helm install is like giving someone sudo access to only run a script they control. Ideally RBAC would be set so that a regular app deployer doesn't even have access to create/modify SCC.

I would second @hickeyma that SCC should be done in a separate chart and run as part of preparing a cluster for applications to be deployed. If a given Chart requires special SCC then that should be handled seperately, otherwise you're effectively allowing an application deployer to set an SCC that allows pretty much anything.

As an example of how I solve similar, I have a set of resources that I always deploy as the cluster admin, things like prometheus, fluentd, cert-manager, etc, as well as all my default Pod Security Policies etc, These are all separate charts that I wrap up in a helmfile chart. If anyone requires a resource that is blocked by my PSP I decide if they should be able to do it and update my charts/helmfile appropriately at the cluster level.

[ashokponkumar]

Regardless of Helm's ability to determine when to apply an SCC I would have some concerns about putting them in the same chart as deployment. Clearly I am not you and don't know your full context and reasoning ... but these are my thoughts on wrapping stuff like SCC or PSP up in a helm chart.

Valid points. Just to give some more context, I am trying to create a helm chart based operator. The servicename used by the operator generally runs with more permissions than required by the application. So I am trying to let the helm chart create a scc that is limited in permission, and use it for its resources. When I have to add a serviceName to a scc, I have to edit the scc. So instead of polluting other scc's for each of my deployment, I am creating a new scc with only the relevant servicename for each deployment. This is more to limit the permissions I give to my resources/pods.

If I were to propose a generic solution that covers all CRs, then it would go as follows:

1. Have an annotation by the name "helm.sh/weight", which takes an Integer value (-ve or +ve). This a concept that Helm anyway uses for hooks ("helm.sh/hook-weight").
2. We retain the InstallOrder. We install CRs which have a negative weight in ascending order before those in InstallOrder. Then those with InstallOrder, and then Install those with +ve weights in ascending order.
3. If anyone specifies a weight for a resource in Install order, which is +ve or -ve, it goes with that batch.
4. If the weight is 0, then it happens just before those in the install order. So if there is a resource which in InstallOrder, but has a weight of 0, it will be applied before any other resource.

Do you think it makes sense to use this as a generic CR implementation, so that it can handle such resources, which are dependent on others? Users can fall back to the dual chart approach based on their need and usage pattern. Its gives a choice.

[paulczar]

@ashokponkumar there have been some attempts in the past (example https://github.com/helm/helm/pull/5214) to implement chart template ordering, but they haven't been followed through to a mergeable state. I would suggest we'd be open to the idea, but I don't see it as a roadmapped item that the maintainers are actively working on.

[ashokponkumar]

@ashokponkumar there have been some attempts in the past (example #5214) to implement chart template ordering, but they haven't been followed through to a mergeable state. I would suggest we'd be open to the idea, but I don't see it as a roadmapped item that the maintainers are actively working on.

Yes. I too noticed it. Example #3635 seems pretty close to this suggestion. I did not see any objection to this idea though. If the commenters in this thread see this as an useful idea, I will create a new issue for this one.

[ashokponkumar]

Closing this in favor of #8446, since it covers a more generic solution to this issue.

[hickeyma]

Re-opened as it contains comments so far. Updated as proposal.

[hickeyma]

@ashokponkumar This proposal will be discussed at the Helm dev weekly meeting on this Thursday, the 16th. It would be great if you could attend to pitch the proposal.

[ashokponkumar]

sure @hickeyma . Will attend.

[liuming-dev]

@ashokponkumar This proposal will be discussed at the Helm dev weekly meeting on this Thursday, the 16th. It would be great if you could attend to pitch the proposal.

PR #8292 is the same topic, I wish this PR could be discussed together. :pray:

[bacongobbler]

There are multiple threads all asking for similar requests. A short list of those threads:

• #8291
• #7072
• #6851
• #5916
• #5642

I would propose closing all of these threads in favour of #6851. I'm not sure why there's a need to track discussions for each individual resource here.

[ashokponkumar]

There are multiple threads all asking for similar requests. A short list of those threads:

• #8291
• #7072
• #6851
• #5916
• #5642

I would propose closing all of these threads in favour of #6851. I'm not sure why there's a need to track discussions for each individual resource here.

@bacongobbler Though the initial discussion started with an individual resource, we converged on a generic solution for handling all CRs using a weight concept as seen in the new modified description. Also we have a PR #8448 attached to this issue which has this fix implemented.

Repasting the updated description for ease of reference:

Similar to how we have an ordering requirement for known resources in the InstallOrder, users want the capability to order custom resources too. For example a custom resource like SCC need to be applied before creation of any pods.

To enable this capability we can follow the same approach we use for hooks. Essentially add a annotation "helm.sh/order-weight" which can take an integer value (+ve or -ve). This will behave the same way as "helm.sh/hook-weight", but for resources not covered by hooks.

Here is how it will work:

The first resources to be installed will be all resources that have negative weights, and then those with zero or no weights, and then those with positive weights. All in ascending order.
For resources which have same weight, Install order is preserved for known resources.

PR here showing the solution.

[ashokponkumar]

To address the question raised in today's developer call around dependent charts, one way to handle it could be to have a chart weight specifiable along with the dependency specification in chart.yaml. The value specified in it will get added as a "helm.io/chart-weight" annotation in each of those charts resources, when the resources are templated. In case of dependency within a dependency, the "helm.io/chart-weight" will be the sum of the all chart-weights encountered till now.

And while each resource order-weight is computed, the chart-weight gets added to its value. For the base chart, the value is 0.

[github-actions[bot]]

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

[jecnua]

Issue still valid, please remove stale label.

[github-actions[bot]]

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

[jecnua]

Issue still valid, please remove stale label. (I really need to get back to my project of an anti-stale-bot bot for github).

[henrikb123]

I have a similar problems as addressed here. I want a CR in my chart to be installed before the general application (deployment, service, ...). I even want it to go before a job with helm hook-annotation also part of the chart, which further complicates things.

A suggested solution that would solve my problem and possibly cover other cases discussed here:

Make it possible for resources with hook-annotations to become managed objects. For example I want my CR with hook-weight -2 to be managed, but my job with hook-weight -1 to be un-managed as currently supported.

If there was an annotation like helm.sh/hook-object-ownership: managed

then I would be able to update my CR object using Helm and also avoid the deletion of the object that comes with hooks (before-hook-creation, hook-succeeded, hook-failed) and is unwanted for my CR object.

[github-actions[bot]]

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

[ashb]

Still not stale. Can one of the helm committers pin this issue please 😃

[github-actions[bot]]

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.