From d2f0a194cd2c9499d3f7c45a620eaddf69b65dda Mon Sep 17 00:00:00 2001 From: dbaumgarten Date: Tue, 11 Jun 2024 17:28:32 +0200 Subject: [PATCH] Fix ratelimit-scaling for mergable ingresses (#5728) --- internal/configs/ingress.go | 38 +++++++------- internal/configs/ingress_test.go | 89 ++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 18 deletions(-) diff --git a/internal/configs/ingress.go b/internal/configs/ingress.go index 4b41c32a7b..53ac2636fd 100644 --- a/internal/configs/ingress.go +++ b/internal/configs/ingress.go @@ -640,15 +640,16 @@ func generateNginxCfgForMergeableIngresses(p NginxCfgParams) (version1.IngressNg isMinion := false masterNginxCfg, warnings := generateNginxCfg(NginxCfgParams{ - staticParams: p.staticParams, - ingEx: p.mergeableIngs.Master, - apResources: p.apResources, - dosResource: p.dosResource, - isMinion: isMinion, - isPlus: p.isPlus, - baseCfgParams: p.baseCfgParams, - isResolverConfigured: p.isResolverConfigured, - isWildcardEnabled: p.isWildcardEnabled, + staticParams: p.staticParams, + ingEx: p.mergeableIngs.Master, + apResources: p.apResources, + dosResource: p.dosResource, + isMinion: isMinion, + isPlus: p.isPlus, + baseCfgParams: p.baseCfgParams, + isResolverConfigured: p.isResolverConfigured, + isWildcardEnabled: p.isWildcardEnabled, + ingressControllerReplicas: p.ingressControllerReplicas, }) // because p.mergeableIngs.Master.Ingress is a deepcopy of the original master @@ -690,15 +691,16 @@ func generateNginxCfgForMergeableIngresses(p NginxCfgParams) (version1.IngressNg dummyApResources := &AppProtectResources{} dummyDosResource := &appProtectDosResource{} nginxCfg, minionWarnings := generateNginxCfg(NginxCfgParams{ - staticParams: p.staticParams, - ingEx: minion, - apResources: dummyApResources, - dosResource: dummyDosResource, - isMinion: isMinion, - isPlus: p.isPlus, - baseCfgParams: p.baseCfgParams, - isResolverConfigured: p.isResolverConfigured, - isWildcardEnabled: p.isWildcardEnabled, + staticParams: p.staticParams, + ingEx: minion, + apResources: dummyApResources, + dosResource: dummyDosResource, + isMinion: isMinion, + isPlus: p.isPlus, + baseCfgParams: p.baseCfgParams, + isResolverConfigured: p.isResolverConfigured, + isWildcardEnabled: p.isWildcardEnabled, + ingressControllerReplicas: p.ingressControllerReplicas, }) warnings.Add(minionWarnings) diff --git a/internal/configs/ingress_test.go b/internal/configs/ingress_test.go index 8c7d1ee4fd..c222116a94 100644 --- a/internal/configs/ingress_test.go +++ b/internal/configs/ingress_test.go @@ -1308,6 +1308,95 @@ func TestGenerateNginxCfgForLimitReqWithScaling(t *testing.T) { } } +func TestGenerateNginxCfgForMergeableIngressesForLimitReqWithScaling(t *testing.T) { + t.Parallel() + mergeableIngresses := createMergeableCafeIngress() + + mergeableIngresses.Minions[0].Ingress.Annotations["nginx.org/limit-req-rate"] = "200r/s" + mergeableIngresses.Minions[0].Ingress.Annotations["nginx.org/limit-req-key"] = "${request_uri}" + mergeableIngresses.Minions[0].Ingress.Annotations["nginx.org/limit-req-burst"] = "100" + mergeableIngresses.Minions[0].Ingress.Annotations["nginx.org/limit-req-delay"] = "80" + mergeableIngresses.Minions[0].Ingress.Annotations["nginx.org/limit-req-no-delay"] = "true" + mergeableIngresses.Minions[0].Ingress.Annotations["nginx.org/limit-req-reject-code"] = "429" + mergeableIngresses.Minions[0].Ingress.Annotations["nginx.org/limit-req-zone-size"] = "11m" + mergeableIngresses.Minions[0].Ingress.Annotations["nginx.org/limit-req-dry-run"] = "true" + mergeableIngresses.Minions[0].Ingress.Annotations["nginx.org/limit-req-log-level"] = "info" + mergeableIngresses.Minions[0].Ingress.Annotations["nginx.org/limit-req-scale"] = "true" + + mergeableIngresses.Minions[1].Ingress.Annotations["nginx.org/limit-req-rate"] = "400r/s" + mergeableIngresses.Minions[1].Ingress.Annotations["nginx.org/limit-req-burst"] = "200" + mergeableIngresses.Minions[1].Ingress.Annotations["nginx.org/limit-req-delay"] = "160" + mergeableIngresses.Minions[1].Ingress.Annotations["nginx.org/limit-req-reject-code"] = "503" + mergeableIngresses.Minions[1].Ingress.Annotations["nginx.org/limit-req-zone-size"] = "12m" + mergeableIngresses.Minions[1].Ingress.Annotations["nginx.org/limit-req-scale"] = "true" + + expectedZones := []version1.LimitReqZone{ + { + Name: "default/cafe-ingress-coffee-minion", + Key: "${request_uri}", + Size: "11m", + Rate: "100r/s", + }, + { + Name: "default/cafe-ingress-tea-minion", + Key: "${binary_remote_addr}", + Size: "12m", + Rate: "200r/s", + }, + } + + expectedReqs := map[string]*version1.LimitReq{ + "cafe-ingress-coffee-minion": { + Zone: "default/cafe-ingress-coffee-minion", + Burst: 100, + Delay: 80, + LogLevel: "info", + RejectCode: 429, + NoDelay: true, + DryRun: true, + }, + "cafe-ingress-tea-minion": { + Zone: "default/cafe-ingress-tea-minion", + Burst: 200, + Delay: 160, + LogLevel: "error", + RejectCode: 503, + }, + } + + isPlus := false + + configParams := NewDefaultConfigParams(isPlus) + + result, warnings := generateNginxCfgForMergeableIngresses(NginxCfgParams{ + mergeableIngs: mergeableIngresses, + baseCfgParams: configParams, + isPlus: isPlus, + staticParams: &StaticConfigParams{}, + ingressControllerReplicas: 2, + }) + + if !reflect.DeepEqual(result.LimitReqZones, expectedZones) { + t.Errorf("generateNginxCfg returned \n%v, but expected \n%v", result.LimitReqZones, expectedZones) + } + + for _, server := range result.Servers { + for _, location := range server.Locations { + expectedLimitReq := expectedReqs[location.MinionIngress.Name] + if !reflect.DeepEqual(location.LimitReq, expectedLimitReq) { + t.Errorf("generateNginxCfg returned \n%v, but expected \n%v", location.LimitReq, expectedLimitReq) + } + } + } + + if !reflect.DeepEqual(result.LimitReqZones, expectedZones) { + t.Errorf("generateNginxCfg returned \n%v, but expected \n%v", result.LimitReqZones, expectedZones) + } + if len(warnings) != 0 { + t.Errorf("generateNginxCfg returned warnings: %v", warnings) + } +} + func createMergeableCafeIngress() *MergeableIngresses { master := networking.Ingress{ ObjectMeta: meta_v1.ObjectMeta{