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

Use ReplicatedSecrets to control Medusa bucket secrets replication into K8ssandraCluster namespaces #1238

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Miles-Garnsey
Copy link
Member

@Miles-Garnsey Miles-Garnsey commented Mar 12, 2024

What this PR does:

Switch to using ReplicatedSecrets to control replication of the medusa bucket secrets.

Which issue(s) this PR fixes:
Fixes #1217
#1260

Checklist

  • Changes manually tested
  • Automated Tests added/updated
  • Documentation added/updated
  • CHANGELOG.md updated (not required for documentation PRs)
  • CLA Signed: DataStax CLA

@Miles-Garnsey Miles-Garnsey requested a review from a team as a code owner March 12, 2024 05:53
@Miles-Garnsey
Copy link
Member Author

I've marked out the places where I think these changes should take place, and commented with the specifics of the changes I intend to make.

There is a slight issue to work around here as I've currently drafted the design. It relates to the fact that (in my current round of comments) I talk about creating a single ReplicatedSecret for a single MedusaConfig, but this probably won't work for the following reason;

  • When a cluster is created, it would presumably go and create a replicatedSecret if one didn't already exist in the MedusaConfig namespace.
  • But if a replicatedSecret did exist, then it would go and add an entry to the ReplicationTargets in the ReplicatedSecret.
  • The problem arises on cluster deletion, where the finalizer might go and either remove the replicationTarget entry or remove the secret (if no more targets exist).
  • However, in the case that multiple K8ssandraClusters existed in the same context-namespace, this might cause the deletion of one cluster to delete replicated secrets relied on by another.

As a result, I think we're going to have to

  1. Create multiple independant ReplicatedSecrets per cluster in the MedusaConfig's namespace.
  2. Somehow ensure that (if two clusters reference the same MedusaConfig and are in the same namespace) the secrets are renamed while being replicated so that we don't end up with a conflict in the K8ssandraCluster's namespace instead.

@adejanovski let me know if you agree with this approach, and with the locations in the code where I've proposed changes. I haven't worked on the Medusa Controller extensively, so your input is appreciated.

Copy link

codecov bot commented Mar 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 57.39%. Comparing base (9039c02) to head (3a925ee).

❗ Current head 3a925ee differs from pull request most recent head fa2a73b. Consider uploading reports for the commit fa2a73b to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1238      +/-   ##
==========================================
- Coverage   57.42%   57.39%   -0.03%     
==========================================
  Files         103      103              
  Lines       10801    10791      -10     
==========================================
- Hits         6202     6194       -8     
+ Misses       4062     4061       -1     
+ Partials      537      536       -1     
Files Coverage Δ
controllers/k8ssandra/cleanup.go 53.43% <ø> (ø)
controllers/k8ssandra/medusa_reconciler.go 66.20% <100.00%> (+0.15%) ⬆️

... and 7 files with indirect coverage changes

…conflict instances where the same source may target the same target multiple times due to presence of several clusters in the target namespace.

Envtest for this functionality.
@Miles-Garnsey Miles-Garnsey marked this pull request as draft March 21, 2024 05:42
Copy link

sonarcloud bot commented Mar 21, 2024

Quality Gate Passed Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@Miles-Garnsey
Copy link
Member Author

@burmanm thinks there might be a way to create replicas of the Medusa StorageSecretRef without needing to add a new name-namespace type selector to identify the secret.

We are going to catch up about this next week to see if we can progress this work.

@Miles-Garnsey
Copy link
Member Author

Closing as I no longer think this is required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updating a medusaConfiguration referenced secret should propagate
1 participant