From 253514af62cbbd9ad175346a5db98f7b9510cc7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Amand?= Date: Wed, 15 Apr 2026 12:22:15 +0200 Subject: [PATCH 1/9] feat: Add LoadBalancer Create/Update methods to stackit client --- pkg/stackit/client/loadbalancing.go | 30 +++++++++- pkg/stackit/client/mock/loadbalancing_mock.go | 59 +++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/pkg/stackit/client/loadbalancing.go b/pkg/stackit/client/loadbalancing.go index 6b1f8212..46eb7b94 100644 --- a/pkg/stackit/client/loadbalancing.go +++ b/pkg/stackit/client/loadbalancing.go @@ -11,9 +11,14 @@ import ( ) type LoadBalancingClient interface { + ProjectID() string + ListLoadBalancers(ctx context.Context) ([]loadbalancer.LoadBalancer, error) DeleteLoadBalancer(ctx context.Context, lbName string) error GetLoadBalancer(ctx context.Context, id string) (*loadbalancer.LoadBalancer, error) + CreateLoadBalancer(ctx context.Context, payload loadbalancer.CreateLoadBalancerPayload) (*loadbalancer.LoadBalancer, error) + UpdateLoadBalancer(ctx context.Context, lbName string, payload loadbalancer.UpdateLoadBalancerPayload) (*loadbalancer.LoadBalancer, error) + UpdateLoadBalancerTargetPool(ctx context.Context, lbName, tpName string, payload loadbalancer.UpdateTargetPoolPayload) (*loadbalancer.TargetPool, error) } type loadBalancingClient struct { @@ -40,6 +45,10 @@ func NewLoadBalancingClient(_ context.Context, region string, endpoints stackitv }, nil } +func (l loadBalancingClient) ProjectID() string { + return l.projectID +} + func (l loadBalancingClient) ListLoadBalancers(ctx context.Context) ([]loadbalancer.LoadBalancer, error) { lbResponse, err := l.Client.ListLoadBalancers(ctx, l.projectID, l.region).Execute() if err != nil { @@ -54,5 +63,24 @@ func (l loadBalancingClient) DeleteLoadBalancer(ctx context.Context, lbName stri } func (l loadBalancingClient) GetLoadBalancer(ctx context.Context, lbName string) (*loadbalancer.LoadBalancer, error) { - return l.Client.GetLoadBalancer(ctx, l.projectID, l.region, lbName).Execute() + lb, err := l.Client.GetLoadBalancer(ctx, l.projectID, l.region, lbName).Execute() + if err != nil { + if IsNotFound(err) { + return nil, nil + } + return nil, err + } + return lb, nil +} + +func (l loadBalancingClient) CreateLoadBalancer(ctx context.Context, payload loadbalancer.CreateLoadBalancerPayload) (*loadbalancer.LoadBalancer, error) { + return l.Client.CreateLoadBalancer(ctx, l.projectID, l.region).CreateLoadBalancerPayload(payload).Execute() +} + +func (l loadBalancingClient) UpdateLoadBalancer(ctx context.Context, lbName string, payload loadbalancer.UpdateLoadBalancerPayload) (*loadbalancer.LoadBalancer, error) { + return l.Client.UpdateLoadBalancer(ctx, l.projectID, l.region, lbName).UpdateLoadBalancerPayload(payload).Execute() +} + +func (l loadBalancingClient) UpdateLoadBalancerTargetPool(ctx context.Context, lbName, tpName string, payload loadbalancer.UpdateTargetPoolPayload) (*loadbalancer.TargetPool, error) { + return l.Client.UpdateTargetPool(ctx, l.projectID, l.region, lbName, tpName).UpdateTargetPoolPayload(payload).Execute() } diff --git a/pkg/stackit/client/mock/loadbalancing_mock.go b/pkg/stackit/client/mock/loadbalancing_mock.go index 9dcfd1c2..ba8f4cc2 100644 --- a/pkg/stackit/client/mock/loadbalancing_mock.go +++ b/pkg/stackit/client/mock/loadbalancing_mock.go @@ -41,6 +41,21 @@ func (m *MockLoadBalancingClient) EXPECT() *MockLoadBalancingClientMockRecorder return m.recorder } +// CreateLoadBalancer mocks base method. +func (m *MockLoadBalancingClient) CreateLoadBalancer(ctx context.Context, payload v2api.CreateLoadBalancerPayload) (*v2api.LoadBalancer, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateLoadBalancer", ctx, payload) + ret0, _ := ret[0].(*v2api.LoadBalancer) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CreateLoadBalancer indicates an expected call of CreateLoadBalancer. +func (mr *MockLoadBalancingClientMockRecorder) CreateLoadBalancer(ctx, payload any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateLoadBalancer", reflect.TypeOf((*MockLoadBalancingClient)(nil).CreateLoadBalancer), ctx, payload) +} + // DeleteLoadBalancer mocks base method. func (m *MockLoadBalancingClient) DeleteLoadBalancer(ctx context.Context, lbName string) error { m.ctrl.T.Helper() @@ -84,3 +99,47 @@ func (mr *MockLoadBalancingClientMockRecorder) ListLoadBalancers(ctx any) *gomoc mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListLoadBalancers", reflect.TypeOf((*MockLoadBalancingClient)(nil).ListLoadBalancers), ctx) } + +// ProjectID mocks base method. +func (m *MockLoadBalancingClient) ProjectID() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ProjectID") + ret0, _ := ret[0].(string) + return ret0 +} + +// ProjectID indicates an expected call of ProjectID. +func (mr *MockLoadBalancingClientMockRecorder) ProjectID() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ProjectID", reflect.TypeOf((*MockLoadBalancingClient)(nil).ProjectID)) +} + +// UpdateLoadBalancer mocks base method. +func (m *MockLoadBalancingClient) UpdateLoadBalancer(ctx context.Context, lbName string, payload v2api.UpdateLoadBalancerPayload) (*v2api.LoadBalancer, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateLoadBalancer", ctx, lbName, payload) + ret0, _ := ret[0].(*v2api.LoadBalancer) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateLoadBalancer indicates an expected call of UpdateLoadBalancer. +func (mr *MockLoadBalancingClientMockRecorder) UpdateLoadBalancer(ctx, lbName, payload any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateLoadBalancer", reflect.TypeOf((*MockLoadBalancingClient)(nil).UpdateLoadBalancer), ctx, lbName, payload) +} + +// UpdateLoadBalancerTargetPool mocks base method. +func (m *MockLoadBalancingClient) UpdateLoadBalancerTargetPool(ctx context.Context, lbName, tpName string, payload v2api.UpdateTargetPoolPayload) (*v2api.TargetPool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateLoadBalancerTargetPool", ctx, lbName, tpName, payload) + ret0, _ := ret[0].(*v2api.TargetPool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateLoadBalancerTargetPool indicates an expected call of UpdateLoadBalancerTargetPool. +func (mr *MockLoadBalancingClientMockRecorder) UpdateLoadBalancerTargetPool(ctx, lbName, tpName, payload any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateLoadBalancerTargetPool", reflect.TypeOf((*MockLoadBalancingClient)(nil).UpdateLoadBalancerTargetPool), ctx, lbName, tpName, payload) +} From 0a2022dab152b490f9b78d62afeb153396f5ccc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Amand?= Date: Wed, 15 Apr 2026 12:52:34 +0200 Subject: [PATCH 2/9] feat: Add SelfHostedShootExposure controller (GEP-36) --- .../templates/deployment.yaml | 1 + .../templates/rbac.yaml | 2 + .../values.yaml | 2 + .../app/app.go | 9 + docs/cloudprovider.md | 14 +- hack/api-reference/api.md | 72 ++++ pkg/apis/stackit/v1alpha1/register.go | 1 + .../v1alpha1/types_selfhostedshootexposure.go | 25 ++ .../stackit/v1alpha1/zz_generated.deepcopy.go | 51 +++ pkg/cmd/options.go | 3 + .../selfhostedshootexposure/actuator.go | 126 ++++++ pkg/controller/selfhostedshootexposure/add.go | 42 ++ .../selfhostedshootexposure/options.go | 103 +++++ .../selfhostedshootexposure/resources.go | 33 ++ .../resources_loadbalancer.go | 408 ++++++++++++++++++ 15 files changed, 885 insertions(+), 7 deletions(-) create mode 100644 pkg/apis/stackit/v1alpha1/types_selfhostedshootexposure.go create mode 100644 pkg/controller/selfhostedshootexposure/actuator.go create mode 100644 pkg/controller/selfhostedshootexposure/add.go create mode 100644 pkg/controller/selfhostedshootexposure/options.go create mode 100644 pkg/controller/selfhostedshootexposure/resources.go create mode 100644 pkg/controller/selfhostedshootexposure/resources_loadbalancer.go diff --git a/charts/gardener-extension-provider-stackit/templates/deployment.yaml b/charts/gardener-extension-provider-stackit/templates/deployment.yaml index e5d5c02e..37e91247 100644 --- a/charts/gardener-extension-provider-stackit/templates/deployment.yaml +++ b/charts/gardener-extension-provider-stackit/templates/deployment.yaml @@ -66,6 +66,7 @@ spec: - --heartbeat-namespace={{ .Release.Namespace }} - --heartbeat-renew-interval-seconds={{ .Values.controllers.heartbeat.renewIntervalSeconds }} - --infrastructure-max-concurrent-reconciles={{ .Values.controllers.infrastructure.concurrentSyncs }} + - --selfhostedshootexposure-max-concurrent-reconciles={{ .Values.controllers.selfhostedshootexposure.concurrentSyncs }} - --ignore-operation-annotation={{ .Values.controllers.ignoreOperationAnnotation }} - --worker-max-concurrent-reconciles={{ .Values.controllers.worker.concurrentSyncs }} - --webhook-config-namespace={{ .Release.Namespace }} diff --git a/charts/gardener-extension-provider-stackit/templates/rbac.yaml b/charts/gardener-extension-provider-stackit/templates/rbac.yaml index 16d82dd5..ae32f725 100644 --- a/charts/gardener-extension-provider-stackit/templates/rbac.yaml +++ b/charts/gardener-extension-provider-stackit/templates/rbac.yaml @@ -23,6 +23,8 @@ rules: - dnsrecords/status - infrastructures - infrastructures/status + - selfhostedshootexposures + - selfhostedshootexposures/status - workers - workers/status verbs: diff --git a/charts/gardener-extension-provider-stackit/values.yaml b/charts/gardener-extension-provider-stackit/values.yaml index 59575d99..f72ae03f 100644 --- a/charts/gardener-extension-provider-stackit/values.yaml +++ b/charts/gardener-extension-provider-stackit/values.yaml @@ -29,6 +29,8 @@ controllers: renewIntervalSeconds: 30 infrastructure: concurrentSyncs: 5 + selfhostedshootexposure: + concurrentSyncs: 5 worker: concurrentSyncs: 5 ignoreOperationAnnotation: false diff --git a/cmd/gardener-extension-provider-stackit/app/app.go b/cmd/gardener-extension-provider-stackit/app/app.go index c0b5b5d2..9134f404 100644 --- a/cmd/gardener-extension-provider-stackit/app/app.go +++ b/cmd/gardener-extension-provider-stackit/app/app.go @@ -35,6 +35,7 @@ import ( stackitdnsrecord "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/controller/dnsrecord" "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/controller/healthcheck" "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/controller/infrastructure" + stackitselfhostedshootexposure "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/controller/selfhostedshootexposure" stackitworker "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/controller/worker" "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/feature" "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit" @@ -92,6 +93,11 @@ func NewControllerManagerCommand(ctx context.Context) *cobra.Command { MaxConcurrentReconciles: 5, } + // options for the selfhostedshootexposure controller + selfHostedShootExposureCtrlOpts = &controllercmd.ControllerOptions{ + MaxConcurrentReconciles: 5, + } + // options for the worker controller workerCtrlOpts = &controllercmd.ControllerOptions{ MaxConcurrentReconciles: 5, @@ -121,6 +127,7 @@ func NewControllerManagerCommand(ctx context.Context) *cobra.Command { controllercmd.PrefixOption("controlplane-", controlPlaneCtrlOpts), controllercmd.PrefixOption("dnsrecord-", dnsRecordCtrlOpts), controllercmd.PrefixOption("infrastructure-", infraCtrlOpts), + controllercmd.PrefixOption("selfhostedshootexposure-", selfHostedShootExposureCtrlOpts), controllercmd.PrefixOption("worker-", workerCtrlOpts), controllercmd.PrefixOption("healthcheck-", healthCheckCtrlOpts), controllercmd.PrefixOption("heartbeat-", heartbeatCtrlOpts), @@ -197,12 +204,14 @@ func NewControllerManagerCommand(ctx context.Context) *cobra.Command { heartbeatCtrlOpts.Completed().Apply(&heartbeat.DefaultAddOptions) configFileOpts.Completed().ApplyCustomLabelDomain(&infrastructure.DefaultAddOptions.CustomLabelDomain) infraCtrlOpts.Completed().Apply(&infrastructure.DefaultAddOptions.Controller) + selfHostedShootExposureCtrlOpts.Completed().Apply(&stackitselfhostedshootexposure.DefaultAddOptions.Controller) workerCtrlOpts.Completed().Apply(&stackitworker.DefaultAddOptions.Controller) reconcileOpts.Completed().Apply(&stackitbastion.DefaultAddOptions.IgnoreOperationAnnotation, &stackitbastion.DefaultAddOptions.ExtensionClasses) reconcileOpts.Completed().Apply(&stackitcontrolplane.DefaultAddOptions.IgnoreOperationAnnotation, &stackitcontrolplane.DefaultAddOptions.ExtensionClasses) reconcileOpts.Completed().Apply(&stackitdnsrecord.DefaultAddOptions.IgnoreOperationAnnotation, &stackitdnsrecord.DefaultAddOptions.ExtensionClasses) reconcileOpts.Completed().Apply(&infrastructure.DefaultAddOptions.IgnoreOperationAnnotation, &infrastructure.DefaultAddOptions.ExtensionClasses) + reconcileOpts.Completed().Apply(&stackitselfhostedshootexposure.DefaultAddOptions.IgnoreOperationAnnotation, &stackitselfhostedshootexposure.DefaultAddOptions.ExtensionClasses) reconcileOpts.Completed().Apply(&stackitworker.DefaultAddOptions.IgnoreOperationAnnotation, &stackitworker.DefaultAddOptions.ExtensionClasses) stackitworker.DefaultAddOptions.GardenCluster = gardenCluster diff --git a/docs/cloudprovider.md b/docs/cloudprovider.md index 02017bee..0a5e13c6 100644 --- a/docs/cloudprovider.md +++ b/docs/cloudprovider.md @@ -27,13 +27,13 @@ stringData: The service account needs the following permissions: -| Permission | Purpose | -| ------------------------------ | ------------------------------------------------ | -| `nlb.admin` | CCM service-controller and network load balancer | -| `blockstorage.admin` | CSI driver | -| `compute.admin` | CCM node-controller and MCM | -| `iaas.network.admin` | bastion and infrastructure controller | -| `iaas.isoplated-network.admin` | infrastructure controller | +| Permission | Purpose | +| ------------------------------ | --------------------------------------------------------------------------------------- | +| `nlb.admin` | CCM service-controller, network load balancer and self-hosted shoot exposure controller | +| `blockstorage.admin` | CSI driver | +| `compute.admin` | CCM node-controller and MCM | +| `iaas.network.admin` | bastion and infrastructure controller | +| `iaas.isoplated-network.admin` | infrastructure controller | ## CloudProfileConfig Fields diff --git a/hack/api-reference/api.md b/hack/api-reference/api.md index 1d30d87d..6559139e 100644 --- a/hack/api-reference/api.md +++ b/hack/api-reference/api.md @@ -971,6 +971,44 @@ string +

LoadBalancerConfig +

+ + +

+(Appears on:SelfHostedShootExposureConfig) +

+ +

+LoadBalancerConfig contains configuration for the load balancer. +

+ + + + + + + + + + + + + + + + +
FieldDescription
+planId
+ +string + +		 +(Optional) +

PlanId specifies the service plan (size) of the load balancer.
Currently supported plans are p10, p50, p250, p750 (compare API docs).

+
+ +

MachineImage

@@ -1686,6 +1724,40 @@ string +

SelfHostedShootExposureConfig +

+ + +

+SelfHostedShootExposureConfig contains configuration settings for exposing self-hosted shoots. +

+ + + + + + + + + + + + + + + + +
FieldDescription
+loadBalancer
+ +LoadBalancerConfig + +		 +(Optional) +

LoadBalancer contains configuration for the load balancer.

+
+ +

ServerGroupDependency

diff --git a/pkg/apis/stackit/v1alpha1/register.go b/pkg/apis/stackit/v1alpha1/register.go index 0007e25b..eff8ac87 100644 --- a/pkg/apis/stackit/v1alpha1/register.go +++ b/pkg/apis/stackit/v1alpha1/register.go @@ -45,6 +45,7 @@ func addKnownTypes(scheme *runtime.Scheme) error { &ControlPlaneConfig{}, &WorkerStatus{}, &WorkerConfig{}, + &SelfHostedShootExposureConfig{}, ) return nil } diff --git a/pkg/apis/stackit/v1alpha1/types_selfhostedshootexposure.go b/pkg/apis/stackit/v1alpha1/types_selfhostedshootexposure.go new file mode 100644 index 00000000..269f2639 --- /dev/null +++ b/pkg/apis/stackit/v1alpha1/types_selfhostedshootexposure.go @@ -0,0 +1,25 @@ +package v1alpha1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// +genclient +// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object + +// SelfHostedShootExposureConfig contains configuration settings for exposing self-hosted shoots. +type SelfHostedShootExposureConfig struct { + metav1.TypeMeta `json:",inline"` + + // LoadBalancer contains configuration for the load balancer. + // +optional + LoadBalancer *LoadBalancerConfig `json:"loadBalancer,omitempty"` +} + +// LoadBalancerConfig contains configuration for the load balancer. +type LoadBalancerConfig struct { + // PlanId specifies the service plan (size) of the load balancer. + // Currently supported plans are p10, p50, p250, p750 (compare API docs). + // +optional + PlanId *string `json:"planId,omitempty"` +} diff --git a/pkg/apis/stackit/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/stackit/v1alpha1/zz_generated.deepcopy.go index 501a3d1c..e37e68ef 100644 --- a/pkg/apis/stackit/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/stackit/v1alpha1/zz_generated.deepcopy.go @@ -478,6 +478,27 @@ func (in *KeyStoneURL) DeepCopy() *KeyStoneURL { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *LoadBalancerConfig) DeepCopyInto(out *LoadBalancerConfig) { + *out = *in + if in.PlanId != nil { + in, out := &in.PlanId, &out.PlanId + *out = new(string) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LoadBalancerConfig. +func (in *LoadBalancerConfig) DeepCopy() *LoadBalancerConfig { + if in == nil { + return nil + } + out := new(LoadBalancerConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MachineImage) DeepCopyInto(out *MachineImage) { *out = *in @@ -726,6 +747,36 @@ func (in *SecurityGroup) DeepCopy() *SecurityGroup { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SelfHostedShootExposureConfig) DeepCopyInto(out *SelfHostedShootExposureConfig) { + *out = *in + out.TypeMeta = in.TypeMeta + if in.LoadBalancer != nil { + in, out := &in.LoadBalancer, &out.LoadBalancer + *out = new(LoadBalancerConfig) + (*in).DeepCopyInto(*out) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SelfHostedShootExposureConfig. +func (in *SelfHostedShootExposureConfig) DeepCopy() *SelfHostedShootExposureConfig { + if in == nil { + return nil + } + out := new(SelfHostedShootExposureConfig) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *SelfHostedShootExposureConfig) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ServerGroupDependency) DeepCopyInto(out *ServerGroupDependency) { *out = *in diff --git a/pkg/cmd/options.go b/pkg/cmd/options.go index 25e0259a..b9cd8ba3 100644 --- a/pkg/cmd/options.go +++ b/pkg/cmd/options.go @@ -12,6 +12,7 @@ import ( extensionshealthcheckcontroller "github.com/gardener/gardener/extensions/pkg/controller/healthcheck" extensionsheartbeatcontroller "github.com/gardener/gardener/extensions/pkg/controller/heartbeat" extensionsinfrastructurecontroller "github.com/gardener/gardener/extensions/pkg/controller/infrastructure" + extensionsselfhostedshootcontroller "github.com/gardener/gardener/extensions/pkg/controller/selfhostedshootexposure" extensionsworkercontroller "github.com/gardener/gardener/extensions/pkg/controller/worker" extensionscloudproviderwebhook "github.com/gardener/gardener/extensions/pkg/webhook/cloudprovider" webhookcmd "github.com/gardener/gardener/extensions/pkg/webhook/cmd" @@ -22,6 +23,7 @@ import ( dnsrecordcontroller "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/controller/dnsrecord" healthcheckcontroller "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/controller/healthcheck" infrastructurecontroller "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/controller/infrastructure" + selfhostedshootexposurecontroller "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/controller/selfhostedshootexposure" workercontroller "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/controller/worker" cloudproviderwebhook "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/webhook/cloudprovider" controlplanewebhook "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/webhook/controlplane" @@ -35,6 +37,7 @@ func ControllerSwitchOptions() *controllercmd.SwitchOptions { controllercmd.Switch(extensionscontrolplanecontroller.ControllerName, controlplanecontroller.AddToManager), controllercmd.Switch(extensionsdnsrecordcontroller.ControllerName, dnsrecordcontroller.AddToManager), controllercmd.Switch(extensionsinfrastructurecontroller.ControllerName, infrastructurecontroller.AddToManager), + controllercmd.Switch(extensionsselfhostedshootcontroller.ControllerName, selfhostedshootexposurecontroller.AddToManager), controllercmd.Switch(extensionsworkercontroller.ControllerName, workercontroller.AddToManager), controllercmd.Switch(extensionshealthcheckcontroller.ControllerName, healthcheckcontroller.AddToManager), controllercmd.Switch(extensionsheartbeatcontroller.ControllerName, extensionsheartbeatcontroller.AddToManager), diff --git a/pkg/controller/selfhostedshootexposure/actuator.go b/pkg/controller/selfhostedshootexposure/actuator.go new file mode 100644 index 00000000..e7705f6d --- /dev/null +++ b/pkg/controller/selfhostedshootexposure/actuator.go @@ -0,0 +1,126 @@ +package selfhostedshootexposure + +import ( + "context" + "fmt" + + extensionscontroller "github.com/gardener/gardener/extensions/pkg/controller" + "github.com/gardener/gardener/extensions/pkg/util" + v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants" + extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/serializer" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/manager" + + "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/apis/stackit/helper" + "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit" + stackitclient "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit/client" +) + +type Actuator struct { + Client client.Client + Decoder runtime.Decoder +} + +func (a *Actuator) WithManager(mgr manager.Manager) *Actuator { + if a.Client == nil { + a.Client = mgr.GetClient() + } + if a.Decoder == nil { + a.Decoder = serializer.NewCodecFactory(a.Client.Scheme(), serializer.EnableStrict).UniversalDecoder() + } + + return a +} + +func (a *Actuator) Reconcile(ctx context.Context, log logr.Logger, exposure *extensionsv1alpha1.SelfHostedShootExposure, cluster *extensionscontroller.Cluster) ([]corev1.LoadBalancerIngress, error) { + ingresses, err := a.reconcile(ctx, log, exposure, cluster) + return ingresses, util.DetermineError(err, helper.KnownCodes) +} + +func (a *Actuator) reconcile(ctx context.Context, log logr.Logger, exposure *extensionsv1alpha1.SelfHostedShootExposure, cluster *extensionscontroller.Cluster) ([]corev1.LoadBalancerIngress, error) { + r, err := a.getResources(ctx, log, exposure, cluster) + if err != nil { + return nil, err + } + + if err := r.reconcileLoadBalancer(ctx, log); err != nil { + return nil, fmt.Errorf("error reconciling load balancer: %w", err) + } + + if err := r.checkLoadBalancerReady(log); err != nil { + return nil, err + } + + return []corev1.LoadBalancerIngress{ + {IP: *r.LoadBalancer.ExternalAddress}, + }, nil +} + +func (a *Actuator) Delete(ctx context.Context, log logr.Logger, exposure *extensionsv1alpha1.SelfHostedShootExposure, cluster *extensionscontroller.Cluster) error { + return util.DetermineError(a.delete(ctx, log, exposure, cluster), helper.KnownCodes) +} + +func (a *Actuator) delete(ctx context.Context, log logr.Logger, exposure *extensionsv1alpha1.SelfHostedShootExposure, cluster *extensionscontroller.Cluster) error { + r, err := a.getResources(ctx, log, exposure, cluster) + if err != nil { + return err + } + + if err := r.deleteLoadBalancer(ctx, log); err != nil { + return fmt.Errorf("error deleting loadbalancer: %w", err) + } + + return nil +} + +func (a *Actuator) ForceDelete(ctx context.Context, log logr.Logger, exposure *extensionsv1alpha1.SelfHostedShootExposure, cluster *extensionscontroller.Cluster) error { + return a.Delete(ctx, log, exposure, cluster) +} + +// getResources initializes Resources and Options for the given SelfHostedShootExposure, needed for reconciliation/deletion. +func (a *Actuator) getResources(ctx context.Context, log logr.Logger, exposure *extensionsv1alpha1.SelfHostedShootExposure, cluster *extensionscontroller.Cluster) (*Resources, error) { + region := stackit.DetermineRegion(cluster) + + // Determine which secret to use for credentials. + // Support explicit CredentialsRef (required by GEP-0036), with fallback to default cloud-provider secret. + // TODO(jamand): Support WorkloadIdentity once integrated into SKE. + // For now, we only support secret-based credentials via ObjectReference pointing to a Secret. + var secretRef corev1.SecretReference + if exposure.Spec.CredentialsRef != nil { + // Use the explicitly provided ObjectReference (must point to a Secret for now) + secretRef = corev1.SecretReference{ + Name: exposure.Spec.CredentialsRef.Name, + Namespace: exposure.Spec.CredentialsRef.Namespace, + } + } else { + // Fall back to the default cloud-provider secret + secretRef = corev1.SecretReference{ + Name: v1beta1constants.SecretNameCloudProvider, + Namespace: exposure.Namespace, + } + } + + lbClient, err := stackitclient.New(region, cluster).LoadBalancing(ctx, a.Client, secretRef) + if err != nil { + return nil, fmt.Errorf("error creating LoadBalancer client: %w", err) + } + + opts, err := a.DetermineOptions(ctx, exposure, cluster, lbClient.ProjectID()) + if err != nil { + return nil, fmt.Errorf("error determining options: %w", err) + } + + r := &Resources{ + Options: *opts, + LBClient: lbClient, + } + if err := r.getExistingResources(ctx, log); err != nil { + return nil, fmt.Errorf("error getting existing resources: %w", err) + } + + return r, nil +} diff --git a/pkg/controller/selfhostedshootexposure/add.go b/pkg/controller/selfhostedshootexposure/add.go new file mode 100644 index 00000000..3f25ac4d --- /dev/null +++ b/pkg/controller/selfhostedshootexposure/add.go @@ -0,0 +1,42 @@ +package selfhostedshootexposure + +import ( + "context" + + "github.com/gardener/gardener/extensions/pkg/controller/selfhostedshootexposure" + extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/manager" + + "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit" +) + +// DefaultAddOptions are the default AddOptions for AddToManager. +var DefaultAddOptions = AddOptions{} + +// AddOptions are Options to apply when adding the SelfHostedShootExposure controller to the manager. +type AddOptions struct { + // Controller are the controller.Options. + Controller controller.Options + // IgnoreOperationAnnotation specifies whether to ignore the operation annotation or not. + IgnoreOperationAnnotation bool + // ExtensionClasses defines the extension class this extension is responsible for. + ExtensionClasses []extensionsv1alpha1.ExtensionClass +} + +// AddToManagerWithOptions adds a controller with the given Options to the given manager. +// The opts.Reconciler is being set with a newly instantiated Actuator. +func AddToManagerWithOptions(mgr manager.Manager, opts AddOptions) error { + return selfhostedshootexposure.Add(mgr, selfhostedshootexposure.AddArgs{ + Actuator: (&Actuator{}).WithManager(mgr), + ControllerOptions: opts.Controller, + Predicates: selfhostedshootexposure.DefaultPredicates(opts.IgnoreOperationAnnotation), + Type: stackit.Type, + ExtensionClasses: opts.ExtensionClasses, + }) +} + +// AddToManager adds a controller with the default Options. +func AddToManager(_ context.Context, mgr manager.Manager) error { + return AddToManagerWithOptions(mgr, DefaultAddOptions) +} diff --git a/pkg/controller/selfhostedshootexposure/options.go b/pkg/controller/selfhostedshootexposure/options.go new file mode 100644 index 00000000..7c4af9b5 --- /dev/null +++ b/pkg/controller/selfhostedshootexposure/options.go @@ -0,0 +1,103 @@ +package selfhostedshootexposure + +import ( + "context" + "fmt" + "time" + + extensionscontroller "github.com/gardener/gardener/extensions/pkg/controller" + extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" + reconcilerutils "github.com/gardener/gardener/pkg/controllerutils/reconciler" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/apis/stackit/helper" + stackitv1alpha1 "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/apis/stackit/v1alpha1" + "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/controller/controlplane" + "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit" +) + +const ( + ExposureLabelKey = "exposure.stackit.cloud" +) + +// Options contains all input required for creating a STACKIT LB for a self-hosted shoot on STACKIT. +// The options are determined from the SelfHostedShootExposure and Cluster object. +type Options struct { + SelfHostedShootExposure *extensionsv1alpha1.SelfHostedShootExposure + + // ProjectID is the STACKIT project ID of the shoot. Currently determined from the cloudprovider (credentials) secret. + ProjectID string + // ResourceName of all STACKIT resources for this SelfHostedShootExposure. + ResourceName string + // Labels added to all STACKIT resources. + Labels map[string]string + + // Region for the LB, determined from Cluster.spec.shoot.spec.region (RegionOne is replaced with eu01). + Region string + // NetworkID is the ID of the network where the control plane nodes reside. + NetworkID string + // PlanId specifies the service plan (size) of the load balancer. + PlanId string +} + +func (a *Actuator) DetermineOptions(ctx context.Context, exposure *extensionsv1alpha1.SelfHostedShootExposure, cluster *extensionscontroller.Cluster, projectID string) (*Options, error) { + opts := &Options{ + SelfHostedShootExposure: exposure, + ProjectID: projectID, + ResourceName: fmt.Sprintf("%s-exposure-%s", cluster.Shoot.Status.TechnicalID, exposure.Name), + // STACKIT LB labels do not allow '/' in keys, so we use the flat dot-separated form + // matching the convention used for other STACKIT LBs (see controlplane.STACKITLBClusterLabelKey). + Labels: map[string]string{ + controlplane.STACKITLBClusterLabelKey: cluster.Shoot.Status.TechnicalID, + ExposureLabelKey: exposure.Name, + }, + Region: stackit.DetermineRegion(cluster), + } + + // Get the network where the control plane resides + infraStatus, err := getInfrastructureStatus(ctx, a.Client, cluster) + if err != nil { + return nil, fmt.Errorf("error getting InfrastructureStatus: %w", err) + } + opts.NetworkID = infraStatus.Networks.ID + + // Decode providerConfig to extract STACKIT-specific settings + if exposure.Spec.ProviderConfig != nil { + providerConfig := &stackitv1alpha1.SelfHostedShootExposureConfig{} + if _, _, err := a.Decoder.Decode(exposure.Spec.ProviderConfig.Raw, nil, providerConfig); err != nil { + return nil, fmt.Errorf("error decoding providerConfig: %w", err) + } + if providerConfig.LoadBalancer != nil && providerConfig.LoadBalancer.PlanId != nil { + opts.PlanId = *providerConfig.LoadBalancer.PlanId + } + } + // Default plan if not specified + if opts.PlanId == "" { + opts.PlanId = "p10" + } + + return opts, nil +} + +func getInfrastructureStatus(ctx context.Context, c client.Client, cluster *extensionscontroller.Cluster) (*stackitv1alpha1.InfrastructureStatus, error) { + infra := &extensionsv1alpha1.Infrastructure{} + if err := c.Get(ctx, client.ObjectKey{Namespace: cluster.ObjectMeta.Name, Name: cluster.Shoot.Name}, infra); err != nil { + if apierrors.IsNotFound(err) { + // Infrastructure is reconciled before SelfHostedShootExposure; absence is a normal transient state on initial creation. + return nil, &reconcilerutils.RequeueAfterError{ + RequeueAfter: 30 * time.Second, + Cause: fmt.Errorf("waiting for Infrastructure resource to be created"), + } + } + return nil, fmt.Errorf("error getting infrastructure: %w", err) + } + if infra.Status.ProviderStatus == nil { + // Infrastructure exists but hasn't been reconciled yet — ProviderStatus (and thus the network ID) is not yet populated. + return nil, &reconcilerutils.RequeueAfterError{ + RequeueAfter: 30 * time.Second, + Cause: fmt.Errorf("waiting for Infrastructure status to be populated"), + } + } + return helper.InfrastructureStatusFromRaw(infra.Status.ProviderStatus) +} diff --git a/pkg/controller/selfhostedshootexposure/resources.go b/pkg/controller/selfhostedshootexposure/resources.go new file mode 100644 index 00000000..07ac64b6 --- /dev/null +++ b/pkg/controller/selfhostedshootexposure/resources.go @@ -0,0 +1,33 @@ +package selfhostedshootexposure + +import ( + "context" + "fmt" + + "github.com/go-logr/logr" + loadbalancer "github.com/stackitcloud/stackit-sdk-go/services/loadbalancer/v2api" + + stackitclient "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit/client" +) + +// Resources holds the STACKIT resources created for a Self-hosted Shoot Exposure +// along with all inputs (options) and the needed clients. +type Resources struct { + Options + LBClient stackitclient.LoadBalancingClient + + LoadBalancer *loadbalancer.LoadBalancer +} + +func (r *Resources) getExistingResources(ctx context.Context, log logr.Logger) error { + lb, err := r.LBClient.GetLoadBalancer(ctx, r.ResourceName) + if err != nil { + return fmt.Errorf("error getting load balancer: %w", err) + } + if lb != nil { + r.LoadBalancer = lb + log.V(1).Info("Found existing load balancer", "loadbalancer", r.LoadBalancer.GetName()) + } + + return nil +} diff --git a/pkg/controller/selfhostedshootexposure/resources_loadbalancer.go b/pkg/controller/selfhostedshootexposure/resources_loadbalancer.go new file mode 100644 index 00000000..c8245bb5 --- /dev/null +++ b/pkg/controller/selfhostedshootexposure/resources_loadbalancer.go @@ -0,0 +1,408 @@ +package selfhostedshootexposure + +import ( + "context" + "fmt" + "sort" + "strings" + "time" + + extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" + reconcilerutils "github.com/gardener/gardener/pkg/controllerutils/reconciler" + "github.com/go-logr/logr" + loadbalancer "github.com/stackitcloud/stackit-sdk-go/services/loadbalancer/v2api" + loadbalancerwait "github.com/stackitcloud/stackit-sdk-go/services/loadbalancer/v2api/wait" + corev1 "k8s.io/api/core/v1" + + stackitclient "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit/client" +) + +const ( + // lbNetworkRoleListenersAndTargets is the network role for listeners and targets in the load balancer network. + lbNetworkRoleListenersAndTargets = "ROLE_LISTENERS_AND_TARGETS" + // protocolTCP is the TCP protocol identifier for the listener. + protocolTCP = "PROTOCOL_TCP" + // listenerName is the (single) hardcoded listener name for exposing the control plane API server. + listenerName = "listener-control-plane" + // targetPoolName is the (single) hardcoded target pool name for control plane nodes. + targetPoolName = "target-pool-control-plane" +) + +// STACKIT LB error types reported in LoadBalancer.Errors[].Type. v2api weakened Type from a +// generated enum to *string (known openapi-generator limitation confirmed with the LB team); +// the authoritative set of values still lives as LOADBALANCERERRORTYPE_* constants in the +// deprecated top-level stackit-sdk-go/services/loadbalancer package. +const ( + // lbErrTypeTargetNotActive encodes that target may not be ready (yet). + lbErrTypeTargetNotActive = "TYPE_TARGET_NOT_ACTIVE" +) + +func (r *Resources) reconcileLoadBalancer(ctx context.Context, log logr.Logger) error { + targets, err := r.buildTargets() + if err != nil { + return err + } + + if r.LoadBalancer == nil { + return r.createLoadBalancer(ctx, log, targets) + } + + targetPoolNeedsUpdate, err := r.targetPoolNeedsUpdate(targets) + if err != nil { + return err + } + planNeedsUpdate := r.planNeedsUpdate() + + if !targetPoolNeedsUpdate && !planNeedsUpdate { + return nil + } + + // Fast path: only targets changed (e.g. control-plane node added/removed). The sub-resource + // endpoint is scoped to the target pool, so we avoid re-sending the full LB state. + if targetPoolNeedsUpdate && !planNeedsUpdate { + return r.updateTargetPool(ctx, log, targets) + } + + // Full-state update: STACKIT's UpdateLoadBalancer (PUT endpoint) + return r.updateLoadBalancer(ctx, log, targets) +} + +func (r *Resources) createLoadBalancer(ctx context.Context, log logr.Logger, targets []loadbalancer.Target) error { + if len(targets) == 0 { + // Endpoints are populated asynchronously by gardenlet from healthy control-plane nodes. + // Empty endpoints on first create is a normal transient state. + return &reconcilerutils.RequeueAfterError{ + RequeueAfter: 30 * time.Second, + Cause: fmt.Errorf("waiting for endpoints to be populated in spec"), + } + } + + log.V(1).Info("Creating load balancer", "loadBalancer", r.ResourceName, "networkID", r.NetworkID, "planID", r.PlanId) + createdLB, err := r.LBClient.CreateLoadBalancer(ctx, loadbalancer.CreateLoadBalancerPayload{ + Name: &r.ResourceName, + Labels: &r.Labels, + Networks: r.desiredNetworks(), + Listeners: r.desiredListeners(), + TargetPools: r.desiredTargetPools(targets), + PlanId: &r.PlanId, + Options: r.desiredOptions(), + }) + if err != nil { + return wrapLBAPIError("creating load balancer", err) + } + + r.LoadBalancer = createdLB + log.Info("Created load balancer", "loadBalancer", r.ResourceName) + return nil +} + +func (r *Resources) updateTargetPool(ctx context.Context, log logr.Logger, targets []loadbalancer.Target) error { + log.Info("Target pool needs updating", "loadBalancer", r.ResourceName) + _, err := r.LBClient.UpdateLoadBalancerTargetPool(ctx, + r.ResourceName, + targetPoolName, + loadbalancer.UpdateTargetPoolPayload{ + Name: new(targetPoolName), + TargetPort: &r.SelfHostedShootExposure.Spec.Port, + Targets: targets, + }) + if err != nil { + return wrapLBAPIError("updating load balancer target pool", err) + } + log.Info("Updated load balancer target pool", "loadBalancer", r.ResourceName) + + // Re-read the LB so downstream readiness checks don't see the pre-write status (STACKIT + // transitions the LB to STATUS_PENDING on any write). UpdateLoadBalancerTargetPool only + // returns the TargetPool, so a full GET is needed. + refreshed, err := r.LBClient.GetLoadBalancer(ctx, r.ResourceName) + if err != nil { + return fmt.Errorf("error refreshing load balancer after target pool update: %w", err) + } + r.LoadBalancer = refreshed + return nil +} + +func (r *Resources) updateLoadBalancer(ctx context.Context, log logr.Logger, targets []loadbalancer.Target) error { + log.Info("Load balancer needs updating", "loadBalancer", r.ResourceName, "newPlan", r.PlanId) + + // LB Endpoint only available as PUT, requires sending whole resource. + payload := loadbalancer.UpdateLoadBalancerPayload{ + Name: &r.ResourceName, + Version: r.LoadBalancer.Version, + ExternalAddress: r.LoadBalancer.ExternalAddress, + Labels: &r.Labels, + Networks: r.desiredNetworks(), + Listeners: r.desiredListeners(), + TargetPools: r.desiredTargetPools(targets), + PlanId: &r.PlanId, + Options: r.desiredOptions(), + } + + updated, err := r.LBClient.UpdateLoadBalancer(ctx, r.ResourceName, payload) + if err != nil { + return wrapLBAPIError("updating load balancer", err) + } + // Capture the post-write LB so readiness is evaluated against actual current status + r.LoadBalancer = updated + log.Info("Updated load balancer", "loadBalancer", r.ResourceName) + return nil +} + +// desiredNetworks returns the LB's network block. The control-plane exposure LB uses a single +// network acting as both listener and target network. +func (r *Resources) desiredNetworks() []loadbalancer.Network { + return []loadbalancer.Network{{ + NetworkId: &r.NetworkID, + Role: new(lbNetworkRoleListenersAndTargets), + }} +} + +// desiredListeners returns the single TCP listener that fronts the kube-apiserver. +func (r *Resources) desiredListeners() []loadbalancer.Listener { + return []loadbalancer.Listener{{ + DisplayName: new(listenerName), + Port: &r.SelfHostedShootExposure.Spec.Port, + Protocol: new(protocolTCP), + TargetPool: new(targetPoolName), + }} +} + +// desiredTargetPools returns the single target pool referenced by the listener. +func (r *Resources) desiredTargetPools(targets []loadbalancer.Target) []loadbalancer.TargetPool { + return []loadbalancer.TargetPool{{ + Name: new(targetPoolName), + TargetPort: &r.SelfHostedShootExposure.Spec.Port, + Targets: targets, + }} +} + +// desiredOptions returns the LB options. +// +// Initial call: ephemeral (provides external IP) +// Subsequent calls (PUT updates): provide the initially provided IP, do no set ephemeral to true (error!). +func (r *Resources) desiredOptions() *loadbalancer.LoadBalancerOptions { + opts := &loadbalancer.LoadBalancerOptions{} + if r.LoadBalancer == nil { + opts.EphemeralAddress = new(true) + } + return opts +} + +func (r *Resources) planNeedsUpdate() bool { + currentPlan := "" + if r.LoadBalancer != nil && r.LoadBalancer.PlanId != nil { + currentPlan = *r.LoadBalancer.PlanId + } + return currentPlan != r.PlanId +} + +// wrapLBAPIError classifies STACKIT LB API errors: 409 Conflicts are transient (another caller +// modified the LB between our GET and our write) and are retried via RequeueAfterError; anything +// else is returned as a regular error so Gardener can classify + surface it. +func wrapLBAPIError(op string, err error) error { + if stackitclient.IsConflict(err) { + return &reconcilerutils.RequeueAfterError{ + RequeueAfter: 15 * time.Second, + Cause: fmt.Errorf("load balancer is being modified while %s, retrying: %w", op, err), + } + } + return fmt.Errorf("error %s: %w", op, err) +} + +// checkLoadBalancerReady returns nil only if the LB is fully provisioned (STATUS_READY with an +// external VIP), otherwise RequeueAfterError. +// +// STATUS_ERROR is ambiguous: it may be transient (e.g. control-plane nodes not yet serving +// traffic during fresh shoot creation, reported as TYPE_TARGET_NOT_ACTIVE) or permanent. +func (r *Resources) checkLoadBalancerReady(log logr.Logger) error { + if r.LoadBalancer == nil || r.LoadBalancer.Status == nil { + return &reconcilerutils.RequeueAfterError{ + RequeueAfter: 15 * time.Second, + Cause: fmt.Errorf("waiting for load balancer status to be reported"), + } + } + + switch *r.LoadBalancer.Status { + case loadbalancerwait.LOADBALANCERSTATUS_READY: + if r.LoadBalancer.ExternalAddress == nil { + return &reconcilerutils.RequeueAfterError{ + RequeueAfter: 15 * time.Second, + Cause: fmt.Errorf("waiting for load balancer external address to be assigned"), + } + } + return nil + case loadbalancerwait.LOADBALANCERSTATUS_PENDING, loadbalancerwait.LOADBALANCERSTATUS_UNSPECIFIED: + return &reconcilerutils.RequeueAfterError{ + RequeueAfter: 15 * time.Second, + Cause: fmt.Errorf("waiting for load balancer to become ready (status=%s)", *r.LoadBalancer.Status), + } + case loadbalancerwait.LOADBALANCERSTATUS_ERROR: + if lbErrorsAllTransient(r.LoadBalancer.Errors) { + log.Info("Load balancer reports STATUS_ERROR with only transient errors, requeuing", + "loadBalancer", r.ResourceName, "errors", formatLBErrors(r.LoadBalancer.Errors)) + return &reconcilerutils.RequeueAfterError{ + RequeueAfter: 15 * time.Second, + Cause: fmt.Errorf("load balancer is in transient STATUS_ERROR: %s", formatLBErrors(r.LoadBalancer.Errors)), + } + } + return fmt.Errorf("load balancer is in unrecoverable state %s: %s", *r.LoadBalancer.Status, formatLBErrors(r.LoadBalancer.Errors)) + case loadbalancerwait.LOADBALANCERSTATUS_TERMINATING: + return fmt.Errorf("load balancer is in unrecoverable state %s: %s", *r.LoadBalancer.Status, formatLBErrors(r.LoadBalancer.Errors)) + default: + return fmt.Errorf("load balancer has unexpected status %s: %s", *r.LoadBalancer.Status, formatLBErrors(r.LoadBalancer.Errors)) + } +} + +// lbErrorsAllTransient reports whether every entry in errs is in the transient allowlist. +// An empty slice returns false: STATUS_ERROR without any diagnostics should not be silently +// swallowed as transient. +func lbErrorsAllTransient(errs []loadbalancer.LoadBalancerError) bool { + if len(errs) == 0 { + return false + } + for _, e := range errs { + if e.Type == nil { + return false + } + switch *e.Type { + case lbErrTypeTargetNotActive: + // transient + default: + return false + } + } + return true +} + +// formatLBErrors renders the LB's reported errors for inclusion in an error message. +func formatLBErrors(errs []loadbalancer.LoadBalancerError) string { + if len(errs) == 0 { + return "no error details reported" + } + parts := make([]string, 0, len(errs)) + for _, e := range errs { + t, d := "", "" + if e.Type != nil { + t = *e.Type + } + if e.Description != nil { + d = *e.Description + } + parts = append(parts, fmt.Sprintf("%s: %s", t, d)) + } + return strings.Join(parts, "; ") +} + +func (r *Resources) deleteLoadBalancer(ctx context.Context, log logr.Logger) error { + if r.LoadBalancer == nil { + return nil + } + + if err := r.LBClient.DeleteLoadBalancer(ctx, r.ResourceName); err != nil { + return fmt.Errorf("error deleting load balancer: %w", err) + } + + log.Info("Deleted load balancer", "loadBalancer", r.ResourceName) + return nil +} + +// buildTargets creates a sorted list of load balancer targets from the endpoints in the spec. +// Targets are sorted by IP address for deterministic ordering. +func (r *Resources) buildTargets() ([]loadbalancer.Target, error) { + targets := make([]loadbalancer.Target, len(r.SelfHostedShootExposure.Spec.Endpoints)) + for i, endpoint := range r.SelfHostedShootExposure.Spec.Endpoints { + ip, err := extractInternalIP(&endpoint) + if err != nil { + return nil, err + } + targets[i] = loadbalancer.Target{ + DisplayName: &endpoint.NodeName, + Ip: &ip, + } + } + + // Sort targets by IP (primary) and DisplayName (secondary) for deterministic ordering + sort.Slice(targets, func(i, j int) bool { + if *targets[i].Ip != *targets[j].Ip { + return *targets[i].Ip < *targets[j].Ip + } + return *targets[i].DisplayName < *targets[j].DisplayName + }) + + return targets, nil +} + +// extractInternalIP finds and returns the internal IP address from an endpoint's addresses. +// This function requires InternalIP because the STACKIT LB only supposts IPs as target. +func extractInternalIP(endpoint *extensionsv1alpha1.ControlPlaneEndpoint) (string, error) { + for _, addr := range endpoint.Addresses { + if addr.Type == corev1.NodeInternalIP { + return addr.Address, nil + } + } + return "", fmt.Errorf("endpoint %s has no InternalIP address", endpoint.NodeName) +} + +// targetsEqual compares two target lists for equality. +// Both lists should be sorted by IP for correct comparison. +func targetsEqual(spec, lb []loadbalancer.Target) bool { + if len(spec) != len(lb) { + return false + } + + for i := range spec { + if spec[i].Ip == nil || lb[i].Ip == nil { + return false + } + if *spec[i].Ip != *lb[i].Ip { + return false + } + // Also verify the display name matches + if spec[i].DisplayName == nil || lb[i].DisplayName == nil { + return false + } + if *spec[i].DisplayName != *lb[i].DisplayName { + return false + } + } + return true +} + +// targetPoolNeedsUpdate checks if the target pool in the load balancer needs updating. +// specTargets should be pre-built to avoid double-building. +func (r *Resources) targetPoolNeedsUpdate(specTargets []loadbalancer.Target) (bool, error) { + // If no load balancer exists yet, no update needed (will be created fresh) + if r.LoadBalancer == nil { + return false, nil + } + + // If LB exists but has no target pools, check if spec has targets + if len(r.LoadBalancer.TargetPools) == 0 { + return len(specTargets) > 0, nil + } + + // Validate that the load balancer has the expected target pool + if r.LoadBalancer.TargetPools[0].Name == nil || *r.LoadBalancer.TargetPools[0].Name != targetPoolName { + actualName := "" + if r.LoadBalancer.TargetPools[0].Name != nil { + actualName = *r.LoadBalancer.TargetPools[0].Name + } + return false, fmt.Errorf("unexpected target pool name: expected %q, got %q", + targetPoolName, actualName) + } + + // Get targets from the first target pool and copy before sorting + lbTargets := make([]loadbalancer.Target, len(r.LoadBalancer.TargetPools[0].Targets)) + copy(lbTargets, r.LoadBalancer.TargetPools[0].Targets) + + // Sort the LB targets for comparison (same order as spec targets) + sort.Slice(lbTargets, func(i, j int) bool { + if *lbTargets[i].Ip != *lbTargets[j].Ip { + return *lbTargets[i].Ip < *lbTargets[j].Ip + } + return *lbTargets[i].DisplayName < *lbTargets[j].DisplayName + }) + + // Compare semantically (order-independent after sorting) + return !targetsEqual(specTargets, lbTargets), nil +} From 53ba00d76e289555b5b9cfa1ab328e0f3942f410 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Amand?= Date: Wed, 15 Apr 2026 13:10:59 +0200 Subject: [PATCH 3/9] test: Add unit tests for SelfHostedShootExposure controller --- .../selfhostedshootexposure/actuator_test.go | 100 +++ .../selfhostedshootexposure/options_test.go | 163 +++++ .../resources_loadbalancer_test.go | 636 ++++++++++++++++++ .../selfhostedshootexposure/resources_test.go | 79 +++ .../selfhostedshootexposure/suite_test.go | 13 + 5 files changed, 991 insertions(+) create mode 100644 pkg/controller/selfhostedshootexposure/actuator_test.go create mode 100644 pkg/controller/selfhostedshootexposure/options_test.go create mode 100644 pkg/controller/selfhostedshootexposure/resources_loadbalancer_test.go create mode 100644 pkg/controller/selfhostedshootexposure/resources_test.go create mode 100644 pkg/controller/selfhostedshootexposure/suite_test.go diff --git a/pkg/controller/selfhostedshootexposure/actuator_test.go b/pkg/controller/selfhostedshootexposure/actuator_test.go new file mode 100644 index 00000000..90be98da --- /dev/null +++ b/pkg/controller/selfhostedshootexposure/actuator_test.go @@ -0,0 +1,100 @@ +package selfhostedshootexposure_test + +import ( + "context" + + extensionscontroller "github.com/gardener/gardener/extensions/pkg/controller" + gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" + extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" + "github.com/go-logr/logr" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/controller/selfhostedshootexposure" +) + +var _ = Describe("Actuator", func() { + var ( + ctx context.Context + logger logr.Logger + actuator *selfhostedshootexposure.Actuator + ) + + BeforeEach(func() { + ctx = context.Background() + logger = logr.Discard() + actuator = &selfhostedshootexposure.Actuator{ + Client: fake.NewClientBuilder().Build(), + } + }) + + Describe("#Reconcile", func() { + It("should return error when getResources fails (no client configured)", func() { + exposure := &extensionsv1alpha1.SelfHostedShootExposure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "kube-system", + }, + Spec: extensionsv1alpha1.SelfHostedShootExposureSpec{ + DefaultSpec: extensionsv1alpha1.DefaultSpec{ + Type: "stackit", + }, + Port: 443, + }, + } + cluster := &extensionscontroller.Cluster{ + Shoot: &gardencorev1beta1.Shoot{ + Spec: gardencorev1beta1.ShootSpec{Region: "eu01"}, + }, + } + + ingress, err := actuator.Reconcile(ctx, logger, exposure, cluster) + + Expect(err).To(HaveOccurred()) + Expect(ingress).To(BeNil()) + }) + }) + + Describe("#Delete", func() { + It("should return error when getResources fails (no client configured)", func() { + exposure := &extensionsv1alpha1.SelfHostedShootExposure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "kube-system", + }, + } + cluster := &extensionscontroller.Cluster{ + Shoot: &gardencorev1beta1.Shoot{ + Spec: gardencorev1beta1.ShootSpec{Region: "eu01"}, + }, + } + + err := actuator.Delete(ctx, logger, exposure, cluster) + + Expect(err).To(HaveOccurred()) + }) + }) + + Describe("#ForceDelete", func() { + It("should delegate to Delete", func() { + exposure := &extensionsv1alpha1.SelfHostedShootExposure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "kube-system", + }, + } + cluster := &extensionscontroller.Cluster{ + Shoot: &gardencorev1beta1.Shoot{ + Spec: gardencorev1beta1.ShootSpec{Region: "eu01"}, + }, + } + + err := actuator.ForceDelete(ctx, logger, exposure, cluster) + + // Should fail with same error as Delete (no client configured) + Expect(err).To(HaveOccurred()) + }) + }) +}) diff --git a/pkg/controller/selfhostedshootexposure/options_test.go b/pkg/controller/selfhostedshootexposure/options_test.go new file mode 100644 index 00000000..5020581e --- /dev/null +++ b/pkg/controller/selfhostedshootexposure/options_test.go @@ -0,0 +1,163 @@ +package selfhostedshootexposure_test + +import ( + "context" + + extensionscontroller "github.com/gardener/gardener/extensions/pkg/controller" + gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" + extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/serializer" + "k8s.io/apimachinery/pkg/runtime/serializer/json" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" + + stackitv1alpha1 "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/apis/stackit/v1alpha1" + . "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/controller/selfhostedshootexposure" +) + +var _ = Describe("Options", func() { + const ( + projectID = "garden-project-uuid" + ) + + var ( + ctx = context.Background() + + fakeClient client.Client + + a *Actuator + + exposure *extensionsv1alpha1.SelfHostedShootExposure + shoot *gardencorev1beta1.Shoot + cluster *extensionscontroller.Cluster + infraStatus *stackitv1alpha1.InfrastructureStatus + ) + + BeforeEach(func() { + scheme := runtime.NewScheme() + utilruntime.Must(extensionscontroller.AddToScheme(scheme)) + utilruntime.Must(stackitv1alpha1.AddToScheme(scheme)) + + fakeClient = fakeclient.NewClientBuilder().WithScheme(scheme).Build() + + a = &Actuator{ + Client: fakeClient, + Decoder: serializer.NewCodecFactory(fakeClient.Scheme(), serializer.EnableStrict).UniversalDecoder(), + } + + exposure = &extensionsv1alpha1.SelfHostedShootExposure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-exposure", + Namespace: "control-plane-namespace", + }, + Spec: extensionsv1alpha1.SelfHostedShootExposureSpec{ + DefaultSpec: extensionsv1alpha1.DefaultSpec{ + Type: "stackit", + }, + Port: 443, + }, + } + + shoot = &gardencorev1beta1.Shoot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hops", + Namespace: "garden", + }, + Spec: gardencorev1beta1.ShootSpec{ + Region: "eu01", + }, + Status: gardencorev1beta1.ShootStatus{ + TechnicalID: "shoot--garden--hops", + }, + } + + cluster = &extensionscontroller.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: exposure.Namespace, + }, + Shoot: shoot, + } + + infraStatus = &stackitv1alpha1.InfrastructureStatus{ + Networks: stackitv1alpha1.NetworkStatus{ + ID: "network-id", + }, + } + }) + + JustBeforeEach(func() { + encoder := serializer.NewCodecFactory(fakeClient.Scheme()).EncoderForVersion(&json.Serializer{}, stackitv1alpha1.SchemeGroupVersion) + + infraStatusBytes, err := runtime.Encode(encoder, infraStatus) + Expect(err).NotTo(HaveOccurred()) + infra := &extensionsv1alpha1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ + Name: shoot.Name, + Namespace: exposure.Namespace, + }, + Status: extensionsv1alpha1.InfrastructureStatus{ + DefaultStatus: extensionsv1alpha1.DefaultStatus{ + ProviderStatus: &runtime.RawExtension{Raw: infraStatusBytes}, + }, + }, + } + Expect(fakeClient.Create(ctx, infra)).To(Succeed()) + }) + + It("should correctly determine the options with default plan", func() { + opts, err := a.DetermineOptions(ctx, exposure, cluster, projectID) + + Expect(err).NotTo(HaveOccurred()) + Expect(opts).To(Equal(&Options{ + SelfHostedShootExposure: exposure, + ProjectID: projectID, + ResourceName: "shoot--garden--hops-exposure-test-exposure", + Labels: map[string]string{ + "cluster.stackit.cloud": "shoot--garden--hops", + "exposure.stackit.cloud": "test-exposure", + }, + Region: "eu01", + NetworkID: "network-id", + PlanId: "p10", + })) + }) + + It("should use PlanId from providerConfig", func() { + encoder := serializer.NewCodecFactory(fakeClient.Scheme()).EncoderForVersion(&json.Serializer{}, stackitv1alpha1.SchemeGroupVersion) + providerConfig := &stackitv1alpha1.SelfHostedShootExposureConfig{ + LoadBalancer: &stackitv1alpha1.LoadBalancerConfig{ + PlanId: new("p250"), + }, + } + providerConfigBytes, err := runtime.Encode(encoder, providerConfig) + Expect(err).NotTo(HaveOccurred()) + exposure.Spec.ProviderConfig = &runtime.RawExtension{Raw: providerConfigBytes} + + opts, err := a.DetermineOptions(ctx, exposure, cluster, projectID) + + Expect(err).NotTo(HaveOccurred()) + Expect(opts.PlanId).To(Equal("p250")) + }) + + It("should handle the RegionOne value", func() { + shoot.Spec.Region = "RegionOne" + + opts, err := a.DetermineOptions(ctx, exposure, cluster, projectID) + + Expect(err).NotTo(HaveOccurred()) + Expect(opts.Region).To(Equal("eu01")) + }) + + It("should set flat STACKIT LB label keys (no '/' — rejected by STACKIT LB API)", func() { + options, err := a.DetermineOptions(ctx, exposure, cluster, projectID) + Expect(err).NotTo(HaveOccurred()) + + Expect(options.Labels).To(HaveKeyWithValue("cluster.stackit.cloud", "shoot--garden--hops")) + Expect(options.Labels).To(HaveKeyWithValue("exposure.stackit.cloud", "test-exposure")) + }) +}) diff --git a/pkg/controller/selfhostedshootexposure/resources_loadbalancer_test.go b/pkg/controller/selfhostedshootexposure/resources_loadbalancer_test.go new file mode 100644 index 00000000..b79170b7 --- /dev/null +++ b/pkg/controller/selfhostedshootexposure/resources_loadbalancer_test.go @@ -0,0 +1,636 @@ +package selfhostedshootexposure + +import ( + "context" + "errors" + "fmt" + + extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" + reconcilerutils "github.com/gardener/gardener/pkg/controllerutils/reconciler" + "github.com/go-logr/logr" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + loadbalancer "github.com/stackitcloud/stackit-sdk-go/services/loadbalancer/v2api" + "go.uber.org/mock/gomock" + corev1 "k8s.io/api/core/v1" + + mock "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit/client/mock" +) + +var _ = Describe("reconcileLoadBalancer", func() { + var ( + ctx context.Context + logger logr.Logger + mockCtrl *gomock.Controller + mockLBClient *mock.MockLoadBalancingClient + r *Resources + ) + + BeforeEach(func() { + ctx = context.Background() + logger = logr.Discard() + mockCtrl = gomock.NewController(GinkgoT()) + mockLBClient = mock.NewMockLoadBalancingClient(mockCtrl) + r = &Resources{ + Options: Options{ + ResourceName: "test-lb", + Labels: map[string]string{"cluster": "shoot--foo--bar"}, + NetworkID: "network-123", + PlanId: "p10", + SelfHostedShootExposure: &extensionsv1alpha1.SelfHostedShootExposure{ + Spec: extensionsv1alpha1.SelfHostedShootExposureSpec{ + Port: 443, + }, + }, + }, + LBClient: mockLBClient, + LoadBalancer: nil, + } + }) + + AfterEach(func() { + mockCtrl.Finish() + }) + + Context("when no load balancer exists", func() { + It("should requeue creation without endpoints", func() { + r.SelfHostedShootExposure.Spec.Endpoints = []extensionsv1alpha1.ControlPlaneEndpoint{} + + err := r.reconcileLoadBalancer(ctx, logger) + + Expect(err).To(HaveOccurred()) + // Endpoints are populated asynchronously by gardenlet; empty endpoints should trigger a clean requeue, + // not a fatal error. + var rae *reconcilerutils.RequeueAfterError + Expect(errors.As(err, &rae)).To(BeTrue()) + Expect(rae.Cause.Error()).To(ContainSubstring("waiting for endpoints to be populated")) + }) + + It("should create a new load balancer", func() { + r.SelfHostedShootExposure.Spec.Endpoints = []extensionsv1alpha1.ControlPlaneEndpoint{ + { + NodeName: "node-1", + Addresses: []corev1.NodeAddress{ + {Type: corev1.NodeInternalIP, Address: "10.0.1.10"}, + }, + }, + { + NodeName: "node-2", + Addresses: []corev1.NodeAddress{ + {Type: corev1.NodeInternalIP, Address: "10.0.1.20"}, + }, + }, + } + + createdLB := &loadbalancer.LoadBalancer{ + Name: new("test-lb"), + ExternalAddress: new("203.0.113.1"), + } + mockLBClient.EXPECT(). + CreateLoadBalancer(ctx, gomock.Any()). + DoAndReturn(func(_ context.Context, payload loadbalancer.CreateLoadBalancerPayload) (*loadbalancer.LoadBalancer, error) { + // Verify the payload structure + Expect(payload.Name).To(Equal(new("test-lb"))) + Expect(payload.PlanId).To(Equal(new("p10"))) + Expect(payload.Networks).To(HaveLen(1)) + Expect(payload.Networks[0].NetworkId).To(Equal(new("network-123"))) + Expect(payload.Listeners).To(HaveLen(1)) + Expect(*payload.Listeners[0].Port).To(BeEquivalentTo(443)) + Expect(payload.TargetPools).To(HaveLen(1)) + Expect(*payload.TargetPools[0].Name).To(Equal("target-pool-control-plane")) + // Targets should be sorted by IP + Expect(payload.TargetPools[0].Targets).To(HaveLen(2)) + Expect(*payload.TargetPools[0].Targets[0].Ip).To(Equal("10.0.1.10")) + Expect(*payload.TargetPools[0].Targets[1].Ip).To(Equal("10.0.1.20")) + return createdLB, nil + }) + + err := r.reconcileLoadBalancer(ctx, logger) + + Expect(err).NotTo(HaveOccurred()) + Expect(r.LoadBalancer).To(Equal(createdLB)) + }) + + It("should return error when CreateLoadBalancer fails", func() { + r.SelfHostedShootExposure.Spec.Endpoints = []extensionsv1alpha1.ControlPlaneEndpoint{ + { + NodeName: "node-1", + Addresses: []corev1.NodeAddress{ + {Type: corev1.NodeInternalIP, Address: "10.0.1.10"}, + }, + }, + } + + mockLBClient.EXPECT(). + CreateLoadBalancer(ctx, gomock.Any()). + Return(nil, fmt.Errorf("API error")) + + err := r.reconcileLoadBalancer(ctx, logger) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("error creating load balancer")) + }) + }) + + Context("when load balancer already exists", func() { + BeforeEach(func() { + r.SelfHostedShootExposure.Spec.Endpoints = []extensionsv1alpha1.ControlPlaneEndpoint{ + { + NodeName: "node-1", + Addresses: []corev1.NodeAddress{ + {Type: corev1.NodeInternalIP, Address: "10.0.1.10"}, + }, + }, + } + r.LoadBalancer = &loadbalancer.LoadBalancer{ + Name: new("test-lb"), + PlanId: new("p10"), + Version: new("v1"), + TargetPools: []loadbalancer.TargetPool{ + { + Name: new("target-pool-control-plane"), + Targets: []loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, + }, + }, + }, + } + }) + + It("should do nothing if no updates are needed", func() { + // No mock expectations — nothing should be called + err := r.reconcileLoadBalancer(ctx, logger) + + Expect(err).NotTo(HaveOccurred()) + }) + + It("should update target pool only when targets changed", func() { + // Spec has a new node + r.SelfHostedShootExposure.Spec.Endpoints = []extensionsv1alpha1.ControlPlaneEndpoint{ + { + NodeName: "node-1", + Addresses: []corev1.NodeAddress{ + {Type: corev1.NodeInternalIP, Address: "10.0.1.10"}, + }, + }, + { + NodeName: "node-2", + Addresses: []corev1.NodeAddress{ + {Type: corev1.NodeInternalIP, Address: "10.0.1.20"}, + }, + }, + } + + mockLBClient.EXPECT(). + UpdateLoadBalancerTargetPool(ctx, "test-lb", "target-pool-control-plane", gomock.Any()). + DoAndReturn(func(_ context.Context, _, _ string, payload loadbalancer.UpdateTargetPoolPayload) (*loadbalancer.TargetPool, error) { + Expect(payload.Targets).To(HaveLen(2)) + Expect(*payload.Targets[0].Ip).To(Equal("10.0.1.10")) + Expect(*payload.Targets[1].Ip).To(Equal("10.0.1.20")) + Expect(*payload.TargetPort).To(BeEquivalentTo(443)) + return &loadbalancer.TargetPool{}, nil + }) + + // After the target pool write, reconcileLoadBalancer re-GETs the LB so downstream + // readiness checks see the post-write status (STACKIT flips the LB to PENDING). + mockLBClient.EXPECT(). + GetLoadBalancer(ctx, "test-lb"). + Return(&loadbalancer.LoadBalancer{ + Name: new("test-lb"), + Status: new("STATUS_PENDING"), + }, nil) + + err := r.reconcileLoadBalancer(ctx, logger) + + Expect(err).NotTo(HaveOccurred()) + }) + + It("should update plan via UpdateLoadBalancer when only plan changed", func() { + r.PlanId = "p100" // Changed plan + + mockLBClient.EXPECT(). + UpdateLoadBalancer(ctx, "test-lb", gomock.Any()). + DoAndReturn(func(_ context.Context, _ string, payload loadbalancer.UpdateLoadBalancerPayload) (*loadbalancer.LoadBalancer, error) { + Expect(payload.PlanId).To(Equal(new("p100"))) + Expect(payload.Version).To(Equal(new("v1"))) + // STACKIT UpdateLoadBalancer has PUT semantics — full desired state is required, + // so TargetPools (and the other invariant fields) must be present even when only + // the plan changed. + Expect(payload.TargetPools).To(HaveLen(1)) + Expect(payload.Networks).To(HaveLen(1)) + Expect(payload.Listeners).To(HaveLen(1)) + return &loadbalancer.LoadBalancer{}, nil + }) + + err := r.reconcileLoadBalancer(ctx, logger) + + Expect(err).NotTo(HaveOccurred()) + }) + + It("should update plan and targets in a single call when both changed", func() { + r.PlanId = "p100" + r.SelfHostedShootExposure.Spec.Endpoints = []extensionsv1alpha1.ControlPlaneEndpoint{ + { + NodeName: "node-3", + Addresses: []corev1.NodeAddress{ + {Type: corev1.NodeInternalIP, Address: "10.0.1.30"}, + }, + }, + } + + mockLBClient.EXPECT(). + UpdateLoadBalancer(ctx, "test-lb", gomock.Any()). + DoAndReturn(func(_ context.Context, _ string, payload loadbalancer.UpdateLoadBalancerPayload) (*loadbalancer.LoadBalancer, error) { + Expect(payload.PlanId).To(Equal(new("p100"))) + Expect(payload.Version).To(Equal(new("v1"))) + Expect(payload.TargetPools).To(HaveLen(1)) + Expect(*payload.TargetPools[0].Targets[0].Ip).To(Equal("10.0.1.30")) + return &loadbalancer.LoadBalancer{}, nil + }) + + err := r.reconcileLoadBalancer(ctx, logger) + + Expect(err).NotTo(HaveOccurred()) + }) + + It("should return error when UpdateLoadBalancerTargetPool fails", func() { + r.SelfHostedShootExposure.Spec.Endpoints = []extensionsv1alpha1.ControlPlaneEndpoint{ + { + NodeName: "node-new", + Addresses: []corev1.NodeAddress{ + {Type: corev1.NodeInternalIP, Address: "10.0.1.99"}, + }, + }, + } + + mockLBClient.EXPECT(). + UpdateLoadBalancerTargetPool(ctx, "test-lb", "target-pool-control-plane", gomock.Any()). + Return(nil, fmt.Errorf("API error")) + + err := r.reconcileLoadBalancer(ctx, logger) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("error updating load balancer target pool")) + }) + + It("should return error when UpdateLoadBalancer fails", func() { + r.PlanId = "p100" + + mockLBClient.EXPECT(). + UpdateLoadBalancer(ctx, "test-lb", gomock.Any()). + Return(nil, fmt.Errorf("API error")) + + err := r.reconcileLoadBalancer(ctx, logger) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("error updating load balancer")) + }) + }) +}) + +var _ = Describe("deleteLoadBalancer", func() { + var ( + ctx context.Context + logger logr.Logger + mockCtrl *gomock.Controller + mockLBClient *mock.MockLoadBalancingClient + r *Resources + ) + + BeforeEach(func() { + ctx = context.Background() + logger = logr.Discard() + mockCtrl = gomock.NewController(GinkgoT()) + mockLBClient = mock.NewMockLoadBalancingClient(mockCtrl) + r = &Resources{ + Options: Options{ + ResourceName: "test-lb", + }, + LBClient: mockLBClient, + LoadBalancer: nil, + } + }) + + AfterEach(func() { + mockCtrl.Finish() + }) + + It("should return nil if load balancer is not set (idempotent)", func() { + r.LoadBalancer = nil + + err := r.deleteLoadBalancer(ctx, logger) + + Expect(err).NotTo(HaveOccurred()) + }) + + It("should delete the load balancer if it exists", func() { + r.LoadBalancer = &loadbalancer.LoadBalancer{ + Name: new("test-lb"), + } + + mockLBClient.EXPECT(). + DeleteLoadBalancer(ctx, "test-lb"). + Return(nil) + + err := r.deleteLoadBalancer(ctx, logger) + + Expect(err).NotTo(HaveOccurred()) + }) + + It("should return error when DeleteLoadBalancer fails", func() { + r.LoadBalancer = &loadbalancer.LoadBalancer{ + Name: new("test-lb"), + } + + mockLBClient.EXPECT(). + DeleteLoadBalancer(ctx, "test-lb"). + Return(fmt.Errorf("API error")) + + err := r.deleteLoadBalancer(ctx, logger) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("error deleting load balancer")) + }) +}) + +var _ = Describe("buildTargets", func() { + var r *Resources + + BeforeEach(func() { + r = &Resources{ + Options: Options{ + SelfHostedShootExposure: &extensionsv1alpha1.SelfHostedShootExposure{ + Spec: extensionsv1alpha1.SelfHostedShootExposureSpec{ + Port: 443, + Endpoints: []extensionsv1alpha1.ControlPlaneEndpoint{}, + }, + }, + }, + } + }) + + It("should extract InternalIP addresses and sort them", func() { + r.SelfHostedShootExposure.Spec.Endpoints = []extensionsv1alpha1.ControlPlaneEndpoint{ + { + NodeName: "node-2", + Addresses: []corev1.NodeAddress{ + {Type: corev1.NodeInternalIP, Address: "10.0.1.20"}, + }, + }, + { + NodeName: "node-1", + Addresses: []corev1.NodeAddress{ + {Type: corev1.NodeInternalIP, Address: "10.0.1.10"}, + }, + }, + } + + targets, err := r.buildTargets() + + Expect(err).NotTo(HaveOccurred()) + Expect(targets).To(HaveLen(2)) + Expect(*targets[0].Ip).To(Equal("10.0.1.10")) + Expect(*targets[0].DisplayName).To(Equal("node-1")) + Expect(*targets[1].Ip).To(Equal("10.0.1.20")) + Expect(*targets[1].DisplayName).To(Equal("node-2")) + }) + + It("should error when endpoint has no InternalIP", func() { + r.SelfHostedShootExposure.Spec.Endpoints = []extensionsv1alpha1.ControlPlaneEndpoint{ + { + NodeName: "node-1", + Addresses: []corev1.NodeAddress{ + {Type: corev1.NodeHostName, Address: "node-1"}, + }, + }, + } + + targets, err := r.buildTargets() + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("no InternalIP address")) + Expect(targets).To(BeNil()) + }) + + It("should handle empty endpoints", func() { + r.SelfHostedShootExposure.Spec.Endpoints = []extensionsv1alpha1.ControlPlaneEndpoint{} + + targets, err := r.buildTargets() + + Expect(err).NotTo(HaveOccurred()) + Expect(targets).To(BeEmpty()) + }) + + It("should select InternalIP when multiple address types present", func() { + r.SelfHostedShootExposure.Spec.Endpoints = []extensionsv1alpha1.ControlPlaneEndpoint{ + { + NodeName: "node-1", + Addresses: []corev1.NodeAddress{ + {Type: corev1.NodeHostName, Address: "node-1.example.com"}, + {Type: corev1.NodeExternalIP, Address: "203.0.113.1"}, + {Type: corev1.NodeInternalIP, Address: "10.0.1.10"}, + }, + }, + } + + targets, err := r.buildTargets() + + Expect(err).NotTo(HaveOccurred()) + Expect(targets).To(HaveLen(1)) + Expect(*targets[0].Ip).To(Equal("10.0.1.10")) + }) +}) + +var _ = Describe("targetsEqual", func() { + It("should return true for equal target lists", func() { + a := []loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, + } + b := []loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, + } + + Expect(targetsEqual(a, b)).To(BeTrue()) + }) + + It("should return false for different IPs", func() { + a := []loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, + } + b := []loadbalancer.Target{ + {Ip: new("10.0.1.99"), DisplayName: new("node-1")}, + } + + Expect(targetsEqual(a, b)).To(BeFalse()) + }) + + It("should return false for different display names", func() { + a := []loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, + } + b := []loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: new("node-2")}, + } + + Expect(targetsEqual(a, b)).To(BeFalse()) + }) + + It("should return false for different lengths", func() { + a := []loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, + } + b := []loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, + {Ip: new("10.0.1.20"), DisplayName: new("node-2")}, + } + + Expect(targetsEqual(a, b)).To(BeFalse()) + }) + + It("should handle empty target lists", func() { + Expect(targetsEqual([]loadbalancer.Target{}, []loadbalancer.Target{})).To(BeTrue()) + }) + + It("should return false when IP is nil", func() { + a := []loadbalancer.Target{ + {Ip: nil, DisplayName: new("node-1")}, + } + b := []loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, + } + + Expect(targetsEqual(a, b)).To(BeFalse()) + }) + + It("should return false when DisplayName is nil", func() { + a := []loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: nil}, + } + b := []loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, + } + + Expect(targetsEqual(a, b)).To(BeFalse()) + }) +}) + +var _ = Describe("targetPoolNeedsUpdate", func() { + var r *Resources + + BeforeEach(func() { + r = &Resources{} + }) + + It("should return false when no load balancer exists", func() { + r.LoadBalancer = nil + + needsUpdate, err := r.targetPoolNeedsUpdate([]loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, + }) + + Expect(err).NotTo(HaveOccurred()) + Expect(needsUpdate).To(BeFalse()) + }) + + It("should return true when LB has no target pools but spec has targets", func() { + r.LoadBalancer = &loadbalancer.LoadBalancer{ + TargetPools: []loadbalancer.TargetPool{}, + } + + needsUpdate, err := r.targetPoolNeedsUpdate([]loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, + }) + + Expect(err).NotTo(HaveOccurred()) + Expect(needsUpdate).To(BeTrue()) + }) + + It("should return false when LB has no target pools and spec has no targets", func() { + r.LoadBalancer = &loadbalancer.LoadBalancer{ + TargetPools: []loadbalancer.TargetPool{}, + } + + needsUpdate, err := r.targetPoolNeedsUpdate([]loadbalancer.Target{}) + + Expect(err).NotTo(HaveOccurred()) + Expect(needsUpdate).To(BeFalse()) + }) + + It("should return error when target pool has unexpected name", func() { + r.LoadBalancer = &loadbalancer.LoadBalancer{ + TargetPools: []loadbalancer.TargetPool{ + {Name: new("wrong-name")}, + }, + } + + _, err := r.targetPoolNeedsUpdate([]loadbalancer.Target{}) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("unexpected target pool name")) + }) + + It("should return false when targets match", func() { + r.LoadBalancer = &loadbalancer.LoadBalancer{ + TargetPools: []loadbalancer.TargetPool{ + { + Name: new("target-pool-control-plane"), + Targets: []loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, + }, + }, + }, + } + + needsUpdate, err := r.targetPoolNeedsUpdate([]loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, + }) + + Expect(err).NotTo(HaveOccurred()) + Expect(needsUpdate).To(BeFalse()) + }) + + It("should return true when targets differ", func() { + r.LoadBalancer = &loadbalancer.LoadBalancer{ + TargetPools: []loadbalancer.TargetPool{ + { + Name: new("target-pool-control-plane"), + Targets: []loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, + }, + }, + }, + } + + needsUpdate, err := r.targetPoolNeedsUpdate([]loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, + {Ip: new("10.0.1.20"), DisplayName: new("node-2")}, + }) + + Expect(err).NotTo(HaveOccurred()) + Expect(needsUpdate).To(BeTrue()) + }) + + It("should compare correctly regardless of LB target order", func() { + r.LoadBalancer = &loadbalancer.LoadBalancer{ + TargetPools: []loadbalancer.TargetPool{ + { + Name: new("target-pool-control-plane"), + Targets: []loadbalancer.Target{ + // LB returns targets in reverse order + {Ip: new("10.0.1.20"), DisplayName: new("node-2")}, + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, + }, + }, + }, + } + + // Spec targets are sorted + needsUpdate, err := r.targetPoolNeedsUpdate([]loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, + {Ip: new("10.0.1.20"), DisplayName: new("node-2")}, + }) + + Expect(err).NotTo(HaveOccurred()) + Expect(needsUpdate).To(BeFalse()) + }) +}) diff --git a/pkg/controller/selfhostedshootexposure/resources_test.go b/pkg/controller/selfhostedshootexposure/resources_test.go new file mode 100644 index 00000000..9e96f84e --- /dev/null +++ b/pkg/controller/selfhostedshootexposure/resources_test.go @@ -0,0 +1,79 @@ +package selfhostedshootexposure + +import ( + "context" + "fmt" + + "github.com/go-logr/logr" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + loadbalancer "github.com/stackitcloud/stackit-sdk-go/services/loadbalancer/v2api" + "go.uber.org/mock/gomock" + + mock "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit/client/mock" +) + +var _ = Describe("Resources", func() { + var ( + ctx context.Context + logger logr.Logger + mockCtrl *gomock.Controller + mockLBClient *mock.MockLoadBalancingClient + r *Resources + ) + + BeforeEach(func() { + ctx = context.Background() + logger = logr.Discard() + mockCtrl = gomock.NewController(GinkgoT()) + mockLBClient = mock.NewMockLoadBalancingClient(mockCtrl) + r = &Resources{ + Options: Options{ + ResourceName: "test-lb", + }, + LBClient: mockLBClient, + } + }) + + AfterEach(func() { + mockCtrl.Finish() + }) + + Describe("#getExistingResources", func() { + It("should populate LoadBalancer when found", func() { + expectedLB := &loadbalancer.LoadBalancer{ + Name: new("test-lb"), + } + mockLBClient.EXPECT(). + GetLoadBalancer(ctx, "test-lb"). + Return(expectedLB, nil) + + err := r.getExistingResources(ctx, logger) + + Expect(err).NotTo(HaveOccurred()) + Expect(r.LoadBalancer).To(Equal(expectedLB)) + }) + + It("should leave LoadBalancer nil when not found", func() { + mockLBClient.EXPECT(). + GetLoadBalancer(ctx, "test-lb"). + Return(nil, nil) + + err := r.getExistingResources(ctx, logger) + + Expect(err).NotTo(HaveOccurred()) + Expect(r.LoadBalancer).To(BeNil()) + }) + + It("should return error when GetLoadBalancer fails", func() { + mockLBClient.EXPECT(). + GetLoadBalancer(ctx, "test-lb"). + Return(nil, fmt.Errorf("API error")) + + err := r.getExistingResources(ctx, logger) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("error getting load balancer")) + }) + }) +}) diff --git a/pkg/controller/selfhostedshootexposure/suite_test.go b/pkg/controller/selfhostedshootexposure/suite_test.go new file mode 100644 index 00000000..fd9ccdb2 --- /dev/null +++ b/pkg/controller/selfhostedshootexposure/suite_test.go @@ -0,0 +1,13 @@ +package selfhostedshootexposure_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestSelfHostedShootExposure(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "SelfHostedShootExposure Controller Suite") +} From 70e80e39417131ae255680803964c379e86a2212 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Amand?= Date: Wed, 15 Apr 2026 13:17:03 +0200 Subject: [PATCH 4/9] test: Add integration tests for SelfHostedShootExposure --- Makefile | 10 + .../selfhostedshootexposure_suite_test.go | 13 + .../stackit/selfhostedshootexposure_test.go | 634 ++++++++++++++++++ ...rdener.cloud_selfhostedshootexposures.yaml | 396 +++++++++++ 4 files changed, 1053 insertions(+) create mode 100644 test/integration/selfhostedshootexposure/stackit/selfhostedshootexposure_suite_test.go create mode 100644 test/integration/selfhostedshootexposure/stackit/selfhostedshootexposure_test.go create mode 100644 test/integration/testdata/upstream-crds/10-crd-extensions.gardener.cloud_selfhostedshootexposures.yaml diff --git a/Makefile b/Makefile index 2c1df759..b337d074 100644 --- a/Makefile +++ b/Makefile @@ -173,6 +173,16 @@ test-integration-infra: $(REPORT_COLLECTOR) $(SETUP_ENVTEST) $(GINKGO) ## Run in -- \ $(INFRA_TEST_FLAGS) +.PHONY: test-integration-exposure +test-integration-exposure: $(REPORT_COLLECTOR) $(SETUP_ENVTEST) $(GINKGO) ## Run selfhostedshootexposure integration tests + @GINKGO=$(GINKGO) ./hack/test-integration.sh \ + -v --show-node-events \ + --timeout 20m \ + --grace-period 3m \ + ./test/integration/selfhostedshootexposure/stackit \ + -- \ + $(EXPOSURE_TEST_FLAGS) + help: ## Display this help @awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make \033[36m\033[0m\n"} /^[a-zA-Z_-]+:.*?##/ { printf " \033[36m%-15s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST) diff --git a/test/integration/selfhostedshootexposure/stackit/selfhostedshootexposure_suite_test.go b/test/integration/selfhostedshootexposure/stackit/selfhostedshootexposure_suite_test.go new file mode 100644 index 00000000..18f5c342 --- /dev/null +++ b/test/integration/selfhostedshootexposure/stackit/selfhostedshootexposure_suite_test.go @@ -0,0 +1,13 @@ +package stackit + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestSelfHostedShootExposure(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "STACKIT SelfHostedShootExposure Controller Suite") +} diff --git a/test/integration/selfhostedshootexposure/stackit/selfhostedshootexposure_test.go b/test/integration/selfhostedshootexposure/stackit/selfhostedshootexposure_test.go new file mode 100644 index 00000000..1bcf58cc --- /dev/null +++ b/test/integration/selfhostedshootexposure/stackit/selfhostedshootexposure_test.go @@ -0,0 +1,634 @@ +package stackit + +import ( + "bytes" + "context" + "flag" + "fmt" + "os" + "path/filepath" + "time" + + gardenerv1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" + v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants" + extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" + "github.com/gardener/gardener/pkg/extensions" + "github.com/gardener/gardener/pkg/logger" + gardenerutils "github.com/gardener/gardener/pkg/utils" + "github.com/go-logr/logr" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" + loadbalancer "github.com/stackitcloud/stackit-sdk-go/services/loadbalancer/v2api" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/runtime/serializer" + "k8s.io/apimachinery/pkg/runtime/serializer/json" + "k8s.io/apimachinery/pkg/util/uuid" + schemev1 "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/envtest" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/manager" + metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + + stackitv1alpha1 "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/apis/stackit/v1alpha1" + "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/controller/selfhostedshootexposure" + "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit" + stackitclient "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit/client" +) + +const ( + dnsServer = "1.1.1.1" + workerCIDR = "10.250.0.0/16" + + // exposureName is the name of the SelfHostedShootExposure resource. + // The resulting LB name will be: "-exposure-" + exposureName = "apiserver" +) + +var ( + stackitProjectID string + stackitServiceAccount string + region = flag.String("region", "eu01", "Region") +) + +var ( + ctx = context.Background() + log logr.Logger + + testEnv *envtest.Environment + mgrCancel context.CancelFunc + c client.Client + + encoder runtime.Encoder + iaasClient stackitclient.IaaSClient + lbClient stackitclient.LoadBalancingClient + endpoints stackitv1alpha1.APIEndpoints + + testID = string(uuid.NewUUID()) +) + +func validateEnvs() error { + requiredVars := []string{ + "STACKIT_PROJECT_ID", + "STACKIT_SERVICE_ACCOUNT_KEY", + } + + for _, varName := range requiredVars { + if os.Getenv(varName) == "" { + return fmt.Errorf("error: environment variable '%s' is not set", varName) + } + } + + return nil +} + +var _ = BeforeSuite(func() { + flag.Parse() + + var err error + stackitProjectID = os.Getenv("STACKIT_PROJECT_ID") + stackitServiceAccount = os.Getenv("STACKIT_SERVICE_ACCOUNT_KEY") + + credentials := &stackit.Credentials{ + ProjectID: stackitProjectID, + SaKeyJSON: stackitServiceAccount, + } + endpoints = stackitv1alpha1.APIEndpoints{ + IaaS: new("https://iaas.api.stackit.cloud"), + } + + Expect(*region).NotTo(BeEmpty()) + Expect(validateEnvs()).To(Succeed()) + + iaasClient, err = stackitclient.NewIaaSClient(*region, endpoints, credentials) + Expect(err).NotTo(HaveOccurred()) + + lbClient, err = stackitclient.NewLoadBalancingClient(ctx, *region, endpoints, credentials) + Expect(err).NotTo(HaveOccurred()) + + repoRoot := filepath.Join("..", "..", "..", "..") + + logf.SetLogger(logger.MustNewZapLogger(logger.DebugLevel, logger.FormatJSON, zap.WriteTo(GinkgoWriter))) + log = logf.Log.WithName("selfhostedshootexposure-test") + + DeferCleanup(func() { + By("stopping manager") + mgrCancel() + + By("stopping test environment") + Expect(testEnv.Stop()).To(Succeed()) + }) + + By("starting test environment") + testEnv = &envtest.Environment{ + CRDInstallOptions: envtest.CRDInstallOptions{ + Paths: []string{ + filepath.Join(repoRoot, "test", "integration", "testdata", "upstream-crds", "10-crd-extensions.gardener.cloud_clusters.yaml"), + filepath.Join(repoRoot, "test", "integration", "testdata", "upstream-crds", "10-crd-extensions.gardener.cloud_infrastructures.yaml"), + filepath.Join(repoRoot, "test", "integration", "testdata", "upstream-crds", "10-crd-extensions.gardener.cloud_selfhostedshootexposures.yaml"), + }, + }, + } + + restConfig, err := testEnv.Start() + Expect(err).ToNot(HaveOccurred()) + Expect(restConfig).ToNot(BeNil()) + + httpClient, err := rest.HTTPClientFor(restConfig) + Expect(err).NotTo(HaveOccurred()) + mapper, err := apiutil.NewDynamicRESTMapper(restConfig, httpClient) + Expect(err).NotTo(HaveOccurred()) + + scheme := runtime.NewScheme() + Expect(schemev1.AddToScheme(scheme)).To(Succeed()) + Expect(extensionsv1alpha1.AddToScheme(scheme)).To(Succeed()) + Expect(stackitv1alpha1.AddToScheme(scheme)).To(Succeed()) + Expect(gardenerv1beta1.AddToScheme(scheme)).To(Succeed()) + + By("setup manager") + mgr, err := manager.New(restConfig, manager.Options{ + Scheme: scheme, + Metrics: metricsserver.Options{ + BindAddress: "0", + }, + Cache: cache.Options{ + Mapper: mapper, + ByObject: map[client.Object]cache.ByObject{ + &extensionsv1alpha1.SelfHostedShootExposure{}: { + Label: labels.SelectorFromSet(labels.Set{"test-id": testID}), + }, + }, + }, + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(selfhostedshootexposure.AddToManagerWithOptions(mgr, selfhostedshootexposure.AddOptions{ + Controller: controller.Options{ + MaxConcurrentReconciles: 5, + }, + })).To(Succeed()) + + var mgrContext context.Context + mgrContext, mgrCancel = context.WithCancel(ctx) + + By("start manager") + go func() { + defer GinkgoRecover() + err := mgr.Start(mgrContext) + Expect(err).NotTo(HaveOccurred()) + }() + + c = mgr.GetClient() + Expect(c).NotTo(BeNil()) + + gv := schema.GroupVersions{ + stackitv1alpha1.SchemeGroupVersion, + gardenerv1beta1.SchemeGroupVersion, + } + encoder = serializer.NewCodecFactory(mgr.GetScheme()).EncoderForVersion(&json.Serializer{}, gv) +}) + +var _ = Describe("SelfHostedShootExposure tests", func() { + var ( + namespaceName string + networkID string + lbName string + ) + + BeforeEach(func() { + suffix, err := gardenerutils.GenerateRandomStringFromCharset(5, "0123456789abcdefghijklmnopqrstuvwxyz") + Expect(err).NotTo(HaveOccurred()) + namespaceName = "stackit--exp-it--" + suffix + // ResourceName = "-exposure-" + lbName = namespaceName + "-exposure-" + exposureName + }) + + AfterEach(func() { + // Best-effort cleanup of LB via STACKIT API (in case the controller didn't delete it) + if lbName != "" { + lb, _ := lbClient.GetLoadBalancer(ctx, lbName) + if lb != nil { + log.Info("Cleaning up leftover load balancer", "name", lbName) + _ = lbClient.DeleteLoadBalancer(ctx, lbName) + } + } + + // Cleanup network + if networkID != "" { + log.Info("Cleaning up network", "id", networkID) + _ = iaasClient.DeleteNetwork(ctx, networkID) + } + }) + + It("should create, update, and delete a load balancer for a self-hosted shoot exposure", func() { + By("create isolated network") + networkName := namespaceName + "-network" + network, err := iaasClient.CreateIsolatedNetwork(ctx, iaas.CreateIsolatedNetworkPayload{ + Name: networkName, + Dhcp: new(true), + Ipv4: &iaas.CreateNetworkIPv4{ + CreateNetworkIPv4WithPrefix: &iaas.CreateNetworkIPv4WithPrefix{ + Nameservers: []string{dnsServer}, + Prefix: workerCIDR, + }, + }, + }) + Expect(err).NotTo(HaveOccurred()) + networkID = network.Id + + By("create namespace") + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespaceName, + }, + } + Expect(c.Create(ctx, namespace)).To(Succeed()) + + By("create cloudprovider secret") + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cloudprovider", + Namespace: namespaceName, + }, + Data: map[string][]byte{ + stackit.SaKeyJSON: []byte(stackitServiceAccount), + stackit.ProjectID: []byte(stackitProjectID), + }, + } + Expect(c.Create(ctx, secret)).To(Succeed()) + + By("create cluster") + shoot := &gardenerv1beta1.Shoot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "shoot", + }, + Spec: gardenerv1beta1.ShootSpec{ + Region: *region, + }, + Status: gardenerv1beta1.ShootStatus{ + TechnicalID: namespaceName, + }, + } + + shootBytes := new(bytes.Buffer) + Expect(encoder.Encode(shoot, shootBytes)).To(Succeed()) + + cluster := &extensionsv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespaceName, + }, + Spec: extensionsv1alpha1.ClusterSpec{ + CloudProfile: runtime.RawExtension{Raw: []byte("{}")}, + Seed: runtime.RawExtension{Raw: []byte("{}")}, + Shoot: runtime.RawExtension{Raw: shootBytes.Bytes()}, + }, + } + Expect(c.Create(ctx, cluster)).To(Succeed()) + + By("create infrastructure with status containing network ID") + infraStatus := &stackitv1alpha1.InfrastructureStatus{ + Networks: stackitv1alpha1.NetworkStatus{ + ID: networkID, + }, + } + infraStatusBytes := new(bytes.Buffer) + Expect(encoder.Encode(infraStatus, infraStatusBytes)).To(Succeed()) + + infra := &extensionsv1alpha1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ + Name: shoot.Name, + Namespace: namespaceName, + }, + Spec: extensionsv1alpha1.InfrastructureSpec{ + DefaultSpec: extensionsv1alpha1.DefaultSpec{ + Type: "stackit", + }, + SecretRef: corev1.SecretReference{ + Name: "cloudprovider", + Namespace: namespaceName, + }, + Region: *region, + }, + } + Expect(c.Create(ctx, infra)).To(Succeed()) + + // Patch infrastructure status to include network ID + patch := client.MergeFrom(infra.DeepCopy()) + infra.Status = extensionsv1alpha1.InfrastructureStatus{ + DefaultStatus: extensionsv1alpha1.DefaultStatus{ + ProviderStatus: &runtime.RawExtension{Raw: infraStatusBytes.Bytes()}, + }, + } + Expect(c.Status().Patch(ctx, infra, patch)).To(Succeed()) + + By("create SelfHostedShootExposure with two endpoints") + exposure := &extensionsv1alpha1.SelfHostedShootExposure{ + ObjectMeta: metav1.ObjectMeta{ + Name: exposureName, + Namespace: namespaceName, + Labels: map[string]string{ + "test-id": testID, + }, + }, + Spec: extensionsv1alpha1.SelfHostedShootExposureSpec{ + DefaultSpec: extensionsv1alpha1.DefaultSpec{ + Type: "stackit", + }, + Port: 6443, + Endpoints: []extensionsv1alpha1.ControlPlaneEndpoint{ + { + NodeName: "node-1", + Addresses: []corev1.NodeAddress{ + {Type: corev1.NodeInternalIP, Address: "10.250.0.10"}, + }, + }, + { + NodeName: "node-2", + Addresses: []corev1.NodeAddress{ + {Type: corev1.NodeInternalIP, Address: "10.250.0.11"}, + }, + }, + }, + }, + } + Expect(c.Create(ctx, exposure)).To(Succeed()) + + // We do not wait for the SelfHostedShootExposure to become Ready: the endpoint IPs + // above do not point at real VMs, so the STACKIT LB's own target health probe drives + // the LB into STATUS_ERROR/TYPE_TARGET_NOT_ACTIVE and checkLoadBalancerReady requeues + // indefinitely. We instead assert on the LB spec via the STACKIT API — LB readiness + // classification is covered by unit tests. + By("verify load balancer was created with the expected spec via STACKIT API") + var lb *loadbalancer.LoadBalancer + Eventually(func(g Gomega) { + var err error + lb, err = lbClient.GetLoadBalancer(ctx, lbName) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(lb).NotTo(BeNil()) + + // Identity + g.Expect(lb.Name).NotTo(BeNil()) + g.Expect(*lb.Name).To(Equal(lbName)) + g.Expect(lb.Version).NotTo(BeNil()) + + // Labels — validates the '/'-rejection workaround (flat dot-separated keys) + g.Expect(lb.Labels).NotTo(BeNil()) + g.Expect(*lb.Labels).To(HaveKeyWithValue("cluster.stackit.cloud", namespaceName)) + g.Expect(*lb.Labels).To(HaveKeyWithValue("exposure.stackit.cloud", exposureName)) + + // Network + g.Expect(lb.Networks).To(HaveLen(1)) + g.Expect(lb.Networks[0].NetworkId).NotTo(BeNil()) + g.Expect(*lb.Networks[0].NetworkId).To(Equal(networkID)) + g.Expect(lb.Networks[0].Role).NotTo(BeNil()) + g.Expect(*lb.Networks[0].Role).To(Equal("ROLE_LISTENERS_AND_TARGETS")) + + // Listener + g.Expect(lb.Listeners).To(HaveLen(1)) + g.Expect(lb.Listeners[0].DisplayName).NotTo(BeNil()) + g.Expect(*lb.Listeners[0].DisplayName).To(Equal("listener-control-plane")) + g.Expect(lb.Listeners[0].Port).NotTo(BeNil()) + g.Expect(*lb.Listeners[0].Port).To(BeEquivalentTo(6443)) + g.Expect(lb.Listeners[0].Protocol).NotTo(BeNil()) + g.Expect(*lb.Listeners[0].Protocol).To(Equal("PROTOCOL_TCP")) + g.Expect(lb.Listeners[0].TargetPool).NotTo(BeNil()) + g.Expect(*lb.Listeners[0].TargetPool).To(Equal("target-pool-control-plane")) + + // Target pool + g.Expect(lb.TargetPools).To(HaveLen(1)) + g.Expect(lb.TargetPools[0].Name).NotTo(BeNil()) + g.Expect(*lb.TargetPools[0].Name).To(Equal("target-pool-control-plane")) + g.Expect(lb.TargetPools[0].TargetPort).NotTo(BeNil()) + g.Expect(*lb.TargetPools[0].TargetPort).To(BeEquivalentTo(6443)) + g.Expect(lb.TargetPools[0].Targets).To(HaveLen(2)) + + targetIPs := make([]string, 0, len(lb.TargetPools[0].Targets)) + for _, t := range lb.TargetPools[0].Targets { + g.Expect(t.Ip).NotTo(BeNil()) + g.Expect(t.DisplayName).NotTo(BeNil()) + targetIPs = append(targetIPs, *t.Ip) + } + g.Expect(targetIPs).To(ConsistOf("10.250.0.10", "10.250.0.11")) + + // Plan — default since no providerConfig.LoadBalancer.PlanId set + g.Expect(lb.PlanId).NotTo(BeNil()) + g.Expect(*lb.PlanId).To(Equal("p10")) + + // Options — EphemeralAddress is set on create + g.Expect(lb.Options).NotTo(BeNil()) + g.Expect(lb.Options.EphemeralAddress).NotTo(BeNil()) + g.Expect(*lb.Options.EphemeralAddress).To(BeTrue()) + + // External VIP — STACKIT assigns the ephemeral address even while the LB is + // stuck in STATUS_ERROR (the API reports it even when not shown in the portal). + g.Expect(lb.ExternalAddress).NotTo(BeNil()) + g.Expect(*lb.ExternalAddress).NotTo(BeEmpty()) + + // Status — we don't require READY, but TERMINATING would be wrong + g.Expect(lb.Status).NotTo(BeNil()) + g.Expect(*lb.Status).NotTo(Equal("STATUS_TERMINATING")) + + // If STACKIT reports STATUS_ERROR, the only expected cause given fake target IPs is + // TYPE_TARGET_NOT_ACTIVE. Any other error type would mean the extension produced an + // unexpected LB spec (and is a test failure worth investigating). + if *lb.Status == "STATUS_ERROR" { + g.Expect(lb.Errors).NotTo(BeEmpty()) + for _, e := range lb.Errors { + g.Expect(e.Type).NotTo(BeNil()) + g.Expect(*e.Type).To(Equal("TYPE_TARGET_NOT_ACTIVE")) + } + } + }).WithTimeout(5 * time.Minute).WithPolling(5 * time.Second).Should(Succeed()) + + By("verify SelfHostedShootExposure CR is being reconciled") + Expect(c.Get(ctx, client.ObjectKeyFromObject(exposure), exposure)).To(Succeed()) + // Finalizer proves Gardener bound the controller to this resource. + Expect(exposure.Finalizers).NotTo(BeEmpty()) + // Status.Ingress is only populated once the LB reaches READY in the actuator; with + // fake targets we never get there, so it stays empty. + Expect(exposure.Status.Ingress).To(BeEmpty()) + + // Snapshot the LB version so we can assert the update actually hit the API. + initialVersion := *lb.Version + + By("update endpoints (add a third node)") + patchExposureReconcile(exposure, func() { + exposure.Spec.Endpoints = append(exposure.Spec.Endpoints, extensionsv1alpha1.ControlPlaneEndpoint{ + NodeName: "node-3", + Addresses: []corev1.NodeAddress{ + {Type: corev1.NodeInternalIP, Address: "10.250.0.12"}, + }, + }) + }) + + By("verify load balancer targets were updated via STACKIT API") + Eventually(func(g Gomega) { + var err error + lb, err = lbClient.GetLoadBalancer(ctx, lbName) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(lb.TargetPools).To(HaveLen(1)) + g.Expect(lb.TargetPools[0].Targets).To(HaveLen(3)) + g.Expect(targetIPs(lb)).To(ConsistOf("10.250.0.10", "10.250.0.11", "10.250.0.12")) + + // Version must change on write (Version is opaque — equality comparison only, per SDK). + g.Expect(lb.Version).NotTo(BeNil()) + g.Expect(*lb.Version).NotTo(Equal(initialVersion)) + + // Invariants that must not have shifted during the target-pool fast-path update. + g.Expect(lb.Listeners).To(HaveLen(1)) + g.Expect(*lb.Listeners[0].Port).To(BeEquivalentTo(6443)) + g.Expect(lb.Networks).To(HaveLen(1)) + g.Expect(*lb.Networks[0].NetworkId).To(Equal(networkID)) + g.Expect(*lb.PlanId).To(Equal("p10")) + }).WithTimeout(2 * time.Minute).WithPolling(5 * time.Second).Should(Succeed()) + + By("no-op reconcile must not write to the LB") + versionBeforeNoOp := *lb.Version + patchExposureReconcile(exposure, func() {}) + Consistently(func(g Gomega) { + lbCheck, err := lbClient.GetLoadBalancer(ctx, lbName) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(lbCheck.Version).NotTo(BeNil()) + g.Expect(*lbCheck.Version).To(Equal(versionBeforeNoOp)) + }).WithTimeout(30 * time.Second).WithPolling(5 * time.Second).Should(Succeed()) + + By("remove an endpoint (fast-path shrink)") + versionBeforeRemove := versionBeforeNoOp + patchExposureReconcile(exposure, func() { + // Drop node-2 (10.250.0.11), leaving node-1 and node-3. + exposure.Spec.Endpoints = []extensionsv1alpha1.ControlPlaneEndpoint{ + {NodeName: "node-1", Addresses: []corev1.NodeAddress{{Type: corev1.NodeInternalIP, Address: "10.250.0.10"}}}, + {NodeName: "node-3", Addresses: []corev1.NodeAddress{{Type: corev1.NodeInternalIP, Address: "10.250.0.12"}}}, + } + }) + + Eventually(func(g Gomega) { + var err error + lb, err = lbClient.GetLoadBalancer(ctx, lbName) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(lb.TargetPools).To(HaveLen(1)) + g.Expect(lb.TargetPools[0].Targets).To(HaveLen(2)) + g.Expect(targetIPs(lb)).To(ConsistOf("10.250.0.10", "10.250.0.12")) + + g.Expect(lb.Version).NotTo(BeNil()) + g.Expect(*lb.Version).NotTo(Equal(versionBeforeRemove)) + // Plan unchanged by a pure target update. + g.Expect(*lb.PlanId).To(Equal("p10")) + }).WithTimeout(2 * time.Minute).WithPolling(5 * time.Second).Should(Succeed()) + + By("change plan only (full PUT update)") + versionBeforePlanChange := *lb.Version + patchExposureReconcile(exposure, func() { + setExposurePlanId(exposure, "p50") + }) + + Eventually(func(g Gomega) { + var err error + lb, err = lbClient.GetLoadBalancer(ctx, lbName) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(lb.PlanId).NotTo(BeNil()) + g.Expect(*lb.PlanId).To(Equal("p50")) + + g.Expect(lb.Version).NotTo(BeNil()) + g.Expect(*lb.Version).NotTo(Equal(versionBeforePlanChange)) + + // Targets untouched by a plan-only change. + g.Expect(lb.TargetPools).To(HaveLen(1)) + g.Expect(lb.TargetPools[0].Targets).To(HaveLen(2)) + g.Expect(targetIPs(lb)).To(ConsistOf("10.250.0.10", "10.250.0.12")) + }).WithTimeout(2 * time.Minute).WithPolling(5 * time.Second).Should(Succeed()) + + By("change plan and endpoints together (full PUT update)") + versionBeforeCombined := *lb.Version + patchExposureReconcile(exposure, func() { + setExposurePlanId(exposure, "p250") + // Add node-2 back, so we're changing targets *and* plan in the same reconcile. + exposure.Spec.Endpoints = append(exposure.Spec.Endpoints, extensionsv1alpha1.ControlPlaneEndpoint{ + NodeName: "node-2", + Addresses: []corev1.NodeAddress{{Type: corev1.NodeInternalIP, Address: "10.250.0.11"}}, + }) + }) + + Eventually(func(g Gomega) { + var err error + lb, err = lbClient.GetLoadBalancer(ctx, lbName) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(lb.PlanId).NotTo(BeNil()) + g.Expect(*lb.PlanId).To(Equal("p250")) + + g.Expect(lb.TargetPools).To(HaveLen(1)) + g.Expect(lb.TargetPools[0].Targets).To(HaveLen(3)) + g.Expect(targetIPs(lb)).To(ConsistOf("10.250.0.10", "10.250.0.11", "10.250.0.12")) + + g.Expect(lb.Version).NotTo(BeNil()) + g.Expect(*lb.Version).NotTo(Equal(versionBeforeCombined)) + }).WithTimeout(2 * time.Minute).WithPolling(5 * time.Second).Should(Succeed()) + + By("delete SelfHostedShootExposure") + Expect(client.IgnoreNotFound(c.Delete(ctx, exposure))).To(Succeed()) + + By("wait until SelfHostedShootExposure is deleted") + Expect(extensions.WaitUntilExtensionObjectDeleted( + ctx, + c, + log, + exposure, + "SelfHostedShootExposure", + 2*time.Second, + 10*time.Minute, + )).To(Succeed()) + + By("verify load balancer was deleted via STACKIT API") + Eventually(func(g Gomega) { + lb, err := lbClient.GetLoadBalancer(ctx, lbName) + g.Expect(stackitclient.IgnoreNotFoundError(err)).NotTo(HaveOccurred()) + g.Expect(lb).To(BeNil()) + }).WithTimeout(5 * time.Minute).WithPolling(10 * time.Second).Should(Succeed()) + }) +}) + +// patchExposureReconcile re-reads the exposure, applies mutate, sets the gardener-operation +// reconcile annotation, and patches. Used to nudge the controller into re-reconciling after +// a spec change (or with no spec change to exercise the no-op path). +func patchExposureReconcile(exposure *extensionsv1alpha1.SelfHostedShootExposure, mutate func()) { + GinkgoHelper() + Expect(c.Get(ctx, client.ObjectKeyFromObject(exposure), exposure)).To(Succeed()) + patch := client.MergeFrom(exposure.DeepCopy()) + mutate() + metav1.SetMetaDataAnnotation(&exposure.ObjectMeta, v1beta1constants.GardenerOperation, v1beta1constants.GardenerOperationReconcile) + Expect(c.Patch(ctx, exposure, patch)).To(Succeed()) +} + +// targetIPs extracts the target IP strings from the LB's single target pool. +func targetIPs(lb *loadbalancer.LoadBalancer) []string { + ips := make([]string, 0, len(lb.TargetPools[0].Targets)) + for _, t := range lb.TargetPools[0].Targets { + if t.Ip != nil { + ips = append(ips, *t.Ip) + } + } + return ips +} + +// setExposurePlanId encodes a SelfHostedShootExposureConfig with the given plan and sets it +// as the exposure's ProviderConfig. +func setExposurePlanId(exposure *extensionsv1alpha1.SelfHostedShootExposure, planId string) { + GinkgoHelper() + buf := new(bytes.Buffer) + Expect(encoder.Encode(&stackitv1alpha1.SelfHostedShootExposureConfig{ + LoadBalancer: &stackitv1alpha1.LoadBalancerConfig{ + PlanId: &planId, + }, + }, buf)).To(Succeed()) + exposure.Spec.ProviderConfig = &runtime.RawExtension{Raw: buf.Bytes()} +} diff --git a/test/integration/testdata/upstream-crds/10-crd-extensions.gardener.cloud_selfhostedshootexposures.yaml b/test/integration/testdata/upstream-crds/10-crd-extensions.gardener.cloud_selfhostedshootexposures.yaml new file mode 100644 index 00000000..5413b10c --- /dev/null +++ b/test/integration/testdata/upstream-crds/10-crd-extensions.gardener.cloud_selfhostedshootexposures.yaml @@ -0,0 +1,396 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.20.1 + name: selfhostedshootexposures.extensions.gardener.cloud +spec: + group: extensions.gardener.cloud + names: + kind: SelfHostedShootExposure + listKind: SelfHostedShootExposureList + plural: selfhostedshootexposures + shortNames: + - exp + singular: selfhostedshootexposure + scope: Namespaced + versions: + - additionalPrinterColumns: + - description: The type of the self hosted shoot exposure provider for this resource. + jsonPath: .spec.type + name: Type + type: string + - description: The IP of the first LoadBalancer ingress. + jsonPath: .status.ingress[0].ip + name: IP + type: string + - description: The Hostname of the first LoadBalancer ingress. + jsonPath: .status.ingress[0].hostname + name: Hostname + type: string + - description: Status of self hosted shoot exposure resource. + jsonPath: .status.lastOperation.state + name: Status + type: string + - description: creation timestamp + jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + schema: + openAPIV3Schema: + description: SelfHostedShootExposure contains the configuration for the exposure + of a self-hosted shoot control plane. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: |- + Specification of the SelfHostedShootExposure. + If the object's deletion timestamp is set, this field is immutable. + properties: + class: + description: Class holds the extension class used to control the responsibility + for multiple provider extensions. + type: string + x-kubernetes-validations: + - message: Value is immutable + rule: self == oldSelf + credentialsRef: + description: |- + CredentialsRef is a reference to the cloud provider credentials. + It is only set for shoots with managed infrastructure (i.e., if `Shoot.spec.{credentials,secret}BindingName` is set). + properties: + apiVersion: + description: API version of the referent. + type: string + fieldPath: + description: |- + If referring to a piece of an object instead of an entire object, this string + should contain a valid JSON/Go field access statement, such as desiredState.manifest.containers[2]. + For example, if the object reference is to a container within a pod, this would take on a value like: + "spec.containers{name}" (where "name" refers to the name of the container that triggered + the event) or if no container name is specified "spec.containers[2]" (container with + index 2 in this pod). This syntax is chosen only to have some well-defined way of + referencing a part of an object. + type: string + kind: + description: |- + Kind of the referent. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + namespace: + description: |- + Namespace of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/ + type: string + resourceVersion: + description: |- + Specific resourceVersion to which this reference is made, if any. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency + type: string + uid: + description: |- + UID of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids + type: string + type: object + x-kubernetes-map-type: atomic + endpoints: + description: Endpoints contains a list of healthy control plane nodes + to expose. + items: + description: ControlPlaneEndpoint is an endpoint that should be + exposed. + properties: + addresses: + description: Addresses is a list of addresses of type NodeAddress + to expose. + items: + description: NodeAddress contains information for the node's + address. + properties: + address: + description: The node address. + type: string + type: + description: Node address type, one of Hostname, ExternalIP + or InternalIP. + type: string + required: + - address + - type + type: object + type: array + nodeName: + description: NodeName is the name of the node to expose. + type: string + required: + - addresses + - nodeName + type: object + type: array + port: + description: |- + Port is the port number that should be exposed by the exposure mechanism. + It is the port where the API server listens on the control plane nodes and the port on which the load balancer (or + any other exposure mechanism) should listen on. + format: int32 + type: integer + providerConfig: + description: ProviderConfig is the provider specific configuration. + type: object + x-kubernetes-preserve-unknown-fields: true + type: + description: Type contains the instance of the resource's kind. + type: string + required: + - endpoints + - port + - type + type: object + status: + description: SelfHostedShootExposureStatus is the status for an SelfHostedShootExposure + resource. + properties: + conditions: + description: Conditions represents the latest available observations + of a Seed's current state. + items: + description: Condition holds the information about the state of + a resource. + properties: + codes: + description: Well-defined error codes in case the condition + reports a problem. + items: + description: ErrorCode is a string alias. + type: string + type: array + lastTransitionTime: + description: Last time the condition transitioned from one status + to another. + format: date-time + type: string + lastUpdateTime: + description: Last time the condition was updated. + format: date-time + type: string + message: + description: A human readable message indicating details about + the transition. + type: string + reason: + description: The reason for the condition's last transition. + type: string + status: + description: Status of the condition, one of True, False, Unknown. + type: string + type: + description: Type of the condition. + type: string + required: + - lastTransitionTime + - lastUpdateTime + - message + - reason + - status + - type + type: object + type: array + ingress: + description: Ingress is a list of endpoints of the exposure mechanism. + items: + description: |- + LoadBalancerIngress represents the status of a load-balancer ingress point: + traffic intended for the service should be sent to an ingress point. + properties: + hostname: + description: |- + Hostname is set for load-balancer ingress points that are DNS based + (typically AWS load-balancers) + type: string + ip: + description: |- + IP is set for load-balancer ingress points that are IP based + (typically GCE or OpenStack load-balancers) + type: string + ipMode: + description: |- + IPMode specifies how the load-balancer IP behaves, and may only be specified when the ip field is specified. + Setting this to "VIP" indicates that traffic is delivered to the node with + the destination set to the load-balancer's IP and port. + Setting this to "Proxy" indicates that traffic is delivered to the node or pod with + the destination set to the node's IP and node port or the pod's IP and port. + Service implementations may use this information to adjust traffic routing. + type: string + ports: + description: |- + Ports is a list of records of service ports + If used, every port defined in the service should have an entry in it + items: + description: PortStatus represents the error condition of + a service port + properties: + error: + description: |- + Error is to record the problem with the service port + The format of the error shall comply with the following rules: + - built-in error values shall be specified in this file and those shall use + CamelCase names + - cloud provider specific error values must have names that comply with the + format foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + port: + description: Port is the port number of the service port + of which status is recorded here + format: int32 + type: integer + protocol: + description: |- + Protocol is the protocol of the service port of which status is recorded here + The supported values are: "TCP", "UDP", "SCTP" + type: string + required: + - error + - port + - protocol + type: object + type: array + x-kubernetes-list-type: atomic + type: object + type: array + lastError: + description: LastError holds information about the last occurred error + during an operation. + properties: + codes: + description: Well-defined error codes of the last error(s). + items: + description: ErrorCode is a string alias. + type: string + type: array + description: + description: A human readable message indicating details about + the last error. + type: string + lastUpdateTime: + description: Last time the error was reported + format: date-time + type: string + taskID: + description: ID of the task which caused this last error + type: string + required: + - description + type: object + lastOperation: + description: LastOperation holds information about the last operation + on the resource. + properties: + description: + description: A human readable message indicating details about + the last operation. + type: string + lastUpdateTime: + description: Last time the operation state transitioned from one + to another. + format: date-time + type: string + progress: + description: The progress in percentage (0-100) of the last operation. + format: int32 + type: integer + state: + description: Status of the last operation, one of Aborted, Processing, + Succeeded, Error, Failed. + type: string + type: + description: Type of the last operation, one of Create, Reconcile, + Delete, Migrate, Restore. + type: string + required: + - description + - lastUpdateTime + - progress + - state + - type + type: object + observedGeneration: + description: ObservedGeneration is the most recent generation observed + for this resource. + format: int64 + type: integer + providerStatus: + description: ProviderStatus contains provider-specific status. + type: object + x-kubernetes-preserve-unknown-fields: true + resources: + description: Resources holds a list of named resource references that + can be referred to in the state by their names. + items: + description: NamedResourceReference is a named reference to a resource. + properties: + name: + description: Name of the resource reference. + type: string + resourceRef: + description: ResourceRef is a reference to a resource. + properties: + apiVersion: + description: apiVersion is the API version of the referent + type: string + kind: + description: 'kind is the kind of the referent; More info: + https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + name: + description: 'name is the name of the referent; More info: + https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' + type: string + required: + - kind + - name + type: object + x-kubernetes-map-type: atomic + required: + - name + - resourceRef + type: object + type: array + state: + description: State can be filled by the operating controller with + what ever data it needs. + type: object + x-kubernetes-preserve-unknown-fields: true + type: object + required: + - spec + type: object + served: true + storage: true + subresources: + status: {} From 0f58533c95c9231373ec3c4cdd06d02d03f9674c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Amand?= Date: Thu, 16 Apr 2026 13:30:49 +0200 Subject: [PATCH 5/9] feat: Add accessControl.allowedSourceRanges to SelfHostedShootExposure providerConfig --- hack/api-reference/api.md | 50 ++++++ .../v1alpha1/types_selfhostedshootexposure.go | 11 ++ .../stackit/v1alpha1/zz_generated.deepcopy.go | 26 ++++ .../validation/selfhostedshootexposure.go | 36 +++++ .../selfhostedshootexposure_test.go | 72 +++++++++ .../selfhostedshootexposure/options.go | 17 +- .../selfhostedshootexposure/options_test.go | 17 ++ .../resources_loadbalancer.go | 30 +++- .../resources_loadbalancer_test.go | 145 ++++++++++++++++++ .../stackit/selfhostedshootexposure_test.go | 89 ++++++++++- 10 files changed, 480 insertions(+), 13 deletions(-) create mode 100644 pkg/apis/stackit/validation/selfhostedshootexposure.go create mode 100644 pkg/apis/stackit/validation/selfhostedshootexposure_test.go diff --git a/hack/api-reference/api.md b/hack/api-reference/api.md index 6559139e..618b213e 100644 --- a/hack/api-reference/api.md +++ b/hack/api-reference/api.md @@ -108,6 +108,44 @@ string +

AccessControlConfig +

+ + +

+(Appears on:LoadBalancerConfig) +

+ +

+AccessControlConfig restricts access to the load balancer by source IP range. +

+ + + + + + + + + + + + + + + + +
FieldDescription
+allowedSourceRanges
+ +string array + +		 +(Optional) +

AllowedSourceRanges is the list of CIDRs permitted to reach the load balancer.
An empty or missing list means no source-IP restriction is applied.

+
+ +

ApplicationLoadBalancerConfig

@@ -1004,6 +1042,18 @@ string

PlanId specifies the service plan (size) of the load balancer.
Currently supported plans are p10, p50, p250, p750 (compare API docs).

+ + +accessControl
+ +AccessControlConfig + + + +(Optional) +

AccessControl restricts which source IP ranges may reach the load balancer.

+ + diff --git a/pkg/apis/stackit/v1alpha1/types_selfhostedshootexposure.go b/pkg/apis/stackit/v1alpha1/types_selfhostedshootexposure.go index 269f2639..e65b9615 100644 --- a/pkg/apis/stackit/v1alpha1/types_selfhostedshootexposure.go +++ b/pkg/apis/stackit/v1alpha1/types_selfhostedshootexposure.go @@ -22,4 +22,15 @@ type LoadBalancerConfig struct { // Currently supported plans are p10, p50, p250, p750 (compare API docs). // +optional PlanId *string `json:"planId,omitempty"` + // AccessControl restricts which source IP ranges may reach the load balancer. + // +optional + AccessControl *AccessControlConfig `json:"accessControl,omitempty"` +} + +// AccessControlConfig restricts access to the load balancer by source IP range. +type AccessControlConfig struct { + // AllowedSourceRanges is the list of CIDRs permitted to reach the load balancer. + // An empty or missing list means no source-IP restriction is applied. + // +optional + AllowedSourceRanges []string `json:"allowedSourceRanges,omitempty"` } diff --git a/pkg/apis/stackit/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/stackit/v1alpha1/zz_generated.deepcopy.go index e37e68ef..b385e5fe 100644 --- a/pkg/apis/stackit/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/stackit/v1alpha1/zz_generated.deepcopy.go @@ -59,6 +59,27 @@ func (in *APIEndpoints) DeepCopy() *APIEndpoints { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AccessControlConfig) DeepCopyInto(out *AccessControlConfig) { + *out = *in + if in.AllowedSourceRanges != nil { + in, out := &in.AllowedSourceRanges, &out.AllowedSourceRanges + *out = make([]string, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccessControlConfig. +func (in *AccessControlConfig) DeepCopy() *AccessControlConfig { + if in == nil { + return nil + } + out := new(AccessControlConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ApplicationLoadBalancerConfig) DeepCopyInto(out *ApplicationLoadBalancerConfig) { *out = *in @@ -486,6 +507,11 @@ func (in *LoadBalancerConfig) DeepCopyInto(out *LoadBalancerConfig) { *out = new(string) **out = **in } + if in.AccessControl != nil { + in, out := &in.AccessControl, &out.AccessControl + *out = new(AccessControlConfig) + (*in).DeepCopyInto(*out) + } return } diff --git a/pkg/apis/stackit/validation/selfhostedshootexposure.go b/pkg/apis/stackit/validation/selfhostedshootexposure.go new file mode 100644 index 00000000..76c1ccc4 --- /dev/null +++ b/pkg/apis/stackit/validation/selfhostedshootexposure.go @@ -0,0 +1,36 @@ +package validation + +import ( + cidrvalidation "github.com/gardener/gardener/pkg/utils/validation/cidr" + "k8s.io/apimachinery/pkg/util/validation/field" + + stackitv1alpha1 "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/apis/stackit/v1alpha1" +) + +// ValidateSelfHostedShootExposureConfig validates a SelfHostedShootExposureConfig object. +func ValidateSelfHostedShootExposureConfig(config *stackitv1alpha1.SelfHostedShootExposureConfig, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if config == nil || config.LoadBalancer == nil { + return allErrs + } + + lbPath := fldPath.Child("loadBalancer") + if config.LoadBalancer.AccessControl != nil { + allErrs = append(allErrs, validateAllowedSourceRanges(config.LoadBalancer.AccessControl.AllowedSourceRanges, lbPath.Child("accessControl", "allowedSourceRanges"))...) + } + + return allErrs +} + +func validateAllowedSourceRanges(ranges []string, fldPath *field.Path) field.ErrorList { + allErrs := make(field.ErrorList, 0, len(ranges)) + cidrs := make([]cidrvalidation.CIDR, 0, len(ranges)) + for i, r := range ranges { + idxPath := fldPath.Index(i) + cidrs = append(cidrs, cidrvalidation.NewCIDR(r, idxPath)) + allErrs = append(allErrs, cidrvalidation.ValidateCIDRIsCanonical(idxPath, r)...) + } + allErrs = append(allErrs, cidrvalidation.ValidateCIDRParse(cidrs...)...) + return allErrs +} diff --git a/pkg/apis/stackit/validation/selfhostedshootexposure_test.go b/pkg/apis/stackit/validation/selfhostedshootexposure_test.go new file mode 100644 index 00000000..a758aa2e --- /dev/null +++ b/pkg/apis/stackit/validation/selfhostedshootexposure_test.go @@ -0,0 +1,72 @@ +package validation_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gstruct" + "k8s.io/apimachinery/pkg/util/validation/field" + + stackitv1alpha1 "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/apis/stackit/v1alpha1" + . "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/apis/stackit/validation" +) + +var _ = Describe("SelfHostedShootExposureConfig validation", func() { + var ( + nilPath *field.Path + config *stackitv1alpha1.SelfHostedShootExposureConfig + ) + + BeforeEach(func() { + config = &stackitv1alpha1.SelfHostedShootExposureConfig{ + LoadBalancer: &stackitv1alpha1.LoadBalancerConfig{ + AccessControl: &stackitv1alpha1.AccessControlConfig{}, + }, + } + }) + + Describe("#ValidateSelfHostedShootExposureConfig", func() { + It("should accept a nil config", func() { + Expect(ValidateSelfHostedShootExposureConfig(nil, nilPath)).To(BeEmpty()) + }) + + It("should accept a config without a load balancer section", func() { + Expect(ValidateSelfHostedShootExposureConfig(&stackitv1alpha1.SelfHostedShootExposureConfig{}, nilPath)).To(BeEmpty()) + }) + + It("should accept a config without an access control section", func() { + config.LoadBalancer.AccessControl = nil + Expect(ValidateSelfHostedShootExposureConfig(config, nilPath)).To(BeEmpty()) + }) + + It("should accept valid canonical CIDRs", func() { + config.LoadBalancer.AccessControl.AllowedSourceRanges = []string{"10.0.0.0/8", "192.168.1.0/24", "2001:db8::/32"} + Expect(ValidateSelfHostedShootExposureConfig(config, nilPath)).To(BeEmpty()) + }) + + It("should reject a malformed CIDR", func() { + config.LoadBalancer.AccessControl.AllowedSourceRanges = []string{"not-a-cidr"} + errs := ValidateSelfHostedShootExposureConfig(config, nilPath) + Expect(errs).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeInvalid), + "Field": Equal("loadBalancer.accessControl.allowedSourceRanges[0]"), + })))) + }) + + It("should reject a non-canonical CIDR", func() { + config.LoadBalancer.AccessControl.AllowedSourceRanges = []string{"10.1.2.3/8"} + errs := ValidateSelfHostedShootExposureConfig(config, nilPath) + Expect(errs).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeInvalid), + "Field": Equal("loadBalancer.accessControl.allowedSourceRanges[0]"), + })))) + }) + + It("should flag each invalid entry with its index", func() { + config.LoadBalancer.AccessControl.AllowedSourceRanges = []string{"10.0.0.0/8", "bad", "also-bad"} + errs := ValidateSelfHostedShootExposureConfig(config, nilPath) + Expect(errs).To(HaveLen(2)) + Expect(errs[0].Field).To(Equal("loadBalancer.accessControl.allowedSourceRanges[1]")) + Expect(errs[1].Field).To(Equal("loadBalancer.accessControl.allowedSourceRanges[2]")) + }) + }) +}) diff --git a/pkg/controller/selfhostedshootexposure/options.go b/pkg/controller/selfhostedshootexposure/options.go index 7c4af9b5..81be1ec8 100644 --- a/pkg/controller/selfhostedshootexposure/options.go +++ b/pkg/controller/selfhostedshootexposure/options.go @@ -9,10 +9,12 @@ import ( extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" reconcilerutils "github.com/gardener/gardener/pkg/controllerutils/reconciler" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/apis/stackit/helper" stackitv1alpha1 "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/apis/stackit/v1alpha1" + "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/apis/stackit/validation" "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/controller/controlplane" "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit" ) @@ -39,6 +41,9 @@ type Options struct { NetworkID string // PlanId specifies the service plan (size) of the load balancer. PlanId string + // AllowedSourceRanges restricts which source CIDRs may reach the load balancer. + // Empty means unrestricted. + AllowedSourceRanges []string } func (a *Actuator) DetermineOptions(ctx context.Context, exposure *extensionsv1alpha1.SelfHostedShootExposure, cluster *extensionscontroller.Cluster, projectID string) (*Options, error) { @@ -68,8 +73,16 @@ func (a *Actuator) DetermineOptions(ctx context.Context, exposure *extensionsv1a if _, _, err := a.Decoder.Decode(exposure.Spec.ProviderConfig.Raw, nil, providerConfig); err != nil { return nil, fmt.Errorf("error decoding providerConfig: %w", err) } - if providerConfig.LoadBalancer != nil && providerConfig.LoadBalancer.PlanId != nil { - opts.PlanId = *providerConfig.LoadBalancer.PlanId + if errs := validation.ValidateSelfHostedShootExposureConfig(providerConfig, field.NewPath("providerConfig")); len(errs) > 0 { + return nil, fmt.Errorf("invalid providerConfig: %w", errs.ToAggregate()) + } + if providerConfig.LoadBalancer != nil { + if providerConfig.LoadBalancer.PlanId != nil { + opts.PlanId = *providerConfig.LoadBalancer.PlanId + } + if providerConfig.LoadBalancer.AccessControl != nil { + opts.AllowedSourceRanges = providerConfig.LoadBalancer.AccessControl.AllowedSourceRanges + } } } // Default plan if not specified diff --git a/pkg/controller/selfhostedshootexposure/options_test.go b/pkg/controller/selfhostedshootexposure/options_test.go index 5020581e..fdef94b9 100644 --- a/pkg/controller/selfhostedshootexposure/options_test.go +++ b/pkg/controller/selfhostedshootexposure/options_test.go @@ -144,6 +144,23 @@ var _ = Describe("Options", func() { Expect(opts.PlanId).To(Equal("p250")) }) + It("should reject an invalid AllowedSourceRanges CIDR", func() { + encoder := serializer.NewCodecFactory(fakeClient.Scheme()).EncoderForVersion(&json.Serializer{}, stackitv1alpha1.SchemeGroupVersion) + providerConfig := &stackitv1alpha1.SelfHostedShootExposureConfig{ + LoadBalancer: &stackitv1alpha1.LoadBalancerConfig{ + AccessControl: &stackitv1alpha1.AccessControlConfig{ + AllowedSourceRanges: []string{"not-a-cidr"}, + }, + }, + } + providerConfigBytes, err := runtime.Encode(encoder, providerConfig) + Expect(err).NotTo(HaveOccurred()) + exposure.Spec.ProviderConfig = &runtime.RawExtension{Raw: providerConfigBytes} + + _, err = a.DetermineOptions(ctx, exposure, cluster, projectID) + Expect(err).To(MatchError(ContainSubstring("allowedSourceRanges[0]"))) + }) + It("should handle the RegionOne value", func() { shoot.Spec.Region = "RegionOne" diff --git a/pkg/controller/selfhostedshootexposure/resources_loadbalancer.go b/pkg/controller/selfhostedshootexposure/resources_loadbalancer.go index c8245bb5..43f28e47 100644 --- a/pkg/controller/selfhostedshootexposure/resources_loadbalancer.go +++ b/pkg/controller/selfhostedshootexposure/resources_loadbalancer.go @@ -3,6 +3,7 @@ package selfhostedshootexposure import ( "context" "fmt" + "slices" "sort" "strings" "time" @@ -51,15 +52,15 @@ func (r *Resources) reconcileLoadBalancer(ctx context.Context, log logr.Logger) if err != nil { return err } - planNeedsUpdate := r.planNeedsUpdate() + fullStateNeedsUpdate := r.planNeedsUpdate() || r.accessControlNeedsUpdate() - if !targetPoolNeedsUpdate && !planNeedsUpdate { + if !targetPoolNeedsUpdate && !fullStateNeedsUpdate { return nil } // Fast path: only targets changed (e.g. control-plane node added/removed). The sub-resource // endpoint is scoped to the target pool, so we avoid re-sending the full LB state. - if targetPoolNeedsUpdate && !planNeedsUpdate { + if targetPoolNeedsUpdate && !fullStateNeedsUpdate { return r.updateTargetPool(ctx, log, targets) } @@ -185,6 +186,11 @@ func (r *Resources) desiredOptions() *loadbalancer.LoadBalancerOptions { if r.LoadBalancer == nil { opts.EphemeralAddress = new(true) } + if len(r.AllowedSourceRanges) > 0 { + opts.AccessControl = &loadbalancer.LoadbalancerOptionAccessControl{ + AllowedSourceRanges: r.AllowedSourceRanges, + } + } return opts } @@ -196,6 +202,24 @@ func (r *Resources) planNeedsUpdate() bool { return currentPlan != r.PlanId } +// accessControlNeedsUpdate reports whether the LB's currently configured source-IP allowlist +// differs from the desired set. The desired set is order-independent; we compare as sorted lists. +// An empty desired list means "no restriction" — detected by diff against whatever the LB reports. +func (r *Resources) accessControlNeedsUpdate() bool { + var current []string + if r.LoadBalancer != nil && + r.LoadBalancer.Options != nil && + r.LoadBalancer.Options.AccessControl != nil { + current = r.LoadBalancer.Options.AccessControl.AllowedSourceRanges + } + return !stringSetsEqual(current, r.AllowedSourceRanges) +} + +// stringSetsEqual compares two string slices as unordered sets. +func stringSetsEqual(a, b []string) bool { + return slices.Equal(slices.Sorted(slices.Values(a)), slices.Sorted(slices.Values(b))) +} + // wrapLBAPIError classifies STACKIT LB API errors: 409 Conflicts are transient (another caller // modified the LB between our GET and our write) and are retried via RequeueAfterError; anything // else is returned as a regular error so Gardener can classify + surface it. diff --git a/pkg/controller/selfhostedshootexposure/resources_loadbalancer_test.go b/pkg/controller/selfhostedshootexposure/resources_loadbalancer_test.go index b79170b7..d132312f 100644 --- a/pkg/controller/selfhostedshootexposure/resources_loadbalancer_test.go +++ b/pkg/controller/selfhostedshootexposure/resources_loadbalancer_test.go @@ -111,6 +111,30 @@ var _ = Describe("reconcileLoadBalancer", func() { Expect(r.LoadBalancer).To(Equal(createdLB)) }) + It("should create a load balancer with AccessControl when AllowedSourceRanges is set", func() { + r.AllowedSourceRanges = []string{"10.0.0.0/8", "192.168.0.0/16"} + r.SelfHostedShootExposure.Spec.Endpoints = []extensionsv1alpha1.ControlPlaneEndpoint{ + { + NodeName: "node-1", + Addresses: []corev1.NodeAddress{ + {Type: corev1.NodeInternalIP, Address: "10.0.1.10"}, + }, + }, + } + + mockLBClient.EXPECT(). + CreateLoadBalancer(ctx, gomock.Any()). + DoAndReturn(func(_ context.Context, payload loadbalancer.CreateLoadBalancerPayload) (*loadbalancer.LoadBalancer, error) { + Expect(payload.Options).NotTo(BeNil()) + Expect(payload.Options.AccessControl).NotTo(BeNil()) + Expect(payload.Options.AccessControl.AllowedSourceRanges).To(ConsistOf("10.0.0.0/8", "192.168.0.0/16")) + return &loadbalancer.LoadBalancer{}, nil + }) + + err := r.reconcileLoadBalancer(ctx, logger) + Expect(err).NotTo(HaveOccurred()) + }) + It("should return error when CreateLoadBalancer fails", func() { r.SelfHostedShootExposure.Spec.Endpoints = []extensionsv1alpha1.ControlPlaneEndpoint{ { @@ -205,6 +229,41 @@ var _ = Describe("reconcileLoadBalancer", func() { Expect(err).NotTo(HaveOccurred()) }) + It("should do nothing when LB already has the same AllowedSourceRanges, in any order", func() { + r.AllowedSourceRanges = []string{"10.0.0.0/8", "192.168.0.0/16"} + // LB returns the set in the opposite order — diff must be set-based, not sequence-based. + r.LoadBalancer.Options = &loadbalancer.LoadBalancerOptions{ + AccessControl: &loadbalancer.LoadbalancerOptionAccessControl{ + AllowedSourceRanges: []string{"192.168.0.0/16", "10.0.0.0/8"}, + }, + } + + // No mock expectations — nothing should be called. + err := r.reconcileLoadBalancer(ctx, logger) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should update via UpdateLoadBalancer when AllowedSourceRanges changed", func() { + r.AllowedSourceRanges = []string{"10.0.0.0/8", "172.16.0.0/12"} + r.LoadBalancer.Options = &loadbalancer.LoadBalancerOptions{ + AccessControl: &loadbalancer.LoadbalancerOptionAccessControl{ + AllowedSourceRanges: []string{"10.0.0.0/8"}, + }, + } + + mockLBClient.EXPECT(). + UpdateLoadBalancer(ctx, "test-lb", gomock.Any()). + DoAndReturn(func(_ context.Context, _ string, payload loadbalancer.UpdateLoadBalancerPayload) (*loadbalancer.LoadBalancer, error) { + Expect(payload.Options).NotTo(BeNil()) + Expect(payload.Options.AccessControl).NotTo(BeNil()) + Expect(payload.Options.AccessControl.AllowedSourceRanges).To(ConsistOf("10.0.0.0/8", "172.16.0.0/12")) + return &loadbalancer.LoadBalancer{}, nil + }) + + err := r.reconcileLoadBalancer(ctx, logger) + Expect(err).NotTo(HaveOccurred()) + }) + It("should update plan via UpdateLoadBalancer when only plan changed", func() { r.PlanId = "p100" // Changed plan @@ -634,3 +693,89 @@ var _ = Describe("targetPoolNeedsUpdate", func() { Expect(needsUpdate).To(BeFalse()) }) }) + +var _ = Describe("accessControlNeedsUpdate", func() { + var r *Resources + + BeforeEach(func() { + r = &Resources{} + }) + + It("should return false when neither desired nor current has any ranges", func() { + r.LoadBalancer = &loadbalancer.LoadBalancer{} + Expect(r.accessControlNeedsUpdate()).To(BeFalse()) + }) + + It("should return true when desired is set but LB has no Options", func() { + r.AllowedSourceRanges = []string{"10.0.0.0/8"} + r.LoadBalancer = &loadbalancer.LoadBalancer{} + Expect(r.accessControlNeedsUpdate()).To(BeTrue()) + }) + + It("should return true when desired is set but LB has no AccessControl", func() { + r.AllowedSourceRanges = []string{"10.0.0.0/8"} + r.LoadBalancer = &loadbalancer.LoadBalancer{ + Options: &loadbalancer.LoadBalancerOptions{}, + } + Expect(r.accessControlNeedsUpdate()).To(BeTrue()) + }) + + It("should return true when desired is empty but LB has ranges", func() { + r.LoadBalancer = &loadbalancer.LoadBalancer{ + Options: &loadbalancer.LoadBalancerOptions{ + AccessControl: &loadbalancer.LoadbalancerOptionAccessControl{ + AllowedSourceRanges: []string{"10.0.0.0/8"}, + }, + }, + } + Expect(r.accessControlNeedsUpdate()).To(BeTrue()) + }) + + It("should return false when sets match in the same order", func() { + r.AllowedSourceRanges = []string{"10.0.0.0/8", "192.168.0.0/16"} + r.LoadBalancer = &loadbalancer.LoadBalancer{ + Options: &loadbalancer.LoadBalancerOptions{ + AccessControl: &loadbalancer.LoadbalancerOptionAccessControl{ + AllowedSourceRanges: []string{"10.0.0.0/8", "192.168.0.0/16"}, + }, + }, + } + Expect(r.accessControlNeedsUpdate()).To(BeFalse()) + }) + + It("should return false when sets match in a different order", func() { + r.AllowedSourceRanges = []string{"10.0.0.0/8", "192.168.0.0/16"} + r.LoadBalancer = &loadbalancer.LoadBalancer{ + Options: &loadbalancer.LoadBalancerOptions{ + AccessControl: &loadbalancer.LoadbalancerOptionAccessControl{ + AllowedSourceRanges: []string{"192.168.0.0/16", "10.0.0.0/8"}, + }, + }, + } + Expect(r.accessControlNeedsUpdate()).To(BeFalse()) + }) + + It("should return true when one range differs", func() { + r.AllowedSourceRanges = []string{"10.0.0.0/8", "192.168.0.0/16"} + r.LoadBalancer = &loadbalancer.LoadBalancer{ + Options: &loadbalancer.LoadBalancerOptions{ + AccessControl: &loadbalancer.LoadbalancerOptionAccessControl{ + AllowedSourceRanges: []string{"10.0.0.0/8", "172.16.0.0/12"}, + }, + }, + } + Expect(r.accessControlNeedsUpdate()).To(BeTrue()) + }) + + It("should return true when lengths differ", func() { + r.AllowedSourceRanges = []string{"10.0.0.0/8"} + r.LoadBalancer = &loadbalancer.LoadBalancer{ + Options: &loadbalancer.LoadBalancerOptions{ + AccessControl: &loadbalancer.LoadbalancerOptionAccessControl{ + AllowedSourceRanges: []string{"10.0.0.0/8", "192.168.0.0/16"}, + }, + }, + } + Expect(r.accessControlNeedsUpdate()).To(BeTrue()) + }) +}) diff --git a/test/integration/selfhostedshootexposure/stackit/selfhostedshootexposure_test.go b/test/integration/selfhostedshootexposure/stackit/selfhostedshootexposure_test.go index 1bcf58cc..8915b36e 100644 --- a/test/integration/selfhostedshootexposure/stackit/selfhostedshootexposure_test.go +++ b/test/integration/selfhostedshootexposure/stackit/selfhostedshootexposure_test.go @@ -529,7 +529,7 @@ var _ = Describe("SelfHostedShootExposure tests", func() { By("change plan only (full PUT update)") versionBeforePlanChange := *lb.Version patchExposureReconcile(exposure, func() { - setExposurePlanId(exposure, "p50") + setExposureLBConfig(exposure, &stackitv1alpha1.LoadBalancerConfig{PlanId: new("p50")}) }) Eventually(func(g Gomega) { @@ -551,7 +551,7 @@ var _ = Describe("SelfHostedShootExposure tests", func() { By("change plan and endpoints together (full PUT update)") versionBeforeCombined := *lb.Version patchExposureReconcile(exposure, func() { - setExposurePlanId(exposure, "p250") + setExposureLBConfig(exposure, &stackitv1alpha1.LoadBalancerConfig{PlanId: new("p250")}) // Add node-2 back, so we're changing targets *and* plan in the same reconcile. exposure.Spec.Endpoints = append(exposure.Spec.Endpoints, extensionsv1alpha1.ControlPlaneEndpoint{ NodeName: "node-2", @@ -574,6 +574,81 @@ var _ = Describe("SelfHostedShootExposure tests", func() { g.Expect(*lb.Version).NotTo(Equal(versionBeforeCombined)) }).WithTimeout(2 * time.Minute).WithPolling(5 * time.Second).Should(Succeed()) + By("add AccessControl.AllowedSourceRanges (full PUT update)") + versionBeforeACLAdd := *lb.Version + patchExposureReconcile(exposure, func() { + setExposureLBConfig(exposure, &stackitv1alpha1.LoadBalancerConfig{ + PlanId: new("p250"), + AccessControl: &stackitv1alpha1.AccessControlConfig{ + AllowedSourceRanges: []string{"0.0.0.0/0"}, + }, + }) + }) + + Eventually(func(g Gomega) { + var err error + lb, err = lbClient.GetLoadBalancer(ctx, lbName) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(lb.Options).NotTo(BeNil()) + g.Expect(lb.Options.AccessControl).NotTo(BeNil()) + g.Expect(lb.Options.AccessControl.AllowedSourceRanges).To(ConsistOf("0.0.0.0/0")) + + g.Expect(lb.Version).NotTo(BeNil()) + g.Expect(*lb.Version).NotTo(Equal(versionBeforeACLAdd)) + + // Plan + targets unchanged by AccessControl-only addition. + g.Expect(*lb.PlanId).To(Equal("p250")) + g.Expect(lb.TargetPools[0].Targets).To(HaveLen(3)) + }).WithTimeout(2 * time.Minute).WithPolling(5 * time.Second).Should(Succeed()) + + By("update AllowedSourceRanges to a different set (order-independent)") + versionBeforeACLUpdate := *lb.Version + patchExposureReconcile(exposure, func() { + setExposureLBConfig(exposure, &stackitv1alpha1.LoadBalancerConfig{ + PlanId: new("p250"), + AccessControl: &stackitv1alpha1.AccessControlConfig{ + AllowedSourceRanges: []string{"192.168.0.0/16", "10.0.0.0/8"}, + }, + }) + }) + + Eventually(func(g Gomega) { + var err error + lb, err = lbClient.GetLoadBalancer(ctx, lbName) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(lb.Options).NotTo(BeNil()) + g.Expect(lb.Options.AccessControl).NotTo(BeNil()) + g.Expect(lb.Options.AccessControl.AllowedSourceRanges).To(ConsistOf("10.0.0.0/8", "192.168.0.0/16")) + + g.Expect(lb.Version).NotTo(BeNil()) + g.Expect(*lb.Version).NotTo(Equal(versionBeforeACLUpdate)) + }).WithTimeout(2 * time.Minute).WithPolling(5 * time.Second).Should(Succeed()) + + By("remove AccessControl (full PUT update)") + versionBeforeACLRemove := *lb.Version + patchExposureReconcile(exposure, func() { + // Drop AccessControl entirely from the providerConfig. + setExposureLBConfig(exposure, &stackitv1alpha1.LoadBalancerConfig{PlanId: new("p250")}) + }) + + Eventually(func(g Gomega) { + var err error + lb, err = lbClient.GetLoadBalancer(ctx, lbName) + g.Expect(err).NotTo(HaveOccurred()) + // After removal, the LB API may report Options.AccessControl as nil OR with an empty + // AllowedSourceRanges slice; both mean "unrestricted". + if lb.Options != nil && lb.Options.AccessControl != nil { + g.Expect(lb.Options.AccessControl.AllowedSourceRanges).To(BeEmpty()) + } + + g.Expect(lb.Version).NotTo(BeNil()) + g.Expect(*lb.Version).NotTo(Equal(versionBeforeACLRemove)) + + // Plan + targets unchanged. + g.Expect(*lb.PlanId).To(Equal("p250")) + g.Expect(lb.TargetPools[0].Targets).To(HaveLen(3)) + }).WithTimeout(2 * time.Minute).WithPolling(5 * time.Second).Should(Succeed()) + By("delete SelfHostedShootExposure") Expect(client.IgnoreNotFound(c.Delete(ctx, exposure))).To(Succeed()) @@ -620,15 +695,13 @@ func targetIPs(lb *loadbalancer.LoadBalancer) []string { return ips } -// setExposurePlanId encodes a SelfHostedShootExposureConfig with the given plan and sets it -// as the exposure's ProviderConfig. -func setExposurePlanId(exposure *extensionsv1alpha1.SelfHostedShootExposure, planId string) { +// setExposureLBConfig encodes a SelfHostedShootExposureConfig wrapping lbConfig and sets it +// as the exposure's ProviderConfig. Fully replaces any previous ProviderConfig. +func setExposureLBConfig(exposure *extensionsv1alpha1.SelfHostedShootExposure, lbConfig *stackitv1alpha1.LoadBalancerConfig) { GinkgoHelper() buf := new(bytes.Buffer) Expect(encoder.Encode(&stackitv1alpha1.SelfHostedShootExposureConfig{ - LoadBalancer: &stackitv1alpha1.LoadBalancerConfig{ - PlanId: &planId, - }, + LoadBalancer: lbConfig, }, buf)).To(Succeed()) exposure.Spec.ProviderConfig = &runtime.RawExtension{Raw: buf.Bytes()} } From a8cb8e7589f882da2a751d8382e598e620f38453 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Amand?= Date: Tue, 21 Apr 2026 15:12:34 +0200 Subject: [PATCH 6/9] fix: Typos + AllowedSourceRanges deduplication (PR feedback) --- docs/cloudprovider.md | 2 +- pkg/controller/selfhostedshootexposure/actuator.go | 2 +- .../resources_loadbalancer.go | 14 +++++++++++--- .../resources_loadbalancer_test.go | 14 ++++++++++++++ 4 files changed, 27 insertions(+), 5 deletions(-) diff --git a/docs/cloudprovider.md b/docs/cloudprovider.md index 0a5e13c6..ed8f94cc 100644 --- a/docs/cloudprovider.md +++ b/docs/cloudprovider.md @@ -33,7 +33,7 @@ The service account needs the following permissions: | `blockstorage.admin` | CSI driver | | `compute.admin` | CCM node-controller and MCM | | `iaas.network.admin` | bastion and infrastructure controller | -| `iaas.isoplated-network.admin` | infrastructure controller | +| `iaas.isolated-network.admin` | infrastructure controller | ## CloudProfileConfig Fields diff --git a/pkg/controller/selfhostedshootexposure/actuator.go b/pkg/controller/selfhostedshootexposure/actuator.go index e7705f6d..836291de 100644 --- a/pkg/controller/selfhostedshootexposure/actuator.go +++ b/pkg/controller/selfhostedshootexposure/actuator.go @@ -71,7 +71,7 @@ func (a *Actuator) delete(ctx context.Context, log logr.Logger, exposure *extens } if err := r.deleteLoadBalancer(ctx, log); err != nil { - return fmt.Errorf("error deleting loadbalancer: %w", err) + return fmt.Errorf("error deleting load balancer: %w", err) } return nil diff --git a/pkg/controller/selfhostedshootexposure/resources_loadbalancer.go b/pkg/controller/selfhostedshootexposure/resources_loadbalancer.go index 43f28e47..6c4ab2e4 100644 --- a/pkg/controller/selfhostedshootexposure/resources_loadbalancer.go +++ b/pkg/controller/selfhostedshootexposure/resources_loadbalancer.go @@ -215,9 +215,17 @@ func (r *Resources) accessControlNeedsUpdate() bool { return !stringSetsEqual(current, r.AllowedSourceRanges) } -// stringSetsEqual compares two string slices as unordered sets. +// stringSetsEqual compares two string slices as unordered sets, ignoring duplicates. +// The LB API may or may not de-duplicate (causing reconciliation loop), remove duplicates +// for a clean approach. func stringSetsEqual(a, b []string) bool { - return slices.Equal(slices.Sorted(slices.Values(a)), slices.Sorted(slices.Values(b))) + return slices.Equal(sortedUnique(a), sortedUnique(b)) +} + +// sortedUnique returns the input as a sorted slice with consecutive duplicates removed, +// i.e. the canonical representation of the set. +func sortedUnique(s []string) []string { + return slices.Compact(slices.Sorted(slices.Values(s))) } // wrapLBAPIError classifies STACKIT LB API errors: 409 Conflicts are transient (another caller @@ -357,7 +365,7 @@ func (r *Resources) buildTargets() ([]loadbalancer.Target, error) { } // extractInternalIP finds and returns the internal IP address from an endpoint's addresses. -// This function requires InternalIP because the STACKIT LB only supposts IPs as target. +// This function requires InternalIP because the STACKIT LB only supports IPs as target. func extractInternalIP(endpoint *extensionsv1alpha1.ControlPlaneEndpoint) (string, error) { for _, addr := range endpoint.Addresses { if addr.Type == corev1.NodeInternalIP { diff --git a/pkg/controller/selfhostedshootexposure/resources_loadbalancer_test.go b/pkg/controller/selfhostedshootexposure/resources_loadbalancer_test.go index d132312f..9287e87c 100644 --- a/pkg/controller/selfhostedshootexposure/resources_loadbalancer_test.go +++ b/pkg/controller/selfhostedshootexposure/resources_loadbalancer_test.go @@ -755,6 +755,20 @@ var _ = Describe("accessControlNeedsUpdate", func() { Expect(r.accessControlNeedsUpdate()).To(BeFalse()) }) + It("should return false when desired has duplicates but LB has the de-duped set", func() { + // The LB API may normalize/de-duplicate; treating duplicates as significant would + // oscillate the diff and cause unnecessary full-state PUTs. + r.AllowedSourceRanges = []string{"10.0.0.0/8", "192.168.0.0/16", "10.0.0.0/8"} + r.LoadBalancer = &loadbalancer.LoadBalancer{ + Options: &loadbalancer.LoadBalancerOptions{ + AccessControl: &loadbalancer.LoadbalancerOptionAccessControl{ + AllowedSourceRanges: []string{"10.0.0.0/8", "192.168.0.0/16"}, + }, + }, + } + Expect(r.accessControlNeedsUpdate()).To(BeFalse()) + }) + It("should return true when one range differs", func() { r.AllowedSourceRanges = []string{"10.0.0.0/8", "192.168.0.0/16"} r.LoadBalancer = &loadbalancer.LoadBalancer{ From 351f3fb28f95693b1f0f4cda2a17a6a97ecd053e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Amand?= Date: Mon, 27 Apr 2026 16:05:27 +0200 Subject: [PATCH 7/9] fix: Improve SelfHostedShootExposure controller after PR feedback Co-authored-by: Tim Ebert --- Makefile | 1 + .../templates/deployment.yaml | 2 +- docs/testing.md | 17 + hack/api-reference/api.md | 18 +- pkg/apis/stackit/v1alpha1/defaults.go | 12 + pkg/apis/stackit/v1alpha1/register.go | 8 +- .../v1alpha1/types_selfhostedshootexposure.go | 18 +- .../stackit/v1alpha1/zz_generated.deepcopy.go | 24 +- .../stackit/v1alpha1/zz_generated.defaults.go | 7 + .../validation/selfhostedshootexposure.go | 1 - .../selfhostedshootexposure_test.go | 4 +- .../selfhostedshootexposure/actuator.go | 27 +- .../selfhostedshootexposure/actuator_test.go | 45 ++- .../selfhostedshootexposure/options.go | 54 ++-- .../selfhostedshootexposure/options_test.go | 14 +- .../selfhostedshootexposure/resources.go | 10 +- .../resources_loadbalancer.go | 282 +++++------------ .../resources_loadbalancer_test.go | 293 +++--------------- pkg/stackit/client/loadbalancing.go | 45 ++- pkg/stackit/client/mock/loadbalancing_mock.go | 32 +- .../stackit/selfhostedshootexposure_test.go | 96 +++--- 21 files changed, 375 insertions(+), 635 deletions(-) diff --git a/Makefile b/Makefile index b337d074..38bb3bc1 100644 --- a/Makefile +++ b/Makefile @@ -148,6 +148,7 @@ cleanup-crds: gardener-crds: @cp $(GARDENER_DIR)/example/seed-crds/10-crd-extensions.gardener.cloud_clusters.yaml $(UPSTREAM_CRDS_DIR) @cp $(GARDENER_DIR)/example/seed-crds/10-crd-extensions.gardener.cloud_infrastructures.yaml $(UPSTREAM_CRDS_DIR) + @cp $(GARDENER_DIR)/example/seed-crds/10-crd-extensions.gardener.cloud_selfhostedshootexposures.yaml $(UPSTREAM_CRDS_DIR) .PHONY: test test: $(REPORT_COLLECTOR) $(SETUP_ENVTEST) ## Runs the unit-test suite diff --git a/charts/gardener-extension-provider-stackit/templates/deployment.yaml b/charts/gardener-extension-provider-stackit/templates/deployment.yaml index 37e91247..73882df6 100644 --- a/charts/gardener-extension-provider-stackit/templates/deployment.yaml +++ b/charts/gardener-extension-provider-stackit/templates/deployment.yaml @@ -66,8 +66,8 @@ spec: - --heartbeat-namespace={{ .Release.Namespace }} - --heartbeat-renew-interval-seconds={{ .Values.controllers.heartbeat.renewIntervalSeconds }} - --infrastructure-max-concurrent-reconciles={{ .Values.controllers.infrastructure.concurrentSyncs }} - - --selfhostedshootexposure-max-concurrent-reconciles={{ .Values.controllers.selfhostedshootexposure.concurrentSyncs }} - --ignore-operation-annotation={{ .Values.controllers.ignoreOperationAnnotation }} + - --selfhostedshootexposure-max-concurrent-reconciles={{ .Values.controllers.selfhostedshootexposure.concurrentSyncs }} - --worker-max-concurrent-reconciles={{ .Values.controllers.worker.concurrentSyncs }} - --webhook-config-namespace={{ .Release.Namespace }} - --webhook-config-service-port={{ .Values.webhookConfig.servicePort }} diff --git a/docs/testing.md b/docs/testing.md index 9f7f5eb6..c8d7fe21 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -1,5 +1,7 @@ # Integration Tests +## Infrastructure + The infraflow code path of the infrastructure controller contains nearly no unit tests, as it primarily consists of code interacting with the STACKIT IaaS API. Instead, these code paths are tested via the `infrastructure` integration tests. @@ -15,3 +17,18 @@ make test-integration-infra The `STACKIT_SERVICE_ACCOUNT_KEY` is simply the JSON struct obtained from the Portal or API, when creating a new Service-Account key. Additionally the ServiceAccount also needs to have the `iaas.network.admin` as well as the `iaas.isolated-network.admin` roles in-order to create all necessary resources via the API. + +## SelfHostedShootExposure + +The `selfhostedshootexposure` controller provisions an NLB through the STACKIT Network Load Balancer API. Run the integration tests via: + +```bash +export STACKIT_PROJECT_ID= +export STACKIT_SERVICE_ACCOUNT_KEY=$(pbpaste) +make test-integration-exposure +``` + +The ServiceAccount needs the same level of access as the infra tests (it provisions an isolated +network used as the LB's target network) plus permissions to create/update/delete STACKIT load +balancers. In practice, the `owner` role on the project (or a parent folder) covers everything +the suite does. diff --git a/hack/api-reference/api.md b/hack/api-reference/api.md index 618b213e..8bbbc206 100644 --- a/hack/api-reference/api.md +++ b/hack/api-reference/api.md @@ -108,16 +108,16 @@ string -

AccessControlConfig +

AccessControl

-(Appears on:LoadBalancerConfig) +(Appears on:LoadBalancer)

-AccessControlConfig restricts access to the load balancer by source IP range. +AccessControl restricts access to the load balancer by source IP range.

@@ -1009,7 +1009,7 @@ string
-

LoadBalancerConfig +

LoadBalancer

@@ -1018,7 +1018,7 @@ string

-LoadBalancerConfig contains configuration for the load balancer. +LoadBalancer contains configuration for the load balancer.

@@ -1032,21 +1032,21 @@ LoadBalancerConfig contains configuration for the load balancer.
-planId
+planID
string 		(Optional) -

PlanId specifies the service plan (size) of the load balancer.
Currently supported plans are p10, p50, p250, p750 (compare API docs).

+

PlanID specifies the service plan (size) of the load balancer.
Currently supported plans are p10, p50, p250, p750 (compare API docs).
See https://docs.stackit.cloud/products/network/load-balancing-and-content-delivery/network-load-balancer/reference/service-plans/
Defaults to "p10".
accessControl
-AccessControlConfig +AccessControl 		@@ -1795,7 +1795,7 @@ SelfHostedShootExposureConfig contains configuration settings for exposing self- loadBalancer
-LoadBalancerConfig +LoadBalancer 		diff --git a/pkg/apis/stackit/v1alpha1/defaults.go b/pkg/apis/stackit/v1alpha1/defaults.go index ef901072..d72cba93 100644 --- a/pkg/apis/stackit/v1alpha1/defaults.go +++ b/pkg/apis/stackit/v1alpha1/defaults.go @@ -8,10 +8,22 @@ import ( "k8s.io/apimachinery/pkg/runtime" ) +// DefaultLoadBalancerPlanID is the default LB service plan for SelfHostedShootExposure. +const DefaultLoadBalancerPlanID = "p10" + func addDefaultingFuncs(scheme *runtime.Scheme) error { return RegisterDefaults(scheme) } +func SetDefaults_SelfHostedShootExposureConfig(obj *SelfHostedShootExposureConfig) { + if obj.LoadBalancer == nil { + obj.LoadBalancer = &LoadBalancer{} + } + if obj.LoadBalancer.PlanID == nil { + obj.LoadBalancer.PlanID = new(DefaultLoadBalancerPlanID) + } +} + func SetDefaults_ControlPlaneConfig(obj *ControlPlaneConfig) { if obj == nil { obj = &ControlPlaneConfig{} diff --git a/pkg/apis/stackit/v1alpha1/register.go b/pkg/apis/stackit/v1alpha1/register.go index eff8ac87..0b2ed24f 100644 --- a/pkg/apis/stackit/v1alpha1/register.go +++ b/pkg/apis/stackit/v1alpha1/register.go @@ -39,13 +39,13 @@ func init() { func addKnownTypes(scheme *runtime.Scheme) error { scheme.AddKnownTypes(SchemeGroupVersion, &CloudProfileConfig{}, + &ControlPlaneConfig{}, &InfrastructureConfig{}, - &InfrastructureStatus{}, &InfrastructureState{}, - &ControlPlaneConfig{}, - &WorkerStatus{}, - &WorkerConfig{}, + &InfrastructureStatus{}, &SelfHostedShootExposureConfig{}, + &WorkerConfig{}, + &WorkerStatus{}, ) return nil } diff --git a/pkg/apis/stackit/v1alpha1/types_selfhostedshootexposure.go b/pkg/apis/stackit/v1alpha1/types_selfhostedshootexposure.go index e65b9615..c081eec6 100644 --- a/pkg/apis/stackit/v1alpha1/types_selfhostedshootexposure.go +++ b/pkg/apis/stackit/v1alpha1/types_selfhostedshootexposure.go @@ -13,22 +13,24 @@ type SelfHostedShootExposureConfig struct { // LoadBalancer contains configuration for the load balancer. // +optional - LoadBalancer *LoadBalancerConfig `json:"loadBalancer,omitempty"` + LoadBalancer *LoadBalancer `json:"loadBalancer,omitempty"` } -// LoadBalancerConfig contains configuration for the load balancer. -type LoadBalancerConfig struct { - // PlanId specifies the service plan (size) of the load balancer. +// LoadBalancer contains configuration for the load balancer. +type LoadBalancer struct { + // PlanID specifies the service plan (size) of the load balancer. // Currently supported plans are p10, p50, p250, p750 (compare API docs). + // See https://docs.stackit.cloud/products/network/load-balancing-and-content-delivery/network-load-balancer/reference/service-plans/ + // Defaults to "p10". // +optional - PlanId *string `json:"planId,omitempty"` + PlanID *string `json:"planID,omitempty"` // AccessControl restricts which source IP ranges may reach the load balancer. // +optional - AccessControl *AccessControlConfig `json:"accessControl,omitempty"` + AccessControl *AccessControl `json:"accessControl,omitempty"` } -// AccessControlConfig restricts access to the load balancer by source IP range. -type AccessControlConfig struct { +// AccessControl restricts access to the load balancer by source IP range. +type AccessControl struct { // AllowedSourceRanges is the list of CIDRs permitted to reach the load balancer. // An empty or missing list means no source-IP restriction is applied. // +optional diff --git a/pkg/apis/stackit/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/stackit/v1alpha1/zz_generated.deepcopy.go index b385e5fe..5225f019 100644 --- a/pkg/apis/stackit/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/stackit/v1alpha1/zz_generated.deepcopy.go @@ -60,7 +60,7 @@ func (in *APIEndpoints) DeepCopy() *APIEndpoints { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *AccessControlConfig) DeepCopyInto(out *AccessControlConfig) { +func (in *AccessControl) DeepCopyInto(out *AccessControl) { *out = *in if in.AllowedSourceRanges != nil { in, out := &in.AllowedSourceRanges, &out.AllowedSourceRanges @@ -70,12 +70,12 @@ func (in *AccessControlConfig) DeepCopyInto(out *AccessControlConfig) { return } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccessControlConfig. -func (in *AccessControlConfig) DeepCopy() *AccessControlConfig { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccessControl. +func (in *AccessControl) DeepCopy() *AccessControl { if in == nil { return nil } - out := new(AccessControlConfig) + out := new(AccessControl) in.DeepCopyInto(out) return out } @@ -500,27 +500,27 @@ func (in *KeyStoneURL) DeepCopy() *KeyStoneURL { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *LoadBalancerConfig) DeepCopyInto(out *LoadBalancerConfig) { +func (in *LoadBalancer) DeepCopyInto(out *LoadBalancer) { *out = *in - if in.PlanId != nil { - in, out := &in.PlanId, &out.PlanId + if in.PlanID != nil { + in, out := &in.PlanID, &out.PlanID *out = new(string) **out = **in } if in.AccessControl != nil { in, out := &in.AccessControl, &out.AccessControl - *out = new(AccessControlConfig) + *out = new(AccessControl) (*in).DeepCopyInto(*out) } return } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LoadBalancerConfig. -func (in *LoadBalancerConfig) DeepCopy() *LoadBalancerConfig { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LoadBalancer. +func (in *LoadBalancer) DeepCopy() *LoadBalancer { if in == nil { return nil } - out := new(LoadBalancerConfig) + out := new(LoadBalancer) in.DeepCopyInto(out) return out } @@ -779,7 +779,7 @@ func (in *SelfHostedShootExposureConfig) DeepCopyInto(out *SelfHostedShootExposu out.TypeMeta = in.TypeMeta if in.LoadBalancer != nil { in, out := &in.LoadBalancer, &out.LoadBalancer - *out = new(LoadBalancerConfig) + *out = new(LoadBalancer) (*in).DeepCopyInto(*out) } return diff --git a/pkg/apis/stackit/v1alpha1/zz_generated.defaults.go b/pkg/apis/stackit/v1alpha1/zz_generated.defaults.go index c22bee43..fe5cc591 100644 --- a/pkg/apis/stackit/v1alpha1/zz_generated.defaults.go +++ b/pkg/apis/stackit/v1alpha1/zz_generated.defaults.go @@ -16,9 +16,16 @@ import ( // All generated defaulters are covering - they call all nested defaulters. func RegisterDefaults(scheme *runtime.Scheme) error { scheme.AddTypeDefaultingFunc(&ControlPlaneConfig{}, func(obj interface{}) { SetObjectDefaults_ControlPlaneConfig(obj.(*ControlPlaneConfig)) }) + scheme.AddTypeDefaultingFunc(&SelfHostedShootExposureConfig{}, func(obj interface{}) { + SetObjectDefaults_SelfHostedShootExposureConfig(obj.(*SelfHostedShootExposureConfig)) + }) return nil } func SetObjectDefaults_ControlPlaneConfig(in *ControlPlaneConfig) { SetDefaults_ControlPlaneConfig(in) } + +func SetObjectDefaults_SelfHostedShootExposureConfig(in *SelfHostedShootExposureConfig) { + SetDefaults_SelfHostedShootExposureConfig(in) +} diff --git a/pkg/apis/stackit/validation/selfhostedshootexposure.go b/pkg/apis/stackit/validation/selfhostedshootexposure.go index 76c1ccc4..e9644a7b 100644 --- a/pkg/apis/stackit/validation/selfhostedshootexposure.go +++ b/pkg/apis/stackit/validation/selfhostedshootexposure.go @@ -19,7 +19,6 @@ func ValidateSelfHostedShootExposureConfig(config *stackitv1alpha1.SelfHostedSho if config.LoadBalancer.AccessControl != nil { allErrs = append(allErrs, validateAllowedSourceRanges(config.LoadBalancer.AccessControl.AllowedSourceRanges, lbPath.Child("accessControl", "allowedSourceRanges"))...) } - return allErrs } diff --git a/pkg/apis/stackit/validation/selfhostedshootexposure_test.go b/pkg/apis/stackit/validation/selfhostedshootexposure_test.go index a758aa2e..a093478c 100644 --- a/pkg/apis/stackit/validation/selfhostedshootexposure_test.go +++ b/pkg/apis/stackit/validation/selfhostedshootexposure_test.go @@ -18,8 +18,8 @@ var _ = Describe("SelfHostedShootExposureConfig validation", func() { BeforeEach(func() { config = &stackitv1alpha1.SelfHostedShootExposureConfig{ - LoadBalancer: &stackitv1alpha1.LoadBalancerConfig{ - AccessControl: &stackitv1alpha1.AccessControlConfig{}, + LoadBalancer: &stackitv1alpha1.LoadBalancer{ + AccessControl: &stackitv1alpha1.AccessControl{}, }, } }) diff --git a/pkg/controller/selfhostedshootexposure/actuator.go b/pkg/controller/selfhostedshootexposure/actuator.go index 836291de..ecc3d8bd 100644 --- a/pkg/controller/selfhostedshootexposure/actuator.go +++ b/pkg/controller/selfhostedshootexposure/actuator.go @@ -6,7 +6,6 @@ import ( extensionscontroller "github.com/gardener/gardener/extensions/pkg/controller" "github.com/gardener/gardener/extensions/pkg/util" - v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants" extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" @@ -77,31 +76,23 @@ func (a *Actuator) delete(ctx context.Context, log logr.Logger, exposure *extens return nil } -func (a *Actuator) ForceDelete(ctx context.Context, log logr.Logger, exposure *extensionsv1alpha1.SelfHostedShootExposure, cluster *extensionscontroller.Cluster) error { - return a.Delete(ctx, log, exposure, cluster) +func (a *Actuator) ForceDelete(_ context.Context, _ logr.Logger, _ *extensionsv1alpha1.SelfHostedShootExposure, _ *extensionscontroller.Cluster) error { + return nil } // getResources initializes Resources and Options for the given SelfHostedShootExposure, needed for reconciliation/deletion. func (a *Actuator) getResources(ctx context.Context, log logr.Logger, exposure *extensionsv1alpha1.SelfHostedShootExposure, cluster *extensionscontroller.Cluster) (*Resources, error) { region := stackit.DetermineRegion(cluster) - // Determine which secret to use for credentials. - // Support explicit CredentialsRef (required by GEP-0036), with fallback to default cloud-provider secret. + // Support explicit CredentialsRef (required by GEP-0036). // TODO(jamand): Support WorkloadIdentity once integrated into SKE. // For now, we only support secret-based credentials via ObjectReference pointing to a Secret. - var secretRef corev1.SecretReference - if exposure.Spec.CredentialsRef != nil { - // Use the explicitly provided ObjectReference (must point to a Secret for now) - secretRef = corev1.SecretReference{ - Name: exposure.Spec.CredentialsRef.Name, - Namespace: exposure.Spec.CredentialsRef.Namespace, - } - } else { - // Fall back to the default cloud-provider secret - secretRef = corev1.SecretReference{ - Name: v1beta1constants.SecretNameCloudProvider, - Namespace: exposure.Namespace, - } + if exposure.Spec.CredentialsRef == nil { + return nil, fmt.Errorf("spec.credentialsRef is required") + } + secretRef := corev1.SecretReference{ + Name: exposure.Spec.CredentialsRef.Name, + Namespace: exposure.Spec.CredentialsRef.Namespace, } lbClient, err := stackitclient.New(region, cluster).LoadBalancing(ctx, a.Client, secretRef) diff --git a/pkg/controller/selfhostedshootexposure/actuator_test.go b/pkg/controller/selfhostedshootexposure/actuator_test.go index 90be98da..bbf9cf08 100644 --- a/pkg/controller/selfhostedshootexposure/actuator_test.go +++ b/pkg/controller/selfhostedshootexposure/actuator_test.go @@ -9,6 +9,7 @@ import ( "github.com/go-logr/logr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -42,6 +43,10 @@ var _ = Describe("Actuator", func() { Type: "stackit", }, Port: 443, + CredentialsRef: &corev1.ObjectReference{ + Name: "cloudprovider", + Namespace: "kube-system", + }, }, } cluster := &extensionscontroller.Cluster{ @@ -55,35 +60,38 @@ var _ = Describe("Actuator", func() { Expect(err).To(HaveOccurred()) Expect(ingress).To(BeNil()) }) - }) - Describe("#Delete", func() { - It("should return error when getResources fails (no client configured)", func() { + It("should return a clean error when CredentialsRef is missing", func() { exposure := &extensionsv1alpha1.SelfHostedShootExposure{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "kube-system", + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "kube-system"}, + Spec: extensionsv1alpha1.SelfHostedShootExposureSpec{ + DefaultSpec: extensionsv1alpha1.DefaultSpec{Type: "stackit"}, + Port: 443, }, } cluster := &extensionscontroller.Cluster{ - Shoot: &gardencorev1beta1.Shoot{ - Spec: gardencorev1beta1.ShootSpec{Region: "eu01"}, - }, + Shoot: &gardencorev1beta1.Shoot{Spec: gardencorev1beta1.ShootSpec{Region: "eu01"}}, } - err := actuator.Delete(ctx, logger, exposure, cluster) + _, err := actuator.Reconcile(ctx, logger, exposure, cluster) - Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(ContainSubstring("credentialsRef is required"))) }) }) - Describe("#ForceDelete", func() { - It("should delegate to Delete", func() { + Describe("#Delete", func() { + It("should return error when getResources fails (no client configured)", func() { exposure := &extensionsv1alpha1.SelfHostedShootExposure{ ObjectMeta: metav1.ObjectMeta{ Name: "test", Namespace: "kube-system", }, + Spec: extensionsv1alpha1.SelfHostedShootExposureSpec{ + CredentialsRef: &corev1.ObjectReference{ + Name: "cloudprovider", + Namespace: "kube-system", + }, + }, } cluster := &extensionscontroller.Cluster{ Shoot: &gardencorev1beta1.Shoot{ @@ -91,10 +99,17 @@ var _ = Describe("Actuator", func() { }, } - err := actuator.ForceDelete(ctx, logger, exposure, cluster) + err := actuator.Delete(ctx, logger, exposure, cluster) - // Should fail with same error as Delete (no client configured) Expect(err).To(HaveOccurred()) }) }) + + Describe("#ForceDelete", func() { + It("should be a no-op (orphan resources)", func() { + err := actuator.ForceDelete(ctx, logger, &extensionsv1alpha1.SelfHostedShootExposure{}, &extensionscontroller.Cluster{}) + + Expect(err).NotTo(HaveOccurred()) + }) + }) }) diff --git a/pkg/controller/selfhostedshootexposure/options.go b/pkg/controller/selfhostedshootexposure/options.go index 81be1ec8..3a5b7c0f 100644 --- a/pkg/controller/selfhostedshootexposure/options.go +++ b/pkg/controller/selfhostedshootexposure/options.go @@ -3,12 +3,9 @@ package selfhostedshootexposure import ( "context" "fmt" - "time" extensionscontroller "github.com/gardener/gardener/extensions/pkg/controller" extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" - reconcilerutils "github.com/gardener/gardener/pkg/controllerutils/reconciler" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/client" @@ -39,8 +36,8 @@ type Options struct { Region string // NetworkID is the ID of the network where the control plane nodes reside. NetworkID string - // PlanId specifies the service plan (size) of the load balancer. - PlanId string + // PlanID specifies the service plan (size) of the load balancer. + PlanID string // AllowedSourceRanges restricts which source CIDRs may reach the load balancer. // Empty means unrestricted. AllowedSourceRanges []string @@ -52,7 +49,11 @@ func (a *Actuator) DetermineOptions(ctx context.Context, exposure *extensionsv1a ProjectID: projectID, ResourceName: fmt.Sprintf("%s-exposure-%s", cluster.Shoot.Status.TechnicalID, exposure.Name), // STACKIT LB labels do not allow '/' in keys, so we use the flat dot-separated form - // matching the convention used for other STACKIT LBs (see controlplane.STACKITLBClusterLabelKey). + // matching the convention used for other STACKIT LBs (CCM extraLabels in + // controlplane.valuesprovider, infrastructure cleanup in infraflow/delete.go). + // TODO: migrate to utils.BuildLabelKey + CustomLabelDomain once the LB API accepts '/' + // in label keys; this needs to be coordinated across CCM, controlplane and infraflow so + // the infrastructure cleanup keeps finding all LBs by the same key. Labels: map[string]string{ controlplane.STACKITLBClusterLabelKey: cluster.Shoot.Status.TechnicalID, ExposureLabelKey: exposure.Name, @@ -67,27 +68,22 @@ func (a *Actuator) DetermineOptions(ctx context.Context, exposure *extensionsv1a } opts.NetworkID = infraStatus.Networks.ID - // Decode providerConfig to extract STACKIT-specific settings + // Decode providerConfig (when present) and apply API defaults. + providerConfig := &stackitv1alpha1.SelfHostedShootExposureConfig{} if exposure.Spec.ProviderConfig != nil { - providerConfig := &stackitv1alpha1.SelfHostedShootExposureConfig{} if _, _, err := a.Decoder.Decode(exposure.Spec.ProviderConfig.Raw, nil, providerConfig); err != nil { return nil, fmt.Errorf("error decoding providerConfig: %w", err) } - if errs := validation.ValidateSelfHostedShootExposureConfig(providerConfig, field.NewPath("providerConfig")); len(errs) > 0 { - return nil, fmt.Errorf("invalid providerConfig: %w", errs.ToAggregate()) - } - if providerConfig.LoadBalancer != nil { - if providerConfig.LoadBalancer.PlanId != nil { - opts.PlanId = *providerConfig.LoadBalancer.PlanId - } - if providerConfig.LoadBalancer.AccessControl != nil { - opts.AllowedSourceRanges = providerConfig.LoadBalancer.AccessControl.AllowedSourceRanges - } - } } - // Default plan if not specified - if opts.PlanId == "" { - opts.PlanId = "p10" + a.Client.Scheme().Default(providerConfig) + + if errs := validation.ValidateSelfHostedShootExposureConfig(providerConfig, field.NewPath("spec.providerConfig")); len(errs) > 0 { + return nil, fmt.Errorf("invalid providerConfig: %w", errs.ToAggregate()) + } + + opts.PlanID = *providerConfig.LoadBalancer.PlanID + if providerConfig.LoadBalancer.AccessControl != nil { + opts.AllowedSourceRanges = providerConfig.LoadBalancer.AccessControl.AllowedSourceRanges } return opts, nil @@ -96,21 +92,7 @@ func (a *Actuator) DetermineOptions(ctx context.Context, exposure *extensionsv1a func getInfrastructureStatus(ctx context.Context, c client.Client, cluster *extensionscontroller.Cluster) (*stackitv1alpha1.InfrastructureStatus, error) { infra := &extensionsv1alpha1.Infrastructure{} if err := c.Get(ctx, client.ObjectKey{Namespace: cluster.ObjectMeta.Name, Name: cluster.Shoot.Name}, infra); err != nil { - if apierrors.IsNotFound(err) { - // Infrastructure is reconciled before SelfHostedShootExposure; absence is a normal transient state on initial creation. - return nil, &reconcilerutils.RequeueAfterError{ - RequeueAfter: 30 * time.Second, - Cause: fmt.Errorf("waiting for Infrastructure resource to be created"), - } - } return nil, fmt.Errorf("error getting infrastructure: %w", err) } - if infra.Status.ProviderStatus == nil { - // Infrastructure exists but hasn't been reconciled yet — ProviderStatus (and thus the network ID) is not yet populated. - return nil, &reconcilerutils.RequeueAfterError{ - RequeueAfter: 30 * time.Second, - Cause: fmt.Errorf("waiting for Infrastructure status to be populated"), - } - } return helper.InfrastructureStatusFromRaw(infra.Status.ProviderStatus) } diff --git a/pkg/controller/selfhostedshootexposure/options_test.go b/pkg/controller/selfhostedshootexposure/options_test.go index fdef94b9..860b29c1 100644 --- a/pkg/controller/selfhostedshootexposure/options_test.go +++ b/pkg/controller/selfhostedshootexposure/options_test.go @@ -123,15 +123,15 @@ var _ = Describe("Options", func() { }, Region: "eu01", NetworkID: "network-id", - PlanId: "p10", + PlanID: "p10", })) }) - It("should use PlanId from providerConfig", func() { + It("should use PlanID from providerConfig", func() { encoder := serializer.NewCodecFactory(fakeClient.Scheme()).EncoderForVersion(&json.Serializer{}, stackitv1alpha1.SchemeGroupVersion) providerConfig := &stackitv1alpha1.SelfHostedShootExposureConfig{ - LoadBalancer: &stackitv1alpha1.LoadBalancerConfig{ - PlanId: new("p250"), + LoadBalancer: &stackitv1alpha1.LoadBalancer{ + PlanID: new("p250"), }, } providerConfigBytes, err := runtime.Encode(encoder, providerConfig) @@ -141,14 +141,14 @@ var _ = Describe("Options", func() { opts, err := a.DetermineOptions(ctx, exposure, cluster, projectID) Expect(err).NotTo(HaveOccurred()) - Expect(opts.PlanId).To(Equal("p250")) + Expect(opts.PlanID).To(Equal("p250")) }) It("should reject an invalid AllowedSourceRanges CIDR", func() { encoder := serializer.NewCodecFactory(fakeClient.Scheme()).EncoderForVersion(&json.Serializer{}, stackitv1alpha1.SchemeGroupVersion) providerConfig := &stackitv1alpha1.SelfHostedShootExposureConfig{ - LoadBalancer: &stackitv1alpha1.LoadBalancerConfig{ - AccessControl: &stackitv1alpha1.AccessControlConfig{ + LoadBalancer: &stackitv1alpha1.LoadBalancer{ + AccessControl: &stackitv1alpha1.AccessControl{ AllowedSourceRanges: []string{"not-a-cidr"}, }, }, diff --git a/pkg/controller/selfhostedshootexposure/resources.go b/pkg/controller/selfhostedshootexposure/resources.go index 07ac64b6..1ca2d5d3 100644 --- a/pkg/controller/selfhostedshootexposure/resources.go +++ b/pkg/controller/selfhostedshootexposure/resources.go @@ -22,12 +22,12 @@ type Resources struct { func (r *Resources) getExistingResources(ctx context.Context, log logr.Logger) error { lb, err := r.LBClient.GetLoadBalancer(ctx, r.ResourceName) if err != nil { + if stackitclient.IsNotFound(err) { + return nil + } return fmt.Errorf("error getting load balancer: %w", err) } - if lb != nil { - r.LoadBalancer = lb - log.V(1).Info("Found existing load balancer", "loadbalancer", r.LoadBalancer.GetName()) - } - + r.LoadBalancer = lb + log.V(1).Info("Found existing load balancer", "loadBalancer", r.LoadBalancer.GetName()) return nil } diff --git a/pkg/controller/selfhostedshootexposure/resources_loadbalancer.go b/pkg/controller/selfhostedshootexposure/resources_loadbalancer.go index 6c4ab2e4..66f22d01 100644 --- a/pkg/controller/selfhostedshootexposure/resources_loadbalancer.go +++ b/pkg/controller/selfhostedshootexposure/resources_loadbalancer.go @@ -1,95 +1,72 @@ package selfhostedshootexposure import ( + "cmp" "context" "fmt" "slices" - "sort" "strings" "time" extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" reconcilerutils "github.com/gardener/gardener/pkg/controllerutils/reconciler" "github.com/go-logr/logr" + gocmp "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + loadbalancersdk "github.com/stackitcloud/stackit-sdk-go/services/loadbalancer" //nolint:staticcheck // SA1019: see TODO below — v2api lacks the typed enum constants we need. loadbalancer "github.com/stackitcloud/stackit-sdk-go/services/loadbalancer/v2api" loadbalancerwait "github.com/stackitcloud/stackit-sdk-go/services/loadbalancer/v2api/wait" corev1 "k8s.io/api/core/v1" - - stackitclient "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit/client" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/utils/ptr" ) +// TODO(jamand): drop the loadbalancersdk import once v2api re-exports the typed enum constants +// (NetworkRole, ListenerProtocol, LoadBalancerErrorTypes). v2api currently weakened these to +// *string (known openapi-generator limitation confirmed with the LB team); the authoritative +// values still live in the deprecated top-level stackit-sdk-go/services/loadbalancer package, +// scheduled for removal after 2026-09-30. We reference them here as the source of truth and +// convert to string at the call sites. const ( - // lbNetworkRoleListenersAndTargets is the network role for listeners and targets in the load balancer network. - lbNetworkRoleListenersAndTargets = "ROLE_LISTENERS_AND_TARGETS" - // protocolTCP is the TCP protocol identifier for the listener. - protocolTCP = "PROTOCOL_TCP" // listenerName is the (single) hardcoded listener name for exposing the control plane API server. - listenerName = "listener-control-plane" + listenerName = "control-plane" // targetPoolName is the (single) hardcoded target pool name for control plane nodes. - targetPoolName = "target-pool-control-plane" -) - -// STACKIT LB error types reported in LoadBalancer.Errors[].Type. v2api weakened Type from a -// generated enum to *string (known openapi-generator limitation confirmed with the LB team); -// the authoritative set of values still lives as LOADBALANCERERRORTYPE_* constants in the -// deprecated top-level stackit-sdk-go/services/loadbalancer package. -const ( - // lbErrTypeTargetNotActive encodes that target may not be ready (yet). - lbErrTypeTargetNotActive = "TYPE_TARGET_NOT_ACTIVE" + targetPoolName = "control-plane" ) func (r *Resources) reconcileLoadBalancer(ctx context.Context, log logr.Logger) error { targets, err := r.buildTargets() if err != nil { - return err + return fmt.Errorf("error building targets: %w", err) } if r.LoadBalancer == nil { return r.createLoadBalancer(ctx, log, targets) } - targetPoolNeedsUpdate, err := r.targetPoolNeedsUpdate(targets) - if err != nil { - return err - } - fullStateNeedsUpdate := r.planNeedsUpdate() || r.accessControlNeedsUpdate() - - if !targetPoolNeedsUpdate && !fullStateNeedsUpdate { + if !r.loadBalancerNeedsUpdate(targets) { return nil } - - // Fast path: only targets changed (e.g. control-plane node added/removed). The sub-resource - // endpoint is scoped to the target pool, so we avoid re-sending the full LB state. - if targetPoolNeedsUpdate && !fullStateNeedsUpdate { - return r.updateTargetPool(ctx, log, targets) - } - - // Full-state update: STACKIT's UpdateLoadBalancer (PUT endpoint) + // STACKIT's UpdateLoadBalancer is a single PUT covering all managed fields (targets, plan, + // ACL). UpdateLoadBalancerTargetPool would also work for target-only changes, but it returns + // only the TargetPool — we'd still need a follow-up GET to refresh r.LoadBalancer for the + // readiness check, so it costs an extra round-trip without a server-side latency win + // (STACKIT transitions the LB to PENDING on either write). return r.updateLoadBalancer(ctx, log, targets) } func (r *Resources) createLoadBalancer(ctx context.Context, log logr.Logger, targets []loadbalancer.Target) error { - if len(targets) == 0 { - // Endpoints are populated asynchronously by gardenlet from healthy control-plane nodes. - // Empty endpoints on first create is a normal transient state. - return &reconcilerutils.RequeueAfterError{ - RequeueAfter: 30 * time.Second, - Cause: fmt.Errorf("waiting for endpoints to be populated in spec"), - } - } - - log.V(1).Info("Creating load balancer", "loadBalancer", r.ResourceName, "networkID", r.NetworkID, "planID", r.PlanId) createdLB, err := r.LBClient.CreateLoadBalancer(ctx, loadbalancer.CreateLoadBalancerPayload{ Name: &r.ResourceName, Labels: &r.Labels, Networks: r.desiredNetworks(), Listeners: r.desiredListeners(), TargetPools: r.desiredTargetPools(targets), - PlanId: &r.PlanId, + PlanId: &r.PlanID, Options: r.desiredOptions(), }) if err != nil { - return wrapLBAPIError("creating load balancer", err) + return fmt.Errorf("error creating load balancer: %w", err) } r.LoadBalancer = createdLB @@ -97,37 +74,16 @@ func (r *Resources) createLoadBalancer(ctx context.Context, log logr.Logger, tar return nil } -func (r *Resources) updateTargetPool(ctx context.Context, log logr.Logger, targets []loadbalancer.Target) error { - log.Info("Target pool needs updating", "loadBalancer", r.ResourceName) - _, err := r.LBClient.UpdateLoadBalancerTargetPool(ctx, - r.ResourceName, - targetPoolName, - loadbalancer.UpdateTargetPoolPayload{ - Name: new(targetPoolName), - TargetPort: &r.SelfHostedShootExposure.Spec.Port, - Targets: targets, - }) - if err != nil { - return wrapLBAPIError("updating load balancer target pool", err) - } - log.Info("Updated load balancer target pool", "loadBalancer", r.ResourceName) - - // Re-read the LB so downstream readiness checks don't see the pre-write status (STACKIT - // transitions the LB to STATUS_PENDING on any write). UpdateLoadBalancerTargetPool only - // returns the TargetPool, so a full GET is needed. - refreshed, err := r.LBClient.GetLoadBalancer(ctx, r.ResourceName) - if err != nil { - return fmt.Errorf("error refreshing load balancer after target pool update: %w", err) - } - r.LoadBalancer = refreshed - return nil -} - func (r *Resources) updateLoadBalancer(ctx context.Context, log logr.Logger, targets []loadbalancer.Target) error { - log.Info("Load balancer needs updating", "loadBalancer", r.ResourceName, "newPlan", r.PlanId) + // STACKIT requires ExternalAddress to be set on PUT (and rejects EphemeralAddress=true once + // the LB has a floating IP). If the LB hasn't been assigned an external address yet, return + // an error so controller-runtime retries with backoff rather than 400ing the API. + if r.LoadBalancer.ExternalAddress == nil { + return fmt.Errorf("waiting for load balancer external address before updating") + } - // LB Endpoint only available as PUT, requires sending whole resource. - payload := loadbalancer.UpdateLoadBalancerPayload{ + // LB endpoint is PUT-only and requires sending the whole resource. + updated, err := r.LBClient.UpdateLoadBalancer(ctx, r.ResourceName, loadbalancer.UpdateLoadBalancerPayload{ Name: &r.ResourceName, Version: r.LoadBalancer.Version, ExternalAddress: r.LoadBalancer.ExternalAddress, @@ -135,15 +91,12 @@ func (r *Resources) updateLoadBalancer(ctx context.Context, log logr.Logger, tar Networks: r.desiredNetworks(), Listeners: r.desiredListeners(), TargetPools: r.desiredTargetPools(targets), - PlanId: &r.PlanId, + PlanId: &r.PlanID, Options: r.desiredOptions(), - } - - updated, err := r.LBClient.UpdateLoadBalancer(ctx, r.ResourceName, payload) + }) if err != nil { - return wrapLBAPIError("updating load balancer", err) + return fmt.Errorf("error updating load balancer: %w", err) } - // Capture the post-write LB so readiness is evaluated against actual current status r.LoadBalancer = updated log.Info("Updated load balancer", "loadBalancer", r.ResourceName) return nil @@ -154,7 +107,7 @@ func (r *Resources) updateLoadBalancer(ctx context.Context, log logr.Logger, tar func (r *Resources) desiredNetworks() []loadbalancer.Network { return []loadbalancer.Network{{ NetworkId: &r.NetworkID, - Role: new(lbNetworkRoleListenersAndTargets), + Role: new(string(loadbalancersdk.NETWORKROLE_LISTENERS_AND_TARGETS)), //nolint:staticcheck // SA1019: see TODO at the top of the file. }} } @@ -162,8 +115,8 @@ func (r *Resources) desiredNetworks() []loadbalancer.Network { func (r *Resources) desiredListeners() []loadbalancer.Listener { return []loadbalancer.Listener{{ DisplayName: new(listenerName), - Port: &r.SelfHostedShootExposure.Spec.Port, - Protocol: new(protocolTCP), + Port: new(r.SelfHostedShootExposure.Spec.Port), + Protocol: new(string(loadbalancersdk.LISTENERPROTOCOL_TCP)), //nolint:staticcheck // SA1019: see TODO at the top of the file. TargetPool: new(targetPoolName), }} } @@ -172,15 +125,21 @@ func (r *Resources) desiredListeners() []loadbalancer.Listener { func (r *Resources) desiredTargetPools(targets []loadbalancer.Target) []loadbalancer.TargetPool { return []loadbalancer.TargetPool{{ Name: new(targetPoolName), - TargetPort: &r.SelfHostedShootExposure.Spec.Port, + TargetPort: new(r.SelfHostedShootExposure.Spec.Port), Targets: targets, }} } // desiredOptions returns the LB options. // -// Initial call: ephemeral (provides external IP) -// Subsequent calls (PUT updates): provide the initially provided IP, do no set ephemeral to true (error!). +// Initial create: ask STACKIT to assign an ephemeral external IP. +// +// Subsequent PUT updates: do NOT set EphemeralAddress — STACKIT rejects PUTs with +// EphemeralAddress=true once the LB has been assigned a floating IP ("Ephemeral address is +// not supported for existing floating IPs"). The constraint that one of ExternalAddress, +// EphemeralAddress=true, or PrivateNetworkOnly=true must be set is satisfied via +// UpdateLoadBalancerPayload.ExternalAddress, which the caller passes through from the +// existing LB. updateLoadBalancer guards against the LB not yet having an ExternalAddress. func (r *Resources) desiredOptions() *loadbalancer.LoadBalancerOptions { opts := &loadbalancer.LoadBalancerOptions{} if r.LoadBalancer == nil { @@ -194,51 +153,27 @@ func (r *Resources) desiredOptions() *loadbalancer.LoadBalancerOptions { return opts } +// loadBalancerNeedsUpdate reports whether any controller-managed field of the LB (targets, +// plan, ACL) differs from the desired state. Caller must guarantee r.LoadBalancer is non-nil. +func (r *Resources) loadBalancerNeedsUpdate(specTargets []loadbalancer.Target) bool { + return r.targetsNeedUpdate(specTargets) || r.planNeedsUpdate() || r.accessControlNeedsUpdate() +} + +// planNeedsUpdate checks for an existing LB if its current plan needs to be updated. func (r *Resources) planNeedsUpdate() bool { - currentPlan := "" - if r.LoadBalancer != nil && r.LoadBalancer.PlanId != nil { - currentPlan = *r.LoadBalancer.PlanId - } - return currentPlan != r.PlanId + return ptr.Deref(r.LoadBalancer.PlanId, "") != r.PlanID } // accessControlNeedsUpdate reports whether the LB's currently configured source-IP allowlist -// differs from the desired set. The desired set is order-independent; we compare as sorted lists. -// An empty desired list means "no restriction" — detected by diff against whatever the LB reports. +// differs from the desired set. Comparison is order-independent and ignores duplicates: the LB +// API may or may not de-duplicate, so set semantics avoid spurious update churn. An empty +// desired list means "no restriction" — detected by diff against whatever the LB reports. func (r *Resources) accessControlNeedsUpdate() bool { var current []string - if r.LoadBalancer != nil && - r.LoadBalancer.Options != nil && - r.LoadBalancer.Options.AccessControl != nil { + if r.LoadBalancer.Options != nil && r.LoadBalancer.Options.AccessControl != nil { current = r.LoadBalancer.Options.AccessControl.AllowedSourceRanges } - return !stringSetsEqual(current, r.AllowedSourceRanges) -} - -// stringSetsEqual compares two string slices as unordered sets, ignoring duplicates. -// The LB API may or may not de-duplicate (causing reconciliation loop), remove duplicates -// for a clean approach. -func stringSetsEqual(a, b []string) bool { - return slices.Equal(sortedUnique(a), sortedUnique(b)) -} - -// sortedUnique returns the input as a sorted slice with consecutive duplicates removed, -// i.e. the canonical representation of the set. -func sortedUnique(s []string) []string { - return slices.Compact(slices.Sorted(slices.Values(s))) -} - -// wrapLBAPIError classifies STACKIT LB API errors: 409 Conflicts are transient (another caller -// modified the LB between our GET and our write) and are retried via RequeueAfterError; anything -// else is returned as a regular error so Gardener can classify + surface it. -func wrapLBAPIError(op string, err error) error { - if stackitclient.IsConflict(err) { - return &reconcilerutils.RequeueAfterError{ - RequeueAfter: 15 * time.Second, - Cause: fmt.Errorf("load balancer is being modified while %s, retrying: %w", op, err), - } - } - return fmt.Errorf("error %s: %w", op, err) + return !sets.New(current...).Equal(sets.New(r.AllowedSourceRanges...)) } // checkLoadBalancerReady returns nil only if the LB is fully provisioned (STATUS_READY with an @@ -296,10 +231,7 @@ func lbErrorsAllTransient(errs []loadbalancer.LoadBalancerError) bool { if e.Type == nil { return false } - switch *e.Type { - case lbErrTypeTargetNotActive: - // transient - default: + if *e.Type != string(loadbalancersdk.LOADBALANCERERRORTYPE_TARGET_NOT_ACTIVE) { //nolint:staticcheck // SA1019: see TODO at the top of the file. return false } } @@ -352,16 +284,18 @@ func (r *Resources) buildTargets() ([]loadbalancer.Target, error) { Ip: &ip, } } + sortTargets(targets) + return targets, nil +} - // Sort targets by IP (primary) and DisplayName (secondary) for deterministic ordering - sort.Slice(targets, func(i, j int) bool { - if *targets[i].Ip != *targets[j].Ip { - return *targets[i].Ip < *targets[j].Ip - } - return *targets[i].DisplayName < *targets[j].DisplayName +// sortTargets sorts the given target slice in-place by IP (primary) and DisplayName (secondary). +func sortTargets(targets []loadbalancer.Target) { + slices.SortFunc(targets, func(a, b loadbalancer.Target) int { + return cmp.Or( + cmp.Compare(*a.Ip, *b.Ip), + cmp.Compare(*a.DisplayName, *b.DisplayName), + ) }) - - return targets, nil } // extractInternalIP finds and returns the internal IP address from an endpoint's addresses. @@ -375,66 +309,22 @@ func extractInternalIP(endpoint *extensionsv1alpha1.ControlPlaneEndpoint) (strin return "", fmt.Errorf("endpoint %s has no InternalIP address", endpoint.NodeName) } -// targetsEqual compares two target lists for equality. -// Both lists should be sorted by IP for correct comparison. -func targetsEqual(spec, lb []loadbalancer.Target) bool { - if len(spec) != len(lb) { - return false - } - - for i := range spec { - if spec[i].Ip == nil || lb[i].Ip == nil { - return false - } - if *spec[i].Ip != *lb[i].Ip { - return false - } - // Also verify the display name matches - if spec[i].DisplayName == nil || lb[i].DisplayName == nil { - return false - } - if *spec[i].DisplayName != *lb[i].DisplayName { - return false - } - } - return true -} - -// targetPoolNeedsUpdate checks if the target pool in the load balancer needs updating. -// specTargets should be pre-built to avoid double-building. -func (r *Resources) targetPoolNeedsUpdate(specTargets []loadbalancer.Target) (bool, error) { - // If no load balancer exists yet, no update needed (will be created fresh) - if r.LoadBalancer == nil { - return false, nil - } - - // If LB exists but has no target pools, check if spec has targets - if len(r.LoadBalancer.TargetPools) == 0 { - return len(specTargets) > 0, nil - } - - // Validate that the load balancer has the expected target pool - if r.LoadBalancer.TargetPools[0].Name == nil || *r.LoadBalancer.TargetPools[0].Name != targetPoolName { - actualName := "" - if r.LoadBalancer.TargetPools[0].Name != nil { - actualName = *r.LoadBalancer.TargetPools[0].Name +// targetsNeedUpdate compares the LB's current first target pool against the desired targets. +// If no target pool exists or its name doesn't match, signals an update (the full PUT will +// replace whatever is there with the desired single pool); empty current vs. empty desired is +// a no-op so we don't churn updates. +// +// Target.AdditionalProperties is excluded from the comparison: the SDK populates it from any +// JSON fields STACKIT echoes back that the SDK doesn't have a typed field for. We never set +// these on the desired side, so leaving them in would make every reconcile see a diff and PUT. +func (r *Resources) targetsNeedUpdate(specTargets []loadbalancer.Target) bool { + var current []loadbalancer.Target + if len(r.LoadBalancer.TargetPools) > 0 { + if ptr.Deref(r.LoadBalancer.TargetPools[0].Name, "") != targetPoolName { + return true } - return false, fmt.Errorf("unexpected target pool name: expected %q, got %q", - targetPoolName, actualName) + current = slices.Clone(r.LoadBalancer.TargetPools[0].Targets) + sortTargets(current) } - - // Get targets from the first target pool and copy before sorting - lbTargets := make([]loadbalancer.Target, len(r.LoadBalancer.TargetPools[0].Targets)) - copy(lbTargets, r.LoadBalancer.TargetPools[0].Targets) - - // Sort the LB targets for comparison (same order as spec targets) - sort.Slice(lbTargets, func(i, j int) bool { - if *lbTargets[i].Ip != *lbTargets[j].Ip { - return *lbTargets[i].Ip < *lbTargets[j].Ip - } - return *lbTargets[i].DisplayName < *lbTargets[j].DisplayName - }) - - // Compare semantically (order-independent after sorting) - return !targetsEqual(specTargets, lbTargets), nil + return !gocmp.Equal(specTargets, current, cmpopts.IgnoreFields(loadbalancer.Target{}, "AdditionalProperties")) } diff --git a/pkg/controller/selfhostedshootexposure/resources_loadbalancer_test.go b/pkg/controller/selfhostedshootexposure/resources_loadbalancer_test.go index 9287e87c..3b91ba38 100644 --- a/pkg/controller/selfhostedshootexposure/resources_loadbalancer_test.go +++ b/pkg/controller/selfhostedshootexposure/resources_loadbalancer_test.go @@ -2,11 +2,9 @@ package selfhostedshootexposure import ( "context" - "errors" "fmt" extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" - reconcilerutils "github.com/gardener/gardener/pkg/controllerutils/reconciler" "github.com/go-logr/logr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -36,7 +34,7 @@ var _ = Describe("reconcileLoadBalancer", func() { ResourceName: "test-lb", Labels: map[string]string{"cluster": "shoot--foo--bar"}, NetworkID: "network-123", - PlanId: "p10", + PlanID: "p10", SelfHostedShootExposure: &extensionsv1alpha1.SelfHostedShootExposure{ Spec: extensionsv1alpha1.SelfHostedShootExposureSpec{ Port: 443, @@ -53,19 +51,6 @@ var _ = Describe("reconcileLoadBalancer", func() { }) Context("when no load balancer exists", func() { - It("should requeue creation without endpoints", func() { - r.SelfHostedShootExposure.Spec.Endpoints = []extensionsv1alpha1.ControlPlaneEndpoint{} - - err := r.reconcileLoadBalancer(ctx, logger) - - Expect(err).To(HaveOccurred()) - // Endpoints are populated asynchronously by gardenlet; empty endpoints should trigger a clean requeue, - // not a fatal error. - var rae *reconcilerutils.RequeueAfterError - Expect(errors.As(err, &rae)).To(BeTrue()) - Expect(rae.Cause.Error()).To(ContainSubstring("waiting for endpoints to be populated")) - }) - It("should create a new load balancer", func() { r.SelfHostedShootExposure.Spec.Endpoints = []extensionsv1alpha1.ControlPlaneEndpoint{ { @@ -97,7 +82,7 @@ var _ = Describe("reconcileLoadBalancer", func() { Expect(payload.Listeners).To(HaveLen(1)) Expect(*payload.Listeners[0].Port).To(BeEquivalentTo(443)) Expect(payload.TargetPools).To(HaveLen(1)) - Expect(*payload.TargetPools[0].Name).To(Equal("target-pool-control-plane")) + Expect(*payload.TargetPools[0].Name).To(Equal("control-plane")) // Targets should be sorted by IP Expect(payload.TargetPools[0].Targets).To(HaveLen(2)) Expect(*payload.TargetPools[0].Targets[0].Ip).To(Equal("10.0.1.10")) @@ -167,12 +152,13 @@ var _ = Describe("reconcileLoadBalancer", func() { }, } r.LoadBalancer = &loadbalancer.LoadBalancer{ - Name: new("test-lb"), - PlanId: new("p10"), - Version: new("v1"), + Name: new("test-lb"), + PlanId: new("p10"), + Version: new("v1"), + ExternalAddress: new("203.0.113.1"), TargetPools: []loadbalancer.TargetPool{ { - Name: new("target-pool-control-plane"), + Name: new("control-plane"), Targets: []loadbalancer.Target{ {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, }, @@ -188,47 +174,6 @@ var _ = Describe("reconcileLoadBalancer", func() { Expect(err).NotTo(HaveOccurred()) }) - It("should update target pool only when targets changed", func() { - // Spec has a new node - r.SelfHostedShootExposure.Spec.Endpoints = []extensionsv1alpha1.ControlPlaneEndpoint{ - { - NodeName: "node-1", - Addresses: []corev1.NodeAddress{ - {Type: corev1.NodeInternalIP, Address: "10.0.1.10"}, - }, - }, - { - NodeName: "node-2", - Addresses: []corev1.NodeAddress{ - {Type: corev1.NodeInternalIP, Address: "10.0.1.20"}, - }, - }, - } - - mockLBClient.EXPECT(). - UpdateLoadBalancerTargetPool(ctx, "test-lb", "target-pool-control-plane", gomock.Any()). - DoAndReturn(func(_ context.Context, _, _ string, payload loadbalancer.UpdateTargetPoolPayload) (*loadbalancer.TargetPool, error) { - Expect(payload.Targets).To(HaveLen(2)) - Expect(*payload.Targets[0].Ip).To(Equal("10.0.1.10")) - Expect(*payload.Targets[1].Ip).To(Equal("10.0.1.20")) - Expect(*payload.TargetPort).To(BeEquivalentTo(443)) - return &loadbalancer.TargetPool{}, nil - }) - - // After the target pool write, reconcileLoadBalancer re-GETs the LB so downstream - // readiness checks see the post-write status (STACKIT flips the LB to PENDING). - mockLBClient.EXPECT(). - GetLoadBalancer(ctx, "test-lb"). - Return(&loadbalancer.LoadBalancer{ - Name: new("test-lb"), - Status: new("STATUS_PENDING"), - }, nil) - - err := r.reconcileLoadBalancer(ctx, logger) - - Expect(err).NotTo(HaveOccurred()) - }) - It("should do nothing when LB already has the same AllowedSourceRanges, in any order", func() { r.AllowedSourceRanges = []string{"10.0.0.0/8", "192.168.0.0/16"} // LB returns the set in the opposite order — diff must be set-based, not sequence-based. @@ -265,7 +210,7 @@ var _ = Describe("reconcileLoadBalancer", func() { }) It("should update plan via UpdateLoadBalancer when only plan changed", func() { - r.PlanId = "p100" // Changed plan + r.PlanID = "p100" // Changed plan mockLBClient.EXPECT(). UpdateLoadBalancer(ctx, "test-lb", gomock.Any()). @@ -287,7 +232,7 @@ var _ = Describe("reconcileLoadBalancer", func() { }) It("should update plan and targets in a single call when both changed", func() { - r.PlanId = "p100" + r.PlanID = "p100" r.SelfHostedShootExposure.Spec.Endpoints = []extensionsv1alpha1.ControlPlaneEndpoint{ { NodeName: "node-3", @@ -312,37 +257,27 @@ var _ = Describe("reconcileLoadBalancer", func() { Expect(err).NotTo(HaveOccurred()) }) - It("should return error when UpdateLoadBalancerTargetPool fails", func() { - r.SelfHostedShootExposure.Spec.Endpoints = []extensionsv1alpha1.ControlPlaneEndpoint{ - { - NodeName: "node-new", - Addresses: []corev1.NodeAddress{ - {Type: corev1.NodeInternalIP, Address: "10.0.1.99"}, - }, - }, - } + It("should return error when UpdateLoadBalancer fails", func() { + r.PlanID = "p100" mockLBClient.EXPECT(). - UpdateLoadBalancerTargetPool(ctx, "test-lb", "target-pool-control-plane", gomock.Any()). + UpdateLoadBalancer(ctx, "test-lb", gomock.Any()). Return(nil, fmt.Errorf("API error")) err := r.reconcileLoadBalancer(ctx, logger) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("error updating load balancer target pool")) + Expect(err.Error()).To(ContainSubstring("error updating load balancer")) }) - It("should return error when UpdateLoadBalancer fails", func() { - r.PlanId = "p100" - - mockLBClient.EXPECT(). - UpdateLoadBalancer(ctx, "test-lb", gomock.Any()). - Return(nil, fmt.Errorf("API error")) + It("should defer the update when the LB has no external address yet", func() { + r.PlanID = "p100" + r.LoadBalancer.ExternalAddress = nil + // No mock expectations — the controller must not call UpdateLoadBalancer. err := r.reconcileLoadBalancer(ctx, logger) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("error updating load balancer")) + Expect(err).To(MatchError(ContainSubstring("waiting for load balancer external address"))) }) }) }) @@ -500,197 +435,67 @@ var _ = Describe("buildTargets", func() { }) }) -var _ = Describe("targetsEqual", func() { - It("should return true for equal target lists", func() { - a := []loadbalancer.Target{ - {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, - } - b := []loadbalancer.Target{ - {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, - } - - Expect(targetsEqual(a, b)).To(BeTrue()) - }) - - It("should return false for different IPs", func() { - a := []loadbalancer.Target{ - {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, - } - b := []loadbalancer.Target{ - {Ip: new("10.0.1.99"), DisplayName: new("node-1")}, - } - - Expect(targetsEqual(a, b)).To(BeFalse()) - }) - - It("should return false for different display names", func() { - a := []loadbalancer.Target{ - {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, - } - b := []loadbalancer.Target{ - {Ip: new("10.0.1.10"), DisplayName: new("node-2")}, - } - - Expect(targetsEqual(a, b)).To(BeFalse()) - }) - - It("should return false for different lengths", func() { - a := []loadbalancer.Target{ - {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, - } - b := []loadbalancer.Target{ - {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, - {Ip: new("10.0.1.20"), DisplayName: new("node-2")}, - } - - Expect(targetsEqual(a, b)).To(BeFalse()) - }) - - It("should handle empty target lists", func() { - Expect(targetsEqual([]loadbalancer.Target{}, []loadbalancer.Target{})).To(BeTrue()) - }) - - It("should return false when IP is nil", func() { - a := []loadbalancer.Target{ - {Ip: nil, DisplayName: new("node-1")}, - } - b := []loadbalancer.Target{ - {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, - } - - Expect(targetsEqual(a, b)).To(BeFalse()) - }) - - It("should return false when DisplayName is nil", func() { - a := []loadbalancer.Target{ - {Ip: new("10.0.1.10"), DisplayName: nil}, - } - b := []loadbalancer.Target{ - {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, - } - - Expect(targetsEqual(a, b)).To(BeFalse()) - }) -}) - -var _ = Describe("targetPoolNeedsUpdate", func() { +var _ = Describe("targetsNeedUpdate", func() { var r *Resources BeforeEach(func() { - r = &Resources{} - }) - - It("should return false when no load balancer exists", func() { - r.LoadBalancer = nil - - needsUpdate, err := r.targetPoolNeedsUpdate([]loadbalancer.Target{ - {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, - }) - - Expect(err).NotTo(HaveOccurred()) - Expect(needsUpdate).To(BeFalse()) + r = &Resources{LoadBalancer: &loadbalancer.LoadBalancer{}} }) It("should return true when LB has no target pools but spec has targets", func() { - r.LoadBalancer = &loadbalancer.LoadBalancer{ - TargetPools: []loadbalancer.TargetPool{}, - } - - needsUpdate, err := r.targetPoolNeedsUpdate([]loadbalancer.Target{ + Expect(r.targetsNeedUpdate([]loadbalancer.Target{ {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, - }) - - Expect(err).NotTo(HaveOccurred()) - Expect(needsUpdate).To(BeTrue()) + })).To(BeTrue()) }) It("should return false when LB has no target pools and spec has no targets", func() { - r.LoadBalancer = &loadbalancer.LoadBalancer{ - TargetPools: []loadbalancer.TargetPool{}, - } - - needsUpdate, err := r.targetPoolNeedsUpdate([]loadbalancer.Target{}) - - Expect(err).NotTo(HaveOccurred()) - Expect(needsUpdate).To(BeFalse()) + Expect(r.targetsNeedUpdate(nil)).To(BeFalse()) }) - It("should return error when target pool has unexpected name", func() { - r.LoadBalancer = &loadbalancer.LoadBalancer{ - TargetPools: []loadbalancer.TargetPool{ - {Name: new("wrong-name")}, - }, - } - - _, err := r.targetPoolNeedsUpdate([]loadbalancer.Target{}) - - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("unexpected target pool name")) + It("should return true when target pool has unexpected name", func() { + r.LoadBalancer.TargetPools = []loadbalancer.TargetPool{{Name: new("wrong-name")}} + Expect(r.targetsNeedUpdate(nil)).To(BeTrue()) }) It("should return false when targets match", func() { - r.LoadBalancer = &loadbalancer.LoadBalancer{ - TargetPools: []loadbalancer.TargetPool{ - { - Name: new("target-pool-control-plane"), - Targets: []loadbalancer.Target{ - {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, - }, - }, + r.LoadBalancer.TargetPools = []loadbalancer.TargetPool{{ + Name: new("control-plane"), + Targets: []loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, }, - } - - needsUpdate, err := r.targetPoolNeedsUpdate([]loadbalancer.Target{ + }} + Expect(r.targetsNeedUpdate([]loadbalancer.Target{ {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, - }) - - Expect(err).NotTo(HaveOccurred()) - Expect(needsUpdate).To(BeFalse()) + })).To(BeFalse()) }) It("should return true when targets differ", func() { - r.LoadBalancer = &loadbalancer.LoadBalancer{ - TargetPools: []loadbalancer.TargetPool{ - { - Name: new("target-pool-control-plane"), - Targets: []loadbalancer.Target{ - {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, - }, - }, + r.LoadBalancer.TargetPools = []loadbalancer.TargetPool{{ + Name: new("control-plane"), + Targets: []loadbalancer.Target{ + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, }, - } - - needsUpdate, err := r.targetPoolNeedsUpdate([]loadbalancer.Target{ + }} + Expect(r.targetsNeedUpdate([]loadbalancer.Target{ {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, {Ip: new("10.0.1.20"), DisplayName: new("node-2")}, - }) - - Expect(err).NotTo(HaveOccurred()) - Expect(needsUpdate).To(BeTrue()) + })).To(BeTrue()) }) It("should compare correctly regardless of LB target order", func() { - r.LoadBalancer = &loadbalancer.LoadBalancer{ - TargetPools: []loadbalancer.TargetPool{ - { - Name: new("target-pool-control-plane"), - Targets: []loadbalancer.Target{ - // LB returns targets in reverse order - {Ip: new("10.0.1.20"), DisplayName: new("node-2")}, - {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, - }, - }, + r.LoadBalancer.TargetPools = []loadbalancer.TargetPool{{ + Name: new("control-plane"), + Targets: []loadbalancer.Target{ + // LB returns targets in reverse order + {Ip: new("10.0.1.20"), DisplayName: new("node-2")}, + {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, }, - } - + }} // Spec targets are sorted - needsUpdate, err := r.targetPoolNeedsUpdate([]loadbalancer.Target{ + Expect(r.targetsNeedUpdate([]loadbalancer.Target{ {Ip: new("10.0.1.10"), DisplayName: new("node-1")}, {Ip: new("10.0.1.20"), DisplayName: new("node-2")}, - }) - - Expect(err).NotTo(HaveOccurred()) - Expect(needsUpdate).To(BeFalse()) + })).To(BeFalse()) }) }) diff --git a/pkg/stackit/client/loadbalancing.go b/pkg/stackit/client/loadbalancing.go index 46eb7b94..681deded 100644 --- a/pkg/stackit/client/loadbalancing.go +++ b/pkg/stackit/client/loadbalancing.go @@ -13,12 +13,12 @@ import ( type LoadBalancingClient interface { ProjectID() string - ListLoadBalancers(ctx context.Context) ([]loadbalancer.LoadBalancer, error) - DeleteLoadBalancer(ctx context.Context, lbName string) error - GetLoadBalancer(ctx context.Context, id string) (*loadbalancer.LoadBalancer, error) CreateLoadBalancer(ctx context.Context, payload loadbalancer.CreateLoadBalancerPayload) (*loadbalancer.LoadBalancer, error) - UpdateLoadBalancer(ctx context.Context, lbName string, payload loadbalancer.UpdateLoadBalancerPayload) (*loadbalancer.LoadBalancer, error) - UpdateLoadBalancerTargetPool(ctx context.Context, lbName, tpName string, payload loadbalancer.UpdateTargetPoolPayload) (*loadbalancer.TargetPool, error) + GetLoadBalancer(ctx context.Context, name string) (*loadbalancer.LoadBalancer, error) + ListLoadBalancers(ctx context.Context) ([]loadbalancer.LoadBalancer, error) + UpdateLoadBalancer(ctx context.Context, name string, payload loadbalancer.UpdateLoadBalancerPayload) (*loadbalancer.LoadBalancer, error) + UpdateLoadBalancerTargetPool(ctx context.Context, name, targetPool string, payload loadbalancer.UpdateTargetPoolPayload) (*loadbalancer.TargetPool, error) + DeleteLoadBalancer(ctx context.Context, name string) error } type loadBalancingClient struct { @@ -49,38 +49,31 @@ func (l loadBalancingClient) ProjectID() string { return l.projectID } -func (l loadBalancingClient) ListLoadBalancers(ctx context.Context) ([]loadbalancer.LoadBalancer, error) { - lbResponse, err := l.Client.ListLoadBalancers(ctx, l.projectID, l.region).Execute() - if err != nil { - return nil, err - } - return lbResponse.GetLoadBalancers(), nil +func (l loadBalancingClient) CreateLoadBalancer(ctx context.Context, payload loadbalancer.CreateLoadBalancerPayload) (*loadbalancer.LoadBalancer, error) { + return l.Client.CreateLoadBalancer(ctx, l.projectID, l.region).CreateLoadBalancerPayload(payload).Execute() } -func (l loadBalancingClient) DeleteLoadBalancer(ctx context.Context, lbName string) error { - _, err := l.Client.DeleteLoadBalancer(ctx, l.projectID, l.region, lbName).Execute() - return err +func (l loadBalancingClient) GetLoadBalancer(ctx context.Context, name string) (*loadbalancer.LoadBalancer, error) { + return l.Client.GetLoadBalancer(ctx, l.projectID, l.region, name).Execute() } -func (l loadBalancingClient) GetLoadBalancer(ctx context.Context, lbName string) (*loadbalancer.LoadBalancer, error) { - lb, err := l.Client.GetLoadBalancer(ctx, l.projectID, l.region, lbName).Execute() +func (l loadBalancingClient) ListLoadBalancers(ctx context.Context) ([]loadbalancer.LoadBalancer, error) { + lbResponse, err := l.Client.ListLoadBalancers(ctx, l.projectID, l.region).Execute() if err != nil { - if IsNotFound(err) { - return nil, nil - } return nil, err } - return lb, nil + return lbResponse.GetLoadBalancers(), nil } -func (l loadBalancingClient) CreateLoadBalancer(ctx context.Context, payload loadbalancer.CreateLoadBalancerPayload) (*loadbalancer.LoadBalancer, error) { - return l.Client.CreateLoadBalancer(ctx, l.projectID, l.region).CreateLoadBalancerPayload(payload).Execute() +func (l loadBalancingClient) UpdateLoadBalancer(ctx context.Context, name string, payload loadbalancer.UpdateLoadBalancerPayload) (*loadbalancer.LoadBalancer, error) { + return l.Client.UpdateLoadBalancer(ctx, l.projectID, l.region, name).UpdateLoadBalancerPayload(payload).Execute() } -func (l loadBalancingClient) UpdateLoadBalancer(ctx context.Context, lbName string, payload loadbalancer.UpdateLoadBalancerPayload) (*loadbalancer.LoadBalancer, error) { - return l.Client.UpdateLoadBalancer(ctx, l.projectID, l.region, lbName).UpdateLoadBalancerPayload(payload).Execute() +func (l loadBalancingClient) UpdateLoadBalancerTargetPool(ctx context.Context, name, targetPool string, payload loadbalancer.UpdateTargetPoolPayload) (*loadbalancer.TargetPool, error) { + return l.Client.UpdateTargetPool(ctx, l.projectID, l.region, name, targetPool).UpdateTargetPoolPayload(payload).Execute() } -func (l loadBalancingClient) UpdateLoadBalancerTargetPool(ctx context.Context, lbName, tpName string, payload loadbalancer.UpdateTargetPoolPayload) (*loadbalancer.TargetPool, error) { - return l.Client.UpdateTargetPool(ctx, l.projectID, l.region, lbName, tpName).UpdateTargetPoolPayload(payload).Execute() +func (l loadBalancingClient) DeleteLoadBalancer(ctx context.Context, name string) error { + _, err := l.Client.DeleteLoadBalancer(ctx, l.projectID, l.region, name).Execute() + return err } diff --git a/pkg/stackit/client/mock/loadbalancing_mock.go b/pkg/stackit/client/mock/loadbalancing_mock.go index ba8f4cc2..dfe1e7b2 100644 --- a/pkg/stackit/client/mock/loadbalancing_mock.go +++ b/pkg/stackit/client/mock/loadbalancing_mock.go @@ -57,32 +57,32 @@ func (mr *MockLoadBalancingClientMockRecorder) CreateLoadBalancer(ctx, payload a } // DeleteLoadBalancer mocks base method. -func (m *MockLoadBalancingClient) DeleteLoadBalancer(ctx context.Context, lbName string) error { +func (m *MockLoadBalancingClient) DeleteLoadBalancer(ctx context.Context, name string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteLoadBalancer", ctx, lbName) + ret := m.ctrl.Call(m, "DeleteLoadBalancer", ctx, name) ret0, _ := ret[0].(error) return ret0 } // DeleteLoadBalancer indicates an expected call of DeleteLoadBalancer. -func (mr *MockLoadBalancingClientMockRecorder) DeleteLoadBalancer(ctx, lbName any) *gomock.Call { +func (mr *MockLoadBalancingClientMockRecorder) DeleteLoadBalancer(ctx, name any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteLoadBalancer", reflect.TypeOf((*MockLoadBalancingClient)(nil).DeleteLoadBalancer), ctx, lbName) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteLoadBalancer", reflect.TypeOf((*MockLoadBalancingClient)(nil).DeleteLoadBalancer), ctx, name) } // GetLoadBalancer mocks base method. -func (m *MockLoadBalancingClient) GetLoadBalancer(ctx context.Context, id string) (*v2api.LoadBalancer, error) { +func (m *MockLoadBalancingClient) GetLoadBalancer(ctx context.Context, name string) (*v2api.LoadBalancer, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetLoadBalancer", ctx, id) + ret := m.ctrl.Call(m, "GetLoadBalancer", ctx, name) ret0, _ := ret[0].(*v2api.LoadBalancer) ret1, _ := ret[1].(error) return ret0, ret1 } // GetLoadBalancer indicates an expected call of GetLoadBalancer. -func (mr *MockLoadBalancingClientMockRecorder) GetLoadBalancer(ctx, id any) *gomock.Call { +func (mr *MockLoadBalancingClientMockRecorder) GetLoadBalancer(ctx, name any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLoadBalancer", reflect.TypeOf((*MockLoadBalancingClient)(nil).GetLoadBalancer), ctx, id) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLoadBalancer", reflect.TypeOf((*MockLoadBalancingClient)(nil).GetLoadBalancer), ctx, name) } // ListLoadBalancers mocks base method. @@ -115,31 +115,31 @@ func (mr *MockLoadBalancingClientMockRecorder) ProjectID() *gomock.Call { } // UpdateLoadBalancer mocks base method. -func (m *MockLoadBalancingClient) UpdateLoadBalancer(ctx context.Context, lbName string, payload v2api.UpdateLoadBalancerPayload) (*v2api.LoadBalancer, error) { +func (m *MockLoadBalancingClient) UpdateLoadBalancer(ctx context.Context, name string, payload v2api.UpdateLoadBalancerPayload) (*v2api.LoadBalancer, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpdateLoadBalancer", ctx, lbName, payload) + ret := m.ctrl.Call(m, "UpdateLoadBalancer", ctx, name, payload) ret0, _ := ret[0].(*v2api.LoadBalancer) ret1, _ := ret[1].(error) return ret0, ret1 } // UpdateLoadBalancer indicates an expected call of UpdateLoadBalancer. -func (mr *MockLoadBalancingClientMockRecorder) UpdateLoadBalancer(ctx, lbName, payload any) *gomock.Call { +func (mr *MockLoadBalancingClientMockRecorder) UpdateLoadBalancer(ctx, name, payload any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateLoadBalancer", reflect.TypeOf((*MockLoadBalancingClient)(nil).UpdateLoadBalancer), ctx, lbName, payload) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateLoadBalancer", reflect.TypeOf((*MockLoadBalancingClient)(nil).UpdateLoadBalancer), ctx, name, payload) } // UpdateLoadBalancerTargetPool mocks base method. -func (m *MockLoadBalancingClient) UpdateLoadBalancerTargetPool(ctx context.Context, lbName, tpName string, payload v2api.UpdateTargetPoolPayload) (*v2api.TargetPool, error) { +func (m *MockLoadBalancingClient) UpdateLoadBalancerTargetPool(ctx context.Context, name, targetPool string, payload v2api.UpdateTargetPoolPayload) (*v2api.TargetPool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpdateLoadBalancerTargetPool", ctx, lbName, tpName, payload) + ret := m.ctrl.Call(m, "UpdateLoadBalancerTargetPool", ctx, name, targetPool, payload) ret0, _ := ret[0].(*v2api.TargetPool) ret1, _ := ret[1].(error) return ret0, ret1 } // UpdateLoadBalancerTargetPool indicates an expected call of UpdateLoadBalancerTargetPool. -func (mr *MockLoadBalancingClientMockRecorder) UpdateLoadBalancerTargetPool(ctx, lbName, tpName, payload any) *gomock.Call { +func (mr *MockLoadBalancingClientMockRecorder) UpdateLoadBalancerTargetPool(ctx, name, targetPool, payload any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateLoadBalancerTargetPool", reflect.TypeOf((*MockLoadBalancingClient)(nil).UpdateLoadBalancerTargetPool), ctx, lbName, tpName, payload) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateLoadBalancerTargetPool", reflect.TypeOf((*MockLoadBalancingClient)(nil).UpdateLoadBalancerTargetPool), ctx, name, targetPool, payload) } diff --git a/test/integration/selfhostedshootexposure/stackit/selfhostedshootexposure_test.go b/test/integration/selfhostedshootexposure/stackit/selfhostedshootexposure_test.go index 8915b36e..10ba34e8 100644 --- a/test/integration/selfhostedshootexposure/stackit/selfhostedshootexposure_test.go +++ b/test/integration/selfhostedshootexposure/stackit/selfhostedshootexposure_test.go @@ -21,6 +21,7 @@ import ( iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" loadbalancer "github.com/stackitcloud/stackit-sdk-go/services/loadbalancer/v2api" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -29,10 +30,8 @@ import ( "k8s.io/apimachinery/pkg/runtime/serializer/json" "k8s.io/apimachinery/pkg/util/uuid" schemev1 "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/envtest" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -144,11 +143,6 @@ var _ = BeforeSuite(func() { Expect(err).ToNot(HaveOccurred()) Expect(restConfig).ToNot(BeNil()) - httpClient, err := rest.HTTPClientFor(restConfig) - Expect(err).NotTo(HaveOccurred()) - mapper, err := apiutil.NewDynamicRESTMapper(restConfig, httpClient) - Expect(err).NotTo(HaveOccurred()) - scheme := runtime.NewScheme() Expect(schemev1.AddToScheme(scheme)).To(Succeed()) Expect(extensionsv1alpha1.AddToScheme(scheme)).To(Succeed()) @@ -162,7 +156,6 @@ var _ = BeforeSuite(func() { BindAddress: "0", }, Cache: cache.Options{ - Mapper: mapper, ByObject: map[client.Object]cache.ByObject{ &extensionsv1alpha1.SelfHostedShootExposure{}: { Label: labels.SelectorFromSet(labels.Set{"test-id": testID}), @@ -214,19 +207,43 @@ var _ = Describe("SelfHostedShootExposure tests", func() { }) AfterEach(func() { - // Best-effort cleanup of LB via STACKIT API (in case the controller didn't delete it) - if lbName != "" { - lb, _ := lbClient.GetLoadBalancer(ctx, lbName) - if lb != nil { - log.Info("Cleaning up leftover load balancer", "name", lbName) - _ = lbClient.DeleteLoadBalancer(ctx, lbName) - } + // Delete the SelfHostedShootExposure CR first so the controller stops reconciling and + // recreating the LB. controller-runtime keeps retrying failed reconciles with exponential + // backoff regardless of the operation-annotation predicate — without this CR-level delete, + // the LB cleanup below races against the controller's recreate loop. + exposure := &extensionsv1alpha1.SelfHostedShootExposure{ + ObjectMeta: metav1.ObjectMeta{Name: exposureName, Namespace: namespaceName}, } + Expect(client.IgnoreNotFound(c.Delete(ctx, exposure))).To(Succeed()) + Eventually(func() bool { + return apierrors.IsNotFound(c.Get(ctx, client.ObjectKeyFromObject(exposure), exposure)) + }).WithTimeout(5*time.Minute).WithPolling(5*time.Second).Should(BeTrue(), "exposure CR was not deleted") + + // Safety-net: poll until the LB is fully deleted from STACKIT. Normally the controller's + // Delete flow above already removed it; this catches orphans from prior runs that crashed + // before AfterEach could finish. + Eventually(func() error { + lb, err := lbClient.GetLoadBalancer(ctx, lbName) + if stackitclient.IsNotFound(err) || lb == nil { + return nil + } + if err != nil { + return fmt.Errorf("getting load balancer: %w", err) + } + log.Info("Cleaning up leftover load balancer", "name", lbName) + if delErr := lbClient.DeleteLoadBalancer(ctx, lbName); delErr != nil && !stackitclient.IsNotFound(delErr) { + return fmt.Errorf("deleting load balancer: %w", delErr) + } + return fmt.Errorf("load balancer still exists, waiting for deletion") + }).WithTimeout(5 * time.Minute).WithPolling(5 * time.Second).Should(Succeed()) - // Cleanup network + // Network deletion only succeeds once the LB has released the network, so this naturally + // chains after the LB-deletion poll above. if networkID != "" { log.Info("Cleaning up network", "id", networkID) - _ = iaasClient.DeleteNetwork(ctx, networkID) + Eventually(func() error { + return iaasClient.DeleteNetwork(ctx, networkID) + }).WithTimeout(2 * time.Minute).WithPolling(5 * time.Second).Should(Succeed()) } }) @@ -345,6 +362,12 @@ var _ = Describe("SelfHostedShootExposure tests", func() { Type: "stackit", }, Port: 6443, + CredentialsRef: &corev1.ObjectReference{ + APIVersion: "v1", + Kind: "Secret", + Name: "cloudprovider", + Namespace: namespaceName, + }, Endpoints: []extensionsv1alpha1.ControlPlaneEndpoint{ { NodeName: "node-1", @@ -396,18 +419,18 @@ var _ = Describe("SelfHostedShootExposure tests", func() { // Listener g.Expect(lb.Listeners).To(HaveLen(1)) g.Expect(lb.Listeners[0].DisplayName).NotTo(BeNil()) - g.Expect(*lb.Listeners[0].DisplayName).To(Equal("listener-control-plane")) + g.Expect(*lb.Listeners[0].DisplayName).To(Equal("control-plane")) g.Expect(lb.Listeners[0].Port).NotTo(BeNil()) g.Expect(*lb.Listeners[0].Port).To(BeEquivalentTo(6443)) g.Expect(lb.Listeners[0].Protocol).NotTo(BeNil()) g.Expect(*lb.Listeners[0].Protocol).To(Equal("PROTOCOL_TCP")) g.Expect(lb.Listeners[0].TargetPool).NotTo(BeNil()) - g.Expect(*lb.Listeners[0].TargetPool).To(Equal("target-pool-control-plane")) + g.Expect(*lb.Listeners[0].TargetPool).To(Equal("control-plane")) // Target pool g.Expect(lb.TargetPools).To(HaveLen(1)) g.Expect(lb.TargetPools[0].Name).NotTo(BeNil()) - g.Expect(*lb.TargetPools[0].Name).To(Equal("target-pool-control-plane")) + g.Expect(*lb.TargetPools[0].Name).To(Equal("control-plane")) g.Expect(lb.TargetPools[0].TargetPort).NotTo(BeNil()) g.Expect(*lb.TargetPools[0].TargetPort).To(BeEquivalentTo(6443)) g.Expect(lb.TargetPools[0].Targets).To(HaveLen(2)) @@ -451,9 +474,12 @@ var _ = Describe("SelfHostedShootExposure tests", func() { }).WithTimeout(5 * time.Minute).WithPolling(5 * time.Second).Should(Succeed()) By("verify SelfHostedShootExposure CR is being reconciled") - Expect(c.Get(ctx, client.ObjectKeyFromObject(exposure), exposure)).To(Succeed()) - // Finalizer proves Gardener bound the controller to this resource. - Expect(exposure.Finalizers).NotTo(BeEmpty()) + // Finalizer proves Gardener bound the controller to this resource. The cache may take a + // brief moment to observe it, so use Eventually instead of a plain Get. + Eventually(func(g Gomega) { + g.Expect(c.Get(ctx, client.ObjectKeyFromObject(exposure), exposure)).To(Succeed()) + g.Expect(exposure.Finalizers).NotTo(BeEmpty()) + }).WithTimeout(30 * time.Second).WithPolling(2 * time.Second).Should(Succeed()) // Status.Ingress is only populated once the LB reaches READY in the actuator; with // fake targets we never get there, so it stays empty. Expect(exposure.Status.Ingress).To(BeEmpty()) @@ -484,7 +510,7 @@ var _ = Describe("SelfHostedShootExposure tests", func() { g.Expect(lb.Version).NotTo(BeNil()) g.Expect(*lb.Version).NotTo(Equal(initialVersion)) - // Invariants that must not have shifted during the target-pool fast-path update. + // Invariants that must not have shifted during the target-only update. g.Expect(lb.Listeners).To(HaveLen(1)) g.Expect(*lb.Listeners[0].Port).To(BeEquivalentTo(6443)) g.Expect(lb.Networks).To(HaveLen(1)) @@ -502,7 +528,7 @@ var _ = Describe("SelfHostedShootExposure tests", func() { g.Expect(*lbCheck.Version).To(Equal(versionBeforeNoOp)) }).WithTimeout(30 * time.Second).WithPolling(5 * time.Second).Should(Succeed()) - By("remove an endpoint (fast-path shrink)") + By("remove an endpoint") versionBeforeRemove := versionBeforeNoOp patchExposureReconcile(exposure, func() { // Drop node-2 (10.250.0.11), leaving node-1 and node-3. @@ -529,7 +555,7 @@ var _ = Describe("SelfHostedShootExposure tests", func() { By("change plan only (full PUT update)") versionBeforePlanChange := *lb.Version patchExposureReconcile(exposure, func() { - setExposureLBConfig(exposure, &stackitv1alpha1.LoadBalancerConfig{PlanId: new("p50")}) + setExposureLBConfig(exposure, &stackitv1alpha1.LoadBalancer{PlanID: new("p50")}) }) Eventually(func(g Gomega) { @@ -551,7 +577,7 @@ var _ = Describe("SelfHostedShootExposure tests", func() { By("change plan and endpoints together (full PUT update)") versionBeforeCombined := *lb.Version patchExposureReconcile(exposure, func() { - setExposureLBConfig(exposure, &stackitv1alpha1.LoadBalancerConfig{PlanId: new("p250")}) + setExposureLBConfig(exposure, &stackitv1alpha1.LoadBalancer{PlanID: new("p250")}) // Add node-2 back, so we're changing targets *and* plan in the same reconcile. exposure.Spec.Endpoints = append(exposure.Spec.Endpoints, extensionsv1alpha1.ControlPlaneEndpoint{ NodeName: "node-2", @@ -577,9 +603,9 @@ var _ = Describe("SelfHostedShootExposure tests", func() { By("add AccessControl.AllowedSourceRanges (full PUT update)") versionBeforeACLAdd := *lb.Version patchExposureReconcile(exposure, func() { - setExposureLBConfig(exposure, &stackitv1alpha1.LoadBalancerConfig{ - PlanId: new("p250"), - AccessControl: &stackitv1alpha1.AccessControlConfig{ + setExposureLBConfig(exposure, &stackitv1alpha1.LoadBalancer{ + PlanID: new("p250"), + AccessControl: &stackitv1alpha1.AccessControl{ AllowedSourceRanges: []string{"0.0.0.0/0"}, }, }) @@ -604,9 +630,9 @@ var _ = Describe("SelfHostedShootExposure tests", func() { By("update AllowedSourceRanges to a different set (order-independent)") versionBeforeACLUpdate := *lb.Version patchExposureReconcile(exposure, func() { - setExposureLBConfig(exposure, &stackitv1alpha1.LoadBalancerConfig{ - PlanId: new("p250"), - AccessControl: &stackitv1alpha1.AccessControlConfig{ + setExposureLBConfig(exposure, &stackitv1alpha1.LoadBalancer{ + PlanID: new("p250"), + AccessControl: &stackitv1alpha1.AccessControl{ AllowedSourceRanges: []string{"192.168.0.0/16", "10.0.0.0/8"}, }, }) @@ -628,7 +654,7 @@ var _ = Describe("SelfHostedShootExposure tests", func() { versionBeforeACLRemove := *lb.Version patchExposureReconcile(exposure, func() { // Drop AccessControl entirely from the providerConfig. - setExposureLBConfig(exposure, &stackitv1alpha1.LoadBalancerConfig{PlanId: new("p250")}) + setExposureLBConfig(exposure, &stackitv1alpha1.LoadBalancer{PlanID: new("p250")}) }) Eventually(func(g Gomega) { @@ -697,7 +723,7 @@ func targetIPs(lb *loadbalancer.LoadBalancer) []string { // setExposureLBConfig encodes a SelfHostedShootExposureConfig wrapping lbConfig and sets it // as the exposure's ProviderConfig. Fully replaces any previous ProviderConfig. -func setExposureLBConfig(exposure *extensionsv1alpha1.SelfHostedShootExposure, lbConfig *stackitv1alpha1.LoadBalancerConfig) { +func setExposureLBConfig(exposure *extensionsv1alpha1.SelfHostedShootExposure, lbConfig *stackitv1alpha1.LoadBalancer) { GinkgoHelper() buf := new(bytes.Buffer) Expect(encoder.Encode(&stackitv1alpha1.SelfHostedShootExposureConfig{ From af947e84bd75ae0aa2948b6ec001779f996fb9cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Amand?= Date: Wed, 29 Apr 2026 16:19:43 +0200 Subject: [PATCH 8/9] chore: Add test-integration make target (run all integration tests) --- Makefile | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 38bb3bc1..1611ad81 100644 --- a/Makefile +++ b/Makefile @@ -20,8 +20,6 @@ LEADER_ELECTION := false REGION := eu01 FLOATING_POOL_NAME := floating-net -INFRA_TEST_FLAGS := --region='$(REGION)' - SHELL=/usr/bin/env bash -o pipefail ######################################### @@ -164,6 +162,16 @@ verify: check check-format test ## Run check, format and test .PHONY: verify-extended verify-extended: check-generate check check-format test ## Run check-generate, check, format and test +.PHONY: test-integration +test-integration: $(REPORT_COLLECTOR) $(SETUP_ENVTEST) $(GINKGO) ## Run all integration tests + @GINKGO=$(GINKGO) ./hack/test-integration.sh \ + -v --show-node-events \ + --procs 2 --timeout 20m \ + --grace-period 3m \ + ./test/integration/... \ + -- \ + --region='$(REGION)' + .PHONY: test-integration-infra test-integration-infra: $(REPORT_COLLECTOR) $(SETUP_ENVTEST) $(GINKGO) ## Run infrastructure integration tests @GINKGO=$(GINKGO) ./hack/test-integration.sh \ @@ -172,7 +180,7 @@ test-integration-infra: $(REPORT_COLLECTOR) $(SETUP_ENVTEST) $(GINKGO) ## Run in --grace-period 3m \ ./test/integration/infrastructure/stackit \ -- \ - $(INFRA_TEST_FLAGS) + --region='$(REGION)' .PHONY: test-integration-exposure test-integration-exposure: $(REPORT_COLLECTOR) $(SETUP_ENVTEST) $(GINKGO) ## Run selfhostedshootexposure integration tests @@ -182,7 +190,7 @@ test-integration-exposure: $(REPORT_COLLECTOR) $(SETUP_ENVTEST) $(GINKGO) ## Run --grace-period 3m \ ./test/integration/selfhostedshootexposure/stackit \ -- \ - $(EXPOSURE_TEST_FLAGS) + --region='$(REGION)' help: ## Display this help @awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make \033[36m\033[0m\n"} /^[a-zA-Z_-]+:.*?##/ { printf " \033[36m%-15s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST) From 34605367f581ccdbebb3b4cc8d2548336f75e7a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Amand?= Date: Wed, 29 Apr 2026 16:28:04 +0200 Subject: [PATCH 9/9] fix: Small fixes/refactors based on PR feedback --- pkg/controller/selfhostedshootexposure/options.go | 2 +- .../resources_loadbalancer.go | 9 ++++----- pkg/stackit/client/loadbalancing.go | 5 ----- pkg/stackit/client/mock/loadbalancing_mock.go | 15 --------------- 4 files changed, 5 insertions(+), 26 deletions(-) diff --git a/pkg/controller/selfhostedshootexposure/options.go b/pkg/controller/selfhostedshootexposure/options.go index 3a5b7c0f..a5bb7435 100644 --- a/pkg/controller/selfhostedshootexposure/options.go +++ b/pkg/controller/selfhostedshootexposure/options.go @@ -77,7 +77,7 @@ func (a *Actuator) DetermineOptions(ctx context.Context, exposure *extensionsv1a } a.Client.Scheme().Default(providerConfig) - if errs := validation.ValidateSelfHostedShootExposureConfig(providerConfig, field.NewPath("spec.providerConfig")); len(errs) > 0 { + if errs := validation.ValidateSelfHostedShootExposureConfig(providerConfig, field.NewPath("spec", "providerConfig")); len(errs) > 0 { return nil, fmt.Errorf("invalid providerConfig: %w", errs.ToAggregate()) } diff --git a/pkg/controller/selfhostedshootexposure/resources_loadbalancer.go b/pkg/controller/selfhostedshootexposure/resources_loadbalancer.go index 66f22d01..4f1f094e 100644 --- a/pkg/controller/selfhostedshootexposure/resources_loadbalancer.go +++ b/pkg/controller/selfhostedshootexposure/resources_loadbalancer.go @@ -47,11 +47,10 @@ func (r *Resources) reconcileLoadBalancer(ctx context.Context, log logr.Logger) if !r.loadBalancerNeedsUpdate(targets) { return nil } - // STACKIT's UpdateLoadBalancer is a single PUT covering all managed fields (targets, plan, - // ACL). UpdateLoadBalancerTargetPool would also work for target-only changes, but it returns - // only the TargetPool — we'd still need a follow-up GET to refresh r.LoadBalancer for the - // readiness check, so it costs an extra round-trip without a server-side latency win - // (STACKIT transitions the LB to PENDING on either write). + // STACKIT also exposes a partial target-pool update endpoint, but it returns only the + // TargetPool — we'd still need a follow-up GET to refresh r.LoadBalancer for the readiness + // check, so it costs an extra round-trip without a server-side latency win (STACKIT + // transitions the LB to PENDING on either write). Use the full PUT instead. return r.updateLoadBalancer(ctx, log, targets) } diff --git a/pkg/stackit/client/loadbalancing.go b/pkg/stackit/client/loadbalancing.go index 681deded..66bb7e68 100644 --- a/pkg/stackit/client/loadbalancing.go +++ b/pkg/stackit/client/loadbalancing.go @@ -17,7 +17,6 @@ type LoadBalancingClient interface { GetLoadBalancer(ctx context.Context, name string) (*loadbalancer.LoadBalancer, error) ListLoadBalancers(ctx context.Context) ([]loadbalancer.LoadBalancer, error) UpdateLoadBalancer(ctx context.Context, name string, payload loadbalancer.UpdateLoadBalancerPayload) (*loadbalancer.LoadBalancer, error) - UpdateLoadBalancerTargetPool(ctx context.Context, name, targetPool string, payload loadbalancer.UpdateTargetPoolPayload) (*loadbalancer.TargetPool, error) DeleteLoadBalancer(ctx context.Context, name string) error } @@ -69,10 +68,6 @@ func (l loadBalancingClient) UpdateLoadBalancer(ctx context.Context, name string return l.Client.UpdateLoadBalancer(ctx, l.projectID, l.region, name).UpdateLoadBalancerPayload(payload).Execute() } -func (l loadBalancingClient) UpdateLoadBalancerTargetPool(ctx context.Context, name, targetPool string, payload loadbalancer.UpdateTargetPoolPayload) (*loadbalancer.TargetPool, error) { - return l.Client.UpdateTargetPool(ctx, l.projectID, l.region, name, targetPool).UpdateTargetPoolPayload(payload).Execute() -} - func (l loadBalancingClient) DeleteLoadBalancer(ctx context.Context, name string) error { _, err := l.Client.DeleteLoadBalancer(ctx, l.projectID, l.region, name).Execute() return err diff --git a/pkg/stackit/client/mock/loadbalancing_mock.go b/pkg/stackit/client/mock/loadbalancing_mock.go index dfe1e7b2..cd26db6c 100644 --- a/pkg/stackit/client/mock/loadbalancing_mock.go +++ b/pkg/stackit/client/mock/loadbalancing_mock.go @@ -128,18 +128,3 @@ func (mr *MockLoadBalancingClientMockRecorder) UpdateLoadBalancer(ctx, name, pay mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateLoadBalancer", reflect.TypeOf((*MockLoadBalancingClient)(nil).UpdateLoadBalancer), ctx, name, payload) } - -// UpdateLoadBalancerTargetPool mocks base method. -func (m *MockLoadBalancingClient) UpdateLoadBalancerTargetPool(ctx context.Context, name, targetPool string, payload v2api.UpdateTargetPoolPayload) (*v2api.TargetPool, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpdateLoadBalancerTargetPool", ctx, name, targetPool, payload) - ret0, _ := ret[0].(*v2api.TargetPool) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// UpdateLoadBalancerTargetPool indicates an expected call of UpdateLoadBalancerTargetPool. -func (mr *MockLoadBalancingClientMockRecorder) UpdateLoadBalancerTargetPool(ctx, name, targetPool, payload any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateLoadBalancerTargetPool", reflect.TypeOf((*MockLoadBalancingClient)(nil).UpdateLoadBalancerTargetPool), ctx, name, targetPool, payload) -}