diff --git a/api/v1alpha1/tenantcontrolplane_funcs.go b/api/v1alpha1/tenantcontrolplane_funcs.go index b0b1868..daffcf9 100644 --- a/api/v1alpha1/tenantcontrolplane_funcs.go +++ b/api/v1alpha1/tenantcontrolplane_funcs.go @@ -82,6 +82,45 @@ func (in *TenantControlPlane) DeclaredControlPlaneAddress(ctx context.Context, c return "", kamajierrors.MissingValidIPError{} } +// ControlPlaneServiceIPs returns every IP address the Tenant Control Plane Service +// answers on: all of its ClusterIPs (covering both families of a dual-stack Service) +// and all LoadBalancer ingress IPs. It is meant for certificate SANs, so it is +// best-effort about IP availability: a not-yet-provisioned LoadBalancer simply yields +// no ingress IPs rather than an error. A missing Service, in contrast, is returned as +// an error. The caller is expected to also include the primary advertised/management +// address. +func (in *TenantControlPlane) ControlPlaneServiceIPs(ctx context.Context, c client.Client) ([]string, error) { + svc := &corev1.Service{} + if err := c.Get(ctx, types.NamespacedName{Namespace: in.GetNamespace(), Name: in.GetName()}, svc); err != nil { + return nil, fmt.Errorf("cannot retrieve Service for the TenantControlPlane: %w", err) + } + + // ClusterIPs carries both families of a dual-stack Service; fall back to the + // singular ClusterIP for Services that only populate the legacy field. + clusterIPs := svc.Spec.ClusterIPs + if len(clusterIPs) == 0 && len(svc.Spec.ClusterIP) > 0 { + clusterIPs = []string{svc.Spec.ClusterIP} + } + + ips := make([]string, 0, len(clusterIPs)+len(svc.Status.LoadBalancer.Ingress)) + + for _, ip := range clusterIPs { + if ip == "" || ip == corev1.ClusterIPNone { + continue + } + + ips = append(ips, ip) + } + + for _, ingress := range svc.Status.LoadBalancer.Ingress { + if len(ingress.IP) > 0 { + ips = append(ips, ingress.IP) + } + } + + return ips, nil +} + // getLoadBalancerAddress extracts the IP address from LoadBalancer ingress. // It also checks and rejects hostname usage for LoadBalancer ingress. // diff --git a/api/v1alpha1/tenantcontrolplane_funcs_test.go b/api/v1alpha1/tenantcontrolplane_funcs_test.go new file mode 100644 index 0000000..dd57452 --- /dev/null +++ b/api/v1alpha1/tenantcontrolplane_funcs_test.go @@ -0,0 +1,137 @@ +// Copyright 2022 Clastix Labs +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1 + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +const ( + clusterIPv4 = "10.96.0.1" + clusterIPv6 = "2001:db8::1" +) + +func TestControlPlaneServiceIPs(t *testing.T) { + const ( + name = "tcp" + ns = "default" + ) + + tcp := &TenantControlPlane{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns}} + + svc := func(mutate func(*corev1.Service)) *corev1.Service { + s := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns}} + mutate(s) + + return s + } + + tests := []struct { + name string + service *corev1.Service + want []string + }{ + { + name: "dual-stack ClusterIP returns both families", + service: svc(func(s *corev1.Service) { + s.Spec.Type = corev1.ServiceTypeClusterIP + s.Spec.ClusterIP = clusterIPv4 + s.Spec.ClusterIPs = []string{clusterIPv4, clusterIPv6} + }), + want: []string{clusterIPv4, clusterIPv6}, + }, + { + name: "single-stack ClusterIP returns one IP", + service: svc(func(s *corev1.Service) { + s.Spec.Type = corev1.ServiceTypeClusterIP + s.Spec.ClusterIP = clusterIPv6 + s.Spec.ClusterIPs = []string{clusterIPv6} + }), + want: []string{clusterIPv6}, + }, + { + name: "legacy Service with only ClusterIP falls back", + service: svc(func(s *corev1.Service) { + s.Spec.Type = corev1.ServiceTypeClusterIP + s.Spec.ClusterIP = clusterIPv4 + }), + want: []string{clusterIPv4}, + }, + { + name: "headless ClusterIP is skipped", + service: svc(func(s *corev1.Service) { + s.Spec.Type = corev1.ServiceTypeClusterIP + s.Spec.ClusterIP = corev1.ClusterIPNone + s.Spec.ClusterIPs = []string{corev1.ClusterIPNone} + }), + want: []string{}, + }, + { + name: "LoadBalancer returns ClusterIPs and all ingress IPs", + service: svc(func(s *corev1.Service) { + s.Spec.Type = corev1.ServiceTypeLoadBalancer + s.Spec.ClusterIP = clusterIPv4 + s.Spec.ClusterIPs = []string{clusterIPv4, clusterIPv6} + s.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{ + {IP: "192.0.2.10"}, + {IP: "2001:db8:cafe::10"}, + } + }), + want: []string{clusterIPv4, clusterIPv6, "192.0.2.10", "2001:db8:cafe::10"}, + }, + { + name: "LoadBalancer not yet provisioned returns ClusterIPs only", + service: svc(func(s *corev1.Service) { + s.Spec.Type = corev1.ServiceTypeLoadBalancer + s.Spec.ClusterIP = clusterIPv4 + s.Spec.ClusterIPs = []string{clusterIPv4} + }), + want: []string{clusterIPv4}, + }, + { + name: "ingress hostname without IP is skipped", + service: svc(func(s *corev1.Service) { + s.Spec.Type = corev1.ServiceTypeLoadBalancer + s.Spec.ClusterIP = clusterIPv4 + s.Spec.ClusterIPs = []string{clusterIPv4} + s.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{{Hostname: "lb.example.com"}} + }), + want: []string{clusterIPv4}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := fake.NewClientBuilder().WithObjects(tt.service).Build() + + got, err := tcp.ControlPlaneServiceIPs(t.Context(), c) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(got) != len(tt.want) { + t.Fatalf("got %v, want %v", got, tt.want) + } + + for i := range tt.want { + if got[i] != tt.want[i] { + t.Fatalf("got %v, want %v", got, tt.want) + } + } + }) + } +} + +func TestControlPlaneServiceIPsMissingService(t *testing.T) { + tcp := &TenantControlPlane{ObjectMeta: metav1.ObjectMeta{Name: "tcp", Namespace: "default"}} + c := fake.NewClientBuilder().Build() + + if _, err := tcp.ControlPlaneServiceIPs(t.Context(), c); err == nil { + t.Fatal("expected an error when the Service does not exist, got nil") + } +} diff --git a/api/v1alpha1/tenantcontrolplane_types.go b/api/v1alpha1/tenantcontrolplane_types.go index 14b5b19..a3b016c 100644 --- a/api/v1alpha1/tenantcontrolplane_types.go +++ b/api/v1alpha1/tenantcontrolplane_types.go @@ -367,6 +367,24 @@ type ServiceSpec struct { // rejected by validation. //+optional AllocateLoadBalancerNodePorts *bool `json:"allocateLoadBalancerNodePorts,omitempty"` + // IPFamilyPolicy maps directly to the generated Service's spec.ipFamilyPolicy. + // When nil, the management cluster default applies, preserving existing behaviour. + // PreferDualStack and RequireDualStack describe a dual-stack Service and expect + // two entries in ipFamilies; a RequireDualStack policy that cannot be satisfied + // (for example with a single family) is rejected by the API server and surfaces + // as a reconcile error. + //+optional + //+kubebuilder:validation:Enum=SingleStack;PreferDualStack;RequireDualStack + IPFamilyPolicy *corev1.IPFamilyPolicy `json:"ipFamilyPolicy,omitempty"` + // IPFamilies maps directly to the generated Service's spec.ipFamilies. Order is + // significant: the first entry is the primary family. When empty, the management + // cluster default applies. A Service's IP families cannot be reduced or swapped + // after creation (only a single-stack Service may be upgraded to dual-stack); + // forbidden transitions are rejected by the API server and surface as a reconcile error. + //+optional + //+kubebuilder:validation:MaxItems=2 + //+kubebuilder:validation:items:Enum=IPv4;IPv6 + IPFamilies []corev1.IPFamily `json:"ipFamilies,omitempty"` } // AddonSpec defines the spec for every addon. @@ -496,6 +514,8 @@ type DataStoreOverride struct { // +kubebuilder:validation:XValidation:rule="!has(self.networkProfile.loadBalancerClass) || self.controlPlane.service.serviceType == 'LoadBalancer'", message="LoadBalancerClass is supported only with LoadBalancer service type" // +kubebuilder:validation:XValidation:rule="!has(self.controlPlane.service.allocateLoadBalancerNodePorts) || self.controlPlane.service.serviceType == 'LoadBalancer'", message="allocateLoadBalancerNodePorts is supported only with LoadBalancer service type" // +kubebuilder:validation:XValidation:rule="self.controlPlane.service.serviceType != 'LoadBalancer' || (oldSelf.controlPlane.service.serviceType != 'LoadBalancer' && self.controlPlane.service.serviceType == 'LoadBalancer') || has(self.networkProfile.loadBalancerClass) == has(oldSelf.networkProfile.loadBalancerClass)",message="LoadBalancerClass cannot be set or unset at runtime" +// +kubebuilder:validation:XValidation:rule="!has(self.controlPlane.service.ipFamilyPolicy) || self.controlPlane.service.ipFamilyPolicy != 'SingleStack' || !has(self.controlPlane.service.ipFamilies) || size(self.controlPlane.service.ipFamilies) <= 1", message="ipFamilies must contain at most one entry when ipFamilyPolicy is SingleStack" +// +kubebuilder:validation:XValidation:rule="!has(self.controlPlane.service.ipFamilies) || size(self.controlPlane.service.ipFamilies) < 2 || self.controlPlane.service.ipFamilies[0] != self.controlPlane.service.ipFamilies[1]", message="ipFamilies entries must be unique" type TenantControlPlaneSpec struct { // WritePermissions allows to select which operations (create, delete, update) must be blocked: diff --git a/api/v1alpha1/tenantcontrolplane_types_test.go b/api/v1alpha1/tenantcontrolplane_types_test.go index 33ccc65..5729040 100644 --- a/api/v1alpha1/tenantcontrolplane_types_test.go +++ b/api/v1alpha1/tenantcontrolplane_types_test.go @@ -8,6 +8,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" @@ -145,4 +146,61 @@ var _ = Describe("Cluster controller", func() { Expect(err.Error()).To(ContainSubstring("allocateLoadBalancerNodePorts is supported only with LoadBalancer service type")) }) }) + + Context("IPFamilies", func() { + It("allows a valid IPv6-only single-stack service", func() { + tcp.Spec.ControlPlane.Service.ServiceType = ServiceTypeClusterIP + tcp.Spec.ControlPlane.Service.IPFamilyPolicy = ptr.To(corev1.IPFamilyPolicySingleStack) + tcp.Spec.ControlPlane.Service.IPFamilies = []corev1.IPFamily{corev1.IPv6Protocol} + + err := k8sClient.Create(ctx, tcp) + Expect(err).NotTo(HaveOccurred()) + }) + + It("allows a dual-stack service with two families", func() { + tcp.Spec.ControlPlane.Service.ServiceType = ServiceTypeClusterIP + tcp.Spec.ControlPlane.Service.IPFamilyPolicy = ptr.To(corev1.IPFamilyPolicyRequireDualStack) + tcp.Spec.ControlPlane.Service.IPFamilies = []corev1.IPFamily{corev1.IPv6Protocol, corev1.IPv4Protocol} + + err := k8sClient.Create(ctx, tcp) + Expect(err).NotTo(HaveOccurred()) + }) + + It("allows creation when the fields are unset", func() { + tcp.Spec.ControlPlane.Service.ServiceType = ServiceTypeClusterIP + + err := k8sClient.Create(ctx, tcp) + Expect(err).NotTo(HaveOccurred()) + }) + + It("denies SingleStack with two families", func() { + tcp.Spec.ControlPlane.Service.ServiceType = ServiceTypeClusterIP + tcp.Spec.ControlPlane.Service.IPFamilyPolicy = ptr.To(corev1.IPFamilyPolicySingleStack) + tcp.Spec.ControlPlane.Service.IPFamilies = []corev1.IPFamily{corev1.IPv4Protocol, corev1.IPv6Protocol} + + err := k8sClient.Create(ctx, tcp) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("ipFamilies must contain at most one entry when ipFamilyPolicy is SingleStack")) + }) + + It("denies duplicate families", func() { + tcp.Spec.ControlPlane.Service.ServiceType = ServiceTypeClusterIP + tcp.Spec.ControlPlane.Service.IPFamilies = []corev1.IPFamily{corev1.IPv4Protocol, corev1.IPv4Protocol} + + err := k8sClient.Create(ctx, tcp) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("ipFamilies entries must be unique")) + }) + + It("denies an invalid IP family value", func() { + tcp.Spec.ControlPlane.Service.ServiceType = ServiceTypeClusterIP + tcp.Spec.ControlPlane.Service.IPFamilies = []corev1.IPFamily{corev1.IPFamily("Foobar")} + + err := k8sClient.Create(ctx, tcp) + Expect(err).To(HaveOccurred()) + // Actual apiserver error: `spec.controlPlane.service.ipFamilies[0]: Unsupported value: "Foobar": supported values: "IPv4", "IPv6"` + // "Unsupported value" is the stable enum-rejection phrase from the apiserver. + Expect(err.Error()).To(ContainSubstring("Unsupported value")) + }) + }) }) diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index c9b8ecd..4a11cb7 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -1744,6 +1744,16 @@ func (in *ServiceSpec) DeepCopyInto(out *ServiceSpec) { *out = new(bool) **out = **in } + if in.IPFamilyPolicy != nil { + in, out := &in.IPFamilyPolicy, &out.IPFamilyPolicy + *out = new(corev1.IPFamilyPolicy) + **out = **in + } + if in.IPFamilies != nil { + in, out := &in.IPFamilies, &out.IPFamilies + *out = make([]corev1.IPFamily, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ServiceSpec. diff --git a/charts/kamaji-crds/hack/kamaji.clastix.io_tenantcontrolplanes_spec.yaml b/charts/kamaji-crds/hack/kamaji.clastix.io_tenantcontrolplanes_spec.yaml index 9debdfd..cc09aff 100644 --- a/charts/kamaji-crds/hack/kamaji.clastix.io_tenantcontrolplanes_spec.yaml +++ b/charts/kamaji-crds/hack/kamaji.clastix.io_tenantcontrolplanes_spec.yaml @@ -8352,6 +8352,36 @@ versions: valid when serviceType is LoadBalancer; setting it with any other serviceType is rejected by validation. type: boolean + ipFamilies: + description: |- + IPFamilies maps directly to the generated Service's spec.ipFamilies. Order is + significant: the first entry is the primary family. When empty, the management + cluster default applies. A Service's IP families cannot be reduced or swapped + after creation (only a single-stack Service may be upgraded to dual-stack); + forbidden transitions are rejected by the API server and surface as a reconcile error. + items: + description: |- + IPFamily represents the IP Family (IPv4 or IPv6). This type is used + to express the family of an IP expressed by a type (e.g. service.spec.ipFamilies). + enum: + - IPv4 + - IPv6 + type: string + maxItems: 2 + type: array + ipFamilyPolicy: + description: |- + IPFamilyPolicy maps directly to the generated Service's spec.ipFamilyPolicy. + When nil, the management cluster default applies, preserving existing behaviour. + PreferDualStack and RequireDualStack describe a dual-stack Service and expect + two entries in ipFamilies; a RequireDualStack policy that cannot be satisfied + (for example with a single family) is rejected by the API server and surfaces + as a reconcile error. + enum: + - SingleStack + - PreferDualStack + - RequireDualStack + type: string serviceType: description: ServiceType allows specifying how to expose the Tenant Control Plane. enum: @@ -8712,6 +8742,10 @@ versions: rule: '!has(self.controlPlane.service.allocateLoadBalancerNodePorts) || self.controlPlane.service.serviceType == ''LoadBalancer''' - message: LoadBalancerClass cannot be set or unset at runtime rule: self.controlPlane.service.serviceType != 'LoadBalancer' || (oldSelf.controlPlane.service.serviceType != 'LoadBalancer' && self.controlPlane.service.serviceType == 'LoadBalancer') || has(self.networkProfile.loadBalancerClass) == has(oldSelf.networkProfile.loadBalancerClass) + - message: ipFamilies must contain at most one entry when ipFamilyPolicy is SingleStack + rule: '!has(self.controlPlane.service.ipFamilyPolicy) || self.controlPlane.service.ipFamilyPolicy != ''SingleStack'' || !has(self.controlPlane.service.ipFamilies) || size(self.controlPlane.service.ipFamilies) <= 1' + - message: ipFamilies entries must be unique + rule: '!has(self.controlPlane.service.ipFamilies) || size(self.controlPlane.service.ipFamilies) < 2 || self.controlPlane.service.ipFamilies[0] != self.controlPlane.service.ipFamilies[1]' status: description: TenantControlPlaneStatus defines the observed state of TenantControlPlane. properties: diff --git a/charts/kamaji/crds/kamaji.clastix.io_tenantcontrolplanes.yaml b/charts/kamaji/crds/kamaji.clastix.io_tenantcontrolplanes.yaml index 70dd8e0..4e5caa1 100644 --- a/charts/kamaji/crds/kamaji.clastix.io_tenantcontrolplanes.yaml +++ b/charts/kamaji/crds/kamaji.clastix.io_tenantcontrolplanes.yaml @@ -8360,6 +8360,36 @@ spec: valid when serviceType is LoadBalancer; setting it with any other serviceType is rejected by validation. type: boolean + ipFamilies: + description: |- + IPFamilies maps directly to the generated Service's spec.ipFamilies. Order is + significant: the first entry is the primary family. When empty, the management + cluster default applies. A Service's IP families cannot be reduced or swapped + after creation (only a single-stack Service may be upgraded to dual-stack); + forbidden transitions are rejected by the API server and surface as a reconcile error. + items: + description: |- + IPFamily represents the IP Family (IPv4 or IPv6). This type is used + to express the family of an IP expressed by a type (e.g. service.spec.ipFamilies). + enum: + - IPv4 + - IPv6 + type: string + maxItems: 2 + type: array + ipFamilyPolicy: + description: |- + IPFamilyPolicy maps directly to the generated Service's spec.ipFamilyPolicy. + When nil, the management cluster default applies, preserving existing behaviour. + PreferDualStack and RequireDualStack describe a dual-stack Service and expect + two entries in ipFamilies; a RequireDualStack policy that cannot be satisfied + (for example with a single family) is rejected by the API server and surfaces + as a reconcile error. + enum: + - SingleStack + - PreferDualStack + - RequireDualStack + type: string serviceType: description: ServiceType allows specifying how to expose the Tenant Control Plane. enum: @@ -8720,6 +8750,10 @@ spec: rule: '!has(self.controlPlane.service.allocateLoadBalancerNodePorts) || self.controlPlane.service.serviceType == ''LoadBalancer''' - message: LoadBalancerClass cannot be set or unset at runtime rule: self.controlPlane.service.serviceType != 'LoadBalancer' || (oldSelf.controlPlane.service.serviceType != 'LoadBalancer' && self.controlPlane.service.serviceType == 'LoadBalancer') || has(self.networkProfile.loadBalancerClass) == has(oldSelf.networkProfile.loadBalancerClass) + - message: ipFamilies must contain at most one entry when ipFamilyPolicy is SingleStack + rule: '!has(self.controlPlane.service.ipFamilyPolicy) || self.controlPlane.service.ipFamilyPolicy != ''SingleStack'' || !has(self.controlPlane.service.ipFamilies) || size(self.controlPlane.service.ipFamilies) <= 1' + - message: ipFamilies entries must be unique + rule: '!has(self.controlPlane.service.ipFamilies) || size(self.controlPlane.service.ipFamilies) < 2 || self.controlPlane.service.ipFamilies[0] != self.controlPlane.service.ipFamilies[1]' status: description: TenantControlPlaneStatus defines the observed state of TenantControlPlane. properties: diff --git a/docs/content/reference/api.md b/docs/content/reference/api.md index 998dd77..29e9a46 100644 --- a/docs/content/reference/api.md +++ b/docs/content/reference/api.md @@ -31182,6 +31182,33 @@ valid when serviceType is LoadBalancer; setting it with any other serviceType is rejected by validation.
false + + ipFamilies + []enum + + IPFamilies maps directly to the generated Service's spec.ipFamilies. Order is +significant: the first entry is the primary family. When empty, the management +cluster default applies. A Service's IP families cannot be reduced or swapped +after creation (only a single-stack Service may be upgraded to dual-stack); +forbidden transitions are rejected by the API server and surface as a reconcile error.
+
+ Enum: IPv4, IPv6
+ + false + + ipFamilyPolicy + enum + + IPFamilyPolicy maps directly to the generated Service's spec.ipFamilyPolicy. +When nil, the management cluster default applies, preserving existing behaviour. +PreferDualStack and RequireDualStack describe a dual-stack Service and expect +two entries in ipFamilies; a RequireDualStack policy that cannot be satisfied +(for example with a single family) is rejected by the API server and surfaces +as a reconcile error.
+
+ Enum: SingleStack, PreferDualStack, RequireDualStack
+ + false diff --git a/internal/resources/k8s_service_resource.go b/internal/resources/k8s_service_resource.go index 217e0d3..183d1c5 100644 --- a/internal/resources/k8s_service_resource.go +++ b/internal/resources/k8s_service_resource.go @@ -130,6 +130,17 @@ func (r *KubernetesServiceResource) mutate(ctx context.Context, tenantControlPla r.resource.Spec.Ports = ports + // IP families are a pure passthrough to the native Service fields. Only + // write them when the user set them, so an unset spec leaves the + // API-server-defaulted values in place (no reconcile churn). + if policy := tenantControlPlane.Spec.ControlPlane.Service.IPFamilyPolicy; policy != nil { + r.resource.Spec.IPFamilyPolicy = policy + } + + if families := tenantControlPlane.Spec.ControlPlane.Service.IPFamilies; len(families) > 0 { + r.resource.Spec.IPFamilies = families + } + switch tenantControlPlane.Spec.ControlPlane.Service.ServiceType { case kamajiv1alpha1.ServiceTypeLoadBalancer: r.resource.Spec.Type = corev1.ServiceTypeLoadBalancer diff --git a/internal/resources/k8s_service_resource_test.go b/internal/resources/k8s_service_resource_test.go index c3399a6..2ad3aa1 100644 --- a/internal/resources/k8s_service_resource_test.go +++ b/internal/resources/k8s_service_resource_test.go @@ -201,3 +201,107 @@ var _ = Describe("KubernetesServiceResource AllocateLoadBalancerNodePorts", func Expect(svc.Spec.Ports[0].NodePort).To(Equal(seededNodePort)) }) }) + +var _ = Describe("KubernetesServiceResource IP families", func() { + var ( + ctx context.Context + tcp *kamajiv1alpha1.TenantControlPlane + ) + + const tcpName = "test-tcp-ipfamily" + + // existingClusterIPService mimics an already-reconciled ClusterIP Service. + existingClusterIPService := func() *corev1.Service { + return &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: tcpName, Namespace: "default"}, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + Ports: []corev1.ServicePort{{ + Name: "kube-apiserver", + Protocol: corev1.ProtocolTCP, + Port: 6443, + TargetPort: intstr.FromInt32(6443), + }}, + }, + } + } + + newResource := func(objs ...client.Object) *resources.KubernetesServiceResource { + fakeClient := fake.NewClientBuilder(). + WithScheme(runtimeScheme). + WithObjects(objs...). + Build() + + return &resources.KubernetesServiceResource{Client: fakeClient} + } + + BeforeEach(func() { + ctx = context.Background() + tcp = &kamajiv1alpha1.TenantControlPlane{ + ObjectMeta: metav1.ObjectMeta{Name: tcpName, Namespace: "default"}, + Spec: kamajiv1alpha1.TenantControlPlaneSpec{ + ControlPlane: kamajiv1alpha1.ControlPlane{ + Service: kamajiv1alpha1.ServiceSpec{ + ServiceType: kamajiv1alpha1.ServiceTypeClusterIP, + }, + }, + NetworkProfile: kamajiv1alpha1.NetworkProfileSpec{ + Port: 6443, + }, + }, + } + }) + + It("propagates ipFamilyPolicy and ipFamilies verbatim when set", func() { + tcp.Spec.ControlPlane.Service.IPFamilyPolicy = ptr.To(corev1.IPFamilyPolicyRequireDualStack) + tcp.Spec.ControlPlane.Service.IPFamilies = []corev1.IPFamily{corev1.IPv6Protocol, corev1.IPv4Protocol} + resource := newResource(existingClusterIPService()) + + Expect(resource.Define(ctx, tcp)).To(Succeed()) + _, err := resource.CreateOrUpdate(ctx, tcp) + Expect(err).NotTo(HaveOccurred()) + + svc := &corev1.Service{} + Expect(resource.Client.Get(ctx, client.ObjectKey{Name: tcp.Name, Namespace: tcp.Namespace}, svc)).To(Succeed()) + Expect(svc.Spec.IPFamilyPolicy).NotTo(BeNil()) + Expect(*svc.Spec.IPFamilyPolicy).To(Equal(corev1.IPFamilyPolicyRequireDualStack)) + Expect(svc.Spec.IPFamilies).To(Equal([]corev1.IPFamily{corev1.IPv6Protocol, corev1.IPv4Protocol})) + }) + + It("supports an IPv6-only single-stack service", func() { + tcp.Spec.ControlPlane.Service.IPFamilyPolicy = ptr.To(corev1.IPFamilyPolicySingleStack) + tcp.Spec.ControlPlane.Service.IPFamilies = []corev1.IPFamily{corev1.IPv6Protocol} + resource := newResource(existingClusterIPService()) + + Expect(resource.Define(ctx, tcp)).To(Succeed()) + _, err := resource.CreateOrUpdate(ctx, tcp) + Expect(err).NotTo(HaveOccurred()) + + svc := &corev1.Service{} + Expect(resource.Client.Get(ctx, client.ObjectKey{Name: tcp.Name, Namespace: tcp.Namespace}, svc)).To(Succeed()) + Expect(svc.Spec.IPFamilyPolicy).NotTo(BeNil()) + Expect(*svc.Spec.IPFamilyPolicy).To(Equal(corev1.IPFamilyPolicySingleStack)) + Expect(svc.Spec.IPFamilies).To(Equal([]corev1.IPFamily{corev1.IPv6Protocol})) + }) + + It("leaves the Service IP-family fields untouched when unset", func() { + // Both fields nil/empty on the TCP: the builder must not write them, so the + // API-server-defaulted values on the live Service survive (no churn, no fight). + tcp.Spec.ControlPlane.Service.IPFamilyPolicy = nil + tcp.Spec.ControlPlane.Service.IPFamilies = nil + existing := existingClusterIPService() + existing.Spec.IPFamilyPolicy = ptr.To(corev1.IPFamilyPolicySingleStack) // as the API server would default + existing.Spec.IPFamilies = []corev1.IPFamily{corev1.IPv4Protocol} + resource := newResource(existing) + + Expect(resource.Define(ctx, tcp)).To(Succeed()) + _, err := resource.CreateOrUpdate(ctx, tcp) + Expect(err).NotTo(HaveOccurred()) + + svc := &corev1.Service{} + Expect(resource.Client.Get(ctx, client.ObjectKey{Name: tcp.Name, Namespace: tcp.Namespace}, svc)).To(Succeed()) + Expect(svc.Spec.IPFamilyPolicy).NotTo(BeNil()) + Expect(*svc.Spec.IPFamilyPolicy).To(Equal(corev1.IPFamilyPolicySingleStack)) + Expect(svc.Spec.IPFamilies).To(Equal([]corev1.IPFamily{corev1.IPv4Protocol})) + }) +}) diff --git a/internal/resources/kubeadm_config.go b/internal/resources/kubeadm_config.go index 944860a..8c3687d 100644 --- a/internal/resources/kubeadm_config.go +++ b/internal/resources/kubeadm_config.go @@ -77,6 +77,41 @@ func (r *KubeadmConfigResource) UpdateTenantControlPlaneStatus(_ context.Context return nil } +// canonicalSAN normalises an IP SAN to its canonical net.IP string form so that +// the same address in different textual forms (notably IPv6) collapses to one key; +// non-IP entries (DNS names) pass through unchanged. +func canonicalSAN(s string) string { + if ip := net.ParseIP(s); ip != nil { + return ip.String() + } + + return s +} + +// mergeCertSANs appends the additional IPs (typically the control-plane Service IPs) +// to the existing cert SANs, skipping any whose canonical IP form already appears as +// the management address or among the existing SANs. This keeps a single-stack Service +// a no-op (its only IP equals the management address) and prevents duplicate SANs that +// would otherwise churn the certificate. +func mergeCertSANs(managementAddress string, certSANs, additional []string) []string { + seen := map[string]struct{}{canonicalSAN(managementAddress): {}} + for _, san := range certSANs { + seen[canonicalSAN(san)] = struct{}{} + } + + for _, ip := range additional { + key := canonicalSAN(ip) + if _, ok := seen[key]; ok { + continue + } + + seen[key] = struct{}{} + certSANs = append(certSANs, ip) + } + + return certSANs +} + func (r *KubeadmConfigResource) mutate(ctx context.Context, tenantControlPlane *kamajiv1alpha1.TenantControlPlane) controllerutil.MutateFn { return func() error { logger := log.FromContext(ctx, "resource", r.GetName()) @@ -111,12 +146,27 @@ func (r *KubeadmConfigResource) mutate(ctx context.Context, tenantControlPlane * } } - // Add advertise address to cert SANs if different from management address - certSANs := tenantControlPlane.Spec.NetworkProfile.CertSANs + // Add advertise address to cert SANs if different from management address. + // Copy the spec slice so appends below never mutate the user's CertSANs backing array. + certSANs := append([]string{}, tenantControlPlane.Spec.NetworkProfile.CertSANs...) if advAddress != address { - certSANs = append(append([]string{}, certSANs...), advAddress) + certSANs = append(certSANs, advAddress) } + // Add every IP the control-plane Service answers on (all ClusterIPs of a + // dual-stack Service and any LoadBalancer ingress IPs) so the API server + // certificate is valid over both families. Deduplicate against the management + // address and the SANs already present so a single-stack Service adds nothing + // and the certificate does not churn. + serviceIPs, svcErr := tenantControlPlane.ControlPlaneServiceIPs(ctx, r.Client) + if svcErr != nil { + logger.Error(svcErr, "cannot retrieve control plane Service IPs for cert SANs") + + return svcErr + } + + certSANs = mergeCertSANs(address, certSANs, serviceIPs) + // Backward compatibility for deprecated CIDR fields if tenantControlPlane.Spec.NetworkProfile.ServiceCIDR != "" { logger.Info("serviceCidr is deprecated, please migrate to serviceCidrs") diff --git a/internal/resources/kubeadm_config_internal_test.go b/internal/resources/kubeadm_config_internal_test.go new file mode 100644 index 0000000..968f604 --- /dev/null +++ b/internal/resources/kubeadm_config_internal_test.go @@ -0,0 +1,81 @@ +// Copyright 2022 Clastix Labs +// SPDX-License-Identifier: Apache-2.0 + +package resources + +import ( + "reflect" + "testing" +) + +func TestCanonicalSAN(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + {name: "IPv4 unchanged", in: "10.96.0.1", want: "10.96.0.1"}, + {name: "compressed IPv6 unchanged", in: "2001:db8::1", want: "2001:db8::1"}, + {name: "expanded IPv6 collapses to canonical form", in: "2001:db8:0:0:0:0:0:1", want: "2001:db8::1"}, + {name: "uppercase IPv6 lowercased", in: "2001:DB8::1", want: "2001:db8::1"}, + {name: "IPv4-mapped IPv6 collapses to IPv4", in: "::ffff:10.96.0.1", want: "10.96.0.1"}, + {name: "DNS name passes through", in: "tcp.default.svc", want: "tcp.default.svc"}, + {name: "non-IP string passes through", in: "localhost", want: "localhost"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := canonicalSAN(tt.in); got != tt.want { + t.Fatalf("canonicalSAN(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } +} + +func TestMergeCertSANs(t *testing.T) { + tests := []struct { + name string + management string + certSANs []string + additional []string + want []string + }{ + { + name: "single-stack adds nothing (service IP equals management address)", + management: "10.96.0.1", + certSANs: []string{"127.0.0.1", "localhost", "tcp.default.svc"}, + additional: []string{"10.96.0.1"}, + want: []string{"127.0.0.1", "localhost", "tcp.default.svc"}, + }, + { + name: "dual-stack appends the secondary family IP", + management: "10.96.0.1", + certSANs: []string{"127.0.0.1", "localhost"}, + additional: []string{"10.96.0.1", "2001:db8::1"}, + want: []string{"127.0.0.1", "localhost", "2001:db8::1"}, + }, + { + name: "secondary IP already present in a different textual form is not duplicated", + management: "10.96.0.1", + certSANs: []string{"2001:db8:0:0:0:0:0:1"}, + additional: []string{"10.96.0.1", "2001:db8::1"}, + want: []string{"2001:db8:0:0:0:0:0:1"}, + }, + { + name: "LoadBalancer ingress IPs are appended once", + management: "192.0.2.10", + certSANs: []string{"192.0.2.10"}, + additional: []string{"10.96.0.1", "2001:db8::1", "192.0.2.10"}, + want: []string{"192.0.2.10", "10.96.0.1", "2001:db8::1"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := mergeCertSANs(tt.management, tt.certSANs, tt.additional) + if !reflect.DeepEqual(got, tt.want) { + t.Fatalf("mergeCertSANs(%q, %v, %v) = %v, want %v", tt.management, tt.certSANs, tt.additional, got, tt.want) + } + }) + } +} diff --git a/internal/webhook/handlers/tcp_defaults.go b/internal/webhook/handlers/tcp_defaults.go index 71bbc31..0763f04 100644 --- a/internal/webhook/handlers/tcp_defaults.go +++ b/internal/webhook/handlers/tcp_defaults.go @@ -46,12 +46,11 @@ func (t TenantControlPlaneDefaults) OnCreate(object runtime.Object) AdmissionRes return nil, fmt.Errorf("cannot define resulting DNS Service IP: %w", err) } - switch { - case ip.To4() != nil: - ip[len(ip)-1] += 10 - case ip.To16() != nil: - ip[len(ip)-1] += 16 - } + // Match kubeadm's DNS IP convention (the 10th address of the service + // subnet) for both families: 10.96.0.0/16 -> 10.96.0.10 and + // fd00::/120 -> fd00::a. Using a different offset for IPv6 would make + // the kubelet's cluster DNS disagree with the CoreDNS Service ClusterIP. + ip[len(ip)-1] += 10 dnsIPs = append(dnsIPs, ip.String()) } diff --git a/internal/webhook/handlers/tcp_defaults_test.go b/internal/webhook/handlers/tcp_defaults_test.go index e7a0b68..e837f30 100644 --- a/internal/webhook/handlers/tcp_defaults_test.go +++ b/internal/webhook/handlers/tcp_defaults_test.go @@ -140,7 +140,7 @@ var _ = Describe("TCP Defaulting Webhook", func() { Expect(err).ToNot(HaveOccurred()) Expect(ops).To(ContainElement( - jsonpatch.Operation{Operation: "add", Path: "/spec/networkProfile/dnsServiceIPs", Value: []interface{}{"10.96.0.10", "fd00::10"}}, + jsonpatch.Operation{Operation: "add", Path: "/spec/networkProfile/dnsServiceIPs", Value: []interface{}{"10.96.0.10", "fd00::a"}}, )) }) })