[DPE-5965] - Set private key with juju secrets #29

skourta · 2025-01-24T07:13:42Z

This pull request introduces the ability to set TLS private keys through a config option to pass a juju secret id.
The TLS v4 lib used is forked here.

Config and literals:

config.yaml: Added a new configuration option tls-private-key for specifying the private key for TLS certificates.
src/literals.py: Introduced TLS_PRIVATE_KEY_CONFIG constant to reference the new configuration option.

Event Handling Updates:

src/events/etcd.py: Updated event handlers to manage the TLS private key, including fetching and updating the private key from secrets. [1] [2] [3]

Testing Improvements:

tests/integration/helpers.py: Added a helper function add_secret to facilitate adding secrets during integration tests.
tests/integration/tls/test_private_key_rotation.py: Added new integration tests to verify the private key rotation functionality and ensure the cluster remains accessible after key changes.
tests/unit/test_tls.py: Expanded unit tests to cover the new TLS private key configuration and handling.

This pull request added CA rotation to the charm. ### TLS Certificate Management Enhancements: * **New TLS CA Rotation States:** - Added `TLSCARotationState` enum to represent different stages of CA rotation (`src/literals.py`). * **CA Rotation and Cleanup Methods:** - Introduced methods in `TLSManager` for setting CA rotation state, checking CA status across servers, and cleaning up old CAs (`src/managers/tls.py`). [[1]](diffhunk://#diff-a2c036f44ae9bbab595aad84b23bc3106a342c5ab12f7b0cf3d26268b51cdc83R136-R204) [[2]](diffhunk://#diff-a2c036f44ae9bbab595aad84b23bc3106a342c5ab12f7b0cf3d26268b51cdc83L57-R102) * **Event Handling Updates:** - Added `CleanCAEvent` to handle CA cleanup events and integrated it into the `TLSEvents` class (`src/events/tls.py`). [[1]](diffhunk://#diff-3dfb4a104b407f7d4b2e07795a63745a09cde228a8dbe86602e7b72ea600fbe4L15-R57) [[2]](diffhunk://#diff-3dfb4a104b407f7d4b2e07795a63745a09cde228a8dbe86602e7b72ea600fbe4R88-R89) [[3]](diffhunk://#diff-3dfb4a104b407f7d4b2e07795a63745a09cde228a8dbe86602e7b72ea600fbe4R229-R243) ### Codebase Improvements: * **Logging Enhancements:** - Improved logging messages to provide more context during rolling restarts and CA rotations (`src/charm.py`). [[1]](diffhunk://#diff-b9ed39bbc9c0387bd3e07da31d13373745534a1cd723d3e292c73496b12e307cL68-R78) [[2]](diffhunk://#diff-b9ed39bbc9c0387bd3e07da31d13373745534a1cd723d3e292c73496b12e307cR178-R202)

reneradoi

Thank you Smail, I really appreciate the work you did and all the back-and-forth with the TLS team.

Unfortunately the TLS v4 lib has another couple of flaws here. Especially:

not being able to have separate private keys for multiple use cases
not being able to provide a user-supplied private key when regenerating it

are major downsides from my point of view. We should only consider the solution implemented here as a workaround, as it introduced potential liabilities to our code when we need to access "protected" methods of the lib to fulfill our use cases.

We as a team should make a decision here if we want to go with the workaround for now or request these features to be delivered by the TLS team and postponing the TLS private key topic until then.

src/events/etcd.py

tests/integration/tls/test_private_key_rotation.py

tests/unit/test_charm.py

Mehdi-Bendriss

Thanks Smail! great work!
My main concern is on the lack of 2 separate private keys, as per the spec.
In addition to this, we should avoid the # type: ignore hint, it pollutes the code.

.gitignore

config.yaml

src/events/etcd.py

tests/integration/tls/test_private_key_rotation.py

Mehdi-Bendriss

Thanks Smail, I left a couple of comments.

src/events/etcd.py

…ests

This pull request updates multiple test files to replace the `wait_for_idle` method with the `wait_until` method for better consistency and reliability in test execution. Changes in test files: * [`tests/integration/ha/test_scaling.py`](diffhunk://#diff-1511afef9daad75af5001b5c55a0fb9dfc6ba38d784615be51dd492e4a330309R18): Imported `wait_until` from `helpers_deployment` and replaced `wait_for_idle` with `wait_until` in `test_build_and_deploy` and `test_scale_up` functions. [[1]](diffhunk://#diff-1511afef9daad75af5001b5c55a0fb9dfc6ba38d784615be51dd492e4a330309R18) [[2]](diffhunk://#diff-1511afef9daad75af5001b5c55a0fb9dfc6ba38d784615be51dd492e4a330309L42-R43) [[3]](diffhunk://#diff-1511afef9daad75af5001b5c55a0fb9dfc6ba38d784615be51dd492e4a330309L63-R65) * [`tests/integration/test_charm.py`](diffhunk://#diff-08291d7f4ad7b2caecacb6ae26af540ebbaede9fe42d5e8e1b39d2bdefe4c2d4R20): Imported `wait_until` from `helpers_deployment` and replaced `wait_for_idle` with `wait_until` in `test_build_and_deploy` and `test_update_admin_password` functions. [[1]](diffhunk://#diff-08291d7f4ad7b2caecacb6ae26af540ebbaede9fe42d5e8e1b39d2bdefe4c2d4R20) [[2]](diffhunk://#diff-08291d7f4ad7b2caecacb6ae26af540ebbaede9fe42d5e8e1b39d2bdefe4c2d4L41-R42) [[3]](diffhunk://#diff-08291d7f4ad7b2caecacb6ae26af540ebbaede9fe42d5e8e1b39d2bdefe4c2d4L98-R99) [[4]](diffhunk://#diff-08291d7f4ad7b2caecacb6ae26af540ebbaede9fe42d5e8e1b39d2bdefe4c2d4L107-R108) * [`tests/integration/tls/test_ca_rotation.py`](diffhunk://#diff-a8362a8580830e574ae0f1e2fbd35adbf5ea51ed2ae3a85e78c5a2eebaa4d0c7R25): Imported `wait_until` from `helpers_deployment` and replaced `wait_for_idle` with `wait_until` in `test_build_and_deploy_with_tls` function. [[1]](diffhunk://#diff-a8362a8580830e574ae0f1e2fbd35adbf5ea51ed2ae3a85e78c5a2eebaa4d0c7R25) [[2]](diffhunk://#diff-a8362a8580830e574ae0f1e2fbd35adbf5ea51ed2ae3a85e78c5a2eebaa4d0c7L56-R57) [[3]](diffhunk://#diff-a8362a8580830e574ae0f1e2fbd35adbf5ea51ed2ae3a85e78c5a2eebaa4d0c7L131-R133) * [`tests/integration/tls/test_private_key_rotation.py`](diffhunk://#diff-858b4ba3b588d303d7396271ac3575bbe9585625ee8b278feb1411524d533a97R32): Imported `wait_until` from `helpers_deployment` and replaced `wait_for_idle` with `wait_until` in `test_build_and_deploy_with_tls` and `test_set_private_key` functions. [[1]](diffhunk://#diff-858b4ba3b588d303d7396271ac3575bbe9585625ee8b278feb1411524d533a97R32) [[2]](diffhunk://#diff-858b4ba3b588d303d7396271ac3575bbe9585625ee8b278feb1411524d533a97L63-R64) [[3]](diffhunk://#diff-858b4ba3b588d303d7396271ac3575bbe9585625ee8b278feb1411524d533a97L170-R171) [[4]](diffhunk://#diff-858b4ba3b588d303d7396271ac3575bbe9585625ee8b278feb1411524d533a97L235-R236) * [`tests/integration/tls/test_tls.py`](diffhunk://#diff-5877b0903d94204b2a76f3b252020e23fc436bd77688fe77e00a66b864ac9fe0R25): Imported `wait_until` from `helpers_deployment` and replaced `wait_for_idle` with `wait_until` in multiple functions including `test_build_and_deploy_with_tls`, `test_disable_tls`, `test_enable_tls`, `test_disable_and_enable_peer_tls`, `test_disable_and_enable_client_tls`, and `test_certificate_expiration`. [[1]](diffhunk://#diff-5877b0903d94204b2a76f3b252020e23fc436bd77688fe77e00a66b864ac9fe0R25) [[2]](diffhunk://#diff-5877b0903d94204b2a76f3b252020e23fc436bd77688fe77e00a66b864ac9fe0L57-R58) [[3]](diffhunk://#diff-5877b0903d94204b2a76f3b252020e23fc436bd77688fe77e00a66b864ac9fe0L119-R120) [[4]](diffhunk://#diff-5877b0903d94204b2a76f3b252020e23fc436bd77688fe77e00a66b864ac9fe0L175-R176) [[5]](diffhunk://#diff-5877b0903d94204b2a76f3b252020e23fc436bd77688fe77e00a66b864ac9fe0L246-R247) [[6]](diffhunk://#diff-5877b0903d94204b2a76f3b252020e23fc436bd77688fe77e00a66b864ac9fe0L302-R303) [[7]](diffhunk://#diff-5877b0903d94204b2a76f3b252020e23fc436bd77688fe77e00a66b864ac9fe0L371-R372) [[8]](diffhunk://#diff-5877b0903d94204b2a76f3b252020e23fc436bd77688fe77e00a66b864ac9fe0L423-R424) [[9]](diffhunk://#diff-5877b0903d94204b2a76f3b252020e23fc436bd77688fe77e00a66b864ac9fe0L490-R491) [[10]](diffhunk://#diff-5877b0903d94204b2a76f3b252020e23fc436bd77688fe77e00a66b864ac9fe0L549-R550)

Mehdi-Bendriss

Thanks Smail! Great work! (just a small typo to fix)

config.yaml

Co-authored-by: Mehdi Bendriss <[email protected]>

reneradoi

Thank you Smail!

skourta added 30 commits January 8, 2025 15:34

fetched lib and added interface in metadata

21a339c

added dependencies for the tls lib

9ed03f5

added etcdpaths dataclass

8d75058

feat: turn on TLS

ea586d1

feat: turn off TLS

14d012b

add some unit tests and fix failing unit test for the charm

705a9ac

fix typo

d6475d7

remove uneeded deps in charmcraft.yaml and update poetry.lock

949302a

add integration tests

ac355e6

added rolling ops to handle activating and deactivating tls

7b76812

enhanced docstrings

1ee73df

fixed enabling tls on cluster creation

7c81778

fixed unit tests and tweaked integration tests

ea43909

use 3 units instead of 2 in tls integration tests

d07de5d

fixed based on feedback

d3203e5

couple of fixed based on feedback

cd3c4e8

added more unit tests

dd67d30

implementing Mehdi feedback

fe70bdd

fix tests based on feedback

a1a3b19

implement feedback for path handling

c02245c

fixed misuse of tenacity

d72956c

fix updating certs

5a889e6

rework to enable single TLS type activation

9937fd4

update tests

a901a02

go from kebab to snake case for relation data

8b106ef

fix typo

db2e97f

add logging messagign and some tests

193a88e

reformat

a209912

type and bug fix in test

ec3f188

added unit test for certificate expiration

c3a9d96

skourta added 4 commits January 27, 2025 08:42

Merge branch 'main' into DPE-5965-private-keys-juju-secrets

01f2c15

enhance unit test coverage

537c434

updated snap lib

739bd92

reneradoi requested changes Jan 28, 2025

View reviewed changes

Mehdi-Bendriss reviewed Jan 28, 2025

View reviewed changes

use 2 config options

3a01649

skourta requested review from reneradoi and Mehdi-Bendriss January 29, 2025 11:21

add TLS prefix to status

43fc4b6

Mehdi-Bendriss reviewed Jan 29, 2025

View reviewed changes

src/events/etcd.py Outdated Show resolved Hide resolved

src/events/etcd.py Show resolved Hide resolved

skourta added 2 commits January 30, 2025 06:11

add status blocked on secret not found and assertions on it in unit t…

b0c680e

…ests

reneradoi mentioned this pull request Feb 3, 2025

[DPE-6341] Upgrade to 24.04 base and etcd v3.5.18 #32

Merged

skourta added 3 commits February 5, 2025 11:28

upgrade lib from fork

2482b30

add checks on client/peer secret nto changing when the other changes

d943dd3

Merge branch '3.5/edge' into DPE-5965-private-keys-juju-secrets

8edab7e

skourta changed the base branch from main to 3.5/edge February 5, 2025 13:37

skourta added 4 commits February 5, 2025 13:39

added TODO comment

97ecb74

update dp interfaces

0f86fea

fix flakey unit test

624b8ad

remove build charm

14b6c0c

Mehdi-Bendriss previously approved these changes Feb 6, 2025

View reviewed changes

config.yaml Outdated Show resolved Hide resolved

config.yaml Outdated Show resolved Hide resolved

fix typo

419bc44

Co-authored-by: Mehdi Bendriss <[email protected]>

skourta dismissed Mehdi-Bendriss’s stale review via 419bc44 February 6, 2025 12:22

add empty line in config.yaml

06aefe2

reneradoi approved these changes Feb 6, 2025

View reviewed changes

Mehdi-Bendriss approved these changes Feb 6, 2025

View reviewed changes

skourta merged commit d40800d into 3.5/edge Feb 6, 2025
13 of 14 checks passed

skourta deleted the DPE-5965-private-keys-juju-secrets branch February 6, 2025 14:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DPE-5965] - Set private key with juju secrets #29

[DPE-5965] - Set private key with juju secrets #29

skourta commented Jan 24, 2025 •

edited

Loading

reneradoi left a comment

Mehdi-Bendriss left a comment

Mehdi-Bendriss left a comment

Mehdi-Bendriss left a comment

reneradoi left a comment

[DPE-5965] - Set private key with juju secrets #29

[DPE-5965] - Set private key with juju secrets #29

Conversation

skourta commented Jan 24, 2025 • edited Loading

Config and literals:

Event Handling Updates:

Testing Improvements:

reneradoi left a comment

Choose a reason for hiding this comment

Mehdi-Bendriss left a comment

Choose a reason for hiding this comment

Mehdi-Bendriss left a comment

Choose a reason for hiding this comment

Mehdi-Bendriss left a comment

Choose a reason for hiding this comment

reneradoi left a comment

Choose a reason for hiding this comment

skourta commented Jan 24, 2025 •

edited

Loading