feat(helm chart): create chart

[starlightromero]

What this PR does / why we need it:
Utilizes a Helm chart as an alternative installation method for the Gatekeeper library

Which issue(s) does this PR fix (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):
Fixes #51

Are you making changes to Gatekeeper Library policies?
Please refer to How to contribute to the library to add new policies or update existing policies.
No

Are you updating an existing policy?
Please run make generate generate-website-docs generate-artifacthub-artifacts to generate the templates and docs.
No

Special notes for your reviewer:

[stale]

This issue/PR has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

[starlightromero]

All templates and crds have been added. Will be testing this out over the next couple of days

[kfox1111]

In general, looks very good. Thanks for working on this! :)

The crd's though I think can be a problem. Helm hasn't ever managed crd's very well. but in this case, it may work better if the crd's are marked no delete and templated out with the objects that use them, so they can be enabled/disabled with the features. That way you don't get a huge pile of crds to update when you dont use most of them?

[starlightromero]

That's a great idea! No need to clutter up the cluster with unused resource definitions.

[starlightromero]

@kfox1111

CRD files cannot be templated. They must be plain YAML documents.

source

[kfox1111]

templates can be raw yaml wrapped with conditionals. I've seen others do crd's this way. It must be in the template directory instead of the crds directory.

Can also add annotations:
"helm.sh/hook": crd-install
and
"helm.sh/resource-policy": keep

to get it to run first, and to never delete it.

[starlightromero]

CRDs must be installed before the resources which utilize the CRDs can be installed. If CRDs are in the templates directory along with the templates, this would lead to an unstable state in which the resources would attempt to be applied before the CRD associated with it. How I've seen other projects use CRDs in the template/ directory is that they have two different Helm charts. One for CRDs and one for the actual templates. One example of this is the Prometheus operator, which is needed before applying the chart for cAdvisor.

I'm not sure of all the pros and cons of two charts. One pro is there are only CRDs in the cluster which we need. A con is it requires us to maintain two charts and keep them in sync. Curious on your thoughts.

[kfox1111]

as per: https://helm.sh/docs/topics/charts_hooks/
Note that the crd-install hook has been removed in favor of the crds/ directory in Helm 3. :(

CRD's with helm truly are terrible. :( Buggy, inconsistent, too many ways to do it, and none of them work right.

I typically skip managing them with helm and raw install them.

Gatekeeper itself chose a 3rd option of using a helm job to install them. That has yet another set of tradeoffs... hooks don't always play nice with ci/cd....

A separate chart could work. If the values file for both charts could be made to take the same arguments, the user could pass it to both charts without modification, simplifying things.

Having a separate chart just with the crd's templated out with enables would be about the same amount of effort as having just one, with the crd's templated out with enables?

[starlightromero]

I like the idea of both charts taking the same values. It would simplify operations from both the user and the maintainer perspective. However, I'm not 100% sure a 1:1 translation will work. Some values, such as containerLimits.cpu which sets the cpu maximum limit, would not translate in any way to the CRDs. Both charts can definitely take most of the same values, such as enabled.

[kfox1111]

The crd chart could just ignore any value it didn't need and just look at the enabled flags?

[kfox1111]

This is a little confusing to me.... I understand why but the naming doesn't quite fit in my head.

the -crd chart isn't just crds. its the actual library of code that also ends up generating the crds as a sideaffect. in my head its closer to what gatekeeper-library is by name?

the other bits are configuration in a way, not the library.

Naming things is hard.

[starlightromero]

What do you think about gatekeeper-library-constraint-templates and gatekeeper-library-contraints?

[starlightromero]

After doing some testing with the new layout, two separate charts, it appears all ConstraintTemplates are being installed even when the templated if conditions are not met.

[starlightromero]

Ah - it appears the ContraintTemplates are not getting cleaned up properly. This may be a user error on my end.

[starlightromero]

This has been tested and is working well for me.

@kfox1111 Any last pieces of feedback?

[starlightromero]

@sozercan Can I please get your review?

[sozercan]

@starlightromero thanks for the pr! this is a big change which needs to be discussed with the community and maintainers. i added to the agenda for the next community meeting (wednesdays) https://docs.google.com/document/d/1A1-Q-1OMw3QODs1wT6eqfLTagcGmgzAJAjJihiO3T48

[kfox1111]

This has been tested and is working well for me.

@kfox1111 Any last pieces of feedback?

Looks good to me. :)

[starlightromero]

@sozercan Thanks for adding this PR to the agenda. One note, currently the Helm charts are being generated (from library) via a series of automated (charts/gatekeeper-library-constraints/hack) and manual processes. In an effort to reduce long-term maintenance, it would be better to have the Helm chart as the source of truth, then generate kustomize templates from them. This set up is how the aws-fsx-csi-driver repo operates

[sozercan]

@starlightromero we discussed this in the community call today. consensus was this might be a better fit for https://github.com/open-policy-agent/contrib repo since using helm chart in production to manage these policies (constraints and constraint templates) may run into conflicts if CTs can be updated and managed outside of the helm chart lifecycle and may create conflicts

[starlightromero]

@sozercan That is a valid concern. The policies will run into conflicts if the CTs are updated outside of the Helm chart. However, this possibility of conflict is no more or less valid as compared to using kubectl which is documented in the Usage section

[sozercan]

Yea, that was brought up. I think concern was with Helm we are providing a more curated/presciptive guidance. Today, intention with the library is we leave it up to policy authors to manage their own policies (which might be helm or other methods).

[kfox1111]

The plan then is for this to be in contrib?