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

ReplicatedSecrets can have a prefix per target #1243

Merged

Conversation

Miles-Garnsey
Copy link
Member

What this PR does:
Allow a ReplicatedSecret replication target to define a prefix to de-conflict instances where the same source may target the same target multiple times due to presence of several clusters in the target namespace.

Which issue(s) this PR fixes:
Fixes #1242

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 13, 2024 00:33
@Miles-Garnsey Miles-Garnsey force-pushed the feature/replicatedsecret-prefix branch from a7c0ebf to 5b5484a Compare March 13, 2024 01:12
@Miles-Garnsey
Copy link
Member Author

This ticket needs one additional thing. At present the final env test that I've added is failing, as the target secret (which has a prefix in this test) is not being properly deleted after it's source secret is deleted. I don't know quite where this logic is supposed to happen, so I've reached out to @burmanm for some help.

Everything else looks ok however.

@burmanm
Copy link
Contributor

burmanm commented Mar 13, 2024

I don't know quite where this logic is supposed to happen

https://github.com/k8ssandra/k8ssandra-operator/blob/main/controllers/replication/secret_controller.go#L70

@Miles-Garnsey
Copy link
Member Author

@burmanm the link you provided to the cleanup logic only seems to deal with the ReplicatedSecret resource:

I don't know quite where this logic is supposed to happen

https://github.com/k8ssandra/k8ssandra-operator/blob/main/controllers/replication/secret_controller.go#L70

I'm trying to figure out where the cleanup is that is triggered when the source secret is deleted.

@Miles-Garnsey
Copy link
Member Author

I've discussed this with @burmanm and it appears that if a source secret is deleted without the replicatedSecret that references it being deleted, then actually the downstream secret that is produced by the ReplicatedSecret is not deleted.

At the moment we do have orphaned resources in a sense, so I will work with Micke to create a new ticket to address that. But this PR is good to go.

Copy link

codecov bot commented Mar 18, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 57.30%. Comparing base (94bc1d8) to head (0ac1d65).
Report is 3 commits behind head on main.

❗ Current head 0ac1d65 differs from pull request most recent head 54c2b51. Consider uploading reports for the commit 54c2b51 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1243      +/-   ##
==========================================
- Coverage   57.34%   57.30%   -0.05%     
==========================================
  Files         103      103              
  Lines       10790    10795       +5     
==========================================
- Hits         6188     6186       -2     
- Misses       4064     4069       +5     
- Partials      538      540       +2     
Files Coverage Δ
controllers/replication/secret_controller.go 65.01% <85.71%> (+0.58%) ⬆️

... and 4 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 force-pushed the feature/replicatedsecret-prefix branch from 0ac1d65 to 54c2b51 Compare March 21, 2024 00:05
@Miles-Garnsey Miles-Garnsey merged commit c9156ef into k8ssandra:main Mar 26, 2024
9 checks passed
Copy link

sonarcloud bot commented Mar 26, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
4.1% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

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.

Allow ReplicatedSecrets's targets to have a prefix
2 participants