Adapt clusterctl move to the new multi-tenancy model

[fabriziopandini]

User Story

As a User, I would like to use clusterctl for creating multy-tenant clusters.

Detailed Description

kubernetes-sigs/cluster-api-provider-aws#1713 is introducing the possibility for a provider to use many credentials with a single instance of a provider.

We should define if/how this scenario is supported by clusterctl.

Anything else you would like to add:

clusterctl already support two other different types of multy-tenancy, see https://cluster-api.sigs.k8s.io/clusterctl/commands/init.html#multi-tenancy

The approach introduced by CAPA is potentially by far simpler than the existing ones, and if we can potentially have all the providers to converge on the same approach this can result in a relevant simplification of manifests generation (e.g. no more need of the WebHook namespace) and of clusterctl (lots of corner cases won't be necessary anymore)

/kind feature

[fabriziopandini]

/area clusterctl
even do this problem is not clusterctl specific

[vincepri]

@randomvariable is working on this in the providers for v1alpha3, although we should revisit in v1alpha4 and forward. It's definitely something we might want to tackle before getting to beta.

/milestone Next

[randomvariable]

Thanks for this issue Fabrizio.

/priority important/long-term

[k8s-ci-robot]

@randomvariable: The label(s) priority/important/long-term cannot be applied, because the repository doesn't have them

In response to this:

Thanks for this issue Fabrizio.

/priority important/long-term

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/test-infra repository.

[randomvariable]

/priority important-longterm

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fabriziopandini]

/remove-lifecycle stale

[vincepri]

/lifecycle frozen

[vincepri]

We need to add definitions for what multi-tenancy is to the glossary

[randomvariable]

And a provider contract. We should definitely put this in 0.4.0

/assign

[vincepri]

/milestone v0.4.0

[vincepri]

Renamed this, hopefully it's going to be a bit more clear going forward :)

[fabriziopandini]

Given that we are moving to a single manager watching all namespaces for each provider, I started to investigate possible cleanups/action items:

• Run webhooks as part of the main manager Run webhooks as part of the main manager #3822
• Docs: document new definition of multi-tenancy (a single infrastructure provider supporting multiple credentials) 📖 Document multi-tenancy contract #4074
• Docs: remove existing notes about previous models of multitenancy 📖 Document multi-tenancy contract #4074

... March 11th edit
Moved missing actions items to #4056 (comment)

[fabriziopandini]

How would clusterctl support multiple credentials?

That the point I'm trying to understand.

For instance, last I checked the AWS proposal it was introducing different types of cluster principals.
Does clusterctl have to support all the AWS cluster principals types during init? how?
Also, are the other providers implementing a similar approach than AWS, so we can design a single UX?

[fabriziopandini]

Linking #4035 because it touches changing credentials during pivoting

[randomvariable]

Does clusterctl have to support all the AWS cluster principals types during init? how?

For init, other than installing the CRDs, no.
For move, ideally clusterctl should move AWS*Principal resources, including the global ones.

However in the most common case, we only expect the management cluster to be moved, and in that case, will use a singleton AWSClusterControllerPrincipal which is a no-op type of principal that just states the cluster is using the credentials present at controller startup, which is the same secret mounted to ~/.aws/credentials as it is currently in v1alpha3

[nader-ziada]

For capz,
no changes needed for init
no changes need for move; the AzureClusterIdentity type gets moved since its a capz resource and we decided at the time the referenced identity itself should be the responsibility of the user to move/create in the new namespace since its out of the scope of the controller

[CecileRobertMichon]

/area release-blocking

[k8s-ci-robot]

@CecileRobertMichon: The label(s) area/release-blocking cannot be applied, because the repository doesn't have them.

In response to this:

/area release-blocking

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/test-infra repository.

[CecileRobertMichon]

/kind release-blocking

[yastij]

for now on the CAPV side, we're having two types of credentials:

• a secret for namespaced credentials
• later down the road we'd also have aVSphereAccount CRD which would be cluster-scoped for accounts that are available for multiple vcenters.

[sbueringer]

In case it's relevant in any way, CAPO is using a secret, which is usually located in the same namespace as the Cluster resource, but is referenced via a SecretRef which would also allow using a secret in another namespace.

[fabriziopandini]

Trying to distill a generalised approach and possible impacts on clusterctl and in the CAPI provider operator as well:

For the init workflow:

• all the providers/identity management systems do requires to install new CRD types; this could be managed by the existing machinery in clusterctl because identity types will be packaged together with the existing CRDs;
• most of the providers plans to have credentials resources defined in their infrastucture-components.yaml and to use variables substitution for allowing users to specify an initial set of credentials (as of today); CAPZ instead assumes the user has to create credentials before doing clusterctl init (might be there is a chicken egg problem here, given that AzureClusterIdentity CRD is not yet installed in the cluster at this stage...)

For the move workflow:

• we should ensure clusterctl could manage variants variants: Global Identity vs namespace identity, identity in secrets vs identity in CRD
• As of today following constraints stands:
  • Move will consider all the kinds defined in the CRDs installed by clusterctl + Secrets and ConfigMap, including both namespaced and global kinds;
  • If those kind are namespaced, only resources in a given namespace will be considered;
  • The resulting set of resources will be further filtered excluding everything:
    1. Is not linked to a cluster via the owner ref chain
    2. Is not explicitly marked as force move (at CRD or at resource level)

Are 1 or 2 enough to address the requirement for all the providers/identity management systems?

[fabriziopandini]

Thanks to the work in #4514, we can finally nail down required changes in clusterctl move.

• 🌱 clusterctl move should consider Secrets from provider's namespace #4598 during the discovery, on top of what is considered today, clustectl has to collect secrets from the namespaces where the infrastructure providers are running
• 🌱Adapt clusterctl move to requirements for the new multi-tenancy model #4628 the filtering step process at the end of the discovery process should be updated in order to manage additional owner chain roots (not only the owner chain below Cluster and CRS); additional owner chain root to be considered for move will be identified via a new label TBD applied at CRD level or at resource level (IMPORTANT! providers have to apply the label on their CRDs)
• 🌱Adapt clusterctl move to requirements for the new multi-tenancy model #4628 the move/delete order should consider the multiple owner chain (this should be already the case, but there are no unit test in place for multiple namespaces)
• 🌱Adapt clusterctl move to requirements for the new multi-tenancy model #4628 Cluster level objects and their hierachy should not be deleted from the source cluster.
• 🌱Adapt clusterctl move to requirements for the new multi-tenancy model #4628 Cluster level objects and their hierachy should not be updated in the target cluster if already existing.
• there should be check in place about existence of all the target namespaces (this is already the case, test in places)

@randomvariable @nader-ziada @yastij @gab-satchi @sedefsavas @vincepri

[fabriziopandini]

/unassign @randomvariable

/assign
/lifecycle active

[sedefsavas]

@fabriziopandini Will it be straightforward to backport this to v0.3.x release? Asking because CAPA is supporting multi-tenancy in v1alpha3 releases.

[fabriziopandini]

@fabriziopandini Will it be straightforward to backport this to v0.3.x release? Asking because CAPA is supporting multi-tenancy in v1alpha3 releases.

I don't think we are going to backport this; changes are sort of invasive and not all the guidelines for the providers (#4514) are not yet merged

[fabriziopandini]

We should wait for #4628 as well