Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ clusterctl move support for a cross namespace ClusterClass reference #11649

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
14 changes: 13 additions & 1 deletion api/v1beta1/cluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package v1beta1

import (
"cmp"
"fmt"
"net"
"strings"
Expand Down Expand Up @@ -517,6 +518,15 @@ type Topology struct {
// The name of the ClusterClass object to create the topology.
Class string `json:"class"`

// The namespace of the ClusterClass object to create the topology. Empty namespace assumes the namespace of the cluster object.
// Class namespace changes are not supported by the rebase procedure, as different CC namespace uses namespace-local templates.
// Cluster templates namespace modification is not allowed.
// +optional
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
// +kubebuilder:validation:Pattern="^[a-z0-9](?:[-a-z0-9]*[a-z0-9])?(?:\\.[a-z0-9](?:[-a-z0-9]*[a-z0-9])?)*$"
ClassNamespace string `json:"classNamespace,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have consensus about adding this field to the API?

i mean, it seems necessary for this change, but i don't recall what we might have agreed upon in the office hours.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s actually a superset of #11352, I believe we have a consensus on the actual PR in the review #11352 (comment) (correct me if I’m wrong)


// The Kubernetes version of the cluster.
Version string `json:"version"`

Expand Down Expand Up @@ -1045,7 +1055,9 @@ func (c *Cluster) GetClassKey() types.NamespacedName {
if c.Spec.Topology == nil {
return types.NamespacedName{}
}
return types.NamespacedName{Namespace: c.GetNamespace(), Name: c.Spec.Topology.Class}

namespace := cmp.Or(c.Spec.Topology.ClassNamespace, c.Namespace)
return types.NamespacedName{Namespace: namespace, Name: c.Spec.Topology.Class}
}

// GetConditions returns the set of conditions for this object.
Expand Down
26 changes: 26 additions & 0 deletions api/v1beta1/index/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import (
const (
// ClusterClassNameField is used by the Cluster controller to index Clusters by ClusterClass name.
ClusterClassNameField = "spec.topology.class"
// ClusterClassNamespaceField is used by the Cluster controller to index Clusters by ClusterClass namespace.
ClusterClassNamespaceField = "spec.topology.classNamespace"
)

// ByClusterClassName adds the cluster class name index to the
Expand All @@ -44,6 +46,18 @@ func ByClusterClassName(ctx context.Context, mgr ctrl.Manager) error {
return nil
}

// ByClusterClassNamespace adds the cluster class namespace index to the
// managers cache.
func ByClusterClassNamespace(ctx context.Context, mgr ctrl.Manager) error {
if err := mgr.GetCache().IndexField(ctx, &clusterv1.Cluster{},
ClusterClassNamespaceField,
ClusterByClusterClassNamespace,
); err != nil {
return errors.Wrap(err, "error setting index field")
}
return nil
}

// ClusterByClusterClassClassName contains the logic to index Clusters by ClusterClass name.
func ClusterByClusterClassClassName(o client.Object) []string {
cluster, ok := o.(*clusterv1.Cluster)
Expand All @@ -55,3 +69,15 @@ func ClusterByClusterClassClassName(o client.Object) []string {
}
return nil
}

// ClusterByClusterClassNamespace contains the logic to index Clusters by ClusterClass namespace.
func ClusterByClusterClassNamespace(o client.Object) []string {
cluster, ok := o.(*clusterv1.Cluster)
if !ok {
panic(fmt.Sprintf("Expected Cluster but got a %T", o))
}
if cluster.Spec.Topology != nil {
return []string{cluster.GetClassKey().Namespace}
}
return nil
}
4 changes: 4 additions & 0 deletions api/v1beta1/index/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ func AddDefaultIndexes(ctx context.Context, mgr ctrl.Manager) error {
if err := ByClusterClassName(ctx, mgr); err != nil {
return err
}

if err := ByClusterClassNamespace(ctx, mgr); err != nil {
return err
}
}

if feature.Gates.Enabled(feature.MachinePool) {
Expand Down
7 changes: 7 additions & 0 deletions api/v1beta1/zz_generated.openapi.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions cmd/clusterctl/client/cluster/mover_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1846,6 +1846,8 @@ func Test_objectMoverService_ensureNamespaces(t *testing.T) {

cluster1 := test.NewFakeCluster("namespace-1", "cluster-1")
cluster2 := test.NewFakeCluster("namespace-2", "cluster-2")
cluster3 := test.NewFakeCluster("namespace-1", "cluster-3").WithTopologyClass("cluster-class-1").WithTopologyClassWithin("namespace-2")
clusterClass1 := test.NewFakeClusterClass("namespace-2", "cluster-class-1")
globalObj := test.NewFakeClusterExternalObject("eo-1")

clustersObjs := append(cluster1.Objs(), cluster2.Objs()...)
Expand Down Expand Up @@ -1876,6 +1878,16 @@ func Test_objectMoverService_ensureNamespaces(t *testing.T) {
},
expectedNamespaces: []string{"namespace-1", "namespace-2"},
},
{
name: "ensureNamespaces moves namespace-1 and namespace-2 from cross-namespace CC reference",
fields: fields{
objs: append(cluster3.Objs(), clusterClass1.Objs()...),
},
args: args{
toProxy: test.NewFakeProxy(),
},
expectedNamespaces: []string{"namespace-1", "namespace-2"},
},
{
name: "ensureNamespaces moves namespace-2 to target which already has namespace-1",
fields: fields{
Expand Down
10 changes: 9 additions & 1 deletion cmd/clusterctl/client/cluster/objectgraph.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
)

const clusterTopologyNameKey = "cluster.spec.topology.class"
const clusterTopologyNamespaceKey = "cluster.spec.topology.classNamespace"
const clusterResourceSetBindingClusterNameKey = "clusterresourcesetbinding.spec.clustername"

type empty struct{}
Expand Down Expand Up @@ -149,6 +150,7 @@ func (n *node) captureAdditionalInformation(obj *unstructured.Unstructured) erro
n.additionalInfo = map[string]interface{}{}
}
n.additionalInfo[clusterTopologyNameKey] = cluster.GetClassKey().Name
n.additionalInfo[clusterTopologyNamespaceKey] = cluster.GetClassKey().Namespace
}
}

Expand Down Expand Up @@ -620,7 +622,13 @@ func (o *objectGraph) setSoftOwnership() {
for _, cluster := range clusters {
// if the cluster uses a managed topology and uses the clusterclass
// set the clusterclass as a soft owner of the cluster.
if className, ok := cluster.additionalInfo[clusterTopologyNameKey]; ok {
className, hasName := cluster.additionalInfo[clusterTopologyNameKey]
classNamespace, hasNamespace := cluster.additionalInfo[clusterTopologyNamespaceKey]
if hasNamespace && hasName {
if className == clusterClass.identity.Name && clusterClass.identity.Namespace == classNamespace {
cluster.addSoftOwner(clusterClass)
}
} else if hasName {
if className == clusterClass.identity.Name && clusterClass.identity.Namespace == cluster.identity.Namespace {
cluster.addSoftOwner(clusterClass)
}
Expand Down
51 changes: 51 additions & 0 deletions cmd/clusterctl/client/cluster/objectgraph_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2047,6 +2047,57 @@ func Test_objectGraph_setSoftOwnership(t *testing.T) {
},
},
},
{
name: "A different namespaced ClusterClass with a soft owned Cluster",
fields: fields{
objs: func() []client.Object {
objs := test.NewFakeClusterClass("ns1", "class1").Objs()
objs = append(objs, test.NewFakeCluster("ns2", "cluster1").WithTopologyClass("class1").WithTopologyClassWithin("ns1").Objs()...)

return objs
}(),
},
want: wantGraph{
nodes: map[string]wantGraphItem{
"cluster.x-k8s.io/v1beta1, Kind=ClusterClass, ns1/class1": {
forceMove: true,
forceMoveHierarchy: true,
},
"infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureClusterTemplate, ns1/class1": {
owners: []string{
"cluster.x-k8s.io/v1beta1, Kind=ClusterClass, ns1/class1",
},
},
"controlplane.cluster.x-k8s.io/v1beta1, Kind=GenericControlPlaneTemplate, ns1/class1": {
owners: []string{
"cluster.x-k8s.io/v1beta1, Kind=ClusterClass, ns1/class1",
},
},
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns2/cluster1": {
forceMove: true,
forceMoveHierarchy: true,
softOwners: []string{
"cluster.x-k8s.io/v1beta1, Kind=ClusterClass, ns1/class1", // NB. this cluster is not linked to the clusterclass through owner ref, but it is detected as soft ownership
},
},
"infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureCluster, ns2/cluster1": {
owners: []string{
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns2/cluster1",
},
},
"/v1, Kind=Secret, ns2/cluster1-ca": {
softOwners: []string{
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns2/cluster1", // NB. this secret is not linked to the cluster through owner ref, but it is detected as soft ownership
},
},
"/v1, Kind=Secret, ns2/cluster1-kubeconfig": {
owners: []string{
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns2/cluster1",
},
},
},
},
},
{
name: "A Cluster with a soft owned ClusterResourceSetBinding",
fields: fields{
Expand Down
29 changes: 19 additions & 10 deletions cmd/clusterctl/internal/test/fake_objects.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,17 @@ import (
)

type FakeCluster struct {
namespace string
name string
controlPlane *FakeControlPlane
machinePools []*FakeMachinePool
machineDeployments []*FakeMachineDeployment
machineSets []*FakeMachineSet
machines []*FakeMachine
withCloudConfigSecret bool
withCredentialSecret bool
topologyClass *string
namespace string
name string
controlPlane *FakeControlPlane
machinePools []*FakeMachinePool
machineDeployments []*FakeMachineDeployment
machineSets []*FakeMachineSet
machines []*FakeMachine
withCloudConfigSecret bool
withCredentialSecret bool
topologyClass *string
topologyClassNamespace *string
}

// NewFakeCluster return a FakeCluster that can generate a cluster object, all its own ancillary objects:
Expand Down Expand Up @@ -109,6 +110,11 @@ func (f *FakeCluster) WithTopologyClass(class string) *FakeCluster {
return f
}

func (f *FakeCluster) WithTopologyClassWithin(namespace string) *FakeCluster {
f.topologyClassNamespace = &namespace
return f
}

func (f *FakeCluster) Objs() []client.Object {
clusterInfrastructure := &fakeinfrastructure.GenericInfrastructureCluster{
TypeMeta: metav1.TypeMeta{
Expand Down Expand Up @@ -145,6 +151,9 @@ func (f *FakeCluster) Objs() []client.Object {

if f.topologyClass != nil {
cluster.Spec.Topology = &clusterv1.Topology{Class: *f.topologyClass}
if f.topologyClassNamespace != nil {
cluster.Spec.Topology.ClassNamespace = *f.topologyClassNamespace
}
}

// Ensure the cluster gets a UID to be used by dependant objects for creating OwnerReferences.
Expand Down
9 changes: 9 additions & 0 deletions config/crd/bases/cluster.x-k8s.io_clusters.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ flexible enough to be used in as many Clusters as possible by supporting variant
* [Defining a custom naming strategy for MachineDeployment objects](#defining-a-custom-naming-strategy-for-machinedeployment-objects)
* [Defining a custom naming strategy for MachinePool objects](#defining-a-custom-naming-strategy-for-machinepool-objects)
* [Advanced features of ClusterClass with patches](#advanced-features-of-clusterclass-with-patches)
* [MachineDeployment variable overrides](#machinedeployment-variable-overrides)
* [MachineDeployment variable overrides](#machinedeployment-and-machinepool-variable-overrides)
* [Builtin variables](#builtin-variables)
* [Complex variable types](#complex-variable-types)
* [Using variable values in JSON patches](#using-variable-values-in-json-patches)
Expand Down Expand Up @@ -438,11 +438,94 @@ spec:
template: "{{ .cluster.name }}-{{ .machinePool.topologyName }}-{{ .random }}"
```

### Defining a custom namespace for ClusterClass object

As a user, I may need to create a `Cluster` from a `ClusterClass` object that exists only in a different namespace. To uniquely identify the `ClusterClass`, a `NamespacedName` ref is constructed from combination of:
* `cluster.spec.topology.classNamespace` - namespace of the `ClusterClass` object.
* `cluster.spec.topology.class` - name of the `ClusterClass` object.

Example of the `Cluster` object with the `name/namespace` reference:

```yaml
apiVersion: cluster.x-k8s.io/v1beta1
kind: Cluster
metadata:
name: my-docker-cluster
namespace: default
spec:
topology:
class: docker-clusterclass-v0.1.0
classNamespace: default
version: v1.22.4
controlPlane:
replicas: 3
workers:
machineDeployments:
- class: default-worker
name: md-0
replicas: 4
failureDomain: region
```

<aside class="note warning">

<h1>Cluster rebase across namespaces</h1>

Changing `classNamespace` is not supported in rebase procedure, while changing `class` reference to a different `ClusterClass` from the same namespace performs regular `Cluster` rebase procedure.

</aside>

#### Securing cross-namespace reference to the ClusterClass

It is often desirable to restrict free cross-namespace `ClusterClass` access for the `Cluster` object. This can be implemented by defining a [`ValidatingAdmissionPolicy`](https://kubernetes.io/docs/reference/access-authn-authz/validating-admission-policy/#what-is-validating-admission-policy) on the `Cluster` object.

An example of such policy may be:

```yaml
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingAdmissionPolicy
metadata:
name: "cluster-class-ref.cluster.x-k8s.io"
spec:
failurePolicy: Fail
paramKind:
apiVersion: v1
kind: Secret
matchConstraints:
resourceRules:
- apiGroups: ["cluster.x-k8s.io"]
apiVersions: ["v1beta1"]
operations: ["CREATE", "UPDATE"]
resources: ["clusters"]
validations:
- expression: "!has(object.spec.topology.classNamespace) || object.spec.topology.classNamespace in params.data"
---
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingAdmissionPolicyBinding
metadata:
name: "cluster-class-ref-binding.cluster.x-k8s.io"
spec:
policyName: "cluster-class-ref.cluster.x-k8s.io"
validationActions: [Deny]
paramRef:
name: "allowed-namespaces.cluster-class-ref.cluster.x-k8s.io"
namespace: "default"
parameterNotFoundAction: Deny
---
apiVersion: v1
kind: Secret
metadata:
name: "allowed-namespaces.cluster-class-ref.cluster.x-k8s.io"
namespace: "default"
data:
default: ""
```

## Advanced features of ClusterClass with patches

This section will explain more advanced features of ClusterClass patches.

### MachineDeployment & MachinePool variable overrides
### MachineDeployment and MachinePool variable overrides

If you want to use many variations of MachineDeployments in Clusters, you can either define
a MachineDeployment class for every variation or you can define patches and variables to
Expand Down
1 change: 1 addition & 0 deletions internal/apis/core/v1alpha4/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func (src *Cluster) ConvertTo(dstRaw conversion.Hub) error {
if dst.Spec.Topology == nil {
dst.Spec.Topology = &clusterv1.Topology{}
}
dst.Spec.Topology.ClassNamespace = restored.Spec.Topology.ClassNamespace
dst.Spec.Topology.Variables = restored.Spec.Topology.Variables
dst.Spec.Topology.ControlPlane.Variables = restored.Spec.Topology.ControlPlane.Variables

Expand Down
Loading
Loading