spiffe / helm-charts-hardened

Apache License 2.0
12 stars 26 forks source link

Add Openshift ignore namespaces to Controller Manager #363

Closed mrsabath closed 1 month ago

mrsabath commented 1 month ago

Some namespaces that are created to support Openshift platform should be ignored by the Controller Manger

erikgb commented 1 month ago

Does ignoreNamespaces support some kind of wildcard/regex, or only literal values? Having customizations for OpenShift and IBM Cloud on by default seems wrong to me.

edwbuck commented 1 month ago

Does ignoreNamespaces support some kind of wildcard/regex, or only literal values? Having customizations for OpenShift and IBM Cloud on by default seems wrong to me.

To ignore them dependent on configuration, we could push them into the templates, where this becomes possible because it is a space where one can apply if / then statements. But then the values would be encoded into the templates, where they couldn't easily be modified.

In the values.yaml, they are in the specific file that is intended to be edited for customization of each deployment. If a person doesn't want to ignore them in their deployment, they are welcome to edit them out (or add in more) because that's what values.yaml is for within a Helm deployment.

For example, we also ship an example.org trustdomain, which will (for any sensible deployment) need to be changed in this file. That's because this file is the one that someone is supposed to edit to fit their deployment.

--- Edit update ---

Sorry, I realize that I did not answer one of your questions directly. Yes, "ignoreNamespace" does support wildcards, but only in the form of Regex patterns.

kfox1111 commented 1 month ago

Hmm... I was under the impression it was just a list of strings. but looking here, https://github.com/spiffe/spire-controller-manager/blob/main/cmd/main.go#L139 @edwbuck is right. it looks to be a list of regexes.

So, we could switch it to a smaller set of rules that matches anything with an openshift-. or ibm-. ?

We could template it out, but makes it a bit harder to understand whats going on by default. We were thinking it would be more understandable to exclude them by default as it didn't hurt anything on vanilla clusters to exclude namespaces that didn't exist there.

mrsabath commented 4 weeks ago

I like the idea of replacing the list of namespaces with wildcards.