Skip to content

Commit

Permalink
fix: Rate limiting key should respect load balancers (#37409)
Browse files Browse the repository at this point in the history
This PR changes the `key` used for rate limiting so that it includes any
`Forwarded` or `X-Forwarded-For` headers, so that rate-limiting counter
respects any load balancers that are running on top of Appsmith
container.


/test sanity

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/11855493733>
> Commit: af2d760
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11855493733&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Sanity`
> Spec:
> <hr>Fri, 15 Nov 2024 12:04:04 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Enhanced rate limiting configuration for improved performance with
load balancers.
- Adjusted handling of custom domains for better certificate management.

- **Bug Fixes**
- Improved error handling by removing unnecessary headers to enhance
security and response consistency.

- **Documentation**
- Updated internal logic for generating server configurations, ensuring
clarity in server setup.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
sharat87 authored Nov 20, 2024
1 parent ac4c714 commit f7f9fc3
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,10 @@ ${
${isRateLimitingEnabled ? `rate_limit {
zone dynamic_zone {
key {http.request.client_ip}
# This key is designed to work irrespective of any load balancers running on the Appsmith container.
# We use "+" as the separator here since we don't expect it in any of the placeholder values here, and has no
# significance in header value syntax.
key {header.Forwarded}+{header.X-Forwarded-For}+{remote_host}
events ${RATE_LIMIT}
window 1s
}
Expand Down

0 comments on commit f7f9fc3

Please sign in to comment.