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

Simplify RateLimitPolicy #144

Merged
merged 7 commits into from
Feb 13, 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
5 changes: 4 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,11 @@ test: test-unit test-integration ## Run all tests
test-integration: clean-cov generate fmt vet envtest ## Run Integration tests.
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) $(ARCH_PARAM) use $(ENVTEST_K8S_VERSION) -p path)" USE_EXISTING_CLUSTER=true go test ./... -coverprofile $(PROJECT_PATH)/cover.out -tags integration -ginkgo.v -ginkgo.progress -v -timeout 0

ifdef TEST_NAME
test-unit: TEST_PATTERN := --run $(TEST_NAME)
endif
test-unit: clean-cov generate fmt vet ## Run Unit tests.
go test ./... -coverprofile $(PROJECT_PATH)/cover.out -tags unit -v -timeout 0
go test ./... -coverprofile $(PROJECT_PATH)/cover.out -tags unit -v -timeout 0 $(TEST_PATTERN)
eguzki marked this conversation as resolved.
Show resolved Hide resolved

.PHONY: namespace
namespace: ## Creates a namespace where to deploy Kuadrant Operator
Expand Down
2 changes: 1 addition & 1 deletion api/external/maistra/status/status.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//nolint
// nolint
package status

import (
Expand Down
14 changes: 9 additions & 5 deletions api/v1beta1/ratelimitpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,13 @@ type Configuration struct {

// Limit represents partially a Limitador limit.
type Limit struct {
MaxValue int `json:"maxValue"`
Seconds int `json:"seconds"`
Conditions []string `json:"conditions"`
Variables []string `json:"variables"`
MaxValue int `json:"maxValue"`
Seconds int `json:"seconds"`

// +optional
Conditions []string `json:"conditions,omitempty"`
// +optional
Variables []string `json:"variables,omitempty"`
}

func LimitFromLimitadorRateLimit(limit *limitadorv1alpha1.RateLimit) *Limit {
Expand Down Expand Up @@ -151,7 +154,8 @@ func LimitFromLimitadorRateLimit(limit *limitadorv1alpha1.RateLimit) *Limit {
// RateLimit represents a complete rate limit configuration
type RateLimit struct {
// Configurations holds list of (action) configuration.
Configurations []Configuration `json:"configurations"`
// +optional
Configurations []Configuration `json:"configurations,omitempty"`

// Rules represents the definition of the scope of the rate limit object
// Defines a list of conditions for which rate limit configuration will apply
Expand Down
8 changes: 0 additions & 8 deletions bundle/manifests/kuadrant.io_ratelimitpolicies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,8 @@ spec:
type: string
type: array
required:
- conditions
- maxValue
- seconds
- variables
type: object
type: array
rules:
Expand Down Expand Up @@ -188,8 +186,6 @@ spec:
type: array
type: object
type: array
required:
- configurations
type: object
type: array
targetRef:
Expand Down Expand Up @@ -426,10 +422,8 @@ spec:
type: string
type: array
required:
- conditions
- maxValue
- seconds
- variables
type: object
type: array
rules:
Expand Down Expand Up @@ -458,8 +452,6 @@ spec:
type: array
type: object
type: array
required:
- configurations
type: object
type: array
required:
Expand Down
8 changes: 0 additions & 8 deletions config/crd/bases/kuadrant.io_ratelimitpolicies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,8 @@ spec:
type: string
type: array
required:
- conditions
- maxValue
- seconds
- variables
type: object
type: array
rules:
Expand Down Expand Up @@ -178,8 +176,6 @@ spec:
type: array
type: object
type: array
required:
- configurations
type: object
type: array
targetRef:
Expand Down Expand Up @@ -416,10 +412,8 @@ spec:
type: string
type: array
required:
- conditions
- maxValue
- seconds
- variables
type: object
type: array
rules:
Expand Down Expand Up @@ -448,8 +442,6 @@ spec:
type: array
type: object
type: array
required:
- configurations
type: object
type: array
required:
Expand Down
13 changes: 7 additions & 6 deletions controllers/kuadrant_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -627,12 +627,13 @@ func (r *KuadrantReconciler) reconcileAuthorino(ctx context.Context, kObj *kuadr
}

// Builds the Istio/OSSM MeshConfig from a compatible structure:
// meshConfig:
// extensionProviders:
// - envoyExtAuthzGrpc:
// port: <port>
// service: <authorino-service>
// name: kuadrant-authorization
//
// meshConfig:
// extensionProviders:
// - envoyExtAuthzGrpc:
// port: <port>
// service: <authorino-service>
// name: kuadrant-authorization
func meshConfigFromStruct(structure *structpb.Struct) (*istiomeshv1alpha1.MeshConfig, error) {
if structure == nil {
return &istiomeshv1alpha1.MeshConfig{}, nil
Expand Down
125 changes: 125 additions & 0 deletions controllers/ratelimitpolicy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,4 +288,129 @@ var _ = Describe("RateLimitPolicy controller", func() {
common.RateLimitPoliciesBackRefAnnotation, string(serialized)))
})
})

Context("Basic: Simplest RLP targeting HTTPRoute", func() {
It("check created resources", func() {
// Check Limitador Status is Ready
Eventually(func() bool {
limitador := &limitadorv1alpha1.Limitador{}
err := k8sClient.Get(context.Background(), client.ObjectKey{Name: common.LimitadorName, Namespace: testNamespace}, limitador)
if err != nil {
return false
}
if !meta.IsStatusConditionTrue(limitador.Status.Conditions, "Ready") {
return false
}
return true
}, time.Minute, 5*time.Second).Should(BeTrue())

httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"})
err := k8sClient.Create(context.Background(), httpRoute)
Expect(err).ToNot(HaveOccurred())

rlp := &kuadrantv1beta1.RateLimitPolicy{
TypeMeta: metav1.TypeMeta{
Kind: "RateLimitPolicy",
APIVersion: kuadrantv1beta1.GroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: rlpName,
Namespace: testNamespace,
},
Spec: kuadrantv1beta1.RateLimitPolicySpec{
TargetRef: gatewayapiv1alpha2.PolicyTargetReference{
Group: gatewayapiv1alpha2.Group("gateway.networking.k8s.io"),
Kind: "HTTPRoute",
Name: gatewayapiv1alpha2.ObjectName(routeName),
},
RateLimits: []kuadrantv1beta1.RateLimit{
{
Limits: []kuadrantv1beta1.Limit{
{
MaxValue: 5,
Seconds: 10,
},
},
},
},
},
}

rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace}
err = k8sClient.Create(context.Background(), rlp)
Expect(err).ToNot(HaveOccurred())

// Check RLP status is available
Eventually(func() bool {
existingRLP := &kuadrantv1beta1.RateLimitPolicy{}
err := k8sClient.Get(context.Background(), rlpKey, existingRLP)
if err != nil {
return false
}
if !meta.IsStatusConditionTrue(existingRLP.Status.Conditions, "Available") {
return false
}

return true
}, time.Minute, 5*time.Second).Should(BeTrue())

// check limits
limitadorKey := client.ObjectKey{Name: common.LimitadorName, Namespace: testNamespace}
existingLimitador := &limitadorv1alpha1.Limitador{}
err = k8sClient.Get(context.Background(), limitadorKey, existingLimitador)
// must exist
Expect(err).ToNot(HaveOccurred())
Expect(existingLimitador.Spec.Limits).To(ContainElements(limitadorv1alpha1.RateLimit{
MaxValue: 5,
Seconds: 10,
Namespace: common.MarshallNamespace(client.ObjectKeyFromObject(gateway), "*.example.com"),
Conditions: []string{},
Variables: []string{},
}))

// Check wasm plugin
wpName := fmt.Sprintf("kuadrant-%s", gwName)
wasmPluginKey := client.ObjectKey{Name: wpName, Namespace: testNamespace}
existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{}
err = k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin)
// must exist
Expect(err).ToNot(HaveOccurred())
existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig)
Expect(err).ToNot(HaveOccurred())
Expect(existingWASMConfig).To(Equal(&rlptools.WASMPlugin{
FailureModeDeny: true,
RateLimitPolicies: []rlptools.RateLimitPolicy{
{
Name: "*.example.com",
RateLimitDomain: common.MarshallNamespace(client.ObjectKeyFromObject(gateway), "*.example.com"),
UpstreamCluster: common.KuadrantRateLimitClusterName,
Hostnames: []string{"*.example.com"},
GatewayActions: []rlptools.GatewayAction{
{
Rules: []kuadrantv1beta1.Rule{
{
Paths: []string{"/toy*"},
Methods: []string{"GET"},
Hosts: []string{"*.example.com"},
},
},
Configurations: []kuadrantv1beta1.Configuration{
{
Actions: []kuadrantv1beta1.ActionSpecifier{
{
GenericKey: &kuadrantv1beta1.GenericKeySpec{
DescriptorValue: rlpKey.String(),
DescriptorKey: &[]string{"generic_key"}[0],
},
},
},
},
},
},
},
},
},
}))
})
})
})
12 changes: 12 additions & 0 deletions doc/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,18 @@ make local-cleanup
make test-unit
```

Optionally, add `TEST_NAME` makefile variable to run specific test

```sh
make test-unit TEST_NAME=TestLimitIndexEquals
```

or even subtest

```sh
make test-unit TEST_NAME=TestLimitIndexEquals/empty_indexes_are_equal
```

### Integration tests

You need an active session open to a kubernetes cluster.
Expand Down
16 changes: 16 additions & 0 deletions doc/rate-limiting.md
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,22 @@ one entry each descriptor:
**Note**: The external rate limiting service will be called only when there is at least one not empty
descriptor.

### Policy default action configurations

When a rate limit policy does not specify any action configuration, the Kuadrant control plane
will assign a generic default action configuration for the traffic related to the targeted network
resource. This default action configuration allows defining global limits for all the traffic
related the targeted network resource. For instance, the following rate limit policy is valid:

```yaml
spec:
targetRef: { ... }
rateLimits:
- limits:
- maxValue: 5
seconds: 10
```

### Rate limiting configuration rules

Configuration rules allow rate limit configurations to be activated conditionally depending on
Expand Down
6 changes: 3 additions & 3 deletions doc/ratelimitpolicy-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ Generated using [github-markdown-toc](https://github.com/ekalinin/github-markdow

| **json/yaml field**| **Type** | **Required** | **Default value** | **Description** |
| --- | --- | --- | --- | --- |
| `configurations` | [][Configuration](#Configuration) | Yes | N/A | list of action configurations |
| `configurations` | [][Configuration](#Configuration) | No | Empty | list of action configurations |
| `rules` | [][Rule](#Rule) | No | Empty. All configurations apply | list of action configurations rules. Rate limit configuration will apply when at least one rule matches the request |
| `limits` | [][Limit](#Limit) | No | Empty | list of Limitador limit objects |

Expand Down Expand Up @@ -67,8 +67,8 @@ Generated using [github-markdown-toc](https://github.com/ekalinin/github-markdow
| --- | --- | --- | --- | --- |
| `maxValue` | int | Yes | N/A | max number of request for the specified time period |
| `seconds` | int | Yes | N/A | time period in seconds |
| `conditions` | []string | Yes | N/A | Limit conditions. Check [Limitador](https://github.com/Kuadrant/limitador) for more information |
| `variables` | []string | Yes | N/A | Limit variables. Check [Limitador](https://github.com/Kuadrant/limitador) for more information |
| `conditions` | []string | No | Empty list | Limit conditions. Check [Limitador](https://github.com/Kuadrant/limitador) for more information |
| `variables` | []string | No | Empty list | Limit variables. Check [Limitador](https://github.com/Kuadrant/limitador) for more information |

## RateLimitPolicyStatus

Expand Down
20 changes: 20 additions & 0 deletions pkg/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@ func GetDefaultIfNil[T any](val *T, def T) T {
return *val
}

func GetEmptySliceIfNil[T any](val []T) []T {
if val == nil {
return make([]T, 0)
}
return val
}

// NamespacedNameToObjectKey converts <namespace/name> format string to k8s object key.
// It's common for K8s to reference an object using this format. For e.g. gateways in VirtualService.
func NamespacedNameToObjectKey(namespacedName, defaultNamespace string) client.ObjectKey {
Expand Down Expand Up @@ -105,6 +112,19 @@ func SliceCopy[T any](s1 []T) []T {
return s2
}

func ReverseSlice[T any](input []T) []T {
guicassolato marked this conversation as resolved.
Show resolved Hide resolved
inputLen := len(input)
output := make([]T, inputLen)

for i, n := range input {
j := inputLen - i - 1

output[j] = n
}

return output
}

// MergeMapStringString Merge desired into existing.
// Not Thread-Safe. Does it matter?
func MergeMapStringString(existing *map[string]string, desired map[string]string) bool {
Expand Down
8 changes: 6 additions & 2 deletions pkg/rlptools/limit_index.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,16 @@ func (l *LimitIndex) ToLimits() []limitadorv1alpha1.RateLimit {
for gwKey, limitsByDomain := range l.gatewayLimits {
for domain, limitList := range limitsByDomain {
for idx := range limitList {
// Currently, Limitador CRD v0.3.1,
// the fields "Variables" and "Conditions" are required
variables := common.GetEmptySliceIfNil(limitList[idx].Variables)
conditions := common.GetEmptySliceIfNil(limitList[idx].Conditions)
limits = append(limits, limitadorv1alpha1.RateLimit{
Namespace: common.MarshallNamespace(gwKey, domain),
MaxValue: limitList[idx].MaxValue,
Seconds: limitList[idx].Seconds,
Variables: limitList[idx].Variables,
Conditions: limitList[idx].Conditions,
Variables: variables,
Conditions: conditions,
eguzki marked this conversation as resolved.
Show resolved Hide resolved
})
}
}
Expand Down
Loading