Skip to content

Commit

Permalink
fix httproute admission
Browse files Browse the repository at this point in the history
  • Loading branch information
czeslavo committed Nov 2, 2023
1 parent 91510e7 commit 178b999
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 32 deletions.
21 changes: 11 additions & 10 deletions internal/admission/validation/gateway/httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func ValidateHTTPRoute(
) (bool, string, error) {
// validate that no unsupported features are in use
if err := validateHTTPRouteFeatures(httproute, parserFeatures); err != nil {
return false, "HTTPRoute spec did not pass validation", err
return false, fmt.Sprintf("HTTPRoute spec did not pass validation: %s", err), nil
}

// perform Gateway validations for the HTTPRoute (e.g. listener validation, namespace validation, e.t.c.)
Expand All @@ -45,24 +45,25 @@ func ValidateHTTPRoute(
// determine the parentRef for this gateway
parentRef, err := getParentRefForHTTPRouteGateway(httproute, gateway)
if err != nil {
return false, "Couldn't determine parentRefs for httproute", err
return false, fmt.Sprintf("Couldn't determine parentRefs for httproute: %s", err), nil
}

// gather the relevant gateway listeners for the httproute
listeners, err := getListenersForHTTPRouteValidation(parentRef.SectionName, gateway)
if err != nil {
return false, "Couldn't find gateway listeners for httproute", err
return false, fmt.Sprintf("Couldn't find gateway listeners for httproute: %s", err), nil
}

// perform validation of this route against it's linked gateway listeners
for _, listener := range listeners {
if err := validateHTTPRouteListener(listener); err != nil {
return false, "HTTPRoute linked Gateway listeners did not pass validation", err
return false, fmt.Sprintf("HTTPRoute linked Gateway listeners did not pass validation: %s", err), nil
}
}
}

return validateWithKongGateway(ctx, routesValidator, parserFeatures, httproute)
ok, msg := validateWithKongGateway(ctx, routesValidator, parserFeatures, httproute)
return ok, msg, nil
}

// -----------------------------------------------------------------------------
Expand Down Expand Up @@ -188,7 +189,7 @@ func getListenersForHTTPRouteValidation(sectionName *gatewayapi.SectionName, gat

func validateWithKongGateway(
ctx context.Context, routesValidator routeValidator, parserFeatures parser.FeatureFlags, httproute *gatewayapi.HTTPRoute,
) (bool, string, error) {
) (bool, string) {
// Translate HTTPRoute to Kong Route object(s) that can be sent directly to the Admin API for validation.
// Use KIC parser that works both for traditional and expressions based routes.
var kongRoutes []kong.Route
Expand All @@ -211,23 +212,23 @@ func validateWithKongGateway(
}
}
if len(errMsgs) > 0 {
return false, validationMsg(errMsgs), nil
return false, validationMsg(errMsgs)
}
// Validate by using feature of Kong Gateway.
for _, kg := range kongRoutes {
kg := kg
ok, msg, err := routesValidator.Validate(ctx, &kg)
if err != nil {
return false, fmt.Sprintf("Unable to validate HTTPRoute schema: %s", err.Error()), nil
return false, fmt.Sprintf("Unable to validate HTTPRoute schema: %s", err.Error())
}
if !ok {
errMsgs = append(errMsgs, msg)
}
}
if len(errMsgs) > 0 {
return false, validationMsg(errMsgs), nil
return false, validationMsg(errMsgs)
}
return true, "", nil
return true, ""
}

func validationMsg(errMsgs []string) string {
Expand Down
38 changes: 16 additions & 22 deletions internal/admission/validation/gateway/httproute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package gateway

import (
"context"
"fmt"
"testing"

"github.com/kong/go-kong/kong"
Expand Down Expand Up @@ -59,8 +58,7 @@ func TestValidateHTTPRoute(t *testing.T) {
},
}},
valid: false,
validationMsg: "Couldn't determine parentRefs for httproute",
err: fmt.Errorf("no parentRef matched gateway default/testing-gateway"),
validationMsg: "Couldn't determine parentRefs for httproute: no parentRef matched gateway default/testing-gateway",
},
{
msg: "if you use sectionname to attach to a non-existent gateway listener, it fails validation",
Expand Down Expand Up @@ -98,8 +96,7 @@ func TestValidateHTTPRoute(t *testing.T) {
},
}},
valid: false,
validationMsg: "Couldn't find gateway listeners for httproute",
err: fmt.Errorf("sectionname referenced listener listener-that-doesnt-exist was not found on gateway default/testing-gateway"),
validationMsg: "Couldn't find gateway listeners for httproute: sectionname referenced listener listener-that-doesnt-exist was not found on gateway default/testing-gateway",
},
{
msg: "if the provided gateway has NO listeners, the HTTPRoute fails validation",
Expand All @@ -126,8 +123,7 @@ func TestValidateHTTPRoute(t *testing.T) {
},
}},
valid: false,
validationMsg: "Couldn't find gateway listeners for httproute",
err: fmt.Errorf("no listeners could be found for gateway default/testing-gateway"),
validationMsg: "Couldn't find gateway listeners for httproute: no listeners could be found for gateway default/testing-gateway",
},
{
msg: "parentRefs which omit the namespace pass validation in the same namespace",
Expand Down Expand Up @@ -200,8 +196,7 @@ func TestValidateHTTPRoute(t *testing.T) {
},
}},
valid: false,
validationMsg: "HTTPRoute linked Gateway listeners did not pass validation",
err: fmt.Errorf("HTTPRoute not supported by listener http-alternate"),
validationMsg: "HTTPRoute linked Gateway listeners did not pass validation: HTTPRoute not supported by listener http-alternate",
},
{
msg: "if an HTTPRoute is using queryparams matching it fails validation due to only supporting expression router",
Expand Down Expand Up @@ -253,8 +248,7 @@ func TestValidateHTTPRoute(t *testing.T) {
},
}},
valid: false,
validationMsg: "HTTPRoute spec did not pass validation",
err: fmt.Errorf("queryparam matching is supported with expression router only"),
validationMsg: "HTTPRoute spec did not pass validation: queryparam matching is supported with expression router only",
},
{
msg: "we don't support any group except core kubernetes for backendRefs",
Expand Down Expand Up @@ -311,8 +305,7 @@ func TestValidateHTTPRoute(t *testing.T) {
},
}},
valid: false,
validationMsg: "HTTPRoute spec did not pass validation",
err: fmt.Errorf("example is not a supported group for httproute backendRefs, only core is supported"),
validationMsg: "HTTPRoute spec did not pass validation: example is not a supported group for httproute backendRefs, only core is supported",
},
{
msg: "we don't support any core kind except Service for backendRefs",
Expand Down Expand Up @@ -368,17 +361,18 @@ func TestValidateHTTPRoute(t *testing.T) {
},
}},
valid: false,
validationMsg: "HTTPRoute spec did not pass validation",
err: fmt.Errorf("Pod is not a supported kind for httproute backendRefs, only Service is supported"),
validationMsg: "HTTPRoute spec did not pass validation: Pod is not a supported kind for httproute backendRefs, only Service is supported",
},
} {
// Passed routesValidator is irrelevant for the above test cases.
valid, validMsg, err := ValidateHTTPRoute(
context.Background(), mockRoutesValidator{}, parser.FeatureFlags{}, tt.route, tt.gateways...,
)
assert.Equal(t, tt.valid, valid, tt.msg)
assert.Equal(t, tt.validationMsg, validMsg, tt.msg)
assert.Equal(t, tt.err, err, tt.msg)
t.Run(tt.msg, func(t *testing.T) {
// Passed routesValidator is irrelevant for the above test cases.
valid, validMsg, err := ValidateHTTPRoute(
context.Background(), mockRoutesValidator{}, parser.FeatureFlags{}, tt.route, tt.gateways...,
)
assert.Equal(t, tt.valid, valid, tt.msg)
assert.Equal(t, tt.validationMsg, validMsg, tt.msg)
assert.Equal(t, tt.err, err, tt.msg)
})
}
}

Expand Down

0 comments on commit 178b999

Please sign in to comment.