Enable tls

[gabo1208]

Changes

• 🎁 Enable Tls for RabbitMQ Broker for known CA authorities
• 🎁 Enable Tls for RabbitMQ Source for known CA authorities
• 🎁 Now the cluster port is configurable on the rabbitmq cluster secret on the port param (if you are not using the RabbitMQ Topology Operator)
• 🧹 Minor refactor to rabbit service helper

/kind enhancement

Fixes #818 #566

Release Note

- 🎁 Enable Tls for RabbitMQ Broker for known CA authorities
- 🎁 Enable Tls for RabbitMQ Source for known CA authorities
- 🎁 RabbitMQ Cluster connection port is configurable on the rabbitmq cluster secret on the `port` param (if you are not using the RabbitMQ Topology Operator)

Docs

[knative-prow]

@gabo1208: The label(s) kind/<kind> cannot be applied, because the repository doesn't have them.

In response to this:

Changes

/kind

Fixes #

Release Note

Docs

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[codecov]

Codecov Report

Merging #893 (00a9960) into main (70ee1c8) will increase coverage by 0.53%.
The diff coverage is 68.00%.

@@            Coverage Diff             @@
##             main     #893      +/-   ##
==========================================
+ Coverage   71.11%   71.64%   +0.53%     
==========================================
  Files          43       43              
  Lines        2970     2984      +14     
==========================================
+ Hits         2112     2138      +26     
+ Misses        779      766      -13     
- Partials       79       80       +1     

Impacted Files	Coverage Δ
pkg/apis/duck/v1beta1/rabbit.go	0.00% <ø> (ø)
pkg/rabbit/service.go	18.59% <52.94%> (+11.13%)	⬆️
pkg/adapter/adapter.go	59.49% <100.00%> (-2.86%)	⬇️
pkg/rabbit/setup.go	84.15% <100.00%> (+1.00%)	⬆️
pkg/reconciler/source/resources/receive_adapter.go	95.38% <0.00%> (-0.04%)	⬇️
pkg/dispatcher/dispatcher.go	42.96% <0.00%> (+0.31%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[gab-satchi]

Just did a quick pass. There are a few things that stand out:

• I don't think we should have a flag to say "ssl". When it's a RabbitMQCluster, we could get the tls config from that reference. So the ssl field is only useful for the Secret reference. It might be okay to do what messaging-topology-operator does which is just rely on the uri to have an http or https.
• I see us switching the protocol based on the ssl flag, but not doing loading any of the system certs which may be required for self-sigend certs.
• self-signed certs process would need to be documented (can be punted to followup)
• This would be a feature to add some e2e tests for

[gabo1208]

Just did a quick pass. There are a few things that stand out:

• I don't think we should have a flag to say "ssl". When it's a RabbitMQCluster, we could get the tls config from that reference. So the ssl field is only useful for the Secret reference. It might be okay to do what messaging-topology-operator does which is just rely on the uri to have an http or https.
• I see us switching the protocol based on the ssl flag, but not doing loading any of the system certs which may be required for self-sigend certs.
• self-signed certs process would need to be documented (can be punted to followup)
• This would be a feature to add some e2e tests for

For the ssl regard I was thinking if the rabbitmq is exposed on a https endpoint but is not using amqps is better to have something explicit (or that can be easily setup as it is a string flag) the rest of the comments are on point and yes I'd like to left those for a follow up pr

[gab-satchi]

If the self-signed bits aren't in then we should remove those from the release notes and the "Fixes". You'd need to add the ca certs when making the amqp call so it can be trusted.

[gab-satchi]

Just some minor docs readability things. If you can remove the self-signed bits from the description, this is good to go

[gab-satchi]

I think we should test the RabbitMQURL function. It feels more like a util anyways

[gabo1208]

maybe test the rabbitmqurl method in a following pr?

[gab-satchi]

/lgtm

[gabo1208]

/override codecov/patch

[knative-prow]

@gabo1208: gabo1208 unauthorized: /override is restricted to Repo administrators.

In response to this:

/override codecov/patch

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[gab-satchi]

/lgtm

[knative-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gab-satchi, gabo1208

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [gab-satchi,gabo1208]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[dprotaso]

/override codecov/patch

[knative-prow]

@dprotaso: Overrode contexts on behalf of dprotaso: codecov/patch

In response to this:

/override codecov/patch

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.