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

Adding/updating RLP use cases to docs #964

Merged
merged 1 commit into from
Dec 2, 2024

Conversation

R-Lawton
Copy link
Contributor

@R-Lawton R-Lawton commented Oct 29, 2024

WHAT

  • Added or modified docs to add more scenarios for RLP
  • Updated current docs to be more cohesive.
  • Linter updated spacing etc

TODO:

  • RLP spec.targetRef.sectionName api doc
  • RateLimitPolicies targeting a Gateway
  • RateLimitPolicies targeting a specific Listener of a Gateway (with spec.targetRef.sectionName)
  • RateLimitPolicies targeting a HTTPRoute
  • RateLimitPolicies targeting a specific HTTPRouteRule of a HTTPRoute (with spec.targetRef.sectionName)
  • Multiple RateLimitPolicies targeting a same resource - different sections of the resource
  • Multiple RateLimitPolicies targeting a same resource - same section of the resource
  • Defaults and Overrides in RateLimitPolicies targeting Gateways
  • Defaults and Overrides in RateLimitPolicies targeting HTTPRoutes
  • Defaults and Overrides' policy merge strategy in RateLimitPolicies (spec.defaults.strategy: merge, spec.overrides.strategy: merge)

@R-Lawton R-Lawton changed the title Adding RLP use cases Adding RLP use cases to docs Oct 29, 2024
@R-Lawton R-Lawton assigned R-Lawton and unassigned R-Lawton Oct 30, 2024
@eguzki
Copy link
Contributor

eguzki commented Nov 6, 2024

Heads up! RLP API is changing #976

@R-Lawton R-Lawton force-pushed the doc-updates branch 3 times, most recently from 8dc13be to c9d98b4 Compare November 22, 2024 17:55
@R-Lawton R-Lawton changed the title Adding RLP use cases to docs Adding/updating RLP use cases to docs Nov 22, 2024
@@ -10,7 +10,7 @@
- Accessible Redis instance, for persistent storage for your rate limit counters. (Optional)


> Note: By default the following guide will install the "latest" or "main" version of Kuadrant. To pick a specific version, change the image in the `config/deploy/install/standard/kustomization.yaml`. All versions available can be found on the Kuadrant operator [release page](https://github.com/Kuadrant/kuadrant-operator/releases)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this directory doesnt exist

@@ -3,4 +3,4 @@ kind: CatalogSource
metadata:
name: kuadrant-operator-catalog
spec:
image: quay.io/kuadrant/kuadrant-operator-catalog:v1.0.0-rc6 #change this to the version you want to install
Copy link
Contributor Author

Choose a reason for hiding this comment

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

default should be latest

@@ -2,7 +2,7 @@

A Kuadrant TLSPolicy custom resource:

1. Targets Gateway API networking resources [Gateways](https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.Gateway) to provide tls for gateway listeners by managing the lifecycle of tls certificates using [`CertManager`](https://cert-manager.io).
Targets Gateway API networking resources [Gateways](https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.Gateway) to provide tls for gateway listeners by managing the lifecycle of tls certificates using [`CertManager`](https://cert-manager.io).

## How it works

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you expand this file theres a lot of to do's. Do we plan on updating it for GA?

```

### ② Deploy the Toy Store sample application (Persona: _App developer_)
### Deploy the Toy Store sample application (Persona: _App developer_)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing the unicode numbering to match the rest of the docs and to make writing docs easier

@@ -2,46 +2,32 @@

This user guide walks you through an example of how to configure DNS for all routes attached to an ingress gateway.

<br/>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no needed for docs website

@@ -24,33 +24,37 @@ On cluster 2 the DNS Operator managing the existing DNSRecord in that cluster ha
In prometheus alerts, it spots that the number of owners does not correlate to the number of DNSRecord resources and triggers an alert.
To remedy this rather than going to the DNS provider directly and trying to figure out which records to remove, you can instead follow the steps below.

1) Get the owner id of the DNSRecord on cluster 2 for the shared host
Get the owner id of the DNSRecord on cluster 2 for the shared host
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We dont use numbering in a lot of our docs so removing to match

@@ -1,4 +1,4 @@
# Authenticated Rate Limiting for Application Developers
# Authenticated Rate Limiting for Applications
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think having the persona might put people off but happy to change

Copy link
Contributor

Choose a reason for hiding this comment

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

It is referring to the persona defined by Gateway API https://gateway-api.sigs.k8s.io/

@@ -1,43 +1,19 @@
# Gateway Rate Limiting for Cluster Operators
# Rate limiting for ingress gateway
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was just confusing i didnt understand what cluster operators mean. So it would be confusing for users to

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.

Maybe a reference to Gateway API docs could help on the confusion

@R-Lawton R-Lawton marked this pull request as ready for review November 22, 2024 18:43
Copy link
Member

@jasonmadigan jasonmadigan left a comment

Choose a reason for hiding this comment

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

couple of small suggestions

@@ -1,7 +1,6 @@
# Install Kuadrant on a Kubernetes cluster

!!! note

Copy link
Member

Choose a reason for hiding this comment

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

does this render ok with mkdocs (checkout a branch locally, update mkdocs.yaml to point to this branch for the kuadrant-operator component) ? could be wrong by I vaguely recall that maybe these newlines were needed - worth a quick check

Copy link
Contributor Author

@R-Lawton R-Lawton Nov 26, 2024

Choose a reason for hiding this comment

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

no i havent checked good shout ill double check

doc/install/install-make.md Show resolved Hide resolved
doc/install/install-make.md Show resolved Hide resolved
doc/overviews/tls.md Show resolved Hide resolved
doc/reference/ratelimitpolicy.md Show resolved Hide resolved
doc/reference/ratelimitpolicy.md Show resolved Hide resolved
doc/user-guides/dns/orphan-dns-records.md Show resolved Hide resolved
doc/user-guides/dns/orphan-dns-records.md Show resolved Hide resolved
doc/user-guides/dns/orphan-dns-records.md Show resolved Hide resolved
doc/user-guides/ratelimiting/authenticated-rl-for-app.md Outdated Show resolved Hide resolved
@jasonmadigan
Copy link
Member

Is there a docs.kuadrant.io PR to go with this to shift some of the nav items (or add the newer guides)?

@R-Lawton
Copy link
Contributor Author

Is there a docs.kuadrant.io PR to go with this to shift some of the nav items (or add the newer guides)?

Yeah i have a pr on hold to match these changes

@R-Lawton R-Lawton force-pushed the doc-updates branch 2 times, most recently from d5a54fa to 4984c20 Compare November 29, 2024 11:38
Copy link
Contributor

@eguzki eguzki left a comment

Choose a reason for hiding this comment

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

Overall looks good. Great job 🎖️

I left some comments

@@ -35,7 +35,7 @@ to operate the cluster ingress gateway to provide API management with **authenti
### Kuadrant components

| CRD | Description |
|----------------------------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| -------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
Copy link
Contributor

Choose a reason for hiding this comment

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

I did not know you could do this. Certainly is working (I can see in https://github.com/R-Lawton/kuadrant-operator/tree/doc-updates)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I cant take credit was my linter 😁

@@ -10,7 +10,7 @@
- Accessible Redis instance, for persistent storage for your rate limit counters. (Optional)


> Note: By default the following guide will install the "latest" or "main" version of Kuadrant. To pick a specific version, change the image in the `config/deploy/install/standard/kustomization.yaml`. All versions available can be found on the Kuadrant operator [release page](https://github.com/Kuadrant/kuadrant-operator/releases)
> Note: By default the following guide will install the "latest" or "main" version of Kuadrant. To pick a specific version, change the image in the `config/install/standard/kustomization.yaml`. All versions available can be found on the Kuadrant operator [release page](https://github.com/Kuadrant/kuadrant-operator/releases)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe referring to config/install/standard/kuadrant-version.yaml ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops yes this is old

doc/install/install-make.md Outdated Show resolved Hide resolved
doc/overviews/rate-limiting.md Show resolved Hide resolved
doc/overviews/rate-limiting.md Show resolved Hide resolved
@@ -1,4 +1,4 @@
# Authenticated Rate Limiting for Application Developers
# Authenticated Rate Limiting for Applications
Copy link
Contributor

Choose a reason for hiding this comment

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

It is referring to the persona defined by Gateway API https://gateway-api.sigs.k8s.io/

@@ -1,43 +1,19 @@
# Gateway Rate Limiting for Cluster Operators
# Rate limiting for ingress gateway
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -1,43 +1,19 @@
# Gateway Rate Limiting for Cluster Operators
# Rate limiting for ingress gateway
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a reference to Gateway API docs could help on the confusion

@R-Lawton
Copy link
Contributor Author

/hold
Double checking all looks ok on docs website

Copy link
Contributor

@eguzki eguzki left a comment

Choose a reason for hiding this comment

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

This rocks!

@R-Lawton
Copy link
Contributor Author

R-Lawton commented Dec 2, 2024

/unhold

@R-Lawton R-Lawton merged commit 9e8480c into Kuadrant:main Dec 2, 2024
26 checks passed
@jasonmadigan
Copy link
Member

@R-Lawton can you cherry-pick this into release-v1.0.0? Will get picked up by Kuadrant/docs.kuadrant.io#148 then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants