Skip to content

feat: externalClusterReference with default naming scheme #215

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.

Sign up for GitHub

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

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

feat: externalClusterReference with default naming scheme #215

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c8a450f
feat: option to use default nameing scheme with externalclusterref
May 16, 2025
9e856da
fix(crd): boolean instead of bool
May 19, 2025
8b4aaad
fix(name_gen): fixes for CI / lint
May 20, 2025
acb4cc9
fix: minor fixes for CI
May 20, 2025
55c0717
fix(crds): order of externalClusterReference properties
May 20, 2025
d50d28a
feat: replace keepDefaultName with deploymentName
May 30, 2025
2ce99fa
feat: make deploymentName immutable
Jun 3, 2025
aec406b
fix: custom TCP deploymentName immutable once set
Jun 27, 2025
2919877
feat: externalcluster with custom TCP name
florent1s Jun 27, 2025
02c2c12
feat: TCP Collision detection with ExternalCluster
florent1s Jul 15, 2025
9aaffb4
feat(kcp_types): explain KCPUID label
Jul 23, 2025
67688df
feat(kcp_finalizer): use GetLabels
Jul 23, 2025
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
3 changes: 3 additions & 0 deletions api/v1alpha1/kamajicontrolplane_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ type ExternalClusterReference struct {
KubeconfigSecretNamespace string `json:"kubeconfigSecretNamespace,omitempty"`
// The Namespace where the resulting TenantControlPlane must be deployed to.
DeploymentNamespace string `json:"deploymentNamespace"`
// Set tenantControlPlane name to corresponding kamajiControlPlane's name instead of "kcp-<KCP_UID>".
// May lead to collisions in external cluster, use with care.
KeepDefaultName bool `json:"keepDefaultName,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

This option assumes users are okay with preserving their original name. I can imagine somebody else could show up in the future asking for a customised name.

If we want to address this enhancement, we should support an optional custom name, and make it immutable once set (using CEL).

Furthermore, the May lead to collisions is something we can't accept, despite this option being designed for advanced use cases.

Copy link
Author

Choose a reason for hiding this comment

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

This option assumes users are okay with preserving their original name. I can imagine somebody else could show up in the future asking for a customised name.

If we want to address this enhancement, we should support an optional custom name, and make it immutable once set (using CEL).

Good point, I'll adapt this part.

Furthermore, the May lead to collisions is something we can't accept, despite this option being designed for advanced use cases.

Definitely agree that it is not really a good solution, outright preventing those collisons would be way better. I'll see what I can work out based on your suggestion.

Copy link
Author

Choose a reason for hiding this comment

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

Improvements 1 and 2 should be good now, 3 will take more time but I think that I can work something out.

Copy link
Author

Choose a reason for hiding this comment

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

Here is my take on improvement 3 to detect collisions:

  • On TCP creation add a label with the KCP UID
  • On TCP update or deletion check this label to make sure it belongs to the current KCP

}

// KamajiControlPlaneStatus defines the observed state of KamajiControlPlane.
Expand Down
10 changes: 10 additions & 0 deletions config/control-plane-components.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1527,6 +1527,11 @@ spec:
deploymentNamespace:
description: The Namespace where the resulting TenantControlPlane must be deployed to.
type: string
keepDefaultName:
description: |-
Set tenantControlPlane name to corresponding kamajiControlPlane's name instead of "kcp-<KCP_UID>".
May lead to collisions in external cluster, use with care.
type: boolean
kubeconfigSecretKey:
description: The key used to extract the kubeconfig from the specified Secret.
minLength: 1
Expand Down Expand Up @@ -8247,6 +8252,11 @@ spec:
deploymentNamespace:
description: The Namespace where the resulting TenantControlPlane must be deployed to.
type: string
keepDefaultName:
description: |-
Set tenantControlPlane name to corresponding kamajiControlPlane's name instead of "kcp-<KCP_UID>".
May lead to collisions in external cluster, use with care.
type: boolean
kubeconfigSecretKey:
description: The key used to extract the kubeconfig from the specified Secret.
minLength: 1
Expand Down
5 changes: 5 additions & 0 deletions config/crd/bases/controlplane.cluster.x-k8s.io_kamajicontrolplanes.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1576,6 +1576,11 @@ spec:
description: The Namespace where the resulting TenantControlPlane
must be deployed to.
type: string
keepDefaultName:
description: |-
Set tenantControlPlane name to corresponding kamajiControlPlane's name instead of "kcp-<KCP_UID>".
May lead to collisions in external cluster, use with care.
type: boolean
kubeconfigSecretKey:
description: The key used to extract the kubeconfig from the
specified Secret.
Expand Down
5 changes: 5 additions & 0 deletions config/crd/bases/controlplane.cluster.x-k8s.io_kamajicontrolplanetemplates.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1588,6 +1588,11 @@ spec:
description: The Namespace where the resulting TenantControlPlane
must be deployed to.
type: string
keepDefaultName:
description: |-
Set tenantControlPlane name to corresponding kamajiControlPlane's name instead of "kcp-<KCP_UID>".
May lead to collisions in external cluster, use with care.
type: boolean
kubeconfigSecretKey:
description: The key used to extract the kubeconfig
from the specified Secret.
Expand Down
4 changes: 4 additions & 0 deletions pkg/externalclusterreference/name_generator.go
View file
Open in desktop
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.KeepDefaultName {
return kcp.GetName(), kcp.Spec.Deployment.ExternalClusterReference.DeploymentNamespace
}

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

Expand Down