knative-extensions / eventing-rabbitmq

RabbitMQ eventing components. Knative Source and Broker.
Apache License 2.0
91 stars 68 forks source link

Not able to connect to rabbitmq from broker ingress due to error "Certificate required" #1158

Closed Jaryllan closed 7 months ago

Jaryllan commented 1 year ago

Describe the bug The below error occurred immediately by the broker ingress pod after broker is created and is repeated every seconds. Unsure what else is left out due to the CA certificate & rabbitmq certificate is supplied, but the error still specifies "certificate required".

{"level":"error","ts":"2023-07-07T06:47:08.294Z","logger":"rabbitmq-ingress","caller":"rabbit/connections_handler.go:147","msg":"failed to create RabbitMQ connection, error: Exception (501) Reason: \"remote error: tls: certificate required\"","commit":"d9f5856-dirty","stacktrace":"knative.dev/eventing-rabbitmq/pkg/rabbit.(*RabbitMQConnectionHandler).createConnectionAndChannel\n\tknative.dev/eventing-rabbitmq/pkg/rabbit/connections_handler.go:147\nknative.dev/eventing-rabbitmq/pkg/rabbit.(*RabbitMQConnectionHandler).Setup\n\tknative.dev/eventing-rabbitmq/pkg/rabbit/connections_handler.go:118\nmain.main\n\tknative.dev/eventing-rabbitmq/cmd/ingress/main.go:95\nruntime.main\n\truntime/proc.go:250"}

The following is the error retrieved from the rabbitmq cluster's pod. Similar to broker ingress log, every seconds will have such error being logged.

2023-07-07 06:36:21.014112+00:00 [notice] <0.2905.0> TLS server: In state wait_cert at tls_handshake_1_3.erl:1484 generated SERVER ALERT: Fatal - Certificate required
2023-07-07 06:36:21.014112+00:00 [notice] <0.2905.0>  - certificate_required

Expected behavior No error and able to process the event from broker to trigger.

To Reproduce prerequisites: CertManager, RabbitMQ Operator and RabbitMQ Topology Operator

  1. Supply the certificates created by CertManager into the RabbitmqCluster CRD. (ca.crt, tls.crt & tls.key)

    apiVersion: rabbitmq.com/v1beta1
    kind: RabbitmqCluster
    metadata:
    name: rabbitmq
    namespace: rabbitmq
    annotations:
    rabbitmq.com/topology-allowed-namespaces: "*" 
    spec:
    replicas: 3
    service:
    type: "LoadBalancer"
    
    tls:
    secretName: rmq-tls-secret
    caSecretName: rmq-tls-secret
    disableNonTLSListeners: true
    
    rabbitmq:
    additionalConfig: |
      ssl_options.verify = verify_peer
      ssl_options.fail_if_no_peer_cert = true
      management.path_prefix = /rabbitmq
  2. Create a secret into namespace eventing-test based on the same certificate files from point 1 that are assigned to the RabbitmqCluster CRD. kubectl -n eventing-test create secret generic rmq-tls-secret --from-file=ca.crt=ca.crt --from-file=tls.crt=tls.crt --from-file=tls.key=tls.key

  3. Create broker config.

    apiVersion: eventing.knative.dev/v1alpha1
    kind: RabbitmqBrokerConfig
    metadata:
    name: rabbitmq-broker-config
    namespace: eventing-test
    spec:
    rabbitmqClusterReference:
    # Configure name if a RabbitMQ Cluster Operator is being used.
    name: rabbitmq
    namespace: rabbitmq
    queueType: quorum
  4. Create the broker

    apiVersion: eventing.knative.dev/v1
    kind: Broker
    metadata:
    annotations:
    eventing.knative.dev/broker.class: RabbitMQBroker
    name: rabbitmq-broker
    namespace: eventing-test
    spec:
    config:
    apiVersion: eventing.knative.dev/v1alpha1
    kind: RabbitmqBrokerConfig
    name: rabbitmq-broker-config
    namespace: eventing-test
  5. Once the broker ingress pods gets provisioned, check the log of the pod

    Knative release version Knative Operator - v1.10.2 Knative Eventing - v1.10.1 Knative Eventing-RabbitMQ - v1.10.0

    Additional context I can only suspect the broker ingress only includes the CA file when establishing the connection to rabbitmq cluster, but excluding the other certificate files (in this case the tls.crt and tls.key).

gabo1208 commented 1 year ago

Hello @Jaryllan !! Thanks for the thorough and great report

Try this config we use in the e2e tests: https://github.com/knative-sandbox/eventing-rabbitmq/blob/main/test/e2e/config/certsecret/secret.yaml

You also have the go file to check what it's needed and where we put it in the secrets too

With this issue I realize we forgot to add an example of this so I added that issue to to our github issues, probably next week it will be ready to go :)!

Jaryllan commented 1 year ago

Hello @gabo1208,

So far the secret is being placed for

  1. The rabbitmq cluster itself

    tls:
    secretName: rmq-tls-secret
    caSecretName: rmq-tls-secret
    disableNonTLSListeners: true
  2. https://www.rabbitmq.com/kubernetes/operator/tls-topology-operator.html ( only the CA is required as specified by the article )

    kubectl -n rabbitmq-system patch deployment messaging-topology-operator --patch "spec:
    template:
    spec:
      containers:
      - name: manager
        volumeMounts:
        - mountPath: /etc/ssl/certs/rabbitmq-ca.crt
          name: rabbitmq-ca
          subPath: ca.crt
      volumes:
      - name: rabbitmq-ca
        secret:
          defaultMode: 420
          secretName: rmq-tls-secret"
  3. Create the secret "rmq-tls-secret" with the certificate files in eventing-test namespace, which is required by the broker ingress.

Alternatively we have also tried configured the broker config to access the rabbitmq operator as external rabbitmq but ending with same result, getting "Certificate required" error. The yamls is as follows.

kubectl apply -f - << EOF
apiVersion: v1
kind: Secret
metadata:
  name: rabbitmq-secret-credentials
  namespace: eventing-test
stringData:
  username: guest
  password: guest
  uri: https://rabbitmq.rabbitmq.svc.cluster.local:15671/rabbitmq
  port: "5671"
  caSecretName: rmq-tls-secret
EOF
apiVersion: eventing.knative.dev/v1alpha1
kind: RabbitmqBrokerConfig
metadata:
  name: rabbitmq-broker-config
  namespace: eventing-test
spec:
  rabbitmqClusterReference:
    # Configure connectionSecret if an external RabbitMQ cluster is being used.
    connectionSecret:
      name: rabbitmq-secret-credentials
  queueType: quorum

May I know did I left out any other configuration to use the secret? Is there any name constraint on the certificate name? as in the CA has to be "rabbitmq-ca.crt" instead of "ca.crt"?

But I do able to confirm that if I create a random file in the "rmq-tls-secret" secret in "eventing-test" namespace, the broker ingress will instead show the error. "certificate signed by unknown authority". So it is a bit weird when ca.crt is supplied, the error "certificate required" is shown.

gabo1208 commented 1 year ago

No there is no limitations on the naming just on the keys naming, and lets see following the e2e setup:

Basically we create three secrets: `apiVersion: v1 kind: Secret metadata: name: {{ .tlsSecretName }} namespace: {{ BROKER-NAMESPACE }} type: kubernetes.io/tls data: tls.crt: {{ .tlsCert }} tls.key: {{ .tlsKey }}


apiVersion: v1 kind: Secret metadata: name: {{ .caSecretName }} namespace: {{ .broker-namespace }} type: Opaque data: ca.crt: {{ .caCert }}


apiVersion: v1 kind: Secret metadata: name: {{ .caSecretName }} namespace: rabbitmq-system type: Opaque data: ca.crt: {{ .caCert }}`

Try getting this exact same secrets setup (separating your certs in those secrets not just all in one) and then, patch the topology operator (which it seems you are doing perfectly fine. Finally patch the RabbitMQ Cluster as follows:

apiVersion: rabbitmq.com/v1beta1 kind: RabbitmqCluster metadata: name: {{ .rmqClusterName }} namespace: {{ .cluster-namespace }} ... spec: ... tls: caSecretName: {{ .caSecretName }} secretName: {{ .tlsSecretName }} disableNonTLSListeners: true {{ end }}

Jaryllan commented 1 year ago

I have tried the splitting of the secret for different certificate as what you have described and it is still not working.

Actually the issues that we had is on the broker ingress, the pod that gets created upon creation of knative broker. We didn't have issue on the rabbitmq topology, rabbitmq cluster and knative rabbitmq controller. I can see the exchange gets created on the rabbitmq management portal upon applying the yaml manifest of knative rabbitmq controller.

Which mean: Rabbitmq cluster - Working Rabbitmq Topology - Working Knative Rabbitmq Controller - Working Broker Ingress - Not Working (Due to "Certificate required" error in the pod's container log.)

Would like to addon some information

  1. We tried a simple sample app (publish and subscribe) that access the rabbitmq cluster directly with SSL and is working as well.
  2. Compare with the example link that you provided does not have the following section in the rabbitmq cluster, you might want to consider including the following section inside the rabbitmq cluster yaml when you simulate from your side, this is because this enforce the tls certificate must be included as well by the consumer, not just the CA certificate.
  rabbitmq:
    additionalConfig: |
      ssl_options.verify = verify_peer
      ssl_options.fail_if_no_peer_cert = true
gabo1208 commented 1 year ago

Ohh ok ok. One detail right now we don't yet support TLS for https request for the ingress, we just have secured the rabbitmq bits. Tho this feature should be available from v1.11+ we are currently starting to work in the implementation

Jaryllan commented 1 year ago

Correct me if I understand it wrongly from your message. So to say the following is currently not available for TLS?

knative broker ingress --> rabbitmq cluster

gabo1208 commented 1 year ago

That setup is available what is not supported is: https request --> broker ingress

Jaryllan commented 1 year ago

Oh ok, we actually yet to reach the part to curl the broker ingress url due the issue that we have been facing on the knative broker ingress pod itself

knative broker ingress --> rabbitmq cluster

In which the broker ingress kept having "Certificate required" error message in the pod. I have checked that the CA certificate secret is already mounted to the knative broker ingress but still with the same error.

Would like to mention the steps that we did.

  1. Create rabbitmq namespace.
  2. Create certificate with CertManager on to rabbitmq namespace (secret name: rmq-tls-secret)
  3. Based on the link you have provided, create a CA secret with type: Opaque. to rabbitmq and rabbitmq-system namespace (secret name: rmq-ca-secret)
  4. Create the rabbitmq cluster with point no 3's secret.

    tls:
    secretName: rmq-tls-secret     # secret with type: kubernetes.io/tls that contains tls.crt and tls.key
    caSecretName: rmq-ca-secret # secret with type: Opaque that contains ca.crt
    disableNonTLSListeners: true
    
    rabbitmq:
    additionalConfig: |
      ssl_options.verify = verify_peer
      ssl_options.fail_if_no_peer_cert = true
  5. Test the rabbitmq cluster with publish and subscribe with TLS certificate to verify the rabbitmq cluster.

RabbitMQ Cluster: Working

  1. download the rabbitmq topology. https://github.com/rabbitmq/messaging-topology-operator/releases/latest/download/messaging-topology-operator-with-certmanager.yaml
  2. Modify the yaml to use the same Issuer used in point 2 to create the certificate. This is to ensure same CA certificate is being used.
  3. Apply the rabbitmq topology yaml.
  4. As per rabbitmq's article (Source: https://www.rabbitmq.com/kubernetes/operator/tls-topology-operator.html). Patch the topology to use the CA cert generated from point 3.
    kubectl -n rabbitmq-system patch deployment messaging-topology-operator --patch "spec:
    template:
    spec:
      containers:
      - name: manager
        volumeMounts:
        - mountPath: /etc/ssl/certs/rabbitmq-ca.crt
          name: rabbitmq-ca
          subPath: ca.crt
      volumes:
      - name: rabbitmq-ca
        secret:
          defaultMode: 420
          secretName: rmq-ca-secret"
  5. Apply the rabbitmq controller yaml https://github.com/knative-sandbox/eventing-rabbitmq/releases/download/knative-v1.10.0/rabbitmq-broker.yaml.
  6. Verify

Knative rabbitmq controller: Working Knative rabbitmq webhook: Working

  1. Create broker config in the same namespace as the rabbitmq cluster.

    apiVersion: eventing.knative.dev/v1alpha1
    kind: RabbitmqBrokerConfig
    metadata:
    name: rabbitmq-broker-config
    namespace: rabbitmq
    spec:
    rabbitmqClusterReference:
    # Configure name if a RabbitMQ Cluster Operator is being used.
    name: rabbitmq
    namespace: rabbitmq
    queueType: quorum
  2. Create broker in the same namespace as the rabbitmq cluster.

    apiVersion: eventing.knative.dev/v1
    kind: Broker
    metadata:
    annotations:
    eventing.knative.dev/broker.class: RabbitMQBroker
    name: rabbitmq-broker
    namespace: rabbitmq
    spec:
    config:
    apiVersion: eventing.knative.dev/v1alpha1
    kind: RabbitmqBrokerConfig
    name: rabbitmq-broker-config
    namespace: rabbitmq
  3. Check the knative broker ingress pod and check the pod log.

Addons: If the following section is removed from the rabbitmq cluster, the Knative Broker Ingress is working. But we strictly need this configuration section to be enabled.

  rabbitmq:
    additionalConfig: |
      ssl_options.verify = verify_peer
      ssl_options.fail_if_no_peer_cert = true

Summaries: - RabbitMQ cluster - Working - RabbitMQ topology - Working - Knative RabbitMQ controller - Working - Knative Broker Ingress - Not Working (Referring to Knative Broker Ingress ----> RabbitMQ cluster, not the https request to broker ingress.)

gabo1208 commented 1 year ago

I'll be testing this exact same scenario by the end of the week to see what can be going wrong here! I'll post here any findings or questions to see if we get to the bottom of this one :)!

And again, thank you for reporting this!

Question: Which version of the Cluster Operator, Cert Manager and Topology Operator are you using?

Jaryllan commented 1 year ago

Here you go as requested.

RabbitMQ Cluster Operator - v.2.3.0 (Source: https://github.com/rabbitmq/cluster-operator) Cert Manager - v1.12.2 (Source: https://github.com/cert-manager/cert-manager) RabbitMQ Topology Operator - v1.12.0 (Source: https://github.com/rabbitmq/messaging-topology-operator)

Will wait for your findings.

Jaryllan commented 1 year ago

Hi @gabo1208 , any update on your testing for this scenario?

gabo1208 commented 1 year ago

Hey @Jaryllan sorry has been some crazy weeks on my work, I'll try to have it for tomorrow!

gabo1208 commented 1 year ago

Hey @Jaryllan !! You are right, we currently don't support: ssl_options.fail_if_no_peer_cert = true this option since we are not using the certs on the client side in the Dispatcher nor the Ingress.

Want to create the feature request for that :)? Then you or me could work into implementing this 💪🏼

I'm currently changing the docs to reflect this and also creating a basic example on how to do the TLS setup

Jaryllan commented 1 year ago

I believe is good to have this feature since the option will applies to all client that connects to it. I may not be able to assist due to lack of skills in golang, but i found an example that may assist to add the tls certificate and key. Source: https://github.com/rabbitmq/amqp091-go/blob/main/examples_test.go

gabo1208 commented 1 year ago

Is definitely on the roadmap, I'll see what we can do but we need more hands in the project. Thanks for the sample and useful info and feedback :) I'll keep an eye on this issue when we get to implement this

gabo1208 commented 1 year ago

Related to #1112

github-actions[bot] commented 11 months ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

github-actions[bot] commented 8 months ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.