Add a unique label to the Medusa bucket secret

[Miles-Garnsey]

What is missing?

The Medusa bucket secret (whether user supplied or operator-generated) should be labelled with a unique label so that it can be picked up by the ReplicatedSecrets controller for replication into individual K8ssandraCluster namespaces.

Why do we need it?

Currently, a ReplicatedSecret takes a LabelSelector, which is fine when we are marking it out as belonging to a particular cluster/DC.

However, the Medusa bucket secret is a 1:many replication into many potential clusters/DCs, not into a single cluster, namespace or DC.

As such, the design calls for the creation of a new label which will be unique per secret, so that the existing label selectors can be used to pick up the single secret for replication into the K8ssandraCluster's namespace.

Note: This ticket has been edited from an original which called for the creation of a name-based selector. This design should not be widely used as we have not evaluated the impacts of the increased label value cardinality on etcd's indexing mechanisms.

Environment

• K8ssandra Operator version:

  Insert image tag or Git SHA here

Anything else we need to know?:

[Miles-Garnsey]

Looking at this, it actually appears that all of the ListOptions stuff is string based, so maybe that isn't the best choice. We're gonna have to go with just some NamespacedName type struct I think.

[Miles-Garnsey]

This is actually a fair bit more work than I'd realised.

1. The current structure of the selectors is quite complex and actually caches them into a map which is guarded by a mutex.
2. That map/cache is then referenced by the SetupWithManager to filter for only those secrets that are referenced by a ReplicatedSecret.
3. It seems that in many parts of the codebase, we're going to need to bifurcate a lot of logic to allow for referencing source secrets by a LocalObjectReference. This will include in finalization/cleanup logic, SetupWithManager, and the actual main reconciliation itself.
4. This is complicated by the fact that when both any prospective *LocalObjectReference and * metav1.LabelSelector are both null, we'll need to ensure that all secrets in the source namespace are registered for replication.

I'll spend the rest of the day on this, if I can't get close to finishing I might drop it, because there's a lot going on here and I guess we could always revert back to simply doing a direct copy, as we do now, it just isn't elegant.

[burmanm]

This is particularly relevant to the Medusa bucket secret that we're trying to fix over #1238, because in that case the secret needs to be selected using only name and namespace.

Why? We still read it as part of the Medusa reconciliation process, so we can add correct annotations to it. Even if user changes it.

[Miles-Garnsey]

Why? We still read it as part of the Medusa reconciliation process, so we can add correct annotations to it. Even if user changes it.

For three reasons:

1. It is ugly to mutate an input to a controller, especially where the input is user derived and subject to change at any time. It might work, but it is ideal to avoid it based on issues we've seen in the past.
2. Even if we mutated the input, we would still have an issue where it is unclear what cluster-name/cluster-namespace labels to add, which is what we'd usually use to control replication into a namespace. Since a single secret can require replication to many clusters in many namespaces, we can't use such an annotation.
3. If we do not use cluster-name/cluster-namespace, It is then also unclear what label (if any) could be used to uniquely identify the bucket secret we want to replicate.

Open to solutions on this (and I can see some workarounds, but I'm not sure they're good), can you explain what exactly you're proposing as an alternative @burmanm?

[Miles-Garnsey]

@burmanm is fine with approving the prefix PR.

In terms of this ticket, we think:

1. It might be best to use a unique label instead of a namespace. This isn't idiomatic and might be bad for the etcd indices, but we think it is OK given the limited number of secrets we expect to have to deal with.
2. When a secret is referenced in a MedusaConfig we will add a label to the secret, which will have a unique ID as value. That unique ID can be used with the existing ReplicatedSecret label selectors.
3. When it comes to cleaning up (refer here) we need to add an additional area to think about which is how we clean up a ReplicatedSecret in the case that the K8ssandraCluster which created it changes its MedusaConfig reference. In this case we have an unresolved problem where we don't know the old namespace, which is a show stopper. We think we can search all namespaces using a list method, and filter based on labels. We need to complete some research here.
4. *We will also need finalizer on the K8ssandraCluster for when the K8ssandraCluster is entirely deleted, ensuring that finalization logic exists to delete the ReplicatedSecret in the MedusaConfig namespace.

^ Miles to message Alex to see if we can remove the ability to reference MedusaConfigs from non-local namespaces entirely, because this is a pain in the bum.

[adejanovski]

Miles to message Alex to see if we can remove the ability to reference MedusaConfigs from non-local namespaces entirely, because this is a pain in the bum.

That would be quite a restriction as it's the cornerstone of MedusaConfigurations to be reference-able from multiple namespaces.
If using ReplicatedSecrets for the initial copy of the secret is forcing that kind of restriction on us, then we should stick with what we have now.

[adejanovski]

This is marked as ready for review but I don't see any linked PR. Do we have a PR for this @Miles-Garnsey ?

[adejanovski]

ok I see, you've used "Solves" instead of "Fixes" as magic word in the PR description.

[Miles-Garnsey]

ok I see, you've used "Solves" instead of "Fixes" as magic word in the PR description.

Oh? I didn't realise that we needed "Fixes", I think a PR still shows up in an issue if it is linked in any way..? I guess there is more automation here than I'd realised and our custom stuff must be more restrictive in what it is looking for.

[adejanovski]

I think a PR still shows up in an issue if it is linked in any way..? I guess there is more automation here than I'd realised and our custom stuff must be more restrictive in what it

It won't. Here's the doc for this GitHub feature.
It's unrelated to our automation btw, this is purely GitHub.

[Miles-Garnsey]

TIL...