Skip to content

[OSDEV-2328] Add CloudFront caching for facilities and production locations enpoints#851

Merged
VadimKovalenkoSNF merged 6 commits intomainfrom
OSDEV-2328-cache-facilities-production-locations
Jan 8, 2026
Merged

[OSDEV-2328] Add CloudFront caching for facilities and production locations enpoints#851
VadimKovalenkoSNF merged 6 commits intomainfrom
OSDEV-2328-cache-facilities-production-locations

Conversation

@VadimKovalenkoSNF
Copy link
Contributor

@VadimKovalenkoSNF VadimKovalenkoSNF commented Jan 5, 2026

Ticket: OSDEV-2328

  • Added CloudFront caching behaviors for api/facilities/* and api/v1/production-locations/*, keeping cached_methods = ["GET","HEAD","OPTIONS"] while allowed_methods stays broad so non-GET verbs still reach the ALB origin.

  • Introduced endpoint-specific TTL vars (api_facilities_cache_{default,max}_ttl, api_production_locations_cache_{default,max}_ttl) and wired them into the behaviors.

  • Set per-environment values to:

    • 1800s (30 mins) for Prod, RBA, Preprod, Staging
    • 60s (1 min) for Dev, Test, CI tests
  • Forward only Authorization and session cookies (sessionid and csrftoken), and not the Accept/User-Agent etc., so DRF returns the JSON renderer instead of the browsable API.

@VadimKovalenkoSNF VadimKovalenkoSNF self-assigned this Jan 5, 2026
@barecheck
Copy link

barecheck bot commented Jan 5, 2026

React App | Jest test suite - Code coverage report

Total: 40.21%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

@barecheck
Copy link

barecheck bot commented Jan 5, 2026

Dedupe Hub App | Unittest test suite - Code coverage report

Total: 55.73%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

@barecheck
Copy link

barecheck bot commented Jan 5, 2026

Countries App | Unittest test suite - Code coverage report

Total: 100%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

@barecheck
Copy link

barecheck bot commented Jan 5, 2026

Contricleaner App | Unittest test suite - Code coverage report

Total: 98.75%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

@barecheck
Copy link

barecheck bot commented Jan 5, 2026

Django App | Unittest test suite - Code coverage report

Total: 81.87%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
deployment/terraform/cdn.tf (1)

218-221: Consider removing csrftoken from the cookie whitelist.

CSRF tokens are designed to protect state-changing requests (POST, PUT, DELETE), which are not cached by this behavior (cached_methods only includes GET/HEAD/OPTIONS). Including csrftoken in the cache key for GET requests appears unnecessary and will contribute to cache fragmentation—each unique CSRF token value generates a separate cache entry.

If these endpoints don't require CSRF validation for GET requests (which is typical), consider removing it:

♻️ Suggested change
         cookies {
           forward           = "whitelist"
-          whitelisted_names = ["sessionid", "csrftoken"]
+          whitelisted_names = ["sessionid"]
         }

Please confirm whether the CSRF token affects the response content for GET requests on these endpoints.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 15d663f and 38d510e.

📒 Files selected for processing (1)
  • deployment/terraform/cdn.tf
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: init-and-plan
🔇 Additional comments (4)
deployment/terraform/cdn.tf (4)

3-14: Well-structured cache behavior configuration.

The use of a local variable to define the API-specific cache behaviors is clean and extensible. The path patterns correctly match the targeted endpoints, and the TTL parameterization via environment-specific variables provides good flexibility.


205-212: Correct behavior ordering for CloudFront path matching.

The dynamic block is correctly positioned before the generic api/* fallback behavior (lines 232-252). CloudFront evaluates cache behaviors in order and uses the first match, so the more specific patterns (api/facilities/*, api/v1/production-locations/*) will be matched before falling through to the catch-all.


214-222: **Verify cache key aligns with endpoint behavior.**With Authorization header and sessionid cookie in the cache key, "cookies that have user-specific or session-specific values and are unique across thousands (or even millions) of requests are also not good candidates for cache key inclusion." Each authenticated user will get their own cached entry, significantly fragmenting the cache.

If the api/facilities/* and api/v1/production-locations/* endpoints return public data (same response regardless of authentication), consider removing Authorization and sessionid from the cache key to maximize cache hit ratio. "You can get better performance from your website or application when you have a higher cache hit ratio... One way to improve your cache hit ratio is to include only the minimum necessary values in the cache key."

If these endpoints return user-specific data, then the current configuration is appropriate but be aware that cache effectiveness will be limited to per-user hits.

Please confirm whether the facilities and production-locations endpoints return public data or user-specific data that varies by authentication state.


224-228: TTL configuration looks appropriate.

Using min_ttl = 0 allows the origin to control caching via Cache-Control headers when needed, while the parameterized default_ttl and max_ttl provide environment-specific flexibility (30 min for production environments, 1 min for dev/test).

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 7, 2026

Copy link
Contributor

@roman-stolar roman-stolar left a comment

Choose a reason for hiding this comment

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

Looks clean!

Copy link
Collaborator

@protsack-stephan protsack-stephan left a comment

Choose a reason for hiding this comment

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

Good work!

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