From 09a9dde711b5c35e2f22c04d6f43a200005eb0e1 Mon Sep 17 00:00:00 2001 From: Raymond Kroeker Date: Wed, 27 May 2026 13:08:20 -0700 Subject: [PATCH 1/3] fix(profile): Add nil check to profile validation * Add nil check to spec routes in the service profile. * Add test with nil route yaml. --- pkg/profiles/profiles.go | 3 +++ pkg/profiles/profiles_test.go | 11 +++++++++++ 2 files changed, 14 insertions(+) diff --git a/pkg/profiles/profiles.go b/pkg/profiles/profiles.go index b2038ef6ac553..8c4b0ca2b564b 100644 --- a/pkg/profiles/profiles.go +++ b/pkg/profiles/profiles.go @@ -66,6 +66,9 @@ func Validate(data []byte) error { } for _, route := range serviceProfile.Spec.Routes { + if route == nil { + return fmt.Errorf("ServiceProfile %q has a nil route", serviceProfile.Name) + } if route.Name == "" { return fmt.Errorf("ServiceProfile %q has a route with no name", serviceProfile.Name) } diff --git a/pkg/profiles/profiles_test.go b/pkg/profiles/profiles_test.go index d33b39467ae1e..ca2a66679ff02 100644 --- a/pkg/profiles/profiles_test.go +++ b/pkg/profiles/profiles_test.go @@ -396,6 +396,17 @@ spec: method: GET pathRegex: /route-1`, }, + { + err: errors.New("ServiceProfile \"name.ns.svc.cluster.local\" has a nil route"), + sp: `apiVersion: linkerd.io/v1alpha2 +kind: ServiceProfile +metadata: + name: name.ns.svc.cluster.local + namespace: linkerd-ns +spec: + routes: + -`, + }, } for id, exp := range expectations { From 9cb297133a5a875da38e56244f0f77252932aced Mon Sep 17 00:00:00 2001 From: Raymond Kroeker Date: Fri, 29 May 2026 11:02:10 -0700 Subject: [PATCH 2/3] Additional Nil Checks * Check for nil: response class, request match for any/all, response match for any/all * Add tests to cover each. * Update existing wording to state 'null' vs 'nil' --- pkg/profiles/profiles.go | 19 +++++++- pkg/profiles/profiles_test.go | 92 ++++++++++++++++++++++++++++++++++- 2 files changed, 109 insertions(+), 2 deletions(-) diff --git a/pkg/profiles/profiles.go b/pkg/profiles/profiles.go index 8c4b0ca2b564b..348fcefd40033 100644 --- a/pkg/profiles/profiles.go +++ b/pkg/profiles/profiles.go @@ -38,6 +38,8 @@ var ( minStatus uint32 = 100 maxStatus uint32 = 599 + errNilMatchAll = fmt.Errorf("null condition \"all\"") + errNilMatchAny = fmt.Errorf("null condition \"any\"") errRequestMatchField = errors.New("A request match must have a field set") errResponseMatchField = errors.New("A response match must have a field set") ) @@ -67,7 +69,7 @@ func Validate(data []byte) error { for _, route := range serviceProfile.Spec.Routes { if route == nil { - return fmt.Errorf("ServiceProfile %q has a nil route", serviceProfile.Name) + return fmt.Errorf("ServiceProfile %q has a null route", serviceProfile.Name) } if route.Name == "" { return fmt.Errorf("ServiceProfile %q has a route with no name", serviceProfile.Name) @@ -86,6 +88,9 @@ func Validate(data []byte) error { return fmt.Errorf("ServiceProfile %q has a route with an invalid condition: %w", serviceProfile.Name, err) } for _, rc := range route.ResponseClasses { + if rc == nil { + return fmt.Errorf("ServiceProfile %q has a null response class", serviceProfile.Name) + } if rc.Condition == nil { return fmt.Errorf("ServiceProfile %q has a response class with no condition", serviceProfile.Name) } @@ -122,6 +127,9 @@ func ValidateRequestMatch(reqMatch *sp.RequestMatch) error { if reqMatch.All != nil { matchKindSet = true for _, child := range reqMatch.All { + if child == nil { + return errNilMatchAll + } err := ValidateRequestMatch(child) if err != nil { return err @@ -131,6 +139,9 @@ func ValidateRequestMatch(reqMatch *sp.RequestMatch) error { if reqMatch.Any != nil { matchKindSet = true for _, child := range reqMatch.Any { + if child == nil { + return errNilMatchAny + } err := ValidateRequestMatch(child) if err != nil { return err @@ -165,6 +176,9 @@ func ValidateResponseMatch(rspMatch *sp.ResponseMatch) error { if rspMatch.All != nil { matchKindSet = true for _, child := range rspMatch.All { + if child == nil { + return errNilMatchAll + } err := ValidateResponseMatch(child) if err != nil { return err @@ -174,6 +188,9 @@ func ValidateResponseMatch(rspMatch *sp.ResponseMatch) error { if rspMatch.Any != nil { matchKindSet = true for _, child := range rspMatch.Any { + if child == nil { + return errNilMatchAny + } err := ValidateResponseMatch(child) if err != nil { return err diff --git a/pkg/profiles/profiles_test.go b/pkg/profiles/profiles_test.go index ca2a66679ff02..c73a15883e62d 100644 --- a/pkg/profiles/profiles_test.go +++ b/pkg/profiles/profiles_test.go @@ -397,7 +397,7 @@ spec: pathRegex: /route-1`, }, { - err: errors.New("ServiceProfile \"name.ns.svc.cluster.local\" has a nil route"), + err: errors.New(`ServiceProfile "name.ns.svc.cluster.local" has a null route`), sp: `apiVersion: linkerd.io/v1alpha2 kind: ServiceProfile metadata: @@ -407,6 +407,96 @@ spec: routes: -`, }, + { + err: errors.New(`ServiceProfile "name.ns.svc.cluster.local" has a route with an invalid condition: null condition "all"`), + sp: `apiVersion: linkerd.io/v1alpha2 +kind: ServiceProfile +metadata: + name: name.ns.svc.cluster.local + namespace: linkerd-ns +spec: + routes: + - name: name-1 + condition: + method: GET + pathRegex: /route-1 + all: + -`, + }, + { + err: errors.New(`ServiceProfile "name.ns.svc.cluster.local" has a route with an invalid condition: null condition "any"`), + sp: `apiVersion: linkerd.io/v1alpha2 +kind: ServiceProfile +metadata: + name: name.ns.svc.cluster.local + namespace: linkerd-ns +spec: + routes: + - name: name-1 + condition: + method: GET + pathRegex: /route-1 + any: + -`, + }, + { + err: errors.New(`ServiceProfile "name.ns.svc.cluster.local" has a null response class`), + sp: `apiVersion: linkerd.io/v1alpha2 +kind: ServiceProfile +metadata: + name: name.ns.svc.cluster.local + namespace: linkerd-ns +spec: + routes: + - name: name-1 + condition: + method: GET + pathRegex: /route-1 + responseClasses: + -`, + }, + { + err: errors.New(`ServiceProfile "name.ns.svc.cluster.local" has a response class with an invalid condition: null condition "all"`), + sp: `apiVersion: linkerd.io/v1alpha2 +kind: ServiceProfile +metadata: + name: name.ns.svc.cluster.local + namespace: linkerd-ns +spec: + routes: + - name: name-1 + condition: + method: GET + pathRegex: /route-1 + responseClasses: + - condition: + status: + min: 500 + max: 599 + all: + -`, + }, + { + err: errors.New(`ServiceProfile "name.ns.svc.cluster.local" has a response class with an invalid condition: null condition "any"`), + sp: `apiVersion: linkerd.io/v1alpha2 +kind: ServiceProfile +metadata: + name: name.ns.svc.cluster.local + namespace: linkerd-ns +spec: + routes: + - name: name-1 + condition: + method: GET + pathRegex: /route-1 + responseClasses: + - condition: + status: + min: 500 + max: 599 + any: + -`, + }, } for id, exp := range expectations { From 009d6598b5b1e460b8e6d72a18761a8c8f85edbf Mon Sep 17 00:00:00 2001 From: Raymond Kroeker Date: Fri, 29 May 2026 11:15:36 -0700 Subject: [PATCH 3/3] Add test cases for parsing timeout. --- pkg/profiles/profiles_test.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/pkg/profiles/profiles_test.go b/pkg/profiles/profiles_test.go index c73a15883e62d..1eb00565e2c58 100644 --- a/pkg/profiles/profiles_test.go +++ b/pkg/profiles/profiles_test.go @@ -497,6 +497,36 @@ spec: any: -`, }, + { + err: nil, + sp: `apiVersion: linkerd.io/v1alpha2 +kind: ServiceProfile +metadata: + name: name.ns.svc.cluster.local + namespace: linkerd-ns +spec: + routes: + - name: name-1 + condition: + method: GET + pathRegex: /route-1, + timeout: 1ns`, + }, + { + err: errors.New(`ServiceProfile "name.ns.svc.cluster.local" has a route with an invalid timeout: time: invalid duration "one-second"`), + sp: `apiVersion: linkerd.io/v1alpha2 +kind: ServiceProfile +metadata: + name: name.ns.svc.cluster.local + namespace: linkerd-ns +spec: + routes: + - name: name-1 + condition: + method: GET + pathRegex: /route-1, + timeout: one-second`, + }, } for id, exp := range expectations {