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

v1alpha2 validation fix/update #831

Closed
wants to merge 29 commits into from
Closed
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
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
15 changes: 12 additions & 3 deletions apis/v1alpha2/validation/validation_test.go → apis/util/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,27 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package validation
package utils
ccfishk marked this conversation as resolved.
Show resolved Hide resolved

import (
gatewayv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
)

func pathMatchTypePtr(s string) *gatewayv1a2.PathMatchType {
// PathMatchTypePtr generate path match type
ccfishk marked this conversation as resolved.
Show resolved Hide resolved
func PathMatchTypePtr(s string) *gatewayv1a2.PathMatchType {
if s != string(gatewayv1a2.PathMatchExact) && s != string(gatewayv1a2.PathMatchPrefix) && s != string(gatewayv1a2.PathMatchRegularExpression) &&
s != string(gatewayv1a2.PathMatchImplementationSpecific) {
return nil
}
result := gatewayv1a2.PathMatchType(s)
return &result
}

func portNumberPtr(p int) *gatewayv1a2.PortNumber {
// PortNumberPtr generate port number
ccfishk marked this conversation as resolved.
Show resolved Hide resolved
func PortNumberPtr(p int) *gatewayv1a2.PortNumber {
if p < 1 || p > 65535 {
return nil
}
result := gatewayv1a2.PortNumber(p)
return &result
}
96 changes: 96 additions & 0 deletions apis/util/utils_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package utils

import (
"fmt"
"testing"

gatewayv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
)

func Test_Utils(t *testing.T) {
ccfishk marked this conversation as resolved.
Show resolved Hide resolved

var exportedPort1 gatewayv1a2.PortNumber = 65535
var exportedPort3 gatewayv1a2.PortNumber = 1

var expectedPathMatch1 = gatewayv1a2.PathMatchExact
var expectedPathMatch2 = gatewayv1a2.PathMatchPrefix
var expectedPathMatch3 = gatewayv1a2.PathMatchRegularExpression
var expectedPathMatch4 = gatewayv1a2.PathMatchImplementationSpecific
ccfishk marked this conversation as resolved.
Show resolved Hide resolved

table := []struct {
pathType string
expectedPath *gatewayv1a2.PathMatchType
port int
expectedPort *gatewayv1a2.PortNumber
}{
{
pathType: "Exact",
expectedPath: &expectedPathMatch1,
port: 0,
expectedPort: nil,
},
{
pathType: "Exact",
expectedPath: &expectedPathMatch1,
port: 65535,
expectedPort: &exportedPort1,
},
{
pathType: "Exact",
expectedPath: &expectedPathMatch1,
port: 65536,
expectedPort: nil,
},
{
pathType: "Prefix",
expectedPath: &expectedPathMatch2,
port: 0,
expectedPort: nil,
},
{
pathType: "RegularExpression",
expectedPath: &expectedPathMatch3,
port: 65536,
expectedPort: nil,
},
{
pathType: "ImplementationSpecific",
expectedPath: &expectedPathMatch4,
port: 1,
expectedPort: &exportedPort3,
},
{
pathType: "APrefix",
expectedPath: nil,
port: 1,
expectedPort: &exportedPort3,
},
}

for _, entry := range table {
path := PathMatchTypePtr(entry.pathType)
ccfishk marked this conversation as resolved.
Show resolved Hide resolved
if path == nil && entry.expectedPath != nil {
t.Error("failed in path match type pointer. not expecting nil, but get nil.")
ccfishk marked this conversation as resolved.
Show resolved Hide resolved
}

if path != nil && entry.expectedPath == nil {
t.Error("failed in path match type pointer. expecting nil but get non-nil")
}

if path != nil && entry.expectedPath != nil && *path != *entry.expectedPath {
t.Error("failed in path match type pointer. go unexpected.")
}

port := PortNumberPtr(entry.port)
if port == nil && entry.expectedPort != nil {
t.Error("failed in port number pointer. not expected nil, but got nil.")
}
if port != nil && entry.expectedPort == nil {
t.Error("failed in port number port. expecting nil, but got non-nil.")
}
if port != nil && entry.expectedPort != nil && *port != *entry.expectedPort {
fmt.Printf("failed in port number expecting %d got port %d", entry.expectedPort, *port)
}
ccfishk marked this conversation as resolved.
Show resolved Hide resolved

}
}
33 changes: 31 additions & 2 deletions apis/v1alpha2/validation/httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@ func ValidateHTTPRoute(route *gatewayv1a2.HTTPRoute) field.ErrorList {
// validateHTTPRouteSpec validates that required fields of spec are set according to the
// HTTPRoute specification.
func validateHTTPRouteSpec(spec *gatewayv1a2.HTTPRouteSpec, path *field.Path) field.ErrorList {
return validateHTTPRouteUniqueFilters(spec.Rules, path.Child("rules"))
if errList := validateHTTPRouteUniqueFilters(spec.Rules, path.Child("rules")); len(errList) > 0 {
return errList
}

return nil
ccfishk marked this conversation as resolved.
Show resolved Hide resolved
}

// validateHTTPRouteUniqueFilters validates whether each core and extended filter
Expand All @@ -55,7 +59,7 @@ func validateHTTPRouteUniqueFilters(rules []gatewayv1a2.HTTPRouteRule, path *fie
}
// custom filters don't have any validation
for _, key := range repeatableHTTPRouteFilters {
counts[key] = 0
delete(counts, key)
}

for filterType, count := range counts {
Expand All @@ -64,7 +68,32 @@ func validateHTTPRouteUniqueFilters(rules []gatewayv1a2.HTTPRouteRule, path *fie
}
}

if errList := validateHttpRouteBackendRefs(rule.BackendRefs, path, i); len(errList) > 0 {
errs = append(errs, errList...)
}
}

return errs
}

func validateHttpRouteBackendRefs(ref []gatewayv1a2.HTTPBackendRef, path *field.Path, i int) field.ErrorList {
ccfishk marked this conversation as resolved.
Show resolved Hide resolved
var errs field.ErrorList

for _, bkr := range ref {
counts := map[gatewayv1a2.HTTPRouteFilterType]int{}
for _, filter := range bkr.Filters {
counts[filter.Type]++
}

for _, key := range repeatableHTTPRouteFilters {
delete(counts, key)
}

for filterType, count := range counts {
if count > 1 {
errs = append(errs, field.Invalid(path.Index(i).Child("BackendRefs"), filterType, "cannot be used multiple times in the same rule"))
ccfishk marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
return errs
}
ccfishk marked this conversation as resolved.
Show resolved Hide resolved
44 changes: 22 additions & 22 deletions apis/v1alpha2/validation/httproute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ package validation
import (
"testing"

"k8s.io/apimachinery/pkg/util/validation/field"
utilpointer "k8s.io/utils/pointer"

apiutils "sigs.k8s.io/gateway-api/apis/util"
gatewayv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
)

Expand All @@ -41,7 +43,7 @@ func TestValidateHTTPRoute(t *testing.T) {
Matches: []gatewayv1a2.HTTPRouteMatch{
{
Path: &gatewayv1a2.HTTPPathMatch{
Type: pathMatchTypePtr("Prefix"),
Type: apiutils.PathMatchTypePtr("Prefix"),
Value: utilpointer.String("/"),
},
},
Expand All @@ -51,7 +53,7 @@ func TestValidateHTTPRoute(t *testing.T) {
BackendRef: gatewayv1a2.BackendRef{
BackendObjectReference: gatewayv1a2.BackendObjectReference{
Name: testService,
Port: portNumberPtr(8080),
Port: apiutils.PortNumberPtr(8080),
},
Weight: utilpointer.Int32(100),
},
Expand All @@ -72,7 +74,7 @@ func TestValidateHTTPRoute(t *testing.T) {
Matches: []gatewayv1a2.HTTPRouteMatch{
{
Path: &gatewayv1a2.HTTPPathMatch{
Type: pathMatchTypePtr("Prefix"),
Type: apiutils.PathMatchTypePtr("Prefix"),
Value: utilpointer.String("/"),
},
},
Expand All @@ -83,7 +85,7 @@ func TestValidateHTTPRoute(t *testing.T) {
RequestMirror: &gatewayv1a2.HTTPRequestMirrorFilter{
BackendRef: &gatewayv1a2.BackendObjectReference{
Name: testService,
Port: portNumberPtr(8081),
Port: apiutils.PortNumberPtr(8081),
},
},
},
Expand All @@ -103,7 +105,7 @@ func TestValidateHTTPRoute(t *testing.T) {
Matches: []gatewayv1a2.HTTPRouteMatch{
{
Path: &gatewayv1a2.HTTPPathMatch{
Type: pathMatchTypePtr("Prefix"),
Type: apiutils.PathMatchTypePtr("Prefix"),
Value: utilpointer.String("/"),
},
},
Expand All @@ -114,7 +116,7 @@ func TestValidateHTTPRoute(t *testing.T) {
RequestMirror: &gatewayv1a2.HTTPRequestMirrorFilter{
BackendRef: &gatewayv1a2.BackendObjectReference{
Name: testService,
Port: portNumberPtr(8080),
Port: apiutils.PortNumberPtr(8080),
},
},
},
Expand All @@ -123,7 +125,7 @@ func TestValidateHTTPRoute(t *testing.T) {
RequestMirror: &gatewayv1a2.HTTPRequestMirrorFilter{
BackendRef: &gatewayv1a2.BackendObjectReference{
Name: specialService,
Port: portNumberPtr(8080),
Port: apiutils.PortNumberPtr(8080),
},
},
},
Expand All @@ -143,7 +145,7 @@ func TestValidateHTTPRoute(t *testing.T) {
Matches: []gatewayv1a2.HTTPRouteMatch{
{
Path: &gatewayv1a2.HTTPPathMatch{
Type: pathMatchTypePtr("Prefix"),
Type: apiutils.PathMatchTypePtr("Prefix"),
Value: utilpointer.String("/"),
},
},
Expand All @@ -165,7 +167,7 @@ func TestValidateHTTPRoute(t *testing.T) {
RequestMirror: &gatewayv1a2.HTTPRequestMirrorFilter{
BackendRef: &gatewayv1a2.BackendObjectReference{
Name: testService,
Port: portNumberPtr(8080),
Port: apiutils.PortNumberPtr(8080),
},
},
},
Expand Down Expand Up @@ -196,7 +198,7 @@ func TestValidateHTTPRoute(t *testing.T) {
Matches: []gatewayv1a2.HTTPRouteMatch{
{
Path: &gatewayv1a2.HTTPPathMatch{
Type: pathMatchTypePtr("Prefix"),
Type: apiutils.PathMatchTypePtr("Prefix"),
Value: utilpointer.String("/"),
},
},
Expand All @@ -207,7 +209,7 @@ func TestValidateHTTPRoute(t *testing.T) {
RequestMirror: &gatewayv1a2.HTTPRequestMirrorFilter{
BackendRef: &gatewayv1a2.BackendObjectReference{
Name: testService,
Port: portNumberPtr(8080),
Port: apiutils.PortNumberPtr(8080),
},
},
},
Expand All @@ -227,7 +229,7 @@ func TestValidateHTTPRoute(t *testing.T) {
RequestMirror: &gatewayv1a2.HTTPRequestMirrorFilter{
BackendRef: &gatewayv1a2.BackendObjectReference{
Name: testService,
Port: portNumberPtr(8080),
Port: apiutils.PortNumberPtr(8080),
},
},
},
Expand All @@ -247,7 +249,7 @@ func TestValidateHTTPRoute(t *testing.T) {
RequestMirror: &gatewayv1a2.HTTPRequestMirrorFilter{
BackendRef: &gatewayv1a2.BackendObjectReference{
Name: specialService,
Port: portNumberPtr(8080),
Port: apiutils.PortNumberPtr(8080),
},
},
},
Expand All @@ -267,7 +269,7 @@ func TestValidateHTTPRoute(t *testing.T) {
Matches: []gatewayv1a2.HTTPRouteMatch{
{
Path: &gatewayv1a2.HTTPPathMatch{
Type: pathMatchTypePtr("Prefix"),
Type: apiutils.PathMatchTypePtr("Prefix"),
Value: utilpointer.String("/"),
},
},
Expand All @@ -289,7 +291,7 @@ func TestValidateHTTPRoute(t *testing.T) {
RequestMirror: &gatewayv1a2.HTTPRequestMirrorFilter{
BackendRef: &gatewayv1a2.BackendObjectReference{
Name: testService,
Port: portNumberPtr(8080),
Port: apiutils.PortNumberPtr(8080),
},
},
},
Expand All @@ -312,12 +314,10 @@ func TestValidateHTTPRoute(t *testing.T) {
}
for _, tt := range tests {
// copy variable to avoid scope problems with ranges
tt := tt
t.Run(tt.name, func(t *testing.T) {
ccfishk marked this conversation as resolved.
Show resolved Hide resolved
errs := ValidateHTTPRoute(&tt.hRoute)
if len(errs) != tt.errCount {
t.Errorf("ValidateHTTPRoute() got %v errors, want %v errors", len(errs), tt.errCount)
}
})
t.Logf("running test case [%s]", tt.name)
errs := validateHTTPRouteUniqueFilters(tt.hRoute.Spec.Rules, field.NewPath("spec").Child("rules"))
if len(errs) != tt.errCount {
t.Errorf("ValidateHTTPRoute() got %v errors, want %v errors", len(errs), tt.errCount)
}
}
}