Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
howardjohn committed Aug 16, 2024
1 parent d325715 commit 0643c0d
Show file tree
Hide file tree
Showing 15 changed files with 74 additions and 112 deletions.
3 changes: 2 additions & 1 deletion apis/v1/grpcroute_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ type GRPCRouteSpec struct {
// +optional
// +kubebuilder:validation:MaxItems=16
// +kubebuilder:validation:XValidation:message="While 16 rules and 64 matches per rule are allowed, the total number of matches across all rules in a route must be less than 128",rule="(self.size() > 0 ? (has(self[0].matches) ? self[0].matches.size() : 0) : 0) + (self.size() > 1 ? (has(self[1].matches) ? self[1].matches.size() : 0) : 0) + (self.size() > 2 ? (has(self[2].matches) ? self[2].matches.size() : 0) : 0) + (self.size() > 3 ? (has(self[3].matches) ? self[3].matches.size() : 0) : 0) + (self.size() > 4 ? (has(self[4].matches) ? self[4].matches.size() : 0) : 0) + (self.size() > 5 ? (has(self[5].matches) ? self[5].matches.size() : 0) : 0) + (self.size() > 6 ? (has(self[6].matches) ? self[6].matches.size() : 0) : 0) + (self.size() > 7 ? (has(self[7].matches) ? self[7].matches.size() : 0) : 0) + (self.size() > 8 ? (has(self[8].matches) ? self[8].matches.size() : 0) : 0) + (self.size() > 9 ? (has(self[9].matches) ? self[9].matches.size() : 0) : 0) + (self.size() > 10 ? (has(self[10].matches) ? self[10].matches.size() : 0) : 0) + (self.size() > 11 ? (has(self[11].matches) ? self[11].matches.size() : 0) : 0) + (self.size() > 12 ? (has(self[12].matches) ? self[12].matches.size() : 0) : 0) + (self.size() > 13 ? (has(self[13].matches) ? self[13].matches.size() : 0) : 0) + (self.size() > 14 ? (has(self[14].matches) ? self[14].matches.size() : 0) : 0) + (self.size() > 15 ? (has(self[15].matches) ? self[15].matches.size() : 0) : 0) <= 128"
// +kubebuilder:validation:XValidation:message="Rule name must be unique within the route",rule="self.all(l1, !has(l1.name) || self.exists_one(l2, has(l2.name) && l1.name == l2.name))"
// +kubebuilder:experimental:validation:XValidation:message="Rule name must be unique within the route",rule="self.all(l1, !has(l1.name) || self.exists_one(l2, has(l2.name) && l1.name == l2.name))"
Rules []GRPCRouteRule `json:"rules,omitempty"`
}

Expand All @@ -156,6 +156,7 @@ type GRPCRouteRule struct {
//
// Support: Extended
// +optional
// <gateway:experimental>
Name *SectionName `json:"name,omitempty"`

// Matches define conditions used for matching the rule against incoming
Expand Down
3 changes: 2 additions & 1 deletion apis/v1/httproute_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ type HTTPRouteSpec struct {
// Rules are a list of HTTP matchers, filters and actions.
//
// +optional
// +kubebuilder:validation:XValidation:message="Rule name must be unique within the route",rule="self.all(l1, !has(l1.name) || self.exists_one(l2, has(l2.name) && l1.name == l2.name))"
// +kubebuilder:experimental:validation:XValidation:message="Rule name must be unique within the route",rule="self.all(l1, !has(l1.name) || self.exists_one(l2, has(l2.name) && l1.name == l2.name))"
// +kubebuilder:validation:MaxItems=16
// +kubebuilder:default={{matches: {{path: {type: "PathPrefix", value: "/"}}}}}
// +kubebuilder:validation:XValidation:message="While 16 rules and 64 matches per rule are allowed, the total number of matches across all rules in a route must be less than 128",rule="(self.size() > 0 ? self[0].matches.size() : 0) + (self.size() > 1 ? self[1].matches.size() : 0) + (self.size() > 2 ? self[2].matches.size() : 0) + (self.size() > 3 ? self[3].matches.size() : 0) + (self.size() > 4 ? self[4].matches.size() : 0) + (self.size() > 5 ? self[5].matches.size() : 0) + (self.size() > 6 ? self[6].matches.size() : 0) + (self.size() > 7 ? self[7].matches.size() : 0) + (self.size() > 8 ? self[8].matches.size() : 0) + (self.size() > 9 ? self[9].matches.size() : 0) + (self.size() > 10 ? self[10].matches.size() : 0) + (self.size() > 11 ? self[11].matches.size() : 0) + (self.size() > 12 ? self[12].matches.size() : 0) + (self.size() > 13 ? self[13].matches.size() : 0) + (self.size() > 14 ? self[14].matches.size() : 0) + (self.size() > 15 ? self[15].matches.size() : 0) <= 128"
Expand All @@ -138,6 +138,7 @@ type HTTPRouteRule struct {
//
// Support: Extended
// +optional
// <gateway:experimental>
Name *SectionName `json:"name,omitempty"`

// Matches define conditions used for matching the rule against incoming
Expand Down
2 changes: 1 addition & 1 deletion apis/v1alpha2/tcproute_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ type TCPRouteSpec struct {
//
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MaxItems=16
// +kubebuilder:validation:XValidation:message="Rule name must be unique within the route",rule="self.all(l1, !has(l1.name) || self.exists_one(l2, has(l2.name) && l1.name == l2.name))"
// +kubebuilder:experimental:validation:XValidation:message="Rule name must be unique within the route",rule="self.all(l1, !has(l1.name) || self.exists_one(l2, has(l2.name) && l1.name == l2.name))"
Rules []TCPRouteRule `json:"rules"`
}

Expand Down
2 changes: 1 addition & 1 deletion apis/v1alpha2/tlsroute_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ type TLSRouteSpec struct {
//
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MaxItems=16
// +kubebuilder:validation:XValidation:message="Rule name must be unique within the route",rule="self.all(l1, !has(l1.name) || self.exists_one(l2, has(l2.name) && l1.name == l2.name))"
// +kubebuilder:experimental:validation:XValidation:message="Rule name must be unique within the route",rule="self.all(l1, !has(l1.name) || self.exists_one(l2, has(l2.name) && l1.name == l2.name))"
Rules []TLSRouteRule `json:"rules"`
}

Expand Down
2 changes: 1 addition & 1 deletion apis/v1alpha2/udproute_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ type UDPRouteSpec struct {
//
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MaxItems=16
// +kubebuilder:validation:XValidation:message="Rule name must be unique within the route",rule="self.all(l1, !has(l1.name) || self.exists_one(l2, has(l2.name) && l1.name == l2.name))"
// +kubebuilder:experimental:validation:XValidation:message="Rule name must be unique within the route",rule="self.all(l1, !has(l1.name) || self.exists_one(l2, has(l2.name) && l1.name == l2.name))"
Rules []UDPRouteRule `json:"rules"`
}

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 0 additions & 26 deletions config/crd/standard/gateway.networking.k8s.io_grpcroutes.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 0 additions & 26 deletions config/crd/standard/gateway.networking.k8s.io_httproutes.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions pkg/generated/openapi/zz_generated.openapi.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

60 changes: 60 additions & 0 deletions pkg/test/cel/httproute_experimental_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,3 +372,63 @@ func TestHTTPRouteTimeouts(t *testing.T) {
})
}
}

func TestHTTPRouteRuleExperimental(t *testing.T) {
tests := []struct {
name string
wantErrors []string
rules []gatewayv1.HTTPRouteRule
}{
{
name: "invalid because multiple names are repeated",
wantErrors: []string{"Rule name must be unique within the route"},
rules: []gatewayv1.HTTPRouteRule{
{
Name: ptrTo(gatewayv1.SectionName("name1")),
},
{
Name: ptrTo(gatewayv1.SectionName("name1")),
},
},
},
{
name: "invalid because multiple names are repeated with others",
wantErrors: []string{"Rule name must be unique within the route"},
rules: []gatewayv1.HTTPRouteRule{
{
Name: ptrTo(gatewayv1.SectionName("name1")),
},
{
Name: ptrTo(gatewayv1.SectionName("not-name1")),
},
{
Name: ptrTo(gatewayv1.SectionName("name1")),
},
},
},
{
name: "valid because names are unique",
wantErrors: nil,
rules: []gatewayv1.HTTPRouteRule{
// Ok to have multiple nil
{Name: nil},
{Name: nil},
{Name: ptrTo(gatewayv1.SectionName("name1"))},
{Name: ptrTo(gatewayv1.SectionName("name2"))},
},
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
route := &gatewayv1.HTTPRoute{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("foo-%v", time.Now().UnixNano()),
Namespace: metav1.NamespaceDefault,
},
Spec: gatewayv1.HTTPRouteSpec{Rules: tc.rules},
}
validateHTTPRoute(t, route, tc.wantErrors)
})
}
}
Loading

0 comments on commit 0643c0d

Please sign in to comment.