-
Notifications
You must be signed in to change notification settings - Fork 227
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
Feature/pulsar manager v0.2.0 with jwt setup admin account creation #219
Feature/pulsar manager v0.2.0 with jwt setup admin account creation #219
Conversation
Okay, tests done and passing, I think this is good to go. |
b4ffe85
to
71a73fb
Compare
@tuteng Could these changes be merged? I would love to be able to set the JWT via a Secret. |
@csthomas1 would you like to rebase the changes? @Mortom123 Please review this. Would something like this be useful? |
Yes, I'll try to take a look at this later this week.
…On Wed, Feb 14, 2024 at 1:22 PM Lari Hotari ***@***.***> wrote:
@csthomas1 <https://github.com/csthomas1> would you like to rebase the
changes?
@Mortom123 <https://github.com/Mortom123> Please review this. Would
something like this be useful?
—
Reply to this email directly, view it on GitHub
<#219 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AQ5QVSJLE24LWENQJZKJXIDYTT6EZAVCNFSM5M63B5JKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOJUGQZTMNRXGUZQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@csthomas1 Awesome! Please ensure compatibility with changes that @Mortom123 made in #457. Looking forward to your contribution! |
Conflicts: .ci/chart_test.sh .ci/clusters/values-jwt-asymmetric.yaml .ci/clusters/values-jwt-symmetric.yaml .ci/clusters/values-tls.yaml .ci/helm.sh .github/workflows/pulsar_jwt_asymmetric.yml .github/workflows/pulsar_jwt_symmetric.yml .github/workflows/pulsar_tls.yml charts/pulsar/templates/broker-configmap.yaml charts/pulsar/templates/proxy-configmap.yaml charts/pulsar/templates/pulsar-manager-statefulset.yaml charts/pulsar/values.yaml
c9285e2
to
279c454
Compare
… and asymmetric jwt tests
@csthomas1 It's possible to get a ssh shell into the build VM if you open a PR to your own fork. This comment and screenshot might help with that: #448 (comment) . The ssh shell is only available in fork builds because of security reasons. It won't get activated for the master branch build in the fork since the rule is based a PR event. In the shell, you can run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! Thanks for the contribution @csthomas1 !
Fixes #133
Fixes #108
Fixes #86
Fixes #31
Motivation
The docker image for Pulsar Manager v0.2.0 includes support for JWT-based configurations, so long as the appropriate environment variables are specified:
The k8s secrets that can populate each of the above environment variables are created by the 'prepare_helm_release.sh' script when it is run prior to installation of the helm chart. Unfortunately, the helm chart doesn't current provide a way to directly expose these secrets to the Pulsar Manager deployment. Instead, it requires that these values be separately set in the "configData" map of the "pulsar_manager" section, which creates the opportunity for them to go out of sync. Further, it exposes in clear text potentially sensitive values (the superuser token and the key material) in the ConfigMap alongside other environment settings.
Pulsar Manager v0.2.0 has also introduced a user management capability that is enabled by default, and requires that the initial admin/superuser account be created via web service call (see https://github.com/apache/pulsar-manager/blob/master/README.md).
Finally, the current helm chart mistakenly conflates the Pulsar Manager admin user's credentials with those of the Postgres user account used to establish a connection to the Pulsar Manager's internal database -- it mounts the admin user's credentials secret to the "USERNAME" and "PASSWORD" environment variables, despite the fact that these control only the datasource connection parameters as described above. Further, with the current secret mounting approach, changing the admin credentials actually breaks the Pulsar Manager deployment because the pulsar database credentials are hardcoded in the postgres db initialization script.
The changes included in this PR do the following:
Modifications
Upon launch, the sidecar will attempt to curl the pulsar-manager's main page every 3 seconds until it receives a successful status. At that point, it will perform the procedure described in the pulsar manager README.md (get a CSRF token, then
make another service call to create the admin account). It will then sleep forever.
Verifying this change