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

feat: validate HTTPRoute features #5312

Merged
merged 2 commits into from
Dec 12, 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
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,14 @@ Adding a new version? You'll need three changes:
invalid confiugration of plugins for `KongPlugin`s or `KongClusterPlugin`s
referencing to the secret.
[#5203](https://github.com/Kong/kubernetes-ingress-controller/pull/5203)
- Validate `HTTPRoute` in admission webhook and reject it if the spec uses
the following features that we do not support:
- `parentRefs` other than `gatewayapi.networking.k8s.io/Gateway`
- using `timeouts` in rules
randmonkey marked this conversation as resolved.
Show resolved Hide resolved
- `URLRewrite`, `RequestMirror` filters
- using filters in backendRefs of rules
[#5312](https://github.com/Kong/kubernetes-ingress-controller/pull/5312)


### Fixed

Expand Down
74 changes: 62 additions & 12 deletions internal/admission/validation/gateway/httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,25 @@ func ValidateHTTPRoute(
return ok, msg, nil
}

// ValidateHTTPRouteParentRefs checks the group/kind of each parentRef in spec and allows only
// empty or `gateway.networking.k8s.io.Gateway`.
func ValidateHTTPRouteParentRefs(httproute *gatewayapi.HTTPRoute) error {
const KindGateway = gatewayapi.Kind("Gateway")

for parentRefIndex, parentRef := range httproute.Spec.ParentRefs {
if parentRef.Group != nil && *parentRef.Group != "" && *parentRef.Group != gatewayapi.V1Group {
return fmt.Errorf("parentRefs[%d]: %s is not a supported group for httproute parentRefs, only %s is supported",
parentRefIndex, *parentRef.Group, gatewayapi.V1Group)
}
if parentRef.Kind != nil && *parentRef.Kind != "" && *parentRef.Kind != KindGateway {
return fmt.Errorf("parentRefs[%d]: %s is not a supported kind for httproute parentRefs, only kind %s is supported",
parentRefIndex, *parentRef.Kind, KindGateway)
}
}

return nil
}

// -----------------------------------------------------------------------------
// Validation - HTTPRoute - Private Functions
// -----------------------------------------------------------------------------
Expand Down Expand Up @@ -97,22 +116,53 @@ func validateHTTPRouteListener(listener *gatewayapi.Listener) error {
// HTTPRoute implementation and validates that the provided object is not using
// any of those unsupported features.
func validateHTTPRouteFeatures(httproute *gatewayapi.HTTPRoute, translatorFeatures translator.FeatureFlags) error {
for _, rule := range httproute.Spec.Rules {
for _, match := range rule.Matches {
// We support query parameters matching rules only with expression router.
if len(match.QueryParams) != 0 {
if !translatorFeatures.ExpressionRoutes {
return fmt.Errorf("queryparam matching is supported with expression router only")
}
unsupportedFilterMap := map[gatewayapi.HTTPRouteFilterType]struct{}{
gatewayapi.HTTPRouteFilterURLRewrite: {},
gatewayapi.HTTPRouteFilterRequestMirror: {},
}
const (
KindService = gatewayapi.Kind("Service")
)

for ruleIndex, rule := range httproute.Spec.Rules {
// Rule timeout is not supported.
// TODO: remove the check after https://github.com/Kong/kubernetes-ingress-controller/issues/4914 fixed.
if rule.Timeouts != nil {
return fmt.Errorf("rules[%d]: rule timeout is unsupported", ruleIndex)
}
// Filters URLRewrite, RequestMirror are not supported.
for filterIndex, filter := range rule.Filters {
if _, unsupported := unsupportedFilterMap[filter.Type]; unsupported {
return fmt.Errorf("rules[%d].filters[%d]: filter type %s is unsupported",
ruleIndex, filterIndex, filter.Type)
}
}
// We don't support any backendRef types except Kubernetes Services.
for _, ref := range rule.BackendRefs {

for refIndex, ref := range rule.BackendRefs {
// Specifying filters in backendRef is not supported.
if len(ref.Filters) != 0 {
return fmt.Errorf("rules[%d].backendRefs[%d]: filters in backendRef is unsupported",
ruleIndex, refIndex)
}

// We don't support any backendRef types except Kubernetes Services.
if ref.BackendRef.Group != nil && *ref.BackendRef.Group != "core" && *ref.BackendRef.Group != "" {
return fmt.Errorf("%s is not a supported group for httproute backendRefs, only core is supported", *ref.BackendRef.Group)
return fmt.Errorf("rules[%d].backendRefs[%d]: %s is not a supported group for httproute backendRefs, only core is supported",
ruleIndex, refIndex, *ref.BackendRef.Group)
}
if ref.BackendRef.Kind != nil && *ref.BackendRef.Kind != KindService {
return fmt.Errorf("rules[%d].backendRefs[%d]: %s is not a supported kind for httproute backendRefs, only %s is supported",
ruleIndex, refIndex, *ref.BackendRef.Kind, KindService)
randmonkey marked this conversation as resolved.
Show resolved Hide resolved
}
if ref.BackendRef.Kind != nil && *ref.BackendRef.Kind != "Service" {
return fmt.Errorf("%s is not a supported kind for httproute backendRefs, only Service is supported", *ref.BackendRef.Kind)
}

for matchIndex, match := range rule.Matches {
// We support query parameters matching rules only with expression router.
if len(match.QueryParams) != 0 {
if !translatorFeatures.ExpressionRoutes {
return fmt.Errorf("rules[%d].matches[%d]: queryparam matching is supported with expression router only",
ruleIndex, matchIndex)
}
}
}
}
Expand Down
182 changes: 179 additions & 3 deletions internal/admission/validation/gateway/httproute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"

"github.com/kong/go-kong/kong"
"github.com/samber/lo"
"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -248,7 +249,7 @@ func TestValidateHTTPRoute(t *testing.T) {
},
}},
valid: false,
validationMsg: "HTTPRoute spec did not pass validation: queryparam matching is supported with expression router only",
validationMsg: "HTTPRoute spec did not pass validation: rules[0].matches[0]: queryparam matching is supported with expression router only",
},
{
msg: "we don't support any group except core kubernetes for backendRefs",
Expand Down Expand Up @@ -305,7 +306,7 @@ func TestValidateHTTPRoute(t *testing.T) {
},
}},
valid: false,
validationMsg: "HTTPRoute spec did not pass validation: example is not a supported group for httproute backendRefs, only core is supported",
validationMsg: "HTTPRoute spec did not pass validation: rules[0].backendRefs[0]: 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 @@ -361,7 +362,182 @@ func TestValidateHTTPRoute(t *testing.T) {
},
}},
valid: false,
validationMsg: "HTTPRoute spec did not pass validation: Pod is not a supported kind for httproute backendRefs, only Service is supported",
validationMsg: "HTTPRoute spec did not pass validation: rules[0].backendRefs[0]: Pod is not a supported kind for httproute backendRefs, only Service is supported",
},
{
msg: "we do not support RequestMirror filter",
route: &gatewayapi.HTTPRoute{
ObjectMeta: metav1.ObjectMeta{
Namespace: corev1.NamespaceDefault,
Name: "testing-httproute",
},
Spec: gatewayapi.HTTPRouteSpec{
CommonRouteSpec: gatewayapi.CommonRouteSpec{
ParentRefs: []gatewayapi.ParentReference{{
Name: "testing-gateway",
}},
},
Rules: []gatewayapi.HTTPRouteRule{{
Matches: []gatewayapi.HTTPRouteMatch{{
Headers: []gatewayapi.HTTPHeaderMatch{{
Name: "Content-Type",
Value: "audio/vorbis",
}},
}},
BackendRefs: []gatewayapi.HTTPBackendRef{
{
BackendRef: gatewayapi.BackendRef{
BackendObjectReference: gatewayapi.BackendObjectReference{
Name: "service1",
},
},
},
},
Filters: []gatewayapi.HTTPRouteFilter{
{
Type: gatewayapi.HTTPRouteFilterRequestMirror,
},
},
}},
},
},
gateways: []*gatewayapi.Gateway{{
ObjectMeta: metav1.ObjectMeta{
Namespace: corev1.NamespaceDefault,
Name: "testing-gateway",
},
Spec: gatewayapi.GatewaySpec{
Listeners: []gatewayapi.Listener{{
Name: "http",
Port: 80,
Protocol: (gatewayapi.HTTPProtocolType),
AllowedRoutes: &gatewayapi.AllowedRoutes{
Kinds: []gatewayapi.RouteGroupKind{{
Group: &group,
Kind: "HTTPRoute",
}},
},
}},
},
}},
valid: false,
validationMsg: "HTTPRoute spec did not pass validation: rules[0].filters[0]: filter type RequestMirror is unsupported",
},
{
msg: "we do not support setting timeouts on rules",
route: &gatewayapi.HTTPRoute{
ObjectMeta: metav1.ObjectMeta{
Namespace: corev1.NamespaceDefault,
Name: "testing-httproute",
},
Spec: gatewayapi.HTTPRouteSpec{
CommonRouteSpec: gatewayapi.CommonRouteSpec{
ParentRefs: []gatewayapi.ParentReference{{
Name: "testing-gateway",
}},
},
Rules: []gatewayapi.HTTPRouteRule{{
Matches: []gatewayapi.HTTPRouteMatch{{
Headers: []gatewayapi.HTTPHeaderMatch{{
Name: "Content-Type",
Value: "audio/vorbis",
}},
}},
BackendRefs: []gatewayapi.HTTPBackendRef{
{
BackendRef: gatewayapi.BackendRef{
BackendObjectReference: gatewayapi.BackendObjectReference{
Name: "service1",
},
},
},
},
Timeouts: &gatewayapi.HTTPRouteTimeouts{
Request: lo.ToPtr(gatewayapi.Duration("1s")),
},
}},
},
},
gateways: []*gatewayapi.Gateway{{
ObjectMeta: metav1.ObjectMeta{
Namespace: corev1.NamespaceDefault,
Name: "testing-gateway",
},
Spec: gatewayapi.GatewaySpec{
Listeners: []gatewayapi.Listener{{
Name: "http",
Port: 80,
Protocol: (gatewayapi.HTTPProtocolType),
AllowedRoutes: &gatewayapi.AllowedRoutes{
Kinds: []gatewayapi.RouteGroupKind{{
Group: &group,
Kind: "HTTPRoute",
}},
},
}},
},
}},
valid: false,
validationMsg: "HTTPRoute spec did not pass validation: rules[0]: rule timeout is unsupported",
},
{
msg: "we do not support filters in backendRefs",
route: &gatewayapi.HTTPRoute{
ObjectMeta: metav1.ObjectMeta{
Namespace: corev1.NamespaceDefault,
Name: "testing-httproute",
},
Spec: gatewayapi.HTTPRouteSpec{
CommonRouteSpec: gatewayapi.CommonRouteSpec{
ParentRefs: []gatewayapi.ParentReference{{
Name: "testing-gateway",
}},
},
Rules: []gatewayapi.HTTPRouteRule{{
Matches: []gatewayapi.HTTPRouteMatch{{
Headers: []gatewayapi.HTTPHeaderMatch{{
Name: "Content-Type",
Value: "audio/vorbis",
}},
}},
BackendRefs: []gatewayapi.HTTPBackendRef{
{
BackendRef: gatewayapi.BackendRef{
BackendObjectReference: gatewayapi.BackendObjectReference{
Name: "service1",
},
},
Filters: []gatewayapi.HTTPRouteFilter{
{
Type: gatewayapi.HTTPRouteFilterRequestHeaderModifier,
},
},
},
},
}},
},
},
gateways: []*gatewayapi.Gateway{{
ObjectMeta: metav1.ObjectMeta{
Namespace: corev1.NamespaceDefault,
Name: "testing-gateway",
},
Spec: gatewayapi.GatewaySpec{
Listeners: []gatewayapi.Listener{{
Name: "http",
Port: 80,
Protocol: (gatewayapi.HTTPProtocolType),
AllowedRoutes: &gatewayapi.AllowedRoutes{
Kinds: []gatewayapi.RouteGroupKind{{
Group: &group,
Kind: "HTTPRoute",
}},
},
}},
},
}},
valid: false,
validationMsg: "HTTPRoute spec did not pass validation: rules[0].backendRefs[0]: filters in backendRef is unsupported",
},
} {
t.Run(tt.msg, func(t *testing.T) {
Expand Down
3 changes: 3 additions & 0 deletions internal/admission/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,9 @@ func (validator KongHTTPValidator) ValidateGateway(
func (validator KongHTTPValidator) ValidateHTTPRoute(
ctx context.Context, httproute gatewayapi.HTTPRoute,
) (bool, string, error) {
if err := gatewayvalidation.ValidateHTTPRouteParentRefs(&httproute); err != nil {
return false, fmt.Sprintf("HTTPRoute has invalid parentRef: %s", err), nil
}
// in order to be sure whether or not an HTTPRoute resource is managed by this
// controller we disallow references to Gateway resources that do not exist.
var managedGateways []*gatewayapi.Gateway
Expand Down
Loading