From 66c880796dae89dd3d9e717a2586e716219718b2 Mon Sep 17 00:00:00 2001 From: mohamad aldawamneh Date: Wed, 27 Aug 2025 13:29:39 +0100 Subject: [PATCH 1/3] added some validation annotaion for ingress --- internal/configs/annotations.go | 36 ++++++++++++++++++++++++++++ internal/configs/annotations_test.go | 33 +++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/internal/configs/annotations.go b/internal/configs/annotations.go index 8718dd151e..94072ba985 100644 --- a/internal/configs/annotations.go +++ b/internal/configs/annotations.go @@ -454,6 +454,42 @@ func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool //gocyclo:ignore func parseRateLimitAnnotations(annotations map[string]string, cfgParams *ConfigParams, context apiObject) []error { errors := make([]error, 0) + + rateLimitAnnotations := []string{ + "nginx.org/limit-req-rate", + "nginx.org/limit-req-key", + "nginx.org/limit-req-zone-size", + "nginx.org/limit-req-burst", + "nginx.org/limit-req-delay", + "nginx.org/limit-req-no-delay", + "nginx.org/limit-req-dry-run", + "nginx.org/limit-req-log-level", + "nginx.org/limit-req-reject-code", + "nginx.org/limit-req-scale", + } + + hasRateLimitAnnotation := false + for _, annotation := range rateLimitAnnotations { + if _, exists := annotations[annotation]; exists { + hasRateLimitAnnotation = true + break + } + } + + if hasRateLimitAnnotation { + mandatoryAnnotations := []string{ + "nginx.org/limit-req-rate", + "nginx.org/limit-req-key", + "nginx.org/limit-req-zone-size", + } + + for _, mandatory := range mandatoryAnnotations { + if _, exists := annotations[mandatory]; !exists { + errors = append(errors, fmt.Errorf("ingress %s/%s: rate-limiting configuration requires mandatory annotation %s", context.GetNamespace(), context.GetName(), mandatory)) + } + } + } + if requestRateLimit, exists := annotations["nginx.org/limit-req-rate"]; exists { if rate, err := ParseRequestRate(requestRateLimit); err != nil { errors = append(errors, fmt.Errorf("ingress %s/%s: invalid value for nginx.org/limit-req-rate: got %s: %w", context.GetNamespace(), context.GetName(), requestRateLimit, err)) diff --git a/internal/configs/annotations_test.go b/internal/configs/annotations_test.go index beaa966b28..9e7d112bad 100644 --- a/internal/configs/annotations_test.go +++ b/internal/configs/annotations_test.go @@ -215,6 +215,39 @@ func TestParseRateLimitAnnotations(t *testing.T) { }, NewDefaultConfigParams(context.Background(), false), ctx); len(errors) == 0 { t.Error("No Errors when parsing invalid log level") } + + if errors := parseRateLimitAnnotations(map[string]string{ + "nginx.org/limit-req-rate": "1r/s", + }, NewDefaultConfigParams(context.Background(), false), ctx); len(errors) != 2 { + t.Errorf("Expected 2 errors for missing mandatory annotations, got %d", len(errors)) + } + + if errors := parseRateLimitAnnotations(map[string]string{ + "nginx.org/limit-req-rate": "1r/s", + "nginx.org/limit-req-key": "${binary_remote_addr}", + }, NewDefaultConfigParams(context.Background(), false), ctx); len(errors) != 1 { + t.Errorf("Expected 1 error for missing mandatory annotation, got %d", len(errors)) + } + + if errors := parseRateLimitAnnotations(map[string]string{ + "nginx.org/limit-req-burst": "10", + }, NewDefaultConfigParams(context.Background(), false), ctx); len(errors) != 3 { + t.Errorf("Expected 3 errors for missing all mandatory annotations, got %d", len(errors)) + } + + if errors := parseRateLimitAnnotations(map[string]string{ + "nginx.org/proxy-connect-timeout": "30s", + }, NewDefaultConfigParams(context.Background(), false), ctx); len(errors) != 0 { + t.Errorf("Expected 0 errors for non rate-limiting annotations, got %d", len(errors)) + } + + if errors := parseRateLimitAnnotations(map[string]string{ + "nginx.org/limit-req-rate": "1r/s", + "nginx.org/limit-req-key": "${binary_remote_addr}", + "nginx.org/limit-req-zone-size": "10m", + }, NewDefaultConfigParams(context.Background(), false), ctx); len(errors) != 0 { + t.Errorf("Expected 0 errors for complete mandatory annotations, got %d", len(errors)) + } } func BenchmarkParseRewrites(b *testing.B) { From d12b87150cec6fb232e0e48f8e92a06bbf0558e8 Mon Sep 17 00:00:00 2001 From: mohamad_al Date: Wed, 27 Aug 2025 16:05:21 +0100 Subject: [PATCH 2/3] Update internal/configs/annotations.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: mohamad_al --- internal/configs/annotations.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/configs/annotations.go b/internal/configs/annotations.go index 94072ba985..65efe554d1 100644 --- a/internal/configs/annotations.go +++ b/internal/configs/annotations.go @@ -478,9 +478,9 @@ func parseRateLimitAnnotations(annotations map[string]string, cfgParams *ConfigP if hasRateLimitAnnotation { mandatoryAnnotations := []string{ - "nginx.org/limit-req-rate", - "nginx.org/limit-req-key", - "nginx.org/limit-req-zone-size", + LimitReqRateAnnotation, + LimitReqKeyAnnotation, + LimitReqZoneSizeAnnotation, } for _, mandatory := range mandatoryAnnotations { From 30d0eed930ad4cbf0204d2b203cefb04ed30d95b Mon Sep 17 00:00:00 2001 From: mohamad aldawamneh Date: Wed, 27 Aug 2025 16:14:03 +0100 Subject: [PATCH 3/3] Revert "Update internal/configs/annotations.go" This reverts commit d12b87150cec6fb232e0e48f8e92a06bbf0558e8. --- internal/configs/annotations.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/configs/annotations.go b/internal/configs/annotations.go index 65efe554d1..94072ba985 100644 --- a/internal/configs/annotations.go +++ b/internal/configs/annotations.go @@ -478,9 +478,9 @@ func parseRateLimitAnnotations(annotations map[string]string, cfgParams *ConfigP if hasRateLimitAnnotation { mandatoryAnnotations := []string{ - LimitReqRateAnnotation, - LimitReqKeyAnnotation, - LimitReqZoneSizeAnnotation, + "nginx.org/limit-req-rate", + "nginx.org/limit-req-key", + "nginx.org/limit-req-zone-size", } for _, mandatory := range mandatoryAnnotations {