EVEREST-496 Multi namespace support

[gen1us2k]

CHANGE DESCRIPTION

Problem:
EVEREST-496

That's a small PoC for cluster-wide operators that stores backup storage (the same applies for monitoring configs) in the percona-everest and replicates secrets across namespaces that it manages.

Pros:

1. BE needs to manage only one BackupStorage or MonitoringConfig with the referenced secret in only one namespace
2. Operator handles the state of the secret and updates it across all namespaces where it was created

Cons:

1. Increased blast radius. When (not if, but when) we will release a broken version of the operator DBaaS features will be broken for all database clusters across all namespaces. Having it in the namespace mode locks it only for the one namespace and this approach is recommended by Percona. Read more https://www.percona.com/blog/percona-operators-deployment-explained-delving-into-cluster-wide-vs-namespace-scoped/
2. Bloated service account permissions to run everest operator in the cluster-wide mode. Instead of having one SA per namespace with the least privileges. If percona-everest namespace is compromised, the attacker can get access to all namespaces that manage everest-operator
3. Additional load to the QA team because they need to test two different modes of everest operator and this is an additional step for their workflow
4. The configuration becomes more fragile because CLI needs to set env var for percona-everest namespace for two components: BE and operator
5. During the update of a secret and when the operator fails to reconcile it in any namespace the end user will stay unnotified without any messaging from the everest side
6. There is no separation of concerns for the sensitive data because everything is stored in the one namespace yet I believe we need to store it in different namespaces to follow security best practices

Yet we have an additional request for the cluster-wide operator to use admission webhooks to move validation logic from the BE side to the operator, however, the scope of change is huge because

1. Would be best to implement multi-namespace mode for the operator without additional logic, ensure that everything works as expected
2. Ensure that everest operator can work with the different versions of the upstream operators
3. Implement reconcile loops for monitoring configs, and backup storages
4. Implement admission webhoooks or any other stuff

CHECKLIST

Jira

• Is the Jira ticket created and referenced properly?

Tests

• Is an Integration test/test case added for the new feature/change?
• Are unit tests added where appropriate?

[oksana-grishchenko]

In general the idea with targetNamespaces for the allowed list and status.namespaces for the actual list looks good, however to keep the lists consistent we would need to handle the deletion of

• the targetNamespaces
• the actual namespaces in the cluster

[oksana-grishchenko]

The implementation itself looks good, my concerns are resolved. Approving the PR to unblock it in case the massive "Cons" section is considered by the product as the limitations we can live with.

[recharte]

Overall the PR looks like it is functionally correct however, I think the followed approach is overly complex.

I argue that we don't need the BackupStorage controller at all. We already have an index and watcher configured in the DBC controller that runs the reconciliation loop every time a secret referenced by a BS triggers an event.
Therefore, we can use this same mechanism to create or update the end secret without needing to keep track of any namespace. In a sense we can think of this secret as being linked to a specific DBC than to the generic BS.

Moreover, we already use this approach for PG because the secret format needs to be different than the one specified in the BS which is the same for PXC/PSMDB. See here and here

[gen1us2k]

I argue that we don't need the BackupStorage controller at all. We already have an index and watcher configured in the DBC controller that runs the reconciliation loop every time a secret referenced by a BS triggers an event.
Therefore, we can use this same mechanism to create or update the end secret without needing to keep track of any namespace. In a sense we can think of this secret as being linked to a specific DBC than to the generic BS.

Moreover, we already use this approach for PG because the secret format needs to be different than the one specified in the BS which is the same for PXC/PSMDB. See here and here

Well, this was my first approach to solving the issue. This works fine for the backupStorages or monitoringConfigs in the single namespace scenario.

Since the idea is to create backupstorages or monitoring configs in the default namespace (percona-everest) the existence of the objects is only in the default namespace. However, databasecluster controller replicates secrets across namespaces (since they are namespaced) there's no way (at least I know) to bind that secret with the backupStorage or monitoringConfig. Since the logic for backup storage and monitoring config is the same I'll describe the flow based on backup storage

Let's take a look at the backup storage In the default namespace. To ensure that the secret will be deleted we can use ownerreferences here and we don't quite need a controller here. The backend or some other controller can create it for you yet, once the backupstorage is deleted, the secret will be deleted as well.

Since secrets are namespaced to enable the multi namespace support in the cluster-wide mode we need to replicate them to the targetNamespace during the reconciliation of database clusters(because this is the only place when we need to ensure that the secret exists). We can't use ownerreferences here because they are not designed for that and we need to come up with a way to store the information where the secret was created. Here are the labels to link the secret in the different namespace with the existing backup storage.

After testing this using indexers and watchers, when the user tries to delete the secret from a different namespace the reconciliation loop does not start because there's no event generated/passed to the operator after the deletion and it ends in the inconsistent state.

Hence, we need to have an additional controller to ensure that after backup storage deletion, all secrets across all namespaces will be created and here we have finalizers as an additional handle to block the deletion. We need to delete all secrets across all namespaces except the default one (because we use owner references here and the deletion might block here because of cross-linking between entities)

Since the backupstorage controller is namespaced (because we create backup storages only in the default namespace) we need to have a way to watch for secrets and run reconciliation of the backup storage entity on the secret change. For the default secret in the default namespace we have Owns for that and this works fine. For all other namespaces, we need to listen to secrets and run a reconcile loop only for labeled secrets.

I hope that it explains the solution after a couple of days of trying to implement as simply as I can.

[recharte]

This is such an important change that we can't merge it without proper tests for it.

[recharte]

While reviewing I found a couple of tiny code style improvements, these are just some nitpicks so feel free to ignore them.

[recharte]

Thank you for adding the tests 🤗.
I think that the only important case we're missing would be to validate the TargetNamespaces feature works as intended but that's part of the error cases which we aren't testing anywhere so I can't say this is mandatory for now.