Add option (value) for specifying cluster issuer directly about charts-clickhouse HOT 5 CLOSED

This seems reasonable, would you be able to put up a pull request for us to review?

from charts-clickhouse.

@tiina303 while checking what I needed to do, I noticed that the ingress already has a placeholder for custom annotations.

   {{- if eq (include "ingress.letsencrypt" .) "true"}}
    cert-manager.io/cluster-issuer: "letsencrypt-prod"
   {{- end }}
   ...
   {{- if .Values.ingress.annotations }}
    {{- range $key, $value := .Values.ingress.annotations }}
    {{ $key }}: {{ $value | quote }}
    {{- end }}
   {{- end }}

So I guess the right way to do this would be just to add to the values:

certManager:
  enabled: false
  
ingress:
   letsencrypt: false
   annotations:
      cert-manager.io/cluster-issuer: my-cluster-certrificate

As the later annotations will override the chart's value.

So we could just add an empty ingress.annotations key to values.yml to document the possibility of adding them? I don't know if this is something that is so common to charts that everyone expects it to happen. But for me it wasn't obvious.

Let me know how you would like me to proceed :)

from charts-clickhouse.

put up a quick pr for this ^

from charts-clickhouse.

So we could just add an empty ingress.annotations key to values.yml to document the possibility of adding them? I don't know if this is something that is so common to charts that everyone expects it to happen. But for me it wasn't obvious.

Yes, I'd love to have all the possible values documented in the values file, I know we're a bit inconsistent about that atm.

qq: Do you need the secret name to be configurable too https://github.com/PostHog/charts-clickhouse/blob/main/charts/posthog/templates/ingress.yaml#L40 (which can be done, but feels a bit odd to put it under gcp if one is not on gcp)?

from charts-clickhouse.

@tiina303 Indeed, I think I do. If letsencrypt: false then the secret is not set.

I ended up deploying using K8 manifests directly to try out posthog, and specify the TLS secret.
If I do letsencrypt: true and the the extra annotations it has the side effect of creating an unused certificate, which is not the worst.

This seems to be a more involved change and refactoring, and I'm not sure if I can do it by myself. But maybe it's worth considering, as having a centralised cluster issuer and shared CertManager for the entire cluster is, as far as I understood, a common practice.

Bellow is my ingress setup for reference

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: posthog-webserver-ingress
  namespace: posthog
  annotations:
    kubernetes.io/ingress.class: "nginx"
    cert-manager.io/cluster-issuer: cluster-certificate-issuer
spec:
  rules:
  - host: my.url.for.posthog.com
    http:
      paths:
      - backend:
          service:
            name: posthog-webserver-service
            port:
              number: 8000
        path: /
        pathType: Prefix
  tls:
    - hosts:  
      - my.url.for.posthog.com
      secretName: posthog-cert

from charts-clickhouse.