From ee5a1ec9c0bdac79044026aac86a4b3b09cc4d41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fischer?= Date: Thu, 23 Jul 2026 10:08:06 +0200 Subject: [PATCH] Validating webhook for ingress and ingress class --- .../main.go | 19 ++ deploy/components/cert-manager/issuer.yaml | 27 +++ .../cert-manager/kustomization.yaml | 14 ++ deploy/deployment.yaml | 11 + deploy/kustomization.yaml | 2 + deploy/service.yaml | 21 ++ deploy/validating-webhook.yaml | 47 ++++ docs/user.md | 16 ++ .../ingress/ingressclass_controller.go | 6 +- .../ingress/ingressclass_controller_test.go | 8 +- pkg/controller/ingress/setup.go | 8 +- pkg/controller/ingress/spec/validation.go | 75 +++++++ pkg/controller/ingress/spec/worktree.go | 52 +---- .../ingress/webhook/ingress_webhook.go | 113 ++++++++++ .../ingress/webhook/ingress_webhook_test.go | 129 +++++++++++ .../ingress/webhook/ingressclass_webhook.go | 195 +++++++++++++++++ .../webhook/ingressclass_webhook_test.go | 203 ++++++++++++++++++ pkg/controller/ingress/webhook/suite_test.go | 25 +++ 18 files changed, 919 insertions(+), 52 deletions(-) create mode 100644 deploy/components/cert-manager/issuer.yaml create mode 100644 deploy/components/cert-manager/kustomization.yaml create mode 100644 deploy/service.yaml create mode 100644 deploy/validating-webhook.yaml create mode 100644 pkg/controller/ingress/spec/validation.go create mode 100644 pkg/controller/ingress/webhook/ingress_webhook.go create mode 100644 pkg/controller/ingress/webhook/ingress_webhook_test.go create mode 100644 pkg/controller/ingress/webhook/ingressclass_webhook.go create mode 100644 pkg/controller/ingress/webhook/ingressclass_webhook_test.go create mode 100644 pkg/controller/ingress/webhook/suite_test.go diff --git a/cmd/application-load-balancer-controller/main.go b/cmd/application-load-balancer-controller/main.go index 390f4e7..823ae42 100644 --- a/cmd/application-load-balancer-controller/main.go +++ b/cmd/application-load-balancer-controller/main.go @@ -5,6 +5,7 @@ import ( "os" "github.com/stackitcloud/application-load-balancer-controller/pkg/controller/ingress" + ingresswebhook "github.com/stackitcloud/application-load-balancer-controller/pkg/controller/ingress/webhook" albclient "github.com/stackitcloud/application-load-balancer-controller/pkg/stackit" stackitconfig "github.com/stackitcloud/application-load-balancer-controller/pkg/stackit/config" sdkconfig "github.com/stackitcloud/stackit-sdk-go/core/config" @@ -16,6 +17,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) var ( @@ -124,6 +127,22 @@ func main() { os.Exit(1) } + // The API reader is used instead of the cached client so that the IngressClass + // lookups always reflect the current API server state. + decoder := admission.NewDecoder(mgr.GetScheme()) + mgr.GetWebhookServer().Register("/validate-ingress", &webhook.Admission{ + Handler: &ingresswebhook.IngressValidator{ + Client: mgr.GetAPIReader(), + Decoder: decoder, + }, + }) + mgr.GetWebhookServer().Register("/validate-ingressclass", &webhook.Admission{ + Handler: &ingresswebhook.IngressClassValidator{ + Client: mgr.GetAPIReader(), + Decoder: decoder, + }, + }) + if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { setupLog.Error(err, "unable to set up health check") os.Exit(1) diff --git a/deploy/components/cert-manager/issuer.yaml b/deploy/components/cert-manager/issuer.yaml new file mode 100644 index 0000000..8dfaf9d --- /dev/null +++ b/deploy/components/cert-manager/issuer.yaml @@ -0,0 +1,27 @@ +apiVersion: cert-manager.io/v1 +kind: Issuer +metadata: + name: application-load-balancer-controller + namespace: kube-system + labels: + app.kubernetes.io/name: application-load-balancer-controller + app.kubernetes.io/part-of: stackit-alb +spec: + selfSigned: {} +--- +apiVersion: cert-manager.io/v1 +kind: Certificate +metadata: + name: application-load-balancer-controller-webhook-cert + namespace: kube-system + labels: + app.kubernetes.io/name: application-load-balancer-controller + app.kubernetes.io/part-of: stackit-alb +spec: + secretName: application-load-balancer-controller-webhook-cert + dnsNames: + - application-load-balancer-controller.kube-system.svc + - application-load-balancer-controller.kube-system.svc.cluster.local + issuerRef: + kind: Issuer + name: application-load-balancer-controller diff --git a/deploy/components/cert-manager/kustomization.yaml b/deploy/components/cert-manager/kustomization.yaml new file mode 100644 index 0000000..bcbbe36 --- /dev/null +++ b/deploy/components/cert-manager/kustomization.yaml @@ -0,0 +1,14 @@ +apiVersion: kustomize.config.k8s.io/v1alpha1 +kind: Component + +# Optional cert-manager component. When included from a Kustomization via the +# `components:` field, it contributes a self-signed cert-manager Issuer and a +# Certificate that produces the TLS secret consumed by the webhook server. +# +# Requires cert-manager to be installed in the target cluster. If cert-manager +# is not available, omit this component and provide the secret +# `application-load-balancer-controller-webhook-cert` in the kube-system +# namespace out-of-band, and inject the CA bundle into the +# ValidatingWebhookConfiguration manually. +resources: + - issuer.yaml diff --git a/deploy/deployment.yaml b/deploy/deployment.yaml index ffde176..3def49f 100644 --- a/deploy/deployment.yaml +++ b/deploy/deployment.yaml @@ -47,6 +47,9 @@ spec: - name: probes containerPort: 8081 protocol: TCP + - name: webhook + containerPort: 9443 + protocol: TCP livenessProbe: httpGet: path: /healthz @@ -77,6 +80,9 @@ spec: name: cloud-config - mountPath: /etc/serviceaccount name: cloud-secret + - mountPath: /tmp/k8s-webhook-server/serving-certs + name: webhook-cert + readOnly: true volumes: - name: cloud-config configMap: @@ -84,4 +90,9 @@ spec: - name: cloud-secret secret: secretName: stackit-cloud-secret + - name: webhook-cert + secret: + # The secret is created by the cert-manager overlay (deploy/cert-manager) + # or must be provided out-of-band by the cluster operator. + secretName: application-load-balancer-controller-webhook-cert terminationGracePeriodSeconds: 30 diff --git a/deploy/kustomization.yaml b/deploy/kustomization.yaml index 026c170..2a0094e 100644 --- a/deploy/kustomization.yaml +++ b/deploy/kustomization.yaml @@ -5,3 +5,5 @@ resources: - serviceaccount.yaml - rbac.yaml - deployment.yaml + - service.yaml + - validating-webhook.yaml diff --git a/deploy/service.yaml b/deploy/service.yaml new file mode 100644 index 0000000..7e0cc3c --- /dev/null +++ b/deploy/service.yaml @@ -0,0 +1,21 @@ +apiVersion: v1 +kind: Service +metadata: + name: application-load-balancer-controller + namespace: kube-system + labels: + app.kubernetes.io/name: application-load-balancer-controller + app.kubernetes.io/part-of: stackit-alb +spec: + type: ClusterIP + selector: + app.kubernetes.io/name: application-load-balancer-controller + ports: + - name: webhook + port: 443 + targetPort: webhook + protocol: TCP + - name: metrics + port: 8080 + targetPort: metrics + protocol: TCP diff --git a/deploy/validating-webhook.yaml b/deploy/validating-webhook.yaml new file mode 100644 index 0000000..2bd4c28 --- /dev/null +++ b/deploy/validating-webhook.yaml @@ -0,0 +1,47 @@ +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + name: application-load-balancer-controller + labels: + app.kubernetes.io/name: application-load-balancer-controller + app.kubernetes.io/part-of: stackit-alb + annotations: + # cert-manager injects the CA bundle from the certificate referenced below. + # When applied without the cert-manager overlay, operators must set + # webhooks[*].clientConfig.caBundle out-of-band. + cert-manager.io/inject-ca-from: kube-system/application-load-balancer-controller-webhook-cert +webhooks: + - name: validate-ingress.alb.stackit.cloud + rules: + - apiGroups: ["networking.k8s.io"] + apiVersions: ["v1"] + operations: ["CREATE", "UPDATE"] + resources: ["ingresses"] + scope: "Namespaced" + clientConfig: + service: + namespace: kube-system + name: application-load-balancer-controller + path: /validate-ingress + port: 443 + admissionReviewVersions: ["v1"] + sideEffects: None + timeoutSeconds: 5 + failurePolicy: Fail + - name: validate-ingressclass.alb.stackit.cloud + rules: + - apiGroups: ["networking.k8s.io"] + apiVersions: ["v1"] + operations: ["CREATE", "UPDATE"] + resources: ["ingressclasses"] + scope: "Cluster" + clientConfig: + service: + namespace: kube-system + name: application-load-balancer-controller + path: /validate-ingressclass + port: 443 + admissionReviewVersions: ["v1"] + sideEffects: None + timeoutSeconds: 5 + failurePolicy: Fail diff --git a/docs/user.md b/docs/user.md index aeb0612..731918f 100644 --- a/docs/user.md +++ b/docs/user.md @@ -200,6 +200,22 @@ If the ingress class results in zero listeners, a dummy listener on port 80 is a This listener always returns the HTTP status code 404. Common scenarios where this can happen is when there are zero ingresses or an HTTPS-only load balancer does not have any certificates yet. +### Validating Admission Webhook + +The controller ships two Kubernetes validating admission webhooks that reject +Ingress and IngressClass objects with invalid annotations at admission time. +This lets you catch mistakes (invalid IP addresses, unsupported plan IDs, +missing mandatory annotations, immutable field changes, etc.) before they are +persisted, instead of only observing them via reconciliation events later. + +The webhooks only validate resources managed by the STACKIT ALB controller: + +- For an `Ingress`, validation is skipped when `spec.ingressClassName` is unset + or references an `IngressClass` whose `spec.controller` is not + `stackit.cloud/alb-ingress`. +- For an `IngressClass`, validation is skipped when `spec.controller` is not + `stackit.cloud/alb-ingress`. + ### Troubleshooting The controller emits events on both ingresses and ingress classes. diff --git a/pkg/controller/ingress/ingressclass_controller.go b/pkg/controller/ingress/ingressclass_controller.go index 87dbd42..e3c97de 100644 --- a/pkg/controller/ingress/ingressclass_controller.go +++ b/pkg/controller/ingress/ingressclass_controller.go @@ -26,8 +26,8 @@ import ( const ( // finalizerName is the name of the finalizer that is added to Ingress and IngressClass finalizerName = "stackit.cloud/alb-ingress" - // controllerName is the name of the ALB controller that the IngressClass should point to for reconciliation - controllerName = "stackit.cloud/alb-ingress" + // ControllerName is the name of the ALB controller that the IngressClass should point to for reconciliation + ControllerName = "stackit.cloud/alb-ingress" // readyRequeueInterval defines how often the controller should check for the ALB to become ready. readyRequeueInterval = 10 * time.Second @@ -53,7 +53,7 @@ func (r *IngressClassReconciler) Reconcile(ctx context.Context, req ctrl.Request } // Check if the IngressClass points to the ALB controller - if ingressClass.Spec.Controller != controllerName { + if ingressClass.Spec.Controller != ControllerName { // If this IngressClass doesn't point to the ALB controller, ignore this IngressClass return ctrl.Result{}, nil } diff --git a/pkg/controller/ingress/ingressclass_controller_test.go b/pkg/controller/ingress/ingressclass_controller_test.go index 986191e..5bd5230 100644 --- a/pkg/controller/ingress/ingressclass_controller_test.go +++ b/pkg/controller/ingress/ingressclass_controller_test.go @@ -156,7 +156,7 @@ var _ = Describe("IngressClassController", func() { }, }, Spec: networkingv1.IngressClassSpec{ - Controller: controllerName, + Controller: ControllerName, }, } Expect(k8sClient.Create(ctx, ingressClass)).To(Succeed()) @@ -185,7 +185,7 @@ var _ = Describe("IngressClassController", func() { }, }, Spec: networkingv1.IngressClassSpec{ - Controller: controllerName, + Controller: ControllerName, }, } Expect(k8sClient.Create(ctx, ingressClass)).To(Succeed()) @@ -418,7 +418,7 @@ var _ = Describe("IngressClassController", func() { }, }, Spec: networkingv1.IngressClassSpec{ - Controller: controllerName, + Controller: ControllerName, }, } testutil.CreateKubernetesResourceAndDeferDeletion(ctx, k8sClient, ingressClass) @@ -468,7 +468,7 @@ var _ = Describe("IngressClassController", func() { }, }, Spec: networkingv1.IngressClassSpec{ - Controller: controllerName, + Controller: ControllerName, }, } testutil.CreateKubernetesResourceAndDeferDeletion(ctx, k8sClient, ignoredIngressClass) diff --git a/pkg/controller/ingress/setup.go b/pkg/controller/ingress/setup.go index 60a9730..ee3483b 100644 --- a/pkg/controller/ingress/setup.go +++ b/pkg/controller/ingress/setup.go @@ -135,7 +135,7 @@ func ingressClassRequestsForReferencingIngresses(ctx context.Context, c client.C if err := c.Get(ctx, types.NamespacedName{Name: className}, class); err != nil { continue } - if class.Spec.Controller == controllerName { + if class.Spec.Controller == ControllerName { reqs = append(reqs, reconcile.Request{NamespacedName: types.NamespacedName{Name: className}}) } } @@ -151,7 +151,7 @@ func nodeEventHandler(c client.Client) handler.EventHandler { } requestList := []ctrl.Request{} for i := range ingressClassList.Items { - if ingressClassList.Items[i].Spec.Controller != controllerName { + if ingressClassList.Items[i].Spec.Controller != ControllerName { continue } requestList = append(requestList, ctrl.Request{ @@ -175,7 +175,7 @@ func ingressEventHandler(c client.Client) handler.EventHandler { return nil } - if ingressClass.Spec.Controller != controllerName { + if ingressClass.Spec.Controller != ControllerName { return nil } @@ -221,6 +221,6 @@ func ingressClassPredicate() predicate.Predicate { if !ok { return false } - return ingressClass.Spec.Controller == controllerName + return ingressClass.Spec.Controller == ControllerName }) } diff --git a/pkg/controller/ingress/spec/validation.go b/pkg/controller/ingress/spec/validation.go new file mode 100644 index 0000000..cee3a22 --- /dev/null +++ b/pkg/controller/ingress/spec/validation.go @@ -0,0 +1,75 @@ +package spec + +import ( + "fmt" + "net" + "net/netip" + "slices" + "strings" + + networkingv1 "k8s.io/api/networking/v1" +) + +// ServicePlans lists all supported ALB service plans. +var ServicePlans = []string{ + "p10", +} + +// DefaultServicePlan is the plan used when no plan is explicitly requested. +const DefaultServicePlan = "p10" + +// ValidateNetworkMode validates that the network mode annotation is set to a supported value. +// Currently only NetworkModeNodePort is supported and the annotation is mandatory. +func ValidateNetworkMode(ingressClass *networkingv1.IngressClass) error { + networkMode := ingressClass.Annotations[AnnotationNetworkMode] + if networkMode != NetworkModeNodePort { + return fmt.Errorf("annotation %s must be set to %s", AnnotationNetworkMode, NetworkModeNodePort) + } + return nil +} + +// ValidateExternalIP validates that the provided external IP annotation value is a valid IPv4 address. +// An empty value is considered valid. +func ValidateExternalIP(value string) error { + if value == "" { + return nil + } + addr, err := netip.ParseAddr(value) + if err != nil { + return fmt.Errorf("failed to parse external IP annotation: %w", err) + } + if !addr.Is4() { + return fmt.Errorf("external IP annotation is not an IPv4 address") + } + return nil +} + +// ValidatePlanID validates that the provided plan ID is one of the supported service plans. +// An empty value is considered valid (defaults to DefaultServicePlan at reconcile time). +func ValidatePlanID(value string) error { + if value == "" { + return nil + } + if !slices.Contains(ServicePlans, value) { + return fmt.Errorf("invalid plan id %q", value) + } + return nil +} + +// ValidateAllowedSourceRanges validates a comma-separated list of CIDR ranges. +// An empty value is considered valid. +func ValidateAllowedSourceRanges(value string) ([]string, error) { + if value == "" { + return nil, nil + } + ranges := strings.Split(value, ",") + for i, r := range ranges { + if k := slices.Index(ranges, r); k < i { + return nil, fmt.Errorf("duplicate range in annotation %s", AnnotationAllowedSourceRanges) + } + if _, _, err := net.ParseCIDR(r); err != nil { + return nil, fmt.Errorf("IP range %d is invalid: %w", i, err) + } + } + return ranges, nil +} diff --git a/pkg/controller/ingress/spec/worktree.go b/pkg/controller/ingress/spec/worktree.go index b0105cb..ff8a96c 100644 --- a/pkg/controller/ingress/spec/worktree.go +++ b/pkg/controller/ingress/spec/worktree.go @@ -10,10 +10,7 @@ import ( "fmt" "maps" "math" - "net" - "net/netip" "slices" - "strings" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" @@ -127,7 +124,7 @@ func BuildTree( //nolint:gocyclo,funlen // Breaking up this function won't make targets := getTargetsOfNodes(nodes) - if err := parseNetworkMode(ingressClass); err != nil { + if err := ValidateNetworkMode(ingressClass); err != nil { return nil, nil, err } @@ -323,59 +320,32 @@ func BuildTree( //nolint:gocyclo,funlen // Breaking up this function won't make return tree, errors, nil } -func parseNetworkMode(ingressClass *networkingv1.IngressClass) error { - networkMode := ingressClass.Annotations[AnnotationNetworkMode] - if networkMode != NetworkModeNodePort { - return fmt.Errorf("annotation %s must be set to %s", AnnotationNetworkMode, NetworkModeNodePort) - } - return nil -} - func parseExternalIP(ingressClass *networkingv1.IngressClass) (string, error) { externalIP := ingressClass.Annotations[AnnotationExternalIP] - if externalIP != "" { - addr, err := netip.ParseAddr(externalIP) - if err != nil { - return "", fmt.Errorf("failed to parse external IP annotation: %w", err) - } - if !addr.Is4() { - return "", fmt.Errorf("external IP annotation is not an IPv4 address") - } + if err := ValidateExternalIP(externalIP); err != nil { + return "", err } return externalIP, nil } -var servicePlans = []string{ - "p10", -} - -const defaultServicePlan = "p10" - func parsePlanID(ingressClass *networkingv1.IngressClass) (string, error) { planID := ingressClass.Annotations[AnnotationPlanID] if planID == "" { - planID = defaultServicePlan + return DefaultServicePlan, nil } - if !slices.Contains(servicePlans, planID) { - return "", fmt.Errorf("invalid plan id %q", planID) + if err := ValidatePlanID(planID); err != nil { + return "", err } return planID, nil } func addAccessControlToTree(tree *WorkTreeALB, ingressClass *networkingv1.IngressClass) error { - annotation := ingressClass.Annotations[AnnotationAllowedSourceRanges] - if annotation == "" { - return nil + ranges, err := ValidateAllowedSourceRanges(ingressClass.Annotations[AnnotationAllowedSourceRanges]) + if err != nil { + return err } - ranges := strings.Split(annotation, ",") - for i, r := range ranges { - if k := slices.Index(ranges, r); k < i { - return fmt.Errorf("duplicate range in annotation %s", AnnotationAllowedSourceRanges) - } - _, _, err := net.ParseCIDR(r) - if err != nil { - return fmt.Errorf("IP range %d is invalid: %w", i, err) - } + if ranges == nil { + return nil } tree.accessControl = &albsdk.LoadbalancerOptionAccessControl{ AllowedSourceRanges: ranges, diff --git a/pkg/controller/ingress/webhook/ingress_webhook.go b/pkg/controller/ingress/webhook/ingress_webhook.go new file mode 100644 index 0000000..6a74600 --- /dev/null +++ b/pkg/controller/ingress/webhook/ingress_webhook.go @@ -0,0 +1,113 @@ +// Package webhook provides validating admission webhooks for STACKIT ALB Ingress and IngressClass resources. +package webhook + +import ( + "context" + "fmt" + "net/http" + "regexp" + "strconv" + + admissionv1 "k8s.io/api/admission/v1" + networkingv1 "k8s.io/api/networking/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/stackitcloud/application-load-balancer-controller/pkg/controller/ingress" + "github.com/stackitcloud/application-load-balancer-controller/pkg/controller/ingress/spec" +) + +// wafNameRegex validates the WAF name annotation. +// The name must start and end with a lowercase alphanumeric character, +// contain only lowercase alphanumeric characters or hyphens, and be +// between 1 and 63 characters long. +var wafNameRegex = regexp.MustCompile(`^[0-9a-z](?:(?:[0-9a-z]|-){0,61}[0-9a-z])?$`) + +// IngressValidator is a validating admission webhook for Ingress resources. +// It only validates ingresses that reference an IngressClass managed by the STACKIT ALB controller. +type IngressValidator struct { + Client client.Reader + Decoder admission.Decoder +} + +// Handle routes the admission request based on the operation type. +// +//nolint:gocritic // admission.Request is passed by value to satisfy the admission.Handler interface +func (v *IngressValidator) Handle(ctx context.Context, req admission.Request) admission.Response { + switch req.Operation { + case admissionv1.Create, admissionv1.Update: + return v.validate(ctx, req) + default: + return admission.Allowed("unhandled operation allowed") + } +} + +//nolint:gocritic // admission.Request is passed by value to match the interface convention +func (v *IngressValidator) validate(ctx context.Context, req admission.Request) admission.Response { + ing := &networkingv1.Ingress{} + if err := v.Decoder.Decode(req, ing); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + if ing.Spec.IngressClassName == nil { + return admission.Allowed("no ingress class specified; ignoring") + } + + ingressClass := &networkingv1.IngressClass{} + // TODO: How do we deal with 404s? The ingress class might be created just moments later. + if err := v.Client.Get(ctx, client.ObjectKey{Name: *ing.Spec.IngressClassName}, ingressClass); err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + if ingressClass.Spec.Controller != ingress.ControllerName { + return admission.Allowed("ingress managed by a different controller; allowing") + } + + if err := validateIngressAnnotations(ing); err != nil { + return admission.Denied(err.Error()) + } + + return admission.Allowed("validation passed") +} + +// validateIngressAnnotations checks formatting, allowed values, and basic constraints +// for annotations that may be set on an Ingress managed by the STACKIT ALB controller. +func validateIngressAnnotations(ing *networkingv1.Ingress) error { + if val, ok := ing.Annotations[spec.AnnotationWAFName]; ok { + if !wafNameRegex.MatchString(val) { + return fmt.Errorf("annotation %q has an invalid value %q: must match %s", + spec.AnnotationWAFName, val, wafNameRegex.String()) + } + } + + boolAnnotations := []string{ + spec.AnnotationTargetPoolTLSEnabled, + spec.AnnotationTargetPoolTLSSkipCertificateValidation, + spec.AnnotationHTTPSOnly, + spec.AnnotationWebSocket, + } + for _, ann := range boolAnnotations { + if val, ok := ing.Annotations[ann]; ok { + if _, err := strconv.ParseBool(val); err != nil { + return fmt.Errorf("annotation %q must be a valid boolean: %w", ann, err) + } + } + } + + if val, ok := ing.Annotations[spec.AnnotationPriority]; ok { + if _, err := strconv.Atoi(val); err != nil { + return fmt.Errorf("annotation %q must be a valid integer: %w", spec.AnnotationPriority, err) + } + } + + for _, ann := range []string{spec.AnnotationHTTPPort, spec.AnnotationHTTPSPort} { + if val, ok := ing.Annotations[ann]; ok { + port, err := strconv.Atoi(val) + if err != nil || port < 1 || port > 65535 { + return fmt.Errorf("annotation %q must be a valid port number between 1 and 65535", ann) + } + } + } + + return nil +} diff --git a/pkg/controller/ingress/webhook/ingress_webhook_test.go b/pkg/controller/ingress/webhook/ingress_webhook_test.go new file mode 100644 index 0000000..054f4b3 --- /dev/null +++ b/pkg/controller/ingress/webhook/ingress_webhook_test.go @@ -0,0 +1,129 @@ +package webhook_test + +import ( + "context" + "encoding/json" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + admissionv1 "k8s.io/api/admission/v1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/stackitcloud/application-load-balancer-controller/pkg/controller/ingress" + "github.com/stackitcloud/application-load-balancer-controller/pkg/controller/ingress/spec" + ingresswebhook "github.com/stackitcloud/application-load-balancer-controller/pkg/controller/ingress/webhook" +) + +var _ = Describe("IngressValidator", func() { + const ( + managedClass = "stackit-alb" + unmanagedClass = "nginx" + ) + + var ( + validator *ingresswebhook.IngressValidator + ) + + BeforeEach(func() { + managed := &networkingv1.IngressClass{ + ObjectMeta: metav1.ObjectMeta{Name: managedClass}, + Spec: networkingv1.IngressClassSpec{Controller: ingress.ControllerName}, + } + unmanaged := &networkingv1.IngressClass{ + ObjectMeta: metav1.ObjectMeta{Name: unmanagedClass}, + Spec: networkingv1.IngressClassSpec{Controller: "k8s.io/ingress-nginx"}, + } + c := fake.NewClientBuilder().WithScheme(testScheme).WithObjects(managed, unmanaged).Build() + validator = &ingresswebhook.IngressValidator{ + Client: c, + Decoder: admission.NewDecoder(testScheme), + } + }) + + DescribeTable("Handle", + func(operation admissionv1.Operation, className *string, annotations map[string]string, expectAllowed bool) { + ing := &networkingv1.Ingress{ + TypeMeta: metav1.TypeMeta{APIVersion: "networking.k8s.io/v1", Kind: "Ingress"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress", + Namespace: "default", + Annotations: annotations, + }, + Spec: networkingv1.IngressSpec{ + IngressClassName: className, + }, + } + raw, err := json.Marshal(ing) + Expect(err).ToNot(HaveOccurred()) + + req := admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: operation, + Object: runtime.RawExtension{Raw: raw}, + }, + } + if operation == admissionv1.Update { + req.OldObject = runtime.RawExtension{Raw: raw} + } + + res := validator.Handle(context.Background(), req) + Expect(res.Allowed).To(Equal(expectAllowed), + "unexpected result, message: %s", resultMessage(res)) + }, + Entry("valid ingress (create)", admissionv1.Create, ptr.To(managedClass), + map[string]string{ + spec.AnnotationHTTPSOnly: "true", + spec.AnnotationPriority: "100", + }, true), + Entry("valid ingress (update)", admissionv1.Update, ptr.To(managedClass), + map[string]string{ + spec.AnnotationHTTPSOnly: "false", + }, true), + Entry("no ingress class - allowed", admissionv1.Create, nil, + map[string]string{}, true), + Entry("unmanaged ingress class - allowed", admissionv1.Create, ptr.To(unmanagedClass), + map[string]string{spec.AnnotationHTTPSOnly: "not-a-bool"}, true), + Entry("invalid boolean (https-only)", admissionv1.Create, ptr.To(managedClass), + map[string]string{spec.AnnotationHTTPSOnly: "not-a-bool"}, false), + Entry("invalid boolean (websocket)", admissionv1.Create, ptr.To(managedClass), + map[string]string{spec.AnnotationWebSocket: "yes"}, false), + Entry("invalid boolean (tls-enabled)", admissionv1.Create, ptr.To(managedClass), + map[string]string{spec.AnnotationTargetPoolTLSEnabled: "on"}, false), + Entry("invalid boolean (tls-skip)", admissionv1.Create, ptr.To(managedClass), + map[string]string{spec.AnnotationTargetPoolTLSSkipCertificateValidation: "on"}, false), + Entry("invalid integer (priority)", admissionv1.Create, ptr.To(managedClass), + map[string]string{spec.AnnotationPriority: "high"}, false), + Entry("valid WAF name", admissionv1.Create, ptr.To(managedClass), + map[string]string{spec.AnnotationWAFName: "my-valid-waf-123"}, true), + Entry("valid WAF name (single char)", admissionv1.Create, ptr.To(managedClass), + map[string]string{spec.AnnotationWAFName: "a"}, true), + Entry("invalid WAF name (uppercase)", admissionv1.Create, ptr.To(managedClass), + map[string]string{spec.AnnotationWAFName: "My-Waf-Name"}, false), + Entry("invalid WAF name (leading hyphen)", admissionv1.Create, ptr.To(managedClass), + map[string]string{spec.AnnotationWAFName: "-my-waf"}, false), + Entry("invalid WAF name (trailing hyphen)", admissionv1.Create, ptr.To(managedClass), + map[string]string{spec.AnnotationWAFName: "my-waf-"}, false), + Entry("invalid WAF name (underscore)", admissionv1.Create, ptr.To(managedClass), + map[string]string{spec.AnnotationWAFName: "my_waf_name"}, false), + Entry("invalid WAF name (too long)", admissionv1.Create, ptr.To(managedClass), + map[string]string{spec.AnnotationWAFName: "a123456789012345678901234567890123456789012345678901234567890123"}, false), + Entry("valid HTTP port", admissionv1.Create, ptr.To(managedClass), + map[string]string{spec.AnnotationHTTPPort: "8080"}, true), + Entry("invalid HTTP port (out of range)", admissionv1.Create, ptr.To(managedClass), + map[string]string{spec.AnnotationHTTPPort: "70000"}, false), + Entry("invalid HTTP port (not a number)", admissionv1.Create, ptr.To(managedClass), + map[string]string{spec.AnnotationHTTPPort: "abc"}, false), + ) +}) + +func resultMessage(res admission.Response) string { //nolint:gocritic // admission.Response is the return type of Handle + if res.Result == nil { + return "" + } + return res.Result.Message +} diff --git a/pkg/controller/ingress/webhook/ingressclass_webhook.go b/pkg/controller/ingress/webhook/ingressclass_webhook.go new file mode 100644 index 0000000..47b0e90 --- /dev/null +++ b/pkg/controller/ingress/webhook/ingressclass_webhook.go @@ -0,0 +1,195 @@ +package webhook + +import ( + "context" + "fmt" + "net/http" + "strconv" + + admissionv1 "k8s.io/api/admission/v1" + networkingv1 "k8s.io/api/networking/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/stackitcloud/application-load-balancer-controller/pkg/controller/ingress" + "github.com/stackitcloud/application-load-balancer-controller/pkg/controller/ingress/spec" +) + +// IngressClassValidator is a validating admission webhook for IngressClass resources +// managed by the STACKIT ALB controller. +type IngressClassValidator struct { + Client client.Reader + Decoder admission.Decoder +} + +// Handle routes the admission request based on the operation type. +// +//nolint:gocritic // admission.Request is passed by value to satisfy the admission.Handler interface +func (v *IngressClassValidator) Handle(ctx context.Context, req admission.Request) admission.Response { + switch req.Operation { + case admissionv1.Create: + return v.handleCreate(ctx, req) + case admissionv1.Update: + return v.handleUpdate(ctx, req) + default: + return admission.Allowed("unhandled operation allowed") + } +} + +//nolint:gocritic // admission.Request is passed by value to match the interface convention +func (v *IngressClassValidator) handleCreate(_ context.Context, req admission.Request) admission.Response { + newClass := &networkingv1.IngressClass{} + if err := v.Decoder.Decode(req, newClass); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + if newClass.Spec.Controller != ingress.ControllerName { + return admission.Allowed("not a STACKIT ALB IngressClass; allowing") + } + + if err := validateIngressClassAnnotations(newClass); err != nil { + return admission.Denied(err.Error()) + } + + return admission.Allowed("validation passed") +} + +//nolint:gocritic // admission.Request is passed by value to match the interface convention +func (v *IngressClassValidator) handleUpdate(ctx context.Context, req admission.Request) admission.Response { + newClass := &networkingv1.IngressClass{} + if err := v.Decoder.Decode(req, newClass); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + oldClass := &networkingv1.IngressClass{} + if err := v.Decoder.DecodeRaw(req.OldObject, oldClass); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + if newClass.Spec.Controller != ingress.ControllerName { + return admission.Allowed("not a STACKIT ALB IngressClass; allowing") + } + + if err := validateIngressClassAnnotations(newClass); err != nil { + return admission.Denied(err.Error()) + } + + // AnnotationInternal is immutable after creation. + oldInternal := oldClass.Annotations[spec.AnnotationInternal] + newInternal := newClass.Annotations[spec.AnnotationInternal] + if oldInternal != newInternal { + return admission.Denied(fmt.Sprintf("annotation %q is immutable and cannot be changed after creation", + spec.AnnotationInternal)) + } + + if err := v.validateExternalIPUpdate(ctx, oldClass, newClass); err != nil { + return admission.Denied(err.Error()) + } + + return admission.Allowed("validation passed") +} + +// validateIngressClassAnnotations checks formatting, allowed values, and basic constraints +// for annotations that may be set on an IngressClass managed by the STACKIT ALB controller. +// +//nolint:gocyclo // Straightforward sequence of independent annotation checks. +func validateIngressClassAnnotations(class *networkingv1.IngressClass) error { + if err := spec.ValidateNetworkMode(class); err != nil { + return err + } + + if val, ok := class.Annotations[spec.AnnotationExternalIP]; ok { + if err := spec.ValidateExternalIP(val); err != nil { + return err + } + } + + if val, ok := class.Annotations[spec.AnnotationInternal]; ok { + if _, err := strconv.ParseBool(val); err != nil { + return fmt.Errorf("annotation %q must be a valid boolean: %w", spec.AnnotationInternal, err) + } + } + + if val, ok := class.Annotations[spec.AnnotationPlanID]; ok { + if err := spec.ValidatePlanID(val); err != nil { + return err + } + } + + for _, ann := range []string{spec.AnnotationHTTPPort, spec.AnnotationHTTPSPort} { + if val, ok := class.Annotations[ann]; ok { + port, err := strconv.Atoi(val) + if err != nil || port < 1 || port > 65535 { + return fmt.Errorf("annotation %q must be a valid port number between 1 and 65535", ann) + } + } + } + + if val, ok := class.Annotations[spec.AnnotationAllowedSourceRanges]; ok { + if _, err := spec.ValidateAllowedSourceRanges(val); err != nil { + return err + } + } + + if val, ok := class.Annotations[spec.AnnotationWAFName]; ok { + if !wafNameRegex.MatchString(val) { + return fmt.Errorf("annotation %q has an invalid value %q: must match %s", + spec.AnnotationWAFName, val, wafNameRegex.String()) + } + } + + return nil +} + +// validateExternalIPUpdate enforces the update rules for the external IP annotation: +// - Changing an existing static IP is not allowed. +// - Promoting an ephemeral IP to a static IP is only allowed when the requested static +// IP matches the currently assigned ephemeral IP. +func (v *IngressClassValidator) validateExternalIPUpdate(ctx context.Context, oldClass, newClass *networkingv1.IngressClass) error { + oldIP, oldHadIP := oldClass.Annotations[spec.AnnotationExternalIP] + newIP, newHasIP := newClass.Annotations[spec.AnnotationExternalIP] + + if oldHadIP && newHasIP && oldIP != newIP { + return fmt.Errorf( + "changing an existing static IP address is not allowed: annotation %q cannot be updated from %q to %q", + spec.AnnotationExternalIP, oldIP, newIP, + ) + } + + if !oldHadIP && newHasIP { + currentIP, err := v.getAssignedEphemeralIP(ctx, newClass.Name) + if err != nil { + return fmt.Errorf("failed to look up currently assigned IP: %w", err) + } + if currentIP == "" || currentIP != newIP { + return fmt.Errorf( + "the load balancer can only be promoted to a static IP address that matches its current ephemeral IP (currently assigned: %q, requested: %q)", + currentIP, newIP, + ) + } + } + + return nil +} + +// getAssignedEphemeralIP scans the cluster for any Ingress that references the given class +// and returns the first IP reported in its status. Returns an empty string if no IP is +// currently assigned. +func (v *IngressClassValidator) getAssignedEphemeralIP(ctx context.Context, className string) (string, error) { + ingressList := &networkingv1.IngressList{} + if err := v.Client.List(ctx, ingressList); err != nil { + return "", err + } + for i := range ingressList.Items { + ing := &ingressList.Items[i] + if ing.Spec.IngressClassName == nil || *ing.Spec.IngressClassName != className { + continue + } + for _, lb := range ing.Status.LoadBalancer.Ingress { + if lb.IP != "" { + return lb.IP, nil + } + } + } + return "", nil +} diff --git a/pkg/controller/ingress/webhook/ingressclass_webhook_test.go b/pkg/controller/ingress/webhook/ingressclass_webhook_test.go new file mode 100644 index 0000000..8b9189c --- /dev/null +++ b/pkg/controller/ingress/webhook/ingressclass_webhook_test.go @@ -0,0 +1,203 @@ +package webhook_test + +import ( + "context" + "encoding/json" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + admissionv1 "k8s.io/api/admission/v1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/stackitcloud/application-load-balancer-controller/pkg/controller/ingress" + "github.com/stackitcloud/application-load-balancer-controller/pkg/controller/ingress/spec" + ingresswebhook "github.com/stackitcloud/application-load-balancer-controller/pkg/controller/ingress/webhook" +) + +var _ = Describe("IngressClassValidator", func() { + const ( + testClassName = "test-class" + managedController = ingress.ControllerName + unmanagedController = "k8s.io/ingress-nginx" + ) + + newValidator := func(ephemeralIP string) *ingresswebhook.IngressClassValidator { + builder := fake.NewClientBuilder().WithScheme(testScheme) + if ephemeralIP != "" { + builder = builder.WithObjects(&networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{Name: "dummy-ingress", Namespace: "default"}, + Spec: networkingv1.IngressSpec{ + IngressClassName: ptr.To(testClassName), + }, + Status: networkingv1.IngressStatus{ + LoadBalancer: networkingv1.IngressLoadBalancerStatus{ + Ingress: []networkingv1.IngressLoadBalancerIngress{{IP: ephemeralIP}}, + }, + }, + }) + } + return &ingresswebhook.IngressClassValidator{ + Client: builder.Build(), + Decoder: admission.NewDecoder(testScheme), + } + } + + handle := func( + v *ingresswebhook.IngressClassValidator, + operation admissionv1.Operation, + controller string, + oldAnnotations, newAnnotations map[string]string, + ) admission.Response { + newClass := &networkingv1.IngressClass{ + ObjectMeta: metav1.ObjectMeta{Name: testClassName, Annotations: newAnnotations}, + Spec: networkingv1.IngressClassSpec{Controller: controller}, + } + rawNew, err := json.Marshal(newClass) + Expect(err).ToNot(HaveOccurred()) + + req := admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: operation, + Object: runtime.RawExtension{Raw: rawNew}, + }, + } + if operation == admissionv1.Update { + oldClass := &networkingv1.IngressClass{ + ObjectMeta: metav1.ObjectMeta{Name: testClassName, Annotations: oldAnnotations}, + Spec: networkingv1.IngressClassSpec{Controller: controller}, + } + rawOld, err := json.Marshal(oldClass) + Expect(err).ToNot(HaveOccurred()) + req.OldObject = runtime.RawExtension{Raw: rawOld} + } + return v.Handle(context.Background(), req) + } + + DescribeTable("Create", + func(controller string, annotations map[string]string, expectAllowed bool) { + res := handle(newValidator(""), admissionv1.Create, controller, nil, annotations) + Expect(res.Allowed).To(Equal(expectAllowed), + "unexpected result, message: %s", resultMessage(res)) + }, + Entry("valid IngressClass", managedController, map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationPlanID: "p10", + spec.AnnotationHTTPPort: "80", + }, true), + Entry("unmanaged controller - allowed even with invalid values", + unmanagedController, map[string]string{spec.AnnotationPlanID: "invalid-plan"}, true), + Entry("missing network-mode - denied", managedController, map[string]string{ + spec.AnnotationPlanID: "p10", + }, false), + Entry("invalid network-mode - denied", managedController, map[string]string{ + spec.AnnotationNetworkMode: "LoadBalancer", + }, false), + Entry("invalid IP address - denied", managedController, map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationExternalIP: "not-an-ip-address", + }, false), + Entry("IPv6 address - denied", managedController, map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationExternalIP: "2001:db8::1", + }, false), + Entry("valid IPv4 - allowed", managedController, map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationExternalIP: "1.2.3.4", + }, true), + Entry("invalid plan-id - denied", managedController, map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationPlanID: "p100", + }, false), + Entry("invalid port - denied", managedController, map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationHTTPPort: "99999", + }, false), + Entry("valid allowed-source-ranges - allowed", managedController, map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationAllowedSourceRanges: "10.0.0.0/24,192.168.0.0/16", + }, true), + Entry("invalid allowed-source-ranges - denied", managedController, map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationAllowedSourceRanges: "10.0.0.0/24,not-a-cidr", + }, false), + Entry("invalid internal (bool) - denied", managedController, map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationInternal: "maybe", + }, false), + Entry("valid WAF name - allowed", managedController, map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationWAFName: "my-waf", + }, true), + Entry("invalid WAF name - denied", managedController, map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationWAFName: "My-WAF", + }, false), + ) + + DescribeTable("Update", + func(oldAnn, newAnn map[string]string, ephemeralIP string, expectAllowed bool) { + res := handle(newValidator(ephemeralIP), admissionv1.Update, managedController, oldAnn, newAnn) + Expect(res.Allowed).To(Equal(expectAllowed), + "unexpected result, message: %s", resultMessage(res)) + }, + Entry("keep same static IP - allowed", + map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationExternalIP: "1.2.3.4", + }, + map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationExternalIP: "1.2.3.4", + }, "", true), + Entry("change existing static IP - denied", + map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationExternalIP: "1.2.3.4", + }, + map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationExternalIP: "5.6.7.8", + }, "", false), + Entry("promote ephemeral to static (match) - allowed", + map[string]string{spec.AnnotationNetworkMode: spec.NetworkModeNodePort}, + map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationExternalIP: "9.9.9.9", + }, "9.9.9.9", true), + Entry("promote ephemeral to static (mismatch) - denied", + map[string]string{spec.AnnotationNetworkMode: spec.NetworkModeNodePort}, + map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationExternalIP: "8.8.8.8", + }, "9.9.9.9", false), + Entry("promote ephemeral to static (no IP assigned) - denied", + map[string]string{spec.AnnotationNetworkMode: spec.NetworkModeNodePort}, + map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationExternalIP: "8.8.8.8", + }, "", false), + Entry("change internal annotation - denied", + map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationInternal: "true", + }, + map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationInternal: "false", + }, "", false), + Entry("keep internal annotation - allowed", + map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationInternal: "true", + }, + map[string]string{ + spec.AnnotationNetworkMode: spec.NetworkModeNodePort, + spec.AnnotationInternal: "true", + }, "", true), + ) +}) diff --git a/pkg/controller/ingress/webhook/suite_test.go b/pkg/controller/ingress/webhook/suite_test.go new file mode 100644 index 0000000..30e938e --- /dev/null +++ b/pkg/controller/ingress/webhook/suite_test.go @@ -0,0 +1,25 @@ +package webhook_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + networkingv1 "k8s.io/api/networking/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/scheme" +) + +// testScheme is the scheme used by all webhook tests. +// It includes the built-in client-go scheme plus networking/v1 types. +var testScheme *runtime.Scheme + +func TestWebhook(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Webhook Suite") +} + +var _ = BeforeSuite(func() { + testScheme = scheme.Scheme + Expect(networkingv1.AddToScheme(testScheme)).To(Succeed()) +})