Skip to content

Commit

Permalink
add more crd validation
Browse files Browse the repository at this point in the history
Signed-off-by: Alice Wasko <[email protected]>
  • Loading branch information
Alice Wasko committed Oct 27, 2023
1 parent 131178b commit d096ee6
Show file tree
Hide file tree
Showing 9 changed files with 464 additions and 2 deletions.
7 changes: 5 additions & 2 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=btpolicy
// +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 @@ -28,6 +27,8 @@ type BackendTrafficPolicy struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

// +kubebuilder:validation:Required
//
// spec defines the desired state of BackendTrafficPolicy.
Spec BackendTrafficPolicySpec `json:"spec"`

Expand All @@ -37,8 +38,10 @@ type BackendTrafficPolicy struct {

// spec defines the desired state of BackendTrafficPolicy.
type BackendTrafficPolicySpec 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' || 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="has(self.sectionName) ? self.kind == 'Gateway': true", message="sectionName can only be set when kind is 'Gateway'."
// +kubebuilder:validation:Required
//
// targetRef is the name of the resource this policy
// is being attached to.
Expand Down
5 changes: 5 additions & 0 deletions api/v1alpha1/clienttrafficpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ type ClientTrafficPolicy struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

// +kubebuilder:validation:Required
//
// Spec defines the desired state of ClientTrafficPolicy.
Spec ClientTrafficPolicySpec `json:"spec"`

Expand All @@ -36,6 +38,9 @@ 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:Required
// 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
6 changes: 6 additions & 0 deletions api/v1alpha1/securitypolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ type SecurityPolicy struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

// +kubebuilder:validation:Required
//
// Spec defines the desired state of SecurityPolicy.
Spec SecurityPolicySpec `json:"spec"`

Expand All @@ -35,6 +37,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:Required
//
// 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,14 @@ 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'
- message: sectionName can only be set when kind is 'Gateway'.
rule: 'has(self.sectionName) ? self.kind == ''Gateway'': true'
required:
- targetRef
type: object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ 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'
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 @@ -233,6 +233,11 @@ 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'
required:
- targetRef
type: object
Expand Down
172 changes: 172 additions & 0 deletions test/cel-validation/backendtrafficpolicy_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
// 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"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
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{},
}

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{
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{
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{},
},
{
desc: "targetRef unsupported kind",
mutate: func(btp *egv1a1.BackendTrafficPolicy) {
btp.Spec = egv1a1.BackendTrafficPolicySpec{
TargetRef: gwapiv1a2.PolicyTargetReferenceWithSectionName{
Group: gwapiv1a2.Group("gateway.networking.k8s.io"),
Kind: gwapiv1a2.Kind("foo"),
Name: gwapiv1a2.ObjectName("eg"),
},
}
},
wantErrors: []string{},
},
{
desc: "targetRef unsupported group",
mutate: func(btp *egv1a1.BackendTrafficPolicy) {
btp.Spec = egv1a1.BackendTrafficPolicySpec{
TargetRef: gwapiv1a2.PolicyTargetReferenceWithSectionName{
Group: gwapiv1a2.Group("foo"),
Kind: gwapiv1a2.Kind("Gateway"),
Name: gwapiv1a2.ObjectName("eg"),
},
}
},
wantErrors: []string{},
},
{
desc: "targetRef unsupported group and kind",
mutate: func(btp *egv1a1.BackendTrafficPolicy) {
btp.Spec = egv1a1.BackendTrafficPolicySpec{
TargetRef: gwapiv1a2.PolicyTargetReferenceWithSectionName{
Group: gwapiv1a2.Group("foo"),
Kind: gwapiv1a2.Kind("bar"),
Name: gwapiv1a2.ObjectName("eg"),
},
}
},
wantErrors: []string{},
},
{
desc: "targetRef section name with gateway",
mutate: func(btp *egv1a1.BackendTrafficPolicy) {
btp.Spec = egv1a1.BackendTrafficPolicySpec{
TargetRef: gwapiv1a2.PolicyTargetReferenceWithSectionName{
Group: gwapiv1a2.Group("gateway.networking.k8s.io"),
Kind: gwapiv1a2.Kind("Gateway"),
Name: gwapiv1a2.ObjectName("eg"),
SectionName: &gwapiv1a2.SectionName("foo")

Check failure on line 120 in test/cel-validation/backendtrafficpolicy_test.go

View workflow job for this annotation

GitHub Actions / coverage-test

missing ',' before newline in composite literal
},
}
},
wantErrors: []string{},
},
{
desc: "targetRef section name with httproute",
mutate: func(btp *egv1a1.BackendTrafficPolicy) {
btp.Spec = egv1a1.BackendTrafficPolicySpec{
TargetRef: gwapiv1a2.PolicyTargetReferenceWithSectionName{
Group: gwapiv1a2.Group("gateway.networking.k8s.io"),
Kind: gwapiv1a2.Kind("HTTPRoute"),
Name: gwapiv1a2.ObjectName("eg"),
SectionName: &gwapiv1a2.SectionName("foo")
},
}
},
wantErrors: []string{},
},
}

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 != nil)
}

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 d096ee6

Please sign in to comment.