From f4a1ae5a88f8988854b6f39fe040120219ba5a11 Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 2 Jul 2025 11:57:01 +0100 Subject: [PATCH 01/30] initial cache-policy commit --- config/crd/bases/k8s.nginx.org_policies.yaml | 46 ++++++++++ internal/configs/version2/http.go | 21 +++++ .../version2/nginx-plus.virtualserver.tmpl | 84 +++++++++++++++++++ .../configs/version2/nginx.virtualserver.tmpl | 40 +++++++++ internal/configs/virtualserver.go | 75 +++++++++++++++++ pkg/apis/configuration/v1/types.go | 12 +++ pkg/apis/configuration/validation/policy.go | 66 ++++++++++++++- 7 files changed, 343 insertions(+), 1 deletion(-) diff --git a/config/crd/bases/k8s.nginx.org_policies.yaml b/config/crd/bases/k8s.nginx.org_policies.yaml index d5a51d49e3..647f656c9a 100644 --- a/config/crd/bases/k8s.nginx.org_policies.yaml +++ b/config/crd/bases/k8s.nginx.org_policies.yaml @@ -282,6 +282,52 @@ spec: type: object type: array type: object + cache: + description: Cache defines a cache policy for proxy caching. + properties: + allowedCodes: + description: AllowedCodes defines which response codes should be cached. Can be HTTP status codes (100-599) or the string "any" to cache all responses. + items: + x-kubernetes-int-or-string: true + type: array + allowedMethods: + description: "AllowedMethods defines which HTTP methods should be cached. Only GET, HEAD, and POST are supported by NGINX proxy_cache_methods directive. GET and HEAD are always cached by default." + items: + type: string + enum: + - "GET" + - "HEAD" + - "POST" + type: array + cachePurgeAllow: + description: CachePurgeAllow defines IP addresses allowed to purge cache (NGINX Plus only) + items: + type: string + type: array + cacheZoneName: + description: CacheZoneName defines the name of the cache zone + type: string + cacheZoneSize: + description: CacheZoneSize defines the size of the cache zone + type: string + pattern: '^[0-9]+[kmg]$' + time: + description: Time defines the default cache time (required when allowedCodes is specified) + type: string + pattern: '^[0-9]+[smhd]$' + required: + - cacheZoneName + - cacheZoneSize + anyOf: + - not: + required: + - allowedCodes + - allOf: + - required: + - allowedCodes + - required: + - time + type: object type: object status: description: PolicyStatus is the status of the policy resource diff --git a/internal/configs/version2/http.go b/internal/configs/version2/http.go index 3f27badf1c..826e40fb87 100644 --- a/internal/configs/version2/http.go +++ b/internal/configs/version2/http.go @@ -22,6 +22,7 @@ type VirtualServerConfig struct { LimitReqZones []LimitReqZone Maps []Map AuthJWTClaimSets []AuthJWTClaimSet + CacheZones []CacheZone Server Server SpiffeCerts bool SpiffeClientCerts bool @@ -102,6 +103,7 @@ type Server struct { APIKeyEnabled bool WAF *WAF Dos *Dos + Cache *Cache PoliciesErrorReturn *Return VSNamespace string VSName string @@ -228,6 +230,7 @@ type Location struct { WAF *WAF Dos *Dos PoliciesErrorReturn *Return + Cache *Cache ServiceName string IsVSR bool VSRName string @@ -478,3 +481,21 @@ type Variable struct { Name string Value string } + +// CacheZone defines a proxy cache zone configuration. +type CacheZone struct { + Name string + Size string + Path string +} + +// Cache defines cache configuration for locations. +type Cache struct { + ZoneName string + ZoneSize string + Enable bool + Time string + Valid map[string]string // map for codes to time + AllowedMethods []string // HTTP methods allowed for caching based on proxy_cache_methods + CachePurgeAllow []string // IPs/CIDRs allowed to purge cache +} diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index dd08f53014..8e7762cf44 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -70,6 +70,10 @@ map {{ $m.Source }} {{ $m.Variable }} { limit_req_zone {{ $z.Key }} zone={{ $z.ZoneName }}:{{ $z.ZoneSize }} rate={{ $z.Rate }}{{- if $z.Sync }} sync{{- end }}; {{- end }} +{{- range $c := .CacheZones }} +proxy_cache_path {{ $c.Path }} keys_zone={{ $c.Name }}:{{ $c.Size }}; +{{- end }} + {{- range $m := .StatusMatches }} match {{ $m.Name }} { status {{ $m.Code }}; @@ -78,6 +82,44 @@ match {{ $m.Name }} { {{- $s := .Server }} +{{- /* Generate cache purge configuration if any cache has purge restrictions */ -}} +{{- $hasCachePurge := false }} +{{- /* Check server-level cache purge restrictions */ -}} +{{- if and $s.Cache $s.Cache.Enable (gt (len $s.Cache.CachePurgeAllow) 0) }} +{{- $hasCachePurge = true }} +{{- end }} +{{- /* Check location-level cache purge restrictions */ -}} +{{- range $l := $s.Locations }} +{{- if and $l.Cache $l.Cache.Enable (gt (len $l.Cache.CachePurgeAllow) 0) }} +{{- $hasCachePurge = true }} +{{- end }} +{{- end }} + +{{- if $hasCachePurge }} +geo $purge_allowed { + default 0; +{{- /* Add server-level cache purge IPs */ -}} +{{- if and $s.Cache $s.Cache.Enable (gt (len $s.Cache.CachePurgeAllow) 0) }} +{{- range $ip := $s.Cache.CachePurgeAllow }} + {{ $ip }} 1; +{{- end }} +{{- end }} +{{- /* Add location-level cache purge IPs */ -}} +{{- range $l := $s.Locations }} +{{- if and $l.Cache $l.Cache.Enable (gt (len $l.Cache.CachePurgeAllow) 0) }} +{{- range $ip := $l.Cache.CachePurgeAllow }} + {{ $ip }} 1; +{{- end }} +{{- end }} +{{- end }} +} + +map $request_method $cache_purge { + PURGE $purge_allowed; + default 0; +} +{{- end }} + {{- with $s.JWKSAuthEnabled }} proxy_cache_path /var/cache/nginx/jwks_uri_{{$s.VSName}} levels=1 keys_zone=jwks_uri_{{$s.VSName}}:1m max_size=10m; {{- end }} @@ -187,6 +229,27 @@ server { return {{ .Code }}; {{- end }} + {{- with $s.Cache }} + {{- if $s.Cache.Enable }} + # Server-level cache configuration + proxy_cache {{ $s.Cache.ZoneName }}; + proxy_cache_key $scheme$proxy_host$request_uri; + proxy_ignore_headers Cache-Control Expires; + {{- if and $s.Cache.Time (eq (len $s.Cache.Valid) 0) }} + proxy_cache_valid {{ $s.Cache.Time }}; + {{- end }} + {{- range $code, $time := $s.Cache.Valid }} + proxy_cache_valid {{ $code }} {{ $time }}; + {{- end }} + {{- if $s.Cache.AllowedMethods }} + proxy_cache_methods{{ range $s.Cache.AllowedMethods }} {{ . }}{{ end }}; + {{- end }} + {{- if gt (len $s.Cache.CachePurgeAllow) 0 }} + proxy_cache_purge $cache_purge; + {{- end }} + {{- end }} + {{- end }} + {{- range $allow := $s.Allow }} allow {{ $allow }}; {{- end }} @@ -664,6 +727,27 @@ server { {{ $proxyOrGRPC }}_ssl_verify_depth 25; {{ $proxyOrGRPC }}_ssl_name {{ $l.ProxySSLName }}; {{- end }} + + {{- with $l.Cache }} + {{- if $l.Cache.Enable }} + proxy_cache {{ $l.Cache.ZoneName }}; + proxy_cache_key $scheme$proxy_host$request_uri; + proxy_ignore_headers Cache-Control Expires; + {{- if and $l.Cache.Time (eq (len $l.Cache.Valid) 0) }} + proxy_cache_valid {{ $l.Cache.Time }}; + {{- end }} + {{- range $code, $time := $l.Cache.Valid }} + proxy_cache_valid {{ $code }} {{ $time }}; + {{- end }} + {{- if $l.Cache.AllowedMethods }} + proxy_cache_methods{{ range $l.Cache.AllowedMethods }} {{ . }}{{ end }}; + {{- end }} + {{- if gt (len $l.Cache.CachePurgeAllow) 0 }} + proxy_cache_purge $cache_purge; + {{- end }} + {{- end }} + {{- end }} + {{- if $l.GRPCPass }} grpc_pass {{ $l.GRPCPass }}; {{- else }} diff --git a/internal/configs/version2/nginx.virtualserver.tmpl b/internal/configs/version2/nginx.virtualserver.tmpl index 4721b3c879..f5e64b0cf6 100644 --- a/internal/configs/version2/nginx.virtualserver.tmpl +++ b/internal/configs/version2/nginx.virtualserver.tmpl @@ -40,6 +40,10 @@ map {{ $m.Source }} {{ $m.Variable }} { limit_req_zone {{ $z.Key }} zone={{ $z.ZoneName }}:{{ $z.ZoneSize }} rate={{ $z.Rate }}; {{- end }} +{{- range $c := .CacheZones }} +proxy_cache_path {{ $c.Path }} keys_zone={{ $c.Name }}:{{ $c.Size }}; +{{- end }} + {{- $s := .Server }} server { {{- if $s.Gunzip }} @@ -114,6 +118,24 @@ server { return {{ .Code }}; {{- end }} + {{- with $s.Cache }} + {{- if $s.Cache.Enable }} + # Server-level cache configuration + proxy_cache {{ $s.Cache.ZoneName }}; + proxy_cache_key $scheme$proxy_host$request_uri; + proxy_ignore_headers Cache-Control Expires; + {{- if and $s.Cache.Time (eq (len $s.Cache.Valid) 0) }} + proxy_cache_valid {{ $s.Cache.Time }}; + {{- end }} + {{- range $code, $time := $s.Cache.Valid }} + proxy_cache_valid {{ $code }} {{ $time }}; + {{- end }} + {{- if $s.Cache.AllowedMethods }} + proxy_cache_methods{{ range $s.Cache.AllowedMethods }} {{ . }}{{ end }}; + {{- end }} + {{- end }} + {{- end }} + {{- range $allow := $s.Allow }} allow {{ $allow }}; {{- end }} @@ -412,6 +434,24 @@ server { {{ $proxyOrGRPC }}_ssl_verify_depth 25; {{ $proxyOrGRPC }}_ssl_name {{ $l.ProxySSLName }}; {{- end }} + + {{- with $l.Cache }} + {{- if $l.Cache.Enable }} + proxy_cache {{ $l.Cache.ZoneName }}; + proxy_cache_key $scheme$proxy_host$request_uri; + proxy_ignore_headers Cache-Control Expires; + {{- if and $l.Cache.Time (eq (len $l.Cache.Valid) 0) }} + proxy_cache_valid {{ $l.Cache.Time }}; + {{- end }} + {{- range $code, $time := $l.Cache.Valid }} + proxy_cache_valid {{ $code }} {{ $time }}; + {{- end }} + {{- if $l.Cache.AllowedMethods }} + proxy_cache_methods{{ range $l.Cache.AllowedMethods }} {{ . }}{{ end }}; + {{- end }} + {{- end }} + {{- end }} + {{- if $l.GRPCPass }} grpc_pass {{ $l.GRPCPass }}; {{- else }} diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index fc17a4f45c..fd10434cbb 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" ) const ( @@ -467,10 +468,14 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( var healthChecks []version2.HealthCheck var limitReqZones []version2.LimitReqZone var authJWTClaimSets []version2.AuthJWTClaimSet + var cacheZones []version2.CacheZone limitReqZones = append(limitReqZones, policiesCfg.RateLimit.Zones...) authJWTClaimSets = append(authJWTClaimSets, policiesCfg.RateLimit.AuthJWTClaimSets...) + // Add cache zone from global policy if present + addCacheZone(&cacheZones, policiesCfg.Cache) + // generate upstreams for VirtualServer for _, u := range vsEx.VirtualServer.Spec.Upstreams { @@ -632,6 +637,9 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( authJWTClaimSets = append(authJWTClaimSets, routePoliciesCfg.RateLimit.AuthJWTClaimSets...) + // Add cache zone from route policy if present + addCacheZone(&cacheZones, routePoliciesCfg.Cache) + dosRouteCfg := generateDosCfg(dosResources[r.Path]) if len(r.Matches) > 0 { @@ -785,6 +793,9 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( authJWTClaimSets = append(authJWTClaimSets, routePoliciesCfg.RateLimit.AuthJWTClaimSets...) + // Add cache zone from subroute policy if present + addCacheZone(&cacheZones, routePoliciesCfg.Cache) + dosRouteCfg := generateDosCfg(dosResources[r.Path]) if len(r.Matches) > 0 { @@ -872,6 +883,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( StatusMatches: statusMatches, LimitReqZones: removeDuplicateLimitReqZones(limitReqZones), AuthJWTClaimSets: removeDuplicateAuthJWTClaimSets(authJWTClaimSets), + CacheZones: cacheZones, HTTPSnippets: httpSnippets, Server: version2.Server{ ServerName: vsEx.VirtualServer.Spec.Host, @@ -913,6 +925,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( OIDC: vsc.oidcPolCfg.oidc, WAF: policiesCfg.WAF, Dos: dosCfg, + Cache: policiesCfg.Cache, PoliciesErrorReturn: policiesCfg.ErrorReturn, VSNamespace: vsEx.VirtualServer.Namespace, VSName: vsEx.VirtualServer.Name, @@ -967,6 +980,7 @@ type policiesCfg struct { OIDC bool APIKey apiKeyAuth WAF *version2.WAF + Cache *version2.Cache ErrorReturn *version2.Return BundleValidator bundleValidator } @@ -1724,6 +1738,13 @@ func (vsc *virtualServerConfigurator) generatePolicies( ownerDetails.vsName, policyOpts.secretRefs) case pol.Spec.WAF != nil: res = config.addWAFConfig(vsc.cfgParams.Context, pol.Spec.WAF, key, polNamespace, policyOpts.apResources) + case pol.Spec.Cache != nil: + res = newValidationResults() + if config.Cache != nil { + res.addWarningf("Multiple cache policies in the same context is not valid. Cache policy %s will be ignored", key) + } else { + config.Cache = generateCacheConfig(pol.Spec.Cache) + } default: res = newValidationResults() } @@ -1883,6 +1904,59 @@ func generateLimitReqOptions(rateLimitPol *conf_v1.RateLimit) version2.LimitReqO } } +func generateCacheConfig(cache *conf_v1.Cache) *version2.Cache { + cacheConfig := &version2.Cache{ + ZoneName: cache.CacheZoneName, + Enable: true, + Time: cache.Time, + Valid: make(map[string]string), + AllowedMethods: cache.AllowedMethods, + CachePurgeAllow: cache.CachePurgeAllow, + ZoneSize: cache.CacheZoneSize, + } + + // Convert allowed codes to proxy_cache_valid entries + for _, code := range cache.AllowedCodes { + if cache.Time != "" { + if code.Type == intstr.String { + // Handle the "any" string case + cacheConfig.Valid[code.StrVal] = cache.Time + } else { + // Handle integer status codes + cacheConfig.Valid[fmt.Sprintf("%d", code.IntVal)] = cache.Time + } + } + } + + return cacheConfig +} + +func addCacheZone(cacheZones *[]version2.CacheZone, cache *version2.Cache) { + if cache == nil { + return + } + + zoneSize := "10m" // default + if cache.ZoneSize != "" { + zoneSize = cache.ZoneSize + } + + cacheZone := version2.CacheZone{ + Name: cache.ZoneName, + Size: zoneSize, + Path: fmt.Sprintf("/var/cache/nginx/%s", cache.ZoneName), + } + + // Check for duplicates + for _, existing := range *cacheZones { + if existing.Name == cacheZone.Name { + return // Already exists, don't add duplicate + } + } + + *cacheZones = append(*cacheZones, cacheZone) +} + func removeDuplicateLimitReqZones(rlz []version2.LimitReqZone) []version2.LimitReqZone { encountered := make(map[string]bool) result := []version2.LimitReqZone{} @@ -1967,6 +2041,7 @@ func addPoliciesCfgToLocation(cfg policiesCfg, location *version2.Location) { location.OIDC = cfg.OIDC location.WAF = cfg.WAF location.APIKey = cfg.APIKey.Key + location.Cache = cfg.Cache location.PoliciesErrorReturn = cfg.ErrorReturn } diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index e700fd5dba..9ef6223493 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -2,6 +2,7 @@ package v1 import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" ) const ( @@ -580,6 +581,7 @@ type PolicySpec struct { OIDC *OIDC `json:"oidc"` WAF *WAF `json:"waf"` APIKey *APIKey `json:"apiKey"` + Cache *Cache `json:"cache"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -731,3 +733,13 @@ type SuppliedIn struct { Header []string `json:"header"` Query []string `json:"query"` } + +// Cache defines a cache policy for proxy caching. +type Cache struct { + CacheZoneName string `json:"cacheZoneName"` + CacheZoneSize string `json:"cacheZoneSize"` + AllowedCodes []intstr.IntOrString `json:"allowedCodes,omitempty"` + AllowedMethods []string `json:"allowedMethods,omitempty"` + Time string `json:"time,omitempty"` + CachePurgeAllow []string `json:"cachePurgeAllow,omitempty"` +} diff --git a/pkg/apis/configuration/validation/policy.go b/pkg/apis/configuration/validation/policy.go index c698a455ba..79865f7106 100644 --- a/pkg/apis/configuration/validation/policy.go +++ b/pkg/apis/configuration/validation/policy.go @@ -10,6 +10,7 @@ import ( "unicode" v1 "github.com/nginx/kubernetes-ingress/pkg/apis/configuration/v1" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" ) @@ -90,8 +91,13 @@ func validatePolicySpec(spec *v1.PolicySpec, fieldPath *field.Path, isPlus, enab fieldCount++ } + if spec.Cache != nil { + allErrs = append(allErrs, validateCache(spec.Cache, fieldPath.Child("cache"), isPlus)...) + fieldCount++ + } + if fieldCount != 1 { - msg := "must specify exactly one of: `accessControl`, `rateLimit`, `ingressMTLS`, `egressMTLS`, `basicAuth`, `apiKey`" + msg := "must specify exactly one of: `accessControl`, `rateLimit`, `ingressMTLS`, `egressMTLS`, `basicAuth`, `apiKey`, `cache`" if isPlus { msg = fmt.Sprint(msg, ", `jwt`, `oidc`, `waf`") } @@ -392,6 +398,64 @@ func validateLogConfs(logs []*v1.SecurityLog, fieldPath *field.Path, bundleMode return allErrs } +// validateCache validates a cache policy +func validateCache(cache *v1.Cache, fieldPath *field.Path, isPlus bool) field.ErrorList { + allErrs := field.ErrorList{} + + // Validate required fields + if cache.CacheZoneName == "" { + allErrs = append(allErrs, field.Required(fieldPath.Child("cacheZoneName"), "cacheZoneName is required")) + } else { + // Validate zone name format (should be a valid identifier) + if len(cache.CacheZoneName) > 255 { + allErrs = append(allErrs, field.TooLong(fieldPath.Child("cacheZoneName"), cache.CacheZoneName, 255)) + } + // Basic validation for zone name (no special characters except underscore) + for _, char := range cache.CacheZoneName { + if !((char >= 'a' && char <= 'z') || (char >= 'A' && char <= 'Z') || (char >= '0' && char <= '9') || char == '_') { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("cacheZoneName"), cache.CacheZoneName, "zone name can only contain alphanumeric characters and underscores")) + break + } + } + } + + // Validate allowed codes if provided + for i, code := range cache.AllowedCodes { + if code.Type == intstr.String { + // Only allow the string "any" + if code.StrVal != "any" { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("allowedCodes").Index(i), code.StrVal, "only the string 'any' is allowed")) + } + } else { + // Validate integer codes (100-599) + intCode := code.IntVal + if intCode < 100 || intCode > 599 { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("allowedCodes").Index(i), intCode, "HTTP status code must be between 100 and 599")) + } + } + } + + // Validate cache purge allow IPs if provided + if len(cache.CachePurgeAllow) > 0 { + // Check if NGINX Plus is required for cache purge + if !isPlus { + allErrs = append(allErrs, field.Forbidden(fieldPath.Child("cachePurgeAllow"), "cache purge is only supported in NGINX Plus")) + } else { + // Validate IP addresses/CIDRs if NGINX Plus is available + for i, ip := range cache.CachePurgeAllow { + if net.ParseIP(ip) == nil { + // Try parsing as CIDR + if _, _, err := net.ParseCIDR(ip); err != nil { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("cachePurgeAllow").Index(i), ip, "must be a valid IP address or CIDR")) + } + } + } + } + } + + return allErrs +} + func validateLogConf(logConf *v1.SecurityLog, fieldPath *field.Path, bundleMode bool) field.ErrorList { allErrs := field.ErrorList{} From c0f6d66bee27f790fc213811ef24e69fad6ebfbc Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 2 Jul 2025 12:03:58 +0100 Subject: [PATCH 02/30] add example files --- .../custom-resources/cache-policy/cache.yaml | 12 ++++ .../cache-policy/cafe-secret.yaml | 1 + .../cache-policy/cafe-virtual-server.yaml | 33 ++++++++++ .../custom-resources/cache-policy/cafe.yaml | 65 +++++++++++++++++++ 4 files changed, 111 insertions(+) create mode 100644 examples/custom-resources/cache-policy/cache.yaml create mode 120000 examples/custom-resources/cache-policy/cafe-secret.yaml create mode 100644 examples/custom-resources/cache-policy/cafe-virtual-server.yaml create mode 100644 examples/custom-resources/cache-policy/cafe.yaml diff --git a/examples/custom-resources/cache-policy/cache.yaml b/examples/custom-resources/cache-policy/cache.yaml new file mode 100644 index 0000000000..1598c76c58 --- /dev/null +++ b/examples/custom-resources/cache-policy/cache.yaml @@ -0,0 +1,12 @@ +apiVersion: k8s.nginx.org/v1 +kind: Policy +metadata: + name: cache-policy +spec: + cache: + cacheZoneName: "mycache" #Required + cacheZoneSize: "14m" #Required + allowedCodes: ["any"] #Optional ["any"] or ["200", "301", ...] + allowedMethods: ["GET", "HEAD", "POST"] #Optional + time: "15m" #Optional # e.g. "15m", "1h", "2d". Default is "10m" + # cachePurgeAllow: ["", "", ...] # Optional, e.g. ["192.168.1.1", "192.168.1.0/24"]. This functionality is only available in NGINX Plus. \ No newline at end of file diff --git a/examples/custom-resources/cache-policy/cafe-secret.yaml b/examples/custom-resources/cache-policy/cafe-secret.yaml new file mode 120000 index 0000000000..efa8919b4b --- /dev/null +++ b/examples/custom-resources/cache-policy/cafe-secret.yaml @@ -0,0 +1 @@ +../../common-secrets/cafe-secret-cafe.example.com.yaml \ No newline at end of file diff --git a/examples/custom-resources/cache-policy/cafe-virtual-server.yaml b/examples/custom-resources/cache-policy/cafe-virtual-server.yaml new file mode 100644 index 0000000000..4d3d636730 --- /dev/null +++ b/examples/custom-resources/cache-policy/cafe-virtual-server.yaml @@ -0,0 +1,33 @@ +apiVersion: k8s.nginx.org/v1 +kind: VirtualServer +metadata: + name: cafe +spec: + server-snippets: | + add_header X-Cache-Status $upstream_cache_status; + # This header will show the cache status for each request, e.g. X-Cache-Status: MISS or X-Cache-Status: HIT. + # The cache status can be "HIT", "MISS", "EXPIRED", etc. + # This is useful for debugging and monitoring cache behavior but not required for cache functionality. + + policies: + - name: cache-policy + host: cafe.example.com + tls: + secret: cafe-secret + upstreams: + - name: tea + service: tea-svc + port: 80 + - name: coffee + service: coffee-svc + port: 80 + routes: + - path: /tea + action: + pass: tea + - path: /coffee + action: + pass: coffee + + + diff --git a/examples/custom-resources/cache-policy/cafe.yaml b/examples/custom-resources/cache-policy/cafe.yaml new file mode 100644 index 0000000000..f049e8bf29 --- /dev/null +++ b/examples/custom-resources/cache-policy/cafe.yaml @@ -0,0 +1,65 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: coffee +spec: + replicas: 2 + selector: + matchLabels: + app: coffee + template: + metadata: + labels: + app: coffee + spec: + containers: + - name: coffee + image: nginxdemos/nginx-hello:plain-text + ports: + - containerPort: 8080 +--- +apiVersion: v1 +kind: Service +metadata: + name: coffee-svc +spec: + ports: + - port: 80 + targetPort: 8080 + protocol: TCP + name: http + selector: + app: coffee +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: tea +spec: + replicas: 1 + selector: + matchLabels: + app: tea + template: + metadata: + labels: + app: tea + spec: + containers: + - name: tea + image: nginxdemos/nginx-hello:plain-text + ports: + - containerPort: 8080 +--- +apiVersion: v1 +kind: Service +metadata: + name: tea-svc +spec: + ports: + - port: 80 + targetPort: 8080 + protocol: TCP + name: http + selector: + app: tea From 7acfa857f984a23c96696ed0d7d24b6b83a854b2 Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 2 Jul 2025 17:51:17 +0100 Subject: [PATCH 03/30] update CRD validation and add sessionAffinity --- .../templates/controller-service.yaml | 8 +++ charts/nginx-ingress/values.schema.json | 51 ++++++++++++++++++- config/crd/bases/k8s.nginx.org_policies.yaml | 2 + pkg/apis/configuration/validation/policy.go | 17 ------- 4 files changed, 60 insertions(+), 18 deletions(-) diff --git a/charts/nginx-ingress/templates/controller-service.yaml b/charts/nginx-ingress/templates/controller-service.yaml index 0073813227..2e3c0a186d 100644 --- a/charts/nginx-ingress/templates/controller-service.yaml +++ b/charts/nginx-ingress/templates/controller-service.yaml @@ -65,6 +65,14 @@ spec: {{- end }} selector: {{- include "nginx-ingress.selectorLabels" . | nindent 4 }} + {{- if .Values.controller.service.sessionAffinity.enable }} + sessionAffinity: {{ .Values.controller.service.sessionAffinity.type }} + {{- if eq .Values.controller.service.sessionAffinity.type "ClientIP" }} + sessionAffinityConfig: + clientIP: + timeoutSeconds: {{ .Values.controller.service.sessionAffinity.timeoutSeconds }} + {{- end }} + {{- end }} {{- if .Values.controller.service.externalIPs }} externalIPs: {{ toYaml .Values.controller.service.externalIPs | indent 4 }} diff --git a/charts/nginx-ingress/values.schema.json b/charts/nginx-ingress/values.schema.json index bbb6658e04..348acba954 100644 --- a/charts/nginx-ingress/values.schema.json +++ b/charts/nginx-ingress/values.schema.json @@ -1454,6 +1454,50 @@ "type": "object", "ref": "https://raw.githubusercontent.com/nginxinc/kubernetes-json-schema/master/v1.33.1/_definitions.json#/definitions/io.k8s.api.core.v1.ServicePort" } + }, + "sessionAffinity": { + "type": "object", + "default": {}, + "title": "The sessionAffinity Schema", + "required": [], + "properties": { + "enable": { + "type": "boolean", + "default": false, + "title": "Enable session affinity", + "examples": [ + false + ] + }, + "type": { + "type": "string", + "default": "ClientIP", + "title": "Session affinity type", + "enum": [ + "ClientIP" + ], + "examples": [ + "ClientIP" + ] + }, + "timeoutSeconds": { + "type": "integer", + "default": 3600, + "title": "Session affinity timeout in seconds", + "minimum": 1, + "maximum": 86400, + "examples": [ + 3600 + ] + } + }, + "examples": [ + { + "enable": false, + "type": "ClientIP", + "timeoutSeconds": 3600 + } + ] } }, "examples": [ @@ -1482,7 +1526,12 @@ "targetPort": 443, "name": "https" }, - "customPorts": [] + "customPorts": [], + "sessionAffinity": { + "enable": false, + "type": "ClientIP", + "timeoutSeconds": 3600 + } } ] }, diff --git a/config/crd/bases/k8s.nginx.org_policies.yaml b/config/crd/bases/k8s.nginx.org_policies.yaml index 647f656c9a..2e217ab69f 100644 --- a/config/crd/bases/k8s.nginx.org_policies.yaml +++ b/config/crd/bases/k8s.nginx.org_policies.yaml @@ -307,6 +307,8 @@ spec: cacheZoneName: description: CacheZoneName defines the name of the cache zone type: string + pattern: '^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$' + maxLength: 64 cacheZoneSize: description: CacheZoneSize defines the size of the cache zone type: string diff --git a/pkg/apis/configuration/validation/policy.go b/pkg/apis/configuration/validation/policy.go index 79865f7106..062a2d5e2e 100644 --- a/pkg/apis/configuration/validation/policy.go +++ b/pkg/apis/configuration/validation/policy.go @@ -402,23 +402,6 @@ func validateLogConfs(logs []*v1.SecurityLog, fieldPath *field.Path, bundleMode func validateCache(cache *v1.Cache, fieldPath *field.Path, isPlus bool) field.ErrorList { allErrs := field.ErrorList{} - // Validate required fields - if cache.CacheZoneName == "" { - allErrs = append(allErrs, field.Required(fieldPath.Child("cacheZoneName"), "cacheZoneName is required")) - } else { - // Validate zone name format (should be a valid identifier) - if len(cache.CacheZoneName) > 255 { - allErrs = append(allErrs, field.TooLong(fieldPath.Child("cacheZoneName"), cache.CacheZoneName, 255)) - } - // Basic validation for zone name (no special characters except underscore) - for _, char := range cache.CacheZoneName { - if !((char >= 'a' && char <= 'z') || (char >= 'A' && char <= 'Z') || (char >= '0' && char <= '9') || char == '_') { - allErrs = append(allErrs, field.Invalid(fieldPath.Child("cacheZoneName"), cache.CacheZoneName, "zone name can only contain alphanumeric characters and underscores")) - break - } - } - } - // Validate allowed codes if provided for i, code := range cache.AllowedCodes { if code.Type == intstr.String { From a82a79029e850d51251925309419d268db09baac Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 2 Jul 2025 17:52:37 +0100 Subject: [PATCH 04/30] add values.yaml --- charts/nginx-ingress/values.yaml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/charts/nginx-ingress/values.yaml b/charts/nginx-ingress/values.yaml index 5557e20b83..8ede4c44b9 100644 --- a/charts/nginx-ingress/values.yaml +++ b/charts/nginx-ingress/values.yaml @@ -502,6 +502,15 @@ controller: ## A list of custom ports to expose through the Ingress Controller service. Follows the conventional Kubernetes yaml syntax for service ports. customPorts: [] + ## Session affinity configuration for the Ingress Controller service, ensures requests from the same client IP go to the same pod + sessionAffinity: + ## Enable session affinity. Valid values: None, ClientIP + enable: false + ## Session affinity type. Currently only ClientIP is supported. + type: ClientIP + ## Session affinity timeout in seconds (default: 3600 = 1 hour) + timeoutSeconds: 3600 + serviceAccount: ## The annotations of the service account of the Ingress Controller pods. annotations: {} From d90c2d213dc7cc9064afa6993a123e30aa218d6c Mon Sep 17 00:00:00 2001 From: Venktesh Date: Thu, 3 Jul 2025 12:42:33 +0100 Subject: [PATCH 05/30] add overrideUpstreamCache option --- config/crd/bases/k8s.nginx.org_policies.yaml | 4 ++++ internal/configs/version2/http.go | 15 ++++++++------- .../version2/nginx-plus.virtualserver.tmpl | 8 ++++++-- .../configs/version2/nginx.virtualserver.tmpl | 8 ++++++-- internal/configs/virtualserver.go | 15 ++++++++------- pkg/apis/configuration/v1/types.go | 13 +++++++------ 6 files changed, 39 insertions(+), 24 deletions(-) diff --git a/config/crd/bases/k8s.nginx.org_policies.yaml b/config/crd/bases/k8s.nginx.org_policies.yaml index 2e217ab69f..428b05be28 100644 --- a/config/crd/bases/k8s.nginx.org_policies.yaml +++ b/config/crd/bases/k8s.nginx.org_policies.yaml @@ -313,6 +313,10 @@ spec: description: CacheZoneSize defines the size of the cache zone type: string pattern: '^[0-9]+[kmg]$' + overrideUpstreamCache: + description: OverrideUpstreamCache controls whether to override upstream cache headers (using proxy_ignore_headers directive) + type: boolean + default: false time: description: Time defines the default cache time (required when allowedCodes is specified) type: string diff --git a/internal/configs/version2/http.go b/internal/configs/version2/http.go index 826e40fb87..4995c98e8a 100644 --- a/internal/configs/version2/http.go +++ b/internal/configs/version2/http.go @@ -491,11 +491,12 @@ type CacheZone struct { // Cache defines cache configuration for locations. type Cache struct { - ZoneName string - ZoneSize string - Enable bool - Time string - Valid map[string]string // map for codes to time - AllowedMethods []string // HTTP methods allowed for caching based on proxy_cache_methods - CachePurgeAllow []string // IPs/CIDRs allowed to purge cache + ZoneName string + ZoneSize string + Enable bool + Time string + Valid map[string]string // map for codes to time + AllowedMethods []string // HTTP methods allowed for caching based on proxy_cache_methods + CachePurgeAllow []string // IPs/CIDRs allowed to purge cache + OverrideUpstreamCache bool // Controls whether to override upstream cache headers } diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index 8e7762cf44..1c9812093e 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -234,7 +234,9 @@ server { # Server-level cache configuration proxy_cache {{ $s.Cache.ZoneName }}; proxy_cache_key $scheme$proxy_host$request_uri; - proxy_ignore_headers Cache-Control Expires; + {{- if $s.Cache.OverrideUpstreamCache }} + proxy_ignore_headers Cache-Control Expires Set-Cookie Vary X-Accel-Expires; + {{- end }} {{- if and $s.Cache.Time (eq (len $s.Cache.Valid) 0) }} proxy_cache_valid {{ $s.Cache.Time }}; {{- end }} @@ -732,7 +734,9 @@ server { {{- if $l.Cache.Enable }} proxy_cache {{ $l.Cache.ZoneName }}; proxy_cache_key $scheme$proxy_host$request_uri; - proxy_ignore_headers Cache-Control Expires; + {{- if $l.Cache.OverrideUpstreamCache }} + proxy_ignore_headers Cache-Control Expires Set-Cookie Vary X-Accel-Expires; + {{- end }} {{- if and $l.Cache.Time (eq (len $l.Cache.Valid) 0) }} proxy_cache_valid {{ $l.Cache.Time }}; {{- end }} diff --git a/internal/configs/version2/nginx.virtualserver.tmpl b/internal/configs/version2/nginx.virtualserver.tmpl index f5e64b0cf6..2baea2c42b 100644 --- a/internal/configs/version2/nginx.virtualserver.tmpl +++ b/internal/configs/version2/nginx.virtualserver.tmpl @@ -123,7 +123,9 @@ server { # Server-level cache configuration proxy_cache {{ $s.Cache.ZoneName }}; proxy_cache_key $scheme$proxy_host$request_uri; - proxy_ignore_headers Cache-Control Expires; + {{- if $s.Cache.OverrideUpstreamCache }} + proxy_ignore_headers Cache-Control Expires Set-Cookie Vary X-Accel-Expires; + {{- end }} {{- if and $s.Cache.Time (eq (len $s.Cache.Valid) 0) }} proxy_cache_valid {{ $s.Cache.Time }}; {{- end }} @@ -439,7 +441,9 @@ server { {{- if $l.Cache.Enable }} proxy_cache {{ $l.Cache.ZoneName }}; proxy_cache_key $scheme$proxy_host$request_uri; - proxy_ignore_headers Cache-Control Expires; + {{- if $l.Cache.OverrideUpstreamCache }} + proxy_ignore_headers Cache-Control Expires Set-Cookie Vary X-Accel-Expires;; + {{- end }} {{- if and $l.Cache.Time (eq (len $l.Cache.Valid) 0) }} proxy_cache_valid {{ $l.Cache.Time }}; {{- end }} diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index fd10434cbb..f6b2f95eb0 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -1906,13 +1906,14 @@ func generateLimitReqOptions(rateLimitPol *conf_v1.RateLimit) version2.LimitReqO func generateCacheConfig(cache *conf_v1.Cache) *version2.Cache { cacheConfig := &version2.Cache{ - ZoneName: cache.CacheZoneName, - Enable: true, - Time: cache.Time, - Valid: make(map[string]string), - AllowedMethods: cache.AllowedMethods, - CachePurgeAllow: cache.CachePurgeAllow, - ZoneSize: cache.CacheZoneSize, + ZoneName: cache.CacheZoneName, + Enable: true, + Time: cache.Time, + Valid: make(map[string]string), + AllowedMethods: cache.AllowedMethods, + CachePurgeAllow: cache.CachePurgeAllow, + ZoneSize: cache.CacheZoneSize, + OverrideUpstreamCache: cache.OverrideUpstreamCache, } // Convert allowed codes to proxy_cache_valid entries diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index 9ef6223493..1671e404ec 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -736,10 +736,11 @@ type SuppliedIn struct { // Cache defines a cache policy for proxy caching. type Cache struct { - CacheZoneName string `json:"cacheZoneName"` - CacheZoneSize string `json:"cacheZoneSize"` - AllowedCodes []intstr.IntOrString `json:"allowedCodes,omitempty"` - AllowedMethods []string `json:"allowedMethods,omitempty"` - Time string `json:"time,omitempty"` - CachePurgeAllow []string `json:"cachePurgeAllow,omitempty"` + CacheZoneName string `json:"cacheZoneName"` + CacheZoneSize string `json:"cacheZoneSize"` + AllowedCodes []intstr.IntOrString `json:"allowedCodes,omitempty"` + AllowedMethods []string `json:"allowedMethods,omitempty"` + Time string `json:"time,omitempty"` + CachePurgeAllow []string `json:"cachePurgeAllow,omitempty"` + OverrideUpstreamCache bool `json:"overrideUpstreamCache,omitempty"` } From d7604cf56975a49a448a2561161ce66d83fd1ebf Mon Sep 17 00:00:00 2001 From: Venktesh Date: Fri, 4 Jul 2025 12:48:41 +0100 Subject: [PATCH 06/30] add unique purge var per VS, VSR --- .../version2/nginx-plus.virtualserver.tmpl | 39 ++++++++----------- internal/configs/virtualserver.go | 17 ++++++-- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index 1c9812093e..9694d49725 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -82,43 +82,38 @@ match {{ $m.Name }} { {{- $s := .Server }} -{{- /* Generate cache purge configuration if any cache has purge restrictions */ -}} -{{- $hasCachePurge := false }} +{{- /* Generate cache-zone-specific purge configuration with VirtualServer isolation */ -}} {{- /* Check server-level cache purge restrictions */ -}} {{- if and $s.Cache $s.Cache.Enable (gt (len $s.Cache.CachePurgeAllow) 0) }} -{{- $hasCachePurge = true }} -{{- end }} -{{- /* Check location-level cache purge restrictions */ -}} -{{- range $l := $s.Locations }} -{{- if and $l.Cache $l.Cache.Enable (gt (len $l.Cache.CachePurgeAllow) 0) }} -{{- $hasCachePurge = true }} -{{- end }} -{{- end }} - -{{- if $hasCachePurge }} -geo $purge_allowed { +geo $purge_allowed_{{ $s.Cache.ZoneName }} { default 0; -{{- /* Add server-level cache purge IPs */ -}} -{{- if and $s.Cache $s.Cache.Enable (gt (len $s.Cache.CachePurgeAllow) 0) }} {{- range $ip := $s.Cache.CachePurgeAllow }} {{ $ip }} 1; {{- end }} +} + +map $request_method $cache_purge_{{ $s.Cache.ZoneName }} { + PURGE $purge_allowed_{{ $s.Cache.ZoneName }}; + default 0; +} {{- end }} -{{- /* Add location-level cache purge IPs */ -}} + +{{- /* Check location-level cache purge restrictions */ -}} {{- range $l := $s.Locations }} {{- if and $l.Cache $l.Cache.Enable (gt (len $l.Cache.CachePurgeAllow) 0) }} +geo $purge_allowed_{{ $l.Cache.ZoneName }} { + default 0; {{- range $ip := $l.Cache.CachePurgeAllow }} {{ $ip }} 1; {{- end }} -{{- end }} -{{- end }} } -map $request_method $cache_purge { - PURGE $purge_allowed; +map $request_method $cache_purge_{{ $l.Cache.ZoneName }} { + PURGE $purge_allowed_{{ $l.Cache.ZoneName }}; default 0; } {{- end }} +{{- end }} {{- with $s.JWKSAuthEnabled }} proxy_cache_path /var/cache/nginx/jwks_uri_{{$s.VSName}} levels=1 keys_zone=jwks_uri_{{$s.VSName}}:1m max_size=10m; @@ -247,7 +242,7 @@ server { proxy_cache_methods{{ range $s.Cache.AllowedMethods }} {{ . }}{{ end }}; {{- end }} {{- if gt (len $s.Cache.CachePurgeAllow) 0 }} - proxy_cache_purge $cache_purge; + proxy_cache_purge $cache_purge_{{ $s.Cache.ZoneName }}; {{- end }} {{- end }} {{- end }} @@ -747,7 +742,7 @@ server { proxy_cache_methods{{ range $l.Cache.AllowedMethods }} {{ . }}{{ end }}; {{- end }} {{- if gt (len $l.Cache.CachePurgeAllow) 0 }} - proxy_cache_purge $cache_purge; + proxy_cache_purge $cache_purge_{{ $l.Cache.ZoneName }}; {{- end }} {{- end }} {{- end }} diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index f6b2f95eb0..7b50e2eabc 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -1743,7 +1743,7 @@ func (vsc *virtualServerConfigurator) generatePolicies( if config.Cache != nil { res.addWarningf("Multiple cache policies in the same context is not valid. Cache policy %s will be ignored", key) } else { - config.Cache = generateCacheConfig(pol.Spec.Cache) + config.Cache = generateCacheConfig(pol.Spec.Cache, ownerDetails.vsNamespace, ownerDetails.vsName, ownerDetails.ownerNamespace, ownerDetails.ownerName) } default: res = newValidationResults() @@ -1904,9 +1904,20 @@ func generateLimitReqOptions(rateLimitPol *conf_v1.RateLimit) version2.LimitReqO } } -func generateCacheConfig(cache *conf_v1.Cache) *version2.Cache { +func generateCacheConfig(cache *conf_v1.Cache, vsNamespace, vsName, ownerNamespace, ownerName string) *version2.Cache { + // Create unique zone name including VS namespace/name and owner namespace/name for policy reuse + // This ensures that the same cache policy can be safely reused across different VS/VSR + var uniqueZoneName string + if vsNamespace == ownerNamespace && vsName == ownerName { + // Policy is applied directly to VirtualServer, use VS namespace/name only + uniqueZoneName = fmt.Sprintf("%s_%s_%s", vsNamespace, vsName, cache.CacheZoneName) + } else { + // Policy is applied to VirtualServerRoute, include both VS and owner info + uniqueZoneName = fmt.Sprintf("%s_%s_%s_%s_%s", vsNamespace, vsName, ownerNamespace, ownerName, cache.CacheZoneName) + } + cacheConfig := &version2.Cache{ - ZoneName: cache.CacheZoneName, + ZoneName: uniqueZoneName, Enable: true, Time: cache.Time, Valid: make(map[string]string), From 1563406a3cb41e0c0f8e6939ab76e1f0f6ad740c Mon Sep 17 00:00:00 2001 From: Venktesh Date: Fri, 4 Jul 2025 13:47:39 +0100 Subject: [PATCH 07/30] remove redundant enable flag --- internal/configs/version2/http.go | 1 - internal/configs/version2/nginx-plus.virtualserver.tmpl | 8 ++------ internal/configs/version2/nginx.virtualserver.tmpl | 4 ---- internal/configs/virtualserver.go | 1 - 4 files changed, 2 insertions(+), 12 deletions(-) diff --git a/internal/configs/version2/http.go b/internal/configs/version2/http.go index 4995c98e8a..4d44d1d568 100644 --- a/internal/configs/version2/http.go +++ b/internal/configs/version2/http.go @@ -493,7 +493,6 @@ type CacheZone struct { type Cache struct { ZoneName string ZoneSize string - Enable bool Time string Valid map[string]string // map for codes to time AllowedMethods []string // HTTP methods allowed for caching based on proxy_cache_methods diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index 9694d49725..62b1fb31bd 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -84,7 +84,7 @@ match {{ $m.Name }} { {{- /* Generate cache-zone-specific purge configuration with VirtualServer isolation */ -}} {{- /* Check server-level cache purge restrictions */ -}} -{{- if and $s.Cache $s.Cache.Enable (gt (len $s.Cache.CachePurgeAllow) 0) }} +{{- if and $s.Cache (gt (len $s.Cache.CachePurgeAllow) 0) }} geo $purge_allowed_{{ $s.Cache.ZoneName }} { default 0; {{- range $ip := $s.Cache.CachePurgeAllow }} @@ -100,7 +100,7 @@ map $request_method $cache_purge_{{ $s.Cache.ZoneName }} { {{- /* Check location-level cache purge restrictions */ -}} {{- range $l := $s.Locations }} -{{- if and $l.Cache $l.Cache.Enable (gt (len $l.Cache.CachePurgeAllow) 0) }} +{{- if and $l.Cache (gt (len $l.Cache.CachePurgeAllow) 0) }} geo $purge_allowed_{{ $l.Cache.ZoneName }} { default 0; {{- range $ip := $l.Cache.CachePurgeAllow }} @@ -225,7 +225,6 @@ server { {{- end }} {{- with $s.Cache }} - {{- if $s.Cache.Enable }} # Server-level cache configuration proxy_cache {{ $s.Cache.ZoneName }}; proxy_cache_key $scheme$proxy_host$request_uri; @@ -244,7 +243,6 @@ server { {{- if gt (len $s.Cache.CachePurgeAllow) 0 }} proxy_cache_purge $cache_purge_{{ $s.Cache.ZoneName }}; {{- end }} - {{- end }} {{- end }} {{- range $allow := $s.Allow }} @@ -726,7 +724,6 @@ server { {{- end }} {{- with $l.Cache }} - {{- if $l.Cache.Enable }} proxy_cache {{ $l.Cache.ZoneName }}; proxy_cache_key $scheme$proxy_host$request_uri; {{- if $l.Cache.OverrideUpstreamCache }} @@ -744,7 +741,6 @@ server { {{- if gt (len $l.Cache.CachePurgeAllow) 0 }} proxy_cache_purge $cache_purge_{{ $l.Cache.ZoneName }}; {{- end }} - {{- end }} {{- end }} {{- if $l.GRPCPass }} diff --git a/internal/configs/version2/nginx.virtualserver.tmpl b/internal/configs/version2/nginx.virtualserver.tmpl index 2baea2c42b..e42f3a5b02 100644 --- a/internal/configs/version2/nginx.virtualserver.tmpl +++ b/internal/configs/version2/nginx.virtualserver.tmpl @@ -119,7 +119,6 @@ server { {{- end }} {{- with $s.Cache }} - {{- if $s.Cache.Enable }} # Server-level cache configuration proxy_cache {{ $s.Cache.ZoneName }}; proxy_cache_key $scheme$proxy_host$request_uri; @@ -135,7 +134,6 @@ server { {{- if $s.Cache.AllowedMethods }} proxy_cache_methods{{ range $s.Cache.AllowedMethods }} {{ . }}{{ end }}; {{- end }} - {{- end }} {{- end }} {{- range $allow := $s.Allow }} @@ -438,7 +436,6 @@ server { {{- end }} {{- with $l.Cache }} - {{- if $l.Cache.Enable }} proxy_cache {{ $l.Cache.ZoneName }}; proxy_cache_key $scheme$proxy_host$request_uri; {{- if $l.Cache.OverrideUpstreamCache }} @@ -453,7 +450,6 @@ server { {{- if $l.Cache.AllowedMethods }} proxy_cache_methods{{ range $l.Cache.AllowedMethods }} {{ . }}{{ end }}; {{- end }} - {{- end }} {{- end }} {{- if $l.GRPCPass }} diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 7b50e2eabc..186ec8953f 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -1918,7 +1918,6 @@ func generateCacheConfig(cache *conf_v1.Cache, vsNamespace, vsName, ownerNamespa cacheConfig := &version2.Cache{ ZoneName: uniqueZoneName, - Enable: true, Time: cache.Time, Valid: make(map[string]string), AllowedMethods: cache.AllowedMethods, From 028f7b1a3147d99a0e4c9db3a937cceabde40d8b Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 9 Jul 2025 12:14:32 +0100 Subject: [PATCH 08/30] add levels and shared cache config --- charts/nginx-ingress/templates/_helpers.tpl | 13 ++++++++++ charts/nginx-ingress/values.yaml | 10 ++++++++ config/crd/bases/k8s.nginx.org_policies.yaml | 4 +++ .../custom-resources/cache-policy/cache.yaml | 20 +++++++++++++-- .../cache-policy/cafe-virtual-server.yaml | 2 ++ .../shared-cache/secure-shared-cache-pvc.yaml | 25 +++++++++++++++++++ internal/configs/version2/http.go | 10 +++++--- .../version2/nginx-plus.virtualserver.tmpl | 2 +- .../configs/version2/nginx.virtualserver.tmpl | 2 +- internal/configs/virtualserver.go | 10 +++++--- pkg/apis/configuration/v1/types.go | 15 +++++------ 11 files changed, 94 insertions(+), 19 deletions(-) create mode 100644 examples/shared-cache/secure-shared-cache-pvc.yaml diff --git a/charts/nginx-ingress/templates/_helpers.tpl b/charts/nginx-ingress/templates/_helpers.tpl index c1700c9fa1..fa964b30b6 100644 --- a/charts/nginx-ingress/templates/_helpers.tpl +++ b/charts/nginx-ingress/templates/_helpers.tpl @@ -352,14 +352,24 @@ List of volumes for controller. {{- if eq (include "nginx-ingress.readOnlyRootFilesystem" .) "true" }} - name: nginx-etc emptyDir: {} +{{- if .Values.controller.cache.enableShared }} +- name: nginx-cache + persistentVolumeClaim: + claimName: {{ .Values.controller.cache.sharedPVCName }} +{{- else }} - name: nginx-cache emptyDir: {} +{{- end }} - name: nginx-lib emptyDir: {} - name: nginx-state emptyDir: {} - name: nginx-log emptyDir: {} +{{- else if .Values.controller.cache.enableShared }} +- name: nginx-cache + persistentVolumeClaim: + claimName: {{ .Values.controller.cache.sharedPVCName }} {{- end }} {{- if .Values.controller.appprotect.v5 }} {{ toYaml .Values.controller.appprotect.volumes }} @@ -419,6 +429,9 @@ volumeMounts: name: nginx-state - mountPath: /var/log/nginx name: nginx-log +{{- else if .Values.controller.cache.enableShared }} +- mountPath: /var/cache/nginx + name: nginx-cache {{- end }} {{- if .Values.controller.appprotect.v5 }} - name: app-protect-bd-config diff --git a/charts/nginx-ingress/values.yaml b/charts/nginx-ingress/values.yaml index 8ede4c44b9..61b17aa2f3 100644 --- a/charts/nginx-ingress/values.yaml +++ b/charts/nginx-ingress/values.yaml @@ -164,6 +164,16 @@ controller: ## Sets the log format of Ingress Controller. Options include: glog, json, text logFormat: glog + ## Cache configuration options + cache: + ## Enables shared cache across multiple pods using an external persistent volume + ## When enabled, the /var/cache/nginx directory will be mounted from a PVC instead of using emptyDir + ## User must create and configure a PVC with appropriate access mode + enableShared: true + + ## The name of the PersistentVolumeClaim to use for shared cache, should match the name of the PVC created by the user + sharedPVCName: "nginx-shared-cache" + ## A list of custom ports to expose on the NGINX Ingress Controller pod. Follows the conventional Kubernetes yaml syntax for container ports. customPorts: [] diff --git a/config/crd/bases/k8s.nginx.org_policies.yaml b/config/crd/bases/k8s.nginx.org_policies.yaml index 428b05be28..255aee93b3 100644 --- a/config/crd/bases/k8s.nginx.org_policies.yaml +++ b/config/crd/bases/k8s.nginx.org_policies.yaml @@ -321,6 +321,10 @@ spec: description: Time defines the default cache time (required when allowedCodes is specified) type: string pattern: '^[0-9]+[smhd]$' + levels: + description: Directory hierarchy for cache files. Controls the number of subdirectory levels used for cache storage. + type: string + pattern: '^[12](?::[12]){0,2}$' required: - cacheZoneName - cacheZoneSize diff --git a/examples/custom-resources/cache-policy/cache.yaml b/examples/custom-resources/cache-policy/cache.yaml index 1598c76c58..220dbbe36e 100644 --- a/examples/custom-resources/cache-policy/cache.yaml +++ b/examples/custom-resources/cache-policy/cache.yaml @@ -8,5 +8,21 @@ spec: cacheZoneSize: "14m" #Required allowedCodes: ["any"] #Optional ["any"] or ["200", "301", ...] allowedMethods: ["GET", "HEAD", "POST"] #Optional - time: "15m" #Optional # e.g. "15m", "1h", "2d". Default is "10m" - # cachePurgeAllow: ["", "", ...] # Optional, e.g. ["192.168.1.1", "192.168.1.0/24"]. This functionality is only available in NGINX Plus. \ No newline at end of file + time: "25m" #Optional # e.g. "15m", "1h", "2d". Default is "10m" + # cachePurgeAllow: [""] + overrideUpstreamCache: false +--- +apiVersion: k8s.nginx.org/v1 +kind: Policy +metadata: + name: cache-policy2 +spec: + cache: + cacheZoneName: "mycache2" #Required + cacheZoneSize: "18m" #Required + allowedCodes: ["any"] #Optional + allowedMethods: ["GET"] #Optional + time: "15m" #Optional + # cachePurgeAllow: [""] + overrideUpstreamCache: true + levels: "1:2" # Optional, e.g. "1:2" or "2:2". This controls the number of subdirectory levels used for cache storage. \ No newline at end of file diff --git a/examples/custom-resources/cache-policy/cafe-virtual-server.yaml b/examples/custom-resources/cache-policy/cafe-virtual-server.yaml index 4d3d636730..8b8d83d73f 100644 --- a/examples/custom-resources/cache-policy/cafe-virtual-server.yaml +++ b/examples/custom-resources/cache-policy/cafe-virtual-server.yaml @@ -25,6 +25,8 @@ spec: - path: /tea action: pass: tea + policies: + - name: cache-policy2 - path: /coffee action: pass: coffee diff --git a/examples/shared-cache/secure-shared-cache-pvc.yaml b/examples/shared-cache/secure-shared-cache-pvc.yaml new file mode 100644 index 0000000000..a1a97eb0f6 --- /dev/null +++ b/examples/shared-cache/secure-shared-cache-pvc.yaml @@ -0,0 +1,25 @@ +apiVersion: v1 +kind: PersistentVolume +metadata: + name: nginx-shared-cache +spec: + storageClassName: manual + capacity: + storage: 1Gi + accessModes: + - ReadWriteOnce + hostPath: + path: "/tmp/" + +--- +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: nginx-shared-cache +spec: + storageClassName: manual + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi \ No newline at end of file diff --git a/internal/configs/version2/http.go b/internal/configs/version2/http.go index 4d44d1d568..ce87df6555 100644 --- a/internal/configs/version2/http.go +++ b/internal/configs/version2/http.go @@ -484,9 +484,10 @@ type Variable struct { // CacheZone defines a proxy cache zone configuration. type CacheZone struct { - Name string - Size string - Path string + Name string + Size string + Path string + Levels string // Optional. Directory hierarchy for cache files (e.g., "1:2", "2:2", "1:2:2") } // Cache defines cache configuration for locations. @@ -497,5 +498,6 @@ type Cache struct { Valid map[string]string // map for codes to time AllowedMethods []string // HTTP methods allowed for caching based on proxy_cache_methods CachePurgeAllow []string // IPs/CIDRs allowed to purge cache - OverrideUpstreamCache bool // Controls whether to override upstream cache headers + OverrideUpstreamCache bool // Controls whether to override upstream cache headers + Levels string // Optional. Directory hierarchy for cache files (e.g., "1:2", "2:2", "1:2:2") } diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index 62b1fb31bd..d025b72e33 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -71,7 +71,7 @@ limit_req_zone {{ $z.Key }} zone={{ $z.ZoneName }}:{{ $z.ZoneSize }} rate={{ $z. {{- end }} {{- range $c := .CacheZones }} -proxy_cache_path {{ $c.Path }} keys_zone={{ $c.Name }}:{{ $c.Size }}; +proxy_cache_path {{ $c.Path }}{{ if $c.Levels }} levels={{ $c.Levels }}{{ end }} keys_zone={{ $c.Name }}:{{ $c.Size }}; {{- end }} {{- range $m := .StatusMatches }} diff --git a/internal/configs/version2/nginx.virtualserver.tmpl b/internal/configs/version2/nginx.virtualserver.tmpl index e42f3a5b02..f7d229929e 100644 --- a/internal/configs/version2/nginx.virtualserver.tmpl +++ b/internal/configs/version2/nginx.virtualserver.tmpl @@ -41,7 +41,7 @@ limit_req_zone {{ $z.Key }} zone={{ $z.ZoneName }}:{{ $z.ZoneSize }} rate={{ $z. {{- end }} {{- range $c := .CacheZones }} -proxy_cache_path {{ $c.Path }} keys_zone={{ $c.Name }}:{{ $c.Size }}; +proxy_cache_path {{ $c.Path }}{{ if $c.Levels }} levels={{ $c.Levels }}{{ end }} keys_zone={{ $c.Name }}:{{ $c.Size }}; {{- end }} {{- $s := .Server }} diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 186ec8953f..b0fed7219c 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -1915,7 +1915,7 @@ func generateCacheConfig(cache *conf_v1.Cache, vsNamespace, vsName, ownerNamespa // Policy is applied to VirtualServerRoute, include both VS and owner info uniqueZoneName = fmt.Sprintf("%s_%s_%s_%s_%s", vsNamespace, vsName, ownerNamespace, ownerName, cache.CacheZoneName) } - + cacheConfig := &version2.Cache{ ZoneName: uniqueZoneName, Time: cache.Time, @@ -1924,6 +1924,7 @@ func generateCacheConfig(cache *conf_v1.Cache, vsNamespace, vsName, ownerNamespa CachePurgeAllow: cache.CachePurgeAllow, ZoneSize: cache.CacheZoneSize, OverrideUpstreamCache: cache.OverrideUpstreamCache, + Levels: cache.Levels, // Pass Levels from Cache to CacheZone } // Convert allowed codes to proxy_cache_valid entries @@ -1953,9 +1954,10 @@ func addCacheZone(cacheZones *[]version2.CacheZone, cache *version2.Cache) { } cacheZone := version2.CacheZone{ - Name: cache.ZoneName, - Size: zoneSize, - Path: fmt.Sprintf("/var/cache/nginx/%s", cache.ZoneName), + Name: cache.ZoneName, + Size: zoneSize, + Path: fmt.Sprintf("/var/cache/nginx/%s", cache.ZoneName), + Levels: cache.Levels, // Pass Levels from Cache to CacheZone } // Check for duplicates diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index 1671e404ec..7217ba9659 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -736,11 +736,12 @@ type SuppliedIn struct { // Cache defines a cache policy for proxy caching. type Cache struct { - CacheZoneName string `json:"cacheZoneName"` - CacheZoneSize string `json:"cacheZoneSize"` - AllowedCodes []intstr.IntOrString `json:"allowedCodes,omitempty"` - AllowedMethods []string `json:"allowedMethods,omitempty"` - Time string `json:"time,omitempty"` - CachePurgeAllow []string `json:"cachePurgeAllow,omitempty"` - OverrideUpstreamCache bool `json:"overrideUpstreamCache,omitempty"` + CacheZoneName string `json:"cacheZoneName"` + CacheZoneSize string `json:"cacheZoneSize"` + AllowedCodes []intstr.IntOrString `json:"allowedCodes,omitempty"` + AllowedMethods []string `json:"allowedMethods,omitempty"` + Time string `json:"time,omitempty"` + CachePurgeAllow []string `json:"cachePurgeAllow,omitempty"` + OverrideUpstreamCache bool `json:"overrideUpstreamCache,omitempty"` + Levels string `json:"levels,omitempty"` // Optional. Directory hierarchy for cache files (e.g., "1:2", "2:2", "1:2:2") } From 4c9653250d5947c8261d5928127db34ba176c90f Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 9 Jul 2025 11:17:56 +0000 Subject: [PATCH 09/30] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- examples/custom-resources/cache-policy/cache.yaml | 2 +- .../custom-resources/cache-policy/cafe-virtual-server.yaml | 5 +---- examples/shared-cache/secure-shared-cache-pvc.yaml | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/examples/custom-resources/cache-policy/cache.yaml b/examples/custom-resources/cache-policy/cache.yaml index 220dbbe36e..9d66f190f5 100644 --- a/examples/custom-resources/cache-policy/cache.yaml +++ b/examples/custom-resources/cache-policy/cache.yaml @@ -25,4 +25,4 @@ spec: time: "15m" #Optional # cachePurgeAllow: [""] overrideUpstreamCache: true - levels: "1:2" # Optional, e.g. "1:2" or "2:2". This controls the number of subdirectory levels used for cache storage. \ No newline at end of file + levels: "1:2" # Optional, e.g. "1:2" or "2:2". This controls the number of subdirectory levels used for cache storage. diff --git a/examples/custom-resources/cache-policy/cafe-virtual-server.yaml b/examples/custom-resources/cache-policy/cafe-virtual-server.yaml index 8b8d83d73f..0b49021d8f 100644 --- a/examples/custom-resources/cache-policy/cafe-virtual-server.yaml +++ b/examples/custom-resources/cache-policy/cafe-virtual-server.yaml @@ -4,7 +4,7 @@ metadata: name: cafe spec: server-snippets: | - add_header X-Cache-Status $upstream_cache_status; + add_header X-Cache-Status $upstream_cache_status; # This header will show the cache status for each request, e.g. X-Cache-Status: MISS or X-Cache-Status: HIT. # The cache status can be "HIT", "MISS", "EXPIRED", etc. # This is useful for debugging and monitoring cache behavior but not required for cache functionality. @@ -30,6 +30,3 @@ spec: - path: /coffee action: pass: coffee - - - diff --git a/examples/shared-cache/secure-shared-cache-pvc.yaml b/examples/shared-cache/secure-shared-cache-pvc.yaml index a1a97eb0f6..99ac0225d9 100644 --- a/examples/shared-cache/secure-shared-cache-pvc.yaml +++ b/examples/shared-cache/secure-shared-cache-pvc.yaml @@ -22,4 +22,4 @@ spec: - ReadWriteOnce resources: requests: - storage: 1Gi \ No newline at end of file + storage: 1Gi From 0857c5b1d0c637dab2bd4daff7ebcfc19fec87e1 Mon Sep 17 00:00:00 2001 From: Venktesh Date: Thu, 7 Aug 2025 16:50:54 +0100 Subject: [PATCH 10/30] add unit tests --- internal/configs/virtualserver_test.go | 760 ++++++++++++++++++ pkg/apis/configuration/v1/types.go | 4 +- .../configuration/validation/policy_test.go | 454 +++++++++++ 3 files changed, 1216 insertions(+), 2 deletions(-) diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index 106e4162e2..0cdc576d04 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -22,6 +22,7 @@ import ( api_v1 "k8s.io/api/core/v1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" ) func createPointerFromBool(b bool) *bool { @@ -11327,13 +11328,559 @@ func TestGenerateVirtualServerConfigWithRateLimitGroupsWarning(t *testing.T) { } } +func TestGenerateVirtualServerConfigCache(t *testing.T) { + t.Parallel() + + tests := []struct { + msg string + virtualServerEx VirtualServerEx + expected version2.VirtualServerConfig + }{ + { + msg: "cache policy at vs spec level", + virtualServerEx: VirtualServerEx{ + VirtualServer: &conf_v1.VirtualServer{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "cafe", + Namespace: "default", + }, + Spec: conf_v1.VirtualServerSpec{ + Host: "cafe.example.com", + Policies: []conf_v1.PolicyReference{ + { + Name: "cache-policy", + }, + }, + Upstreams: []conf_v1.Upstream{ + { + Name: "tea", + Service: "tea-svc", + Port: 80, + }, + { + Name: "coffee", + Service: "coffee-svc", + Port: 80, + }, + }, + Routes: []conf_v1.Route{ + { + Path: "/tea", + Action: &conf_v1.Action{ + Pass: "tea", + }, + }, + { + Path: "/coffee", + Action: &conf_v1.Action{ + Pass: "coffee", + }, + }, + }, + }, + }, + Policies: map[string]*conf_v1.Policy{ + "default/cache-policy": { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "cache-policy", + Namespace: "default", + }, + Spec: conf_v1.PolicySpec{ + Cache: &conf_v1.Cache{ + CacheZoneName: "my-cache", + CacheZoneSize: "10m", + Time: "1h", + }, + }, + }, + }, + Endpoints: map[string][]string{ + "default/tea-svc:80": { + "10.0.0.20:80", + }, + "default/coffee-svc:80": { + "10.0.0.30:80", + }, + }, + }, + expected: version2.VirtualServerConfig{ + Upstreams: []version2.Upstream{ + { + UpstreamLabels: version2.UpstreamLabels{ + Service: "coffee-svc", + ResourceType: "virtualserver", + ResourceName: "cafe", + ResourceNamespace: "default", + }, + Name: "vs_default_cafe_coffee", + Servers: []version2.UpstreamServer{ + { + Address: "10.0.0.30:80", + }, + }, + }, + { + UpstreamLabels: version2.UpstreamLabels{ + Service: "tea-svc", + ResourceType: "virtualserver", + ResourceName: "cafe", + ResourceNamespace: "default", + }, + Name: "vs_default_cafe_tea", + Servers: []version2.UpstreamServer{ + { + Address: "10.0.0.20:80", + }, + }, + }, + }, + HTTPSnippets: []string{}, + LimitReqZones: []version2.LimitReqZone{}, + CacheZones: []version2.CacheZone{ + { + Name: "default_cafe_my-cache", + Size: "10m", + Path: "/var/cache/nginx/default_cafe_my-cache", + Levels: "", + }, + }, + Server: version2.Server{ + ServerName: "cafe.example.com", + StatusZone: "cafe.example.com", + ServerTokens: "off", + VSNamespace: "default", + VSName: "cafe", + Cache: &version2.Cache{ + ZoneName: "default_cafe_my-cache", + ZoneSize: "10m", + Time: "1h", + Valid: map[string]string{}, + AllowedMethods: nil, + CachePurgeAllow: nil, + OverrideUpstreamCache: false, + Levels: "", + }, + Locations: []version2.Location{ + { + Path: "/tea", + ProxyPass: "http://vs_default_cafe_tea", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, + ProxySSLName: "tea-svc.default.svc", + ProxyPassRequestHeaders: true, + ProxySetHeaders: []version2.Header{{Name: "Host", Value: "$host"}}, + ServiceName: "tea-svc", + }, + { + Path: "/coffee", + ProxyPass: "http://vs_default_cafe_coffee", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, + ProxySSLName: "coffee-svc.default.svc", + ProxyPassRequestHeaders: true, + ProxySetHeaders: []version2.Header{{Name: "Host", Value: "$host"}}, + ServiceName: "coffee-svc", + }, + }, + }, + }, + }, + { + msg: "cache policy at route level", + virtualServerEx: VirtualServerEx{ + VirtualServer: &conf_v1.VirtualServer{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "cafe", + Namespace: "default", + }, + Spec: conf_v1.VirtualServerSpec{ + Host: "cafe.example.com", + Upstreams: []conf_v1.Upstream{ + { + Name: "tea", + Service: "tea-svc", + Port: 80, + }, + { + Name: "coffee", + Service: "coffee-svc", + Port: 80, + }, + }, + Routes: []conf_v1.Route{ + { + Path: "/tea", + Policies: []conf_v1.PolicyReference{ + { + Name: "route-cache-policy", + }, + }, + Action: &conf_v1.Action{ + Pass: "tea", + }, + }, + { + Path: "/coffee", + Action: &conf_v1.Action{ + Pass: "coffee", + }, + }, + }, + }, + }, + Policies: map[string]*conf_v1.Policy{ + "default/route-cache-policy": { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "route-cache-policy", + Namespace: "default", + }, + Spec: conf_v1.PolicySpec{ + Cache: &conf_v1.Cache{ + CacheZoneName: "route-cache", + CacheZoneSize: "5m", + Time: "30m", + AllowedCodes: []intstr.IntOrString{ + intstr.FromInt(200), + intstr.FromInt(404), + }, + }, + }, + }, + }, + Endpoints: map[string][]string{ + "default/tea-svc:80": { + "10.0.0.20:80", + }, + "default/coffee-svc:80": { + "10.0.0.30:80", + }, + }, + }, + expected: version2.VirtualServerConfig{ + Upstreams: []version2.Upstream{ + { + UpstreamLabels: version2.UpstreamLabels{ + Service: "coffee-svc", + ResourceType: "virtualserver", + ResourceName: "cafe", + ResourceNamespace: "default", + }, + Name: "vs_default_cafe_coffee", + Servers: []version2.UpstreamServer{ + { + Address: "10.0.0.30:80", + }, + }, + }, + { + UpstreamLabels: version2.UpstreamLabels{ + Service: "tea-svc", + ResourceType: "virtualserver", + ResourceName: "cafe", + ResourceNamespace: "default", + }, + Name: "vs_default_cafe_tea", + Servers: []version2.UpstreamServer{ + { + Address: "10.0.0.20:80", + }, + }, + }, + }, + HTTPSnippets: []string{}, + LimitReqZones: []version2.LimitReqZone{}, + CacheZones: []version2.CacheZone{ + { + Name: "default_cafe_route-cache", + Size: "5m", + Path: "/var/cache/nginx/default_cafe_route-cache", + Levels: "", + }, + }, + Server: version2.Server{ + ServerName: "cafe.example.com", + StatusZone: "cafe.example.com", + ServerTokens: "off", + VSNamespace: "default", + VSName: "cafe", + Locations: []version2.Location{ + { + Path: "/tea", + ProxyPass: "http://vs_default_cafe_tea", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, + ProxySSLName: "tea-svc.default.svc", + ProxyPassRequestHeaders: true, + ProxySetHeaders: []version2.Header{{Name: "Host", Value: "$host"}}, + ServiceName: "tea-svc", + Cache: &version2.Cache{ + ZoneName: "default_cafe_route-cache", + ZoneSize: "5m", + Time: "30m", + Valid: map[string]string{"200": "30m", "404": "30m"}, + AllowedMethods: nil, + CachePurgeAllow: nil, + OverrideUpstreamCache: false, + Levels: "", + }, + }, + { + Path: "/coffee", + ProxyPass: "http://vs_default_cafe_coffee", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, + ProxySSLName: "coffee-svc.default.svc", + ProxyPassRequestHeaders: true, + ProxySetHeaders: []version2.Header{{Name: "Host", Value: "$host"}}, + ServiceName: "coffee-svc", + }, + }, + }, + }, + }, + { + msg: "cache policy at VSR subroute level", + virtualServerEx: VirtualServerEx{ + VirtualServer: &conf_v1.VirtualServer{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "cafe", + Namespace: "default", + }, + Spec: conf_v1.VirtualServerSpec{ + Host: "cafe.example.com", + Upstreams: []conf_v1.Upstream{ + { + Name: "tea", + Service: "tea-svc", + Port: 80, + }, + }, + Routes: []conf_v1.Route{ + { + Path: "/tea", + Route: "default/tea-vsr", + }, + }, + }, + }, + VirtualServerRoutes: []*conf_v1.VirtualServerRoute{ + { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "tea-vsr", + Namespace: "default", + }, + Spec: conf_v1.VirtualServerRouteSpec{ + Host: "cafe.example.com", + Upstreams: []conf_v1.Upstream{ + { + Name: "tea-v1", + Service: "tea-v1-svc", + Port: 80, + }, + { + Name: "tea-v2", + Service: "tea-v2-svc", + Port: 80, + }, + }, + Subroutes: []conf_v1.Route{ + { + Path: "/tea/v1", + Policies: []conf_v1.PolicyReference{ + { + Name: "vsr-cache-policy", + }, + }, + Action: &conf_v1.Action{ + Pass: "tea-v1", + }, + }, + { + Path: "/tea/v2", + Action: &conf_v1.Action{ + Pass: "tea-v2", + }, + }, + }, + }, + }, + }, + Policies: map[string]*conf_v1.Policy{ + "default/vsr-cache-policy": { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "vsr-cache-policy", + Namespace: "default", + }, + Spec: conf_v1.PolicySpec{ + Cache: &conf_v1.Cache{ + CacheZoneName: "vsr-cache", + CacheZoneSize: "20m", + Time: "2h", + OverrideUpstreamCache: true, + CachePurgeAllow: []string{"127.0.0.1"}, + }, + }, + }, + }, + Endpoints: map[string][]string{ + "default/tea-svc:80": { + "10.0.0.20:80", + }, + "default/tea-v1-svc:80": { + "10.0.0.21:80", + }, + "default/tea-v2-svc:80": { + "10.0.0.22:80", + }, + }, + }, + expected: version2.VirtualServerConfig{ + Upstreams: []version2.Upstream{ + { + UpstreamLabels: version2.UpstreamLabels{ + Service: "tea-svc", + ResourceType: "virtualserver", + ResourceName: "cafe", + ResourceNamespace: "default", + }, + Name: "vs_default_cafe_tea", + Servers: []version2.UpstreamServer{ + { + Address: "10.0.0.20:80", + }, + }, + }, + { + UpstreamLabels: version2.UpstreamLabels{ + Service: "tea-v1-svc", + ResourceType: "virtualserverroute", + ResourceName: "tea-vsr", + ResourceNamespace: "default", + }, + Name: "vs_default_cafe_vsr_default_tea-vsr_tea-v1", + Servers: []version2.UpstreamServer{ + { + Address: "10.0.0.21:80", + }, + }, + }, + { + UpstreamLabels: version2.UpstreamLabels{ + Service: "tea-v2-svc", + ResourceType: "virtualserverroute", + ResourceName: "tea-vsr", + ResourceNamespace: "default", + }, + Name: "vs_default_cafe_vsr_default_tea-vsr_tea-v2", + Servers: []version2.UpstreamServer{ + { + Address: "10.0.0.22:80", + }, + }, + }, + }, + HTTPSnippets: []string{}, + LimitReqZones: []version2.LimitReqZone{}, + CacheZones: []version2.CacheZone{ + { + Name: "default_cafe_default_tea-vsr_vsr-cache", + Size: "20m", + Path: "/var/cache/nginx/default_cafe_default_tea-vsr_vsr-cache", + Levels: "", + }, + }, + Server: version2.Server{ + ServerName: "cafe.example.com", + StatusZone: "cafe.example.com", + ServerTokens: "off", + VSNamespace: "default", + VSName: "cafe", + Locations: []version2.Location{ + { + Path: "/tea/v1", + ProxyPass: "http://vs_default_cafe_vsr_default_tea-vsr_tea-v1", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, + ProxySSLName: "tea-v1-svc.default.svc", + ProxyPassRequestHeaders: true, + ProxySetHeaders: []version2.Header{{Name: "Host", Value: "$host"}}, + ServiceName: "tea-v1-svc", + IsVSR: true, + VSRName: "tea-vsr", + VSRNamespace: "default", + Cache: &version2.Cache{ + ZoneName: "default_cafe_default_tea-vsr_vsr-cache", + ZoneSize: "20m", + Time: "2h", + Valid: map[string]string{}, + AllowedMethods: nil, + CachePurgeAllow: []string{"127.0.0.1"}, + OverrideUpstreamCache: true, + Levels: "", + }, + }, + { + Path: "/tea/v2", + ProxyPass: "http://vs_default_cafe_vsr_default_tea-vsr_tea-v2", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, + ProxySSLName: "tea-v2-svc.default.svc", + ProxyPassRequestHeaders: true, + ProxySetHeaders: []version2.Header{{Name: "Host", Value: "$host"}}, + ServiceName: "tea-v2-svc", + IsVSR: true, + VSRName: "tea-vsr", + VSRNamespace: "default", + }, + }, + }, + }, + }, + } + + baseCfgParams := ConfigParams{ + Context: context.Background(), + ServerTokens: "off", + } + + vsc := newVirtualServerConfigurator( + &baseCfgParams, + false, + false, + &StaticConfigParams{}, + false, + &fakeBV, + ) + + for _, test := range tests { + result, warnings := vsc.GenerateVirtualServerConfig(&test.virtualServerEx, nil, nil) + + if diff := cmp.Diff(test.expected, result); diff != "" { + t.Errorf("GenerateVirtualServerConfig() mismatch (-want +got):\n%s", diff) + t.Error(test.msg) + } + + if len(warnings) != 0 { + t.Errorf("GenerateVirtualServerConfig returned warnings: %v", warnings) + } + } +} + func TestGeneratePolicies(t *testing.T) { t.Parallel() + ctx := context.Background() ownerDetails := policyOwnerDetails{ owner: nil, // nil is OK for the unit test ownerNamespace: "default", vsNamespace: "default", vsName: "test", + ownerName: "test", } mTLSCertPath := "/etc/nginx/secrets/default-ingress-mtls-secret-ca.crt" mTLSCrlPath := "/etc/nginx/secrets/default-ingress-mtls-secret-ca.crl" @@ -12281,6 +12828,219 @@ func TestGeneratePolicies(t *testing.T) { }, msg: "WAF reference", }, + { + policyRefs: []conf_v1.PolicyReference{ + { + Name: "cache-policy-basic", + Namespace: "default", + }, + }, + policies: map[string]*conf_v1.Policy{ + "default/cache-policy-basic": { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "cache-policy-basic", + Namespace: "default", + }, + Spec: conf_v1.PolicySpec{ + Cache: &conf_v1.Cache{ + CacheZoneName: "basic-cache", + CacheZoneSize: "10m", + }, + }, + }, + }, + expected: policiesCfg{ + Context: ctx, + Cache: &version2.Cache{ + ZoneName: "default_test_basic-cache", + ZoneSize: "10m", + Valid: map[string]string{}, + }, + }, + msg: "basic cache policy reference", + }, + { + policyRefs: []conf_v1.PolicyReference{ + { + Name: "cache-policy-full", + Namespace: "default", + }, + }, + policies: map[string]*conf_v1.Policy{ + "default/cache-policy-full": { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "cache-policy-full", + Namespace: "default", + }, + Spec: conf_v1.PolicySpec{ + Cache: &conf_v1.Cache{ + CacheZoneName: "full-cache", + CacheZoneSize: "100m", + AllowedCodes: []intstr.IntOrString{intstr.FromString("any")}, + AllowedMethods: []string{"GET", "HEAD", "POST"}, + Time: "1h", + OverrideUpstreamCache: true, + Levels: "1:2", + }, + }, + }, + }, + expected: policiesCfg{ + Context: ctx, + Cache: &version2.Cache{ + ZoneName: "default_test_full-cache", + ZoneSize: "100m", + Time: "1h", + Valid: map[string]string{"any": "1h"}, + AllowedMethods: []string{"GET", "HEAD", "POST"}, + OverrideUpstreamCache: true, + Levels: "1:2", + }, + }, + msg: "full cache policy with all options", + }, + { + policyRefs: []conf_v1.PolicyReference{ + { + Name: "cache-policy-status-codes", + Namespace: "default", + }, + }, + policies: map[string]*conf_v1.Policy{ + "default/cache-policy-status-codes": { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "cache-policy-status-codes", + Namespace: "default", + }, + Spec: conf_v1.PolicySpec{ + Cache: &conf_v1.Cache{ + CacheZoneName: "status-cache", + CacheZoneSize: "50m", + AllowedCodes: []intstr.IntOrString{ + intstr.FromInt(200), + intstr.FromInt(301), + intstr.FromInt(404), + }, + Time: "30m", + }, + }, + }, + }, + expected: policiesCfg{ + Context: ctx, + Cache: &version2.Cache{ + ZoneName: "default_test_status-cache", + ZoneSize: "50m", + Time: "30m", + Valid: map[string]string{ + "200": "30m", + "301": "30m", + "404": "30m", + }, + }, + }, + msg: "cache policy with specific status codes", + }, + { + policyRefs: []conf_v1.PolicyReference{ + { + Name: "cache-policy-methods", + Namespace: "default", + }, + }, + policies: map[string]*conf_v1.Policy{ + "default/cache-policy-methods": { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "cache-policy-methods", + Namespace: "default", + }, + Spec: conf_v1.PolicySpec{ + Cache: &conf_v1.Cache{ + CacheZoneName: "methods-cache", + CacheZoneSize: "25m", + AllowedMethods: []string{"GET", "HEAD"}, + Levels: "2:2", + }, + }, + }, + }, + expected: policiesCfg{ + Context: ctx, + Cache: &version2.Cache{ + ZoneName: "default_test_methods-cache", + ZoneSize: "25m", + Valid: map[string]string{}, + AllowedMethods: []string{"GET", "HEAD"}, + Levels: "2:2", + }, + }, + msg: "cache policy with allowed methods and levels", + }, + { + policyRefs: []conf_v1.PolicyReference{ + { + Name: "cache-policy-purge", + Namespace: "default", + }, + }, + policies: map[string]*conf_v1.Policy{ + "default/cache-policy-purge": { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "cache-policy-purge", + Namespace: "default", + }, + Spec: conf_v1.PolicySpec{ + Cache: &conf_v1.Cache{ + CacheZoneName: "purge-cache", + CacheZoneSize: "75m", + CachePurgeAllow: []string{"192.168.1.0/24", "10.0.0.1"}, + }, + }, + }, + }, + expected: policiesCfg{ + Context: ctx, + Cache: &version2.Cache{ + ZoneName: "default_test_purge-cache", + ZoneSize: "75m", + Valid: map[string]string{}, + CachePurgeAllow: []string{"192.168.1.0/24", "10.0.0.1"}, + }, + }, + msg: "cache policy with purge allow IPs", + }, + { + policyRefs: []conf_v1.PolicyReference{ + { + Name: "cache-policy-implicit", + }, + }, + policies: map[string]*conf_v1.Policy{ + "default/cache-policy-implicit": { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "cache-policy-implicit", + Namespace: "default", + }, + Spec: conf_v1.PolicySpec{ + Cache: &conf_v1.Cache{ + CacheZoneName: "implicit-cache", + CacheZoneSize: "15m", + Time: "45m", + }, + }, + }, + }, + expected: policiesCfg{ + Context: ctx, + Cache: &version2.Cache{ + ZoneName: "default_test_implicit-cache", + ZoneSize: "15m", + Time: "45m", + Valid: map[string]string{}, + }, + }, + msg: "implicit cache policy reference", + }, } vsc := newVirtualServerConfigurator(&ConfigParams{Context: ctx}, false, false, &StaticConfigParams{}, false, &fakeBV) diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index a8f1125419..f36ef1957e 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -789,8 +789,8 @@ type PolicySpec struct { WAF *WAF `json:"waf"` // The API Key policy configures NGINX to authorize requests which provide a valid API Key in a specified header or query param. APIKey *APIKey `json:"apiKey"` - // The Cache Key defines a cache policy for proxy caching - Cache *Cache `json:"cache"` + // The Cache Key defines a cache policy for proxy caching + Cache *Cache `json:"cache"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/apis/configuration/validation/policy_test.go b/pkg/apis/configuration/validation/policy_test.go index 4dc5b2b739..b9f41f7f76 100644 --- a/pkg/apis/configuration/validation/policy_test.go +++ b/pkg/apis/configuration/validation/policy_test.go @@ -1,9 +1,11 @@ package validation import ( + "strings" "testing" v1 "github.com/nginx/kubernetes-ingress/pkg/apis/configuration/v1" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/validation/field" ) @@ -2424,3 +2426,455 @@ func TestValidateWAF_FailsOnInvalidApLogBundle(t *testing.T) { }) } } + +func TestValidateCache(t *testing.T) { + t.Parallel() + + validCacheTests := []struct { + name string + cache *v1.Cache + isPlus bool + expected bool + }{ + { + name: "valid cache with basic configuration", + cache: &v1.Cache{ + CacheZoneName: "mycache", + CacheZoneSize: "10m", + }, + isPlus: false, + expected: true, + }, + { + name: "valid cache with all options", + cache: &v1.Cache{ + CacheZoneName: "mycache", + CacheZoneSize: "100m", + AllowedCodes: []intstr.IntOrString{intstr.FromString("any")}, + AllowedMethods: []string{"GET", "HEAD", "POST"}, + Time: "1h", + OverrideUpstreamCache: true, + Levels: "1:2", + }, + isPlus: false, + expected: true, + }, + { + name: "valid cache with specific status codes", + cache: &v1.Cache{ + CacheZoneName: "statuscache", + CacheZoneSize: "50m", + AllowedCodes: []intstr.IntOrString{ + intstr.FromInt(200), + intstr.FromInt(301), + intstr.FromInt(404), + }, + Time: "30m", + }, + isPlus: false, + expected: true, + }, + { + name: "valid cache with purge (NGINX Plus)", + cache: &v1.Cache{ + CacheZoneName: "purgecache", + CacheZoneSize: "20m", + CachePurgeAllow: []string{"192.168.1.0/24", "10.0.0.1"}, + }, + isPlus: true, + expected: true, + }, + { + name: "valid cache with GET method only", + cache: &v1.Cache{ + CacheZoneName: "getcache", + CacheZoneSize: "15m", + AllowedMethods: []string{"GET"}, + }, + isPlus: false, + expected: true, + }, + { + name: "valid cache with complex levels", + cache: &v1.Cache{ + CacheZoneName: "levelcache", + CacheZoneSize: "25m", + Levels: "2:2", + }, + isPlus: false, + expected: true, + }, + } + + for _, test := range validCacheTests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + allErrs := validateCache(test.cache, field.NewPath("cache"), test.isPlus) + + if test.expected && len(allErrs) > 0 { + t.Errorf("Expected no validation errors for valid cache, got: %v", allErrs) + } + if !test.expected && len(allErrs) == 0 { + t.Errorf("Expected validation errors for invalid cache, got none") + } + }) + } + + invalidCacheTests := []struct { + name string + cache *v1.Cache + isPlus bool + expectedError string + }{ + { + name: "invalid allowed code string", + cache: &v1.Cache{ + CacheZoneName: "invalidcache", + CacheZoneSize: "10m", + AllowedCodes: []intstr.IntOrString{intstr.FromString("invalid")}, + Time: "1h", + }, + isPlus: false, + expectedError: "only the string 'any' is allowed", + }, + { + name: "invalid status code too low", + cache: &v1.Cache{ + CacheZoneName: "invalidcache", + CacheZoneSize: "10m", + AllowedCodes: []intstr.IntOrString{intstr.FromInt(99)}, + Time: "1h", + }, + isPlus: false, + expectedError: "HTTP status code must be between 100 and 599", + }, + { + name: "invalid status code too high", + cache: &v1.Cache{ + CacheZoneName: "invalidcache", + CacheZoneSize: "10m", + AllowedCodes: []intstr.IntOrString{intstr.FromInt(600)}, + Time: "1h", + }, + isPlus: false, + expectedError: "HTTP status code must be between 100 and 599", + }, + { + name: "cache purge not allowed on OSS", + cache: &v1.Cache{ + CacheZoneName: "purgeoss", + CacheZoneSize: "10m", + CachePurgeAllow: []string{"192.168.1.1"}, + }, + isPlus: false, + expectedError: "cache purge is only supported in NGINX Plus", + }, + { + name: "invalid IP address in purge allow", + cache: &v1.Cache{ + CacheZoneName: "invalidip", + CacheZoneSize: "10m", + CachePurgeAllow: []string{"invalid-ip"}, + }, + isPlus: true, + expectedError: "must be a valid IP address or CIDR", + }, + { + name: "invalid CIDR in purge allow", + cache: &v1.Cache{ + CacheZoneName: "invalidcidr", + CacheZoneSize: "10m", + CachePurgeAllow: []string{"192.168.1.1/99"}, + }, + isPlus: true, + expectedError: "must be a valid IP address or CIDR", + }, + } + + for _, test := range invalidCacheTests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + allErrs := validateCache(test.cache, field.NewPath("cache"), test.isPlus) + + if len(allErrs) == 0 { + t.Errorf("Expected validation error containing '%s', got no errors", test.expectedError) + return + } + + found := false + for _, err := range allErrs { + if strings.Contains(err.Detail, test.expectedError) { + found = true + break + } + } + + if !found { + t.Errorf("Expected validation error containing '%s', got: %v", test.expectedError, allErrs) + } + }) + } +} + +func TestValidatePolicy_CachePolicy(t *testing.T) { + t.Parallel() + + validPolicyTests := []struct { + name string + policy *v1.Policy + isPlus bool + }{ + { + name: "valid cache policy basic", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "basiccache", + CacheZoneSize: "10m", + }, + }, + }, + isPlus: false, + }, + { + name: "valid cache policy with all options", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "fullcache", + CacheZoneSize: "100m", + AllowedCodes: []intstr.IntOrString{intstr.FromString("any")}, + AllowedMethods: []string{"GET", "HEAD", "POST"}, + Time: "2h", + OverrideUpstreamCache: true, + Levels: "1:2", + }, + }, + }, + isPlus: false, + }, + { + name: "valid cache policy with purge (NGINX Plus)", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "purgecache", + CacheZoneSize: "50m", + CachePurgeAllow: []string{"10.0.0.0/8", "192.168.1.100"}, + }, + }, + }, + isPlus: true, + }, + } + + for _, test := range validPolicyTests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + err := ValidatePolicy(test.policy, test.isPlus, false, false) + if err != nil { + t.Errorf("Expected valid cache policy, got error: %v", err) + } + }) + } + + invalidPolicyTests := []struct { + name string + policy *v1.Policy + isPlus bool + }{ + { + name: "multiple policies defined (cache + access control)", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "cache1", + CacheZoneSize: "10m", + }, + AccessControl: &v1.AccessControl{ + Allow: []string{"127.0.0.1"}, + }, + }, + }, + isPlus: false, + }, + { + name: "multiple policies defined (cache + rate limit)", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "cache2", + CacheZoneSize: "10m", + }, + RateLimit: &v1.RateLimit{ + Rate: "10r/s", + }, + }, + }, + isPlus: false, + }, + { + name: "invalid cache with bad status code", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "invalidcache", + CacheZoneSize: "10m", + AllowedCodes: []intstr.IntOrString{intstr.FromInt(1000)}, + Time: "1h", + }, + }, + }, + isPlus: false, + }, + } + + for _, test := range invalidPolicyTests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + err := ValidatePolicy(test.policy, test.isPlus, false, false) + if err == nil { + t.Errorf("Expected invalid cache policy to return error, got none") + } + }) + } +} + +func TestValidatePolicy_CacheRequiredFields(t *testing.T) { + t.Parallel() + + // Test the CRD-level validation that requires time when allowedCodes is specified + validPolicies := []struct { + name string + policy *v1.Policy + }{ + { + name: "no allowedCodes, no time required", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "notime", + CacheZoneSize: "10m", + }, + }, + }, + }, + { + name: "allowedCodes with time", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "withtime", + CacheZoneSize: "10m", + AllowedCodes: []intstr.IntOrString{intstr.FromString("any")}, + Time: "1h", + }, + }, + }, + }, + { + name: "specific status codes with time", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "statuscodes", + CacheZoneSize: "10m", + AllowedCodes: []intstr.IntOrString{ + intstr.FromInt(200), + intstr.FromInt(404), + }, + Time: "30m", + }, + }, + }, + }, + } + + for _, test := range validPolicies { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + err := ValidatePolicy(test.policy, false, false, false) + if err != nil { + t.Errorf("Expected valid cache policy, got error: %v", err) + } + }) + } +} + +func TestValidatePolicy_CacheMethodsValidation(t *testing.T) { + t.Parallel() + + // Test allowed methods validation + tests := []struct { + name string + allowedMethods []string + expectValid bool + }{ + { + name: "valid GET method", + allowedMethods: []string{"GET"}, + expectValid: true, + }, + { + name: "valid HEAD method", + allowedMethods: []string{"HEAD"}, + expectValid: true, + }, + { + name: "valid POST method", + allowedMethods: []string{"POST"}, + expectValid: true, + }, + { + name: "valid multiple methods", + allowedMethods: []string{"GET", "HEAD", "POST"}, + expectValid: true, + }, + { + name: "empty methods (should be valid)", + allowedMethods: []string{}, + expectValid: true, + }, + { + name: "nil methods (should be valid)", + allowedMethods: nil, + expectValid: true, + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + policy := &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "methodcache", + CacheZoneSize: "10m", + AllowedMethods: test.allowedMethods, + }, + }, + } + + err := ValidatePolicy(policy, false, false, false) + + if test.expectValid && err != nil { + t.Errorf("Expected valid policy for methods %v, got error: %v", test.allowedMethods, err) + } + if !test.expectValid && err == nil { + t.Errorf("Expected invalid policy for methods %v, got no error", test.allowedMethods) + } + }) + } +} From 1b635a2605100589af896f8ac22613ca1a2e4955 Mon Sep 17 00:00:00 2001 From: Venktesh Date: Thu, 7 Aug 2025 17:32:38 +0100 Subject: [PATCH 11/30] fix kubebuilder issues anbd add more validation to policy.go --- charts/nginx-ingress/values.yaml | 2 +- config/crd/bases/k8s.nginx.org_policies.yaml | 123 ++++++++++--------- deploy/crds.yaml | 58 +++++++++ docs/crd/k8s.nginx.org_policies.md | 9 ++ internal/k8s/controller_test.go | 4 +- pkg/apis/configuration/v1/types.go | 39 ++++-- pkg/apis/configuration/validation/policy.go | 57 +++++++++ 7 files changed, 225 insertions(+), 67 deletions(-) diff --git a/charts/nginx-ingress/values.yaml b/charts/nginx-ingress/values.yaml index 65251d3345..29f4cd5fa7 100644 --- a/charts/nginx-ingress/values.yaml +++ b/charts/nginx-ingress/values.yaml @@ -169,7 +169,7 @@ controller: ## Enables shared cache across multiple pods using an external persistent volume ## When enabled, the /var/cache/nginx directory will be mounted from a PVC instead of using emptyDir ## User must create and configure a PVC with appropriate access mode - enableShared: true + enableShared: false ## The name of the PersistentVolumeClaim to use for shared cache, should match the name of the PVC created by the user sharedPVCName: "nginx-shared-cache" diff --git a/config/crd/bases/k8s.nginx.org_policies.yaml b/config/crd/bases/k8s.nginx.org_policies.yaml index 302ab12a58..f99ef56b63 100644 --- a/config/crd/bases/k8s.nginx.org_policies.yaml +++ b/config/crd/bases/k8s.nginx.org_policies.yaml @@ -109,6 +109,73 @@ spec: otherwise the secret will be rejected as invalid. type: string type: object + cache: + description: The Cache Key defines a cache policy for proxy caching + properties: + allowedCodes: + description: AllowedCodes defines which response codes should + be cached. Can be HTTP status codes (100-599) or the string + "any" to cache all responses. + items: + anyOf: + - type: integer + - type: string + x-kubernetes-int-or-string: true + type: array + allowedMethods: + description: AllowedMethods defines which HTTP methods should + be cached. Only GET, HEAD, and POST are supported by NGINX proxy_cache_methods + directive. GET and HEAD are always cached by default. + enum: + - GET + - HEAD + - POST + items: + type: string + type: array + cachePurgeAllow: + description: CachePurgeAllow defines IP addresses allowed to purge + cache (NGINX Plus only). + items: + type: string + type: array + cacheZoneName: + description: CacheZoneName defines the name of the cache zone. + maxLength: 64 + pattern: ^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$ + type: string + cacheZoneSize: + description: CacheZoneSize defines the size of the cache zone. + pattern: ^[0-9]+[kmg]$ + type: string + levels: + description: Levels defines the cache directory hierarchy levels + for storing cached files (e.g., "1:2", "2:2", "1:2:2"). + pattern: ^[12](?::[12]){0,2}$ + type: string + overrideUpstreamCache: + default: false + description: OverrideUpstreamCache controls whether to override + upstream cache headers (using proxy_ignore_headers directive). + type: boolean + time: + description: Time defines the default cache time (required when + allowedCodes is specified). + pattern: ^[0-9]+[smhd]$ + type: string + required: + - cacheZoneName + - cacheZoneSize + anyOf: + - not: + required: + - allowedCodes + - allOf: + - required: + - allowedCodes + - required: + - time + type: object egressMTLS: description: The EgressMTLS policy configures upstreams authentication and certificate verification. @@ -458,62 +525,6 @@ spec: type: object type: array type: object - cache: - description: Cache defines a cache policy for proxy caching. - properties: - allowedCodes: - description: AllowedCodes defines which response codes should be cached. Can be HTTP status codes (100-599) or the string "any" to cache all responses. - items: - x-kubernetes-int-or-string: true - type: array - allowedMethods: - description: "AllowedMethods defines which HTTP methods should be cached. Only GET, HEAD, and POST are supported by NGINX proxy_cache_methods directive. GET and HEAD are always cached by default." - items: - type: string - enum: - - "GET" - - "HEAD" - - "POST" - type: array - cachePurgeAllow: - description: CachePurgeAllow defines IP addresses allowed to purge cache (NGINX Plus only) - items: - type: string - type: array - cacheZoneName: - description: CacheZoneName defines the name of the cache zone - type: string - pattern: '^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$' - maxLength: 64 - cacheZoneSize: - description: CacheZoneSize defines the size of the cache zone - type: string - pattern: '^[0-9]+[kmg]$' - overrideUpstreamCache: - description: OverrideUpstreamCache controls whether to override upstream cache headers (using proxy_ignore_headers directive) - type: boolean - default: false - time: - description: Time defines the default cache time (required when allowedCodes is specified) - type: string - pattern: '^[0-9]+[smhd]$' - levels: - description: Directory hierarchy for cache files. Controls the number of subdirectory levels used for cache storage. - type: string - pattern: '^[12](?::[12]){0,2}$' - required: - - cacheZoneName - - cacheZoneSize - anyOf: - - not: - required: - - allowedCodes - - allOf: - - required: - - allowedCodes - - required: - - time - type: object type: object status: description: the status of the Policy resource diff --git a/deploy/crds.yaml b/deploy/crds.yaml index 5b80d9dff9..ee38300953 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -280,6 +280,64 @@ spec: otherwise the secret will be rejected as invalid. type: string type: object + cache: + description: The Cache Key defines a cache policy for proxy caching + properties: + allowedCodes: + description: AllowedCodes defines which response codes should + be cached. Can be HTTP status codes (100-599) or the string + "any" to cache all responses. + items: + anyOf: + - type: integer + - type: string + x-kubernetes-int-or-string: true + type: array + allowedMethods: + description: AllowedMethods defines which HTTP methods should + be cached. Only GET, HEAD, and POST are supported by NGINX proxy_cache_methods + directive. GET and HEAD are always cached by default. + enum: + - GET + - HEAD + - POST + items: + type: string + type: array + cachePurgeAllow: + description: CachePurgeAllow defines IP addresses allowed to purge + cache (NGINX Plus only). + items: + type: string + type: array + cacheZoneName: + description: CacheZoneName defines the name of the cache zone. + maxLength: 64 + pattern: ^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$ + type: string + cacheZoneSize: + description: CacheZoneSize defines the size of the cache zone. + pattern: ^[0-9]+[kmg]$ + type: string + levels: + description: Levels defines the cache directory hierarchy levels + for storing cached files (e.g., "1:2", "2:2", "1:2:2"). + pattern: ^[12](?::[12]){0,2}$ + type: string + overrideUpstreamCache: + default: false + description: OverrideUpstreamCache controls whether to override + upstream cache headers (using proxy_ignore_headers directive). + type: boolean + time: + description: Time defines the default cache time (required when + allowedCodes is specified). + pattern: ^[0-9]+[smhd]$ + type: string + required: + - cacheZoneName + - cacheZoneSize + type: object egressMTLS: description: The EgressMTLS policy configures upstreams authentication and certificate verification. diff --git a/docs/crd/k8s.nginx.org_policies.md b/docs/crd/k8s.nginx.org_policies.md index d8c8bb0f1c..1006f8f639 100644 --- a/docs/crd/k8s.nginx.org_policies.md +++ b/docs/crd/k8s.nginx.org_policies.md @@ -26,6 +26,15 @@ The `.spec` object supports the following fields: | `basicAuth` | `object` | The basic auth policy configures NGINX to authenticate client requests using HTTP Basic authentication credentials. | | `basicAuth.realm` | `string` | The realm for the basic authentication. | | `basicAuth.secret` | `string` | The name of the Kubernetes secret that stores the Htpasswd configuration. It must be in the same namespace as the Policy resource. The secret must be of the type nginx.org/htpasswd, and the config must be stored in the secret under the key htpasswd, otherwise the secret will be rejected as invalid. | +| `cache` | `object` | The Cache Key defines a cache policy for proxy caching | +| `cache.allowedCodes` | `array` | AllowedCodes defines which response codes should be cached. Can be HTTP status codes (100-599) or the string "any" to cache all responses. | +| `cache.allowedMethods` | `array[string]` | AllowedMethods defines which HTTP methods should be cached. Only GET, HEAD, and POST are supported by NGINX proxy_cache_methods directive. GET and HEAD are always cached by default. Allowed values: `"GET"`, `"HEAD"`, `"POST"`. | +| `cache.cachePurgeAllow` | `array[string]` | CachePurgeAllow defines IP addresses allowed to purge cache (NGINX Plus only). | +| `cache.cacheZoneName` | `string` | CacheZoneName defines the name of the cache zone. | +| `cache.cacheZoneSize` | `string` | CacheZoneSize defines the size of the cache zone. | +| `cache.levels` | `string` | Levels defines the cache directory hierarchy levels for storing cached files (e.g., "1:2", "2:2", "1:2:2"). | +| `cache.overrideUpstreamCache` | `boolean` | OverrideUpstreamCache controls whether to override upstream cache headers (using proxy_ignore_headers directive). | +| `cache.time` | `string` | Time defines the default cache time (required when allowedCodes is specified). | | `egressMTLS` | `object` | The EgressMTLS policy configures upstreams authentication and certificate verification. | | `egressMTLS.ciphers` | `string` | Specifies the enabled ciphers for requests to an upstream HTTPS server. The default is DEFAULT. | | `egressMTLS.protocols` | `string` | Specifies the protocols for requests to an upstream HTTPS server. The default is TLSv1 TLSv1.1 TLSv1.2. | diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index e23443bd2d..93b8eacdc2 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -2109,7 +2109,7 @@ func TestGetPoliciesGlobalWatch(t *testing.T) { expectedPolicies := []*conf_v1.Policy{validPolicy} expectedErrors := []error{ - errors.New("policy default/invalid-policy is invalid: spec: Invalid value: \"\": must specify exactly one of: `accessControl`, `rateLimit`, `ingressMTLS`, `egressMTLS`, `basicAuth`, `apiKey`, `jwt`, `oidc`, `waf`"), + errors.New("policy default/invalid-policy is invalid: spec: Invalid value: \"\": must specify exactly one of: `accessControl`, `rateLimit`, `ingressMTLS`, `egressMTLS`, `basicAuth`, `apiKey`, `cache`, `jwt`, `oidc`, `waf`"), errors.New("policy nginx-ingress/valid-policy doesn't exist"), errors.New("failed to get policy nginx-ingress/some-policy: GetByKey error"), errors.New("referenced policy default/valid-policy-ingress-class has incorrect ingress class: test-class (controller ingress class: )"), @@ -2207,7 +2207,7 @@ func TestGetPoliciesNamespacedWatch(t *testing.T) { expectedPolicies := []*conf_v1.Policy{validPolicy} expectedErrors := []error{ - errors.New("policy default/invalid-policy is invalid: spec: Invalid value: \"\": must specify exactly one of: `accessControl`, `rateLimit`, `ingressMTLS`, `egressMTLS`, `basicAuth`, `apiKey`, `jwt`, `oidc`, `waf`"), + errors.New("policy default/invalid-policy is invalid: spec: Invalid value: \"\": must specify exactly one of: `accessControl`, `rateLimit`, `ingressMTLS`, `egressMTLS`, `basicAuth`, `apiKey`, `cache`, `jwt`, `oidc`, `waf`"), errors.New("failed to get namespace nginx-ingress"), errors.New("referenced policy default/valid-policy-ingress-class has incorrect ingress class: test-class (controller ingress class: )"), } diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index f36ef1957e..74364405df 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -1009,12 +1009,35 @@ type SuppliedIn struct { // Cache defines a cache policy for proxy caching. type Cache struct { - CacheZoneName string `json:"cacheZoneName"` - CacheZoneSize string `json:"cacheZoneSize"` - AllowedCodes []intstr.IntOrString `json:"allowedCodes,omitempty"` - AllowedMethods []string `json:"allowedMethods,omitempty"` - Time string `json:"time,omitempty"` - CachePurgeAllow []string `json:"cachePurgeAllow,omitempty"` - OverrideUpstreamCache bool `json:"overrideUpstreamCache,omitempty"` - Levels string `json:"levels,omitempty"` // Optional. Directory hierarchy for cache files (e.g., "1:2", "2:2", "1:2:2") + // +kubebuilder:validation:Required + // +kubebuilder:validation:Pattern=`^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$` + // +kubebuilder:validation:MaxLength=64 + // CacheZoneName defines the name of the cache zone. + CacheZoneName string `json:"cacheZoneName"` + // +kubebuilder:validation:Required + // +kubebuilder:validation:Pattern=`^[0-9]+[kmg]$` + // CacheZoneSize defines the size of the cache zone. + CacheZoneSize string `json:"cacheZoneSize"` + // +kubebuilder:validation:Optional + // AllowedCodes defines which response codes should be cached. Can be HTTP status codes (100-599) or the string "any" to cache all responses. + AllowedCodes []intstr.IntOrString `json:"allowedCodes,omitempty"` + // +kubebuilder:validation:Optional + // +kubebuilder:validation:Enum=GET;HEAD;POST + // AllowedMethods defines which HTTP methods should be cached. Only GET, HEAD, and POST are supported by NGINX proxy_cache_methods directive. GET and HEAD are always cached by default. + AllowedMethods []string `json:"allowedMethods,omitempty"` + // +kubebuilder:validation:Optional + // +kubebuilder:validation:Pattern=`^[0-9]+[smhd]$` + // Time defines the default cache time (required when allowedCodes is specified). + Time string `json:"time,omitempty"` + // +kubebuilder:validation:Optional + // CachePurgeAllow defines IP addresses allowed to purge cache (NGINX Plus only). + CachePurgeAllow []string `json:"cachePurgeAllow,omitempty"` + // +kubebuilder:validation:Optional + // +kubebuilder:default=false + // OverrideUpstreamCache controls whether to override upstream cache headers (using proxy_ignore_headers directive). + OverrideUpstreamCache bool `json:"overrideUpstreamCache,omitempty"` + // +kubebuilder:validation:Optional + // +kubebuilder:validation:Pattern=`^[12](?::[12]){0,2}$` + // Levels defines the cache directory hierarchy levels for storing cached files (e.g., "1:2", "2:2", "1:2:2"). + Levels string `json:"levels,omitempty"` } diff --git a/pkg/apis/configuration/validation/policy.go b/pkg/apis/configuration/validation/policy.go index df771582b6..8fc66a2702 100644 --- a/pkg/apis/configuration/validation/policy.go +++ b/pkg/apis/configuration/validation/policy.go @@ -428,6 +428,39 @@ func validateLogConfs(logs []*v1.SecurityLog, fieldPath *field.Path, bundleMode func validateCache(cache *v1.Cache, fieldPath *field.Path, isPlus bool) field.ErrorList { allErrs := field.ErrorList{} + // Validate required fields + if cache.CacheZoneName == "" { + allErrs = append(allErrs, field.Required(fieldPath.Child("cacheZoneName"), "cache zone name is required")) + } else { + // Validate cache zone name pattern: ^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$ + cacheZoneNamePattern := regexp.MustCompile(`^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$`) + if !cacheZoneNamePattern.MatchString(cache.CacheZoneName) { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("cacheZoneName"), cache.CacheZoneName, "cache zone name must start with a lowercase letter and contain only letters, numbers, and underscores")) + } + // Validate cache zone name max length + if len(cache.CacheZoneName) > 64 { + allErrs = append(allErrs, field.TooLong(fieldPath.Child("cacheZoneName"), cache.CacheZoneName, 64)) + } + } + + if cache.CacheZoneSize == "" { + allErrs = append(allErrs, field.Required(fieldPath.Child("cacheZoneSize"), "cache zone size is required")) + } else { + // Validate cache zone size pattern: ^[0-9]+[kmg]$ + cacheZoneSizePattern := regexp.MustCompile(`^[0-9]+[kmg]$`) + if !cacheZoneSizePattern.MatchString(cache.CacheZoneSize) { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("cacheZoneSize"), cache.CacheZoneSize, "cache zone size must be a number followed by k, m, or g (e.g., '10m', '1g')")) + } + } + + // Validate anyOf constraint: either allowedCodes is not specified, or both allowedCodes and time are specified + hasAllowedCodes := len(cache.AllowedCodes) > 0 + hasTime := cache.Time != "" + + if hasAllowedCodes && !hasTime { + allErrs = append(allErrs, field.Required(fieldPath.Child("time"), "time is required when allowedCodes is specified")) + } + // Validate allowed codes if provided for i, code := range cache.AllowedCodes { if code.Type == intstr.String { @@ -444,6 +477,30 @@ func validateCache(cache *v1.Cache, fieldPath *field.Path, isPlus bool) field.Er } } + // Validate allowed methods if provided + validMethods := map[string]bool{"GET": true, "HEAD": true, "POST": true} + for i, method := range cache.AllowedMethods { + if !validMethods[method] { + allErrs = append(allErrs, field.NotSupported(fieldPath.Child("allowedMethods").Index(i), method, []string{"GET", "HEAD", "POST"})) + } + } + + // Validate time pattern if provided: ^[0-9]+[smhd]$ + if cache.Time != "" { + timePattern := regexp.MustCompile(`^[0-9]+[smhd]$`) + if !timePattern.MatchString(cache.Time) { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("time"), cache.Time, "time must be a number followed by s, m, h, or d (e.g., '30s', '5m', '1h', '1d')")) + } + } + + // Validate levels pattern if provided: ^[12](?::[12]){0,2}$ + if cache.Levels != "" { + levelsPattern := regexp.MustCompile(`^[12](?::[12]){0,2}$`) + if !levelsPattern.MatchString(cache.Levels) { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("levels"), cache.Levels, "levels must be in format like '1:2' or '1:2:2' with values of 1 or 2")) + } + } + // Validate cache purge allow IPs if provided if len(cache.CachePurgeAllow) > 0 { // Check if NGINX Plus is required for cache purge From 948c8165f909568298941c983816164a0783cfdd Mon Sep 17 00:00:00 2001 From: Venktesh Date: Thu, 7 Aug 2025 17:46:45 +0100 Subject: [PATCH 12/30] use CEL rules --- config/crd/bases/k8s.nginx.org_policies.yaml | 12 +++--------- deploy/crds.yaml | 3 +++ pkg/apis/configuration/v1/types.go | 1 + 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/config/crd/bases/k8s.nginx.org_policies.yaml b/config/crd/bases/k8s.nginx.org_policies.yaml index f99ef56b63..5425e0257c 100644 --- a/config/crd/bases/k8s.nginx.org_policies.yaml +++ b/config/crd/bases/k8s.nginx.org_policies.yaml @@ -166,16 +166,10 @@ spec: required: - cacheZoneName - cacheZoneSize - anyOf: - - not: - required: - - allowedCodes - - allOf: - - required: - - allowedCodes - - required: - - time type: object + x-kubernetes-validations: + - message: time is required when allowedCodes is specified + rule: '!has(self.allowedCodes) || (has(self.allowedCodes) && has(self.time))' egressMTLS: description: The EgressMTLS policy configures upstreams authentication and certificate verification. diff --git a/deploy/crds.yaml b/deploy/crds.yaml index ee38300953..525dd9887b 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -338,6 +338,9 @@ spec: - cacheZoneName - cacheZoneSize type: object + x-kubernetes-validations: + - message: time is required when allowedCodes is specified + rule: '!has(self.allowedCodes) || (has(self.allowedCodes) && has(self.time))' egressMTLS: description: The EgressMTLS policy configures upstreams authentication and certificate verification. diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index 74364405df..e0e294e4e4 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -1008,6 +1008,7 @@ type SuppliedIn struct { } // Cache defines a cache policy for proxy caching. +// +kubebuilder:validation:XValidation:rule="!has(self.allowedCodes) || (has(self.allowedCodes) && has(self.time))",message="time is required when allowedCodes is specified" type Cache struct { // +kubebuilder:validation:Required // +kubebuilder:validation:Pattern=`^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$` From 4aa91e1dc58806e4e4e87368a6cd0722e192f14b Mon Sep 17 00:00:00 2001 From: Venktesh Date: Thu, 7 Aug 2025 18:29:06 +0100 Subject: [PATCH 13/30] reduce cyclomatic complexity and update codegen --- .../configuration/v1/zz_generated.deepcopy.go | 37 ++ pkg/apis/configuration/validation/policy.go | 43 +- .../configuration/validation/policy_test.go | 416 +++++++++++++++--- 3 files changed, 444 insertions(+), 52 deletions(-) diff --git a/pkg/apis/configuration/v1/zz_generated.deepcopy.go b/pkg/apis/configuration/v1/zz_generated.deepcopy.go index 75049fea24..943fb76a03 100644 --- a/pkg/apis/configuration/v1/zz_generated.deepcopy.go +++ b/pkg/apis/configuration/v1/zz_generated.deepcopy.go @@ -7,6 +7,7 @@ package v1 import ( runtime "k8s.io/apimachinery/pkg/runtime" + intstr "k8s.io/apimachinery/pkg/util/intstr" ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. @@ -183,6 +184,37 @@ func (in *BasicAuth) DeepCopy() *BasicAuth { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Cache) DeepCopyInto(out *Cache) { + *out = *in + if in.AllowedCodes != nil { + in, out := &in.AllowedCodes, &out.AllowedCodes + *out = make([]intstr.IntOrString, len(*in)) + copy(*out, *in) + } + if in.AllowedMethods != nil { + in, out := &in.AllowedMethods, &out.AllowedMethods + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.CachePurgeAllow != nil { + in, out := &in.CachePurgeAllow, &out.CachePurgeAllow + *out = make([]string, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Cache. +func (in *Cache) DeepCopy() *Cache { + if in == nil { + return nil + } + out := new(Cache) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CertManager) DeepCopyInto(out *CertManager) { *out = *in @@ -731,6 +763,11 @@ func (in *PolicySpec) DeepCopyInto(out *PolicySpec) { *out = new(APIKey) (*in).DeepCopyInto(*out) } + if in.Cache != nil { + in, out := &in.Cache, &out.Cache + *out = new(Cache) + (*in).DeepCopyInto(*out) + } return } diff --git a/pkg/apis/configuration/validation/policy.go b/pkg/apis/configuration/validation/policy.go index 8fc66a2702..874e09f98a 100644 --- a/pkg/apis/configuration/validation/policy.go +++ b/pkg/apis/configuration/validation/policy.go @@ -428,7 +428,26 @@ func validateLogConfs(logs []*v1.SecurityLog, fieldPath *field.Path, bundleMode func validateCache(cache *v1.Cache, fieldPath *field.Path, isPlus bool) field.ErrorList { allErrs := field.ErrorList{} - // Validate required fields + // Validate required fields and basic constraints + allErrs = append(allErrs, validateCacheRequiredFields(cache, fieldPath)...) + + // Validate conditional logic (anyOf constraint) + allErrs = append(allErrs, validateCacheConditionalFields(cache, fieldPath)...) + + // Validate field formats and patterns + allErrs = append(allErrs, validateCacheFieldFormats(cache, fieldPath)...) + + // Validate NGINX Plus specific features + allErrs = append(allErrs, validateCachePlusFeatures(cache, fieldPath, isPlus)...) + + return allErrs +} + +// validateCacheRequiredFields validates required fields and their constraints +func validateCacheRequiredFields(cache *v1.Cache, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + // Validate cacheZoneName if cache.CacheZoneName == "" { allErrs = append(allErrs, field.Required(fieldPath.Child("cacheZoneName"), "cache zone name is required")) } else { @@ -443,6 +462,7 @@ func validateCache(cache *v1.Cache, fieldPath *field.Path, isPlus bool) field.Er } } + // Validate cacheZoneSize if cache.CacheZoneSize == "" { allErrs = append(allErrs, field.Required(fieldPath.Child("cacheZoneSize"), "cache zone size is required")) } else { @@ -453,6 +473,13 @@ func validateCache(cache *v1.Cache, fieldPath *field.Path, isPlus bool) field.Er } } + return allErrs +} + +// validateCacheConditionalFields validates the anyOf constraint for allowedCodes and time +func validateCacheConditionalFields(cache *v1.Cache, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + // Validate anyOf constraint: either allowedCodes is not specified, or both allowedCodes and time are specified hasAllowedCodes := len(cache.AllowedCodes) > 0 hasTime := cache.Time != "" @@ -461,6 +488,13 @@ func validateCache(cache *v1.Cache, fieldPath *field.Path, isPlus bool) field.Er allErrs = append(allErrs, field.Required(fieldPath.Child("time"), "time is required when allowedCodes is specified")) } + return allErrs +} + +// validateCacheFieldFormats validates field formats and patterns +func validateCacheFieldFormats(cache *v1.Cache, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + // Validate allowed codes if provided for i, code := range cache.AllowedCodes { if code.Type == intstr.String { @@ -501,6 +535,13 @@ func validateCache(cache *v1.Cache, fieldPath *field.Path, isPlus bool) field.Er } } + return allErrs +} + +// validateCachePlusFeatures validates NGINX Plus specific features, such as cache purge allow IPs +func validateCachePlusFeatures(cache *v1.Cache, fieldPath *field.Path, isPlus bool) field.ErrorList { + allErrs := field.ErrorList{} + // Validate cache purge allow IPs if provided if len(cache.CachePurgeAllow) > 0 { // Check if NGINX Plus is required for cache purge diff --git a/pkg/apis/configuration/validation/policy_test.go b/pkg/apis/configuration/validation/policy_test.go index b9f41f7f76..ca8bc973bf 100644 --- a/pkg/apis/configuration/validation/policy_test.go +++ b/pkg/apis/configuration/validation/policy_test.go @@ -2430,20 +2430,22 @@ func TestValidateWAF_FailsOnInvalidApLogBundle(t *testing.T) { func TestValidateCache(t *testing.T) { t.Parallel() - validCacheTests := []struct { - name string - cache *v1.Cache - isPlus bool - expected bool + tests := []struct { + name string + cache *v1.Cache + isPlus bool + expectValid bool + expectedError string }{ + // Valid cache configurations { name: "valid cache with basic configuration", cache: &v1.Cache{ CacheZoneName: "mycache", CacheZoneSize: "10m", }, - isPlus: false, - expected: true, + isPlus: false, + expectValid: true, }, { name: "valid cache with all options", @@ -2456,8 +2458,8 @@ func TestValidateCache(t *testing.T) { OverrideUpstreamCache: true, Levels: "1:2", }, - isPlus: false, - expected: true, + isPlus: false, + expectValid: true, }, { name: "valid cache with specific status codes", @@ -2471,8 +2473,8 @@ func TestValidateCache(t *testing.T) { }, Time: "30m", }, - isPlus: false, - expected: true, + isPlus: false, + expectValid: true, }, { name: "valid cache with purge (NGINX Plus)", @@ -2481,8 +2483,8 @@ func TestValidateCache(t *testing.T) { CacheZoneSize: "20m", CachePurgeAllow: []string{"192.168.1.0/24", "10.0.0.1"}, }, - isPlus: true, - expected: true, + isPlus: true, + expectValid: true, }, { name: "valid cache with GET method only", @@ -2491,8 +2493,8 @@ func TestValidateCache(t *testing.T) { CacheZoneSize: "15m", AllowedMethods: []string{"GET"}, }, - isPlus: false, - expected: true, + isPlus: false, + expectValid: true, }, { name: "valid cache with complex levels", @@ -2501,33 +2503,210 @@ func TestValidateCache(t *testing.T) { CacheZoneSize: "25m", Levels: "2:2", }, - isPlus: false, - expected: true, + isPlus: false, + expectValid: true, + }, + { + name: "valid cache zone name with underscores", + cache: &v1.Cache{ + CacheZoneName: "valid_cache_name", + CacheZoneSize: "10m", + }, + isPlus: false, + expectValid: true, + }, + { + name: "valid cache zone size with k unit", + cache: &v1.Cache{ + CacheZoneName: "validname", + CacheZoneSize: "1024k", + }, + isPlus: false, + expectValid: true, + }, + { + name: "valid cache zone size with g unit", + cache: &v1.Cache{ + CacheZoneName: "validname", + CacheZoneSize: "2g", + }, + isPlus: false, + expectValid: true, + }, + { + name: "valid time in seconds", + cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + Time: "30s", + }, + isPlus: false, + expectValid: true, + }, + { + name: "valid time in days", + cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + Time: "1d", + }, + isPlus: false, + expectValid: true, + }, + { + name: "valid allowedCodes 'any' with time", + cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + AllowedCodes: []intstr.IntOrString{intstr.FromString("any")}, + Time: "30m", + }, + isPlus: false, + expectValid: true, + }, + { + name: "valid boundary status codes", + cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + AllowedCodes: []intstr.IntOrString{ + intstr.FromInt(100), // minimum + intstr.FromInt(599), // maximum + }, + Time: "1h", + }, + isPlus: false, + expectValid: true, + }, + { + name: "valid IPv6 address in purge allow", + cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + CachePurgeAllow: []string{"2001:db8::1"}, + }, + isPlus: true, + expectValid: true, + }, + { + name: "valid IPv6 CIDR in purge allow", + cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + CachePurgeAllow: []string{"2001:db8::/32"}, + }, + isPlus: true, + expectValid: true, }, - } - - for _, test := range validCacheTests { - test := test - t.Run(test.name, func(t *testing.T) { - t.Parallel() - allErrs := validateCache(test.cache, field.NewPath("cache"), test.isPlus) + // Invalid cache configurations - Required fields + { + name: "missing cache zone name", + cache: &v1.Cache{ + CacheZoneSize: "10m", + }, + isPlus: false, + expectValid: false, + expectedError: "cache zone name is required", + }, + { + name: "missing cache zone size", + cache: &v1.Cache{ + CacheZoneName: "validname", + }, + isPlus: false, + expectValid: false, + expectedError: "cache zone size is required", + }, + { + name: "invalid cache zone name - starts with uppercase", + cache: &v1.Cache{ + CacheZoneName: "InvalidName", + CacheZoneSize: "10m", + }, + isPlus: false, + expectValid: false, + expectedError: "cache zone name must start with a lowercase letter", + }, + { + name: "invalid cache zone name - starts with number", + cache: &v1.Cache{ + CacheZoneName: "1invalidname", + CacheZoneSize: "10m", + }, + isPlus: false, + expectValid: false, + expectedError: "cache zone name must start with a lowercase letter", + }, + { + name: "invalid cache zone name - special characters", + cache: &v1.Cache{ + CacheZoneName: "invalid-name", + CacheZoneSize: "10m", + }, + isPlus: false, + expectValid: false, + expectedError: "cache zone name must start with a lowercase letter", + }, + { + name: "cache zone name too long", + cache: &v1.Cache{ + CacheZoneName: "a" + strings.Repeat("x", 64), // 65 characters + CacheZoneSize: "10m", + }, + isPlus: false, + expectValid: false, + expectedError: "may not be more than 64 bytes", + }, + { + name: "invalid cache zone size - no unit", + cache: &v1.Cache{ + CacheZoneName: "validname", + CacheZoneSize: "10", + }, + isPlus: false, + expectValid: false, + expectedError: "cache zone size must be a number followed by k, m, or g", + }, + { + name: "invalid cache zone size - invalid unit", + cache: &v1.Cache{ + CacheZoneName: "validname", + CacheZoneSize: "10x", + }, + isPlus: false, + expectValid: false, + expectedError: "cache zone size must be a number followed by k, m, or g", + }, - if test.expected && len(allErrs) > 0 { - t.Errorf("Expected no validation errors for valid cache, got: %v", allErrs) - } - if !test.expected && len(allErrs) == 0 { - t.Errorf("Expected validation errors for invalid cache, got none") - } - }) - } + // Invalid cache configurations - Conditional fields + { + name: "allowedCodes without time", + cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + AllowedCodes: []intstr.IntOrString{intstr.FromInt(200)}, + }, + isPlus: false, + expectValid: false, + expectedError: "time is required when allowedCodes is specified", + }, + { + name: "multiple allowedCodes without time", + cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + AllowedCodes: []intstr.IntOrString{ + intstr.FromInt(200), + intstr.FromInt(404), + }, + }, + isPlus: false, + expectValid: false, + expectedError: "time is required when allowedCodes is specified", + }, - invalidCacheTests := []struct { - name string - cache *v1.Cache - isPlus bool - expectedError string - }{ + // Invalid cache configurations - Field formats { name: "invalid allowed code string", cache: &v1.Cache{ @@ -2537,6 +2716,7 @@ func TestValidateCache(t *testing.T) { Time: "1h", }, isPlus: false, + expectValid: false, expectedError: "only the string 'any' is allowed", }, { @@ -2548,6 +2728,7 @@ func TestValidateCache(t *testing.T) { Time: "1h", }, isPlus: false, + expectValid: false, expectedError: "HTTP status code must be between 100 and 599", }, { @@ -2559,8 +2740,99 @@ func TestValidateCache(t *testing.T) { Time: "1h", }, isPlus: false, + expectValid: false, expectedError: "HTTP status code must be between 100 and 599", }, + { + name: "invalid HTTP method", + cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + AllowedMethods: []string{"PUT"}, + }, + isPlus: false, + expectValid: false, + expectedError: "supported values:", + }, + { + name: "mixed valid and invalid methods", + cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + AllowedMethods: []string{"GET", "DELETE"}, + }, + isPlus: false, + expectValid: false, + expectedError: "supported values:", + }, + { + name: "invalid time without unit", + cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + Time: "30", + }, + isPlus: false, + expectValid: false, + expectedError: "time must be a number followed by s, m, h, or d", + }, + { + name: "invalid time with invalid unit", + cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + Time: "30x", + }, + isPlus: false, + expectValid: false, + expectedError: "time must be a number followed by s, m, h, or d", + }, + { + name: "invalid levels with value 3", + cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + Levels: "1:3", + }, + isPlus: false, + expectValid: false, + expectedError: "levels must be in format like '1:2' or '1:2:2' with values of 1 or 2", + }, + { + name: "invalid levels with value 0", + cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + Levels: "0:1", + }, + isPlus: false, + expectValid: false, + expectedError: "levels must be in format like '1:2' or '1:2:2' with values of 1 or 2", + }, + { + name: "invalid levels too many parts", + cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + Levels: "1:2:1:2", + }, + isPlus: false, + expectValid: false, + expectedError: "levels must be in format like '1:2' or '1:2:2' with values of 1 or 2", + }, + { + name: "invalid levels format", + cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + Levels: "1-2", + }, + isPlus: false, + expectValid: false, + expectedError: "levels must be in format like '1:2' or '1:2:2' with values of 1 or 2", + }, + + // Invalid cache configurations - NGINX Plus features { name: "cache purge not allowed on OSS", cache: &v1.Cache{ @@ -2569,6 +2841,7 @@ func TestValidateCache(t *testing.T) { CachePurgeAllow: []string{"192.168.1.1"}, }, isPlus: false, + expectValid: false, expectedError: "cache purge is only supported in NGINX Plus", }, { @@ -2579,6 +2852,7 @@ func TestValidateCache(t *testing.T) { CachePurgeAllow: []string{"invalid-ip"}, }, isPlus: true, + expectValid: false, expectedError: "must be a valid IP address or CIDR", }, { @@ -2589,32 +2863,72 @@ func TestValidateCache(t *testing.T) { CachePurgeAllow: []string{"192.168.1.1/99"}, }, isPlus: true, + expectValid: false, + expectedError: "must be a valid IP address or CIDR", + }, + { + name: "mixed valid and invalid IPs in purge allow", + cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + CachePurgeAllow: []string{"192.168.1.1", "not-an-ip"}, + }, + isPlus: true, + expectValid: false, + expectedError: "must be a valid IP address or CIDR", + }, + { + name: "empty string in IP list", + cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + CachePurgeAllow: []string{"192.168.1.1", ""}, + }, + isPlus: true, + expectValid: false, + expectedError: "must be a valid IP address or CIDR", + }, + { + name: "hostname instead of IP in purge allow", + cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + CachePurgeAllow: []string{"example.com"}, + }, + isPlus: true, + expectValid: false, expectedError: "must be a valid IP address or CIDR", }, } - for _, test := range invalidCacheTests { + for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { t.Parallel() allErrs := validateCache(test.cache, field.NewPath("cache"), test.isPlus) - if len(allErrs) == 0 { - t.Errorf("Expected validation error containing '%s', got no errors", test.expectedError) - return - } + if test.expectValid { + if len(allErrs) > 0 { + t.Errorf("Expected no validation errors for valid cache, got: %v", allErrs) + } + } else { + if len(allErrs) == 0 { + t.Errorf("Expected validation error containing '%s', got no errors", test.expectedError) + return + } - found := false - for _, err := range allErrs { - if strings.Contains(err.Detail, test.expectedError) { - found = true - break + found := false + for _, err := range allErrs { + if strings.Contains(err.Detail, test.expectedError) { + found = true + break + } } - } - if !found { - t.Errorf("Expected validation error containing '%s', got: %v", test.expectedError, allErrs) + if !found { + t.Errorf("Expected validation error containing '%s', got: %v", test.expectedError, allErrs) + } } }) } From c1ccf4c448223fd1350d1aea7268c166182bd62c Mon Sep 17 00:00:00 2001 From: Venktesh Date: Fri, 8 Aug 2025 16:44:48 +0100 Subject: [PATCH 14/30] add more CEL based validation in CRDs --- config/crd/bases/k8s.nginx.org_policies.yaml | 29 ++++++++++++++------ deploy/crds.yaml | 29 ++++++++++++++------ docs/crd/k8s.nginx.org_policies.md | 2 +- pkg/apis/configuration/v1/types.go | 11 ++++---- 4 files changed, 46 insertions(+), 25 deletions(-) diff --git a/config/crd/bases/k8s.nginx.org_policies.yaml b/config/crd/bases/k8s.nginx.org_policies.yaml index 5425e0257c..7f17dc64ef 100644 --- a/config/crd/bases/k8s.nginx.org_policies.yaml +++ b/config/crd/bases/k8s.nginx.org_policies.yaml @@ -126,13 +126,12 @@ spec: description: AllowedMethods defines which HTTP methods should be cached. Only GET, HEAD, and POST are supported by NGINX proxy_cache_methods directive. GET and HEAD are always cached by default. - enum: - - GET - - HEAD - - POST items: type: string type: array + x-kubernetes-validations: + - message: 'allowed methods must be one of: GET, HEAD, POST' + rule: self.all(method, method in ['GET', 'HEAD', 'POST']) cachePurgeAllow: description: CachePurgeAllow defines IP addresses allowed to purge cache (NGINX Plus only). @@ -141,18 +140,27 @@ spec: type: array cacheZoneName: description: CacheZoneName defines the name of the cache zone. - maxLength: 64 - pattern: ^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$ type: string + x-kubernetes-validations: + - message: cache zone name must be 1-64 characters, start with + lowercase letter, and contain only alphanumeric characters + and underscores + rule: size(self) <= 64 && self.matches('^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$') cacheZoneSize: description: CacheZoneSize defines the size of the cache zone. - pattern: ^[0-9]+[kmg]$ type: string + x-kubernetes-validations: + - message: cache zone size must be a number followed by k, m, + or g (e.g., '10m', '1g') + rule: self.matches('^[0-9]+[kmg]$') levels: description: Levels defines the cache directory hierarchy levels for storing cached files (e.g., "1:2", "2:2", "1:2:2"). - pattern: ^[12](?::[12]){0,2}$ type: string + x-kubernetes-validations: + - message: levels must be in format like '1:2', '2:2', or '1:2:2' + with values 1 or 2 + rule: self.matches('^[12](?::[12]){0,2}$') overrideUpstreamCache: default: false description: OverrideUpstreamCache controls whether to override @@ -161,8 +169,11 @@ spec: time: description: Time defines the default cache time (required when allowedCodes is specified). - pattern: ^[0-9]+[smhd]$ type: string + x-kubernetes-validations: + - message: time must be a number followed by s, m, h, or d (e.g., + '30s', '5m', '1h', '2d') + rule: self.matches('^[0-9]+[smhd]$') required: - cacheZoneName - cacheZoneSize diff --git a/deploy/crds.yaml b/deploy/crds.yaml index 525dd9887b..e70666d0db 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -297,13 +297,12 @@ spec: description: AllowedMethods defines which HTTP methods should be cached. Only GET, HEAD, and POST are supported by NGINX proxy_cache_methods directive. GET and HEAD are always cached by default. - enum: - - GET - - HEAD - - POST items: type: string type: array + x-kubernetes-validations: + - message: 'allowed methods must be one of: GET, HEAD, POST' + rule: self.all(method, method in ['GET', 'HEAD', 'POST']) cachePurgeAllow: description: CachePurgeAllow defines IP addresses allowed to purge cache (NGINX Plus only). @@ -312,18 +311,27 @@ spec: type: array cacheZoneName: description: CacheZoneName defines the name of the cache zone. - maxLength: 64 - pattern: ^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$ type: string + x-kubernetes-validations: + - message: cache zone name must be 1-64 characters, start with + lowercase letter, and contain only alphanumeric characters + and underscores + rule: size(self) <= 64 && self.matches('^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$') cacheZoneSize: description: CacheZoneSize defines the size of the cache zone. - pattern: ^[0-9]+[kmg]$ type: string + x-kubernetes-validations: + - message: cache zone size must be a number followed by k, m, + or g (e.g., '10m', '1g') + rule: self.matches('^[0-9]+[kmg]$') levels: description: Levels defines the cache directory hierarchy levels for storing cached files (e.g., "1:2", "2:2", "1:2:2"). - pattern: ^[12](?::[12]){0,2}$ type: string + x-kubernetes-validations: + - message: levels must be in format like '1:2', '2:2', or '1:2:2' + with values 1 or 2 + rule: self.matches('^[12](?::[12]){0,2}$') overrideUpstreamCache: default: false description: OverrideUpstreamCache controls whether to override @@ -332,8 +340,11 @@ spec: time: description: Time defines the default cache time (required when allowedCodes is specified). - pattern: ^[0-9]+[smhd]$ type: string + x-kubernetes-validations: + - message: time must be a number followed by s, m, h, or d (e.g., + '30s', '5m', '1h', '2d') + rule: self.matches('^[0-9]+[smhd]$') required: - cacheZoneName - cacheZoneSize diff --git a/docs/crd/k8s.nginx.org_policies.md b/docs/crd/k8s.nginx.org_policies.md index 1006f8f639..500c13dbdc 100644 --- a/docs/crd/k8s.nginx.org_policies.md +++ b/docs/crd/k8s.nginx.org_policies.md @@ -28,7 +28,7 @@ The `.spec` object supports the following fields: | `basicAuth.secret` | `string` | The name of the Kubernetes secret that stores the Htpasswd configuration. It must be in the same namespace as the Policy resource. The secret must be of the type nginx.org/htpasswd, and the config must be stored in the secret under the key htpasswd, otherwise the secret will be rejected as invalid. | | `cache` | `object` | The Cache Key defines a cache policy for proxy caching | | `cache.allowedCodes` | `array` | AllowedCodes defines which response codes should be cached. Can be HTTP status codes (100-599) or the string "any" to cache all responses. | -| `cache.allowedMethods` | `array[string]` | AllowedMethods defines which HTTP methods should be cached. Only GET, HEAD, and POST are supported by NGINX proxy_cache_methods directive. GET and HEAD are always cached by default. Allowed values: `"GET"`, `"HEAD"`, `"POST"`. | +| `cache.allowedMethods` | `array[string]` | AllowedMethods defines which HTTP methods should be cached. Only GET, HEAD, and POST are supported by NGINX proxy_cache_methods directive. GET and HEAD are always cached by default. | | `cache.cachePurgeAllow` | `array[string]` | CachePurgeAllow defines IP addresses allowed to purge cache (NGINX Plus only). | | `cache.cacheZoneName` | `string` | CacheZoneName defines the name of the cache zone. | | `cache.cacheZoneSize` | `string` | CacheZoneSize defines the size of the cache zone. | diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index e0e294e4e4..1fd26d1528 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -1011,23 +1011,22 @@ type SuppliedIn struct { // +kubebuilder:validation:XValidation:rule="!has(self.allowedCodes) || (has(self.allowedCodes) && has(self.time))",message="time is required when allowedCodes is specified" type Cache struct { // +kubebuilder:validation:Required - // +kubebuilder:validation:Pattern=`^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$` - // +kubebuilder:validation:MaxLength=64 + // +kubebuilder:validation:XValidation:rule="size(self) <= 64 && self.matches('^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$')",message="cache zone name must be 1-64 characters, start with lowercase letter, and contain only alphanumeric characters and underscores" // CacheZoneName defines the name of the cache zone. CacheZoneName string `json:"cacheZoneName"` // +kubebuilder:validation:Required - // +kubebuilder:validation:Pattern=`^[0-9]+[kmg]$` + // +kubebuilder:validation:XValidation:rule="self.matches('^[0-9]+[kmg]$')",message="cache zone size must be a number followed by k, m, or g (e.g., '10m', '1g')" // CacheZoneSize defines the size of the cache zone. CacheZoneSize string `json:"cacheZoneSize"` // +kubebuilder:validation:Optional // AllowedCodes defines which response codes should be cached. Can be HTTP status codes (100-599) or the string "any" to cache all responses. AllowedCodes []intstr.IntOrString `json:"allowedCodes,omitempty"` // +kubebuilder:validation:Optional - // +kubebuilder:validation:Enum=GET;HEAD;POST // AllowedMethods defines which HTTP methods should be cached. Only GET, HEAD, and POST are supported by NGINX proxy_cache_methods directive. GET and HEAD are always cached by default. + // +kubebuilder:validation:XValidation:rule="self.all(method, method in ['GET', 'HEAD', 'POST'])",message="allowed methods must be one of: GET, HEAD, POST" AllowedMethods []string `json:"allowedMethods,omitempty"` // +kubebuilder:validation:Optional - // +kubebuilder:validation:Pattern=`^[0-9]+[smhd]$` + // +kubebuilder:validation:XValidation:rule="self.matches('^[0-9]+[smhd]$')",message="time must be a number followed by s, m, h, or d (e.g., '30s', '5m', '1h', '2d')" // Time defines the default cache time (required when allowedCodes is specified). Time string `json:"time,omitempty"` // +kubebuilder:validation:Optional @@ -1038,7 +1037,7 @@ type Cache struct { // OverrideUpstreamCache controls whether to override upstream cache headers (using proxy_ignore_headers directive). OverrideUpstreamCache bool `json:"overrideUpstreamCache,omitempty"` // +kubebuilder:validation:Optional - // +kubebuilder:validation:Pattern=`^[12](?::[12]){0,2}$` + // +kubebuilder:validation:XValidation:rule="self.matches('^[12](?::[12]){0,2}$')",message="levels must be in format like '1:2', '2:2', or '1:2:2' with values 1 or 2" // Levels defines the cache directory hierarchy levels for storing cached files (e.g., "1:2", "2:2", "1:2:2"). Levels string `json:"levels,omitempty"` } From f47a8b2f2b741aca983f381d6c9d90bdde32ccbd Mon Sep 17 00:00:00 2001 From: Venktesh Date: Fri, 8 Aug 2025 16:57:52 +0100 Subject: [PATCH 15/30] add maxItems for allowedMethods --- config/crd/bases/k8s.nginx.org_policies.yaml | 1 + deploy/crds.yaml | 1 + pkg/apis/configuration/v1/types.go | 1 + 3 files changed, 3 insertions(+) diff --git a/config/crd/bases/k8s.nginx.org_policies.yaml b/config/crd/bases/k8s.nginx.org_policies.yaml index 7f17dc64ef..4b39f24c6c 100644 --- a/config/crd/bases/k8s.nginx.org_policies.yaml +++ b/config/crd/bases/k8s.nginx.org_policies.yaml @@ -128,6 +128,7 @@ spec: directive. GET and HEAD are always cached by default. items: type: string + maxItems: 3 type: array x-kubernetes-validations: - message: 'allowed methods must be one of: GET, HEAD, POST' diff --git a/deploy/crds.yaml b/deploy/crds.yaml index e70666d0db..ab186ed616 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -299,6 +299,7 @@ spec: directive. GET and HEAD are always cached by default. items: type: string + maxItems: 3 type: array x-kubernetes-validations: - message: 'allowed methods must be one of: GET, HEAD, POST' diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index 1fd26d1528..be05a9432f 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -1022,6 +1022,7 @@ type Cache struct { // AllowedCodes defines which response codes should be cached. Can be HTTP status codes (100-599) or the string "any" to cache all responses. AllowedCodes []intstr.IntOrString `json:"allowedCodes,omitempty"` // +kubebuilder:validation:Optional + // +kubebuilder:validation:MaxItems=3 // AllowedMethods defines which HTTP methods should be cached. Only GET, HEAD, and POST are supported by NGINX proxy_cache_methods directive. GET and HEAD are always cached by default. // +kubebuilder:validation:XValidation:rule="self.all(method, method in ['GET', 'HEAD', 'POST'])",message="allowed methods must be one of: GET, HEAD, POST" AllowedMethods []string `json:"allowedMethods,omitempty"` From 3b643ba99feafd0dba48b29271764a604c31c51d Mon Sep 17 00:00:00 2001 From: Venktesh Date: Mon, 11 Aug 2025 15:36:07 +0100 Subject: [PATCH 16/30] move more validations from go to CRDs --- config/crd/bases/k8s.nginx.org_policies.yaml | 12 +- deploy/crds.yaml | 12 +- docs/crd/k8s.nginx.org_policies.md | 2 +- pkg/apis/configuration/v1/types.go | 3 +- pkg/apis/configuration/validation/policy.go | 107 +--- .../configuration/validation/policy_test.go | 511 +----------------- 6 files changed, 30 insertions(+), 617 deletions(-) diff --git a/config/crd/bases/k8s.nginx.org_policies.yaml b/config/crd/bases/k8s.nginx.org_policies.yaml index 4b39f24c6c..6312b29bfb 100644 --- a/config/crd/bases/k8s.nginx.org_policies.yaml +++ b/config/crd/bases/k8s.nginx.org_policies.yaml @@ -114,14 +114,22 @@ spec: properties: allowedCodes: description: AllowedCodes defines which response codes should - be cached. Can be HTTP status codes (100-599) or the string - "any" to cache all responses. + be cached. Can be HTTP status codes (100-599) as integers or + the string "any" to cache all responses. The string "any" cannot + be combined with other codes. items: anyOf: - type: integer - type: string x-kubernetes-int-or-string: true type: array + x-kubernetes-validations: + - message: allowed codes must be either the single string 'any', + or a list of HTTP status codes (100-599) as integers only + - 'any' cannot be mixed with other codes + rule: (size(self) == 1 && type(self[0]) == string && self[0] + == 'any') || self.all(code, type(code) == int && code >= 100 + && code <= 599) allowedMethods: description: AllowedMethods defines which HTTP methods should be cached. Only GET, HEAD, and POST are supported by NGINX proxy_cache_methods diff --git a/deploy/crds.yaml b/deploy/crds.yaml index ab186ed616..b462bb0b7a 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -285,14 +285,22 @@ spec: properties: allowedCodes: description: AllowedCodes defines which response codes should - be cached. Can be HTTP status codes (100-599) or the string - "any" to cache all responses. + be cached. Can be HTTP status codes (100-599) as integers or + the string "any" to cache all responses. The string "any" cannot + be combined with other codes. items: anyOf: - type: integer - type: string x-kubernetes-int-or-string: true type: array + x-kubernetes-validations: + - message: allowed codes must be either the single string 'any', + or a list of HTTP status codes (100-599) as integers only + - 'any' cannot be mixed with other codes + rule: (size(self) == 1 && type(self[0]) == string && self[0] + == 'any') || self.all(code, type(code) == int && code >= 100 + && code <= 599) allowedMethods: description: AllowedMethods defines which HTTP methods should be cached. Only GET, HEAD, and POST are supported by NGINX proxy_cache_methods diff --git a/docs/crd/k8s.nginx.org_policies.md b/docs/crd/k8s.nginx.org_policies.md index 500c13dbdc..f60fa1e1b0 100644 --- a/docs/crd/k8s.nginx.org_policies.md +++ b/docs/crd/k8s.nginx.org_policies.md @@ -27,7 +27,7 @@ The `.spec` object supports the following fields: | `basicAuth.realm` | `string` | The realm for the basic authentication. | | `basicAuth.secret` | `string` | The name of the Kubernetes secret that stores the Htpasswd configuration. It must be in the same namespace as the Policy resource. The secret must be of the type nginx.org/htpasswd, and the config must be stored in the secret under the key htpasswd, otherwise the secret will be rejected as invalid. | | `cache` | `object` | The Cache Key defines a cache policy for proxy caching | -| `cache.allowedCodes` | `array` | AllowedCodes defines which response codes should be cached. Can be HTTP status codes (100-599) or the string "any" to cache all responses. | +| `cache.allowedCodes` | `array` | AllowedCodes defines which response codes should be cached. Can be HTTP status codes (100-599) as integers or the string "any" to cache all responses. The string "any" cannot be combined with other codes. | | `cache.allowedMethods` | `array[string]` | AllowedMethods defines which HTTP methods should be cached. Only GET, HEAD, and POST are supported by NGINX proxy_cache_methods directive. GET and HEAD are always cached by default. | | `cache.cachePurgeAllow` | `array[string]` | CachePurgeAllow defines IP addresses allowed to purge cache (NGINX Plus only). | | `cache.cacheZoneName` | `string` | CacheZoneName defines the name of the cache zone. | diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index be05a9432f..10654417ff 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -1019,7 +1019,8 @@ type Cache struct { // CacheZoneSize defines the size of the cache zone. CacheZoneSize string `json:"cacheZoneSize"` // +kubebuilder:validation:Optional - // AllowedCodes defines which response codes should be cached. Can be HTTP status codes (100-599) or the string "any" to cache all responses. + // +kubebuilder:validation:XValidation:rule="(size(self) == 1 && type(self[0]) == string && self[0] == 'any') || self.all(code, type(code) == int && code >= 100 && code <= 599)",message="allowed codes must be either the single string 'any', or a list of HTTP status codes (100-599) as integers only - 'any' cannot be mixed with other codes" + // AllowedCodes defines which response codes should be cached. Can be HTTP status codes (100-599) as integers or the string "any" to cache all responses. The string "any" cannot be combined with other codes. AllowedCodes []intstr.IntOrString `json:"allowedCodes,omitempty"` // +kubebuilder:validation:Optional // +kubebuilder:validation:MaxItems=3 diff --git a/pkg/apis/configuration/validation/policy.go b/pkg/apis/configuration/validation/policy.go index 874e09f98a..553b05bd30 100644 --- a/pkg/apis/configuration/validation/policy.go +++ b/pkg/apis/configuration/validation/policy.go @@ -11,7 +11,6 @@ import ( validation2 "github.com/nginx/kubernetes-ingress/internal/validation" v1 "github.com/nginx/kubernetes-ingress/pkg/apis/configuration/v1" - "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" ) @@ -428,116 +427,12 @@ func validateLogConfs(logs []*v1.SecurityLog, fieldPath *field.Path, bundleMode func validateCache(cache *v1.Cache, fieldPath *field.Path, isPlus bool) field.ErrorList { allErrs := field.ErrorList{} - // Validate required fields and basic constraints - allErrs = append(allErrs, validateCacheRequiredFields(cache, fieldPath)...) - - // Validate conditional logic (anyOf constraint) - allErrs = append(allErrs, validateCacheConditionalFields(cache, fieldPath)...) - - // Validate field formats and patterns - allErrs = append(allErrs, validateCacheFieldFormats(cache, fieldPath)...) - - // Validate NGINX Plus specific features + // Validate NGINX Plus specific features only - CRD handles all other validation allErrs = append(allErrs, validateCachePlusFeatures(cache, fieldPath, isPlus)...) return allErrs } -// validateCacheRequiredFields validates required fields and their constraints -func validateCacheRequiredFields(cache *v1.Cache, fieldPath *field.Path) field.ErrorList { - allErrs := field.ErrorList{} - - // Validate cacheZoneName - if cache.CacheZoneName == "" { - allErrs = append(allErrs, field.Required(fieldPath.Child("cacheZoneName"), "cache zone name is required")) - } else { - // Validate cache zone name pattern: ^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$ - cacheZoneNamePattern := regexp.MustCompile(`^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$`) - if !cacheZoneNamePattern.MatchString(cache.CacheZoneName) { - allErrs = append(allErrs, field.Invalid(fieldPath.Child("cacheZoneName"), cache.CacheZoneName, "cache zone name must start with a lowercase letter and contain only letters, numbers, and underscores")) - } - // Validate cache zone name max length - if len(cache.CacheZoneName) > 64 { - allErrs = append(allErrs, field.TooLong(fieldPath.Child("cacheZoneName"), cache.CacheZoneName, 64)) - } - } - - // Validate cacheZoneSize - if cache.CacheZoneSize == "" { - allErrs = append(allErrs, field.Required(fieldPath.Child("cacheZoneSize"), "cache zone size is required")) - } else { - // Validate cache zone size pattern: ^[0-9]+[kmg]$ - cacheZoneSizePattern := regexp.MustCompile(`^[0-9]+[kmg]$`) - if !cacheZoneSizePattern.MatchString(cache.CacheZoneSize) { - allErrs = append(allErrs, field.Invalid(fieldPath.Child("cacheZoneSize"), cache.CacheZoneSize, "cache zone size must be a number followed by k, m, or g (e.g., '10m', '1g')")) - } - } - - return allErrs -} - -// validateCacheConditionalFields validates the anyOf constraint for allowedCodes and time -func validateCacheConditionalFields(cache *v1.Cache, fieldPath *field.Path) field.ErrorList { - allErrs := field.ErrorList{} - - // Validate anyOf constraint: either allowedCodes is not specified, or both allowedCodes and time are specified - hasAllowedCodes := len(cache.AllowedCodes) > 0 - hasTime := cache.Time != "" - - if hasAllowedCodes && !hasTime { - allErrs = append(allErrs, field.Required(fieldPath.Child("time"), "time is required when allowedCodes is specified")) - } - - return allErrs -} - -// validateCacheFieldFormats validates field formats and patterns -func validateCacheFieldFormats(cache *v1.Cache, fieldPath *field.Path) field.ErrorList { - allErrs := field.ErrorList{} - - // Validate allowed codes if provided - for i, code := range cache.AllowedCodes { - if code.Type == intstr.String { - // Only allow the string "any" - if code.StrVal != "any" { - allErrs = append(allErrs, field.Invalid(fieldPath.Child("allowedCodes").Index(i), code.StrVal, "only the string 'any' is allowed")) - } - } else { - // Validate integer codes (100-599) - intCode := code.IntVal - if intCode < 100 || intCode > 599 { - allErrs = append(allErrs, field.Invalid(fieldPath.Child("allowedCodes").Index(i), intCode, "HTTP status code must be between 100 and 599")) - } - } - } - - // Validate allowed methods if provided - validMethods := map[string]bool{"GET": true, "HEAD": true, "POST": true} - for i, method := range cache.AllowedMethods { - if !validMethods[method] { - allErrs = append(allErrs, field.NotSupported(fieldPath.Child("allowedMethods").Index(i), method, []string{"GET", "HEAD", "POST"})) - } - } - - // Validate time pattern if provided: ^[0-9]+[smhd]$ - if cache.Time != "" { - timePattern := regexp.MustCompile(`^[0-9]+[smhd]$`) - if !timePattern.MatchString(cache.Time) { - allErrs = append(allErrs, field.Invalid(fieldPath.Child("time"), cache.Time, "time must be a number followed by s, m, h, or d (e.g., '30s', '5m', '1h', '1d')")) - } - } - - // Validate levels pattern if provided: ^[12](?::[12]){0,2}$ - if cache.Levels != "" { - levelsPattern := regexp.MustCompile(`^[12](?::[12]){0,2}$`) - if !levelsPattern.MatchString(cache.Levels) { - allErrs = append(allErrs, field.Invalid(fieldPath.Child("levels"), cache.Levels, "levels must be in format like '1:2' or '1:2:2' with values of 1 or 2")) - } - } - - return allErrs -} - // validateCachePlusFeatures validates NGINX Plus specific features, such as cache purge allow IPs func validateCachePlusFeatures(cache *v1.Cache, fieldPath *field.Path, isPlus bool) field.ErrorList { allErrs := field.ErrorList{} diff --git a/pkg/apis/configuration/validation/policy_test.go b/pkg/apis/configuration/validation/policy_test.go index ca8bc973bf..c139fbf687 100644 --- a/pkg/apis/configuration/validation/policy_test.go +++ b/pkg/apis/configuration/validation/policy_test.go @@ -2437,45 +2437,7 @@ func TestValidateCache(t *testing.T) { expectValid bool expectedError string }{ - // Valid cache configurations - { - name: "valid cache with basic configuration", - cache: &v1.Cache{ - CacheZoneName: "mycache", - CacheZoneSize: "10m", - }, - isPlus: false, - expectValid: true, - }, - { - name: "valid cache with all options", - cache: &v1.Cache{ - CacheZoneName: "mycache", - CacheZoneSize: "100m", - AllowedCodes: []intstr.IntOrString{intstr.FromString("any")}, - AllowedMethods: []string{"GET", "HEAD", "POST"}, - Time: "1h", - OverrideUpstreamCache: true, - Levels: "1:2", - }, - isPlus: false, - expectValid: true, - }, - { - name: "valid cache with specific status codes", - cache: &v1.Cache{ - CacheZoneName: "statuscache", - CacheZoneSize: "50m", - AllowedCodes: []intstr.IntOrString{ - intstr.FromInt(200), - intstr.FromInt(301), - intstr.FromInt(404), - }, - Time: "30m", - }, - isPlus: false, - expectValid: true, - }, + // Valid cache configurations - Only testing Plus features since CRD handles all other validation { name: "valid cache with purge (NGINX Plus)", cache: &v1.Cache{ @@ -2486,98 +2448,6 @@ func TestValidateCache(t *testing.T) { isPlus: true, expectValid: true, }, - { - name: "valid cache with GET method only", - cache: &v1.Cache{ - CacheZoneName: "getcache", - CacheZoneSize: "15m", - AllowedMethods: []string{"GET"}, - }, - isPlus: false, - expectValid: true, - }, - { - name: "valid cache with complex levels", - cache: &v1.Cache{ - CacheZoneName: "levelcache", - CacheZoneSize: "25m", - Levels: "2:2", - }, - isPlus: false, - expectValid: true, - }, - { - name: "valid cache zone name with underscores", - cache: &v1.Cache{ - CacheZoneName: "valid_cache_name", - CacheZoneSize: "10m", - }, - isPlus: false, - expectValid: true, - }, - { - name: "valid cache zone size with k unit", - cache: &v1.Cache{ - CacheZoneName: "validname", - CacheZoneSize: "1024k", - }, - isPlus: false, - expectValid: true, - }, - { - name: "valid cache zone size with g unit", - cache: &v1.Cache{ - CacheZoneName: "validname", - CacheZoneSize: "2g", - }, - isPlus: false, - expectValid: true, - }, - { - name: "valid time in seconds", - cache: &v1.Cache{ - CacheZoneName: "test", - CacheZoneSize: "10m", - Time: "30s", - }, - isPlus: false, - expectValid: true, - }, - { - name: "valid time in days", - cache: &v1.Cache{ - CacheZoneName: "test", - CacheZoneSize: "10m", - Time: "1d", - }, - isPlus: false, - expectValid: true, - }, - { - name: "valid allowedCodes 'any' with time", - cache: &v1.Cache{ - CacheZoneName: "test", - CacheZoneSize: "10m", - AllowedCodes: []intstr.IntOrString{intstr.FromString("any")}, - Time: "30m", - }, - isPlus: false, - expectValid: true, - }, - { - name: "valid boundary status codes", - cache: &v1.Cache{ - CacheZoneName: "test", - CacheZoneSize: "10m", - AllowedCodes: []intstr.IntOrString{ - intstr.FromInt(100), // minimum - intstr.FromInt(599), // maximum - }, - Time: "1h", - }, - isPlus: false, - expectValid: true, - }, { name: "valid IPv6 address in purge allow", cache: &v1.Cache{ @@ -2598,241 +2468,17 @@ func TestValidateCache(t *testing.T) { isPlus: true, expectValid: true, }, - - // Invalid cache configurations - Required fields - { - name: "missing cache zone name", - cache: &v1.Cache{ - CacheZoneSize: "10m", - }, - isPlus: false, - expectValid: false, - expectedError: "cache zone name is required", - }, - { - name: "missing cache zone size", - cache: &v1.Cache{ - CacheZoneName: "validname", - }, - isPlus: false, - expectValid: false, - expectedError: "cache zone size is required", - }, - { - name: "invalid cache zone name - starts with uppercase", - cache: &v1.Cache{ - CacheZoneName: "InvalidName", - CacheZoneSize: "10m", - }, - isPlus: false, - expectValid: false, - expectedError: "cache zone name must start with a lowercase letter", - }, - { - name: "invalid cache zone name - starts with number", - cache: &v1.Cache{ - CacheZoneName: "1invalidname", - CacheZoneSize: "10m", - }, - isPlus: false, - expectValid: false, - expectedError: "cache zone name must start with a lowercase letter", - }, - { - name: "invalid cache zone name - special characters", - cache: &v1.Cache{ - CacheZoneName: "invalid-name", - CacheZoneSize: "10m", - }, - isPlus: false, - expectValid: false, - expectedError: "cache zone name must start with a lowercase letter", - }, - { - name: "cache zone name too long", - cache: &v1.Cache{ - CacheZoneName: "a" + strings.Repeat("x", 64), // 65 characters - CacheZoneSize: "10m", - }, - isPlus: false, - expectValid: false, - expectedError: "may not be more than 64 bytes", - }, - { - name: "invalid cache zone size - no unit", - cache: &v1.Cache{ - CacheZoneName: "validname", - CacheZoneSize: "10", - }, - isPlus: false, - expectValid: false, - expectedError: "cache zone size must be a number followed by k, m, or g", - }, - { - name: "invalid cache zone size - invalid unit", - cache: &v1.Cache{ - CacheZoneName: "validname", - CacheZoneSize: "10x", - }, - isPlus: false, - expectValid: false, - expectedError: "cache zone size must be a number followed by k, m, or g", - }, - - // Invalid cache configurations - Conditional fields - { - name: "allowedCodes without time", - cache: &v1.Cache{ - CacheZoneName: "test", - CacheZoneSize: "10m", - AllowedCodes: []intstr.IntOrString{intstr.FromInt(200)}, - }, - isPlus: false, - expectValid: false, - expectedError: "time is required when allowedCodes is specified", - }, - { - name: "multiple allowedCodes without time", - cache: &v1.Cache{ - CacheZoneName: "test", - CacheZoneSize: "10m", - AllowedCodes: []intstr.IntOrString{ - intstr.FromInt(200), - intstr.FromInt(404), - }, - }, - isPlus: false, - expectValid: false, - expectedError: "time is required when allowedCodes is specified", - }, - - // Invalid cache configurations - Field formats - { - name: "invalid allowed code string", - cache: &v1.Cache{ - CacheZoneName: "invalidcache", - CacheZoneSize: "10m", - AllowedCodes: []intstr.IntOrString{intstr.FromString("invalid")}, - Time: "1h", - }, - isPlus: false, - expectValid: false, - expectedError: "only the string 'any' is allowed", - }, - { - name: "invalid status code too low", - cache: &v1.Cache{ - CacheZoneName: "invalidcache", - CacheZoneSize: "10m", - AllowedCodes: []intstr.IntOrString{intstr.FromInt(99)}, - Time: "1h", - }, - isPlus: false, - expectValid: false, - expectedError: "HTTP status code must be between 100 and 599", - }, - { - name: "invalid status code too high", - cache: &v1.Cache{ - CacheZoneName: "invalidcache", - CacheZoneSize: "10m", - AllowedCodes: []intstr.IntOrString{intstr.FromInt(600)}, - Time: "1h", - }, - isPlus: false, - expectValid: false, - expectedError: "HTTP status code must be between 100 and 599", - }, - { - name: "invalid HTTP method", - cache: &v1.Cache{ - CacheZoneName: "test", - CacheZoneSize: "10m", - AllowedMethods: []string{"PUT"}, - }, - isPlus: false, - expectValid: false, - expectedError: "supported values:", - }, - { - name: "mixed valid and invalid methods", - cache: &v1.Cache{ - CacheZoneName: "test", - CacheZoneSize: "10m", - AllowedMethods: []string{"GET", "DELETE"}, - }, - isPlus: false, - expectValid: false, - expectedError: "supported values:", - }, - { - name: "invalid time without unit", - cache: &v1.Cache{ - CacheZoneName: "test", - CacheZoneSize: "10m", - Time: "30", - }, - isPlus: false, - expectValid: false, - expectedError: "time must be a number followed by s, m, h, or d", - }, { - name: "invalid time with invalid unit", + name: "valid cache without purge features", cache: &v1.Cache{ - CacheZoneName: "test", + CacheZoneName: "basiccache", CacheZoneSize: "10m", - Time: "30x", }, - isPlus: false, - expectValid: false, - expectedError: "time must be a number followed by s, m, h, or d", - }, - { - name: "invalid levels with value 3", - cache: &v1.Cache{ - CacheZoneName: "test", - CacheZoneSize: "10m", - Levels: "1:3", - }, - isPlus: false, - expectValid: false, - expectedError: "levels must be in format like '1:2' or '1:2:2' with values of 1 or 2", - }, - { - name: "invalid levels with value 0", - cache: &v1.Cache{ - CacheZoneName: "test", - CacheZoneSize: "10m", - Levels: "0:1", - }, - isPlus: false, - expectValid: false, - expectedError: "levels must be in format like '1:2' or '1:2:2' with values of 1 or 2", - }, - { - name: "invalid levels too many parts", - cache: &v1.Cache{ - CacheZoneName: "test", - CacheZoneSize: "10m", - Levels: "1:2:1:2", - }, - isPlus: false, - expectValid: false, - expectedError: "levels must be in format like '1:2' or '1:2:2' with values of 1 or 2", - }, - { - name: "invalid levels format", - cache: &v1.Cache{ - CacheZoneName: "test", - CacheZoneSize: "10m", - Levels: "1-2", - }, - isPlus: false, - expectValid: false, - expectedError: "levels must be in format like '1:2' or '1:2:2' with values of 1 or 2", + isPlus: false, + expectValid: true, }, - // Invalid cache configurations - NGINX Plus features + // Invalid cache configurations - NGINX Plus features only { name: "cache purge not allowed on OSS", cache: &v1.Cache{ @@ -3033,20 +2679,6 @@ func TestValidatePolicy_CachePolicy(t *testing.T) { }, isPlus: false, }, - { - name: "invalid cache with bad status code", - policy: &v1.Policy{ - Spec: v1.PolicySpec{ - Cache: &v1.Cache{ - CacheZoneName: "invalidcache", - CacheZoneSize: "10m", - AllowedCodes: []intstr.IntOrString{intstr.FromInt(1000)}, - Time: "1h", - }, - }, - }, - isPlus: false, - }, } for _, test := range invalidPolicyTests { @@ -3061,134 +2693,3 @@ func TestValidatePolicy_CachePolicy(t *testing.T) { }) } } - -func TestValidatePolicy_CacheRequiredFields(t *testing.T) { - t.Parallel() - - // Test the CRD-level validation that requires time when allowedCodes is specified - validPolicies := []struct { - name string - policy *v1.Policy - }{ - { - name: "no allowedCodes, no time required", - policy: &v1.Policy{ - Spec: v1.PolicySpec{ - Cache: &v1.Cache{ - CacheZoneName: "notime", - CacheZoneSize: "10m", - }, - }, - }, - }, - { - name: "allowedCodes with time", - policy: &v1.Policy{ - Spec: v1.PolicySpec{ - Cache: &v1.Cache{ - CacheZoneName: "withtime", - CacheZoneSize: "10m", - AllowedCodes: []intstr.IntOrString{intstr.FromString("any")}, - Time: "1h", - }, - }, - }, - }, - { - name: "specific status codes with time", - policy: &v1.Policy{ - Spec: v1.PolicySpec{ - Cache: &v1.Cache{ - CacheZoneName: "statuscodes", - CacheZoneSize: "10m", - AllowedCodes: []intstr.IntOrString{ - intstr.FromInt(200), - intstr.FromInt(404), - }, - Time: "30m", - }, - }, - }, - }, - } - - for _, test := range validPolicies { - test := test - t.Run(test.name, func(t *testing.T) { - t.Parallel() - - err := ValidatePolicy(test.policy, false, false, false) - if err != nil { - t.Errorf("Expected valid cache policy, got error: %v", err) - } - }) - } -} - -func TestValidatePolicy_CacheMethodsValidation(t *testing.T) { - t.Parallel() - - // Test allowed methods validation - tests := []struct { - name string - allowedMethods []string - expectValid bool - }{ - { - name: "valid GET method", - allowedMethods: []string{"GET"}, - expectValid: true, - }, - { - name: "valid HEAD method", - allowedMethods: []string{"HEAD"}, - expectValid: true, - }, - { - name: "valid POST method", - allowedMethods: []string{"POST"}, - expectValid: true, - }, - { - name: "valid multiple methods", - allowedMethods: []string{"GET", "HEAD", "POST"}, - expectValid: true, - }, - { - name: "empty methods (should be valid)", - allowedMethods: []string{}, - expectValid: true, - }, - { - name: "nil methods (should be valid)", - allowedMethods: nil, - expectValid: true, - }, - } - - for _, test := range tests { - test := test - t.Run(test.name, func(t *testing.T) { - t.Parallel() - - policy := &v1.Policy{ - Spec: v1.PolicySpec{ - Cache: &v1.Cache{ - CacheZoneName: "methodcache", - CacheZoneSize: "10m", - AllowedMethods: test.allowedMethods, - }, - }, - } - - err := ValidatePolicy(policy, false, false, false) - - if test.expectValid && err != nil { - t.Errorf("Expected valid policy for methods %v, got error: %v", test.allowedMethods, err) - } - if !test.expectValid && err == nil { - t.Errorf("Expected invalid policy for methods %v, got no error", test.allowedMethods) - } - }) - } -} From 9301c631bf10472164644b0acb9744c80fc09ef1 Mon Sep 17 00:00:00 2001 From: Venktesh Date: Mon, 11 Aug 2025 16:02:21 +0100 Subject: [PATCH 17/30] add better descriptions and remove costly CEL expressions --- config/crd/bases/k8s.nginx.org_policies.yaml | 80 +++++++++++--------- deploy/crds.yaml | 80 +++++++++++--------- docs/crd/k8s.nginx.org_policies.md | 16 ++-- pkg/apis/configuration/v1/types.go | 50 +++++++++--- 4 files changed, 138 insertions(+), 88 deletions(-) diff --git a/config/crd/bases/k8s.nginx.org_policies.yaml b/config/crd/bases/k8s.nginx.org_policies.yaml index 6312b29bfb..6adc34648a 100644 --- a/config/crd/bases/k8s.nginx.org_policies.yaml +++ b/config/crd/bases/k8s.nginx.org_policies.yaml @@ -113,10 +113,13 @@ spec: description: The Cache Key defines a cache policy for proxy caching properties: allowedCodes: - description: AllowedCodes defines which response codes should - be cached. Can be HTTP status codes (100-599) as integers or - the string "any" to cache all responses. The string "any" cannot - be combined with other codes. + description: |- + AllowedCodes defines which HTTP response codes should be cached. + Accepts either: + - The string "any" to cache all response codes (must be the only element) + - A list of HTTP status codes as integers (100-599) + Examples: ["any"], [200, 301, 404], [200]. + Invalid: ["any", 200] (cannot mix "any" with specific codes). items: anyOf: - type: integer @@ -131,9 +134,12 @@ spec: == 'any') || self.all(code, type(code) == int && code >= 100 && code <= 599) allowedMethods: - description: AllowedMethods defines which HTTP methods should - be cached. Only GET, HEAD, and POST are supported by NGINX proxy_cache_methods - directive. GET and HEAD are always cached by default. + description: |- + AllowedMethods defines which HTTP methods should be cached. + Only "GET", "HEAD", and "POST" are supported by NGINX proxy_cache_methods directive. + GET and HEAD are always cached by default even if not specified. + Maximum of 3 items allowed. Examples: ["GET"], ["GET", "HEAD", "POST"]. + Invalid methods: PUT, DELETE, PATCH, etc. items: type: string maxItems: 3 @@ -142,47 +148,53 @@ spec: - message: 'allowed methods must be one of: GET, HEAD, POST' rule: self.all(method, method in ['GET', 'HEAD', 'POST']) cachePurgeAllow: - description: CachePurgeAllow defines IP addresses allowed to purge - cache (NGINX Plus only). + description: |- + CachePurgeAllow defines IP addresses or CIDR blocks allowed to purge cache. + This feature is only available in NGINX Plus. + Examples: ["192.168.1.100", "10.0.0.0/8", "::1"]. + Invalid in NGINX OSS (will be ignored). items: type: string type: array cacheZoneName: - description: CacheZoneName defines the name of the cache zone. + description: |- + CacheZoneName defines the name of the cache zone. Must start with a lowercase letter, + followed by alphanumeric characters or underscores, and end with an alphanumeric character. + Single lowercase letters are also allowed. Examples: "cache", "my_cache", "cache1". + pattern: ^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$ type: string - x-kubernetes-validations: - - message: cache zone name must be 1-64 characters, start with - lowercase letter, and contain only alphanumeric characters - and underscores - rule: size(self) <= 64 && self.matches('^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$') cacheZoneSize: - description: CacheZoneSize defines the size of the cache zone. + description: |- + CacheZoneSize defines the size of the cache zone. Must be a number followed by a size unit: + 'k' for kilobytes, 'm' for megabytes, or 'g' for gigabytes. + Examples: "10m", "1g", "512k". + pattern: ^[0-9]+[kmg]$ type: string - x-kubernetes-validations: - - message: cache zone size must be a number followed by k, m, - or g (e.g., '10m', '1g') - rule: self.matches('^[0-9]+[kmg]$') levels: - description: Levels defines the cache directory hierarchy levels - for storing cached files (e.g., "1:2", "2:2", "1:2:2"). + description: |- + Levels defines the cache directory hierarchy levels for storing cached files. + Must be in format "X:Y" or "X:Y:Z" where X, Y, Z are either 1 or 2. + This controls the number of subdirectory levels and their name lengths. + Examples: "1:2", "2:2", "1:2:2". + Invalid: "3:1", "1:3", "1:2:3". + pattern: ^[12](?::[12]){0,2}$ type: string - x-kubernetes-validations: - - message: levels must be in format like '1:2', '2:2', or '1:2:2' - with values 1 or 2 - rule: self.matches('^[12](?::[12]){0,2}$') overrideUpstreamCache: default: false - description: OverrideUpstreamCache controls whether to override - upstream cache headers (using proxy_ignore_headers directive). + description: |- + OverrideUpstreamCache controls whether to override upstream cache headers + (using proxy_ignore_headers directive). When true, NGINX will ignore + cache-related headers from upstream servers like Cache-Control, Expires, etc. + Default: false. type: boolean time: - description: Time defines the default cache time (required when - allowedCodes is specified). + description: |- + Time defines the default cache time. Required when allowedCodes is specified. + Must be a number followed by a time unit: + 's' for seconds, 'm' for minutes, 'h' for hours, 'd' for days. + Examples: "30s", "5m", "1h", "2d". + pattern: ^[0-9]+[smhd]$ type: string - x-kubernetes-validations: - - message: time must be a number followed by s, m, h, or d (e.g., - '30s', '5m', '1h', '2d') - rule: self.matches('^[0-9]+[smhd]$') required: - cacheZoneName - cacheZoneSize diff --git a/deploy/crds.yaml b/deploy/crds.yaml index b462bb0b7a..2d9e673e7b 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -284,10 +284,13 @@ spec: description: The Cache Key defines a cache policy for proxy caching properties: allowedCodes: - description: AllowedCodes defines which response codes should - be cached. Can be HTTP status codes (100-599) as integers or - the string "any" to cache all responses. The string "any" cannot - be combined with other codes. + description: |- + AllowedCodes defines which HTTP response codes should be cached. + Accepts either: + - The string "any" to cache all response codes (must be the only element) + - A list of HTTP status codes as integers (100-599) + Examples: ["any"], [200, 301, 404], [200]. + Invalid: ["any", 200] (cannot mix "any" with specific codes). items: anyOf: - type: integer @@ -302,9 +305,12 @@ spec: == 'any') || self.all(code, type(code) == int && code >= 100 && code <= 599) allowedMethods: - description: AllowedMethods defines which HTTP methods should - be cached. Only GET, HEAD, and POST are supported by NGINX proxy_cache_methods - directive. GET and HEAD are always cached by default. + description: |- + AllowedMethods defines which HTTP methods should be cached. + Only "GET", "HEAD", and "POST" are supported by NGINX proxy_cache_methods directive. + GET and HEAD are always cached by default even if not specified. + Maximum of 3 items allowed. Examples: ["GET"], ["GET", "HEAD", "POST"]. + Invalid methods: PUT, DELETE, PATCH, etc. items: type: string maxItems: 3 @@ -313,47 +319,53 @@ spec: - message: 'allowed methods must be one of: GET, HEAD, POST' rule: self.all(method, method in ['GET', 'HEAD', 'POST']) cachePurgeAllow: - description: CachePurgeAllow defines IP addresses allowed to purge - cache (NGINX Plus only). + description: |- + CachePurgeAllow defines IP addresses or CIDR blocks allowed to purge cache. + This feature is only available in NGINX Plus. + Examples: ["192.168.1.100", "10.0.0.0/8", "::1"]. + Invalid in NGINX OSS (will be ignored). items: type: string type: array cacheZoneName: - description: CacheZoneName defines the name of the cache zone. + description: |- + CacheZoneName defines the name of the cache zone. Must start with a lowercase letter, + followed by alphanumeric characters or underscores, and end with an alphanumeric character. + Single lowercase letters are also allowed. Examples: "cache", "my_cache", "cache1". + pattern: ^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$ type: string - x-kubernetes-validations: - - message: cache zone name must be 1-64 characters, start with - lowercase letter, and contain only alphanumeric characters - and underscores - rule: size(self) <= 64 && self.matches('^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$') cacheZoneSize: - description: CacheZoneSize defines the size of the cache zone. + description: |- + CacheZoneSize defines the size of the cache zone. Must be a number followed by a size unit: + 'k' for kilobytes, 'm' for megabytes, or 'g' for gigabytes. + Examples: "10m", "1g", "512k". + pattern: ^[0-9]+[kmg]$ type: string - x-kubernetes-validations: - - message: cache zone size must be a number followed by k, m, - or g (e.g., '10m', '1g') - rule: self.matches('^[0-9]+[kmg]$') levels: - description: Levels defines the cache directory hierarchy levels - for storing cached files (e.g., "1:2", "2:2", "1:2:2"). + description: |- + Levels defines the cache directory hierarchy levels for storing cached files. + Must be in format "X:Y" or "X:Y:Z" where X, Y, Z are either 1 or 2. + This controls the number of subdirectory levels and their name lengths. + Examples: "1:2", "2:2", "1:2:2". + Invalid: "3:1", "1:3", "1:2:3". + pattern: ^[12](?::[12]){0,2}$ type: string - x-kubernetes-validations: - - message: levels must be in format like '1:2', '2:2', or '1:2:2' - with values 1 or 2 - rule: self.matches('^[12](?::[12]){0,2}$') overrideUpstreamCache: default: false - description: OverrideUpstreamCache controls whether to override - upstream cache headers (using proxy_ignore_headers directive). + description: |- + OverrideUpstreamCache controls whether to override upstream cache headers + (using proxy_ignore_headers directive). When true, NGINX will ignore + cache-related headers from upstream servers like Cache-Control, Expires, etc. + Default: false. type: boolean time: - description: Time defines the default cache time (required when - allowedCodes is specified). + description: |- + Time defines the default cache time. Required when allowedCodes is specified. + Must be a number followed by a time unit: + 's' for seconds, 'm' for minutes, 'h' for hours, 'd' for days. + Examples: "30s", "5m", "1h", "2d". + pattern: ^[0-9]+[smhd]$ type: string - x-kubernetes-validations: - - message: time must be a number followed by s, m, h, or d (e.g., - '30s', '5m', '1h', '2d') - rule: self.matches('^[0-9]+[smhd]$') required: - cacheZoneName - cacheZoneSize diff --git a/docs/crd/k8s.nginx.org_policies.md b/docs/crd/k8s.nginx.org_policies.md index f60fa1e1b0..cb37a32ae4 100644 --- a/docs/crd/k8s.nginx.org_policies.md +++ b/docs/crd/k8s.nginx.org_policies.md @@ -27,14 +27,14 @@ The `.spec` object supports the following fields: | `basicAuth.realm` | `string` | The realm for the basic authentication. | | `basicAuth.secret` | `string` | The name of the Kubernetes secret that stores the Htpasswd configuration. It must be in the same namespace as the Policy resource. The secret must be of the type nginx.org/htpasswd, and the config must be stored in the secret under the key htpasswd, otherwise the secret will be rejected as invalid. | | `cache` | `object` | The Cache Key defines a cache policy for proxy caching | -| `cache.allowedCodes` | `array` | AllowedCodes defines which response codes should be cached. Can be HTTP status codes (100-599) as integers or the string "any" to cache all responses. The string "any" cannot be combined with other codes. | -| `cache.allowedMethods` | `array[string]` | AllowedMethods defines which HTTP methods should be cached. Only GET, HEAD, and POST are supported by NGINX proxy_cache_methods directive. GET and HEAD are always cached by default. | -| `cache.cachePurgeAllow` | `array[string]` | CachePurgeAllow defines IP addresses allowed to purge cache (NGINX Plus only). | -| `cache.cacheZoneName` | `string` | CacheZoneName defines the name of the cache zone. | -| `cache.cacheZoneSize` | `string` | CacheZoneSize defines the size of the cache zone. | -| `cache.levels` | `string` | Levels defines the cache directory hierarchy levels for storing cached files (e.g., "1:2", "2:2", "1:2:2"). | -| `cache.overrideUpstreamCache` | `boolean` | OverrideUpstreamCache controls whether to override upstream cache headers (using proxy_ignore_headers directive). | -| `cache.time` | `string` | Time defines the default cache time (required when allowedCodes is specified). | +| `cache.allowedCodes` | `array` | AllowedCodes defines which HTTP response codes should be cached. Accepts either: - The string "any" to cache all response codes (must be the only element) - A list of HTTP status codes as integers (100-599) Examples: ["any"], [200, 301, 404], [200]. Invalid: ["any", 200] (cannot mix "any" with specific codes). | +| `cache.allowedMethods` | `array[string]` | AllowedMethods defines which HTTP methods should be cached. Only "GET", "HEAD", and "POST" are supported by NGINX proxy_cache_methods directive. GET and HEAD are always cached by default even if not specified. Maximum of 3 items allowed. Examples: ["GET"], ["GET", "HEAD", "POST"]. Invalid methods: PUT, DELETE, PATCH, etc. | +| `cache.cachePurgeAllow` | `array[string]` | CachePurgeAllow defines IP addresses or CIDR blocks allowed to purge cache. This feature is only available in NGINX Plus. Examples: ["192.168.1.100", "10.0.0.0/8", "::1"]. Invalid in NGINX OSS (will be ignored). | +| `cache.cacheZoneName` | `string` | CacheZoneName defines the name of the cache zone. Must start with a lowercase letter, followed by alphanumeric characters or underscores, and end with an alphanumeric character. Single lowercase letters are also allowed. Examples: "cache", "my_cache", "cache1". | +| `cache.cacheZoneSize` | `string` | CacheZoneSize defines the size of the cache zone. Must be a number followed by a size unit: 'k' for kilobytes, 'm' for megabytes, or 'g' for gigabytes. Examples: "10m", "1g", "512k". | +| `cache.levels` | `string` | Levels defines the cache directory hierarchy levels for storing cached files. Must be in format "X:Y" or "X:Y:Z" where X, Y, Z are either 1 or 2. This controls the number of subdirectory levels and their name lengths. Examples: "1:2", "2:2", "1:2:2". Invalid: "3:1", "1:3", "1:2:3". | +| `cache.overrideUpstreamCache` | `boolean` | OverrideUpstreamCache controls whether to override upstream cache headers (using proxy_ignore_headers directive). When true, NGINX will ignore cache-related headers from upstream servers like Cache-Control, Expires, etc. Default: false. | +| `cache.time` | `string` | Time defines the default cache time. Required when allowedCodes is specified. Must be a number followed by a time unit: 's' for seconds, 'm' for minutes, 'h' for hours, 'd' for days. Examples: "30s", "5m", "1h", "2d". | | `egressMTLS` | `object` | The EgressMTLS policy configures upstreams authentication and certificate verification. | | `egressMTLS.ciphers` | `string` | Specifies the enabled ciphers for requests to an upstream HTTPS server. The default is DEFAULT. | | `egressMTLS.protocols` | `string` | Specifies the protocols for requests to an upstream HTTPS server. The default is TLSv1 TLSv1.1 TLSv1.2. | diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index 10654417ff..dd001dbbd8 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -1011,35 +1011,61 @@ type SuppliedIn struct { // +kubebuilder:validation:XValidation:rule="!has(self.allowedCodes) || (has(self.allowedCodes) && has(self.time))",message="time is required when allowedCodes is specified" type Cache struct { // +kubebuilder:validation:Required - // +kubebuilder:validation:XValidation:rule="size(self) <= 64 && self.matches('^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$')",message="cache zone name must be 1-64 characters, start with lowercase letter, and contain only alphanumeric characters and underscores" - // CacheZoneName defines the name of the cache zone. + // +kubebuilder:validation:Pattern=`^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$` + // CacheZoneName defines the name of the cache zone. Must start with a lowercase letter, + // followed by alphanumeric characters or underscores, and end with an alphanumeric character. + // Single lowercase letters are also allowed. Examples: "cache", "my_cache", "cache1". CacheZoneName string `json:"cacheZoneName"` // +kubebuilder:validation:Required - // +kubebuilder:validation:XValidation:rule="self.matches('^[0-9]+[kmg]$')",message="cache zone size must be a number followed by k, m, or g (e.g., '10m', '1g')" - // CacheZoneSize defines the size of the cache zone. + // +kubebuilder:validation:Pattern=`^[0-9]+[kmg]$` + // CacheZoneSize defines the size of the cache zone. Must be a number followed by a size unit: + // 'k' for kilobytes, 'm' for megabytes, or 'g' for gigabytes. + // Examples: "10m", "1g", "512k". CacheZoneSize string `json:"cacheZoneSize"` // +kubebuilder:validation:Optional // +kubebuilder:validation:XValidation:rule="(size(self) == 1 && type(self[0]) == string && self[0] == 'any') || self.all(code, type(code) == int && code >= 100 && code <= 599)",message="allowed codes must be either the single string 'any', or a list of HTTP status codes (100-599) as integers only - 'any' cannot be mixed with other codes" - // AllowedCodes defines which response codes should be cached. Can be HTTP status codes (100-599) as integers or the string "any" to cache all responses. The string "any" cannot be combined with other codes. + // AllowedCodes defines which HTTP response codes should be cached. + // Accepts either: + // - The string "any" to cache all response codes (must be the only element) + // - A list of HTTP status codes as integers (100-599) + // Examples: ["any"], [200, 301, 404], [200]. + // Invalid: ["any", 200] (cannot mix "any" with specific codes). AllowedCodes []intstr.IntOrString `json:"allowedCodes,omitempty"` // +kubebuilder:validation:Optional // +kubebuilder:validation:MaxItems=3 - // AllowedMethods defines which HTTP methods should be cached. Only GET, HEAD, and POST are supported by NGINX proxy_cache_methods directive. GET and HEAD are always cached by default. // +kubebuilder:validation:XValidation:rule="self.all(method, method in ['GET', 'HEAD', 'POST'])",message="allowed methods must be one of: GET, HEAD, POST" + // AllowedMethods defines which HTTP methods should be cached. + // Only "GET", "HEAD", and "POST" are supported by NGINX proxy_cache_methods directive. + // GET and HEAD are always cached by default even if not specified. + // Maximum of 3 items allowed. Examples: ["GET"], ["GET", "HEAD", "POST"]. + // Invalid methods: PUT, DELETE, PATCH, etc. AllowedMethods []string `json:"allowedMethods,omitempty"` // +kubebuilder:validation:Optional - // +kubebuilder:validation:XValidation:rule="self.matches('^[0-9]+[smhd]$')",message="time must be a number followed by s, m, h, or d (e.g., '30s', '5m', '1h', '2d')" - // Time defines the default cache time (required when allowedCodes is specified). + // +kubebuilder:validation:Pattern=`^[0-9]+[smhd]$` + // Time defines the default cache time. Required when allowedCodes is specified. + // Must be a number followed by a time unit: + // 's' for seconds, 'm' for minutes, 'h' for hours, 'd' for days. + // Examples: "30s", "5m", "1h", "2d". Time string `json:"time,omitempty"` // +kubebuilder:validation:Optional - // CachePurgeAllow defines IP addresses allowed to purge cache (NGINX Plus only). + // CachePurgeAllow defines IP addresses or CIDR blocks allowed to purge cache. + // This feature is only available in NGINX Plus. + // Examples: ["192.168.1.100", "10.0.0.0/8", "::1"]. + // Invalid in NGINX OSS (will be ignored). CachePurgeAllow []string `json:"cachePurgeAllow,omitempty"` // +kubebuilder:validation:Optional // +kubebuilder:default=false - // OverrideUpstreamCache controls whether to override upstream cache headers (using proxy_ignore_headers directive). + // OverrideUpstreamCache controls whether to override upstream cache headers + // (using proxy_ignore_headers directive). When true, NGINX will ignore + // cache-related headers from upstream servers like Cache-Control, Expires, etc. + // Default: false. OverrideUpstreamCache bool `json:"overrideUpstreamCache,omitempty"` // +kubebuilder:validation:Optional - // +kubebuilder:validation:XValidation:rule="self.matches('^[12](?::[12]){0,2}$')",message="levels must be in format like '1:2', '2:2', or '1:2:2' with values 1 or 2" - // Levels defines the cache directory hierarchy levels for storing cached files (e.g., "1:2", "2:2", "1:2:2"). + // +kubebuilder:validation:Pattern=`^[12](?::[12]){0,2}$` + // Levels defines the cache directory hierarchy levels for storing cached files. + // Must be in format "X:Y" or "X:Y:Z" where X, Y, Z are either 1 or 2. + // This controls the number of subdirectory levels and their name lengths. + // Examples: "1:2", "2:2", "1:2:2". + // Invalid: "3:1", "1:3", "1:2:3". Levels string `json:"levels,omitempty"` } From 9974453598850d689a79d9e2b7a40c250843e81f Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 11 Aug 2025 15:03:13 +0000 Subject: [PATCH 18/30] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pkg/apis/configuration/v1/types.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index dd001dbbd8..e2d6f88626 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -1012,7 +1012,7 @@ type SuppliedIn struct { type Cache struct { // +kubebuilder:validation:Required // +kubebuilder:validation:Pattern=`^[a-z][a-zA-Z0-9_]*[a-zA-Z0-9]$|^[a-z]$` - // CacheZoneName defines the name of the cache zone. Must start with a lowercase letter, + // CacheZoneName defines the name of the cache zone. Must start with a lowercase letter, // followed by alphanumeric characters or underscores, and end with an alphanumeric character. // Single lowercase letters are also allowed. Examples: "cache", "my_cache", "cache1". CacheZoneName string `json:"cacheZoneName"` @@ -1024,7 +1024,7 @@ type Cache struct { CacheZoneSize string `json:"cacheZoneSize"` // +kubebuilder:validation:Optional // +kubebuilder:validation:XValidation:rule="(size(self) == 1 && type(self[0]) == string && self[0] == 'any') || self.all(code, type(code) == int && code >= 100 && code <= 599)",message="allowed codes must be either the single string 'any', or a list of HTTP status codes (100-599) as integers only - 'any' cannot be mixed with other codes" - // AllowedCodes defines which HTTP response codes should be cached. + // AllowedCodes defines which HTTP response codes should be cached. // Accepts either: // - The string "any" to cache all response codes (must be the only element) // - A list of HTTP status codes as integers (100-599) @@ -1034,7 +1034,7 @@ type Cache struct { // +kubebuilder:validation:Optional // +kubebuilder:validation:MaxItems=3 // +kubebuilder:validation:XValidation:rule="self.all(method, method in ['GET', 'HEAD', 'POST'])",message="allowed methods must be one of: GET, HEAD, POST" - // AllowedMethods defines which HTTP methods should be cached. + // AllowedMethods defines which HTTP methods should be cached. // Only "GET", "HEAD", and "POST" are supported by NGINX proxy_cache_methods directive. // GET and HEAD are always cached by default even if not specified. // Maximum of 3 items allowed. Examples: ["GET"], ["GET", "HEAD", "POST"]. @@ -1055,8 +1055,8 @@ type Cache struct { CachePurgeAllow []string `json:"cachePurgeAllow,omitempty"` // +kubebuilder:validation:Optional // +kubebuilder:default=false - // OverrideUpstreamCache controls whether to override upstream cache headers - // (using proxy_ignore_headers directive). When true, NGINX will ignore + // OverrideUpstreamCache controls whether to override upstream cache headers + // (using proxy_ignore_headers directive). When true, NGINX will ignore // cache-related headers from upstream servers like Cache-Control, Expires, etc. // Default: false. OverrideUpstreamCache bool `json:"overrideUpstreamCache,omitempty"` From 9d0c6516527d203097d3ff50697a6c5f1508406b Mon Sep 17 00:00:00 2001 From: Venktesh Date: Mon, 11 Aug 2025 17:43:10 +0100 Subject: [PATCH 19/30] replace CEL with go logic --- config/crd/bases/k8s.nginx.org_policies.yaml | 7 - deploy/crds.yaml | 7 - pkg/apis/configuration/v1/types.go | 1 - pkg/apis/configuration/validation/policy.go | 47 +++++- .../configuration/validation/policy_test.go | 156 ++++++++++-------- 5 files changed, 132 insertions(+), 86 deletions(-) diff --git a/config/crd/bases/k8s.nginx.org_policies.yaml b/config/crd/bases/k8s.nginx.org_policies.yaml index 6adc34648a..1ba74d4ecf 100644 --- a/config/crd/bases/k8s.nginx.org_policies.yaml +++ b/config/crd/bases/k8s.nginx.org_policies.yaml @@ -126,13 +126,6 @@ spec: - type: string x-kubernetes-int-or-string: true type: array - x-kubernetes-validations: - - message: allowed codes must be either the single string 'any', - or a list of HTTP status codes (100-599) as integers only - - 'any' cannot be mixed with other codes - rule: (size(self) == 1 && type(self[0]) == string && self[0] - == 'any') || self.all(code, type(code) == int && code >= 100 - && code <= 599) allowedMethods: description: |- AllowedMethods defines which HTTP methods should be cached. diff --git a/deploy/crds.yaml b/deploy/crds.yaml index 2d9e673e7b..1518d090b0 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -297,13 +297,6 @@ spec: - type: string x-kubernetes-int-or-string: true type: array - x-kubernetes-validations: - - message: allowed codes must be either the single string 'any', - or a list of HTTP status codes (100-599) as integers only - - 'any' cannot be mixed with other codes - rule: (size(self) == 1 && type(self[0]) == string && self[0] - == 'any') || self.all(code, type(code) == int && code >= 100 - && code <= 599) allowedMethods: description: |- AllowedMethods defines which HTTP methods should be cached. diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index e2d6f88626..ca43fc4db8 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -1023,7 +1023,6 @@ type Cache struct { // Examples: "10m", "1g", "512k". CacheZoneSize string `json:"cacheZoneSize"` // +kubebuilder:validation:Optional - // +kubebuilder:validation:XValidation:rule="(size(self) == 1 && type(self[0]) == string && self[0] == 'any') || self.all(code, type(code) == int && code >= 100 && code <= 599)",message="allowed codes must be either the single string 'any', or a list of HTTP status codes (100-599) as integers only - 'any' cannot be mixed with other codes" // AllowedCodes defines which HTTP response codes should be cached. // Accepts either: // - The string "any" to cache all response codes (must be the only element) diff --git a/pkg/apis/configuration/validation/policy.go b/pkg/apis/configuration/validation/policy.go index 553b05bd30..73e8ec76e2 100644 --- a/pkg/apis/configuration/validation/policy.go +++ b/pkg/apis/configuration/validation/policy.go @@ -427,12 +427,57 @@ func validateLogConfs(logs []*v1.SecurityLog, fieldPath *field.Path, bundleMode func validateCache(cache *v1.Cache, fieldPath *field.Path, isPlus bool) field.ErrorList { allErrs := field.ErrorList{} - // Validate NGINX Plus specific features only - CRD handles all other validation + allErrs = append(allErrs, validateCacheAllowedCodes(cache, fieldPath)...) + allErrs = append(allErrs, validateCachePlusFeatures(cache, fieldPath, isPlus)...) return allErrs } +// validateCacheAllowedCodes validates the allowedCodes field +func validateCacheAllowedCodes(cache *v1.Cache, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if len(cache.AllowedCodes) == 0 { + return allErrs // No validation needed for empty slice + } + + // Check if it's the special case: single element "any" + if len(cache.AllowedCodes) == 1 && cache.AllowedCodes[0].Type == 1 && cache.AllowedCodes[0].StrVal == "any" { + return allErrs // Valid: single "any" string + } + + // Check if it contains "any" mixed with other codes (invalid) + hasAny := false + for i, code := range cache.AllowedCodes { + if code.Type == 1 && code.StrVal == "any" { + hasAny = true + if len(cache.AllowedCodes) > 1 { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("allowedCodes").Index(i), code.StrVal, "the string 'any' cannot be mixed with other codes")) + } + } + } + + // If we have "any" mixed with others, we already reported the error above + if hasAny { + return allErrs + } + + // Validate all elements are integers in the range 100-599 + for i, code := range cache.AllowedCodes { + if code.Type == 1 { // String type + allErrs = append(allErrs, field.Invalid(fieldPath.Child("allowedCodes").Index(i), code.StrVal, "must be an integer HTTP status code (100-599) or the single string 'any'")) + } else { // Integer type + intVal := int(code.IntVal) + if intVal < 100 || intVal > 599 { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("allowedCodes").Index(i), intVal, "HTTP status code must be between 100 and 599")) + } + } + } + + return allErrs +} + // validateCachePlusFeatures validates NGINX Plus specific features, such as cache purge allow IPs func validateCachePlusFeatures(cache *v1.Cache, fieldPath *field.Path, isPlus bool) field.ErrorList { allErrs := field.ErrorList{} diff --git a/pkg/apis/configuration/validation/policy_test.go b/pkg/apis/configuration/validation/policy_test.go index c139fbf687..32227b65ca 100644 --- a/pkg/apis/configuration/validation/policy_test.go +++ b/pkg/apis/configuration/validation/policy_test.go @@ -2430,6 +2430,29 @@ func TestValidateWAF_FailsOnInvalidApLogBundle(t *testing.T) { func TestValidateCache(t *testing.T) { t.Parallel() + // Helper function to create Cache with mixed allowedCodes from simple types + createCache := func(zoneName, zoneSize string, allowedCodes []interface{}, purgeAllow []string) *v1.Cache { + cache := &v1.Cache{ + CacheZoneName: zoneName, + CacheZoneSize: zoneSize, + CachePurgeAllow: purgeAllow, + } + + // Convert simple types to IntOrString + if len(allowedCodes) > 0 { + for _, code := range allowedCodes { + switch v := code.(type) { + case string: + cache.AllowedCodes = append(cache.AllowedCodes, intstr.FromString(v)) + case int: + cache.AllowedCodes = append(cache.AllowedCodes, intstr.FromInt(v)) + } + } + } + + return cache + } + tests := []struct { name string cache *v1.Cache @@ -2437,113 +2460,106 @@ func TestValidateCache(t *testing.T) { expectValid bool expectedError string }{ - // Valid cache configurations - Only testing Plus features since CRD handles all other validation + // Valid cache configurations { - name: "valid cache with purge (NGINX Plus)", - cache: &v1.Cache{ - CacheZoneName: "purgecache", - CacheZoneSize: "20m", - CachePurgeAllow: []string{"192.168.1.0/24", "10.0.0.1"}, - }, + name: "valid cache with purge (NGINX Plus)", + cache: createCache("purgecache", "20m", nil, []string{"192.168.1.0/24", "10.0.0.1"}), isPlus: true, expectValid: true, }, { - name: "valid IPv6 address in purge allow", - cache: &v1.Cache{ - CacheZoneName: "test", - CacheZoneSize: "10m", - CachePurgeAllow: []string{"2001:db8::1"}, - }, + name: "valid IPv6 address in purge allow", + cache: createCache("test", "10m", nil, []string{"2001:db8::1"}), isPlus: true, expectValid: true, }, { - name: "valid IPv6 CIDR in purge allow", - cache: &v1.Cache{ - CacheZoneName: "test", - CacheZoneSize: "10m", - CachePurgeAllow: []string{"2001:db8::/32"}, - }, - isPlus: true, + name: "valid cache without purge features", + cache: createCache("basiccache", "10m", nil, nil), + isPlus: false, expectValid: true, }, { - name: "valid cache without purge features", - cache: &v1.Cache{ - CacheZoneName: "basiccache", - CacheZoneSize: "10m", - }, + name: "valid allowedCodes with single 'any'", + cache: createCache("test", "10m", []interface{}{"any"}, nil), + isPlus: false, + expectValid: true, + }, + { + name: "valid allowedCodes with integers", + cache: createCache("test", "10m", []interface{}{200, 404, 500}, nil), + isPlus: false, + expectValid: true, + }, + { + name: "valid allowedCodes with edge case status codes", + cache: createCache("test", "10m", []interface{}{100, 599}, nil), + isPlus: false, + expectValid: true, + }, + { + name: "valid empty allowedCodes", + cache: createCache("test", "10m", []interface{}{}, nil), isPlus: false, expectValid: true, }, - // Invalid cache configurations - NGINX Plus features only + // Invalid cache configurations { - name: "cache purge not allowed on OSS", - cache: &v1.Cache{ - CacheZoneName: "purgeoss", - CacheZoneSize: "10m", - CachePurgeAllow: []string{"192.168.1.1"}, - }, + name: "cache purge not allowed on OSS", + cache: createCache("purgeoss", "10m", nil, []string{"192.168.1.1"}), isPlus: false, expectValid: false, expectedError: "cache purge is only supported in NGINX Plus", }, { - name: "invalid IP address in purge allow", - cache: &v1.Cache{ - CacheZoneName: "invalidip", - CacheZoneSize: "10m", - CachePurgeAllow: []string{"invalid-ip"}, - }, + name: "invalid IP address in purge allow", + cache: createCache("invalidip", "10m", nil, []string{"invalid-ip"}), isPlus: true, expectValid: false, expectedError: "must be a valid IP address or CIDR", }, { - name: "invalid CIDR in purge allow", - cache: &v1.Cache{ - CacheZoneName: "invalidcidr", - CacheZoneSize: "10m", - CachePurgeAllow: []string{"192.168.1.1/99"}, - }, - isPlus: true, + name: "allowedCodes with 'any' mixed with integers", + cache: createCache("test", "10m", []interface{}{"any", 200}, nil), + isPlus: false, expectValid: false, - expectedError: "must be a valid IP address or CIDR", + expectedError: "the string 'any' cannot be mixed with other codes", }, { - name: "mixed valid and invalid IPs in purge allow", - cache: &v1.Cache{ - CacheZoneName: "test", - CacheZoneSize: "10m", - CachePurgeAllow: []string{"192.168.1.1", "not-an-ip"}, - }, - isPlus: true, + name: "allowedCodes with invalid string", + cache: createCache("test", "10m", []interface{}{"invalid"}, nil), + isPlus: false, expectValid: false, - expectedError: "must be a valid IP address or CIDR", + expectedError: "must be an integer HTTP status code (100-599) or the single string 'any'", }, { - name: "empty string in IP list", - cache: &v1.Cache{ - CacheZoneName: "test", - CacheZoneSize: "10m", - CachePurgeAllow: []string{"192.168.1.1", ""}, - }, - isPlus: true, + name: "allowedCodes with status code below 100", + cache: createCache("test", "10m", []interface{}{99}, nil), + isPlus: false, expectValid: false, - expectedError: "must be a valid IP address or CIDR", + expectedError: "HTTP status code must be between 100 and 599", }, { - name: "hostname instead of IP in purge allow", - cache: &v1.Cache{ - CacheZoneName: "test", - CacheZoneSize: "10m", - CachePurgeAllow: []string{"example.com"}, - }, - isPlus: true, + name: "allowedCodes with status code above 599", + cache: createCache("test", "10m", []interface{}{600}, nil), + isPlus: false, expectValid: false, - expectedError: "must be a valid IP address or CIDR", + expectedError: "HTTP status code must be between 100 and 599", + }, + { + name: "allowedCodes with multiple 'any' strings", + cache: createCache("test", "10m", []interface{}{"any", "any"}, nil), + isPlus: false, + expectValid: false, + expectedError: "the string 'any' cannot be mixed with other codes", + }, + { + name: "allowedCodes with valid and invalid status codes", + cache: createCache("test", "10m", []interface{}{200, 700}, nil), + isPlus: false, + expectValid: false, + expectedError: "HTTP status code must be between 100 and 599", }, } From 877e69911c74d06c8c367ef7d146521a2f8c44d9 Mon Sep 17 00:00:00 2001 From: Venktesh Date: Tue, 12 Aug 2025 19:00:45 +0100 Subject: [PATCH 20/30] add functional tests and fix plus vs template --- .../version2/nginx-plus.virtualserver.tmpl | 16 +- .../policies/cache-policy-advanced.yaml | 13 + .../policies/cache-policy-basic.yaml | 12 + .../policies/cache-policy-purge.yaml | 12 + ...al-server-cache-policy-advanced-route.yaml | 24 ++ ...rtual-server-cache-policy-basic-route.yaml | 24 ++ ...ual-server-cache-policy-advanced-spec.yaml | 24 ++ ...irtual-server-cache-policy-basic-spec.yaml | 24 ++ .../virtual-server-cache-policy-purge.yaml | 24 ++ ...al-server-route-cache-policy-advanced.yaml | 22 ++ ...rtual-server-route-cache-policy-basic.yaml | 22 ++ ...rtual-server-route-cache-policy-purge.yaml | 22 ++ ...tual-server-route-with-advanced-cache.yaml | 0 .../vsr/virtual-server-route-with-cache.yaml | 0 ...tual-server-with-advanced-cache-route.yaml | 0 .../vsr/virtual-server-with-cache-route.yaml | 0 .../data/cache-policy/vsr/virtual-server.yaml | 13 + tests/suite/test_cache_policies_vs.py | 287 ++++++++++++++++++ tests/suite/test_cache_policies_vsr.py | 220 ++++++++++++++ tests/suite/utils/resources_utils.py | 22 ++ 20 files changed, 773 insertions(+), 8 deletions(-) create mode 100644 tests/data/cache-policy/policies/cache-policy-advanced.yaml create mode 100644 tests/data/cache-policy/policies/cache-policy-basic.yaml create mode 100644 tests/data/cache-policy/policies/cache-policy-purge.yaml create mode 100644 tests/data/cache-policy/route/virtual-server-cache-policy-advanced-route.yaml create mode 100644 tests/data/cache-policy/route/virtual-server-cache-policy-basic-route.yaml create mode 100644 tests/data/cache-policy/spec/virtual-server-cache-policy-advanced-spec.yaml create mode 100644 tests/data/cache-policy/spec/virtual-server-cache-policy-basic-spec.yaml create mode 100644 tests/data/cache-policy/spec/virtual-server-cache-policy-purge.yaml create mode 100644 tests/data/cache-policy/vsr/virtual-server-route-cache-policy-advanced.yaml create mode 100644 tests/data/cache-policy/vsr/virtual-server-route-cache-policy-basic.yaml create mode 100644 tests/data/cache-policy/vsr/virtual-server-route-cache-policy-purge.yaml create mode 100644 tests/data/cache-policy/vsr/virtual-server-route-with-advanced-cache.yaml create mode 100644 tests/data/cache-policy/vsr/virtual-server-route-with-cache.yaml create mode 100644 tests/data/cache-policy/vsr/virtual-server-with-advanced-cache-route.yaml create mode 100644 tests/data/cache-policy/vsr/virtual-server-with-cache-route.yaml create mode 100644 tests/data/cache-policy/vsr/virtual-server.yaml create mode 100644 tests/suite/test_cache_policies_vs.py create mode 100644 tests/suite/test_cache_policies_vsr.py diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index 6b35aad582..31d4729d04 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -85,15 +85,15 @@ match {{ $m.Name }} { {{- /* Generate cache-zone-specific purge configuration with VirtualServer isolation */ -}} {{- /* Check server-level cache purge restrictions */ -}} {{- if and $s.Cache (gt (len $s.Cache.CachePurgeAllow) 0) }} -geo $purge_allowed_{{ $s.Cache.ZoneName }} { +geo $purge_allowed_{{ replaceAll $s.Cache.ZoneName "-" "_" }} { default 0; {{- range $ip := $s.Cache.CachePurgeAllow }} {{ $ip }} 1; {{- end }} } -map $request_method $cache_purge_{{ $s.Cache.ZoneName }} { - PURGE $purge_allowed_{{ $s.Cache.ZoneName }}; +map $request_method $cache_purge_{{ replaceAll $s.Cache.ZoneName "-" "_" }} { + PURGE $purge_allowed_{{ replaceAll $s.Cache.ZoneName "-" "_" }}; default 0; } {{- end }} @@ -101,15 +101,15 @@ map $request_method $cache_purge_{{ $s.Cache.ZoneName }} { {{- /* Check location-level cache purge restrictions */ -}} {{- range $l := $s.Locations }} {{- if and $l.Cache (gt (len $l.Cache.CachePurgeAllow) 0) }} -geo $purge_allowed_{{ $l.Cache.ZoneName }} { +geo $purge_allowed_{{ replaceAll $l.Cache.ZoneName "-" "_" }} { default 0; {{- range $ip := $l.Cache.CachePurgeAllow }} {{ $ip }} 1; {{- end }} } -map $request_method $cache_purge_{{ $l.Cache.ZoneName }} { - PURGE $purge_allowed_{{ $l.Cache.ZoneName }}; +map $request_method $cache_purge_{{ replaceAll $l.Cache.ZoneName "-" "_" }} { + PURGE $purge_allowed_{{ replaceAll $l.Cache.ZoneName "-" "_" }}; default 0; } {{- end }} @@ -241,7 +241,7 @@ server { proxy_cache_methods{{ range $s.Cache.AllowedMethods }} {{ . }}{{ end }}; {{- end }} {{- if gt (len $s.Cache.CachePurgeAllow) 0 }} - proxy_cache_purge $cache_purge_{{ $s.Cache.ZoneName }}; + proxy_cache_purge $cache_purge_{{ replaceAll $s.Cache.ZoneName "-" "_" }}; {{- end }} {{- end }} @@ -745,7 +745,7 @@ server { proxy_cache_methods{{ range $l.Cache.AllowedMethods }} {{ . }}{{ end }}; {{- end }} {{- if gt (len $l.Cache.CachePurgeAllow) 0 }} - proxy_cache_purge $cache_purge_{{ $l.Cache.ZoneName }}; + proxy_cache_purge $cache_purge_{{ replaceAll $l.Cache.ZoneName "-" "_" }}; {{- end }} {{- end }} diff --git a/tests/data/cache-policy/policies/cache-policy-advanced.yaml b/tests/data/cache-policy/policies/cache-policy-advanced.yaml new file mode 100644 index 0000000000..2e7da75f9e --- /dev/null +++ b/tests/data/cache-policy/policies/cache-policy-advanced.yaml @@ -0,0 +1,13 @@ +apiVersion: k8s.nginx.org/v1 +kind: Policy +metadata: + name: cache-policy-advanced +spec: + cache: + cacheZoneName: "advancedcache" + cacheZoneSize: "20m" + allowedCodes: [200, 404, 301] + allowedMethods: ["GET", "HEAD", "POST"] + time: "2h" + overrideUpstreamCache: true + levels: "2:2" diff --git a/tests/data/cache-policy/policies/cache-policy-basic.yaml b/tests/data/cache-policy/policies/cache-policy-basic.yaml new file mode 100644 index 0000000000..761f17e23a --- /dev/null +++ b/tests/data/cache-policy/policies/cache-policy-basic.yaml @@ -0,0 +1,12 @@ +apiVersion: k8s.nginx.org/v1 +kind: Policy +metadata: + name: cache-policy-basic +spec: + cache: + cacheZoneName: "basiccache" + cacheZoneSize: "5m" + allowedCodes: ["any"] + allowedMethods: ["GET"] + time: "30m" + overrideUpstreamCache: true diff --git a/tests/data/cache-policy/policies/cache-policy-purge.yaml b/tests/data/cache-policy/policies/cache-policy-purge.yaml new file mode 100644 index 0000000000..9e6a34d4fe --- /dev/null +++ b/tests/data/cache-policy/policies/cache-policy-purge.yaml @@ -0,0 +1,12 @@ +apiVersion: k8s.nginx.org/v1 +kind: Policy +metadata: + name: cache-policy-purge +spec: + cache: + cacheZoneName: "mycache" + cacheZoneSize: "5m" + allowedCodes: ["any"] + time: "10m" + overrideUpstreamCache: true + cachePurgeAllow: ["0.0.0.0/0"] diff --git a/tests/data/cache-policy/route/virtual-server-cache-policy-advanced-route.yaml b/tests/data/cache-policy/route/virtual-server-cache-policy-advanced-route.yaml new file mode 100644 index 0000000000..3bf79d2ad2 --- /dev/null +++ b/tests/data/cache-policy/route/virtual-server-cache-policy-advanced-route.yaml @@ -0,0 +1,24 @@ +apiVersion: k8s.nginx.org/v1 +kind: VirtualServer +metadata: + name: virtual-server +spec: + server-snippets: | + add_header X-Cache-Status $upstream_cache_status; + host: virtual-server.example.com + upstreams: + - name: backend2 + service: backend2-svc + port: 80 + - name: backend1 + service: backend1-svc + port: 80 + routes: + - path: "/backend1" + policies: + - name: cache-policy-advanced + action: + pass: backend1 + - path: "/backend2" + action: + pass: backend2 diff --git a/tests/data/cache-policy/route/virtual-server-cache-policy-basic-route.yaml b/tests/data/cache-policy/route/virtual-server-cache-policy-basic-route.yaml new file mode 100644 index 0000000000..11f1877a0d --- /dev/null +++ b/tests/data/cache-policy/route/virtual-server-cache-policy-basic-route.yaml @@ -0,0 +1,24 @@ +apiVersion: k8s.nginx.org/v1 +kind: VirtualServer +metadata: + name: virtual-server +spec: + server-snippets: | + add_header X-Cache-Status $upstream_cache_status; + host: virtual-server.example.com + upstreams: + - name: backend2 + service: backend2-svc + port: 80 + - name: backend1 + service: backend1-svc + port: 80 + routes: + - path: "/backend1" + policies: + - name: cache-policy-basic + action: + pass: backend1 + - path: "/backend2" + action: + pass: backend2 diff --git a/tests/data/cache-policy/spec/virtual-server-cache-policy-advanced-spec.yaml b/tests/data/cache-policy/spec/virtual-server-cache-policy-advanced-spec.yaml new file mode 100644 index 0000000000..1b13317837 --- /dev/null +++ b/tests/data/cache-policy/spec/virtual-server-cache-policy-advanced-spec.yaml @@ -0,0 +1,24 @@ +apiVersion: k8s.nginx.org/v1 +kind: VirtualServer +metadata: + name: virtual-server +spec: + server-snippets: | + add_header X-Cache-Status $upstream_cache_status; + host: virtual-server.example.com + policies: + - name: cache-policy-advanced + upstreams: + - name: backend2 + service: backend2-svc + port: 80 + - name: backend1 + service: backend1-svc + port: 80 + routes: + - path: "/backend1" + action: + pass: backend1 + - path: "/backend2" + action: + pass: backend2 diff --git a/tests/data/cache-policy/spec/virtual-server-cache-policy-basic-spec.yaml b/tests/data/cache-policy/spec/virtual-server-cache-policy-basic-spec.yaml new file mode 100644 index 0000000000..4c8b3f62c9 --- /dev/null +++ b/tests/data/cache-policy/spec/virtual-server-cache-policy-basic-spec.yaml @@ -0,0 +1,24 @@ +apiVersion: k8s.nginx.org/v1 +kind: VirtualServer +metadata: + name: virtual-server +spec: + server-snippets: | + add_header X-Cache-Status $upstream_cache_status; + host: virtual-server.example.com + policies: + - name: cache-policy-basic + upstreams: + - name: backend2 + service: backend2-svc + port: 80 + - name: backend1 + service: backend1-svc + port: 80 + routes: + - path: "/backend1" + action: + pass: backend1 + - path: "/backend2" + action: + pass: backend2 diff --git a/tests/data/cache-policy/spec/virtual-server-cache-policy-purge.yaml b/tests/data/cache-policy/spec/virtual-server-cache-policy-purge.yaml new file mode 100644 index 0000000000..d65da8633e --- /dev/null +++ b/tests/data/cache-policy/spec/virtual-server-cache-policy-purge.yaml @@ -0,0 +1,24 @@ +apiVersion: k8s.nginx.org/v1 +kind: VirtualServer +metadata: + name: virtual-server +spec: + server-snippets: | + add_header X-Cache-Status $upstream_cache_status; + host: virtual-server.example.com + policies: + - name: cache-policy-purge + upstreams: + - name: backend2 + service: backend2-svc + port: 80 + - name: backend1 + service: backend1-svc + port: 80 + routes: + - path: "/backend1" + action: + pass: backend1 + - path: "/backend2" + action: + pass: backend2 diff --git a/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-advanced.yaml b/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-advanced.yaml new file mode 100644 index 0000000000..817a8c33da --- /dev/null +++ b/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-advanced.yaml @@ -0,0 +1,22 @@ +apiVersion: k8s.nginx.org/v1 +kind: VirtualServerRoute +metadata: + name: backends +spec: + host: virtual-server-route.example.com + upstreams: + - name: backend1 + service: backend1-svc + port: 80 + - name: backend3 + service: backend3-svc + port: 80 + subroutes: + - path: "/backends/backend1" + policies: + - name: cache-policy-advanced + action: + pass: backend1 + - path: "/backends/backend3" + action: + pass: backend3 \ No newline at end of file diff --git a/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-basic.yaml b/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-basic.yaml new file mode 100644 index 0000000000..423b224178 --- /dev/null +++ b/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-basic.yaml @@ -0,0 +1,22 @@ +apiVersion: k8s.nginx.org/v1 +kind: VirtualServerRoute +metadata: + name: backends +spec: + host: virtual-server-route.example.com + upstreams: + - name: backend1 + service: backend1-svc + port: 80 + - name: backend3 + service: backend3-svc + port: 80 + subroutes: + - path: "/backends/backend1" + policies: + - name: cache-policy-basic + action: + pass: backend1 + - path: "/backends/backend3" + action: + pass: backend3 \ No newline at end of file diff --git a/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-purge.yaml b/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-purge.yaml new file mode 100644 index 0000000000..c197a91b9d --- /dev/null +++ b/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-purge.yaml @@ -0,0 +1,22 @@ +apiVersion: k8s.nginx.org/v1 +kind: VirtualServerRoute +metadata: + name: backends +spec: + host: virtual-server-route.example.com + upstreams: + - name: backend1 + service: backend1-svc + port: 80 + - name: backend3 + service: backend3-svc + port: 80 + subroutes: + - path: "/backends/backend1" + policies: + - name: cache-policy-purge + action: + pass: backend1 + - path: "/backends/backend3" + action: + pass: backend3 \ No newline at end of file diff --git a/tests/data/cache-policy/vsr/virtual-server-route-with-advanced-cache.yaml b/tests/data/cache-policy/vsr/virtual-server-route-with-advanced-cache.yaml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/data/cache-policy/vsr/virtual-server-route-with-cache.yaml b/tests/data/cache-policy/vsr/virtual-server-route-with-cache.yaml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/data/cache-policy/vsr/virtual-server-with-advanced-cache-route.yaml b/tests/data/cache-policy/vsr/virtual-server-with-advanced-cache-route.yaml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/data/cache-policy/vsr/virtual-server-with-cache-route.yaml b/tests/data/cache-policy/vsr/virtual-server-with-cache-route.yaml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/data/cache-policy/vsr/virtual-server.yaml b/tests/data/cache-policy/vsr/virtual-server.yaml new file mode 100644 index 0000000000..9f257c4c13 --- /dev/null +++ b/tests/data/cache-policy/vsr/virtual-server.yaml @@ -0,0 +1,13 @@ +apiVersion: k8s.nginx.org/v1 +kind: VirtualServer +metadata: + name: virtual-server-route +spec: + server-snippets: | + add_header X-Cache-Status $upstream_cache_status; + host: virtual-server-route.example.com + routes: + - path: "/backends" + route: backends # implicit namespace + - path: "/backend2" + route: backend2-namespace/backend2 diff --git a/tests/suite/test_cache_policies_vs.py b/tests/suite/test_cache_policies_vs.py new file mode 100644 index 0000000000..4ac7240265 --- /dev/null +++ b/tests/suite/test_cache_policies_vs.py @@ -0,0 +1,287 @@ +import re + +import pytest +import requests +from settings import TEST_DATA +from suite.utils.policy_resources_utils import create_policy_from_yaml, delete_policy +from suite.utils.resources_utils import ensure_connection_to_public_endpoint, pod_restart, wait_before_test +from suite.utils.vs_vsr_resources_utils import delete_and_create_vs_from_yaml + +std_vs_src = f"{TEST_DATA}/virtual-server/standard/virtual-server.yaml" +cache_pol_basic_src = f"{TEST_DATA}/cache-policy/policies/cache-policy-basic.yaml" +cache_pol_advanced_src = f"{TEST_DATA}/cache-policy/policies/cache-policy-advanced.yaml" +cache_pol_purge_src = f"{TEST_DATA}/cache-policy/policies/cache-policy-purge.yaml" +cache_vs_basic_spec_src = f"{TEST_DATA}/cache-policy/spec/virtual-server-cache-policy-basic-spec.yaml" +cache_vs_advanced_spec_src = f"{TEST_DATA}/cache-policy/spec/virtual-server-cache-policy-advanced-spec.yaml" +cache_vs_basic_route_src = f"{TEST_DATA}/cache-policy/route/virtual-server-cache-policy-basic-route.yaml" +cache_vs_advanced_route_src = f"{TEST_DATA}/cache-policy/route/virtual-server-cache-policy-advanced-route.yaml" +cache_vs_purge_src = f"{TEST_DATA}/cache-policy/spec/virtual-server-cache-policy-purge.yaml" + + +@pytest.mark.policies +@pytest.mark.policies_cache +@pytest.mark.parametrize( + "crd_ingress_controller, virtual_server_setup", + [ + ( + { + "type": "complete", + "extra_args": [f"-enable-custom-resources", f"-enable-leader-election=false", f"-enable-snippets"], + }, + { + "example": "virtual-server", + "app_type": "simple", + }, + ) + ], + indirect=True, +) +class TestCachePolicies: + def setup_cache_policy(self, kube_apis, test_namespace, policy_src): + print(f"Create cache policy") + pol_name = create_policy_from_yaml(kube_apis.custom_objects, policy_src, test_namespace) + wait_before_test() + return pol_name + + @pytest.mark.parametrize("src", [cache_vs_basic_spec_src, cache_vs_basic_route_src]) + def test_cache_policy_basic( + self, + kube_apis, + ingress_controller_prerequisites, + ingress_controller_endpoint, + crd_ingress_controller, + virtual_server_setup, + test_namespace, + src, + ): + """ + Test cache policy basic (GET only) configured at spec and route level + """ + + pol_name = self.setup_cache_policy(kube_apis, test_namespace, cache_pol_basic_src) + + # Apply VS with basic cache policy at spec level + delete_and_create_vs_from_yaml( + kube_apis.custom_objects, + virtual_server_setup.vs_name, + src, + test_namespace, + ) + + # Test cache behavior for GET requests + # First request should populate cache + resp_1 = requests.get(virtual_server_setup.backend_1_url, headers={"host": virtual_server_setup.vs_host}) + cache_status_1 = resp_1.headers.get("X-Cache-Status") + + # Second request should return cached content (same Request ID) + resp_2 = requests.get(virtual_server_setup.backend_1_url, headers={"host": virtual_server_setup.vs_host}) + cache_status_2 = resp_2.headers.get("X-Cache-Status") + + # POST requests should not be cached (different Request IDs expected) + resp_3 = requests.post(virtual_server_setup.backend_1_url, headers={"host": virtual_server_setup.vs_host}) + cache_status_3 = resp_3.headers.get("X-Cache-Status") + + resp_4 = requests.post(virtual_server_setup.backend_1_url, headers={"host": virtual_server_setup.vs_host}) + cache_status_4 = resp_4.headers.get("X-Cache-Status") + + # Extract Request IDs from response body + req_id_1 = re.search(r"Request ID: (\S+)", resp_1.text) + req_id_2 = re.search(r"Request ID: (\S+)", resp_2.text) + req_id_3 = re.search(r"Request ID: (\S+)", resp_3.text) + req_id_4 = re.search(r"Request ID: (\S+)", resp_4.text) + + assert all( + [ + resp_1.status_code == 200, + resp_2.status_code == 200, + resp_3.status_code == 200, + resp_4.status_code == 200, + "Request ID:" in resp_1.text, # Verify response body contains Request ID + "Request ID:" in resp_2.text, + "Request ID:" in resp_3.text, + "Request ID:" in resp_4.text, + ( + req_id_1.group(1) == req_id_2.group(1) if req_id_1 and req_id_2 else False + ), # GET requests cached (same Request ID) + ( + req_id_3.group(1) != req_id_4.group(1) if req_id_3 and req_id_4 else True + ), # POST requests not cached (different Request IDs) + cache_status_1 in ["MISS", "EXPIRED"], # First GET should be cache miss + cache_status_2 == "HIT", # Second GET should be cache hit + cache_status_3 in ["MISS", "EXPIRED", None], # POST should not be cached or use cached entry + cache_status_4 in ["MISS", "EXPIRED", None], # POST should not be cached + ] + ) + + delete_policy(kube_apis.custom_objects, pol_name, test_namespace) + delete_and_create_vs_from_yaml( + kube_apis.custom_objects, virtual_server_setup.vs_name, std_vs_src, test_namespace + ) + ns = ingress_controller_prerequisites.namespace + # Purge all existing cache entries by removing pods + pod_restart(kube_apis.v1, ns) + ensure_connection_to_public_endpoint( + ingress_controller_endpoint.public_ip, + ingress_controller_endpoint.port, + ingress_controller_endpoint.port_ssl, + ) + + @pytest.mark.parametrize("src", [cache_vs_advanced_spec_src, cache_vs_advanced_route_src]) + def test_cache_policy_advanced( + self, + kube_apis, + ingress_controller_prerequisites, + ingress_controller_endpoint, + crd_ingress_controller, + virtual_server_setup, + test_namespace, + src, + ): + """ + Test cache policy advanced (GET/HEAD/POST) configured at spec and route level + """ + + pol_name = self.setup_cache_policy(kube_apis, test_namespace, cache_pol_advanced_src) + + delete_and_create_vs_from_yaml( + kube_apis.custom_objects, + virtual_server_setup.vs_name, + src, + test_namespace, + ) + + # Test cache behavior for GET requests + resp_1 = requests.get(virtual_server_setup.backend_1_url, headers={"host": virtual_server_setup.vs_host}) + cache_status_1 = resp_1.headers.get("X-Cache-Status") + + resp_2 = requests.get(virtual_server_setup.backend_1_url, headers={"host": virtual_server_setup.vs_host}) + cache_status_2 = resp_2.headers.get("X-Cache-Status") + + # Test cache behavior for POST requests + resp_3 = requests.post(virtual_server_setup.backend_1_url, headers={"host": virtual_server_setup.vs_host}) + cache_status_3 = resp_3.headers.get("X-Cache-Status") + + # Test cache behavior for HEAD requests + resp_4 = requests.head(virtual_server_setup.backend_1_url, headers={"host": virtual_server_setup.vs_host}) + cache_status_4 = resp_4.headers.get("X-Cache-Status") + + # Extract Request IDs from response body (HEAD responses don't have body, use GET and POST) + req_id_1 = re.search(r"Request ID: (\S+)", resp_1.text) + req_id_2 = re.search(r"Request ID: (\S+)", resp_2.text) + req_id_3 = re.search(r"Request ID: (\S+)", resp_3.text) + + assert all( + [ + resp_1.status_code == 200, + resp_2.status_code == 200, + resp_3.status_code == 200, + resp_4.status_code == 200, + "Request ID:" in resp_1.text, + "Request ID:" in resp_2.text, + "Request ID:" in resp_3.text, + req_id_1.group(1) == req_id_2.group(1) == req_id_3.group(1), + cache_status_1 in ["MISS", "EXPIRED", None], + cache_status_2 == "HIT", + cache_status_3 == "HIT", + cache_status_4 == "HIT", + ] + ) + + # Cleanup + delete_policy(kube_apis.custom_objects, pol_name, test_namespace) + delete_and_create_vs_from_yaml( + kube_apis.custom_objects, virtual_server_setup.vs_name, std_vs_src, test_namespace + ) + + ns = ingress_controller_prerequisites.namespace + # Purge all existing cache entries by removing pods + pod_restart(kube_apis.v1, ns) + ensure_connection_to_public_endpoint( + ingress_controller_endpoint.public_ip, + ingress_controller_endpoint.port, + ingress_controller_endpoint.port_ssl, + ) + + @pytest.mark.skip_for_nginx_oss + def test_cache_policy_purge( + self, + kube_apis, + ingress_controller_prerequisites, + ingress_controller_endpoint, + crd_ingress_controller, + virtual_server_setup, + test_namespace, + ): + """ + Test cache policy purge + """ + + pol_name = self.setup_cache_policy(kube_apis, test_namespace, cache_pol_purge_src) + + delete_and_create_vs_from_yaml( + kube_apis.custom_objects, + virtual_server_setup.vs_name, + cache_vs_purge_src, + test_namespace, + ) + + # Test cache behavior for GET requests + resp_1 = requests.get(virtual_server_setup.backend_1_url, headers={"host": virtual_server_setup.vs_host}) + cache_status_1 = resp_1.headers.get("X-Cache-Status") + + resp_2 = requests.get(virtual_server_setup.backend_1_url, headers={"host": virtual_server_setup.vs_host}) + cache_status_2 = resp_2.headers.get("X-Cache-Status") + + # Purge request to remove cached content + # geo $purge_allowed_test_namespace_virtual_server_mycache { + # default 0; + # 0.0.0.0/0 1; + # } + resp_purge = requests.request( + "PURGE", virtual_server_setup.backend_1_url, headers={"host": virtual_server_setup.vs_host} + ) + + resp_3 = requests.get(virtual_server_setup.backend_1_url, headers={"host": virtual_server_setup.vs_host}) + cache_status_3 = resp_3.headers.get("X-Cache-Status") + + resp_4 = requests.get(virtual_server_setup.backend_1_url, headers={"host": virtual_server_setup.vs_host}) + cache_status_4 = resp_4.headers.get("X-Cache-Status") + + # Extract Request IDs from response body (HEAD responses don't have body, use GET and POST) + req_id_1 = re.search(r"Request ID: (\S+)", resp_1.text) + req_id_2 = re.search(r"Request ID: (\S+)", resp_2.text) + req_id_3 = re.search(r"Request ID: (\S+)", resp_3.text) + req_id_4 = re.search(r"Request ID: (\S+)", resp_4.text) + + assert all( + [ + resp_1.status_code == 200, + resp_2.status_code == 200, + resp_purge.status_code == 204, # PURGE should return 204 No Content + resp_3.status_code == 200, + resp_4.status_code == 200, + "Request ID:" in resp_1.text, + "Request ID:" in resp_2.text, + "Request ID:" in resp_3.text, + "Request ID:" in resp_4.text, + req_id_1.group(1) == req_id_2.group(1), + req_id_3.group(1) == req_id_4.group(1), + cache_status_1 == "MISS", + cache_status_2 == "HIT", + cache_status_3 == "MISS", # after PURGE, should be MISS + cache_status_4 == "HIT", + ] + ) + + delete_policy(kube_apis.custom_objects, pol_name, test_namespace) + delete_and_create_vs_from_yaml( + kube_apis.custom_objects, virtual_server_setup.vs_name, std_vs_src, test_namespace + ) + ns = ingress_controller_prerequisites.namespace + # Purge all existing cache entries by removing pods + pod_restart(kube_apis.v1, ns) + ensure_connection_to_public_endpoint( + ingress_controller_endpoint.public_ip, + ingress_controller_endpoint.port, + ingress_controller_endpoint.port_ssl, + ) diff --git a/tests/suite/test_cache_policies_vsr.py b/tests/suite/test_cache_policies_vsr.py new file mode 100644 index 0000000000..0b227ba516 --- /dev/null +++ b/tests/suite/test_cache_policies_vsr.py @@ -0,0 +1,220 @@ +import re + +import pytest +import requests +from settings import TEST_DATA +from suite.utils.custom_resources_utils import read_custom_resource +from suite.utils.policy_resources_utils import create_policy_from_yaml, delete_policy +from suite.utils.resources_utils import ensure_connection_to_public_endpoint, pod_restart, wait_before_test +from suite.utils.vs_vsr_resources_utils import delete_and_create_v_s_route_from_yaml, delete_and_create_vs_from_yaml + +std_vsr_src = f"{TEST_DATA}/virtual-server-route/route-multiple.yaml" +cache_pol_basic_src = f"{TEST_DATA}/cache-policy/policies/cache-policy-basic.yaml" +cache_pol_advanced_src = f"{TEST_DATA}/cache-policy/policies/cache-policy-advanced.yaml" +cache_vs_vsr_src = f"{TEST_DATA}/cache-policy/vsr/virtual-server.yaml" +cache_vsr_basic_src = f"{TEST_DATA}/cache-policy/vsr/virtual-server-route-cache-policy-basic.yaml" +cache_vsr_advanced_src = f"{TEST_DATA}/cache-policy/vsr/virtual-server-route-cache-policy-advanced.yaml" + + +@pytest.mark.policies +@pytest.mark.policies_cache +@pytest.mark.parametrize( + "crd_ingress_controller, v_s_route_setup", + [ + ( + { + "type": "complete", + "extra_args": [ + f"-enable-custom-resources", + f"-enable-leader-election=false", + f"-enable-snippets", + ], + }, + {"example": "virtual-server-route"}, + ) + ], + indirect=True, +) +class TestCachePoliciesVSR: + + def setup_vs_cache_policy(self, kube_apis, namespace, policy_src, vs_name): + print(f"Create cache policy") + pol_name = create_policy_from_yaml(kube_apis.custom_objects, policy_src, namespace) + print("Update Virtual Server with snippets") + delete_and_create_vs_from_yaml(kube_apis.custom_objects, vs_name, cache_vs_vsr_src, namespace) + wait_before_test() + return pol_name + + def test_cache_policy_vsr_basic( + self, + kube_apis, + ingress_controller_prerequisites, + ingress_controller_endpoint, + crd_ingress_controller, + v_s_route_app_setup, + v_s_route_setup, + test_namespace, + ): + """ + Test cache policy basic (GET only) applied to VirtualServerRoute + """ + + req_url = f"http://{v_s_route_setup.public_endpoint.public_ip}:{v_s_route_setup.public_endpoint.port}" + pol_name = self.setup_vs_cache_policy( + kube_apis, v_s_route_setup.route_m.namespace, cache_pol_basic_src, v_s_route_setup.vs_name + ) + + print(f"VSR with basic cache policy: {cache_vsr_basic_src}") + delete_and_create_v_s_route_from_yaml( + kube_apis.custom_objects, + v_s_route_setup.route_m.name, + cache_vsr_basic_src, + v_s_route_setup.route_m.namespace, + ) + wait_before_test() + + # Test cache behavior for GET requests on subroute + resp_1 = requests.get(f"{req_url}{v_s_route_setup.route_m.paths[0]}", headers={"host": v_s_route_setup.vs_host}) + cache_status_1 = resp_1.headers.get("X-Cache-Status") + print(f"Cache status for first GET request: {cache_status_1}") + + resp_2 = requests.get(f"{req_url}{v_s_route_setup.route_m.paths[0]}", headers={"host": v_s_route_setup.vs_host}) + cache_status_2 = resp_2.headers.get("X-Cache-Status") + print(f"Cache status for second GET request: {cache_status_2}") + + # POST requests should not be cached (basic policy allows GET only) + resp_3 = requests.post( + f"{req_url}{v_s_route_setup.route_m.paths[0]}", headers={"host": v_s_route_setup.vs_host} + ) + cache_status_3 = resp_3.headers.get("X-Cache-Status") + print(f"Cache status for POST request: {cache_status_3}") + + resp_4 = requests.post( + f"{req_url}{v_s_route_setup.route_m.paths[0]}", headers={"host": v_s_route_setup.vs_host} + ) + cache_status_4 = resp_4.headers.get("X-Cache-Status") + print(f"Cache status for second POST request: {cache_status_4}") + + # Extract Request IDs from response body + req_id_1 = re.search(r"Request ID: (\S+)", resp_1.text) + req_id_2 = re.search(r"Request ID: (\S+)", resp_2.text) + req_id_3 = re.search(r"Request ID: (\S+)", resp_3.text) + req_id_4 = re.search(r"Request ID: (\S+)", resp_4.text) + + assert all( + [ + resp_1.status_code == 200, + resp_2.status_code == 200, + resp_3.status_code == 200, + resp_4.status_code == 200, + "Request ID:" in resp_1.text, + "Request ID:" in resp_2.text, + "Request ID:" in resp_3.text, + "Request ID:" in resp_4.text, + req_id_1.group(1) == req_id_2.group(1), # GET requests cached (same Request ID) + req_id_3.group(1) != req_id_4.group(1), # POST requests not cached (different Request IDs) + cache_status_1 in ["MISS", "EXPIRED"], + cache_status_2 == "HIT", + cache_status_3 in ["MISS", "EXPIRED", None], + cache_status_4 in ["MISS", "EXPIRED", None], + ] + ) + + delete_policy(kube_apis.custom_objects, pol_name, v_s_route_setup.route_m.namespace) + delete_and_create_v_s_route_from_yaml( + kube_apis.custom_objects, v_s_route_setup.route_m.name, std_vsr_src, v_s_route_setup.route_m.namespace + ) + ns = ingress_controller_prerequisites.namespace + # Purge all existing cache entries by removing pods + pod_restart(kube_apis.v1, ns) + ensure_connection_to_public_endpoint( + ingress_controller_endpoint.public_ip, + ingress_controller_endpoint.port, + ingress_controller_endpoint.port_ssl, + ) + + def test_cache_policy_vsr_advanced( + self, + kube_apis, + ingress_controller_prerequisites, + ingress_controller_endpoint, + crd_ingress_controller, + v_s_route_app_setup, + v_s_route_setup, + test_namespace, + ): + """ + Test cache policy advanced (GET/HEAD/POST) applied to VirtualServerRoute + """ + + req_url = f"http://{v_s_route_setup.public_endpoint.public_ip}:{v_s_route_setup.public_endpoint.port}" + pol_name = self.setup_vs_cache_policy( + kube_apis, v_s_route_setup.route_m.namespace, cache_pol_advanced_src, v_s_route_setup.vs_name + ) + + print(f"VSR with advanced cache policy: {cache_vsr_advanced_src}") + delete_and_create_v_s_route_from_yaml( + kube_apis.custom_objects, + v_s_route_setup.route_m.name, + cache_vsr_advanced_src, + v_s_route_setup.route_m.namespace, + ) + wait_before_test() + + # Test cache behavior for GET requests + resp_1 = requests.get(f"{req_url}{v_s_route_setup.route_m.paths[0]}", headers={"host": v_s_route_setup.vs_host}) + cache_status_1 = resp_1.headers.get("X-Cache-Status") + print(f"Cache status for first GET request: {cache_status_1}") + + resp_2 = requests.get(f"{req_url}{v_s_route_setup.route_m.paths[0]}", headers={"host": v_s_route_setup.vs_host}) + cache_status_2 = resp_2.headers.get("X-Cache-Status") + print(f"Cache status for second GET request: {cache_status_2}") + + # Test cache behavior for POST requests (should be cached with advanced policy) + resp_3 = requests.post( + f"{req_url}{v_s_route_setup.route_m.paths[0]}", headers={"host": v_s_route_setup.vs_host} + ) + cache_status_3 = resp_3.headers.get("X-Cache-Status") + print(f"Cache status for first POST request: {cache_status_3}") + + # Test cache behavior for HEAD requests + resp_4 = requests.head( + f"{req_url}{v_s_route_setup.route_m.paths[0]}", headers={"host": v_s_route_setup.vs_host} + ) + cache_status_4 = resp_4.headers.get("X-Cache-Status") + print(f"Cache status for first HEAD request: {cache_status_4}") + + # Extract Request IDs from response body + req_id_1 = re.search(r"Request ID: (\S+)", resp_1.text) + req_id_2 = re.search(r"Request ID: (\S+)", resp_2.text) + req_id_3 = re.search(r"Request ID: (\S+)", resp_3.text) + + assert all( + [ + resp_1.status_code == 200, + resp_2.status_code == 200, + resp_3.status_code == 200, + resp_4.status_code == 200, + "Request ID:" in resp_1.text, + "Request ID:" in resp_2.text, + "Request ID:" in resp_3.text, + req_id_1.group(1) == req_id_2.group(1) == req_id_3.group(1), + cache_status_1 in ["MISS", "EXPIRED", None], + cache_status_2 == "HIT", + cache_status_3 == "HIT", + cache_status_4 == "HIT", + ] + ) + + delete_policy(kube_apis.custom_objects, pol_name, v_s_route_setup.route_m.namespace) + delete_and_create_v_s_route_from_yaml( + kube_apis.custom_objects, v_s_route_setup.route_m.name, std_vsr_src, v_s_route_setup.route_m.namespace + ) + ns = ingress_controller_prerequisites.namespace + # Purge all existing cache entries by removing pods + pod_restart(kube_apis.v1, ns) + ensure_connection_to_public_endpoint( + ingress_controller_endpoint.public_ip, + ingress_controller_endpoint.port, + ingress_controller_endpoint.port_ssl, + ) diff --git a/tests/suite/utils/resources_utils.py b/tests/suite/utils/resources_utils.py index 9a25cc0aea..4ec4072df1 100644 --- a/tests/suite/utils/resources_utils.py +++ b/tests/suite/utils/resources_utils.py @@ -2062,3 +2062,25 @@ def read_ingress(v1: NetworkingV1Api, name, namespace) -> V1Ingress: """ print(f"Read an ingress named '{name}'") return v1.read_namespaced_ingress(name, namespace) + + +def pod_restart(v1: CoreV1Api, namespace): + """ + Restart all pods in a deployment. + """ + try: + pods = v1.list_namespaced_pod(namespace=namespace) + + print(f"Found {len(pods.items)} pods to restart") + + # Delete all pods (they will be recreated by deployment) + for pod in pods.items: + print(f"Deleting pod {pod.metadata.name}") + v1.delete_namespaced_pod(name=pod.metadata.name, namespace=namespace) + + wait_until_all_pods_are_ready(v1, namespace) + print("Pod restart complete") + + except Exception as e: + print(f"Error in pod restart: {e}") + raise e From 23602f65c29d6ac97407ec39f232fc2e0c2b4bbd Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 12 Aug 2025 18:02:02 +0000 Subject: [PATCH 21/30] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- .../vsr/virtual-server-route-cache-policy-advanced.yaml | 2 +- .../vsr/virtual-server-route-cache-policy-basic.yaml | 2 +- .../vsr/virtual-server-route-cache-policy-purge.yaml | 2 +- tests/suite/test_cache_policies_vsr.py | 1 - 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-advanced.yaml b/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-advanced.yaml index 817a8c33da..caf52d05f1 100644 --- a/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-advanced.yaml +++ b/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-advanced.yaml @@ -19,4 +19,4 @@ spec: pass: backend1 - path: "/backends/backend3" action: - pass: backend3 \ No newline at end of file + pass: backend3 diff --git a/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-basic.yaml b/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-basic.yaml index 423b224178..f479fc4cbc 100644 --- a/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-basic.yaml +++ b/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-basic.yaml @@ -19,4 +19,4 @@ spec: pass: backend1 - path: "/backends/backend3" action: - pass: backend3 \ No newline at end of file + pass: backend3 diff --git a/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-purge.yaml b/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-purge.yaml index c197a91b9d..d380be2e76 100644 --- a/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-purge.yaml +++ b/tests/data/cache-policy/vsr/virtual-server-route-cache-policy-purge.yaml @@ -19,4 +19,4 @@ spec: pass: backend1 - path: "/backends/backend3" action: - pass: backend3 \ No newline at end of file + pass: backend3 diff --git a/tests/suite/test_cache_policies_vsr.py b/tests/suite/test_cache_policies_vsr.py index 0b227ba516..1947a00775 100644 --- a/tests/suite/test_cache_policies_vsr.py +++ b/tests/suite/test_cache_policies_vsr.py @@ -3,7 +3,6 @@ import pytest import requests from settings import TEST_DATA -from suite.utils.custom_resources_utils import read_custom_resource from suite.utils.policy_resources_utils import create_policy_from_yaml, delete_policy from suite.utils.resources_utils import ensure_connection_to_public_endpoint, pod_restart, wait_before_test from suite.utils.vs_vsr_resources_utils import delete_and_create_v_s_route_from_yaml, delete_and_create_vs_from_yaml From 54fa49dade1270cc9c6df14276d429dc004e76a3 Mon Sep 17 00:00:00 2001 From: Venktesh Date: Tue, 12 Aug 2025 21:45:40 +0100 Subject: [PATCH 22/30] update pytest marker --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 63f3570c69..0405d7d069 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -53,6 +53,7 @@ markers = [ "policies_jwt", "policies_ac", "policies_mtls", + "policies_cache", "rewrite", "skip_for_nginx_oss", "skip_for_loadbalancer", From 733485b3a80b23d1d73a521c1b82c29050c5e14f Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 13 Aug 2025 10:30:30 +0100 Subject: [PATCH 23/30] update pytest marker in ci and fix oss template --- .github/data/matrix-smoke-oss.json | 4 ++-- .github/data/matrix-smoke-plus.json | 4 ++-- internal/configs/version2/nginx.virtualserver.tmpl | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/data/matrix-smoke-oss.json b/.github/data/matrix-smoke-oss.json index c1491e62f8..a9e87bbb1d 100644 --- a/.github/data/matrix-smoke-oss.json +++ b/.github/data/matrix-smoke-oss.json @@ -39,14 +39,14 @@ "label": "policies 1/2", "image": "alpine", "type": "oss", - "marker": "'policies and not policies_rl and not policies_ac and not policies_jwt and not policies_mtls'", + "marker": "'policies and not policies_rl and not policies_ac and not policies_jwt and not policies_mtls and not policies_cache'", "platforms": "linux/arm64, linux/amd64" }, { "label": "policies 2/2", "image": "alpine", "type": "oss", - "marker": "'policies_rl or policies_ac or policies_jwt or policies_mtls or otel'", + "marker": "'policies_rl or policies_ac or policies_jwt or policies_mtls or policies_cache or otel'", "platforms": "linux/arm64, linux/amd64" }, { diff --git a/.github/data/matrix-smoke-plus.json b/.github/data/matrix-smoke-plus.json index 19f7cc1672..645d07517a 100644 --- a/.github/data/matrix-smoke-plus.json +++ b/.github/data/matrix-smoke-plus.json @@ -67,7 +67,7 @@ "label": "policies 1/3", "image": "ubi-9-plus", "type": "plus", - "marker": "'policies and not policies_ac and not policies_jwt and not policies_mtls and not policies_rl'", + "marker": "'policies and not policies_ac and not policies_jwt and not policies_mtls and not policies_rl and not policies_cache'", "platforms": "linux/arm64, linux/amd64" }, { @@ -81,7 +81,7 @@ "label": "policies 3/3", "image": "ubi-9-plus", "type": "plus", - "marker": "policies_rl", + "marker": "policies_rl or policies_cache", "platforms": "linux/arm64, linux/amd64" }, { diff --git a/internal/configs/version2/nginx.virtualserver.tmpl b/internal/configs/version2/nginx.virtualserver.tmpl index f7d229929e..bde55f4d9d 100644 --- a/internal/configs/version2/nginx.virtualserver.tmpl +++ b/internal/configs/version2/nginx.virtualserver.tmpl @@ -439,7 +439,7 @@ server { proxy_cache {{ $l.Cache.ZoneName }}; proxy_cache_key $scheme$proxy_host$request_uri; {{- if $l.Cache.OverrideUpstreamCache }} - proxy_ignore_headers Cache-Control Expires Set-Cookie Vary X-Accel-Expires;; + proxy_ignore_headers Cache-Control Expires Set-Cookie Vary X-Accel-Expires; {{- end }} {{- if and $l.Cache.Time (eq (len $l.Cache.Valid) 0) }} proxy_cache_valid {{ $l.Cache.Time }}; From 62182ac83ff4e3e8bdc10a7a9f55246a4ad83917 Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 13 Aug 2025 11:14:56 +0100 Subject: [PATCH 24/30] add template unit tests --- .../__snapshots__/templates_test.snap | 143 +++++++++++ internal/configs/version2/templates_test.go | 239 ++++++++++++++++++ 2 files changed, 382 insertions(+) diff --git a/internal/configs/version2/__snapshots__/templates_test.snap b/internal/configs/version2/__snapshots__/templates_test.snap index c7dc116be4..9f4c43c227 100644 --- a/internal/configs/version2/__snapshots__/templates_test.snap +++ b/internal/configs/version2/__snapshots__/templates_test.snap @@ -3493,3 +3493,146 @@ server { } --- + +[TestExecuteVirtualServerTemplateWithCachePolicyOSS - 1] + +upstream test-upstream {zone test-upstream ; + server 10.0.0.20:8001 max_fails=0 fail_timeout= max_conns=0; +} + +proxy_cache_path /var/cache/nginx/test_cache_basic_cache levels=1:2 keys_zone=test_cache_basic_cache:10m; +proxy_cache_path /var/cache/nginx/test_cache_location_simple_cache keys_zone=test_cache_location_simple_cache:5m; +server { + listen 80; + listen [::]:80; + + + server_name example.com; + + set $resource_type "virtualserver"; + set $resource_name ""; + set $resource_namespace ""; + + server_tokens "off"; + # Server-level cache configuration + proxy_cache test_cache_basic_cache; + proxy_cache_key $scheme$proxy_host$request_uri; + proxy_ignore_headers Cache-Control Expires Set-Cookie Vary X-Accel-Expires; + proxy_cache_valid any 1h; + proxy_cache_methods GET HEAD; + + + + + location / { + set $service ""; + + + set $default_connection_header close; + proxy_connect_timeout ; + proxy_read_timeout ; + proxy_send_timeout ; + client_max_body_size ; + + proxy_buffering off; + proxy_http_version 1.1; + proxy_set_header Upgrade $http_upgrade; + proxy_set_header Connection $vs_connection_header; + proxy_pass_request_headers off; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_cache test_cache_location_simple_cache; + proxy_cache_key $scheme$proxy_host$request_uri; + proxy_cache_valid 200 30m; + proxy_cache_valid 404 30m; + proxy_pass http://test-upstream; + proxy_next_upstream ; + proxy_next_upstream_timeout ; + proxy_next_upstream_tries 0; + } +} + +--- + +[TestExecuteVirtualServerTemplateWithCachePolicyNGINXPlus - 1] + +upstream test-upstream { + zone test-upstream ; + server 10.0.0.20:8001 max_fails=0 fail_timeout= max_conns=0; +} + +proxy_cache_path /var/cache/nginx/test_cache_full_advanced levels=2:2 keys_zone=test_cache_full_advanced:50m; +proxy_cache_path /var/cache/nginx/test_cache_location_location_cache keys_zone=test_cache_location_location_cache:20m; +geo $purge_allowed_test_cache_full_advanced { + default 0; + 127.0.0.1 1; + 10.0.0.0/8 1; + 192.168.1.0/24 1; +} + +map $request_method $cache_purge_test_cache_full_advanced { + PURGE $purge_allowed_test_cache_full_advanced; + default 0; +} + +server { + listen 80; + listen [::]:80; + + + server_name example.com; + status_zone example.com; + set $resource_type "virtualserver"; + set $resource_name ""; + set $resource_namespace ""; + + server_tokens "off"; + # Server-level cache configuration + proxy_cache test_cache_full_advanced; + proxy_cache_key $scheme$proxy_host$request_uri; + proxy_ignore_headers Cache-Control Expires Set-Cookie Vary X-Accel-Expires; + proxy_cache_valid 200 2h; + proxy_cache_valid 301 2h; + proxy_cache_valid 404 2h; + proxy_cache_methods GET HEAD POST; + proxy_cache_purge $cache_purge_test_cache_full_advanced; + + + + + location / { + set $service ""; + status_zone ""; + + + set $default_connection_header close; + proxy_connect_timeout ; + proxy_read_timeout ; + proxy_send_timeout ; + client_max_body_size ; + + proxy_buffering off; + proxy_http_version 1.1; + proxy_set_header Upgrade $http_upgrade; + proxy_set_header Connection $vs_connection_header; + proxy_pass_request_headers off; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_cache test_cache_location_location_cache; + proxy_cache_key $scheme$proxy_host$request_uri; + proxy_cache_valid any 1h; + proxy_cache_methods GET HEAD; + proxy_pass http://test-upstream; + proxy_next_upstream ; + proxy_next_upstream_timeout ; + proxy_next_upstream_tries 0; + } +} + +--- diff --git a/internal/configs/version2/templates_test.go b/internal/configs/version2/templates_test.go index beaa511dfa..d81455d032 100644 --- a/internal/configs/version2/templates_test.go +++ b/internal/configs/version2/templates_test.go @@ -875,6 +875,125 @@ func TestExecuteVirtualServerTemplateWithOIDCAndPKCEPolicyNGINXPlus(t *testing.T t.Log(string(got)) } +func TestExecuteVirtualServerTemplateWithCachePolicyNGINXPlus(t *testing.T) { + t.Parallel() + executor := newTmplExecutorNGINXPlus(t) + got, err := executor.ExecuteVirtualServerTemplate(&virtualServerCfgWithCachePolicyNGINXPlus) + if err != nil { + t.Error(err) + } + + // Check cache zone declaration + expectedCacheZone := "proxy_cache_path /var/cache/nginx/test_cache_full_advanced levels=2:2 keys_zone=test_cache_full_advanced:50m;" + if !bytes.Contains(got, []byte(expectedCacheZone)) { + t.Errorf("Expected cache zone declaration: %s", expectedCacheZone) + } + + // Check cache purge configuration for NGINX Plus + expectedPurgeGeo := "geo $purge_allowed_test_cache_full_advanced {" + if !bytes.Contains(got, []byte(expectedPurgeGeo)) { + t.Errorf("Expected purge geo block: %s", expectedPurgeGeo) + } + + expectedPurgeMap := "map $request_method $cache_purge_test_cache_full_advanced {" + if !bytes.Contains(got, []byte(expectedPurgeMap)) { + t.Errorf("Expected purge map block: %s", expectedPurgeMap) + } + + // Check server-level cache configuration + expectedServerCacheDirectives := []string{ + "proxy_cache test_cache_full_advanced;", + "proxy_cache_key $scheme$proxy_host$request_uri;", + "proxy_ignore_headers Cache-Control Expires Set-Cookie Vary X-Accel-Expires;", + "proxy_cache_valid 200 2h;", + "proxy_cache_valid 404 2h;", + "proxy_cache_valid 301 2h;", + "proxy_cache_methods GET HEAD POST;", + "proxy_cache_purge $cache_purge_test_cache_full_advanced;", + } + + for _, directive := range expectedServerCacheDirectives { + if !bytes.Contains(got, []byte(directive)) { + t.Errorf("Expected server cache directive: %s", directive) + } + } + + // Check location-level cache configuration + expectedLocationCacheDirectives := []string{ + "proxy_cache test_cache_location_location_cache;", + "proxy_cache_valid any 1h;", + "proxy_cache_methods GET HEAD;", + } + + for _, directive := range expectedLocationCacheDirectives { + if !bytes.Contains(got, []byte(directive)) { + t.Errorf("Expected location cache directive: %s", directive) + } + } + + snaps.MatchSnapshot(t, string(got)) + t.Log(string(got)) +} + +func TestExecuteVirtualServerTemplateWithCachePolicyOSS(t *testing.T) { + t.Parallel() + executor := newTmplExecutorNGINX(t) + got, err := executor.ExecuteVirtualServerTemplate(&virtualServerCfgWithCachePolicyOSS) + if err != nil { + t.Error(err) + } + + // Check cache zone declaration + expectedCacheZone := "proxy_cache_path /var/cache/nginx/test_cache_basic_cache levels=1:2 keys_zone=test_cache_basic_cache:10m;" + if !bytes.Contains(got, []byte(expectedCacheZone)) { + t.Errorf("Expected cache zone declaration: %s", expectedCacheZone) + } + + // Ensure no purge configuration for OSS (cachePurgeAllow should be ignored) + if bytes.Contains(got, []byte("geo $purge_allowed")) { + t.Error("OSS template should not contain cache purge geo blocks") + } + + if bytes.Contains(got, []byte("map $request_method $cache_purge")) { + t.Error("OSS template should not contain cache purge map blocks") + } + + if bytes.Contains(got, []byte("proxy_cache_purge")) { + t.Error("OSS template should not contain proxy_cache_purge directive") + } + + // Check server-level cache configuration + expectedServerCacheDirectives := []string{ + "proxy_cache test_cache_basic_cache;", + "proxy_cache_key $scheme$proxy_host$request_uri;", + "proxy_ignore_headers Cache-Control Expires Set-Cookie Vary X-Accel-Expires;", + "proxy_cache_valid any 1h;", + "proxy_cache_methods GET HEAD;", + } + + for _, directive := range expectedServerCacheDirectives { + if !bytes.Contains(got, []byte(directive)) { + t.Errorf("Expected server cache directive: %s", directive) + } + } + + // Check location-level cache configuration + expectedLocationCacheDirectives := []string{ + "proxy_cache test_cache_location_simple_cache;", + "proxy_cache_valid 200 30m;", + "proxy_cache_valid 404 30m;", + } + + for _, directive := range expectedLocationCacheDirectives { + if !bytes.Contains(got, []byte(directive)) { + t.Errorf("Expected location cache directive: %s", directive) + } + } + + snaps.MatchSnapshot(t, string(got)) + t.Log(string(got)) +} + func vsConfig() VirtualServerConfig { return VirtualServerConfig{ LimitReqZones: []LimitReqZone{ @@ -2639,6 +2758,126 @@ var ( }, } + virtualServerCfgWithCachePolicyNGINXPlus = VirtualServerConfig{ + CacheZones: []CacheZone{ + { + Name: "test_cache_full_advanced", + Size: "50m", + Path: "/var/cache/nginx/test_cache_full_advanced", + Levels: "2:2", + }, + { + Name: "test_cache_location_location_cache", + Size: "20m", + Path: "/var/cache/nginx/test_cache_location_location_cache", + Levels: "", + }, + }, + Upstreams: []Upstream{ + { + Name: "test-upstream", + Servers: []UpstreamServer{ + { + Address: "10.0.0.20:8001", + }, + }, + }, + }, + Server: Server{ + ServerName: "example.com", + StatusZone: "example.com", + ServerTokens: "off", + // Server-level cache policy with all advanced options (NGINX Plus) + Cache: &Cache{ + ZoneName: "test_cache_full_advanced", + ZoneSize: "50m", + Time: "2h", + Valid: map[string]string{"200": "2h", "404": "2h", "301": "2h"}, + AllowedMethods: []string{"GET", "HEAD", "POST"}, + CachePurgeAllow: []string{"127.0.0.1", "10.0.0.0/8", "192.168.1.0/24"}, + OverrideUpstreamCache: true, + Levels: "2:2", + }, + Locations: []Location{ + { + Path: "/", + ProxyPass: "http://test-upstream", + // Location-level cache policy with basic options + Cache: &Cache{ + ZoneName: "test_cache_location_location_cache", + ZoneSize: "20m", + Time: "1h", + Valid: map[string]string{"any": "1h"}, + AllowedMethods: []string{"GET", "HEAD"}, + CachePurgeAllow: nil, + OverrideUpstreamCache: false, + Levels: "", + }, + }, + }, + }, + } + + virtualServerCfgWithCachePolicyOSS = VirtualServerConfig{ + CacheZones: []CacheZone{ + { + Name: "test_cache_basic_cache", + Size: "10m", + Path: "/var/cache/nginx/test_cache_basic_cache", + Levels: "1:2", + }, + { + Name: "test_cache_location_simple_cache", + Size: "5m", + Path: "/var/cache/nginx/test_cache_location_simple_cache", + Levels: "", + }, + }, + Upstreams: []Upstream{ + { + Name: "test-upstream", + Servers: []UpstreamServer{ + { + Address: "10.0.0.20:8001", + }, + }, + }, + }, + Server: Server{ + ServerName: "example.com", + StatusZone: "example.com", + ServerTokens: "off", + // Server-level cache policy with basic options (OSS) + Cache: &Cache{ + ZoneName: "test_cache_basic_cache", + ZoneSize: "10m", + Time: "1h", + Valid: map[string]string{"any": "1h"}, + AllowedMethods: []string{"GET", "HEAD"}, + CachePurgeAllow: []string{"127.0.0.1"}, // This should be ignored for OSS + OverrideUpstreamCache: true, + Levels: "1:2", + }, + Locations: []Location{ + { + Path: "/", + ProxyPass: "http://test-upstream", + // Location-level cache policy with specific status codes + Cache: &Cache{ + ZoneName: "test_cache_location_simple_cache", + ZoneSize: "5m", + Time: "30m", + Valid: map[string]string{"200": "30m", "404": "30m"}, + AllowedMethods: nil, + CachePurgeAllow: nil, + OverrideUpstreamCache: false, + Levels: "", + }, + }, + }, + }, + } + transportServerCfg = TransportServerConfig{ Upstreams: []StreamUpstream{ { From 0b6abaa6d50c7563c40f09e7969f5a4825beec4c Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 13 Aug 2025 12:41:05 +0100 Subject: [PATCH 25/30] consolidate policy unit tests --- .../configuration/validation/policy_test.go | 339 ++++++++---------- 1 file changed, 155 insertions(+), 184 deletions(-) diff --git a/pkg/apis/configuration/validation/policy_test.go b/pkg/apis/configuration/validation/policy_test.go index 32227b65ca..24339073dc 100644 --- a/pkg/apis/configuration/validation/policy_test.go +++ b/pkg/apis/configuration/validation/policy_test.go @@ -1,7 +1,6 @@ package validation import ( - "strings" "testing" v1 "github.com/nginx/kubernetes-ingress/pkg/apis/configuration/v1" @@ -2427,185 +2426,142 @@ func TestValidateWAF_FailsOnInvalidApLogBundle(t *testing.T) { } } -func TestValidateCache(t *testing.T) { +func TestValidatePolicy_IsNotValidCachePolicy(t *testing.T) { t.Parallel() - // Helper function to create Cache with mixed allowedCodes from simple types - createCache := func(zoneName, zoneSize string, allowedCodes []interface{}, purgeAllow []string) *v1.Cache { - cache := &v1.Cache{ - CacheZoneName: zoneName, - CacheZoneSize: zoneSize, - CachePurgeAllow: purgeAllow, - } - - // Convert simple types to IntOrString - if len(allowedCodes) > 0 { - for _, code := range allowedCodes { - switch v := code.(type) { - case string: - cache.AllowedCodes = append(cache.AllowedCodes, intstr.FromString(v)) - case int: - cache.AllowedCodes = append(cache.AllowedCodes, intstr.FromInt(v)) - } - } - } - - return cache - } - - tests := []struct { - name string - cache *v1.Cache - isPlus bool - expectValid bool - expectedError string + tt := []struct { + name string + policy *v1.Policy + isPlus bool }{ - // Valid cache configurations - { - name: "valid cache with purge (NGINX Plus)", - cache: createCache("purgecache", "20m", nil, []string{"192.168.1.0/24", "10.0.0.1"}), - isPlus: true, - expectValid: true, - }, - { - name: "valid IPv6 address in purge allow", - cache: createCache("test", "10m", nil, []string{"2001:db8::1"}), - isPlus: true, - expectValid: true, - }, - { - name: "valid cache without purge features", - cache: createCache("basiccache", "10m", nil, nil), - isPlus: false, - expectValid: true, - }, - { - name: "valid allowedCodes with single 'any'", - cache: createCache("test", "10m", []interface{}{"any"}, nil), - isPlus: false, - expectValid: true, - }, - { - name: "valid allowedCodes with integers", - cache: createCache("test", "10m", []interface{}{200, 404, 500}, nil), - isPlus: false, - expectValid: true, - }, - { - name: "valid allowedCodes with edge case status codes", - cache: createCache("test", "10m", []interface{}{100, 599}, nil), - isPlus: false, - expectValid: true, - }, { - name: "valid empty allowedCodes", - cache: createCache("test", "10m", []interface{}{}, nil), - isPlus: false, - expectValid: true, - }, - - // Invalid cache configurations - { - name: "cache purge not allowed on OSS", - cache: createCache("purgeoss", "10m", nil, []string{"192.168.1.1"}), - isPlus: false, - expectValid: false, - expectedError: "cache purge is only supported in NGINX Plus", + name: "cache purge not allowed on OSS", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "purgeoss", + CacheZoneSize: "10m", + CachePurgeAllow: []string{"192.168.1.1"}, + }, + }, + }, + isPlus: false, }, { - name: "invalid IP address in purge allow", - cache: createCache("invalidip", "10m", nil, []string{"invalid-ip"}), - isPlus: true, - expectValid: false, - expectedError: "must be a valid IP address or CIDR", + name: "invalid IP address in purge allow", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "invalidip", + CacheZoneSize: "10m", + CachePurgeAllow: []string{"invalid-ip"}, + }, + }, + }, + isPlus: true, }, { - name: "allowedCodes with 'any' mixed with integers", - cache: createCache("test", "10m", []interface{}{"any", 200}, nil), - isPlus: false, - expectValid: false, - expectedError: "the string 'any' cannot be mixed with other codes", + name: "allowedCodes with 'any' mixed with integers", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + AllowedCodes: []intstr.IntOrString{intstr.FromString("any"), intstr.FromInt(200)}, + }, + }, + }, + isPlus: false, }, { - name: "allowedCodes with invalid string", - cache: createCache("test", "10m", []interface{}{"invalid"}, nil), - isPlus: false, - expectValid: false, - expectedError: "must be an integer HTTP status code (100-599) or the single string 'any'", + name: "allowedCodes with invalid string", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + AllowedCodes: []intstr.IntOrString{intstr.FromString("invalid")}, + }, + }, + }, + isPlus: false, }, { - name: "allowedCodes with status code below 100", - cache: createCache("test", "10m", []interface{}{99}, nil), - isPlus: false, - expectValid: false, - expectedError: "HTTP status code must be between 100 and 599", + name: "allowedCodes with status code below 100", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + AllowedCodes: []intstr.IntOrString{intstr.FromInt(99)}, + }, + }, + }, + isPlus: false, }, { - name: "allowedCodes with status code above 599", - cache: createCache("test", "10m", []interface{}{600}, nil), - isPlus: false, - expectValid: false, - expectedError: "HTTP status code must be between 100 and 599", + name: "allowedCodes with status code above 599", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + AllowedCodes: []intstr.IntOrString{intstr.FromInt(600)}, + }, + }, + }, + isPlus: false, }, { - name: "allowedCodes with multiple 'any' strings", - cache: createCache("test", "10m", []interface{}{"any", "any"}, nil), - isPlus: false, - expectValid: false, - expectedError: "the string 'any' cannot be mixed with other codes", + name: "allowedCodes with multiple 'any' strings", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + AllowedCodes: []intstr.IntOrString{intstr.FromString("any"), intstr.FromString("any")}, + }, + }, + }, + isPlus: false, }, { - name: "allowedCodes with valid and invalid status codes", - cache: createCache("test", "10m", []interface{}{200, 700}, nil), - isPlus: false, - expectValid: false, - expectedError: "HTTP status code must be between 100 and 599", + name: "allowedCodes with valid and invalid status codes", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "test", + CacheZoneSize: "10m", + AllowedCodes: []intstr.IntOrString{intstr.FromInt(200), intstr.FromInt(700)}, + }, + }, + }, + isPlus: false, }, } - for _, test := range tests { - test := test - t.Run(test.name, func(t *testing.T) { + for _, tc := range tt { + tc := tc + t.Run(tc.name, func(t *testing.T) { t.Parallel() - - allErrs := validateCache(test.cache, field.NewPath("cache"), test.isPlus) - - if test.expectValid { - if len(allErrs) > 0 { - t.Errorf("Expected no validation errors for valid cache, got: %v", allErrs) - } - } else { - if len(allErrs) == 0 { - t.Errorf("Expected validation error containing '%s', got no errors", test.expectedError) - return - } - - found := false - for _, err := range allErrs { - if strings.Contains(err.Detail, test.expectedError) { - found = true - break - } - } - - if !found { - t.Errorf("Expected validation error containing '%s', got: %v", test.expectedError, allErrs) - } + err := ValidatePolicy(tc.policy, tc.isPlus, false, false) + if err == nil { + t.Errorf("got no errors on invalid Cache policy spec input") } }) } } -func TestValidatePolicy_CachePolicy(t *testing.T) { +func TestValidatePolicy_IsValidCachePolicy(t *testing.T) { t.Parallel() - validPolicyTests := []struct { + tt := []struct { name string policy *v1.Policy isPlus bool }{ { - name: "valid cache policy basic", + name: "basic cache policy", policy: &v1.Policy{ Spec: v1.PolicySpec{ Cache: &v1.Cache{ @@ -2617,7 +2573,7 @@ func TestValidatePolicy_CachePolicy(t *testing.T) { isPlus: false, }, { - name: "valid cache policy with all options", + name: "cache policy with all options", policy: &v1.Policy{ Spec: v1.PolicySpec{ Cache: &v1.Cache{ @@ -2634,7 +2590,7 @@ func TestValidatePolicy_CachePolicy(t *testing.T) { isPlus: false, }, { - name: "valid cache policy with purge (NGINX Plus)", + name: "cache policy with purge (NGINX Plus)", policy: &v1.Policy{ Spec: v1.PolicySpec{ Cache: &v1.Cache{ @@ -2646,50 +2602,66 @@ func TestValidatePolicy_CachePolicy(t *testing.T) { }, isPlus: true, }, - } - - for _, test := range validPolicyTests { - test := test - t.Run(test.name, func(t *testing.T) { - t.Parallel() - - err := ValidatePolicy(test.policy, test.isPlus, false, false) - if err != nil { - t.Errorf("Expected valid cache policy, got error: %v", err) - } - }) - } - - invalidPolicyTests := []struct { - name string - policy *v1.Policy - isPlus bool - }{ { - name: "multiple policies defined (cache + access control)", + name: "cache policy with IPv6 purge addresses", policy: &v1.Policy{ Spec: v1.PolicySpec{ Cache: &v1.Cache{ - CacheZoneName: "cache1", - CacheZoneSize: "10m", + CacheZoneName: "ipv6cache", + CacheZoneSize: "20m", + CachePurgeAllow: []string{"2001:db8::1", "fe80::/64"}, }, - AccessControl: &v1.AccessControl{ - Allow: []string{"127.0.0.1"}, + }, + }, + isPlus: true, + }, + { + name: "cache policy with specific allowed codes", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "codecache", + CacheZoneSize: "15m", + AllowedCodes: []intstr.IntOrString{intstr.FromInt(200), intstr.FromInt(404), intstr.FromInt(500)}, }, }, }, isPlus: false, }, { - name: "multiple policies defined (cache + rate limit)", + name: "cache policy with edge case status codes", policy: &v1.Policy{ Spec: v1.PolicySpec{ Cache: &v1.Cache{ - CacheZoneName: "cache2", - CacheZoneSize: "10m", + CacheZoneName: "edgecase", + CacheZoneSize: "5m", + AllowedCodes: []intstr.IntOrString{intstr.FromInt(100), intstr.FromInt(599)}, }, - RateLimit: &v1.RateLimit{ - Rate: "10r/s", + }, + }, + isPlus: false, + }, + { + name: "cache policy with purge and CIDR range", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "cidrpurge", + CacheZoneSize: "20m", + CachePurgeAllow: []string{"192.168.1.0/24", "10.0.0.1"}, + }, + }, + }, + isPlus: true, + }, + { + name: "cache policy with empty allowed codes", + policy: &v1.Policy{ + Spec: v1.PolicySpec{ + Cache: &v1.Cache{ + CacheZoneName: "emptycode", + CacheZoneSize: "10m", + AllowedCodes: []intstr.IntOrString{}, }, }, }, @@ -2697,14 +2669,13 @@ func TestValidatePolicy_CachePolicy(t *testing.T) { }, } - for _, test := range invalidPolicyTests { - test := test - t.Run(test.name, func(t *testing.T) { + for _, tc := range tt { + tc := tc + t.Run(tc.name, func(t *testing.T) { t.Parallel() - - err := ValidatePolicy(test.policy, test.isPlus, false, false) - if err == nil { - t.Errorf("Expected invalid cache policy to return error, got none") + err := ValidatePolicy(tc.policy, tc.isPlus, false, false) + if err != nil { + t.Errorf("want no errors, got %+v\n", err) } }) } From a52d7a2514911b985aa1904a9cbab3650e3d7141 Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 13 Aug 2025 13:28:00 +0100 Subject: [PATCH 26/30] add example and readme --- .../custom-resources/cache-policy/README.md | 230 ++++++++++++++++++ .../custom-resources/cache-policy/cache.yaml | 30 +-- .../cache-policy/cafe-virtual-server.yaml | 4 - 3 files changed, 238 insertions(+), 26 deletions(-) create mode 100644 examples/custom-resources/cache-policy/README.md diff --git a/examples/custom-resources/cache-policy/README.md b/examples/custom-resources/cache-policy/README.md new file mode 100644 index 0000000000..07abfa3602 --- /dev/null +++ b/examples/custom-resources/cache-policy/README.md @@ -0,0 +1,230 @@ +# Cache Policy + +In this example, we deploy a web application, configure load balancing for it via a VirtualServer, and apply a cache +policy to improve performance by caching responses. + +## Prerequisites + +1. Follow the [installation](https://docs.nginx.com/nginx-ingress-controller/installation/) instructions to deploy the Ingress Controller. +1. Make sure the snippets are enabled (this is only required for this example as we can see the `X-Cache-Status` header in the response, not required for functionality). +1. Save the public IP address of the Ingress Controller into a shell variable: + + ```shell + IC_IP=XXX.YYY.ZZZ.III + ``` + +1. Save the HTTPS port of the Ingress Controller into a shell variable: + + ```shell + IC_HTTPS_PORT= + ``` + +## Step 1 - Deploy a Web Application + +Create the application deployment and service: + +```shell +kubectl apply -f cafe.yaml +``` + +## Step 2 - Create the TLS Secret + +Create a secret with the TLS certificate and key: + +```shell +kubectl apply -f cafe-secret.yaml +``` + +## Step 3 - Deploy the Cache Policy + +In this step, we create a policy with the name `cache-policy` that configures NGINX to cache responses for 30 minutes. + +Create the cache policy: + +```shell +kubectl apply -f cache.yaml +``` + +This policy configures: + +- A cache zone named `testcache` with a size of 15MB +- Caching for any response codes using `allowedCodes: ["any"]` +- Caching for GET, HEAD, and POST methods +- Cache duration of 30 minutes +- Override upstream cache headers with `overrideUpstreamCache: true`, to ignore upstream cache headers + +## Step 4 - Configure Load Balancing + +Create a VirtualServer resource for the web application: + +```shell +kubectl apply -f cafe-virtual-server.yaml +``` + +Note that the VirtualServer: + +- References the policy `cache-policy` created in Step 3 +- Includes a server snippet to add the `X-Cache-Status` header to responses +- This header shows whether responses are served from cache (HIT) or fetched from upstream (MISS) + +## Step 5 - Test the Configuration + +Let's test the caching behavior by making multiple requests to the same endpoint. + +### Test Cache MISS (First Request) + +Make the first request to the `/coffee` endpoint: + +```shell +curl --resolve cafe.example.com:$IC_HTTPS_PORT:$IC_IP https://cafe.example.com:$IC_HTTPS_PORT/coffee -I --insecure + +HTTP/1.1 200 OK +Server: nginx/1.27.4 +Date: Wed, 13 Aug 2025 12:11:34 GMT +Content-Type: text/plain +Content-Length: 160 +Connection: keep-alive +Expires: Wed, 13 Aug 2025 12:11:33 GMT +Cache-Control: no-cache +X-Cache-Status: MISS +``` + +The `X-Cache-Status: MISS` header indicates this response was fetched from the upstream server and is now cached. + +### Test Cache HIT (Subsequent Requests) + +Make the same request again within the cache duration: + +```shell +curl --resolve cafe.example.com:$IC_HTTPS_PORT:$IC_IP https://cafe.example.com:$IC_HTTPS_PORT/coffee -I --insecure + +HTTP/1.1 200 OK +Server: nginx/1.27.4 +Date: Wed, 13 Aug 2025 12:13:00 GMT +Content-Type: text/plain +Content-Length: 160 +Connection: keep-alive +Expires: Wed, 13 Aug 2025 12:11:33 GMT +Cache-Control: no-cache +X-Cache-Status: HIT +``` + +The `X-Cache-Status: HIT` header indicates this response was served from the cache, providing faster response times. + +### Test with Request ID for Full Response + +You can also view the full response to see the Request ID: + +```shell +curl --resolve cafe.example.com:$IC_HTTPS_PORT:$IC_IP https://cafe.example.com:$IC_HTTPS_PORT/coffee --insecure + +Server address: 10.0.0.215:8080 +Server name: coffee-676c9f8944-bhvxw +Date: 13/Aug/2025:12:11:34 +0000 +URI: /coffee +Request ID: c0ca10182c70590112c622835dd060f2 +``` + +```shell +curl --resolve cafe.example.com:$IC_HTTPS_PORT:$IC_IP https://cafe.example.com:$IC_HTTPS_PORT/coffee --insecure + +Server address: 10.0.0.215:8080 +Server name: coffee-676c9f8944-bhvxw +Date: 13/Aug/2025:12:11:34 +0000 +URI: /coffee +Request ID: c0ca10182c70590112c622835dd060f2 +``` + +When you make the same request again (while it's still cached), you'll get the same cached response with the same Request ID. + +### Test Different Endpoints + +Test the `/tea` endpoint to see cache behavior for different URLs: + +```shell +curl --resolve cafe.example.com:$IC_HTTPS_PORT:$IC_IP https://cafe.example.com:$IC_HTTPS_PORT/tea -I --insecure + +HTTP/1.1 200 OK +Server: nginx/1.27.4 +Date: Wed, 13 Aug 2025 12:16:16 GMT +Content-Type: text/plain +Content-Length: 154 +Connection: keep-alive +Expires: Wed, 13 Aug 2025 12:16:15 GMT +Cache-Control: no-cache +X-Cache-Status: MISS + +``` + +Each unique URL has its own cache entry, so the first request to `/tea` will show `MISS` even if `/coffee` is already cached. + +## Cache Configuration + +The cache policy supports additional configuration options: + +### Cache Purging (NGINX Plus Only) + +For NGINX Plus deployments, you can enable cache purging by adding IP addresses or CIDR ranges to the `cachePurgeAllow` field: + +```yaml +spec: + cache: + cacheZoneName: "testcache" + cacheZoneSize: "15m" + cachePurgeAllow: ["192.168.1.0/24", "10.0.0.1"] + # ... other configuration +``` + +```shell +curl --resolve cafe.example.com:$IC_HTTPS_PORT:$IC_IP https://cafe.example.com:$IC_HTTPS_PORT/coffee -I --insecure + +HTTP/1.1 200 OK +Server: nginx/1.27.4 +Date: Wed, 13 Aug 2025 12:22:07 GMT +Content-Type: text/plain +Content-Length: 160 +Connection: keep-alive +Expires: Wed, 13 Aug 2025 12:19:29 GMT +Cache-Control: no-cache +X-Cache-Status: HIT +``` + +```shell +curl --resolve cafe.example.com:$IC_HTTPS_PORT:$IC_IP https://cafe.example.com:$IC_HTTPS_PORT/coffee -I -X PURGE --insecure + +HTTP/1.1 204 No Content +Server: nginx/1.27.4 +Date: Wed, 13 Aug 2025 12:22:39 GMT +Connection: keep-alive +``` + +```shell +curl --resolve cafe.example.com:$IC_HTTPS_PORT:$IC_IP https://cafe.example.com:$IC_HTTPS_PORT/coffee -I --insecure + +HTTP/1.1 200 OK +Server: nginx/1.27.4 +Date: Wed, 13 Aug 2025 12:22:51 GMT +Content-Type: text/plain +Content-Length: 160 +Connection: keep-alive +Expires: Wed, 13 Aug 2025 12:22:50 GMT +Cache-Control: no-cache +X-Cache-Status: MISS +``` + +This allows authorized clients to purge cached content using the PURGE HTTP method. + +### Specific Response Codes + +Instead of caching all response codes with `["any"]`, you can specify particular codes: + +```yaml +spec: + cache: + cacheZoneName: "testcache" + cacheZoneSize: "15m" + allowedCodes: [200, 301, 404] + # ... other configuration +``` + +This configuration only caches responses with 200, 301, or 404 status codes. diff --git a/examples/custom-resources/cache-policy/cache.yaml b/examples/custom-resources/cache-policy/cache.yaml index 9d66f190f5..807dfb9194 100644 --- a/examples/custom-resources/cache-policy/cache.yaml +++ b/examples/custom-resources/cache-policy/cache.yaml @@ -4,25 +4,11 @@ metadata: name: cache-policy spec: cache: - cacheZoneName: "mycache" #Required - cacheZoneSize: "14m" #Required - allowedCodes: ["any"] #Optional ["any"] or ["200", "301", ...] - allowedMethods: ["GET", "HEAD", "POST"] #Optional - time: "25m" #Optional # e.g. "15m", "1h", "2d". Default is "10m" - # cachePurgeAllow: [""] - overrideUpstreamCache: false ---- -apiVersion: k8s.nginx.org/v1 -kind: Policy -metadata: - name: cache-policy2 -spec: - cache: - cacheZoneName: "mycache2" #Required - cacheZoneSize: "18m" #Required - allowedCodes: ["any"] #Optional - allowedMethods: ["GET"] #Optional - time: "15m" #Optional - # cachePurgeAllow: [""] - overrideUpstreamCache: true - levels: "1:2" # Optional, e.g. "1:2" or "2:2". This controls the number of subdirectory levels used for cache storage. + cacheZoneName: "testcache" # Required + cacheZoneSize: "15m" # Required + allowedCodes: ["any"] # Optional ["any"] or [200, 301, ...], "any" cannnot be combined with specific codes + allowedMethods: ["GET", "HEAD", "POST"] # Optional + time: "30m" # Optional # e.g. "15m", "1h", "2d". Default is "10m" + overrideUpstreamCache: true # Optional, default is false + # levels: "1:2" # Optional, default is "1:1" , see https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_cache_path for more details + # cachePurgeAllow: [""] # Optional, If set allows cache purging from the specified IPs or CIDR ranges. Nginx Plus only. diff --git a/examples/custom-resources/cache-policy/cafe-virtual-server.yaml b/examples/custom-resources/cache-policy/cafe-virtual-server.yaml index 0b49021d8f..8042866050 100644 --- a/examples/custom-resources/cache-policy/cafe-virtual-server.yaml +++ b/examples/custom-resources/cache-policy/cafe-virtual-server.yaml @@ -7,8 +7,6 @@ spec: add_header X-Cache-Status $upstream_cache_status; # This header will show the cache status for each request, e.g. X-Cache-Status: MISS or X-Cache-Status: HIT. # The cache status can be "HIT", "MISS", "EXPIRED", etc. - # This is useful for debugging and monitoring cache behavior but not required for cache functionality. - policies: - name: cache-policy host: cafe.example.com @@ -25,8 +23,6 @@ spec: - path: /tea action: pass: tea - policies: - - name: cache-policy2 - path: /coffee action: pass: coffee From 09c56c3caaad05dc243fd007bd7514d12b1f6b55 Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 13 Aug 2025 14:30:30 +0100 Subject: [PATCH 27/30] remove empty test files --- .../vsr/virtual-server-route-with-advanced-cache.yaml | 0 tests/data/cache-policy/vsr/virtual-server-route-with-cache.yaml | 0 .../vsr/virtual-server-with-advanced-cache-route.yaml | 0 tests/data/cache-policy/vsr/virtual-server-with-cache-route.yaml | 0 4 files changed, 0 insertions(+), 0 deletions(-) delete mode 100644 tests/data/cache-policy/vsr/virtual-server-route-with-advanced-cache.yaml delete mode 100644 tests/data/cache-policy/vsr/virtual-server-route-with-cache.yaml delete mode 100644 tests/data/cache-policy/vsr/virtual-server-with-advanced-cache-route.yaml delete mode 100644 tests/data/cache-policy/vsr/virtual-server-with-cache-route.yaml diff --git a/tests/data/cache-policy/vsr/virtual-server-route-with-advanced-cache.yaml b/tests/data/cache-policy/vsr/virtual-server-route-with-advanced-cache.yaml deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/tests/data/cache-policy/vsr/virtual-server-route-with-cache.yaml b/tests/data/cache-policy/vsr/virtual-server-route-with-cache.yaml deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/tests/data/cache-policy/vsr/virtual-server-with-advanced-cache-route.yaml b/tests/data/cache-policy/vsr/virtual-server-with-advanced-cache-route.yaml deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/tests/data/cache-policy/vsr/virtual-server-with-cache-route.yaml b/tests/data/cache-policy/vsr/virtual-server-with-cache-route.yaml deleted file mode 100644 index e69de29bb2..0000000000 From 7b4926f7796355dc86362ba7fdde99a171718f84 Mon Sep 17 00:00:00 2001 From: Venktesh Shivam Patel Date: Wed, 13 Aug 2025 14:32:40 +0100 Subject: [PATCH 28/30] update example readme with suggestion Co-authored-by: Paul Abel <128620221+pdabelf5@users.noreply.github.com> Signed-off-by: Venktesh Shivam Patel --- examples/custom-resources/cache-policy/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/custom-resources/cache-policy/README.md b/examples/custom-resources/cache-policy/README.md index 07abfa3602..d3082699a5 100644 --- a/examples/custom-resources/cache-policy/README.md +++ b/examples/custom-resources/cache-policy/README.md @@ -89,7 +89,7 @@ Cache-Control: no-cache X-Cache-Status: MISS ``` -The `X-Cache-Status: MISS` header indicates this response was fetched from the upstream server and is now cached. +The `X-Cache-Status: MISS` header indicates this response was fetched from the upstream server. The response is now cached. ### Test Cache HIT (Subsequent Requests) From eb94bd89925cf48a9e51572ebbbcfca06fdba8a3 Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 13 Aug 2025 15:25:46 +0100 Subject: [PATCH 29/30] remove pv-pvc yaml --- .../shared-cache/secure-shared-cache-pvc.yaml | 25 ------------------- 1 file changed, 25 deletions(-) delete mode 100644 examples/shared-cache/secure-shared-cache-pvc.yaml diff --git a/examples/shared-cache/secure-shared-cache-pvc.yaml b/examples/shared-cache/secure-shared-cache-pvc.yaml deleted file mode 100644 index 99ac0225d9..0000000000 --- a/examples/shared-cache/secure-shared-cache-pvc.yaml +++ /dev/null @@ -1,25 +0,0 @@ -apiVersion: v1 -kind: PersistentVolume -metadata: - name: nginx-shared-cache -spec: - storageClassName: manual - capacity: - storage: 1Gi - accessModes: - - ReadWriteOnce - hostPath: - path: "/tmp/" - ---- -apiVersion: v1 -kind: PersistentVolumeClaim -metadata: - name: nginx-shared-cache -spec: - storageClassName: manual - accessModes: - - ReadWriteOnce - resources: - requests: - storage: 1Gi From 5173313df89060bd2dc51085eca121956d5912ae Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 13 Aug 2025 16:21:59 +0100 Subject: [PATCH 30/30] fix test marker in workflow data --- .github/data/matrix-smoke-plus.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/data/matrix-smoke-plus.json b/.github/data/matrix-smoke-plus.json index 645d07517a..3750017dfd 100644 --- a/.github/data/matrix-smoke-plus.json +++ b/.github/data/matrix-smoke-plus.json @@ -81,7 +81,7 @@ "label": "policies 3/3", "image": "ubi-9-plus", "type": "plus", - "marker": "policies_rl or policies_cache", + "marker": "'policies_rl or policies_cache'", "platforms": "linux/arm64, linux/amd64" }, {