Mirror ConfigMaps and Secrets referenced in ScyllaDBCluster into remote datacenters

[zimnx]

The ScyllaDBCluster controller has been extended with a new capability of mirroring the ConfigMaps and Secrets referenced by the ScyllaDBCluster, into remote datacenters.
This improvement provides users with the convenience of managing the configuration of an entire cluster from a single centralized location.

Note: any modifications made to the ConfigMaps or Secrets do not automatically initiate a rollout to apply those changes.
A manual trigger is required to implement the updates - for example using forceRedeploymentReason.

Requires:

• Refactor RemoteKubernetesCluster setup in E2E tests #2523

Fixes #2255

/cc

[scylla-operator-bot]

@zimnx: GitHub didn't allow me to request PR reviews from the following users: zimnx.

Note that only scylladb members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

The ScyllaDBCluster controller has been extended with a new capability of mirroring the ConfigMaps and Secrets referenced by the ScyllaDBCluster, into remote datacenters.
This improvement provides users with the convenience of managing the configuration of an entire cluster from a single centralized location.

Note: any modifications made to the ConfigMaps or Secrets do not automatically initiate a rollout to apply those changes.
A manual trigger is required to implement the updates - for example using forceRedeploymentReason.

Requires:

• Refactor RemoteKubernetesCluster setup in E2E tests #2523

Fixes #2255

/cc

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[zimnx]

ready to review
/cc rzetelskik
/cc mflendrich

[rzetelskik]

A few comments, rest lgtm

[rzetelskik]

lgtm
/assign mflendrich

[mflendrich]

I think that we're granting too wide RBAC access and should design for the necessary minimum, not for catch-all.

[zimnx]

clean rebase to pick up e2e timeout fix

[mflendrich]

We decided offline that, in order to allow the remote SA access only to a predetermined (by the user) list of namespaces (as opposed to access to CMs/secrets across the entire cluster), we can introduce a possibility for the user to specify the set of namespaces that the remote operator will have access to.

But over the course of this review I came to a realization that this is not enough, and is making another problem more prominent -- the possibility to hijack other cms/secrets - as described in one of the comments attached.

Also attached two less important maintainability comments, but the remote resource overwrite issue seems critical to me.

[mflendrich]

As discussed offline (at length), this implementation comes with an unconditional requirement that remote kubernetes cluster access must have very broad permissions effectively permitting reading of secrets, including SA tokens, across all namespaces, which paves a theoretically exploitable way towards privilege escalation (by reading 3rd party access tokens) or other attacks (by overwriting 3rd party configuration stored in secrets/CMs). While mechanisms in the operator exist that will likely (in the typical case) make it very hard to overwrite or leak 3rd party secrets, direct access to the remote kubernetes cluster kubeconfig opens the floodgates.

My first and foremost recommendation is to adjust the design so that

• remote operator access is restricted to a user-defined set of namespaces from day 1
• RBAC-controlled access to all secrets/configmaps is not necessary (e.g. by using deterministically named secrets/CMs in the remote cluster)

but the approach predates my engagement with the project, so I'm not directly questioning it, assuming that these limitations are well-known.

To unblock possible future improvement here, over the course of review we've made adjustments give the user control over the remote namespace name (created by operator in the remote cluster), making it possible to pave a future way (see #2577) to incrementally implement the two traits expressed in the bullet points above.

Therefore I'm conditionally approving this PR with an expectation that it's decided whether those limitations (materialized by #2577) should be a prerequisite for release or not.

Also created #2576 for a code health issue (not directly introduced by this PR but somewhat exacerbated) to be addressed at a later time.

/lgtm
(with the above caveat)

/hold for acknowledgement

[scylla-operator-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mflendrich, rzetelskik, zimnx

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [mflendrich,rzetelskik,zimnx]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[mflendrich]

/lgtm
(it was not picked up from #2524 (review))

[zimnx]

ack
/hold cancel