Skip to content

Commit 8dbe6e0

Browse files
sanposhihosanposhiho
authored
fix: validate SecurityPolicy on controller and egctl translate (envoyproxy#4987)
* fix: validate SecurityPolicy on controller and `egctl translate` Signed-off-by: Kensei Nakada <handbomusic@gmail.com> * fix: move the validation to gatewayapi layer Signed-off-by: Kensei Nakada <handbomusic@gmail.com>
1 parent 84f2ad2 commit 8dbe6e0

File tree

10 files changed

+295
-8
lines changed

10 files changed

+295
-8
lines changed

‎api/v1alpha1/validation/securitypolicy_validate.go‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func ValidateSecurityPolicy(policy *egv1a1.SecurityPolicy) error {
2525
return errors.New("policy is nil")
2626
}
2727
if err := validateSecurityPolicySpec(&policy.Spec); err != nil {
28-
errs = append(errs, errors.New("policy is nil"))
28+
errs = append(errs, err)
2929
}
3030

3131
return utilerrors.NewAggregate(errs)
@@ -43,6 +43,17 @@ func validateSecurityPolicySpec(spec *egv1a1.SecurityPolicySpec) error {
4343
sum++
4444
case spec.JWT != nil:
4545
sum++
46+
if err := ValidateJWTProvider(spec.JWT.Providers); err != nil {
47+
errs = append(errs, err)
48+
}
49+
case spec.Authorization != nil:
50+
sum++
51+
case spec.BasicAuth != nil:
52+
sum++
53+
case spec.ExtAuth != nil:
54+
sum++
55+
case spec.OIDC != nil:
56+
sum++
4657
}
4758
if sum == 0 {
4859
errs = append(errs, errors.New("no security policy is specified"))
@@ -53,10 +64,6 @@ func validateSecurityPolicySpec(spec *egv1a1.SecurityPolicySpec) error {
5364
return utilerrors.NewAggregate(errs)
5465
}
5566

56-
if err := ValidateJWTProvider(spec.JWT.Providers); err != nil {
57-
errs = append(errs, err)
58-
}
59-
6067
return utilerrors.NewAggregate(errs)
6168
}
6269

‎internal/cmd/egctl/testdata/translate/in/invalid-securitypolicy.yaml‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
apiVersion: gateway.networking.k8s.io/v1
2+
kind: GatewayClass
3+
metadata:
4+
name: eg
5+
spec:
6+
controllerName: gateway.envoyproxy.io/gatewayclass-controller
7+
---
8+
apiVersion: gateway.networking.k8s.io/v1
9+
kind: Gateway
10+
metadata:
11+
name: eg
12+
spec:
13+
gatewayClassName: eg
14+
listeners:
15+
- name: http
16+
protocol: HTTP
17+
port: 80
18+
---
19+
apiVersion: v1
20+
kind: ServiceAccount
21+
metadata:
22+
name: backend
23+
---
24+
apiVersion: v1
25+
kind: Service
26+
metadata:
27+
name: backend
28+
labels:
29+
app: backend
30+
service: backend
31+
spec:
32+
clusterIP: 7.7.7.7
33+
ports:
34+
- name: http
35+
port: 3000
36+
targetPort: 3000
37+
selector:
38+
app: backend
39+
---
40+
apiVersion: apps/v1
41+
kind: Deployment
42+
metadata:
43+
name: backend
44+
spec:
45+
replicas: 1
46+
selector:
47+
matchLabels:
48+
app: backend
49+
version: v1
50+
template:
51+
metadata:
52+
labels:
53+
app: backend
54+
version: v1
55+
spec:
56+
serviceAccountName: backend
57+
containers:
58+
- image: gcr.io/k8s-staging-gateway-api/echo-basic:v20231214-v1.0.0-140-gf544a46e
59+
imagePullPolicy: IfNotPresent
60+
name: backend
61+
ports:
62+
- containerPort: 3000
63+
env:
64+
- name: POD_NAME
65+
valueFrom:
66+
fieldRef:
67+
fieldPath: metadata.name
68+
- name: NAMESPACE
69+
valueFrom:
70+
fieldRef:
71+
fieldPath: metadata.namespace
72+
---
73+
apiVersion: gateway.envoyproxy.io/v1alpha1
74+
kind: SecurityPolicy
75+
metadata:
76+
name: jwt-example
77+
spec:
78+
targetRef:
79+
group: gateway.networking.k8s.io
80+
kind: HTTPRoute
81+
name: backend
82+
# No policy inside, which is invalid
83+
---
84+
apiVersion: gateway.networking.k8s.io/v1
85+
kind: HTTPRoute
86+
metadata:
87+
name: backend
88+
spec:
89+
parentRefs:
90+
- name: eg
91+
hostnames:
92+
- "www.example.com"
93+
rules:
94+
- backendRefs:
95+
- group: ""
96+
kind: Service
97+
name: backend
98+
port: 3000
99+
weight: 1
100+
matches:
101+
- path:
102+
type: PathPrefix
103+
value: /foo

‎internal/cmd/egctl/testdata/translate/out/invalid-securitypolicy.all.yaml‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
gatewayClass:
2+
kind: GatewayClass
3+
metadata:
4+
creationTimestamp: null
5+
name: eg
6+
namespace: envoy-gateway-system
7+
spec:
8+
controllerName: gateway.envoyproxy.io/gatewayclass-controller
9+
status:
10+
conditions:
11+
- lastTransitionTime: null
12+
message: Valid GatewayClass
13+
reason: Accepted
14+
status: "True"
15+
type: Accepted
16+
gateways:
17+
- kind: Gateway
18+
metadata:
19+
creationTimestamp: null
20+
name: eg
21+
namespace: envoy-gateway-system
22+
spec:
23+
gatewayClassName: eg
24+
listeners:
25+
- name: http
26+
port: 80
27+
protocol: HTTP
28+
status:
29+
listeners:
30+
- attachedRoutes: 1
31+
conditions:
32+
- lastTransitionTime: null
33+
message: Sending translated listener configuration to the data plane
34+
reason: Programmed
35+
status: "True"
36+
type: Programmed
37+
- lastTransitionTime: null
38+
message: Listener has been successfully translated
39+
reason: Accepted
40+
status: "True"
41+
type: Accepted
42+
- lastTransitionTime: null
43+
message: Listener references have been resolved
44+
reason: ResolvedRefs
45+
status: "True"
46+
type: ResolvedRefs
47+
name: http
48+
supportedKinds:
49+
- group: gateway.networking.k8s.io
50+
kind: HTTPRoute
51+
- group: gateway.networking.k8s.io
52+
kind: GRPCRoute
53+
httpRoutes:
54+
- kind: HTTPRoute
55+
metadata:
56+
creationTimestamp: null
57+
name: backend
58+
namespace: envoy-gateway-system
59+
spec:
60+
hostnames:
61+
- www.example.com
62+
parentRefs:
63+
- name: eg
64+
rules:
65+
- backendRefs:
66+
- group: ""
67+
kind: Service
68+
name: backend
69+
port: 3000
70+
weight: 1
71+
matches:
72+
- path:
73+
type: PathPrefix
74+
value: /foo
75+
status:
76+
parents:
77+
- conditions:
78+
- lastTransitionTime: null
79+
message: Route is accepted
80+
reason: Accepted
81+
status: "True"
82+
type: Accepted
83+
- lastTransitionTime: null
84+
message: Resolved all the Object references for the Route
85+
reason: ResolvedRefs
86+
status: "True"
87+
type: ResolvedRefs
88+
controllerName: gateway.envoyproxy.io/gatewayclass-controller
89+
parentRef:
90+
name: eg
91+
securityPolicies:
92+
- kind: SecurityPolicy
93+
metadata:
94+
creationTimestamp: null
95+
name: jwt-example
96+
namespace: envoy-gateway-system
97+
spec:
98+
targetRef:
99+
group: gateway.networking.k8s.io
100+
kind: HTTPRoute
101+
name: backend
102+
status:
103+
ancestors:
104+
- ancestorRef:
105+
group: gateway.networking.k8s.io
106+
kind: Gateway
107+
name: eg
108+
namespace: envoy-gateway-system
109+
conditions:
110+
- lastTransitionTime: null
111+
message: 'Invalid SecurityPolicy: no security policy is specified.'
112+
reason: Invalid
113+
status: "False"
114+
type: Accepted
115+
controllerName: gateway.envoyproxy.io/gatewayclass-controller

‎internal/cmd/egctl/translate.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,7 @@ func translateGatewayAPIToGatewayAPI(resources *resource.Resources) (resource.Re
322322
}
323323
gRes.EnvoyProxyForGatewayClass = resources.EnvoyProxyForGatewayClass
324324
}
325+
325326
if !epInvalid {
326327
status.SetGatewayClassAccepted(resources.GatewayClass, true, string(gwapiv1.GatewayClassReasonAccepted), status.MsgValidGatewayClass)
327328
}

‎internal/cmd/egctl/translate_test.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,13 @@ func TestTranslate(t *testing.T) {
269269
output: yamlOutput,
270270
expect: true,
271271
},
272+
{
273+
name: "invalid-securitypolicy",
274+
from: "gateway-api",
275+
to: "gateway-api",
276+
output: yamlOutput,
277+
expect: true,
278+
},
272279
{
273280
name: "no-gateway-class-resources",
274281
from: "gateway-api",

‎internal/gatewayapi/securitypolicy.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
gwapiv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
3131

3232
egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
33+
"github.com/envoyproxy/gateway/api/v1alpha1/validation"
3334
"github.com/envoyproxy/gateway/internal/gatewayapi/resource"
3435
"github.com/envoyproxy/gateway/internal/gatewayapi/status"
3536
"github.com/envoyproxy/gateway/internal/ir"
@@ -150,6 +151,17 @@ func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.Security
150151
continue
151152
}
152153

154+
if err := validation.ValidateSecurityPolicy(policy); err != nil {
155+
status.SetTranslationErrorForPolicyAncestors(&policy.Status,
156+
parentGateways,
157+
t.GatewayControllerName,
158+
policy.Generation,
159+
status.Error2ConditionMsg(fmt.Errorf("invalid SecurityPolicy: %w", err)),
160+
)
161+
162+
continue
163+
}
164+
153165
if err := t.translateSecurityPolicyForRoute(policy, targetedRoute, resources, xdsIR); err != nil {
154166
status.SetTranslationErrorForPolicyAncestors(&policy.Status,
155167
parentGateways,

‎internal/gatewayapi/testdata/securitypolicy-override-replace.in.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,3 +142,7 @@ securityPolicies:
142142
group: gateway.networking.k8s.io
143143
kind: HTTPRoute
144144
name: httproute-3
145+
cors:
146+
allowOrigins:
147+
- "http://*.example.com"
148+
maxAge: 1000s

‎internal/gatewayapi/testdata/securitypolicy-override-replace.out.yaml‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,10 @@ securityPolicies:
218218
name: policy-for-route-3
219219
namespace: default
220220
spec:
221+
cors:
222+
allowOrigins:
223+
- http://*.example.com
224+
maxAge: 16m40s
221225
targetRef:
222226
group: gateway.networking.k8s.io
223227
kind: HTTPRoute
@@ -427,4 +431,10 @@ xdsIR:
427431
distinct: false
428432
name: ""
429433
prefix: /baz
430-
security: {}
434+
security:
435+
cors:
436+
allowOrigins:
437+
- distinct: false
438+
name: ""
439+
safeRegex: http://.*\.example\.com
440+
maxAge: 16m40s

‎internal/gatewayapi/testdata/securitypolicy-status-conditions.in.yaml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@ securityPolicies:
2929
group: gateway.networking.k8s.io
3030
kind: HTTPRoute
3131
name: httproute-1
32+
cors:
33+
allowOrigins:
34+
- "http://*.example.com"
35+
maxAge: 1000s
3236
- apiVersion: gateway.envoyproxy.io/v1alpha1
3337
kind: SecurityPolicy
3438
metadata:
@@ -45,6 +49,10 @@ securityPolicies:
4549
namespace: envoy-gateway
4650
name: target-grpcroute-in-gateway-2
4751
spec:
52+
cors:
53+
allowOrigins:
54+
- "http://*.example.com"
55+
maxAge: 1000s
4856
targetRef:
4957
group: gateway.networking.k8s.io
5058
kind: GRPCRoute

0 commit comments

Comments
 (0)