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

[DPE-2633] Update data platform libs to use juju secrets for charm external secrets #259

Merged
merged 19 commits into from
Oct 17, 2023

Conversation

dmitry-ratushnyy
Copy link
Contributor

@dmitry-ratushnyy dmitry-ratushnyy commented Sep 27, 2023

Issue

The charm must support external secrets to keep sensitive relation data

Solution

To support external secrets we need to import a new version of data platform libs and fix any issues it can cause

@dmitry-ratushnyy dmitry-ratushnyy changed the title [DPE-2633] Update data platform libs to use juju secrets for charm external secrets [WIP] [DPE-2633] Update data platform libs to use juju secrets for charm external secrets Sep 27, 2023
@dmitry-ratushnyy dmitry-ratushnyy changed the base branch from 6/edge to 5/edge September 27, 2023 12:22
@dmitry-ratushnyy dmitry-ratushnyy changed the title [WIP] [DPE-2633] Update data platform libs to use juju secrets for charm external secrets [DPE-2633] Update data platform libs to use juju secrets for charm external secrets Sep 27, 2023
@dmitry-ratushnyy dmitry-ratushnyy changed the base branch from 5/edge to 6/edge September 27, 2023 14:44
@dmitry-ratushnyy dmitry-ratushnyy force-pushed the updated_data_platform_libs branch from 1421721 to 9322887 Compare October 5, 2023 16:59
@dmitry-ratushnyy dmitry-ratushnyy force-pushed the updated_data_platform_libs branch 3 times, most recently from 296dd3a to d24c7f4 Compare October 12, 2023 08:16
@dmitry-ratushnyy dmitry-ratushnyy requested review from MiaAltieri and delgod and removed request for MiaAltieri October 13, 2023 09:39
Copy link
Contributor

@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.

LGTM - a question though, do we need to also future PRs for TLS and S3 relations? I think those pass some secrets as well

.github/workflows/ci.yaml Show resolved Hide resolved
lib/charms/mongodb/v0/mongodb_provider.py Outdated Show resolved Hide resolved
@dmitry-ratushnyy dmitry-ratushnyy force-pushed the updated_data_platform_libs branch from 18a9008 to 932c7f6 Compare October 13, 2023 10:09
delgod
delgod previously approved these changes Oct 13, 2023
@dmitry-ratushnyy
Copy link
Contributor Author

LGTM - a question though, do we need to also future PRs for TLS and S3 relations? I think those pass some secrets as well

No, I don't think so. The only changes I made in the mongodb_provides were because how we were getting passwords. The rest should work "out of box"

MiaAltieri
MiaAltieri previously approved these changes Oct 13, 2023
Copy link
Contributor

@juditnovak juditnovak left a comment

Choose a reason for hiding this comment

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

Looks good to me in the 1st place -- however I'd approve once there's a green pipeline for tests that may use any relations.

(I came across too many tests "hiding" using databag, I only believe they're all gone once the pielines come up green :-) )

dmitry-ratushnyy and others added 8 commits October 17, 2023 07:17
juju secrets for charm external secrets
This is a temporary change and will be removed once we will switch to different linter
…ard (#246)

This PR enables the user to pass the following config options:
`replication`(default), `shard`, and `config-server`. This PR supports
this options by starting the charm as a `shard` or a `config-server`. If
the config option `config-server` is provided, then the charm starts an
internal `mongos` service that runs on 0.0.0.0 and is configured to the
provided config server.

As a POC there are not tests included in this PR. Testing was performed
by hand with:
```
charmcraft pack
juju deploy ./*charm --config role="shard" shard-one
juju deploy ./*charm --config role="config-server" config-server

watch -n1 --color juju status --color

juju ssh shard-one/0
systemctl status snap.charmed-mongodb.mongod.service
exit
juju ssh config-server/0
systemctl status snap.charmed-mongodb.mongod.service
systemctl status snap.charmed-mongodb.mongos.service
exit
```

New snap revison packages a new version of the PBM tool which updated
how errors were handled when querying PBM status, commit
[4bf9d5f](4bf9d5f)
reflects these necessary changes

Follow up PR is to be made immediately after merging of this feature is
completed and is a requirement of finishing
[DPE-2561](https://warthogs.atlassian.net/browse/DPE-2561). For this PR
this will be starting `mongos` with a `--auth` and the same `--keyFile`
used to start the`mongod` service

Future PR is to be started after the follow up PR has been made. Once
`mongos` is started with auth
[DPE-2562](https://warthogs.atlassian.net/browse/DPE-2562) will be
started. This includes creating a basic shared library between
`config-server` and `shard` components. In this PR we will:
- implement keyfile sharing across shard and config server components
- implement adding shards to cluster

(Shard removal is saved for later.)

As a POC this PR doesn't include handling edge cases, intelligent status
reporting, unit tests, or integration tests. These will be handled later
on down the line. Specifically:
1. block config change events for changing role of charm
2. report status of internal `mongos` in `update_status`
3. unit tests and integration tests

[DPE-2561]:
https://warthogs.atlassian.net/browse/DPE-2561?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

---------

Co-authored-by: Pedro Guimaraes <[email protected]>
@dmitry-ratushnyy dmitry-ratushnyy dismissed stale reviews from MiaAltieri and delgod via 64a3e57 October 17, 2023 07:20
@dmitry-ratushnyy dmitry-ratushnyy force-pushed the updated_data_platform_libs branch from 932c7f6 to 64a3e57 Compare October 17, 2023 07:20
Copy link
Contributor

@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.

I believe a followup PR will be needed to use external secrets within shards_interface.py :) but this work looks good, just left a question/comment

lib/charms/mongodb/v1/mongodb_provider.py Show resolved Hide resolved
Copy link
Contributor

@juditnovak juditnovak left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@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.

looks good to me, but please make an extra PR for sharding since sharding passes secrets via data bag

@MiaAltieri MiaAltieri merged commit 1e51eff into 6/edge Oct 17, 2023
13 checks passed
@MiaAltieri MiaAltieri deleted the updated_data_platform_libs branch October 17, 2023 11:06
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