-
Notifications
You must be signed in to change notification settings - Fork 4
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
General chart improvements #13
Conversation
I also see that some CI tests are missing. I highly recommed something like this: https://github.com/jkroepke/helm-charts/blob/main/.github/workflows/lint-test.yaml which automaticly does some lint check, spin-up a kubernetes cluster (inside GH Actions) check, if the helm chart is apply-able. |
Thanks! Looks great! I'll do a deep dive tomorrow and get back to you with questions/suggestions. I'd definitely love a follow-up that improves secret handling! |
Thank you, these modifications are very helpful. To simplify the PR and get the other changes merged more quickly, I think we should split out the rbac modifications and handle that separately. I'll add some other minor questions/suggestions inline.
We can use the kubetail /healthz endpoint for the readiness probe too. Currently there's no distinction between readiness/liveness in terms of healthz though we could modify this in the future if necessary.
YES please! |
73656f8
to
b24e090
Compare
Done
Thanks, Implemented. |
Hi,
I added many options to this helm chart now. Some of them base on good-pratice. Taken from other helm charts.
If you feel, this is too big, I will cut them into smaller PRs.
rbac.namespaced
is introduced. if true, Role and Rolebinding will be created instead ClusterRole/ClusterRoleBinding. I also change from the values fromclusterRole.name
andclusterRoleBinding.name
torbac.roleName
andrbac.roleBindingName
. The old name still working.automountServiceAccountToken
to the Deployment and ServiceAccount. A lot of security tools hightlight automountServiceAccountToken=true on a ServiceAccount as insecure. The best-practice is to set automountServiceAccountToken=false on a ServiceAccount and do automountServiceAccountToken=true on the Deployment. Ref: https://securecloud.blog/2021/08/17/azure-aks-reviewing-recommendations-from-security-center-disabling-automounting-api-credentialsnodeSelector
,affinity
,tolerations
,priorityClassName
. I currently neednodeSelector
only because we have mixed OS clusters.tolerations
is needed, because infra-nodes have taints sometimes..Values.config
,.Values.ingress.hosts.hosts
through helm tpl function: If kubelint is part of a bigger umbrella helm chart, values from.Values.global
can be re-used. It's also useful for thenamespace
optionNon-resolved issues:
I have tested the helm chart with
helm upgrade kubetail . --set rbac.namespaced=false
,helm upgrade kubetail . --set rbac.namespaced=true --set-string config.namespace='\{\{ .Release.Namespace \}\}