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

Update gatewayapi to v1.0.0 #286

Merged
merged 7 commits into from
Nov 14, 2023
Merged
Show file tree
Hide file tree
Changes from 6 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
36 changes: 19 additions & 17 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,23 @@
The Operator to install and manage the lifecycle of the [Kuadrant](https://github.com/Kuadrant/) components deployments.

<!--ts-->
* [Overview](#overview)
* [Architecture](#architecture)
* [Kuadrant components](#kuadrant-components)
* [Provided APIs](#provided-apis)
* [Getting started](#getting-started)
* [Pre-requisites](#pre-requisites)
* [Installing Kuadrant](#installing-kuadrant)
* [Protect Your Service](#protect-your-service)
* [If you are an <em>API Provider</em>](#if-you-are-an-api-provider)
* [If you are a <em>Cluster Operator</em>](#if-you-are-a-cluster-operator)
* [User guides](#user-guides)
* [<a href="doc/rate-limiting.md">Kuadrant Rate Limiting</a>](#kuadrant-rate-limiting)
* [Documentation](#documentation)
* [Contributing](#contributing)
* [Licensing](#licensing)
- [Overview](#overview)
- [Architecture](#architecture)
- [Kuadrant components](#kuadrant-components)
- [Provided APIs](#provided-apis)
- [Getting started](#getting-started)
- [Pre-requisites](#pre-requisites)
- [Installing Kuadrant](#installing-kuadrant)
- [1. Install the Kuadrant Operator](#1-install-the-kuadrant-operator)
- [2. Request a Kuadrant instance](#2-request-a-kuadrant-instance)
- [Protect your service](#protect-your-service)
- [If you are an *API Provider*](#if-you-are-an-api-provider)
- [If you are a *Cluster Operator*](#if-you-are-a-cluster-operator)
- [User guides](#user-guides)
- [Kuadrant Rate Limiting](#kuadrant-rate-limiting)
- [Documentation](#documentation)
- [Contributing](#contributing)
- [Licensing](#licensing)

<!--te-->

Expand Down Expand Up @@ -144,15 +146,15 @@ EOF

* Deploy the service/API to be protected ("Upstream")
* Expose the service/API using the kubernetes Gateway API, ie
[HTTPRoute](https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1beta1.HTTPRoute) object.
[HTTPRoute](https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.HTTPRoute) object.
* Write and apply the Kuadrant's [RateLimitPolicy](doc/rate-limiting.md) and/or
[AuthPolicy](doc/auth.md) custom resources targeting the HTTPRoute resource
to have your API protected.

#### If you are a *Cluster Operator*

* (Optionally) deploy istio ingress gateway using the
[Gateway](https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1beta1.Gateway) resource.
[Gateway](https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.Gateway) resource.
* Write and apply the Kuadrant's [RateLimitPolicy](doc/rate-limiting.md) and/or
[AuthPolicy](doc/auth.md) custom resources targeting the Gateway resource
to have your gateway traffic protected.
Expand Down
14 changes: 11 additions & 3 deletions api/v1beta2/authpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
"github.com/google/go-cmp/cmp"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"

authorinoapi "github.com/kuadrant/authorino/api/v1beta2"
"github.com/kuadrant/kuadrant-operator/pkg/common"
Expand All @@ -18,16 +18,19 @@
// Authentication configs.
// At least one config MUST evaluate to a valid identity object for the auth request to be successful.
// +optional
// +kubebuilder:validation:MaxProperties=14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are going with the hard limits for now. We hope soon to be able to remove or substantially increase these limits of maximum number of configs per AP/RLP, by redefining those GWAPI types (HTTPRouteMatch and related) into our code base without the CEL validation rules. After all, Kuadrant API does not need to re-impose validation on those fields if they have been validated once applied in the target GWAPI resources.

Copy link
Contributor

@eguzki eguzki Nov 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, can you briefly explain what is this about? I read from the link

This would work fine until v0.7, but since validation rules based on [Common Expression Language (CEL)](https://github.com/google/cel-go) have been added to the HTTPRoute (https://github.com/kubernetes-sigs/gateway-api/pull/2253), the estimated cost limit exceeds the thresholds, and we have been struggling with having to set some hard constraints on the number of policy rules a user can declare.

What do you mean by the estimated cost limit exceeds the thresholds ? The client upon creating an object is giving timeouts because k8s API server takes too long to validate the resource?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can try to provide some info here @eguzki.

When you submit a CRD to the kube apiserver it makes an estimate of how costly the CEL validation rules are, so that when a CR of this type is submitted to the cluster it can evaluate it. There is a cost limit for this validation, and so if the estimated cost exceeds the limit the CRD is rejected, as the kube apiserver determines it would be too costly for it to perform validations on CR's that are created of this type.

The client upon creating an object is giving timeouts because k8s API server takes too long to validate the resource?

To avoid this case it rejects the CRD early based on the estimation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation.

And those numbers MaxProperties=14 are magic numbers or they have a reasoning behind them? What are those number depending on?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Authentication map[string]AuthenticationSpec `json:"authentication,omitempty"`

// Metadata sources.
// Authorino fetches auth metadata as JSON from sources specified in this config.
// +optional
// +kubebuilder:validation:MaxProperties=14
Metadata map[string]MetadataSpec `json:"metadata,omitempty"`

// Authorization policies.
// All policies MUST evaluate to "allowed = true" for the auth request be successful.
// +optional
// +kubebuilder:validation:MaxProperties=14
Authorization map[string]AuthorizationSpec `json:"authorization,omitempty"`

// Response items.
Expand All @@ -38,6 +41,7 @@
// Callback functions.
// Authorino sends callbacks at the end of the auth pipeline to the endpoints specified in this config.
// +optional
// +kubebuilder:validation:MaxProperties=14
Callbacks map[string]CallbackSpec `json:"callbacks,omitempty"`
}

Expand All @@ -47,6 +51,7 @@
// At least one selected HTTPRoute rule must match to trigger the auth rule.
// If no route selectors are specified, the auth rule will be evaluated at all requests to the protected routes.
// +optional
// +kubebuilder:validation:MaxItems=15
RouteSelectors []RouteSelector `json:"routeSelectors,omitempty"`
}

Expand Down Expand Up @@ -93,11 +98,13 @@
type WrappedSuccessResponseSpec struct {
// Custom success response items wrapped as HTTP headers.
// For integration of Authorino via proxy, the proxy must use these settings to inject data in the request.
// +kubebuilder:validation:MaxProperties=14
Headers map[string]HeaderSuccessResponseSpec `json:"headers,omitempty"`

// Custom success response items wrapped as HTTP headers.
// For integration of Authorino via proxy, the proxy must use these settings to propagate dynamic metadata.
// See https://www.envoyproxy.io/docs/envoy/latest/configuration/advanced/well_known_dynamic_metadata
// +kubebuilder:validation:MaxProperties=14
DynamicMetadata map[string]SuccessResponseSpec `json:"dynamicMetadata,omitempty"`
}

Expand Down Expand Up @@ -133,6 +140,7 @@
// At least one selected HTTPRoute rule must match to trigger the AuthPolicy.
// If no route selectors are specified, the AuthPolicy will be enforced at all requests to the protected routes.
// +optional
// +kubebuilder:validation:MaxItems=15
RouteSelectors []RouteSelector `json:"routeSelectors,omitempty"`

// Named sets of patterns that can be referred in `when` conditions and in pattern-matching authorization policy rules.
Expand Down Expand Up @@ -266,8 +274,8 @@
return ap.Spec.TargetRef
}

func (ap *AuthPolicy) GetWrappedNamespace() gatewayapiv1beta1.Namespace {
return gatewayapiv1beta1.Namespace(ap.Namespace)
func (ap *AuthPolicy) GetWrappedNamespace() gatewayapiv1.Namespace {
return gatewayapiv1.Namespace(ap.Namespace)

Check warning on line 278 in api/v1beta2/authpolicy_types.go

View check run for this annotation

Codecov / codecov/patch

api/v1beta2/authpolicy_types.go#L277-L278

Added lines #L277 - L278 were not covered by tests
}

// GetRulesHostnames returns all hostnames referenced in the route selectors of the policy.
Expand Down
36 changes: 18 additions & 18 deletions api/v1beta2/authpolicy_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"

authorinoapi "github.com/kuadrant/authorino/api/v1beta2"
"github.com/kuadrant/kuadrant-operator/pkg/common"
Expand Down Expand Up @@ -68,7 +68,7 @@ func TestAuthPolicyTargetKey(t *testing.T) {
}

// targetRef with namespace
policy.Spec.TargetRef.Namespace = ptr.To(gatewayapiv1beta1.Namespace("route-namespace"))
policy.Spec.TargetRef.Namespace = ptr.To(gatewayapiv1.Namespace("route-namespace"))
expected = "route-namespace/my-route"
if result := policy.TargetKey().String(); result != expected {
t.Errorf("Expected target key %s, got %s", expected, result)
Expand Down Expand Up @@ -113,7 +113,7 @@ func TestAuthPolicyGetRulesHostnames(t *testing.T) {
}
policy.Spec.RouteSelectors = []RouteSelector{
{
Hostnames: []gatewayapiv1beta1.Hostname{"*.kuadrant.io", "toystore.kuadrant.io"},
Hostnames: []gatewayapiv1.Hostname{"*.kuadrant.io", "toystore.kuadrant.io"},
},
}
// 1 top-level route selectors with 2 hostnames
Expand Down Expand Up @@ -190,7 +190,7 @@ func TestAuthPolicyGetRulesHostnames(t *testing.T) {
CommonAuthRuleSpec: CommonAuthRuleSpec{
RouteSelectors: []RouteSelector{
{
Hostnames: []gatewayapiv1beta1.Hostname{"*.kuadrant.io", "toystore.kuadrant.io"},
Hostnames: []gatewayapiv1.Hostname{"*.kuadrant.io", "toystore.kuadrant.io"},
},
},
},
Expand All @@ -202,7 +202,7 @@ func TestAuthPolicyGetRulesHostnames(t *testing.T) {
CommonAuthRuleSpec: CommonAuthRuleSpec{
RouteSelectors: []RouteSelector{
{
Hostnames: []gatewayapiv1beta1.Hostname{"*.kuadrant.io"},
Hostnames: []gatewayapiv1.Hostname{"*.kuadrant.io"},
},
},
},
Expand Down Expand Up @@ -330,10 +330,10 @@ func TestAuthPolicyValidate(t *testing.T) {
},
RouteSelectors: []RouteSelector{
{
Hostnames: []gatewayapiv1beta1.Hostname{"*.foo.io"},
Matches: []gatewayapiv1beta1.HTTPRouteMatch{
Hostnames: []gatewayapiv1.Hostname{"*.foo.io"},
Matches: []gatewayapiv1.HTTPRouteMatch{
{
Path: &gatewayapiv1beta1.HTTPPathMatch{
Path: &gatewayapiv1.HTTPPathMatch{
Value: ptr.To("/foo"),
},
},
Expand Down Expand Up @@ -368,10 +368,10 @@ func TestAuthPolicyValidate(t *testing.T) {
CommonAuthRuleSpec: CommonAuthRuleSpec{
RouteSelectors: []RouteSelector{
{
Hostnames: []gatewayapiv1beta1.Hostname{"*.foo.io"},
Matches: []gatewayapiv1beta1.HTTPRouteMatch{
Hostnames: []gatewayapiv1.Hostname{"*.foo.io"},
Matches: []gatewayapiv1.HTTPRouteMatch{
{
Path: &gatewayapiv1beta1.HTTPPathMatch{
Path: &gatewayapiv1.HTTPPathMatch{
Value: ptr.To("/foo"),
},
},
Expand All @@ -398,7 +398,7 @@ func TestAuthPolicyValidate(t *testing.T) {
Group: "gateway.networking.k8s.io",
Kind: "HTTPRoute",
Name: "my-route",
Namespace: ptr.To(gatewayapiv1beta1.Namespace("other-namespace")),
Namespace: ptr.To(gatewayapiv1.Namespace("other-namespace")),
},
AuthScheme: AuthSchemeSpec{
Authentication: map[string]AuthenticationSpec{
Expand All @@ -411,10 +411,10 @@ func TestAuthPolicyValidate(t *testing.T) {
CommonAuthRuleSpec: CommonAuthRuleSpec{
RouteSelectors: []RouteSelector{
{
Hostnames: []gatewayapiv1beta1.Hostname{"*.foo.io"},
Matches: []gatewayapiv1beta1.HTTPRouteMatch{
Hostnames: []gatewayapiv1.Hostname{"*.foo.io"},
Matches: []gatewayapiv1.HTTPRouteMatch{
{
Path: &gatewayapiv1beta1.HTTPPathMatch{
Path: &gatewayapiv1.HTTPPathMatch{
Value: ptr.To("/foo"),
},
},
Expand Down Expand Up @@ -445,10 +445,10 @@ func TestAuthPolicyValidate(t *testing.T) {

func testBuildRouteSelector() RouteSelector {
return RouteSelector{
Hostnames: []gatewayapiv1beta1.Hostname{"toystore.kuadrant.io"},
Matches: []gatewayapiv1beta1.HTTPRouteMatch{
Hostnames: []gatewayapiv1.Hostname{"toystore.kuadrant.io"},
Matches: []gatewayapiv1.HTTPRouteMatch{
{
Path: &gatewayapiv1beta1.HTTPPathMatch{
Path: &gatewayapiv1.HTTPPathMatch{
Value: ptr.To("/toy"),
},
},
Expand Down
12 changes: 7 additions & 5 deletions api/v1beta2/ratelimitpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
"github.com/kuadrant/kuadrant-operator/pkg/common"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
)

// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!
Expand Down Expand Up @@ -70,7 +70,7 @@
}

// RouteSelector defines semantics for matching an HTTP request based on conditions
// https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1beta1.HTTPRouteSpec
// https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.HTTPRouteSpec
type WhenCondition struct {
// Selector defines one item from the well known selectors
// TODO Document properly "Well-known selector" https://github.com/Kuadrant/architecture/blob/main/rfcs/0001-rlp-v2.md#well-known-selectors
Expand All @@ -88,6 +88,7 @@
type Limit struct {
// RouteSelectors defines semantics for matching an HTTP request based on conditions
// +optional
// +kubebuilder:validation:MaxItems=15
RouteSelectors []RouteSelector `json:"routeSelectors,omitempty"`

// When holds the list of conditions for the policy to be enforced.
Expand Down Expand Up @@ -119,6 +120,7 @@

// Limits holds the struct of limits indexed by a unique name
// +optional
// +kubebuilder:validation:MaxProperties=14
Limits map[string]Limit `json:"limits,omitempty"`
}

Expand Down Expand Up @@ -226,15 +228,15 @@
return r.Spec.TargetRef
}

func (r *RateLimitPolicy) GetWrappedNamespace() gatewayapiv1beta1.Namespace {
return gatewayapiv1beta1.Namespace(r.Namespace)
func (r *RateLimitPolicy) GetWrappedNamespace() gatewayapiv1.Namespace {
return gatewayapiv1.Namespace(r.Namespace)

Check warning on line 232 in api/v1beta2/ratelimitpolicy_types.go

View check run for this annotation

Codecov / codecov/patch

api/v1beta2/ratelimitpolicy_types.go#L231-L232

Added lines #L231 - L232 were not covered by tests
}

func (r *RateLimitPolicy) GetRulesHostnames() (ruleHosts []string) {
ruleHosts = make([]string, 0)
for _, limit := range r.Spec.Limits {
for _, routeSelector := range limit.RouteSelectors {
convertHostnamesToString := func(gwHostnames []gatewayapiv1beta1.Hostname) []string {
convertHostnamesToString := func(gwHostnames []gatewayapiv1.Hostname) []string {

Check warning on line 239 in api/v1beta2/ratelimitpolicy_types.go

View check run for this annotation

Codecov / codecov/patch

api/v1beta2/ratelimitpolicy_types.go#L239

Added line #L239 was not covered by tests
hostnames := make([]string, 0, len(gwHostnames))
for _, gwHostName := range gwHostnames {
hostnames = append(hostnames, string(gwHostName))
Expand Down
14 changes: 7 additions & 7 deletions api/v1beta2/ratelimitpolicy_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ import (
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"

"github.com/kuadrant/kuadrant-operator/pkg/common"
)

func testBuildBasicRLP(name string, kind gatewayapiv1beta1.Kind) *RateLimitPolicy {
func testBuildBasicRLP(name string, kind gatewayapiv1.Kind) *RateLimitPolicy {
return &RateLimitPolicy{
TypeMeta: metav1.TypeMeta{
Kind: "RateLimitPolicy",
Expand All @@ -34,11 +34,11 @@ func testBuildBasicRLP(name string, kind gatewayapiv1beta1.Kind) *RateLimitPolic
}

func testBuildBasicGatewayRLP(name string) *RateLimitPolicy {
return testBuildBasicRLP(name, gatewayapiv1beta1.Kind("Gateway"))
return testBuildBasicRLP(name, gatewayapiv1.Kind("Gateway"))
}

func testBuildBasicHTTPRouteRLP(name string) *RateLimitPolicy {
return testBuildBasicRLP(name, gatewayapiv1beta1.Kind("HTTPRoute"))
return testBuildBasicRLP(name, gatewayapiv1.Kind("HTTPRoute"))
}

// TestRateLimitPolicyValidation calls rlp.Validate()
Expand All @@ -62,7 +62,7 @@ func TestRateLimitPolicyValidation(t *testing.T) {

// invalid group
rlp = testBuildBasicHTTPRouteRLP(name)
rlp.Spec.TargetRef.Group = gatewayapiv1beta1.Group("foo.example.com")
rlp.Spec.TargetRef.Group = gatewayapiv1.Group("foo.example.com")
err = rlp.Validate()
if err == nil {
t.Fatal(`rlp.Validate() did not return error and should`)
Expand All @@ -73,7 +73,7 @@ func TestRateLimitPolicyValidation(t *testing.T) {

// invalid kind
rlp = testBuildBasicHTTPRouteRLP(name)
rlp.Spec.TargetRef.Kind = gatewayapiv1beta1.Kind("Foo")
rlp.Spec.TargetRef.Kind = gatewayapiv1.Kind("Foo")
err = rlp.Validate()
if err == nil {
t.Fatal(`rlp.Validate() did not return error and should`)
Expand All @@ -84,7 +84,7 @@ func TestRateLimitPolicyValidation(t *testing.T) {

// Different namespace
rlp = testBuildBasicHTTPRouteRLP(name)
otherNS := gatewayapiv1beta1.Namespace(rlp.GetNamespace() + "other")
otherNS := gatewayapiv1.Namespace(rlp.GetNamespace() + "other")
rlp.Spec.TargetRef.Namespace = &otherNS
err = rlp.Validate()
if err == nil {
Expand Down
Loading
Loading