Skip to content

Conversation

@ThomasVerhoeven1998
Copy link
Contributor

@ThomasVerhoeven1998 ThomasVerhoeven1998 commented Apr 8, 2025

Fixes streamnative/pulsar-resources-operator#108

Motivation

As described in the linked issue, we want to support TLS authentication in the Pulsar Resources Operator, which is already supported by Pulsar itself. This improves security and flexibility for Pulsar deployments that require encrypted and authenticated connections.

Modifications

This PR includes the following changes:

  • Allow configuration of only the secure URL properties (adminServiceSecureURL and brokerServiceSecureURL). Previously, it was mandatory to configure adminServiceURL.
  • When using secure URLs (TLS), add support for:
    • Hostname verification
    • Disabling certificate verification via allowInsecureConnection
  • Add support for TLS authentication using client certificates.
  • Extend the Helm chart to support volumes and volumeMounts, so users can mount TLS certificates into the operator.

Verifying this change

  • Make sure that the change passes the CI checks.

This change is not yet covered by tests.

End-to-end (e2e) tests would be most appropriate here. However, I currently don’t have access to a full GitHub Actions setup with a TLS-enabled Pulsar broker. I suggest we add a new e2e test that:

  • Spins up a Pulsar broker with TLS enabled.
  • Loads required certificates.
  • Verifies resource creation, retrieval, and deletion using TLS authentication.

I’d be happy to collaborate on this if another contributor with access to the test infrastructure can help set up the environment.

Documentation

  • doc – This PR includes changes that will require corresponding documentation updates, such as usage examples for the new TLS fields and Helm chart enhancements.

tomjo and others added 30 commits February 15, 2024 15:08
* improve log

* improve

* fix lint

* comment

* Update pkg/connection/reconciler.go

Co-authored-by: Max Xu <[email protected]>

* fix go build

---------

Co-authored-by: Max Xu <[email protected]>
* update pulsar-client-go lib

Signed-off-by: ericsyh <[email protected]>

* update dockerfile

Signed-off-by: ericsyh <[email protected]>

* fix the go mod

Signed-off-by: ericsyh <[email protected]>

---------

Signed-off-by: ericsyh <[email protected]>
* update resource operator chart

Signed-off-by: ericsyh <[email protected]>

* fix lint

Signed-off-by: ericsyh <[email protected]>

* update

Signed-off-by: ericsyh <[email protected]>

* update k8s test

Signed-off-by: ericsyh <[email protected]>

* update appversion

Signed-off-by: ericsyh <[email protected]>

* update the chart version

Signed-off-by: ericsyh <[email protected]>

---------

Signed-off-by: ericsyh <[email protected]>
* fix: add olm required labels

Signed-off-by: ericsyh <[email protected]>

* update gitignore

Signed-off-by: ericsyh <[email protected]>

* update csv

Signed-off-by: ericsyh <[email protected]>

---------

Signed-off-by: ericsyh <[email protected]>
…treamnative#194)

Bumps [golang.org/x/net](https://github.com/golang/net) from 0.17.0 to 0.23.0.
- [Commits](golang/net@v0.17.0...v0.23.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Matteo Merli <[email protected]>
* add crd for pulsar package & function & connector

* add controllers

* add tests

* Update api/v1alpha1/zz_generated.deepcopy.go

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* fix ci

* fix lint

* fix build

* fix build

* fix test

* fix lint

* fix golint

* fix install

* add to charts

* fix role

* fix controller

* fix controller

* fix package

* remove package test

* fix ci

* fix

* use functions worker

* revert

* fix admin cli

* fix

* fix charts

* fix license

* fix client version

* fix

* skip function package

* fix reconcile

* fix ci

* fix

* fix pulsar

* fix rep

* fix url

* fix tab

* fix

* cleanup proxy

* fix container name

* fix builtin

* fix connectors

* use pulsar-all

* docker hub login

* fix pulsarctl

* fix narExtractionDirectory

* timeout

* fix name

* add docs

* fix license

* fix chart role

* fix

* fix license and lint

* fix lint

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Neng Lu <[email protected]>
* chore: upgrade chart and change the image registry

Signed-off-by: ericsyh <[email protected]>

* update to use the 0.4.12

Signed-off-by: ericsyh <[email protected]>

---------

Signed-off-by: ericsyh <[email protected]>
* fix: olm release CI bug

Signed-off-by: ericsyh <[email protected]>

* add a test job

Signed-off-by: ericsyh <[email protected]>

* add dependency

Signed-off-by: ericsyh <[email protected]>

* remove test job

Signed-off-by: ericsyh <[email protected]>

---------

Signed-off-by: ericsyh <[email protected]>
streamnative#219)

* fix function reconcile uses incorrect status to check if update or not

* fix ci to cleanup

* fix style

* fix

* fix cleanup
* mark the configs optional for function

* fix CryptoConfig

* revert chart Changes
* release helm chart 0.5.1

# Conflicts:
#	charts/pulsar-resources-operator/Chart.yaml
#	charts/pulsar-resources-operator/tests/deployment_test.yaml

* fix lint

# Conflicts:
#	charts/pulsar-resources-operator/tests/deployment_test.yaml

* bump to 0.5.2

* fix header
* remove the kube-rbac-proxy

Signed-off-by: ericsyh <[email protected]>

* update the test

Signed-off-by: ericsyh <[email protected]>

* fix ci

Signed-off-by: ericsyh <[email protected]>

---------

Signed-off-by: ericsyh <[email protected]>
freeznet and others added 8 commits April 8, 2025 10:00
fix makefile gen path and noused dep
…ive#287)

* allow using file:// to manage packages

* add cloud storage support

* fix ci

* go mod tidy

* fix mod

* fix ci

* fix ci

* fix path parser

* fix package

* sync charts

* fix lint

* fix ci

* fix lint
Update Chart.yaml and README.md to reflect the new pre-release version, preparing for the next iteration of the Pulsar Resources Operator chart.
# Conflicts:
#	api/v1alpha1/common.go
#	config/crd/bases/resource.streamnative.io_pulsarconnections.yaml
#	docs/pulsar_connection.md
#	pkg/admin/interface.go
@github-actions
Copy link
Contributor

github-actions bot commented Apr 8, 2025

@ThomasVerhoeven1998:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@github-actions github-actions bot added doc-info-missing This pr needs to mark a document option in description and removed doc-info-missing This pr needs to mark a document option in description labels Apr 8, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Apr 8, 2025

@ThomasVerhoeven1998:Thanks for providing doc info!

@github-actions github-actions bot added the doc This pr contains a document label Apr 8, 2025
@lhotari lhotari mentioned this pull request Apr 8, 2025
4 tasks
@lhotari
Copy link
Member

lhotari commented Apr 8, 2025

@freeznet Please review

Copy link
Member

@freeznet freeznet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes lgtm, but could you please add the changes to helm charts as well, to address the Extend the Helm chart to support volumes and volumeMounts, so users can mount TLS certificates into the operator. part?

Copy link
Member

@freeznet freeznet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i noticed the helm charts part are addressed in #184, I will approve this PR first, and later will spin a new PR to add e2e tests.

@ThomasVerhoeven1998
Copy link
Contributor Author

@freeznet do you want me to merge this PR in the #184 PR now? Or open a separate PR with both changes?

@freeznet
Copy link
Member

@ThomasVerhoeven1998 could you please rebase the master so we could make the e2e ready to use? i have created #306 to fix the ci failure from forked repo's PR.

@ThomasVerhoeven1998
Copy link
Contributor Author

@ThomasVerhoeven1998 could you please rebase the master so we could make the e2e ready to use? i have created #306 to fix the ci failure from forked repo's PR.

Rebase done

Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, good work @ThomasVerhoeven1998

@lhotari lhotari merged commit a506176 into streamnative:main Apr 16, 2025
5 checks passed
@ThomasVerhoeven1998
Copy link
Contributor Author

@freeznet @lhotari Is this already released in the latest version? Cause we really want to use it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc This pr contains a document

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support TLS authentication

10 participants