/area topology
/milestone v1.1

NOTE: I'm assuming a ClusterClass and its referenced templates should be in a single namespace as of today; the Cluster only can be in a different namespace

clusterctl move currently moves an entire namespace. Because up until now Clusters and their ClusterClasses had to be in the same namespace we could just move all Clusters and ClusterClasses of the respective namespaces (but of course in the right order).

The feature that ClusterClasses can be in another namespace changes our assumptions, so we have to think about how clusterctl move should behave in those cases.

Would it be more user friendly to change class to a struct with name and namespace(defaulting to cluster namespace if unset? This would mirror what we're doing elsewhere at least and avoids adding new patterns which might be confusing - I think making it a namespaced name could be tough for new users to understand
.

I think you both mean the same thing: types.NamespacedName:

type NamespacedName struct {
	Namespace string
	Name      string
}

https://github.com/kubernetes/apimachinery/blob/master/pkg/types/namespacedname.go#L27-L30

I think you both mean the same thing: types.NamespacedName:

That looks right - I was thinking class: "namespace/classname" was on the table as an option.

/assign

class: "namespace/classname" this could make sense as it's not a breaking change

note: #5717 adds some namespacing to webhook validation for clusterClass that will have to be addressed as part of this work.

note: #5717 adds some namespacing to webhook validation for clusterClass that will have to be addressed as part of this work.

Thanks for the heads up @killianmuldoon! :)

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

/remove-lifecycle stale
/priority-important-soon
I think we should reconsider this given that ClusterClass adoption is growing...

I'm considering moving on with the suggested approach; adding some more considerations

• I would prefer to have consistency across CAP*, so I suggest adding allowed namespace to a ClusterClass, similar to what we have in CAPA
• If allowedNamespace is empty, the ClusterClass can be used only within the same namespace, otherwise only in the namespaces specified (TBD if the same namespace is always included)
• when modifying a CC, if there is a change to allowedNamespace, we should ensure that we are not "orphaning" Clusters actually using the CC
• when deleting a CC, we should make sure that it is not used in other namespaces
• whenever we are reading a CC for a given Cluster, we should make sure that the cluster is in one of the allowed namespaces

Note, in the first iteration, I think we should not consider changing clusterctl move to move CC outsides the namespace being moved, this requires a dedicated deep dive

Sounds good, one comment:

• we might want to not add NamespaceList if we're fine to say that a label selector against the kubernetes.io/metadata.name label is only safe / works (?) with Kubernetes >= 1.21
  • grep for When Cluster API no longer supports Kubernetes versions older than Kubernetes v1.21 in https://github.com/kubernetes-sigs/cluster-api/pull/4514/files

+1

Another use case for this: in provider e2e tests, we provide one namespace per test cluster but want to reuse the same clusterclass and install it in BeforeSuite https://github.com/kubernetes-sigs/cluster-api-provider-azure/pull/2235/files#r865122163

I would prefer to have consistency across CAP*, so I suggest adding allowed namespace to a ClusterClass, similar to what we have in CAPA
If allowedNamespace is empty, the ClusterClass can be used only within the same namespace, otherwise only in the namespaces specified (TBD if the same namespace is always included)

Not sure what CAPA does but in CAPZ if allowedNamespaces is empty it allows all namespaces (as opposed to only the current one) https://capz.sigs.k8s.io/topics/multitenancy.html#allowednamespaces

Would be beneficial for testing to be able to specify all namespaces since cluster name/namespace is randomly generated for each spec so it won't be known ahead of time in BeforeSuite

Another use case for this: in provider e2e tests, we provide one namespace per test cluster but want to reuse the same clusterclass and install it in BeforeSuite https://github.com/kubernetes-sigs/cluster-api-provider-azure/pull/2235/files#r865122163

I'm not sure if that is an issue. clusterctl generate automatically includes the ClusterClass when generating the Cluster (here).

In core CAPI today we use the same ClusterClass in multiple tests and it gets automatically applied in different namespaces because clusterctl generate includes it automatically. So if the requirement is only to reuse the same ClusterClass across multiple tests you don't have to explicitly install it via BeforeSuite. (we didn't implement any explicit ClusterClass deployment at all in core CAPI e2e tests)

EDIT: While it's possible to reuse the same ClusterClass in e2e tests by deploying it multiple times (and it's automatically included in the output of clusterctl generate cluster) it of course might be desirable to intentionally re-use one single deployed ClusterClass in multiple tests in different namespaces.

Not sure what CAPA does but in CAPZ if allowedNamespaces is empty it allows all namespaces (as opposed to only the current one) https://capz.sigs.k8s.io/topics/multitenancy.html#allowednamespaces

I'm not opposed at this behavior as well, however, I'm worried if this is "unsecure", given that if the user forgets to specify allowedNamespaces in a ClasterClass it is accessible from everywhere (instead of defaulting to the more restrictive behavior).

/unassign

Not work on this ATM

/assign

Would like to take a look at this. Perhaps we can add a topic to this week's meeting agenda to define an action plan on how this feature could be implemented.

Should this issue be separated into two? It seems to me that for a general scenario, just adding a namespace/name will be enough to reference a cluster class (although a classNamespace field is preferable IMO). And the policy access considerations in the multi-tenant scenario can have a separate issue. WDYT?

It seems to me that for a general scenario, just adding a #5673 (comment) will be enough to reference a cluster class (although a classNamespace field is preferable IMO).

I also think that the first solution could be the namespace/name reference for a ClusterClass. This would be enough for me.

Service Gateway API moved their referential approach into KEP, and it was merged - https://github.com/kubernetes/enhancements/tree/master/keps/sig-auth/3766-referencegrant

They propose using ReferenceGrant resource as a policy for both side cross-namespace references. This allows to use any sort of namespace/name or

ref:
  name: <name>
  namespace: <namespace>

openly, with a future expectation that a reference grant resource can be checked before processing the reference and executing the logic behind it.

I think this is a clean approach, allowing to rely on expectation that in the future the regular namespace/name or ref can be secured, and does not require changes in the ClusterClass CRD to manage this policy (can be used as-is).
Until the ReferenceGrant support is fully added, a cross-namespace ClusterClass reference inside the cluster can trigger a warning in the webhook output, if the specified namespace is different from the one in the cluster.

What do you think, will this help moving this issue to a solution?

This issue is labeled with priority/important-soon but has not been updated in over 90 days, and should be re-triaged.
Important-soon issues must be staffed and worked on either currently, or very soon, ideally in time for the next release.

You can:

• Confirm that this issue is still relevant with /triage accepted (org members only)
• Deprioritize it with /priority important-longterm or /priority backlog
• Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

/priority important-longterm

/triage accepted
Still something to figure out about the API, but overall there is consensus

This issue is labeled with priority/important-soon but has not been updated in over 90 days, and should be re-triaged.
Important-soon issues must be staffed and worked on either currently, or very soon, ideally in time for the next release.

You can:

• Confirm that this issue is still relevant with /triage accepted (org members only)
• Deprioritize it with /priority important-longterm or /priority backlog
• Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

/triage accepted