From ac681311a50526c4b122d4aae729d4559725131c Mon Sep 17 00:00:00 2001 From: Paul Abel Date: Tue, 28 Jan 2025 14:39:42 +0000 Subject: [PATCH 1/4] refactor rate limit policy config struct --- internal/configs/virtualserver.go | 45 ++++---- internal/configs/virtualserver_test.go | 153 +++++++++++++------------ 2 files changed, 106 insertions(+), 92 deletions(-) diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 64a6adbb3a..187f53bfd5 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -454,7 +454,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( var healthChecks []version2.HealthCheck var limitReqZones []version2.LimitReqZone - limitReqZones = append(limitReqZones, policiesCfg.LimitReqZones...) + limitReqZones = append(limitReqZones, policiesCfg.RateLimit.Zones...) // generate upstreams for VirtualServer for _, u := range vsEx.VirtualServer.Spec.Upstreams { @@ -604,7 +604,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( policiesCfg.APIKeyClientMap[apiMapName] = routePoliciesCfg.APIKeyClients } } - limitReqZones = append(limitReqZones, routePoliciesCfg.LimitReqZones...) + limitReqZones = append(limitReqZones, routePoliciesCfg.RateLimit.Zones...) dosRouteCfg := generateDosCfg(dosResources[r.Path]) @@ -745,7 +745,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( } } - limitReqZones = append(limitReqZones, routePoliciesCfg.LimitReqZones...) + limitReqZones = append(limitReqZones, routePoliciesCfg.RateLimit.Zones...) dosRouteCfg := generateDosCfg(dosResources[r.Path]) @@ -861,8 +861,8 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( TLSPassthrough: vsc.isTLSPassthrough, Allow: policiesCfg.Allow, Deny: policiesCfg.Deny, - LimitReqOptions: policiesCfg.LimitReqOptions, - LimitReqs: policiesCfg.LimitReqs, + LimitReqOptions: policiesCfg.RateLimit.Options, + LimitReqs: policiesCfg.RateLimit.Reqs, JWTAuth: policiesCfg.JWTAuth, BasicAuth: policiesCfg.BasicAuth, JWTAuthList: policiesCfg.JWTAuthList, @@ -891,12 +891,17 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( return vsCfg, vsc.warnings } +// RateLimit hold the configuration for the ratelimiting Policy +type RateLimit struct { + Reqs []version2.LimitReq + Zones []version2.LimitReqZone + Options version2.LimitReqOptions +} + type policiesCfg struct { Allow []string Deny []string - LimitReqOptions version2.LimitReqOptions - LimitReqZones []version2.LimitReqZone - LimitReqs []version2.LimitReq + RateLimit RateLimit JWTAuth *version2.JWTAuth JWTAuthList map[string]*version2.JWTAuth JWKSAuthEnabled bool @@ -994,20 +999,20 @@ func (p *policiesCfg) addRateLimitConfig( ) *validationResults { res := newValidationResults() rlZoneName := fmt.Sprintf("pol_rl_%v_%v_%v_%v", polNamespace, polName, vsNamespace, vsName) - p.LimitReqs = append(p.LimitReqs, generateLimitReq(rlZoneName, rateLimit)) - p.LimitReqZones = append(p.LimitReqZones, generateLimitReqZone(rlZoneName, rateLimit, podReplicas)) - if len(p.LimitReqs) == 1 { - p.LimitReqOptions = generateLimitReqOptions(rateLimit) + p.RateLimit.Reqs = append(p.RateLimit.Reqs, generateLimitReq(rlZoneName, rateLimit)) + p.RateLimit.Zones = append(p.RateLimit.Zones, generateLimitReqZone(rlZoneName, rateLimit, podReplicas)) + if len(p.RateLimit.Reqs) == 1 { + p.RateLimit.Options = generateLimitReqOptions(rateLimit) } else { curOptions := generateLimitReqOptions(rateLimit) - if curOptions.DryRun != p.LimitReqOptions.DryRun { - res.addWarningf("RateLimit policy %s with limit request option dryRun='%v' is overridden to dryRun='%v' by the first policy reference in this context", polKey, curOptions.DryRun, p.LimitReqOptions.DryRun) + if curOptions.DryRun != p.RateLimit.Options.DryRun { + res.addWarningf("RateLimit policy %s with limit request option dryRun='%v' is overridden to dryRun='%v' by the first policy reference in this context", polKey, curOptions.DryRun, p.RateLimit.Options.DryRun) } - if curOptions.LogLevel != p.LimitReqOptions.LogLevel { - res.addWarningf("RateLimit policy %s with limit request option logLevel='%v' is overridden to logLevel='%v' by the first policy reference in this context", polKey, curOptions.LogLevel, p.LimitReqOptions.LogLevel) + if curOptions.LogLevel != p.RateLimit.Options.LogLevel { + res.addWarningf("RateLimit policy %s with limit request option logLevel='%v' is overridden to logLevel='%v' by the first policy reference in this context", polKey, curOptions.LogLevel, p.RateLimit.Options.LogLevel) } - if curOptions.RejectCode != p.LimitReqOptions.RejectCode { - res.addWarningf("RateLimit policy %s with limit request option rejectCode='%v' is overridden to rejectCode='%v' by the first policy reference in this context", polKey, curOptions.RejectCode, p.LimitReqOptions.RejectCode) + if curOptions.RejectCode != p.RateLimit.Options.RejectCode { + res.addWarningf("RateLimit policy %s with limit request option rejectCode='%v' is overridden to rejectCode='%v' by the first policy reference in this context", polKey, curOptions.RejectCode, p.RateLimit.Options.RejectCode) } } return res @@ -1655,8 +1660,8 @@ func removeDuplicateLimitReqZones(rlz []version2.LimitReqZone) []version2.LimitR func addPoliciesCfgToLocation(cfg policiesCfg, location *version2.Location) { location.Allow = cfg.Allow location.Deny = cfg.Deny - location.LimitReqOptions = cfg.LimitReqOptions - location.LimitReqs = cfg.LimitReqs + location.LimitReqOptions = cfg.RateLimit.Options + location.LimitReqs = cfg.RateLimit.Reqs location.JWTAuth = cfg.JWTAuth location.BasicAuth = cfg.BasicAuth location.EgressMTLS = cfg.EgressMTLS diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index a5f3c5b005..ee812a0915 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -6587,21 +6587,23 @@ func TestGeneratePolicies(t *testing.T) { }, }, expected: policiesCfg{ - LimitReqZones: []version2.LimitReqZone{ - { - Key: "test", - ZoneSize: "10M", - Rate: "10r/s", - ZoneName: "pol_rl_default_rateLimit-policy_default_test", + RateLimit: RateLimit{ + Reqs: []version2.LimitReq{ + { + ZoneName: "pol_rl_default_rateLimit-policy_default_test", + }, }, - }, - LimitReqOptions: version2.LimitReqOptions{ - LogLevel: "notice", - RejectCode: 503, - }, - LimitReqs: []version2.LimitReq{ - { - ZoneName: "pol_rl_default_rateLimit-policy_default_test", + Zones: []version2.LimitReqZone{ + { + Key: "test", + ZoneSize: "10M", + Rate: "10r/s", + ZoneName: "pol_rl_default_rateLimit-policy_default_test", + }, + }, + Options: version2.LimitReqOptions{ + LogLevel: "notice", + RejectCode: 503, }, }, }, @@ -6639,30 +6641,32 @@ func TestGeneratePolicies(t *testing.T) { }, }, expected: policiesCfg{ - LimitReqZones: []version2.LimitReqZone{ - { - Key: "test", - ZoneSize: "10M", - Rate: "10r/s", - ZoneName: "pol_rl_default_rateLimit-policy_default_test", - }, - { - Key: "test2", - ZoneSize: "20M", - Rate: "20r/s", - ZoneName: "pol_rl_default_rateLimit-policy2_default_test", + RateLimit: RateLimit{ + Zones: []version2.LimitReqZone{ + { + Key: "test", + ZoneSize: "10M", + Rate: "10r/s", + ZoneName: "pol_rl_default_rateLimit-policy_default_test", + }, + { + Key: "test2", + ZoneSize: "20M", + Rate: "20r/s", + ZoneName: "pol_rl_default_rateLimit-policy2_default_test", + }, }, - }, - LimitReqOptions: version2.LimitReqOptions{ - LogLevel: "error", - RejectCode: 503, - }, - LimitReqs: []version2.LimitReq{ - { - ZoneName: "pol_rl_default_rateLimit-policy_default_test", + Options: version2.LimitReqOptions{ + LogLevel: "error", + RejectCode: 503, }, - { - ZoneName: "pol_rl_default_rateLimit-policy2_default_test", + Reqs: []version2.LimitReq{ + { + ZoneName: "pol_rl_default_rateLimit-policy_default_test", + }, + { + ZoneName: "pol_rl_default_rateLimit-policy2_default_test", + }, }, }, }, @@ -6689,26 +6693,29 @@ func TestGeneratePolicies(t *testing.T) { }, }, expected: policiesCfg{ - LimitReqZones: []version2.LimitReqZone{ - { - Key: "test", - ZoneSize: "10M", - Rate: "5r/s", - ZoneName: "pol_rl_default_rateLimitScale-policy_default_test", + RateLimit: RateLimit{ + Zones: []version2.LimitReqZone{ + { + Key: "test", + ZoneSize: "10M", + Rate: "5r/s", + ZoneName: "pol_rl_default_rateLimitScale-policy_default_test", + }, }, - }, - LimitReqOptions: version2.LimitReqOptions{ - LogLevel: "notice", - RejectCode: 503, - }, - LimitReqs: []version2.LimitReq{ - { - ZoneName: "pol_rl_default_rateLimitScale-policy_default_test", + Options: version2.LimitReqOptions{ + LogLevel: "notice", + RejectCode: 503, + }, + Reqs: []version2.LimitReq{ + { + ZoneName: "pol_rl_default_rateLimitScale-policy_default_test", + }, }, }, }, msg: "rate limit reference with scale", }, + { policyRefs: []conf_v1.PolicyReference{ { @@ -7397,30 +7404,32 @@ func TestGeneratePoliciesFails(t *testing.T) { }, policyOpts: policyOptions{}, expected: policiesCfg{ - LimitReqZones: []version2.LimitReqZone{ - { - Key: "test", - ZoneSize: "10M", - Rate: "10r/s", - ZoneName: "pol_rl_default_rateLimit-policy_default_test", - }, - { - Key: "test2", - ZoneSize: "20M", - Rate: "20r/s", - ZoneName: "pol_rl_default_rateLimit-policy2_default_test", + RateLimit: RateLimit{ + Zones: []version2.LimitReqZone{ + { + Key: "test", + ZoneSize: "10M", + Rate: "10r/s", + ZoneName: "pol_rl_default_rateLimit-policy_default_test", + }, + { + Key: "test2", + ZoneSize: "20M", + Rate: "20r/s", + ZoneName: "pol_rl_default_rateLimit-policy2_default_test", + }, }, - }, - LimitReqOptions: version2.LimitReqOptions{ - LogLevel: "error", - RejectCode: 503, - }, - LimitReqs: []version2.LimitReq{ - { - ZoneName: "pol_rl_default_rateLimit-policy_default_test", + Options: version2.LimitReqOptions{ + LogLevel: "error", + RejectCode: 503, }, - { - ZoneName: "pol_rl_default_rateLimit-policy2_default_test", + Reqs: []version2.LimitReq{ + { + ZoneName: "pol_rl_default_rateLimit-policy_default_test", + }, + { + ZoneName: "pol_rl_default_rateLimit-policy2_default_test", + }, }, }, }, From 227c1cc53f1842e03f9b31a8c17b248efeac739e Mon Sep 17 00:00:00 2001 From: Paul Abel Date: Tue, 28 Jan 2025 15:09:11 +0000 Subject: [PATCH 2/4] make ratelimit struct no longer exported --- internal/configs/virtualserver.go | 6 +++--- internal/configs/virtualserver_test.go | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 187f53bfd5..c28835693b 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -891,8 +891,8 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( return vsCfg, vsc.warnings } -// RateLimit hold the configuration for the ratelimiting Policy -type RateLimit struct { +// rateLimit hold the configuration for the ratelimiting Policy +type rateLimit struct { Reqs []version2.LimitReq Zones []version2.LimitReqZone Options version2.LimitReqOptions @@ -901,7 +901,7 @@ type RateLimit struct { type policiesCfg struct { Allow []string Deny []string - RateLimit RateLimit + RateLimit rateLimit JWTAuth *version2.JWTAuth JWTAuthList map[string]*version2.JWTAuth JWKSAuthEnabled bool diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index ee812a0915..041a14691b 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -6587,7 +6587,7 @@ func TestGeneratePolicies(t *testing.T) { }, }, expected: policiesCfg{ - RateLimit: RateLimit{ + RateLimit: rateLimit{ Reqs: []version2.LimitReq{ { ZoneName: "pol_rl_default_rateLimit-policy_default_test", @@ -6641,7 +6641,7 @@ func TestGeneratePolicies(t *testing.T) { }, }, expected: policiesCfg{ - RateLimit: RateLimit{ + RateLimit: rateLimit{ Zones: []version2.LimitReqZone{ { Key: "test", @@ -6693,7 +6693,7 @@ func TestGeneratePolicies(t *testing.T) { }, }, expected: policiesCfg{ - RateLimit: RateLimit{ + RateLimit: rateLimit{ Zones: []version2.LimitReqZone{ { Key: "test", @@ -7404,7 +7404,7 @@ func TestGeneratePoliciesFails(t *testing.T) { }, policyOpts: policyOptions{}, expected: policiesCfg{ - RateLimit: RateLimit{ + RateLimit: rateLimit{ Zones: []version2.LimitReqZone{ { Key: "test", From 51968658f4bf83368ed71150f2783bb83eaba995 Mon Sep 17 00:00:00 2001 From: Paul Abel Date: Tue, 28 Jan 2025 16:29:13 +0000 Subject: [PATCH 3/4] refactor JWT Auth policy config struct --- internal/configs/virtualserver.go | 63 ++++++++++++++------------ internal/configs/virtualserver_test.go | 62 ++++++++++++++----------- 2 files changed, 69 insertions(+), 56 deletions(-) diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index c28835693b..116f86134d 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -423,10 +423,10 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( } policiesCfg := vsc.generatePolicies(ownerDetails, vsEx.VirtualServer.Spec.Policies, vsEx.Policies, specContext, policyOpts) - if policiesCfg.JWKSAuthEnabled { - jwtAuthKey := policiesCfg.JWTAuth.Key - policiesCfg.JWTAuthList = make(map[string]*version2.JWTAuth) - policiesCfg.JWTAuthList[jwtAuthKey] = policiesCfg.JWTAuth + if policiesCfg.JWTAuth.JWKSEnabled { + jwtAuthKey := policiesCfg.JWTAuth.Auth.Key + policiesCfg.JWTAuth.List = make(map[string]*version2.JWTAuth) + policiesCfg.JWTAuth.List[jwtAuthKey] = policiesCfg.JWTAuth.Auth } if policiesCfg.APIKeyEnabled { @@ -582,16 +582,16 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( if policiesCfg.OIDC { routePoliciesCfg.OIDC = policiesCfg.OIDC } - if routePoliciesCfg.JWKSAuthEnabled { - policiesCfg.JWKSAuthEnabled = routePoliciesCfg.JWKSAuthEnabled + if routePoliciesCfg.JWTAuth.JWKSEnabled { + policiesCfg.JWTAuth.JWKSEnabled = routePoliciesCfg.JWTAuth.JWKSEnabled - if policiesCfg.JWTAuthList == nil { - policiesCfg.JWTAuthList = make(map[string]*version2.JWTAuth) + if policiesCfg.JWTAuth.List == nil { + policiesCfg.JWTAuth.List = make(map[string]*version2.JWTAuth) } - jwtAuthKey := routePoliciesCfg.JWTAuth.Key - if _, exists := policiesCfg.JWTAuthList[jwtAuthKey]; !exists { - policiesCfg.JWTAuthList[jwtAuthKey] = routePoliciesCfg.JWTAuth + jwtAuthKey := routePoliciesCfg.JWTAuth.Auth.Key + if _, exists := policiesCfg.JWTAuth.List[jwtAuthKey]; !exists { + policiesCfg.JWTAuth.List[jwtAuthKey] = routePoliciesCfg.JWTAuth.Auth } } if routePoliciesCfg.APIKeyEnabled { @@ -722,16 +722,16 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( if policiesCfg.OIDC { routePoliciesCfg.OIDC = policiesCfg.OIDC } - if routePoliciesCfg.JWKSAuthEnabled { - policiesCfg.JWKSAuthEnabled = routePoliciesCfg.JWKSAuthEnabled + if routePoliciesCfg.JWTAuth.JWKSEnabled { + policiesCfg.JWTAuth.JWKSEnabled = routePoliciesCfg.JWTAuth.JWKSEnabled - if policiesCfg.JWTAuthList == nil { - policiesCfg.JWTAuthList = make(map[string]*version2.JWTAuth) + if policiesCfg.JWTAuth.List == nil { + policiesCfg.JWTAuth.List = make(map[string]*version2.JWTAuth) } - jwtAuthKey := routePoliciesCfg.JWTAuth.Key - if _, exists := policiesCfg.JWTAuthList[jwtAuthKey]; !exists { - policiesCfg.JWTAuthList[jwtAuthKey] = routePoliciesCfg.JWTAuth + jwtAuthKey := routePoliciesCfg.JWTAuth.Auth.Key + if _, exists := policiesCfg.JWTAuth.List[jwtAuthKey]; !exists { + policiesCfg.JWTAuth.List[jwtAuthKey] = routePoliciesCfg.JWTAuth.Auth } } if routePoliciesCfg.APIKeyEnabled { @@ -863,10 +863,10 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( Deny: policiesCfg.Deny, LimitReqOptions: policiesCfg.RateLimit.Options, LimitReqs: policiesCfg.RateLimit.Reqs, - JWTAuth: policiesCfg.JWTAuth, + JWTAuth: policiesCfg.JWTAuth.Auth, BasicAuth: policiesCfg.BasicAuth, - JWTAuthList: policiesCfg.JWTAuthList, - JWKSAuthEnabled: policiesCfg.JWKSAuthEnabled, + JWTAuthList: policiesCfg.JWTAuth.List, + JWKSAuthEnabled: policiesCfg.JWTAuth.JWKSEnabled, IngressMTLS: policiesCfg.IngressMTLS, EgressMTLS: policiesCfg.EgressMTLS, APIKey: policiesCfg.APIKey, @@ -898,13 +898,18 @@ type rateLimit struct { Options version2.LimitReqOptions } +// jwtAuth hold the configuration for the JWTAuth & JWKSAuth Policies +type jwtAuth struct { + Auth *version2.JWTAuth + List map[string]*version2.JWTAuth + JWKSEnabled bool +} + type policiesCfg struct { Allow []string Deny []string RateLimit rateLimit - JWTAuth *version2.JWTAuth - JWTAuthList map[string]*version2.JWTAuth - JWKSAuthEnabled bool + JWTAuth jwtAuth BasicAuth *version2.BasicAuth IngressMTLS *version2.IngressMTLS EgressMTLS *version2.EgressMTLS @@ -1060,7 +1065,7 @@ func (p *policiesCfg) addJWTAuthConfig( secretRefs map[string]*secrets.SecretReference, ) *validationResults { res := newValidationResults() - if p.JWTAuth != nil { + if p.JWTAuth.Auth != nil { res.addWarningf("Multiple jwt policies in the same context is not valid. JWT policy %s will be ignored", polKey) return res } @@ -1081,7 +1086,7 @@ func (p *policiesCfg) addJWTAuthConfig( return res } - p.JWTAuth = &version2.JWTAuth{ + p.JWTAuth.Auth = &version2.JWTAuth{ Secret: secretRef.Path, Realm: jwtAuth.Realm, Token: jwtAuth.Token, @@ -1097,14 +1102,14 @@ func (p *policiesCfg) addJWTAuthConfig( JwksPath: uri.Path, } - p.JWTAuth = &version2.JWTAuth{ + p.JWTAuth.Auth = &version2.JWTAuth{ Key: polKey, JwksURI: *JwksURI, Realm: jwtAuth.Realm, Token: jwtAuth.Token, KeyCache: jwtAuth.KeyCache, } - p.JWKSAuthEnabled = true + p.JWTAuth.JWKSEnabled = true return res } return res @@ -1662,7 +1667,7 @@ func addPoliciesCfgToLocation(cfg policiesCfg, location *version2.Location) { location.Deny = cfg.Deny location.LimitReqOptions = cfg.RateLimit.Options location.LimitReqs = cfg.RateLimit.Reqs - location.JWTAuth = cfg.JWTAuth + location.JWTAuth = cfg.JWTAuth.Auth location.BasicAuth = cfg.BasicAuth location.EgressMTLS = cfg.EgressMTLS location.OIDC = cfg.OIDC diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index 041a14691b..f63c932746 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -6738,11 +6738,13 @@ func TestGeneratePolicies(t *testing.T) { }, }, expected: policiesCfg{ - JWTAuth: &version2.JWTAuth{ - Secret: "/etc/nginx/secrets/default-jwt-secret", - Realm: "My Test API", + JWTAuth: jwtAuth{ + Auth: &version2.JWTAuth{ + Secret: "/etc/nginx/secrets/default-jwt-secret", + Realm: "My Test API", + }, + JWKSEnabled: false, }, - JWKSAuthEnabled: false, }, msg: "jwt reference", }, @@ -6769,18 +6771,20 @@ func TestGeneratePolicies(t *testing.T) { }, }, expected: policiesCfg{ - JWTAuth: &version2.JWTAuth{ - Key: "default/jwt-policy-2", - Realm: "My Test API", - JwksURI: version2.JwksURI{ - JwksScheme: "https", - JwksHost: "idp.example.com", - JwksPort: "443", - JwksPath: "/keys", + JWTAuth: jwtAuth{ + Auth: &version2.JWTAuth{ + Key: "default/jwt-policy-2", + Realm: "My Test API", + JwksURI: version2.JwksURI{ + JwksScheme: "https", + JwksHost: "idp.example.com", + JwksPort: "443", + JwksPath: "/keys", + }, + KeyCache: "1h", }, - KeyCache: "1h", + JWKSEnabled: true, }, - JWKSAuthEnabled: true, }, msg: "Basic jwks example", }, @@ -6807,18 +6811,20 @@ func TestGeneratePolicies(t *testing.T) { }, }, expected: policiesCfg{ - JWTAuth: &version2.JWTAuth{ - Key: "default/jwt-policy-2", - Realm: "My Test API", - JwksURI: version2.JwksURI{ - JwksScheme: "https", - JwksHost: "idp.example.com", - JwksPort: "", - JwksPath: "/keys", + JWTAuth: jwtAuth{ + Auth: &version2.JWTAuth{ + Key: "default/jwt-policy-2", + Realm: "My Test API", + JwksURI: version2.JwksURI{ + JwksScheme: "https", + JwksHost: "idp.example.com", + JwksPort: "", + JwksPath: "/keys", + }, + KeyCache: "1h", }, - KeyCache: "1h", + JWKSEnabled: true, }, - JWKSAuthEnabled: true, }, msg: "Basic jwks example, no port in JwksURI", }, @@ -7584,9 +7590,11 @@ func TestGeneratePoliciesFails(t *testing.T) { }, }, expected: policiesCfg{ - JWTAuth: &version2.JWTAuth{ - Secret: "/etc/nginx/secrets/default-jwt-secret", - Realm: "test", + JWTAuth: jwtAuth{ + Auth: &version2.JWTAuth{ + Secret: "/etc/nginx/secrets/default-jwt-secret", + Realm: "test", + }, }, }, expectedWarnings: Warnings{ From 3fc3337b7bb26a194443cc9aff44acda40114cc0 Mon Sep 17 00:00:00 2001 From: Paul Abel Date: Tue, 28 Jan 2025 17:00:03 +0000 Subject: [PATCH 4/4] refactor APIKey policy config struct --- internal/configs/virtualserver.go | 65 ++++++++++++++------------ internal/configs/virtualserver_test.go | 48 ++++++++++--------- 2 files changed, 61 insertions(+), 52 deletions(-) diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 116f86134d..8535467b87 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -429,10 +429,10 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( policiesCfg.JWTAuth.List[jwtAuthKey] = policiesCfg.JWTAuth.Auth } - if policiesCfg.APIKeyEnabled { - apiMapName := policiesCfg.APIKey.MapName - policiesCfg.APIKeyClientMap = make(map[string][]apiKeyClient) - policiesCfg.APIKeyClientMap[apiMapName] = policiesCfg.APIKeyClients + if policiesCfg.APIKey.Enabled { + apiMapName := policiesCfg.APIKey.Key.MapName + policiesCfg.APIKey.ClientMap = make(map[string][]apiKeyClient) + policiesCfg.APIKey.ClientMap[apiMapName] = policiesCfg.APIKey.Clients } dosCfg := generateDosCfg(dosResources[""]) @@ -594,14 +594,14 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( policiesCfg.JWTAuth.List[jwtAuthKey] = routePoliciesCfg.JWTAuth.Auth } } - if routePoliciesCfg.APIKeyEnabled { - policiesCfg.APIKeyEnabled = routePoliciesCfg.APIKeyEnabled - apiMapName := routePoliciesCfg.APIKey.MapName - if policiesCfg.APIKeyClientMap == nil { - policiesCfg.APIKeyClientMap = make(map[string][]apiKeyClient) + if routePoliciesCfg.APIKey.Enabled { + policiesCfg.APIKey.Enabled = routePoliciesCfg.APIKey.Enabled + apiMapName := routePoliciesCfg.APIKey.Key.MapName + if policiesCfg.APIKey.ClientMap == nil { + policiesCfg.APIKey.ClientMap = make(map[string][]apiKeyClient) } - if _, exists := policiesCfg.APIKeyClientMap[apiMapName]; !exists { - policiesCfg.APIKeyClientMap[apiMapName] = routePoliciesCfg.APIKeyClients + if _, exists := policiesCfg.APIKey.ClientMap[apiMapName]; !exists { + policiesCfg.APIKey.ClientMap[apiMapName] = routePoliciesCfg.APIKey.Clients } } limitReqZones = append(limitReqZones, routePoliciesCfg.RateLimit.Zones...) @@ -734,14 +734,14 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( policiesCfg.JWTAuth.List[jwtAuthKey] = routePoliciesCfg.JWTAuth.Auth } } - if routePoliciesCfg.APIKeyEnabled { - policiesCfg.APIKeyEnabled = routePoliciesCfg.APIKeyEnabled - apiMapName := routePoliciesCfg.APIKey.MapName - if policiesCfg.APIKeyClientMap == nil { - policiesCfg.APIKeyClientMap = make(map[string][]apiKeyClient) + if routePoliciesCfg.APIKey.Enabled { + policiesCfg.APIKey.Enabled = routePoliciesCfg.APIKey.Enabled + apiMapName := routePoliciesCfg.APIKey.Key.MapName + if policiesCfg.APIKey.ClientMap == nil { + policiesCfg.APIKey.ClientMap = make(map[string][]apiKeyClient) } - if _, exists := policiesCfg.APIKeyClientMap[apiMapName]; !exists { - policiesCfg.APIKeyClientMap[apiMapName] = routePoliciesCfg.APIKeyClients + if _, exists := policiesCfg.APIKey.ClientMap[apiMapName]; !exists { + policiesCfg.APIKey.ClientMap[apiMapName] = routePoliciesCfg.APIKey.Clients } } @@ -812,7 +812,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( } } - for mapName, apiKeyClients := range policiesCfg.APIKeyClientMap { + for mapName, apiKeyClients := range policiesCfg.APIKey.ClientMap { maps = append(maps, *generateAPIKeyClientMap(mapName, apiKeyClients)) } @@ -869,8 +869,8 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( JWKSAuthEnabled: policiesCfg.JWTAuth.JWKSEnabled, IngressMTLS: policiesCfg.IngressMTLS, EgressMTLS: policiesCfg.EgressMTLS, - APIKey: policiesCfg.APIKey, - APIKeyEnabled: policiesCfg.APIKeyEnabled, + APIKey: policiesCfg.APIKey.Key, + APIKeyEnabled: policiesCfg.APIKey.Enabled, OIDC: vsc.oidcPolCfg.oidc, WAF: policiesCfg.WAF, Dos: dosCfg, @@ -905,6 +905,14 @@ type jwtAuth struct { JWKSEnabled bool } +// apiKeyAuth hold the configuration for the APIKey Policy +type apiKeyAuth struct { + Enabled bool + Key *version2.APIKey + Clients []apiKeyClient + ClientMap map[string][]apiKeyClient +} + type policiesCfg struct { Allow []string Deny []string @@ -914,10 +922,7 @@ type policiesCfg struct { IngressMTLS *version2.IngressMTLS EgressMTLS *version2.EgressMTLS OIDC bool - APIKeyEnabled bool - APIKey *version2.APIKey - APIKeyClients []apiKeyClient - APIKeyClientMap map[string][]apiKeyClient + APIKey apiKeyAuth WAF *version2.WAF ErrorReturn *version2.Return BundleValidator bundleValidator @@ -1369,7 +1374,7 @@ func (p *policiesCfg) addAPIKeyConfig( secretRefs map[string]*secrets.SecretReference, ) *validationResults { res := newValidationResults() - if p.APIKey != nil { + if p.APIKey.Key != nil { res.addWarningf( "Multiple API Key policies in the same context is not valid. API Key policy %s will be ignored", polKey, @@ -1394,7 +1399,7 @@ func (p *policiesCfg) addAPIKeyConfig( return res } - p.APIKeyClients = generateAPIKeyClients(secretRef.Secret.Data) + p.APIKey.Clients = generateAPIKeyClients(secretRef.Secret.Data) mapName := fmt.Sprintf( "apikey_auth_client_name_%s_%s_%s", @@ -1402,12 +1407,12 @@ func (p *policiesCfg) addAPIKeyConfig( rfc1123ToSnake(vsName), strings.Split(rfc1123ToSnake(polKey), "/")[1], ) - p.APIKey = &version2.APIKey{ + p.APIKey.Key = &version2.APIKey{ Header: apiKey.SuppliedIn.Header, Query: apiKey.SuppliedIn.Query, MapName: mapName, } - p.APIKeyEnabled = true + p.APIKey.Enabled = true return res } @@ -1672,7 +1677,7 @@ func addPoliciesCfgToLocation(cfg policiesCfg, location *version2.Location) { location.EgressMTLS = cfg.EgressMTLS location.OIDC = cfg.OIDC location.WAF = cfg.WAF - location.APIKey = cfg.APIKey + location.APIKey = cfg.APIKey.Key location.PoliciesErrorReturn = cfg.ErrorReturn } diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index f63c932746..93af610053 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -7085,17 +7085,19 @@ func TestGeneratePolicies(t *testing.T) { }, }, expected: policiesCfg{ - APIKey: &version2.APIKey{ - Header: []string{"X-API-Key"}, - Query: []string{"api-key"}, - MapName: "apikey_auth_client_name_default_test_api_key_policy", - }, - APIKeyEnabled: true, - APIKeyClientMap: nil, - APIKeyClients: []apiKeyClient{ - { - ClientID: "client1", - HashedKey: "5e884898da28047151d0e56f8dc6292773603d0d6aabbdd62a11ef721d1542d8", + APIKey: apiKeyAuth{ + Key: &version2.APIKey{ + Header: []string{"X-API-Key"}, + Query: []string{"api-key"}, + MapName: "apikey_auth_client_name_default_test_api_key_policy", + }, + Enabled: true, + ClientMap: nil, + Clients: []apiKeyClient{ + { + ClientID: "client1", + HashedKey: "5e884898da28047151d0e56f8dc6292773603d0d6aabbdd62a11ef721d1542d8", + }, }, }, }, @@ -7126,17 +7128,19 @@ func TestGeneratePolicies(t *testing.T) { }, }, expected: policiesCfg{ - APIKey: &version2.APIKey{ - Header: []string{"X-API-Key"}, - Query: []string{"api-key"}, - MapName: "apikey_auth_client_name_default_test_api_key_policy", - }, - APIKeyEnabled: true, - APIKeyClientMap: nil, - APIKeyClients: []apiKeyClient{ - { - ClientID: "client1", - HashedKey: "5e884898da28047151d0e56f8dc6292773603d0d6aabbdd62a11ef721d1542d8", + APIKey: apiKeyAuth{ + Key: &version2.APIKey{ + Header: []string{"X-API-Key"}, + Query: []string{"api-key"}, + MapName: "apikey_auth_client_name_default_test_api_key_policy", + }, + Enabled: true, + ClientMap: nil, + Clients: []apiKeyClient{ + { + ClientID: "client1", + HashedKey: "5e884898da28047151d0e56f8dc6292773603d0d6aabbdd62a11ef721d1542d8", + }, }, }, },