diff --git a/internal/webhook/v1/ingress_webhook.go b/internal/webhook/v1/ingress_webhook.go index a2f9f2d4..4c198bde 100644 --- a/internal/webhook/v1/ingress_webhook.go +++ b/internal/webhook/v1/ingress_webhook.go @@ -28,6 +28,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + "github.com/apache/apisix-ingress-controller/internal/adc/translator/annotations" "github.com/apache/apisix-ingress-controller/internal/controller" "github.com/apache/apisix-ingress-controller/internal/webhook/v1/reference" sslvalidator "github.com/apache/apisix-ingress-controller/internal/webhook/v1/ssl" @@ -82,6 +83,10 @@ func (v *IngressCustomValidator) ValidateCreate(ctx context.Context, obj runtime return nil, fmt.Errorf("%s", sslvalidator.FormatConflicts(conflicts)) } + if err := validateAnnotations(ingress); err != nil { + return nil, err + } + warnings := v.collectReferenceWarnings(ctx, ingress) return warnings, nil } @@ -103,10 +108,28 @@ func (v *IngressCustomValidator) ValidateUpdate(ctx context.Context, oldObj, new return nil, fmt.Errorf("%s", sslvalidator.FormatConflicts(conflicts)) } + if err := validateAnnotations(ingress); err != nil { + return nil, err + } + warnings := v.collectReferenceWarnings(ctx, ingress) return warnings, nil } +// validateAnnotations rejects annotation combinations that would otherwise be +// silently dropped, leaving the route without a requested security plugin. +// enable-csrf with no csrf-key is refused here so kubectl apply fails loudly +// instead of programming a route the operator believes is protected. +func validateAnnotations(ingress *networkingv1.Ingress) error { + e := annotations.NewExtractor(ingress.Annotations) + if e.GetBoolAnnotation(annotations.AnnotationsEnableCsrf) && + e.GetStringAnnotation(annotations.AnnotationsCsrfKey) == "" { + return fmt.Errorf("annotation %q is enabled but %q is missing or empty", + annotations.AnnotationsEnableCsrf, annotations.AnnotationsCsrfKey) + } + return nil +} + // ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type Ingress. func (v *IngressCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { return nil, nil diff --git a/internal/webhook/v1/ingress_webhook_test.go b/internal/webhook/v1/ingress_webhook_test.go index d6bce33f..0a5481c4 100644 --- a/internal/webhook/v1/ingress_webhook_test.go +++ b/internal/webhook/v1/ingress_webhook_test.go @@ -113,3 +113,48 @@ func TestIngressCustomValidator_NoWarningsWhenReferencesExist(t *testing.T) { require.NoError(t, err) assert.Empty(t, warnings) } + +func csrfIngress(anno map[string]string) *networkingv1.Ingress { + return &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{Name: "test-ingress", Namespace: "default", Annotations: anno}, + Spec: networkingv1.IngressSpec{}, + } +} + +func TestIngressCustomValidator_RejectsEnableCsrfWithoutKey(t *testing.T) { + validator := buildIngressValidator(t) + + // missing csrf-key + _, err := validator.ValidateCreate(context.Background(), csrfIngress(map[string]string{ + "k8s.apisix.apache.org/enable-csrf": "true", + })) + require.Error(t, err) + + // empty csrf-key + _, err = validator.ValidateCreate(context.Background(), csrfIngress(map[string]string{ + "k8s.apisix.apache.org/enable-csrf": "true", + "k8s.apisix.apache.org/csrf-key": "", + })) + require.Error(t, err) + + // the same invalid update must also be rejected, not only create + old := csrfIngress(nil) + _, err = validator.ValidateUpdate(context.Background(), old, csrfIngress(map[string]string{ + "k8s.apisix.apache.org/enable-csrf": "true", + })) + require.Error(t, err) +} + +func TestIngressCustomValidator_AllowsEnableCsrfWithKey(t *testing.T) { + validator := buildIngressValidator(t) + + _, err := validator.ValidateCreate(context.Background(), csrfIngress(map[string]string{ + "k8s.apisix.apache.org/enable-csrf": "true", + "k8s.apisix.apache.org/csrf-key": "my-secret-key", + })) + require.NoError(t, err) + + // csrf not enabled: key absence is fine + _, err = validator.ValidateCreate(context.Background(), csrfIngress(nil)) + require.NoError(t, err) +}