Skip to content

Commit 4549d49

Browse files
pdabelf5pdabelf5
authored
Add resource reference validation function (#8454)
1 parent 08c3d9d commit 4549d49

File tree

8 files changed

+71
-20
lines changed

8 files changed

+71
-20
lines changed

‎internal/configs/virtualserver.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/nginx/kubernetes-ingress/internal/k8s/secrets"
1818
nl "github.com/nginx/kubernetes-ingress/internal/logger"
1919
"github.com/nginx/kubernetes-ingress/internal/nginx"
20+
"github.com/nginx/kubernetes-ingress/internal/nsutils"
2021
conf_v1 "github.com/nginx/kubernetes-ingress/pkg/apis/configuration/v1"
2122
api_v1 "k8s.io/api/core/v1"
2223
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -147,7 +148,7 @@ func GenerateEndpointsKey(
147148

148149
// ParseServiceReference returns the namespace and name from a service reference.
149150
func ParseServiceReference(serviceRef, defaultNamespace string) (namespace, serviceName string) {
150-
if strings.Contains(serviceRef, "/") {
151+
if nsutils.HasNamespace(serviceRef) {
151152
parts := strings.Split(serviceRef, "/")
152153
if len(parts) == 2 {
153154
return parts[0], parts[1]
@@ -552,7 +553,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(
552553
// ignore routes that reference VirtualServerRoute
553554
if r.Route != "" {
554555
name := r.Route
555-
if !strings.Contains(name, "/") {
556+
if !nsutils.HasNamespace(name) {
556557
name = fmt.Sprintf("%v/%v", vsEx.VirtualServer.Namespace, r.Route)
557558
}
558559

@@ -1732,8 +1733,7 @@ func (p *policiesCfg) addWAFConfig(
17321733

17331734
if waf.ApPolicy != "" {
17341735
apPolKey := waf.ApPolicy
1735-
hasNamespace := strings.Contains(apPolKey, "/")
1736-
if !hasNamespace {
1736+
if !nsutils.HasNamespace(apPolKey) {
17371737
apPolKey = fmt.Sprintf("%v/%v", polNamespace, apPolKey)
17381738
}
17391739

@@ -1768,7 +1768,7 @@ func (p *policiesCfg) addWAFConfig(
17681768

17691769
if loco.ApLogConf != "" {
17701770
logConfKey := loco.ApLogConf
1771-
if !strings.Contains(logConfKey, "/") {
1771+
if !nsutils.HasNamespace(logConfKey) {
17721772
logConfKey = fmt.Sprintf("%v/%v", polNamespace, logConfKey)
17731773
}
17741774
if logConfPath, ok := apResources.LogConfs[logConfKey]; ok {

‎internal/k8s/appprotect_waf.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"github.com/nginx/kubernetes-ingress/internal/k8s/appprotect"
99
"github.com/nginx/kubernetes-ingress/internal/k8s/appprotectcommon"
1010
nl "github.com/nginx/kubernetes-ingress/internal/logger"
11+
"github.com/nginx/kubernetes-ingress/internal/nsutils"
1112
conf_v1 "github.com/nginx/kubernetes-ingress/pkg/apis/configuration/v1"
1213
api_v1 "k8s.io/api/core/v1"
1314
networking "k8s.io/api/networking/v1"
@@ -249,8 +250,7 @@ func getWAFPoliciesForAppProtectLogConf(pols []*conf_v1.Policy, key string) []*c
249250
}
250251

251252
func isMatchingResourceRef(ownerNs, resRef, key string) bool {
252-
hasNamespace := strings.Contains(resRef, "/")
253-
if !hasNamespace {
253+
if !nsutils.HasNamespace(resRef) {
254254
resRef = fmt.Sprintf("%v/%v", ownerNs, resRef)
255255
}
256256
return resRef == key
@@ -269,7 +269,7 @@ func (lbc *LoadBalancerController) addWAFPolicyRefs(
269269

270270
if pol.Spec.WAF.ApPolicy != "" {
271271
apPolKey := pol.Spec.WAF.ApPolicy
272-
if !strings.Contains(pol.Spec.WAF.ApPolicy, "/") {
272+
if !nsutils.HasNamespace(apPolKey) {
273273
apPolKey = fmt.Sprintf("%v/%v", pol.Namespace, apPolKey)
274274
}
275275

@@ -283,7 +283,7 @@ func (lbc *LoadBalancerController) addWAFPolicyRefs(
283283
if pol.Spec.WAF.SecurityLog != nil && pol.Spec.WAF.SecurityLogs == nil {
284284
if pol.Spec.WAF.SecurityLog.ApLogConf != "" {
285285
logConfKey := pol.Spec.WAF.SecurityLog.ApLogConf
286-
if !strings.Contains(pol.Spec.WAF.SecurityLog.ApLogConf, "/") {
286+
if !nsutils.HasNamespace(logConfKey) {
287287
logConfKey = fmt.Sprintf("%v/%v", pol.Namespace, logConfKey)
288288
}
289289

@@ -299,7 +299,7 @@ func (lbc *LoadBalancerController) addWAFPolicyRefs(
299299
for _, SecLog := range pol.Spec.WAF.SecurityLogs {
300300
if SecLog.ApLogConf != "" {
301301
logConfKey := SecLog.ApLogConf
302-
if !strings.Contains(SecLog.ApLogConf, "/") {
302+
if !nsutils.HasNamespace(logConfKey) {
303303
logConfKey = fmt.Sprintf("%v/%v", pol.Namespace, logConfKey)
304304
}
305305

‎internal/k8s/appprotectcommon/app_protect_common_resources.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package appprotectcommon
33
import (
44
"strings"
55

6+
"github.com/nginx/kubernetes-ingress/internal/nsutils"
67
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
78
)
89

@@ -13,7 +14,7 @@ func GetNsName(obj *unstructured.Unstructured) string {
1314

1415
// ParseResourceReferenceAnnotation returns a namespace/name string
1516
func ParseResourceReferenceAnnotation(ns, antn string) string {
16-
if !strings.Contains(antn, "/") {
17+
if !nsutils.HasNamespace(antn) {
1718
return ns + "/" + antn
1819
}
1920
return antn

‎internal/k8s/appprotectdos/app_protect_dos_configuration.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ package appprotectdos
33
import (
44
"errors"
55
"fmt"
6-
"strings"
76

87
"github.com/nginx/kubernetes-ingress/internal/configs"
98
"github.com/nginx/kubernetes-ingress/internal/k8s/appprotectcommon"
109
nl "github.com/nginx/kubernetes-ingress/internal/logger"
10+
"github.com/nginx/kubernetes-ingress/internal/nsutils"
1111
"github.com/nginx/kubernetes-ingress/pkg/apis/dos/v1beta1"
1212
"github.com/nginx/kubernetes-ingress/pkg/apis/dos/validation"
1313
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -169,7 +169,7 @@ func (ci *Configuration) AddOrUpdateDosProtectedResource(protectedConf *v1beta1.
169169
if protectedEx.Obj.Spec.ApDosPolicy != "" {
170170
policyReference := protectedEx.Obj.Spec.ApDosPolicy
171171
// if the policy reference does not have a namespace, use the dos protected' namespace
172-
if !strings.Contains(policyReference, "/") {
172+
if !nsutils.HasNamespace(policyReference) {
173173
policyReference = protectedEx.Obj.Namespace + "/" + policyReference
174174
}
175175
_, err := ci.getPolicy(policyReference)
@@ -181,7 +181,7 @@ func (ci *Configuration) AddOrUpdateDosProtectedResource(protectedConf *v1beta1.
181181
if protectedEx.Obj.Spec.DosSecurityLog != nil && protectedEx.Obj.Spec.DosSecurityLog.ApDosLogConf != "" {
182182
logConfReference := protectedEx.Obj.Spec.DosSecurityLog.ApDosLogConf
183183
// if the log conf reference does not have a namespace, use the dos protected' namespace
184-
if !strings.Contains(logConfReference, "/") {
184+
if !nsutils.HasNamespace(logConfReference) {
185185
logConfReference = protectedEx.Obj.Namespace + "/" + logConfReference
186186
}
187187
_, err := ci.getLogConf(logConfReference)
@@ -243,7 +243,7 @@ func (ci *Configuration) GetValidDosEx(parentNamespace string, nsName string) (*
243243
if protectedEx.Obj.Spec.ApDosPolicy != "" {
244244
policyReference := protectedEx.Obj.Spec.ApDosPolicy
245245
// if the policy reference does not have a namespace, use the dos protected' namespace
246-
if !strings.Contains(policyReference, "/") {
246+
if !nsutils.HasNamespace(policyReference) {
247247
policyReference = protectedEx.Obj.Namespace + "/" + policyReference
248248
}
249249
pol, err := ci.getPolicy(policyReference)
@@ -255,7 +255,7 @@ func (ci *Configuration) GetValidDosEx(parentNamespace string, nsName string) (*
255255
if protectedEx.Obj.Spec.DosSecurityLog != nil && protectedEx.Obj.Spec.DosSecurityLog.ApDosLogConf != "" {
256256
logConfReference := protectedEx.Obj.Spec.DosSecurityLog.ApDosLogConf
257257
// if the log conf reference does not have a namespace, use the dos protected' namespace
258-
if !strings.Contains(logConfReference, "/") {
258+
if !nsutils.HasNamespace(logConfReference) {
259259
logConfReference = protectedEx.Obj.Namespace + "/" + logConfReference
260260
}
261261
log, err := ci.getLogConf(logConfReference)
@@ -268,7 +268,7 @@ func (ci *Configuration) GetValidDosEx(parentNamespace string, nsName string) (*
268268
}
269269

270270
func getNsName(defaultNamespace string, name string) string {
271-
if !strings.Contains(name, "/") {
271+
if !nsutils.HasNamespace(name) {
272272
return defaultNamespace + "/" + name
273273
}
274274
return name

‎internal/k8s/configuration.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@ import (
44
"fmt"
55
"reflect"
66
"sort"
7-
"strings"
87
"sync"
98

109
"github.com/nginx/kubernetes-ingress/internal/configs"
1110
nl "github.com/nginx/kubernetes-ingress/internal/logger"
11+
"github.com/nginx/kubernetes-ingress/internal/nsutils"
1212
internalValidation "github.com/nginx/kubernetes-ingress/internal/validation"
1313
conf_v1 "github.com/nginx/kubernetes-ingress/pkg/apis/configuration/v1"
1414
"github.com/nginx/kubernetes-ingress/pkg/apis/configuration/validation"
@@ -1660,7 +1660,7 @@ func (c *Configuration) buildVirtualServerRoutes(vs *conf_v1.VirtualServer) ([]*
16601660
vsrKey := r.Route
16611661

16621662
// if route is defined without a namespace, use the namespace of VirtualServer.
1663-
if !strings.Contains(r.Route, "/") {
1663+
if !nsutils.HasNamespace(vsrKey) {
16641664
vsrKey = fmt.Sprintf("%s/%s", vs.Namespace, r.Route)
16651665
}
16661666

‎internal/nsutils/utils.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
package nsutils
2+
3+
import (
4+
"strings"
5+
)
6+
7+
// HasNamespace checks if the given string is a resource reference with a namespace (i.e., has a '/' character).
8+
func HasNamespace(s string) bool {
9+
return strings.Contains(s, "/")
10+
}

‎internal/nsutils/utils_test.go‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package nsutils
2+
3+
import "testing"
4+
5+
func TestHasNamespace(t *testing.T) {
6+
tests := []struct {
7+
resource string
8+
withNamespace bool
9+
description string
10+
}{
11+
{
12+
resource: "my-resource",
13+
withNamespace: false,
14+
description: "resource name without namespace",
15+
},
16+
{
17+
resource: "custom/my-resource",
18+
withNamespace: true,
19+
description: "resource name with namespace",
20+
},
21+
{
22+
resource: "default/my-resource",
23+
withNamespace: true,
24+
description: "resource name with default namespace",
25+
},
26+
{
27+
resource: "",
28+
withNamespace: false,
29+
description: "empty resource name",
30+
},
31+
}
32+
for _, tt := range tests {
33+
t.Run(tt.description, func(t *testing.T) {
34+
if r := HasNamespace(tt.resource); r != tt.withNamespace {
35+
t.Errorf("HasNamespace() = %v, want %v", r, tt.withNamespace)
36+
}
37+
})
38+
}
39+
}

‎pkg/apis/configuration/validation/virtualserver.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88

99
"github.com/dlclark/regexp2"
1010
"github.com/nginx/kubernetes-ingress/internal/configs"
11+
"github.com/nginx/kubernetes-ingress/internal/nsutils"
1112
internalValidation "github.com/nginx/kubernetes-ingress/internal/validation"
1213
v1 "github.com/nginx/kubernetes-ingress/pkg/apis/configuration/v1"
1314
"k8s.io/apimachinery/pkg/util/sets"
@@ -737,7 +738,7 @@ func validateServiceName(name string, fieldPath *field.Path) field.ErrorList {
737738

738739
// validateVirtualServerServiceName checks if a namespaced service name is valid for VirtualServer upstreams.
739740
func validateVirtualServerServiceName(name string, fieldPath *field.Path) field.ErrorList {
740-
if strings.Contains(name, "/") {
741+
if nsutils.HasNamespace(name) {
741742
parts := strings.Split(name, "/")
742743
if len(parts) != 2 {
743744
return field.ErrorList{field.Invalid(fieldPath, name, " service reference must be in the format namespace/service-name")}

0 commit comments

Comments
 (0)