Skip to content

Conversation

@marceloneppel
Copy link
Member

@marceloneppel marceloneppel commented Oct 26, 2022

Issue

  • Jira issue: DPE-844
  • The k8s services that are created by the charm to redirect traffic to the primary and the replicas were not part of the certificate signing request sent to the TLS certificates operator.
  • With that, in the first test run on a fresh microk8s installation the charm got stuck trying to call the Patroni REST API without success, as the hostname doesn’t match with the ones from the certificate. This was not happening in other test runs, but could happen in other situations. to the charm sending a certificate signing request (CSR) with it's IP address as a DNS name instead of an IP address.

Solution

  • Update the code to add those services in the certificate signing request.

Context

  • You can FOCUS on the following files when reviewing:

    • lib/charms/postgresql_k8s/v0/postgresql_tls.py: it contains the fix for the IP address issue in the VM charm and also a way to provide additional DNS names for the certificate signing request.

    • src/charm.py: it sends the k8s service names to the PostgreSQL TLS library.

  • Other files and what has changed on them:

  • In a future PR those k8s services probably should be replaced by the pods endpoints.

Testing

  • It's very complicated to add a test to prevent this issue again in this repo because it happens (at least is what we could check) only with charm coming from Charmhub (we imagine that it's because they take some time to be downloaded to the model, so the events from it and the TLS certificates operator take a different order in the first deployment, but in the second and in the later deployment the charm is already there, so the order is different).
  • The test that will check the issue is fixed is the one from https://github.com/canonical/postgresql-k8s-bundle/blob/6444a12423eacf45447debe82e2ab9c1f443c98a/tests/integration/bundle/test_tls_bundle.py (it fails every time on Giithub because every CI run uses a fresh microk8s installation).
  • This fix was manually tested by publishing it in a different channel of Charmhub and using it to run the above test.

Release Notes

  • Add the k8s services as DNS names on the certificate signing request.
  • Update the PostgreSQL k8s library to handle this situation and also IP address issues in the VM charm.

MiaAltieri
MiaAltieri previously approved these changes Oct 27, 2022
Copy link

@MiaAltieri MiaAltieri left a comment

Choose a reason for hiding this comment

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

LATM (looks amazing to me)

WRFitch
WRFitch previously approved these changes Oct 27, 2022
@marceloneppel marceloneppel merged commit df124ae into main Oct 27, 2022
@marceloneppel marceloneppel deleted the fix-sans-list branch October 27, 2022 10:33
@marceloneppel
Copy link
Member Author

Thanks for the reviews, @MiaAltieri, @WRFitch and @Mehdi-Bendriss!

BON4 pushed a commit to BON4/postgresql-k8s-operator that referenced this pull request May 20, 2024
* Fix SANs list

* Improve TLS lib

* Add new exception [skip ci]

* Pin juju version

* Improve library property set
github-actions bot added a commit to canonical/test-runners-2-github-x64-postgresql-k8s-operator that referenced this pull request May 26, 2024
github-actions bot added a commit to canonical/test-runners-2-is-x64-postgresql-k8s-operator that referenced this pull request May 26, 2024
github-actions bot added a commit to canonical/test-runners-2-azure-arm64-postgresql-k8s-operator that referenced this pull request May 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants