Skip to content
11 changes: 11 additions & 0 deletions api/v1alpha1/kamajicontrolplane_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ type KamajiControlPlaneFields struct {
Deployment DeploymentComponent `json:"deployment,omitempty"`
}

// +kubebuilder:validation:XValidation:rule="!has(oldSelf.deploymentName) || has(self.deploymentName)", message="DeploymentName is required once set"
Copy link
Member

Choose a reason for hiding this comment

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

Why this CEL rule is required when we have the ExternalClusterReference.DeploymentName one?

Copy link
Author

@florent1s florent1s Jul 23, 2025

Choose a reason for hiding this comment

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

The way I understand it: this rule prevents the value from being unset while the one on ExternalClusterReference.DeploymentName prevents it from being modified

I based the config on this doc: https://kubernetes.io/blog/2022/09/29/enforce-immutability-using-cel/#immutablility-after-first-modification

type ExternalClusterReference struct {
// The Secret object containing the kubeconfig used to interact with the remote cluster that will host
// the Tenant Control Plane resources generated by the Control Plane Provider.
Expand All @@ -184,6 +185,9 @@ type ExternalClusterReference struct {
KubeconfigSecretNamespace string `json:"kubeconfigSecretNamespace,omitempty"`
// The Namespace where the resulting TenantControlPlane must be deployed to.
DeploymentNamespace string `json:"deploymentNamespace"`
// The Name of the resulting TenantControlPlane.
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="DeploymentName is immutable"
DeploymentName string `json:"deploymentName,omitempty"`
}

// KamajiControlPlaneStatus defines the observed state of KamajiControlPlane.
Expand Down Expand Up @@ -246,3 +250,10 @@ type KamajiControlPlaneList struct {
func init() {
SchemeBuilder.Register(&KamajiControlPlane{}, &KamajiControlPlaneList{})
}

// This label is used to detect collisions when using ExternalClusterReference.DeploymentName
// It allows to keep track of the KCP owning a given TCP when the TCP name is not the default
// "kcp-<KCP_UID>"
const (
KamajiControlPlaneUIDLabel = "kamaji.clastix.io/kamajicontrolplane-uid"
)
20 changes: 20 additions & 0 deletions config/control-plane-components.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1524,6 +1524,13 @@ spec:
When this value is nil, the Cluster API management cluster will be used as a target.
The ExternalClusterReference feature gate must be enabled with one of the available flags.
properties:
deploymentName:
description: |-
The Name of the resulting TenantControlPlane.
type: string
x-kubernetes-validations:
- message: deploymentName is immutable
rule: 'self == oldSelf'
deploymentNamespace:
description: The Namespace where the resulting TenantControlPlane must be deployed to.
type: string
Expand All @@ -1547,6 +1554,9 @@ spec:
- kubeconfigSecretKey
- kubeconfigSecretName
type: object
x-kubernetes-validations:
- message: DeploymentName is required once set
rule: '!has(oldSelf.deploymentName) || has(self.deploymentName)'
extraContainers:
items:
description: A single application container that you want to run within a pod.
Expand Down Expand Up @@ -8244,6 +8254,13 @@ spec:
When this value is nil, the Cluster API management cluster will be used as a target.
The ExternalClusterReference feature gate must be enabled with one of the available flags.
properties:
deploymentName:
description: |-
The Name of the resulting TenantControlPlane.
type: string
x-kubernetes-validations:
- message: deploymentName is immutable
rule: 'self == oldSelf'
deploymentNamespace:
description: The Namespace where the resulting TenantControlPlane must be deployed to.
type: string
Expand All @@ -8267,6 +8284,9 @@ spec:
- kubeconfigSecretKey
- kubeconfigSecretName
type: object
x-kubernetes-validations:
- message: DeploymentName is required once set
rule: '!has(oldSelf.deploymentName) || has(self.deploymentName)'
extraContainers:
items:
description: A single application container that you want to run within a pod.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1572,6 +1572,12 @@ spec:
When this value is nil, the Cluster API management cluster will be used as a target.
The ExternalClusterReference feature gate must be enabled with one of the available flags.
properties:
deploymentName:
description: The Name of the resulting TenantControlPlane.
type: string
x-kubernetes-validations:
- message: DeploymentName is immutable
rule: self == oldSelf
deploymentNamespace:
description: The Namespace where the resulting TenantControlPlane
must be deployed to.
Expand All @@ -1597,6 +1603,9 @@ spec:
- kubeconfigSecretKey
- kubeconfigSecretName
type: object
x-kubernetes-validations:
- message: DeploymentName is required once set
rule: '!has(oldSelf.deploymentName) || has(self.deploymentName)'
extraContainers:
items:
description: A single application container that you want to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1584,6 +1584,12 @@ spec:
When this value is nil, the Cluster API management cluster will be used as a target.
The ExternalClusterReference feature gate must be enabled with one of the available flags.
properties:
deploymentName:
description: The Name of the resulting TenantControlPlane.
type: string
x-kubernetes-validations:
- message: DeploymentName is immutable
rule: self == oldSelf
deploymentNamespace:
description: The Namespace where the resulting TenantControlPlane
must be deployed to.
Expand All @@ -1609,6 +1615,9 @@ spec:
- kubeconfigSecretKey
- kubeconfigSecretName
type: object
x-kubernetes-validations:
- message: DeploymentName is required once set
rule: '!has(oldSelf.deploymentName) || has(self.deploymentName)'
extraContainers:
items:
description: A single application container that you
Expand Down
24 changes: 21 additions & 3 deletions controllers/kamajicontrolplane_controller_tcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,21 @@ import (
"net"
"strings"

kcpv1alpha1 "github.com/clastix/cluster-api-control-plane-provider-kamaji/api/v1alpha1"
"github.com/clastix/cluster-api-control-plane-provider-kamaji/pkg/externalclusterreference"
kamajiv1alpha1 "github.com/clastix/kamaji/api/v1alpha1"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/util/retry"
"k8s.io/utils/ptr"
capiv1beta1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

kcpv1alpha1 "github.com/clastix/cluster-api-control-plane-provider-kamaji/api/v1alpha1"
"github.com/clastix/cluster-api-control-plane-provider-kamaji/pkg/externalclusterreference"
)

var ErrUnsupportedCertificateSAN = errors.New("a certificate SAN must be made of host only with no port")
var ErrTCPCollision = errors.New("Collision on remote TenantControlPlanes")

//+kubebuilder:rbac:groups=kamaji.clastix.io,resources=tenantcontrolplanes,verbs=get;list;watch;create;update

Expand Down Expand Up @@ -57,6 +58,23 @@ func (r *KamajiControlPlaneReconciler) createOrUpdateTenantControlPlane(ctx cont

tcp.Labels = kcp.Labels

// Check the KCP-UID label to avoid collsions if 2 clusters with the same name
// use the same namespace with externalClusterReference
// if label is not present, it will be added
if isDelegatedExternally && kcp.Spec.Deployment.ExternalClusterReference.DeploymentName != "" {
var tcpInCluster kamajiv1alpha1.TenantControlPlane
remoteClient.Get(ctx, types.NamespacedName{Namespace: tcp.Namespace, Name: tcp.Name}, &tcpInCluster)
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason not to check the error here?


if val := tcpInCluster.Labels[kcpv1alpha1.KamajiControlPlaneUIDLabel]; val != "" {
if val != string(kcp.UID) {
return errors.Wrap(ErrTCPCollision, fmt.Sprintf("Collision on TenantControlPlane %s: Value of label '%s' does not match.", tcp.Name, kcpv1alpha1.KamajiControlPlaneUIDLabel))
}
// label matches our kcp UID -> update TCP (nothing to do here)
} else { // label not present -> claim this TCP by adding it
tcp.Labels[kcpv1alpha1.KamajiControlPlaneUIDLabel] = string(kcp.UID)
}
}

if kubeconfigSecretKey := kcp.Annotations[kamajiv1alpha1.KubeconfigSecretKeyAnnotation]; kubeconfigSecretKey != "" {
tcp.Annotations[kamajiv1alpha1.KubeconfigSecretKeyAnnotation] = kubeconfigSecretKey
} else {
Expand Down
17 changes: 17 additions & 0 deletions controllers/kamajicontrolplane_finalizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,23 @@ func (r *KamajiControlPlaneReconciler) handleDeletion(ctx context.Context, kcp v
var tcp kamajiv1alpha1.TenantControlPlane
tcp.Name, tcp.Namespace = externalclusterreference.GenerateRemoteTenantControlPlaneNames(kcp)

// Check KamajiControlPlaneUIDLabel on TCP, to avoid deleting it if it doesn't belong to our KCP
if kcp.Spec.Deployment.ExternalClusterReference.DeploymentName != "" {

if err := remoteClient.Get(ctx, types.NamespacedName{Namespace: tcp.Namespace, Name: tcp.Name}, &tcp); err != nil {
if errors.IsNotFound(err) {
log.Info("resource may have been deleted")
}
log.Error(err, "unable to get remote TenantControlPlane")
Comment on lines +65 to +68
Copy link
Member

Choose a reason for hiding this comment

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

In case of Not Found error, we'll print the Info, as well as the Error level, since we're not blocking execution.

We should return nil for the first case, and err for the second one.

}

if val := tcp.GetLabels()[v1alpha1.KamajiControlPlaneUIDLabel]; val != "" && val != string(kcp.UID) {
log.Info("Did not delete remote TenantControlPlane as it belongs to another KamajiControlPlane")
Copy link
Member

Choose a reason for hiding this comment

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

Could we log as argument the found and mismatching UID here? Just for the sake of debugging.


return nil
}
}

if tcpErr := remoteClient.Delete(ctx, &tcp); tcpErr != nil {
if errors.IsNotFound(tcpErr) {
log.Info("remote TenantControlPlane is already deleted")
Expand Down
4 changes: 4 additions & 0 deletions pkg/externalclusterreference/name_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ func ParseKamajiControlPlaneUIDFromTenantControlPlane(tcp kamajiv1alpha1.TenantC
}

func GenerateRemoteTenantControlPlaneNames(kcp v1alpha1.KamajiControlPlane) (name string, namespace string) { //nolint:nonamedreturns
if kcp.Spec.Deployment.ExternalClusterReference.DeploymentName != "" {
return kcp.Spec.Deployment.ExternalClusterReference.DeploymentName, kcp.Spec.Deployment.ExternalClusterReference.DeploymentNamespace
}

return RemoteTCPPrefix + string(kcp.UID), kcp.Spec.Deployment.ExternalClusterReference.DeploymentNamespace
}

Expand Down