This is an automated email from the ASF dual-hosted git repository.
shreemaan-abhishek pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix-ingress-controller.git
The following commit(s) were added to refs/heads/master by this push:
new dc8db3d7 fix: fail loud when enable-csrf is set but csrf-key
annotation is missing (#2813)
dc8db3d7 is described below
commit dc8db3d714d098fde6cd5eec986287b3c9fd1fad
Author: Shreemaan Abhishek <[email protected]>
AuthorDate: Mon Aug 3 10:38:11 2026 +0800
fix: fail loud when enable-csrf is set but csrf-key annotation is missing
(#2813)
---
.../adc/translator/annotations/plugins/csrf.go | 8 +++-
.../translator/annotations/plugins/csrf_test.go | 10 ++++-
internal/webhook/v1/ingress_webhook.go | 23 +++++++++++
internal/webhook/v1/ingress_webhook_test.go | 45 ++++++++++++++++++++++
4 files changed, 83 insertions(+), 3 deletions(-)
diff --git a/internal/adc/translator/annotations/plugins/csrf.go
b/internal/adc/translator/annotations/plugins/csrf.go
index 1bee407c..811f4753 100644
--- a/internal/adc/translator/annotations/plugins/csrf.go
+++ b/internal/adc/translator/annotations/plugins/csrf.go
@@ -16,6 +16,8 @@
package plugins
import (
+ "fmt"
+
adctypes "github.com/apache/apisix-ingress-controller/api/adc"
"github.com/apache/apisix-ingress-controller/internal/adc/translator/annotations"
)
@@ -39,7 +41,11 @@ func (c *csrf) Handle(e annotations.Extractor) (any, error) {
key := e.GetStringAnnotation(annotations.AnnotationsCsrfKey)
if key == "" {
- return nil, nil
+ // csrf requested without a key: the webhook rejects this, but
log a
+ // breadcrumb for the cases it cannot cover (webhook off,
pre-upgrade
+ // Ingress). Parse skips only this plugin, the rest of the
route stands.
+ return nil, fmt.Errorf("annotation %q is enabled but %q is
missing or empty",
+ annotations.AnnotationsEnableCsrf,
annotations.AnnotationsCsrfKey)
}
return &adctypes.CSRFConfig{
diff --git a/internal/adc/translator/annotations/plugins/csrf_test.go
b/internal/adc/translator/annotations/plugins/csrf_test.go
index 0f681703..e41f8cf5 100644
--- a/internal/adc/translator/annotations/plugins/csrf_test.go
+++ b/internal/adc/translator/annotations/plugins/csrf_test.go
@@ -43,10 +43,16 @@ func TestCSRFHandler(t *testing.T) {
assert.Nil(t, err, "checking given error")
assert.Nil(t, out, "checking given output")
- // Test with enable-csrf true but no key
+ // Test with enable-csrf true but no key: errors so Parse logs a
breadcrumb
anno[annotations.AnnotationsEnableCsrf] = "true"
delete(anno, annotations.AnnotationsCsrfKey)
out, err = p.Handle(annotations.NewExtractor(anno))
- assert.Nil(t, err, "checking given error")
+ assert.Error(t, err, "expecting an error when csrf-key is missing")
assert.Nil(t, out, "checking given output when key is missing")
+
+ // Test with enable-csrf true but empty key: same behavior
+ anno[annotations.AnnotationsCsrfKey] = ""
+ out, err = p.Handle(annotations.NewExtractor(anno))
+ assert.Error(t, err, "expecting an error when csrf-key is empty")
+ assert.Nil(t, out, "checking given output when key is empty")
}
diff --git a/internal/webhook/v1/ingress_webhook.go
b/internal/webhook/v1/ingress_webhook.go
index 3d1a46ad..d6d3e4f5 100644
--- a/internal/webhook/v1/ingress_webhook.go
+++ b/internal/webhook/v1/ingress_webhook.go
@@ -27,6 +27,7 @@ import (
logf "sigs.k8s.io/controller-runtime/pkg/log"
"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"
@@ -75,6 +76,10 @@ func (v *IngressCustomValidator) ValidateCreate(ctx
context.Context, obj runtime
return nil, nil
}
+ if err := validateAnnotations(ingress); err != nil {
+ return nil, err
+ }
+
detector := sslvalidator.NewConflictDetector(v.Client)
conflicts := detector.DetectConflicts(ctx, ingress)
if len(conflicts) > 0 {
@@ -96,6 +101,10 @@ func (v *IngressCustomValidator) ValidateUpdate(ctx
context.Context, oldObj, new
return nil, nil
}
+ if err := validateAnnotations(ingress); err != nil {
+ return nil, err
+ }
+
detector := sslvalidator.NewConflictDetector(v.Client)
conflicts := detector.DetectConflicts(ctx, ingress)
if len(conflicts) > 0 {
@@ -106,6 +115,20 @@ func (v *IngressCustomValidator) ValidateUpdate(ctx
context.Context, oldObj, new
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)
+}
