From 3a41c53c83276a2c6d3ea65ec26246a6df69a943 Mon Sep 17 00:00:00 2001 From: Paul Abel Date: Wed, 22 Oct 2025 11:10:41 +0100 Subject: [PATCH] Add resource reference validation function --- internal/configs/virtualserver.go | 8 ++--- internal/k8s/appprotect_waf.go | 10 +++--- .../app_protect_common_resources.go | 3 +- .../app_protect_dos_configuration.go | 12 +++---- internal/k8s/configuration.go | 3 +- internal/validation/validation.go | 5 +++ internal/validation/validation_test.go | 36 +++++++++++++++++++ 7 files changed, 59 insertions(+), 18 deletions(-) diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index d604c66777..55a6083790 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -17,6 +17,7 @@ import ( "github.com/nginx/kubernetes-ingress/internal/k8s/secrets" nl "github.com/nginx/kubernetes-ingress/internal/logger" "github.com/nginx/kubernetes-ingress/internal/nginx" + "github.com/nginx/kubernetes-ingress/internal/validation" conf_v1 "github.com/nginx/kubernetes-ingress/pkg/apis/configuration/v1" api_v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -537,7 +538,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( // ignore routes that reference VirtualServerRoute if r.Route != "" { name := r.Route - if !strings.Contains(name, "/") { + if !validation.HasNamespace(name) { name = fmt.Sprintf("%v/%v", vsEx.VirtualServer.Namespace, r.Route) } @@ -1671,8 +1672,7 @@ func (p *policiesCfg) addWAFConfig( if waf.ApPolicy != "" { apPolKey := waf.ApPolicy - hasNamespace := strings.Contains(apPolKey, "/") - if !hasNamespace { + if !validation.HasNamespace(apPolKey) { apPolKey = fmt.Sprintf("%v/%v", polNamespace, apPolKey) } @@ -1707,7 +1707,7 @@ func (p *policiesCfg) addWAFConfig( if loco.ApLogConf != "" { logConfKey := loco.ApLogConf - if !strings.Contains(logConfKey, "/") { + if !validation.HasNamespace(logConfKey) { logConfKey = fmt.Sprintf("%v/%v", polNamespace, logConfKey) } if logConfPath, ok := apResources.LogConfs[logConfKey]; ok { diff --git a/internal/k8s/appprotect_waf.go b/internal/k8s/appprotect_waf.go index 8ed8748f23..1dae78db4d 100644 --- a/internal/k8s/appprotect_waf.go +++ b/internal/k8s/appprotect_waf.go @@ -8,6 +8,7 @@ import ( "github.com/nginx/kubernetes-ingress/internal/k8s/appprotect" "github.com/nginx/kubernetes-ingress/internal/k8s/appprotectcommon" nl "github.com/nginx/kubernetes-ingress/internal/logger" + "github.com/nginx/kubernetes-ingress/internal/validation" conf_v1 "github.com/nginx/kubernetes-ingress/pkg/apis/configuration/v1" api_v1 "k8s.io/api/core/v1" networking "k8s.io/api/networking/v1" @@ -249,8 +250,7 @@ func getWAFPoliciesForAppProtectLogConf(pols []*conf_v1.Policy, key string) []*c } func isMatchingResourceRef(ownerNs, resRef, key string) bool { - hasNamespace := strings.Contains(resRef, "/") - if !hasNamespace { + if !validation.HasNamespace(resRef) { resRef = fmt.Sprintf("%v/%v", ownerNs, resRef) } return resRef == key @@ -269,7 +269,7 @@ func (lbc *LoadBalancerController) addWAFPolicyRefs( if pol.Spec.WAF.ApPolicy != "" { apPolKey := pol.Spec.WAF.ApPolicy - if !strings.Contains(pol.Spec.WAF.ApPolicy, "/") { + if !validation.HasNamespace(apPolKey) { apPolKey = fmt.Sprintf("%v/%v", pol.Namespace, apPolKey) } @@ -283,7 +283,7 @@ func (lbc *LoadBalancerController) addWAFPolicyRefs( if pol.Spec.WAF.SecurityLog != nil && pol.Spec.WAF.SecurityLogs == nil { if pol.Spec.WAF.SecurityLog.ApLogConf != "" { logConfKey := pol.Spec.WAF.SecurityLog.ApLogConf - if !strings.Contains(pol.Spec.WAF.SecurityLog.ApLogConf, "/") { + if !validation.HasNamespace(logConfKey) { logConfKey = fmt.Sprintf("%v/%v", pol.Namespace, logConfKey) } @@ -299,7 +299,7 @@ func (lbc *LoadBalancerController) addWAFPolicyRefs( for _, SecLog := range pol.Spec.WAF.SecurityLogs { if SecLog.ApLogConf != "" { logConfKey := SecLog.ApLogConf - if !strings.Contains(SecLog.ApLogConf, "/") { + if !validation.HasNamespace(logConfKey) { logConfKey = fmt.Sprintf("%v/%v", pol.Namespace, logConfKey) } diff --git a/internal/k8s/appprotectcommon/app_protect_common_resources.go b/internal/k8s/appprotectcommon/app_protect_common_resources.go index 42e30105d1..cefaa2701a 100644 --- a/internal/k8s/appprotectcommon/app_protect_common_resources.go +++ b/internal/k8s/appprotectcommon/app_protect_common_resources.go @@ -3,6 +3,7 @@ package appprotectcommon import ( "strings" + "github.com/nginx/kubernetes-ingress/internal/validation" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -13,7 +14,7 @@ func GetNsName(obj *unstructured.Unstructured) string { // ParseResourceReferenceAnnotation returns a namespace/name string func ParseResourceReferenceAnnotation(ns, antn string) string { - if !strings.Contains(antn, "/") { + if !validation.HasNamespace(antn) { return ns + "/" + antn } return antn diff --git a/internal/k8s/appprotectdos/app_protect_dos_configuration.go b/internal/k8s/appprotectdos/app_protect_dos_configuration.go index 19422f6295..a42d4c8744 100644 --- a/internal/k8s/appprotectdos/app_protect_dos_configuration.go +++ b/internal/k8s/appprotectdos/app_protect_dos_configuration.go @@ -3,11 +3,11 @@ package appprotectdos import ( "errors" "fmt" - "strings" "github.com/nginx/kubernetes-ingress/internal/configs" "github.com/nginx/kubernetes-ingress/internal/k8s/appprotectcommon" nl "github.com/nginx/kubernetes-ingress/internal/logger" + internalValidation "github.com/nginx/kubernetes-ingress/internal/validation" "github.com/nginx/kubernetes-ingress/pkg/apis/dos/v1beta1" "github.com/nginx/kubernetes-ingress/pkg/apis/dos/validation" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -169,7 +169,7 @@ func (ci *Configuration) AddOrUpdateDosProtectedResource(protectedConf *v1beta1. if protectedEx.Obj.Spec.ApDosPolicy != "" { policyReference := protectedEx.Obj.Spec.ApDosPolicy // if the policy reference does not have a namespace, use the dos protected' namespace - if !strings.Contains(policyReference, "/") { + if !internalValidation.HasNamespace(policyReference) { policyReference = protectedEx.Obj.Namespace + "/" + policyReference } _, err := ci.getPolicy(policyReference) @@ -181,7 +181,7 @@ func (ci *Configuration) AddOrUpdateDosProtectedResource(protectedConf *v1beta1. if protectedEx.Obj.Spec.DosSecurityLog != nil && protectedEx.Obj.Spec.DosSecurityLog.ApDosLogConf != "" { logConfReference := protectedEx.Obj.Spec.DosSecurityLog.ApDosLogConf // if the log conf reference does not have a namespace, use the dos protected' namespace - if !strings.Contains(logConfReference, "/") { + if !internalValidation.HasNamespace(logConfReference) { logConfReference = protectedEx.Obj.Namespace + "/" + logConfReference } _, err := ci.getLogConf(logConfReference) @@ -243,7 +243,7 @@ func (ci *Configuration) GetValidDosEx(parentNamespace string, nsName string) (* if protectedEx.Obj.Spec.ApDosPolicy != "" { policyReference := protectedEx.Obj.Spec.ApDosPolicy // if the policy reference does not have a namespace, use the dos protected' namespace - if !strings.Contains(policyReference, "/") { + if !internalValidation.HasNamespace(policyReference) { policyReference = protectedEx.Obj.Namespace + "/" + policyReference } pol, err := ci.getPolicy(policyReference) @@ -255,7 +255,7 @@ func (ci *Configuration) GetValidDosEx(parentNamespace string, nsName string) (* if protectedEx.Obj.Spec.DosSecurityLog != nil && protectedEx.Obj.Spec.DosSecurityLog.ApDosLogConf != "" { logConfReference := protectedEx.Obj.Spec.DosSecurityLog.ApDosLogConf // if the log conf reference does not have a namespace, use the dos protected' namespace - if !strings.Contains(logConfReference, "/") { + if !internalValidation.HasNamespace(logConfReference) { logConfReference = protectedEx.Obj.Namespace + "/" + logConfReference } log, err := ci.getLogConf(logConfReference) @@ -268,7 +268,7 @@ func (ci *Configuration) GetValidDosEx(parentNamespace string, nsName string) (* } func getNsName(defaultNamespace string, name string) string { - if !strings.Contains(name, "/") { + if !internalValidation.HasNamespace(name) { return defaultNamespace + "/" + name } return name diff --git a/internal/k8s/configuration.go b/internal/k8s/configuration.go index 0ca8050563..0ead6dca6a 100644 --- a/internal/k8s/configuration.go +++ b/internal/k8s/configuration.go @@ -4,7 +4,6 @@ import ( "fmt" "reflect" "sort" - "strings" "sync" "github.com/nginx/kubernetes-ingress/internal/configs" @@ -1660,7 +1659,7 @@ func (c *Configuration) buildVirtualServerRoutes(vs *conf_v1.VirtualServer) ([]* vsrKey := r.Route // if route is defined without a namespace, use the namespace of VirtualServer. - if !strings.Contains(r.Route, "/") { + if !internalValidation.HasNamespace(vsrKey) { vsrKey = fmt.Sprintf("%s/%s", vs.Namespace, r.Route) } diff --git a/internal/validation/validation.go b/internal/validation/validation.go index e62086e469..5891af268a 100644 --- a/internal/validation/validation.go +++ b/internal/validation/validation.go @@ -178,3 +178,8 @@ func ValidateURI(uri string, options ...URIValidationOption) error { return nil } + +// HasNamespace checks if the given string is a resource reference with a namespace (i.e., has a '/' character). +func HasNamespace(s string) bool { + return strings.Contains(s, "/") +} diff --git a/internal/validation/validation_test.go b/internal/validation/validation_test.go index ac2869651d..cc5811aa4d 100644 --- a/internal/validation/validation_test.go +++ b/internal/validation/validation_test.go @@ -205,3 +205,39 @@ func TestValidateURI(t *testing.T) { }) } } + +func TestHasNamespace(t *testing.T) { + tests := []struct { + resource string + withNamespace bool + description string + }{ + { + resource: "my-resource", + withNamespace: false, + description: "resource name without namespace", + }, + { + resource: "custom/my-resource", + withNamespace: true, + description: "resource name with namespace", + }, + { + resource: "default/my-resource", + withNamespace: true, + description: "resource name with default namespace", + }, + { + resource: "", + withNamespace: false, + description: "empty resource name", + }, + } + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + if r := HasNamespace(tt.resource); r != tt.withNamespace { + t.Errorf("HasNamespace() = %v, want %v", r, tt.withNamespace) + } + }) + } +}