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

[release-1.9] ✨ Add classNamespace to topology #11730

Open
wants to merge 1 commit into
base: release-1.9
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"`

// classNamespace is the namespace of the ClusterClass object to create the topology.
// If the namespace is empty or not set, it is defaulted to the namespace of the cluster object.
// Value must follow the DNS1123Subdomain syntax.
// +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"`

// 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
36 changes: 36 additions & 0 deletions api/v1beta1/index/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,44 @@ import (
const (
// ClusterClassNameField is used by the Cluster controller to index Clusters by ClusterClass name.
ClusterClassNameField = "spec.topology.class"

// ClusterClassRefPath is used by the Cluster controller to index Clusters by ClusterClass name and namespace.
ClusterClassRefPath = "spec.topology.classRef"

// clusterClassRefFmt is used to correctly format class ref index key.
clusterClassRefFmt = "%s/%s"
)

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

// ClusterByClusterClassRef contains the logic to index Clusters by ClusterClass name and namespace.
func ClusterByClusterClassRef(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 {
key := cluster.GetClassKey()
return []string{fmt.Sprintf(clusterClassRefFmt, key.Namespace, key.Name)}
}
return nil
}

// ClusterClassRef returns ClusterClass index key to be used for search.
func ClusterClassRef(cc *clusterv1.ClusterClass) string {
return fmt.Sprintf(clusterClassRefFmt, cc.GetNamespace(), cc.GetName())
}

// ByClusterClassName adds the cluster class name index to the
// managers cache.
func ByClusterClassName(ctx context.Context, mgr ctrl.Manager) error {
Expand Down
27 changes: 24 additions & 3 deletions api/v1beta1/index/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@ import (
"testing"

. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"

clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
)

func TestClusterByClassName(t *testing.T) {
func TestClusterByClusterClassRef(t *testing.T) {
testCases := []struct {
name string
object client.Object
Expand All @@ -39,20 +40,40 @@ func TestClusterByClassName(t *testing.T) {
{
name: "when cluster has a valid Topology",
object: &clusterv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster",
Namespace: "default",
},
Spec: clusterv1.ClusterSpec{
Topology: &clusterv1.Topology{
Class: "class1",
},
},
},
expected: []string{"class1"},
expected: []string{"default/class1"},
},
{
name: "when cluster has a valid Topology with namespace specified",
object: &clusterv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster",
Namespace: "default",
},
Spec: clusterv1.ClusterSpec{
Topology: &clusterv1.Topology{
Class: "class1",
ClassNamespace: "other",
},
},
},
expected: []string{"other/class1"},
},
}

for _, test := range testCases {
t.Run(test.name, func(t *testing.T) {
g := NewWithT(t)
got := ClusterByClusterClassClassName(test.object)
got := ClusterByClusterClassRef(test.object)
g.Expect(got).To(Equal(test.expected))
})
}
Expand Down
2 changes: 1 addition & 1 deletion api/v1beta1/index/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func AddDefaultIndexes(ctx context.Context, mgr ctrl.Manager) error {
}

if feature.Gates.Enabled(feature.ClusterTopology) {
if err := ByClusterClassName(ctx, mgr); err != nil {
if err := ByClusterClassRef(ctx, mgr); err != nil {
return err
}
}
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.

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,87 @@ 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
```


#### 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
1 change: 1 addition & 0 deletions internal/apis/core/v1alpha4/zz_generated.conversion.go

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

5 changes: 3 additions & 2 deletions internal/controllers/topology/cluster/cluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -492,8 +492,9 @@ func (r *Reconciler) clusterClassToCluster(ctx context.Context, o client.Object)
if err := r.Client.List(
ctx,
clusterList,
client.MatchingFields{index.ClusterClassNameField: clusterClass.Name},
client.InNamespace(clusterClass.Namespace),
client.MatchingFields{
index.ClusterClassRefPath: index.ClusterClassRef(clusterClass),
},
); err != nil {
return nil
}
Expand Down
68 changes: 68 additions & 0 deletions internal/controllers/topology/cluster/cluster_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import (
var (
clusterName1 = "cluster1"
clusterName2 = "cluster2"
clusterName3 = "cluster3"
clusterClassName1 = "class1"
clusterClassName2 = "class2"
infrastructureMachineTemplateName1 = "inframachinetemplate1"
Expand Down Expand Up @@ -854,6 +855,19 @@ func setupTestEnvForIntegrationTests(ns *corev1.Namespace) (func() error, error)
Build()).
Build()

// Cross ns referencing cluster
cluster3 := builder.Cluster(ns.Name, clusterName3).
WithTopology(
builder.ClusterTopology().
WithClass(clusterClass.Name).
WithClassNamespace("other").
WithMachineDeployment(machineDeploymentTopology2).
WithMachinePool(machinePoolTopology2).
WithVersion("1.21.0").
WithControlPlaneReplicas(1).
Build()).
Build()

// Setup kubeconfig secrets for the clusters, so the ClusterCacheTracker works.
cluster1Secret := kubeconfig.GenerateSecret(cluster1, kubeconfig.FromEnvTestConfig(env.Config, cluster1))
cluster2Secret := kubeconfig.GenerateSecret(cluster2, kubeconfig.FromEnvTestConfig(env.Config, cluster2))
Expand All @@ -876,6 +890,7 @@ func setupTestEnvForIntegrationTests(ns *corev1.Namespace) (func() error, error)
clusterClassForRebase,
cluster1,
cluster2,
cluster3,
cluster1Secret,
cluster2Secret,
}
Expand Down Expand Up @@ -1577,3 +1592,56 @@ func TestReconciler_ValidateCluster(t *testing.T) {
})
}
}

func TestClusterClassToCluster(t *testing.T) {
utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.ClusterTopology, true)
g := NewWithT(t)

ns, err := env.CreateNamespace(ctx, "cluster-reconcile-namespace")
g.Expect(err).ToNot(HaveOccurred())

// Create the objects needed for the integration test:
cleanup, err := setupTestEnvForIntegrationTests(ns)
g.Expect(err).ToNot(HaveOccurred())

// Defer a cleanup function that deletes each of the objects created during setupTestEnvForIntegrationTests.
defer func() {
g.Expect(cleanup()).To(Succeed())
}()

tests := []struct {
name string
clusterClass *clusterv1.ClusterClass
expected []reconcile.Request
}{
{
name: "ClusterClass change should request reconcile for the referenced class",
clusterClass: builder.ClusterClass(ns.Name, clusterClassName1).Build(),
expected: []reconcile.Request{
{NamespacedName: client.ObjectKeyFromObject(builder.Cluster(ns.Name, clusterName1).Build())},
{NamespacedName: client.ObjectKeyFromObject(builder.Cluster(ns.Name, clusterName2).Build())},
},
},
{
name: "ClusterClass with no matching name and namespace should not trigger reconcile",
clusterClass: builder.ClusterClass("other", clusterClassName2).Build(),
expected: []reconcile.Request{},
},
{
name: "Different ClusterClass with matching name and namespace should trigger reconcile",
clusterClass: builder.ClusterClass("other", clusterClassName1).Build(),
expected: []reconcile.Request{
{NamespacedName: client.ObjectKeyFromObject(builder.Cluster(ns.Name, clusterName3).Build())},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(*testing.T) {
r := &Reconciler{Client: env.GetClient()}

requests := r.clusterClassToCluster(ctx, tt.clusterClass)
g.Expect(requests).To(ConsistOf(tt.expected))
})
}
}
Loading
Loading