Skip to content

Conversation

@reneradoi
Copy link

@reneradoi reneradoi commented Apr 15, 2025

backported from postgresql-k8s-operator

This PR introduces juju user secrets for managing passwords. It contains the following changes:

Functionality:

  • no longer support get-password and set-password actions
  • new config option system-users to configure a secret that includes the system users' password(s)
  • it is no longer possible to leave out the password parameter to trigger a password rotation as secrets cannot have empty values

Implementation:

  • add handler for secret_changed event to charm.py
  • add method get_secret_from_id() to charm.py
  • trigger updating the system user passwords on config_changed
  • consider pre-configured system user passwords on leader_elected
  • remove get_password handler
  • replace set_password handler with _update_admin_passwords() method
  • _update_admin_passwords() is responsible for the actual business logic:
    • retrieving the passwords from the configured secret
    • checking if password updates need to be performed
    • calling the postgresql.update_user_password() method

Testing:

  • remove integration tests that are no longer required, such as testing for empty password or testing if the output of get-password is the same as the output of juju show-secret
  • adjust integration test helpers for get_password() and set_password to use secrets
  • remove unit tests for get-/set-password actions

@reneradoi reneradoi changed the base branch from main to 16/edge April 15, 2025 10:33
@reneradoi reneradoi added the enhancement New feature, UI change, or workload upgrade label Apr 15, 2025
@codecov
Copy link

codecov bot commented Apr 15, 2025

Codecov Report

Attention: Patch coverage is 15.87302% with 53 lines in your changes missing coverage. Please review.

Project coverage is 71.10%. Comparing base (9a41525) to head (b69b311).
Report is 4 commits behind head on 16/edge.

Files with missing lines Patch % Lines
src/charm.py 13.11% 50 Missing and 3 partials ⚠️

❌ Your patch status has failed because the patch coverage (15.87%) is below the target coverage (33.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           16/edge     #833      +/-   ##
===========================================
- Coverage    72.14%   71.10%   -1.04%     
===========================================
  Files           14       14              
  Lines         3701     3724      +23     
  Branches       548      553       +5     
===========================================
- Hits          2670     2648      -22     
- Misses         854      899      +45     
  Partials       177      177              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@reneradoi reneradoi marked this pull request as ready for review April 17, 2025 07:40
@reneradoi reneradoi requested review from a team and taurus-forever and removed request for a team April 17, 2025 07:41
@dragomirp
Copy link
Contributor

Looks like the downgrade of LXD to LTS version didn't get applied in the merge of main and it is likely causing the instability of the arm self healing test. Started a new merge in #843 including a manual addition of that change. I think it's OK the merge the current PR as is or alternatively I can sync it up after #843 gets merged. @reneradoi, @marceloneppel what do you think?

@reneradoi
Copy link
Author

I think it's OK the merge the current PR as is or alternatively I can sync it up after #843 gets merged. @reneradoi, @marceloneppel what do you think?

Thank you for investigating the failure @dragomirp , I'm fine with merging it as it is (after Marcelo confirmed the testing) if there's no concerns from your side.

@reneradoi reneradoi merged commit d7e5a8c into 16/edge Apr 23, 2025
61 of 63 checks passed
@reneradoi reneradoi deleted the manage-passwords-with-user-secrets branch April 23, 2025 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature, UI change, or workload upgrade Libraries: Out of sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants