-
Notifications
You must be signed in to change notification settings - Fork 11
Add configurable rate limit for health endpoint #15
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -215,3 +215,9 @@ Without the annotation, it defaults to the controller class name, method name, a | |
| ## License | ||
|
|
||
| MIT License | ||
|
|
||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This section is placed after ## License — configuration docs should come before license. Also, please add a YAML usage example (as suggested in the existing review comment). |
||
| ## Configuration | ||
|
|
||
| | Property | Default | Description | | ||
| |---|---|---| | ||
| | `logdispatch.health.rate-limit-per-minute` | 60 | Maximum requests per minute for the health check endpoint | | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| import org.springframework.web.bind.annotation.GetMapping; | ||
| import org.springframework.web.bind.annotation.RequestMapping; | ||
| import org.springframework.web.bind.annotation.RestController; | ||
| import org.springframework.beans.factory.annotation.Value; | ||
|
|
||
| import java.time.Instant; | ||
| import java.util.HashMap; | ||
|
|
@@ -23,10 +24,12 @@ public class LogDispatchHealthController { | |
|
|
||
| private final Instant startupTime; | ||
|
|
||
| // Rate Limiting (60 requests per minute per IP) | ||
| // Rate Limiting (configurable requests per minute per IP) | ||
| private final ConcurrentHashMap<String, AtomicInteger> requestCounts = new ConcurrentHashMap<>(); | ||
| private long currentWindowStart = System.currentTimeMillis(); | ||
| private static final int MAX_REQUESTS_PER_MINUTE = 60; | ||
| @Value("${logdispatch.health.rate-limit-per-minute:60}") | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No validation against <= 0. A misconfigured 0 would permanently block all callers. Add a @PostConstruct guard or @validated constraint. |
||
| private int maxRequestsPerMinute; | ||
|
|
||
| private static final long WINDOW_SIZE_MS = 60000; | ||
|
|
||
| /** | ||
|
|
@@ -37,20 +40,20 @@ public LogDispatchHealthController() { | |
| } | ||
|
|
||
| /** | ||
| * Returns the health status and uptime of the application. | ||
| * Enforces a rate limit of 60 requests per minute per IP. | ||
| * | ||
| * @param request the HTTP request, used to determine the client IP | ||
| * @return a ResponseEntity containing the status, startup time, and uptime | ||
| */ | ||
| * Returns the health status and uptime of the application. | ||
| * Enforces a configurable rate limit per IP. | ||
| * | ||
| * @param request the HTTP request, used to determine the client IP | ||
| * @return a ResponseEntity containing the status, startup time, and uptime | ||
| */ | ||
| @GetMapping | ||
| public ResponseEntity<Map<String, Object>> healthCheck(HttpServletRequest request) { | ||
| String clientIp = getClientIp(request); | ||
|
|
||
| if (!isAllowed(clientIp)) { | ||
| Map<String, Object> errorResponse = new HashMap<>(); | ||
| errorResponse.put("status", "RATE_LIMITED"); | ||
| errorResponse.put("message", "Too many requests. Limit is 60 requests per minute."); | ||
| errorResponse.put("message", "Too many requests. Limit is " + maxRequestsPerMinute + " requests per minute."); | ||
| return ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS).body(errorResponse); | ||
| } | ||
|
|
||
|
|
@@ -77,7 +80,7 @@ private boolean isAllowed(String ip) { | |
| } | ||
|
|
||
| int count = requestCounts.computeIfAbsent(ip, k -> new AtomicInteger(0)).incrementAndGet(); | ||
| return count <= MAX_REQUESTS_PER_MINUTE; | ||
| return count <= maxRequestsPerMinute; | ||
| } | ||
|
|
||
| private String getClientIp(HttpServletRequest request) { | ||
|
|
||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Library modules should not ship application.yml — this can conflict with the consuming app's config. The default is already covered by the @value fallback. Please remove this file. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| logdispatch: | ||
| health: | ||
| rate-limit-per-minute: 60 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we update the README with an example showing how users can configure
logdispatch.health.rate-limit-per-minuteafter adding the starter dependency? This will make the new feature easier to discover and use.For example, the README could mention the new optional configuration:
It would also be helpful to document the default value (e.g.,
60requests per minute) when this property is not specified.