Skip to content
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

Support directly mounted TLS certs #1319

Open
max-frank opened this issue Apr 10, 2023 · 3 comments
Open

Support directly mounted TLS certs #1319

max-frank opened this issue Apr 10, 2023 · 3 comments
Assignees
Labels
enhancement New feature or request never-stale Issue or PR marked to never go stale

Comments

@max-frank
Copy link

max-frank commented Apr 10, 2023

Is your feature request related to a problem? Please describe.
In some cases e.g, when using cert-managers CSI driver TLS keys+CA cert can be directly mounted to the pods. Also the now possibly outdated mtls inter node example mounts secrets manually. Right now such cases are not really supported since many feature rely on tls.secretName being set for tls to be considered enabled and tls.caSecretName for mtls e.g.,

Meaning that while it is possible to configure the rabbitmq nodes themselves to run with TLS/mtls using such directly mounted secrets, by also configuring the TLS config directly as is done in the MTLS example, it is not possible to utilise the operators other features related to TLS.

Describe the solution you'd like

This could be solved with backwards compatibility by adding an extra configuration option to force TLS to be enabled even in the absence of tls.secretName e.g., tls.enabled, if necessary a second option could also be added as switch for the MTLS features handled by the operator.

Describe alternatives you've considered
Another option could be instead of having just a simple flag to also allow to pass file paths instead of just secret refs
e.g., tls.secretPath and tls.caSecretPath and then treat them essentially the same as their secret ref equivalents.

Additional context
Also note that using e.g., the cert-manager CSI driver allows you to easily create TLS secrets specific to the pod running. So each cluster node gets their own cert automatically making it a bit easier to scale. So supporting this use case would be beneficial overall.

Example config with cert-manager CSI driver

apiVersion: cert-manager.io/v1
kind: Issuer
metadata:
  name: rabbitmq-ca-issuer
  namespace: test
spec:
  selfSigned: {}
---
apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
  name: rabbitmq-ca
  namespace: test
spec:
  isCA: true
  secretName: rabbitmq-ca
  commonName: rabbitmq-ca
  issuerRef:
    name: rabbitmq-ca-issuer
    kind: Issuer
    group: cert-manager.io
---
apiVersion: cert-manager.io/v1
kind: Issuer
metadata:
  name: rabbitmq-ca
  namespace: test
spec:
  ca:
    secretName: rabbitmq-ca
---
apiVersion: v1
kind: ConfigMap
metadata:
  name: mtls-inter-node-tls-config
  namespace: test
data:
  inter_node_tls.config: |
    [
      {server, [
        {cacertfile, "/etc/rabbitmq/certs/ca.crt"},
        {certfile,   "/etc/rabbitmq/certs/tls.crt"},
        {keyfile,    "/etc/rabbitmq/certs/tls.key"},
        {secure_renegotiate, true},
        {fail_if_no_peer_cert, true},
        {verify, verify_peer},
        {customize_hostname_check, [
          {match_fun, public_key:pkix_verify_hostname_match_fun(https)}
        ]}
      ]},
      {client, [
        {cacertfile, "/etc/rabbitmq/certs/ca.crt"},
        {certfile,   "/etc/rabbitmq/certs/tls.crt"},
        {keyfile,    "/etc/rabbitmq/certs/tls.key"},
        {secure_renegotiate, true},
        {fail_if_no_peer_cert, false},
        {verify, verify_none},
        {customize_hostname_check, [
          {match_fun, public_key:pkix_verify_hostname_match_fun(https)}
        ]}
      ]}
    ].
---
apiVersion: rabbitmq.com/v1beta1
kind: RabbitmqCluster
metadata:
  name: rabbitmqcluster-sample
  namespace: test
  annotations:
    rabbitmq.com/topology-allowed-namespaces: "test2"
spec:
  rabbitmq:
    envConfig: |
      SERVER_ADDITIONAL_ERL_ARGS="-proto_dist inet_tls -ssl_dist_optfile /etc/rabbitmq/inter-node-tls.config"
      RABBITMQ_CTL_ERL_ARGS="-proto_dist inet_tls -ssl_dist_optfile /etc/rabbitmq/inter-node-tls.config"
  terminationGracePeriodSeconds: 60
  replicas: 3
  # if you remove this tls block this will actually deploy fine with the current version
  # of the operator but will not set any TLS ports in the services etc.
  tls: 
     enabled: true
     # can't be set because the operator considers TLS to not be active
     disableNonTLSListeners: true
  override:
    statefulSet:
      spec:
        template:
          spec:
            containers:
            - name: rabbitmq
              volumeMounts:
                - mountPath: /etc/rabbitmq/certs
                  name: mtls-inter-node-nodes-tls
                - mountPath: /etc/rabbitmq/inter-node-tls.config
                  name: inter-node-config
                  subPath: inter_node_tls.config
            volumes:
            - configMap:
                defaultMode: 420
                name: mtls-inter-node-tls-config
              name: inter-node-config
            - name: mtls-inter-node-nodes-tls
              csi:
                driver: csi.cert-manager.io
                readOnly: true
                volumeAttributes:
                      csi.cert-manager.io/issuer-name: rabbitmq-ca
                      csi.cert-manager.io/common-name: "${POD_NAME}.${POD_NAMESPACE}.svc.cluster.local"
                      csi.cert-manager.io/dns-names: "${POD_NAME}.${POD_NAMESPACE}.svc.cluster.local"
@MirahImage MirahImage added the enhancement New feature or request label Apr 11, 2023
@github-actions
Copy link

This issue has been marked as stale due to 60 days of inactivity. Stale issues will be closed after a further 30 days of inactivity; please remove the stale label in order to prevent this occurring.

@github-actions github-actions bot added the stale Issue or PR with long period of inactivity label Jun 11, 2023
@github-actions
Copy link

Closing stale issue due to further inactivity.

@github-actions github-actions bot added the closed-stale Issue or PR closed due to long period of inactivity label Jul 11, 2023
@Zerpet
Copy link
Collaborator

Zerpet commented Jul 14, 2023

This is valueable and an interesting feature. Unfortunately, it's seems a considerable undertaking and we don't have the bandwidth to do this right now. A PR would be very welcome.

@Zerpet Zerpet reopened this Jul 14, 2023
@Zerpet Zerpet added never-stale Issue or PR marked to never go stale and removed stale Issue or PR with long period of inactivity closed-stale Issue or PR closed due to long period of inactivity labels Jul 14, 2023
@ikavgo ikavgo self-assigned this Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request never-stale Issue or PR marked to never go stale
Projects
None yet
Development

No branches or pull requests

4 participants