Skip to content

Conversation

@reneradoi
Copy link

@reneradoi reneradoi commented Apr 15, 2025

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 08:59
@reneradoi reneradoi added the enhancement New feature, UI change, or workload upgrade label Apr 15, 2025
@reneradoi reneradoi marked this pull request as ready for review April 15, 2025 09:02
@reneradoi reneradoi marked this pull request as draft April 15, 2025 09:03
@codecov
Copy link

codecov bot commented Apr 15, 2025

Codecov Report

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

Project coverage is 73.52%. Comparing base (68ef417) to head (8b9a54d).
Report is 8 commits behind head on 16/edge.

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

❌ Your patch status has failed because the patch coverage (14.51%) 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     #926      +/-   ##
===========================================
- Coverage    74.47%   73.52%   -0.96%     
===========================================
  Files           12       12              
  Lines         3550     3565      +15     
  Branches       508      510       +2     
===========================================
- Hits          2644     2621      -23     
- Misses         709      749      +40     
+ Partials       197      195       -2     

☔ 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 16, 2025 11:30
@reneradoi reneradoi requested review from a team, dragomirp, marceloneppel and taurus-forever and removed request for a team April 16, 2025 11:31
@reneradoi reneradoi requested a review from dragomirp April 16, 2025 13:03
@reneradoi reneradoi requested a review from dragomirp April 16, 2025 17:35
Copy link
Member

@marceloneppel marceloneppel left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks, @reneradoi!

@reneradoi reneradoi requested a review from marceloneppel April 17, 2025 05:20
Copy link
Member

@marceloneppel marceloneppel left a comment

Choose a reason for hiding this comment

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

The async replication is failing with the following error after trying to set a secret through the standby cluster:

unit-postgresql-k8s-1: 17:54:38 ERROR unit.postgresql-k8s/1.juju-log Uncaught exception while in charm code:
Traceback (most recent call last):
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/venv/lib/python3.12/site-packages/ops/model.py", line 3320, in _run
    result = subprocess.run(args, **kwargs)  # type: ignore
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/subprocess.py", line 571, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '('/var/lib/juju/tools/unit-postgresql-k8s-1/relation-set', '-r', '3', '--app', '--file', '-')' returned non-zero exit status 1.

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/src/charm.py", line 2475, in <module>
    main(PostgresqlOperatorCharm, use_juju_for_storage=True)
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/venv/lib/python3.12/site-packages/ops/__init__.py", line 343, in __call__
    return _main.main(charm_class=charm_class, use_juju_for_storage=use_juju_for_storage)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/venv/lib/python3.12/site-packages/ops/_main.py", line 558, in main
    manager.run()
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/venv/lib/python3.12/site-packages/ops/_main.py", line 543, in run
    self._emit()
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/venv/lib/python3.12/site-packages/ops/_main.py", line 502, in _emit
    self._emit_charm_event(self.dispatcher.event_name)
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/venv/lib/python3.12/site-packages/ops/_main.py", line 530, in _emit_charm_event
    event_to_emit.emit(*args, **kwargs)
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/venv/lib/python3.12/site-packages/ops/framework.py", line 347, in emit
    framework._emit(event)
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/venv/lib/python3.12/site-packages/ops/framework.py", line 906, in _emit
    self._reemit(event_path)
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/venv/lib/python3.12/site-packages/ops/framework.py", line 996, in _reemit
    custom_handler(event)
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/lib/charms/tempo_coordinator_k8s/v0/charm_tracing.py", line 1109, in wrapped_function
    return callable(*args, **kwargs)  # type: ignore
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/src/charm.py", line 1529, in _on_update_status
    self.async_replication.update_async_replication_data()
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/lib/charms/tempo_coordinator_k8s/v0/charm_tracing.py", line 1109, in wrapped_function
    return callable(*args, **kwargs)  # type: ignore
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/src/relations/async_replication.py", line 725, in update_async_replication_data
    self._update_primary_cluster_data()
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/lib/charms/tempo_coordinator_k8s/v0/charm_tracing.py", line 1109, in wrapped_function
    return callable(*args, **kwargs)  # type: ignore
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/src/relations/async_replication.py", line 772, in _update_primary_cluster_data
    async_relation.data[self.charm.app]["primary-cluster-data"] = json.dumps(
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/venv/lib/python3.12/site-packages/ops/model.py", line 1908, in __setitem__
    self._commit(key, value)
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/venv/lib/python3.12/site-packages/ops/model.py", line 1912, in _commit
    self._backend.update_relation_data(self.relation.id, self._entity, key, value)
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/venv/lib/python3.12/site-packages/ops/model.py", line 3669, in update_relation_data
    self.relation_set(relation_id, key, value, isinstance(_entity, Application))
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/venv/lib/python3.12/site-packages/ops/model.py", line 3423, in relation_set
    self._run(*args, input_stream=content)
  File "/var/lib/juju/agents/unit-postgresql-k8s-1/charm/venv/lib/python3.12/site-packages/ops/model.py", line 3322, in _run
    raise ModelError(e.stderr) from e
ops.model.ModelError: ERROR cannot read relation application settings: permission denied (unauthorized access)

We'll investigate further next week.

@reneradoi
Copy link
Author

Thanks @marceloneppel for finding this. I am not sure how this relates to the change, as it doesn't change how the data is stored internally. We could have a debugging session later today to try to find the root cause for this error. Please let me know how to proceed.

@marceloneppel
Copy link
Member

The previous error I commented on here in the PR was related to using a secret with some messy data.

So, the PR LGTM!

@reneradoi reneradoi merged commit 73d3660 into 16/edge Apr 23, 2025
63 of 64 checks passed
@reneradoi reneradoi deleted the manage-passwords-with-user-secrets branch April 23, 2025 09:55
@shipperizer shipperizer mentioned this pull request Nov 28, 2025
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