Consistent configuration with other apps#7
Conversation
📝 WalkthroughWalkthroughThe PR updates Apache HTTPD vhost configuration to migrate asset caching from a ChangesHTTPD Vhost Configuration
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app-httpd.conf (1)
42-52: ⚡ Quick winScope immutable caching to hashed filenames only.
Line 51 currently applies 1-year immutable caching to every file in
/assets/. If a non-hashed asset ever lands there, clients can get stuck on stale content for a long time. Constrain this header to hashed filenames and set a shorter TTL fallback for others.Proposed config adjustment
<Directory /opt/bitnami/apache/htdocs/gridmonitor/assets/> - Header set Cache-Control "public, max-age=31536000, immutable" + # Immutable cache only for content-hashed build artifacts + <FilesMatch "\.[0-9a-fA-F]{8,}\.(js|css|svg|png|jpg|jpeg|webp|woff2?)$"> + Header set Cache-Control "public, max-age=31536000, immutable" + </FilesMatch> + + # Safer fallback for non-hashed files + <FilesMatch "^(?!.*\.[0-9a-fA-F]{8,}\.).*$"> + Header set Cache-Control "public, max-age=300" + </FilesMatch> </Directory>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-httpd.conf` around lines 42 - 52, The current Header set Cache-Control inside the <Directory /opt/bitnami/apache/htdocs/gridmonitor/assets/> block applies immutable 1-year caching to all files; restrict the long-lived immutable cache to only hashed filenames by adding a FilesMatch (or LocationMatch) that matches a content-hash pattern (e.g., filenames containing "-[a-f0-9]{8,}" before the extension) and apply Header set Cache-Control "public, max-age=31536000, immutable" only there (reference the existing Header set Cache-Control), then add a second, broader Header set Cache-Control with a much shorter TTL (e.g., max-age=3600) for all other files in the same Directory to serve as the fallback for non-hashed assets.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app-httpd.conf`:
- Line 74: Add a short justification comment and linked documentation for the
Increase of the Apache directive "LimitRequestLine 64000": state the business
requirement (e.g., required max URL/query length for feature X or specific
client behavior), why 64k was chosen, and any security/hardening considerations;
then verify and document that upstream/downstream components (gateway, ingress,
reverse proxies, and backend services) have matching or intentionally
complementary limits and list where those limits are enforced (or note required
config changes). Update the repository docs (or add a note next to the
"LimitRequestLine 64000" line) to reference the justification and the components
checked, and include a short test/verification step to reproduce expected
behavior (or a link to the test plan).
---
Nitpick comments:
In `@app-httpd.conf`:
- Around line 42-52: The current Header set Cache-Control inside the <Directory
/opt/bitnami/apache/htdocs/gridmonitor/assets/> block applies immutable 1-year
caching to all files; restrict the long-lived immutable cache to only hashed
filenames by adding a FilesMatch (or LocationMatch) that matches a content-hash
pattern (e.g., filenames containing "-[a-f0-9]{8,}" before the extension) and
apply Header set Cache-Control "public, max-age=31536000, immutable" only there
(reference the existing Header set Cache-Control), then add a second, broader
Header set Cache-Control with a much shorter TTL (e.g., max-age=3600) for all
other files in the same Directory to serve as the fallback for non-hashed
assets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| ProxyPassMatch "^/api/gateway/(.*)$" http://gateway/$1 nocanon | ||
| ProxyPassMatch "^/ws/gateway/(.*)$" ws://gateway/$1 nocanon | ||
|
|
||
| LimitRequestLine 64000 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Apache limit settings =="
rg -n -C2 'LimitRequestLine|LimitRequestFieldSize|LimitRequestFields' .
echo
echo "== Possible upstream/downstream request-size limits =="
rg -n -C2 --iglob '*.{conf,cnf,properties,yml,yaml,json,xml}' \
'max-http-request-header-size|max-http-header-size|maxHttpHeaderSize|large_client_header_buffers|client_header_buffer_size|414|request.?line' .Repository: gridsuite/gridmonitor-app
Length of output: 770
Document the business requirement and justify the 64k request-line limit before merge.
Line 74 sets LimitRequestLine to 64000, a significant increase from defaults. The verification found no documented matching limits in gateway, ingress, or backend configurations within this repository. This isolation creates risk of inconsistent behavior with upstream/downstream components (unexpected 400/414 errors) and weakens request-size hardening. Provide explicit justification for why 64000 is needed and confirm that all components in the request handling chain (proxy, ingress, backend services) enforce consistent or intentionally complementary limits.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app-httpd.conf` at line 74, Add a short justification comment and linked
documentation for the Increase of the Apache directive "LimitRequestLine 64000":
state the business requirement (e.g., required max URL/query length for feature
X or specific client behavior), why 64k was chosen, and any security/hardening
considerations; then verify and document that upstream/downstream components
(gateway, ingress, reverse proxies, and backend services) have matching or
intentionally complementary limits and list where those limits are enforced (or
note required config changes). Update the repository docs (or add a note next to
the "LimitRequestLine 64000" line) to reference the justification and the
components checked, and include a short test/verification step to reproduce
expected behavior (or a link to the test plan).
antoinebhs
changed the title
|
2 participants
PR Summary
Uniformize conf with other gridsuite apps: account for gridsuite/gridexplore-app@9b3930d#diff-dd2c0eb6ea5cfc6c4bd4eac30934e2d5746747af48fef6da689e85b752f39557