-
Notifications
You must be signed in to change notification settings - Fork 27
Fix TLS flag #44
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
Fix TLS flag #44
Conversation
…al/postgresql-operator into fix-early-tls-deployment
Codecov Report
@@ Coverage Diff @@
## main #44 +/- ##
==========================================
+ Coverage 62.45% 64.05% +1.60%
==========================================
Files 6 6
Lines 815 818 +3
Branches 122 121 -1
==========================================
+ Hits 509 524 +15
+ Misses 276 264 -12
Partials 30 30
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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.
Very good work, Marcelo!
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.
Looks great 2 minor comments
Thanks for the reviews, @Mehdi-Bendriss and @MiaAltieri! |
* fix early tls deployment by only reloading patroni config if it's already running * added unit test for reloading patroni * lint * removing postgres restart check * adding series flags to test apps * adding series flags to test apps * made series into a list * Update test_new_relations.py * updating test to better emulate bundle deploymen * updated tls lib * Fix TLS IP address on CSR * removed failing test for now. This will be fixed before merge * Fix TLS * Update TLS lib * Remove unneeded series * Add comment * Add Juju agent version bootstrap constraint * Add test for restart method * Update TLS lib * Add test for update config method * Update TLS lib * Improve comment Co-authored-by: WRFitch <will.fitch@canonical.com> Co-authored-by: Will Fitch <WRFitch@outlook.com>
Issue
Solution
Context
You can
FOCUS
on the following files when reviewing:src/charm.py
: has the change in the restart logic (to match the k8s charm) and also adds the missing change of the TLS flag (that is used internally by the charm to call the Patroni REST API using either HTTP or HTTPS).tests/integration/test_tls.py
: has the updates that Will made to improve the test (simulating what we have on a bundle deployment: nowait_for_idle
).Other files and what has changed on them:
.github/workflows/ci.yaml
: pinned the Juju Python library version due to https://bugs.launchpad.net/juju/+bug/1992833.lib/charms/postgresql_k8s/v0/postgresql_tls.py
: imported from the k8s charm (it contains the fix for the IP address issue) - it should be ignored in this PR and reviewed on Fix SANs list postgresql-k8s-operator#58 (every change there will be copied to this PR).lib/charms/tls_certificates_interface/v1/tls_certificates.py
: this is a patched version of the library that is used to solve the IP address issue. It was copied from https://github.com/canonical/opensearch-operator/blob/main/lib/charms/tls_certificates_interface/v1/tls_certificates.py (that patch will be pushed to the right repo in some weeks). Thanks @Mehdi-Bendriss!Testing
Release Notes
Thanks @WRFitch for starting this fixes.