feat: TLS authentication
(If this PR fixes a github issue, please add Fixes #<xyz>.)
Fixes #108
Motivation
As mentioned in issue, we would like to make use of TLS authentication since pulsar supports this.
Modifications
This PR:
- Allows only configuring the secure url properties (adminServiceSecureURL & brokerServiceSecureURL) - before it was mandatory to configure adminServiceURL
- When using secure urls (TLS) allows configuring host key verification / allowInsecureConnection
- Adds support for TLS authentication
- ~~Add support to helm chart for volumes/volumeMounts so certificates can be specified~~
Would you prefer seperate PRs for these? Then I can split em up.
Verifying this change
- [x] Make sure that the change passes the CI checks.
No tests were added yet, e2e tests would probably be the best to add. I won't be able to run the github actions and have the test setup so it would be a bit hard to add them. My idea was to add a test similar to resources_test but creates a connection with tls authentication parameters and tries to get/create/delete a resource. Certificates will also need to be created and also passed to the pulsar-broker setup, I wasn't sure what the best way to do this is. I am hoping that one of the other contributors or maintainers who have access to an easy e2e test setup could help with this.
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
- Added integration tests for end-to-end deployment with large payloads (10MB)
- Extended integration test for recovery after broker failure
Documentation
Check the box below.
Need to update docs?
-
[ ]
doc-required(If you need help on updating docs, create a doc issue)
-
[ ]
no-need-doc(Please explain why)
-
[x]
doc(If this PR contains doc changes)
Was not sure what version to set for the helm changes, chose v0.5.0 for now
I have removed the helm chart changes as I see these are usually done in separate PRs. These changes added support for adding volumes to the helm chart to provide the necessary certificates + update CRD.
I have been thinking, would it perhaps be more preferable to allow configuring secrets for the certificates and have the operator look them up instead of being available as volumes to the operator? Then they could be in any namespace together with the pulsar resources they belong to (when giving the operator the necessary RBAC permissions).
Also still looking for some help on how to do the test setup.
We're running a version of this in production and it is working fine for us but it would be nice to have this feature upstreamed.
@labuladong Could you also help take a review on this PR?
Hello. Any ETA to merge this? Thanks
When will this get merged?
I no longer work at the company that owns the branch repository so I cannot update the PR with master/fix conflicts.
@ThomasVerhoeven1998 is anyone of you able to do this?
Otherwise I could make a new branch/PR but I will have to find some free time for this
I no longer work at the company that owns the branch repository so I cannot update the PR with master/fix conflicts.
@ThomasVerhoeven1998 is anyone of you able to do this?
Otherwise I could make a new branch/PR but I will have to find some free time for this
@tomjo I assumed that the owners of the project, bring the PR up to date. But if not, we can see if we can do it ourselfs
cc: @FushuWang
I no longer work at the company that owns the branch repository so I cannot update the PR with master/fix conflicts. @ThomasVerhoeven1998 is anyone of you able to do this? Otherwise I could make a new branch/PR but I will have to find some free time for this
@tomjo I assumed that the owners of the project, bring the PR up to date. But if not, we can see if we can do it ourselfs
cc: @FushuWang
@ThomasVerhoeven1998 Please create a new PR.
I no longer work at the company that owns the branch repository so I cannot update the PR with master/fix conflicts. @ThomasVerhoeven1998 is anyone of you able to do this? Otherwise I could make a new branch/PR but I will have to find some free time for this
@tomjo I assumed that the owners of the project, bring the PR up to date. But if not, we can see if we can do it ourselfs cc: @FushuWang
@ThomasVerhoeven1998 Please create a new PR.
New PR created
New PR is #301