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 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: 4 additions & 4 deletions .github/workflows/build-images-base.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,10 @@ jobs:
name: Build Bundle
runs-on: ubuntu-latest
steps:
- name: Set up Go 1.20.x
- name: Set up Go 1.21.x
uses: actions/setup-go@v4
with:
go-version: 1.20.x
go-version: 1.21.x
id: go
- name: Check out code
uses: actions/checkout@v3
Expand Down Expand Up @@ -156,10 +156,10 @@ jobs:
needs: [build, build-bundle]
runs-on: ubuntu-latest
steps:
- name: Set up Go 1.20.x
- name: Set up Go 1.21.x
uses: actions/setup-go@v4
with:
go-version: 1.20.x
go-version: 1.21.x
id: go
- name: Check out code
uses: actions/checkout@v3
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/code-style.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ jobs:
importpath: golang.org/x/tools/cmd/goimports@latest

steps:
- name: Set up Go 1.20.x
- name: Set up Go 1.21.x
uses: actions/setup-go@v4
with:
go-version: 1.20.x
go-version: 1.21.x
id: go

- name: Check out code
Expand Down Expand Up @@ -90,10 +90,10 @@ jobs:
runs-on: ubuntu-latest

steps:
- name: Set up Go 1.20.x
- name: Set up Go 1.21.x
uses: actions/setup-go@v4
with:
go-version: 1.20.x
go-version: 1.21.x
id: go

- name: Check out code
Expand Down
20 changes: 10 additions & 10 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
name: Unit Tests
strategy:
matrix:
go-version: [ 1.20.x ]
go-version: [ 1.21.x ]
platform: [ ubuntu-latest ]
runs-on: ${{ matrix.platform }}
defaults:
Expand Down Expand Up @@ -53,10 +53,10 @@ jobs:
run:
shell: bash
steps:
- name: Set up Go 1.20.x
- name: Set up Go 1.21.x
uses: actions/setup-go@v4
with:
go-version: 1.20.x
go-version: 1.21.x
id: go
- name: Check out code
uses: actions/checkout@v3
Expand Down Expand Up @@ -94,10 +94,10 @@ jobs:
name: Verify manifests
runs-on: ubuntu-latest
steps:
- name: Set up Go 1.20.x
- name: Set up Go 1.21.x
uses: actions/setup-go@v4
with:
go-version: 1.20.x
go-version: 1.21.x
id: go
- name: Check out code
uses: actions/checkout@v3
Expand All @@ -109,10 +109,10 @@ jobs:
name: Verify bundle
runs-on: ubuntu-latest
steps:
- name: Set up Go 1.20.x
- name: Set up Go 1.21.x
uses: actions/setup-go@v4
with:
go-version: 1.20.x
go-version: 1.21.x
id: go
- name: Check out code
uses: actions/checkout@v3
Expand All @@ -124,10 +124,10 @@ jobs:
name: Verify fmt
runs-on: ubuntu-latest
steps:
- name: Set up Go 1.20.x
- name: Set up Go 1.21.x
uses: actions/setup-go@v4
with:
go-version: 1.20.x
go-version: 1.21.x
id: go
- name: Check out code
uses: actions/checkout@v3
Expand All @@ -139,7 +139,7 @@ jobs:
name: Test Scripts
strategy:
matrix:
go-version: [ 1.20.x ]
go-version: [ 1.21.x ]
platform: [ ubuntu-latest, macos-latest ]
runs-on: ${{ matrix.platform }}
defaults:
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Build the manager binary
FROM golang:1.20 as builder
FROM golang:1.21 as builder

WORKDIR /workspace
# Copy the Go Modules manifests
Expand Down
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
Loading
Loading