Remember unknown SDLC projects briefly; rate limit API requests per client

README.md

@@ -79,10 +79,9 @@ The application uses Redis (`spring.data.redis.host`/`port`, default `localhost:
 | Cache | What | TTL |
 |---|---|---|
 | `sdlc-systems` | SDLC systems by id, read on every create, update and SDLC call | `resourceapi.cache.sdlc-systems-ttl` (10m) |
-| `external-projects` | `GET /api/v2/projects/{id}/external` answers per SDLC system and external id; "unknown" answers are not cached | `resourceapi.cache.external-projects-ttl` (5m) |
+| `external-projects` | `GET /api/v2/projects/{id}/external` answers per SDLC system and external id | `resourceapi.cache.external-projects-ttl` (5m); "the SDLC system doesn't know this project" (stored as a cached null) only `resourceapi.cache.missing-external-projects-ttl` (30s), so a project created there is found soon |
 
-A background sync always asks the SDLC system and refreshes the cached answer (or removes it when the SDLC
-system no longer knows the project). SDLC systems are not changed
+A background sync always asks the SDLC system and replaces the cached answer with its own. SDLC systems are not changed
 through this API; one changed directly in the database is picked up when its entry expires. The cache is an
 optimization: if Redis is unreachable, cache reads and writes are logged and skipped (each waits at most the
 500 ms Redis timeouts) and requests are served without it.
@@ -104,10 +103,20 @@ available again one period later, which is also what `Retry-After` says):
 |---|---|---|
 | Calls to one SDLC system (`resourceapi.rate-limit.sdlc.*`), protecting its API quota; cache hits don't count | 10 per 1s, waiting up to 1s for a permit | `503` + `Retry-After` (our own limit, not the SDLC system's failure); a background sync logs it |
 | `POST /api/v2/projects/{id}/sync` per project (`resourceapi.rate-limit.sync.*`); requests refused anyway (unknown project, sync already running) don't count, one refused because no background slot is free (`503`) does | 6 per minute | `429` + `Retry-After` |
+| All `/api/**` requests per client IP (`resourceapi.rate-limit.client.*`); behind a reverse proxy the IP comes from `X-Forwarded-For`, trusted only from internal proxy addresses (`server.forward-headers-strategy=native`, see below) | 100 per second | `429` + `Retry-After` |
 
 A changed rate is applied to the existing limiter when an instance first uses it; unused limiters expire from
-Redis after an hour. Invalid limits (e.g. `permits=0`) stop the application at startup. If Redis can't be reached, requests are allowed (limiting protects, losing it shouldn't
-stop the application).
+Redis after 10 periods (at least a minute), so per-IP limiters don't pile up. Invalid limits (e.g.
+`permits=0`) stop the application at startup. If Redis can't be reached, requests are allowed (limiting
+protects, losing it shouldn't stop the application), and Redis is not asked again for 10 seconds, so an
+outage doesn't add a timeout to every request.
+
+The per-IP limit only means something if the application sees the client's address. Tomcat takes it from
+`X-Forwarded-For` when the request comes from an address matching `server.tomcat.remoteip.internal-proxies`
+(by default 10/8, 192.168/16, 172.16/12, 169.254/16, 127/8 and IPv6 local addresses). If the reverse proxy or
+load balancer has another address, set that property to match it; otherwise every request appears to come from
+the proxy and all clients share one limit. Never match addresses clients can connect from directly, or they
+can choose their own key by sending `X-Forwarded-For`.
 
 **Local development**: `./mvnw spring-boot:run` starts Redis from `compose.yaml` automatically
 (`spring-boot-docker-compose`); with `java -jar`, start it yourself with `docker compose up -d` (it listens on

src/main/java/com/company/resourceapi/config/CacheConfig.java

@@ -10,6 +10,7 @@
 import org.springframework.cache.annotation.EnableCaching;
 import org.springframework.cache.interceptor.CacheErrorHandler;
 import org.springframework.cache.interceptor.LoggingCacheErrorHandler;
+import org.springframework.cache.support.NullValue;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Role;
@@ -26,31 +27,39 @@
 @EnableCaching
 @EnableConfigurationProperties(CacheTtlProperties.class)
 // cached values are rebuilt from JSON by type name: reflection a native image must know about
-@RegisterReflectionForBinding({SdlcSystem.class, ExternalProject.class})
+@RegisterReflectionForBinding({SdlcSystem.class, ExternalProject.class, NullValue.class})
 public class CacheConfig {
 
     public static final String SDLC_SYSTEMS = "sdlc-systems";
     public static final String EXTERNAL_PROJECTS = "external-projects";
 
     /**
      * Starts from Boot's defaults (spring.cache.redis.*: key prefix, null values, ...) and only
-     * replaces the value serializer and sets each cache's time to live.
+     * replaces the value serializer and sets each cache's time to live. External projects rely on
+     * null values being cached (spring.cache.redis.cache-null-values, true by default).
      */
     @Bean
     RedisCacheManagerBuilderCustomizer redisCacheCustomizer(CacheTtlProperties ttl) {
         var typeValidator = BasicPolymorphicTypeValidator.builder()
                 .allowIfSubType("com.company.resourceapi.")
                 .allowIfSubType("java.lang.")
                 .allowIfSubType("java.time.")
+                // Spring's marker for a cached null ("the SDLC system doesn't know this project")
+                .allowIfSubType(NullValue.class)
                 .build();
         var serializer = GenericJacksonJsonRedisSerializer.builder()
                 .enableDefaultTyping(typeValidator)
+                .enableSpringCacheNullValueSupport()
                 .build();
         return builder -> {
             var defaults = builder.cacheDefaults().serializeValuesWith(SerializationPair.fromSerializer(serializer));
             builder.cacheDefaults(defaults)
                     .withCacheConfiguration(SDLC_SYSTEMS, defaults.entryTtl(ttl.sdlcSystemsTtl()))
-                    .withCacheConfiguration(EXTERNAL_PROJECTS, defaults.entryTtl(ttl.externalProjectsTtl()));
+                    // a found project is kept longer than "unknown", so a project created there is found soon
+                    .withCacheConfiguration(EXTERNAL_PROJECTS, defaults.entryTtl((key, value) ->
+                            value == null || value instanceof NullValue
+                                    ? ttl.missingExternalProjectsTtl()
+                                    : ttl.externalProjectsTtl()));
         };
     }
 

src/main/java/com/company/resourceapi/config/CacheTtlProperties.java

@@ -9,9 +9,12 @@
  *
  * @param sdlcSystemsTtl      SDLC systems by id; they change rarely and only outside this API
  * @param externalProjectsTtl projects as described by their SDLC system ({@code GET .../external})
+ * @param missingExternalProjectsTtl "the SDLC system doesn't know this project" answers; short, so a
+ *                                   project created there is found soon
  */
 @ConfigurationProperties("resourceapi.cache")
 public record CacheTtlProperties(
         @DefaultValue("10m") Duration sdlcSystemsTtl,
-        @DefaultValue("5m") Duration externalProjectsTtl) {
+        @DefaultValue("5m") Duration externalProjectsTtl,
+        @DefaultValue("30s") Duration missingExternalProjectsTtl) {
 }

src/main/java/com/company/resourceapi/config/RateLimitProperties.java

@@ -13,15 +13,16 @@
  * Distributed rate limits ({@code resourceapi.rate-limit.*}), shared by all instances through Redis.
  * Validated at startup: a broken limit would otherwise fail, or silently stop limiting, at runtime.
  *
- * @param sdlc calls to one SDLC system (protects its API quota)
- * @param sync sync requests for one project
+ * @param sdlc   calls to one SDLC system (protects its API quota)
+ * @param sync   sync requests for one project
+ * @param client API requests from one client (IP address)
  */
 @Validated
 @ConfigurationProperties("resourceapi.rate-limit")
 // validation reads the record fields reflectively, which a native image must know about
 @RegisterReflection(classes = {RateLimitProperties.class, RateLimitProperties.Limit.class},
         memberCategories = {MemberCategory.ACCESS_DECLARED_FIELDS, MemberCategory.INVOKE_DECLARED_METHODS})
-public record RateLimitProperties(@NotNull @Valid Limit sdlc, @NotNull @Valid Limit sync) {
+public record RateLimitProperties(@NotNull @Valid Limit sdlc, @NotNull @Valid Limit sync, @NotNull @Valid Limit client) {
 
     /**
      * At most {@code permits} per sliding window of {@code period}: each permit becomes available again

src/main/java/com/company/resourceapi/services/ExternalProjectService.java

@@ -10,10 +10,8 @@
 import com.company.resourceapi.exceptions.RateLimitExceededException.Scope;
 import java.util.Optional;
 import lombok.RequiredArgsConstructor;
-import org.springframework.cache.annotation.CacheEvict;
 import org.springframework.cache.annotation.CachePut;
 import org.springframework.cache.annotation.Cacheable;
-import org.springframework.cache.annotation.Caching;
 import org.springframework.stereotype.Service;
 
 /**
@@ -28,22 +26,17 @@ public class ExternalProjectService {
     private final RateLimiters rateLimiters;
     private final RateLimitProperties rateLimits;
 
-    /** Cached; a project unknown to the SDLC system isn't cached, so it is found once it exists. */
-    @Cacheable(cacheNames = EXTERNAL_PROJECTS, key = "#sdlcSystem.id + ':' + #externalId", unless = "#result == null")
+    /**
+     * Cached, including the answer "the SDLC system doesn't know this project" (cached as null), which
+     * is kept only briefly so a project created there is found soon (see CacheConfig).
+     */
+    @Cacheable(cacheNames = EXTERNAL_PROJECTS, key = "#sdlcSystem.id + ':' + #externalId")
     public Optional<ExternalProject> findProject(SdlcSystem sdlcSystem, String externalId) {
         return fetch(sdlcSystem, externalId);
     }
 
-    /**
-     * Always asks the SDLC system and updates the cache with the answer: a found project replaces the
-     * cached one, a project the SDLC system no longer knows removes it.
-     */
-    @Caching(
-            put = @CachePut(cacheNames = EXTERNAL_PROJECTS, key = "#sdlcSystem.id + ':' + #externalId",
-                    unless = "#result == null"),
-            // unlike for @CachePut, #result is the Optional itself here (late evictions get the raw return value)
-            evict = @CacheEvict(cacheNames = EXTERNAL_PROJECTS, key = "#sdlcSystem.id + ':' + #externalId",
-                    condition = "#result == null or #result.isEmpty()"))
+    /** Always asks the SDLC system and caches its answer, found or unknown, replacing the cached one. */
+    @CachePut(cacheNames = EXTERNAL_PROJECTS, key = "#sdlcSystem.id + ':' + #externalId")
     public Optional<ExternalProject> refreshProject(SdlcSystem sdlcSystem, String externalId) {
         return fetch(sdlcSystem, externalId);
     }

src/main/java/com/company/resourceapi/services/RateLimiters.java

@@ -18,18 +18,24 @@
  * <p>A limiter is set up with the configured rate the first time this instance uses it (updating a
  * limiter created with another rate), and again if Redis dropped it after it expired from disuse.
  * Without Redis (unreachable, or no Redisson client) requests are allowed: limiting is a protection,
- * losing it shouldn't stop the application.
+ * losing it shouldn't stop the application. After a failure Redis is left alone for a short while,
+ * so an outage doesn't add a Redis timeout to every request.
  */
 @Component
 @Slf4j
 public class RateLimiters {
 
-    /** Unused limiters are removed from Redis after this. */
-    private static final Duration KEEP_ALIVE = Duration.ofHours(1);
+    /** Unused limiters are removed from Redis after this many periods, but not sooner than MIN_KEEP_ALIVE. */
+    private static final int KEEP_ALIVE_PERIODS = 10;
+    private static final Duration MIN_KEEP_ALIVE = Duration.ofMinutes(1);
+    /** How long requests are allowed without asking Redis after it failed. */
+    private static final Duration PAUSE_AFTER_FAILURE = Duration.ofSeconds(10);
     /** Limiters this instance has set up, most recently used kept (bounded: one per project). */
     private static final int MAX_REMEMBERED = 10_000;
 
     private final RedissonClient redisson;
+    /** System.nanoTime() until which Redis isn't asked; 0 while it works. */
+    private volatile long pausedUntil;
     private final Map<String, Boolean> setUp = Collections.synchronizedMap(new LinkedHashMap<>(16, 0.75f, true) {
         @Override
         protected boolean removeEldestEntry(Map.Entry<String, Boolean> eldest) {
@@ -43,7 +49,7 @@ public RateLimiters(ObjectProvider<RedissonClient> redisson) {
 
     /** @return whether a permit was obtained, waiting at most the limit's max wait */
     public boolean tryAcquire(String name, RateLimitProperties.Limit limit) {
-        if (redisson == null) {
+        if (redisson == null || isPaused()) {
             return true;
         }
         try {
@@ -62,17 +68,36 @@ public boolean tryAcquire(String name, RateLimitProperties.Limit limit) {
                 return acquire(limiter, limit);
             }
         } catch (RuntimeException e) {
-            log.warn("Rate limiter {} unavailable, allowing the request: {}", name, e.getMessage());
+            pausedUntil = System.nanoTime() + PAUSE_AFTER_FAILURE.toNanos();
+            log.warn("Rate limiter {} unavailable, allowing requests without rate limits for {}: {}",
+                    name, PAUSE_AFTER_FAILURE, e.getMessage());
             return true;
         }
     }
 
+    private boolean isPaused() {
+        long until = pausedUntil;
+        if (until == 0) {
+            return false;
+        }
+        if (System.nanoTime() - until < 0) {
+            return true;
+        }
+        pausedUntil = 0;
+        return false;
+    }
+
+    private static Duration keepAlive(RateLimitProperties.Limit limit) {
+        var periods = limit.period().multipliedBy(KEEP_ALIVE_PERIODS);
+        return periods.compareTo(MIN_KEEP_ALIVE) > 0 ? periods : MIN_KEEP_ALIVE;
+    }
+
     private static boolean acquire(RRateLimiter limiter, RateLimitProperties.Limit limit) {
         return limiter.tryAcquireAsync(1, limit.maxWait()).toCompletableFuture().join();
     }
 
     private void setUp(String name, RRateLimiter limiter, RateLimitProperties.Limit limit) {
-        var keepAlive = Duration.ofMillis(Math.max(KEEP_ALIVE.toMillis(), limit.period().toMillis()));
+        var keepAlive = keepAlive(limit);
         boolean created = limiter.trySetRateAsync(RateType.OVERALL, limit.permits(), limit.period(), keepAlive)
                 .toCompletableFuture().join();
         if (!created) {

src/main/java/com/company/resourceapi/web/ApiExceptionHandler.java

@@ -39,8 +39,14 @@ ResponseEntity<Void> handleSyncInProgress(SyncInProgressException e) {
      */
     @ExceptionHandler(RateLimitExceededException.class)
     ResponseEntity<Void> handleRateLimitExceeded(RateLimitExceededException e) {
-        log.info(e.getMessage());
-        var status = e.getScope() == RateLimitExceededException.Scope.CALLER ? TOO_MANY_REQUESTS : SERVICE_UNAVAILABLE;
+        var caller = e.getScope() == RateLimitExceededException.Scope.CALLER;
+        // a refused caller is expected, possibly many times a second; holding back our own calls is worth seeing
+        if (caller) {
+            log.debug(e.getMessage());
+        } else {
+            log.info(e.getMessage());
+        }
+        var status = caller ? TOO_MANY_REQUESTS : SERVICE_UNAVAILABLE;
         return ResponseEntity.status(status).header(RETRY_AFTER, String.valueOf(e.getRetryAfterSeconds())).build();
     }
 

src/main/java/com/company/resourceapi/web/ClientRateLimitConfig.java

@@ -0,0 +1,27 @@
+package com.company.resourceapi.web;
+
+import com.company.resourceapi.config.RateLimitProperties;
+import com.company.resourceapi.services.RateLimiters;
+import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.boot.web.servlet.FilterRegistrationBean;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.core.Ordered;
+import org.springframework.web.servlet.HandlerExceptionResolver;
+
+/** Registers {@link ClientRateLimitFilter} for the API only (not Swagger UI or the OpenAPI document). */
+@Configuration
+public class ClientRateLimitConfig {
+
+    @Bean
+    FilterRegistrationBean<ClientRateLimitFilter> clientRateLimitFilter(RateLimiters rateLimiters,
+            RateLimitProperties rateLimits,
+            @Qualifier("handlerExceptionResolver") HandlerExceptionResolver exceptionResolver) {
+        var registration = new FilterRegistrationBean<>(
+                new ClientRateLimitFilter(rateLimiters, rateLimits, exceptionResolver));
+        registration.addUrlPatterns("/api/*");
+        // after the server observation (+1) and trace id (+2) filters
+        registration.setOrder(Ordered.HIGHEST_PRECEDENCE + 3);
+        return registration;
+    }
+}

src/main/java/com/company/resourceapi/web/ClientRateLimitFilter.java

@@ -0,0 +1,44 @@
+package com.company.resourceapi.web;
+
+import com.company.resourceapi.config.RateLimitProperties;
+import com.company.resourceapi.exceptions.RateLimitExceededException;
+import com.company.resourceapi.exceptions.RateLimitExceededException.Scope;
+import com.company.resourceapi.services.RateLimiters;
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import lombok.RequiredArgsConstructor;
+import org.springframework.web.filter.OncePerRequestFilter;
+import org.springframework.web.servlet.HandlerExceptionResolver;
+
+/**
+ * Limits API requests per client IP address, across all instances (429 + Retry-After). Behind a
+ * reverse proxy the address comes from X-Forwarded-For, which Tomcat only trusts from internal proxy
+ * addresses ({@code server.forward-headers-strategy=native}), so clients can't pick their own key.
+ *
+ * <p>Registered for {@code /api/*} by {@link ClientRateLimitConfig}, after the trace id filter, so a
+ * refused request still gets its X-Trace-Id. The refusal is answered like any other
+ * {@link RateLimitExceededException}, by the application's exception handling.
+ */
+@RequiredArgsConstructor
+public class ClientRateLimitFilter extends OncePerRequestFilter {
+
+    private final RateLimiters rateLimiters;
+    private final RateLimitProperties rateLimits;
+    private final HandlerExceptionResolver exceptionResolver;
+
+    @Override
+    protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response,
+            FilterChain filterChain) throws ServletException, IOException {
+        var limit = rateLimits.client();
+        if (!rateLimiters.tryAcquire("client-rate:" + request.getRemoteAddr(), limit)) {
+            var refusal = new RateLimitExceededException(Scope.CALLER,
+                    "Too many requests from " + request.getRemoteAddr(), limit.retryAfterSeconds());
+            exceptionResolver.resolveException(request, response, null, refusal);
+            return;
+        }
+        filterChain.doFilter(request, response);
+    }
+}

src/main/resources/application.properties

@@ -22,9 +22,9 @@ spring.data.redis.repositories.enabled=false
 # explicit: Redisson brings a JCache provider, which Boot would otherwise prefer over Redis
 spring.cache.type=redis
 spring.cache.redis.key-prefix=resourceapi::
-spring.cache.redis.cache-null-values=false
 resourceapi.cache.sdlc-systems-ttl=10m
 resourceapi.cache.external-projects-ttl=5m
+resourceapi.cache.missing-external-projects-ttl=30s
 # redisson (locks, rate limiters) uses the same Redis; its object names get this prefix
 resourceapi.redisson.key-prefix=resourceapi:
 # a project's sync lock expires after this even if never released (e.g. the instance died)
@@ -37,6 +37,12 @@ resourceapi.rate-limit.sdlc.max-wait=1s
 resourceapi.rate-limit.sync.permits=6
 resourceapi.rate-limit.sync.period=1m
 resourceapi.rate-limit.sync.max-wait=0s
+# API requests per client IP (429 + Retry-After); behind a reverse proxy the client IP comes from
+# X-Forwarded-For, trusted only from internal proxy addresses (Tomcat's RemoteIpValve)
+resourceapi.rate-limit.client.permits=100
+resourceapi.rate-limit.client.period=1s
+resourceapi.rate-limit.client.max-wait=0s
+server.forward-headers-strategy=native
 # datasource
 spring.datasource.url=jdbc:h2:mem:test;MODE=MySQL;DATABASE_TO_LOWER=TRUE
 spring.sql.init.platform=h2