Skip to content
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

CSS-10666: Add enable DB tls config #42

Merged
merged 5 commits into from
Sep 18, 2024
Merged

CSS-10666: Add enable DB tls config #42

merged 5 commits into from
Sep 18, 2024

Conversation

kelkawi-a
Copy link
Contributor

@kelkawi-a kelkawi-a commented Sep 16, 2024

This PR adds the db-tls-enabled config, which allows the user to specify whether or not the persistence and visibility database has TLS enabled. It also updates the integration tests and workflows in line with recent updates made to our charms.

Note: I've disabled one of the integration tests which tests that a server upgrade from v1.20 to v1.21 will result in a schema upgrade. This is due to a bug in pylibjuju in how charms are deployed from Charmhub with custom resources, which will need to be investigated later. In any case, the changes introduces here do not have any effect on this process.

@kelkawi-a kelkawi-a force-pushed the css-10666-tls branch 4 times, most recently from 85e116a to b9e2264 Compare September 16, 2024 13:12
Copy link

@mertalpt mertalpt left a comment

Choose a reason for hiding this comment

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

Will approve after threads are resolved. Please re-request review when you are done.

tests/integration/test_auth.py Show resolved Hide resolved
tests/integration/test_server_upgrade.py Outdated Show resolved Hide resolved
tox.ini Show resolved Hide resolved
@kelkawi-a kelkawi-a requested a review from mertalpt September 16, 2024 19:57
Copy link

@AmberCharitos AmberCharitos left a comment

Choose a reason for hiding this comment

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

Code looks good, but please could you remind me why the tls-enabled is a config rather than being something deduced from the relation with postgresql and then passed to the admin charm?

.github/workflows/publish_charm.yaml Outdated Show resolved Hide resolved
config.yaml Show resolved Hide resolved
tests/unit/test_charm.py Show resolved Hide resolved
Copy link

@AmberCharitos AmberCharitos left a comment

Choose a reason for hiding this comment

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

LGTM - nice work

@kelkawi-a kelkawi-a merged commit 478a1d3 into main Sep 18, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants