Skip to content

Commit

Permalink
add more crd (mostly CEL) validation (#2087)
Browse files Browse the repository at this point in the history
* add more crd validation

Signed-off-by: Alice Wasko <[email protected]>

* remove sectionName CEL validation until it is supported

Signed-off-by: Alice Wasko <[email protected]>

* add cel to forbid sectionName until it is supported

Signed-off-by: Alice Wasko <[email protected]>

---------

Signed-off-by: Alice Wasko <[email protected]>
Signed-off-by: Arko Dasgupta <[email protected]>
Co-authored-by: Arko Dasgupta <[email protected]>
  • Loading branch information
Alice Wasko and arkodg authored Oct 28, 2023
1 parent ef7a2a4 commit 1ebace9
Show file tree
Hide file tree
Showing 9 changed files with 558 additions and 6 deletions.
6 changes: 3 additions & 3 deletions api/v1alpha1/backendtrafficpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ const (
// +kubebuilder:object:root=true
// +kubebuilder:resource:shortName=btp
// +kubebuilder:subresource:status
// +kubebuilder:subresource:overrideStrategy
// +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.conditions[?(@.type=="Accepted")].reason`
// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
//
Expand All @@ -37,8 +36,9 @@ type BackendTrafficPolicy struct {

// spec defines the desired state of BackendTrafficPolicy.
type BackendTrafficPolicySpec struct {

// +kubebuilder:validation:XValidation:rule="self.kind == 'Gateway' || self.kind == 'HTTPRoute' || self.kind == 'GRPCRoute' || self.kind == 'UDPRoute' || self.kind == 'TCPRoute' || self.kind == 'TLSRoute'", message="this policy can only have a targetRef.kind of Gateway/HTTPRoute/GRPCRoute/TCPRoute/UDPRoute/TLSRoute"
// +kubebuilder:validation:XValidation:rule="self.group == 'gateway.networking.k8s.io'", message="this policy can only have a targetRef.group of gateway.networking.k8s.io"
// +kubebuilder:validation:XValidation:rule="self.kind in ['Gateway', 'HTTPRoute', 'GRPCRoute', 'UDPRoute', 'TCPRoute', 'TLSRoute']", message="this policy can only have a targetRef.kind of Gateway/HTTPRoute/GRPCRoute/TCPRoute/UDPRoute/TLSRoute"
// +kubebuilder:validation:XValidation:rule="!has(self.sectionName)",message="this policy does not yet support the sectionName field"
//
// targetRef is the name of the resource this policy
// is being attached to.
Expand Down
4 changes: 4 additions & 0 deletions api/v1alpha1/clienttrafficpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ type ClientTrafficPolicy struct {

// ClientTrafficPolicySpec defines the desired state of ClientTrafficPolicy.
type ClientTrafficPolicySpec struct {
// +kubebuilder:validation:XValidation:rule="self.group == 'gateway.networking.k8s.io'", message="this policy can only have a targetRef.group of gateway.networking.k8s.io"
// +kubebuilder:validation:XValidation:rule="self.kind == 'Gateway'", message="this policy can only have a targetRef.kind of Gateway"
// +kubebuilder:validation:XValidation:rule="!has(self.sectionName)",message="this policy does not yet support the sectionName field"
//
// TargetRef is the name of the Gateway resource this policy
// is being attached to.
// This Policy and the TargetRef MUST be in the same namespace
Expand Down
4 changes: 4 additions & 0 deletions api/v1alpha1/securitypolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ type SecurityPolicy struct {

// SecurityPolicySpec defines the desired state of SecurityPolicy.
type SecurityPolicySpec struct {
// +kubebuilder:validation:XValidation:rule="self.group == 'gateway.networking.k8s.io'", message="this policy can only have a targetRef.group of gateway.networking.k8s.io"
// +kubebuilder:validation:XValidation:rule="self.kind == 'Gateway'", message="this policy can only have a targetRef.kind of Gateway"
// +kubebuilder:validation:XValidation:rule="!has(self.sectionName)",message="this policy does not yet support the sectionName field"
//
// TargetRef is the name of the Gateway resource this policy
// is being attached to.
// This Policy and the TargetRef MUST be in the same namespace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,10 +264,13 @@ spec:
- name
type: object
x-kubernetes-validations:
- message: this policy can only have a targetRef.group of gateway.networking.k8s.io
rule: self.group == 'gateway.networking.k8s.io'
- message: this policy can only have a targetRef.kind of Gateway/HTTPRoute/GRPCRoute/TCPRoute/UDPRoute/TLSRoute
rule: self.kind == 'Gateway' || self.kind == 'HTTPRoute' || self.kind
== 'GRPCRoute' || self.kind == 'UDPRoute' || self.kind == 'TCPRoute'
|| self.kind == 'TLSRoute'
rule: self.kind in ['Gateway', 'HTTPRoute', 'GRPCRoute', 'UDPRoute',
'TCPRoute', 'TLSRoute']
- message: this policy does not yet support the sectionName field
rule: '!has(self.sectionName)'
required:
- targetRef
type: object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,13 @@ spec:
- kind
- name
type: object
x-kubernetes-validations:
- message: this policy can only have a targetRef.group of gateway.networking.k8s.io
rule: self.group == 'gateway.networking.k8s.io'
- message: this policy can only have a targetRef.kind of Gateway
rule: self.kind == 'Gateway'
- message: this policy does not yet support the sectionName field
rule: '!has(self.sectionName)'
tcpKeepalive:
description: TcpKeepalive settings associated with the downstream
client connection. If defined, sets SO_KEEPALIVE on the listener
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,13 @@ spec:
- kind
- name
type: object
x-kubernetes-validations:
- message: this policy can only have a targetRef.group of gateway.networking.k8s.io
rule: self.group == 'gateway.networking.k8s.io'
- message: this policy can only have a targetRef.kind of Gateway
rule: self.kind == 'Gateway'
- message: this policy does not yet support the sectionName field
rule: '!has(self.sectionName)'
required:
- targetRef
type: object
Expand Down
186 changes: 186 additions & 0 deletions test/cel-validation/backendtrafficpolicy_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
// Copyright Envoy Gateway Authors
// SPDX-License-Identifier: Apache-2.0
// The full text of the Apache license is available in the LICENSE file at
// the root of the repo.

//go:build celvalidation
// +build celvalidation

package celvalidation

import (
"context"
"fmt"
"strings"
"testing"
"time"

egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
gwapiv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
)

func TestBackendTrafficPolicyTarget(t *testing.T) {
ctx := context.Background()
baseBTP := egv1a1.BackendTrafficPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "btp",
Namespace: metav1.NamespaceDefault,
},
Spec: egv1a1.BackendTrafficPolicySpec{},
}

sectionName := gwapiv1a2.SectionName("foo")

cases := []struct {
desc string
mutate func(btp *egv1a1.BackendTrafficPolicy)
mutateStatus func(btp *egv1a1.BackendTrafficPolicy)
wantErrors []string
}{
{
desc: "valid gateway targetRef",
mutate: func(btp *egv1a1.BackendTrafficPolicy) {
btp.Spec = egv1a1.BackendTrafficPolicySpec{
TargetRef: gwapiv1a2.PolicyTargetReferenceWithSectionName{
PolicyTargetReference: gwapiv1a2.PolicyTargetReference{
Group: gwapiv1a2.Group("gateway.networking.k8s.io"),
Kind: gwapiv1a2.Kind("Gateway"),
Name: gwapiv1a2.ObjectName("eg"),
},
},
}
},
wantErrors: []string{},
},
{
desc: "valid httproute targetRef",
mutate: func(btp *egv1a1.BackendTrafficPolicy) {
btp.Spec = egv1a1.BackendTrafficPolicySpec{
TargetRef: gwapiv1a2.PolicyTargetReferenceWithSectionName{
PolicyTargetReference: gwapiv1a2.PolicyTargetReference{
Group: gwapiv1a2.Group("gateway.networking.k8s.io"),
Kind: gwapiv1a2.Kind("HTTPRoute"),
Name: gwapiv1a2.ObjectName("httpbin-route"),
},
},
}
},
wantErrors: []string{},
},
{
desc: "no targetRef",
mutate: func(btp *egv1a1.BackendTrafficPolicy) {
btp.Spec = egv1a1.BackendTrafficPolicySpec{}
},
wantErrors: []string{
"spec.targetRef.kind: Invalid value: \"\": spec.targetRef.kind in body should be at least 1 chars long",
"spec.targetRef.name: Invalid value: \"\": spec.targetRef.name in body should be at least 1 chars long",
"spec.targetRef: Invalid value: \"object\": this policy can only have a targetRef.group of gateway.networking.k8s.io",
"spec.targetRef: Invalid value: \"object\": this policy can only have a targetRef.kind of Gateway/HTTPRoute/GRPCRoute/TCPRoute/UDPRoute/TLSRoute",
},
},
{
desc: "targetRef unsupported kind",
mutate: func(btp *egv1a1.BackendTrafficPolicy) {
btp.Spec = egv1a1.BackendTrafficPolicySpec{
TargetRef: gwapiv1a2.PolicyTargetReferenceWithSectionName{
PolicyTargetReference: gwapiv1a2.PolicyTargetReference{
Group: gwapiv1a2.Group("gateway.networking.k8s.io"),
Kind: gwapiv1a2.Kind("foo"),
Name: gwapiv1a2.ObjectName("eg"),
},
},
}
},
wantErrors: []string{
"spec.targetRef: Invalid value: \"object\": this policy can only have a targetRef.kind of Gateway/HTTPRoute/GRPCRoute/TCPRoute/UDPRoute/TLSRoute",
},
},
{
desc: "targetRef unsupported group",
mutate: func(btp *egv1a1.BackendTrafficPolicy) {
btp.Spec = egv1a1.BackendTrafficPolicySpec{
TargetRef: gwapiv1a2.PolicyTargetReferenceWithSectionName{
PolicyTargetReference: gwapiv1a2.PolicyTargetReference{
Group: gwapiv1a2.Group("foo"),
Kind: gwapiv1a2.Kind("Gateway"),
Name: gwapiv1a2.ObjectName("eg"),
},
},
}
},
wantErrors: []string{
"spec.targetRef: Invalid value: \"object\": this policy can only have a targetRef.group of gateway.networking.k8s.io",
},
},
{
desc: "targetRef unsupported group and kind",
mutate: func(btp *egv1a1.BackendTrafficPolicy) {
btp.Spec = egv1a1.BackendTrafficPolicySpec{
TargetRef: gwapiv1a2.PolicyTargetReferenceWithSectionName{
PolicyTargetReference: gwapiv1a2.PolicyTargetReference{
Group: gwapiv1a2.Group("foo"),
Kind: gwapiv1a2.Kind("bar"),
Name: gwapiv1a2.ObjectName("eg"),
},
},
}
},
wantErrors: []string{
"spec.targetRef: Invalid value: \"object\": this policy can only have a targetRef.group of gateway.networking.k8s.io",
"spec.targetRef: Invalid value: \"object\": this policy can only have a targetRef.kind of Gateway/HTTPRoute/GRPCRoute/TCPRoute/UDPRoute/TLSRoute",
},
},
{
desc: "sectionName disabled until supported",
mutate: func(btp *egv1a1.BackendTrafficPolicy) {
btp.Spec = egv1a1.BackendTrafficPolicySpec{
TargetRef: gwapiv1a2.PolicyTargetReferenceWithSectionName{
PolicyTargetReference: gwapiv1a2.PolicyTargetReference{
Group: gwapiv1a2.Group("gateway.networking.k8s.io"),
Kind: gwapiv1a2.Kind("Gateway"),
Name: gwapiv1a2.ObjectName("eg"),
},
SectionName: &sectionName,
},
}
},
wantErrors: []string{
"spec.targetRef: Invalid value: \"object\": this policy does not yet support the sectionName field",
},
},
}

for _, tc := range cases {
t.Run(tc.desc, func(t *testing.T) {
btp := baseBTP.DeepCopy()
btp.Name = fmt.Sprintf("btp-%v", time.Now().UnixNano())

if tc.mutate != nil {
tc.mutate(btp)
}
err := c.Create(ctx, btp)

if tc.mutateStatus != nil {
tc.mutateStatus(btp)
err = c.Status().Update(ctx, btp)
}

if (len(tc.wantErrors) != 0) != (err != nil) {
t.Fatalf("Unexpected response while creating BackendTrafficPolicy; got err=\n%v\n;want error=%v", err, tc.wantErrors)
}

var missingErrorStrings []string
for _, wantError := range tc.wantErrors {
if !strings.Contains(strings.ToLower(err.Error()), strings.ToLower(wantError)) {
missingErrorStrings = append(missingErrorStrings, wantError)
}
}
if len(missingErrorStrings) != 0 {
t.Errorf("Unexpected response while creating BackendTrafficPolicy; got err=\n%v\n;missing strings within error=%q", err, missingErrorStrings)
}
})
}
}
Loading

0 comments on commit 1ebace9

Please sign in to comment.