Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add more crd (mostly CEL) validation #2087

Merged
merged 4 commits into from
Oct 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Alice-Lilith marked this conversation as resolved.
Show resolved Hide resolved
// +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",
Alice-Lilith marked this conversation as resolved.
Show resolved Hide resolved
"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