Add GridCapa Homepage #673
Conversation
WalkthroughAdds a static GridCapa homepage (HTML, CSS, JS, SVG assets), mounts and serves it from nginx (Docker Compose and many nginx configs), and embeds the assets into Kubernetes ConfigMaps with ingress/middleware rules to route / or /homepage to the apps-metadata-server. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
k8s/overlays/coreso/prod/ingress-gridcapa.yaml (2)
696-697:⚠️ Potential issue | 🟡 MinorMinor: duplicate
---document separator.Line 697 is a redundant YAML document separator immediately after line 696. The same issue appears at line 1743. While most YAML parsers tolerate this (it creates an empty document), it's noisy.
1744-1774:⚠️ Potential issue | 🔴 CriticalFix YAML indentation in second path definition and verify middleware intent for
/homepage/.The second path entry has a critical indentation error:
service:(line 1765) should be indented underbackend:, not at the same level. This breaks the YAML structure and makes the Ingress resource invalid.Additionally, the
gridcapa-apps-metadatamiddleware (referenced at the Ingress level) only strips the/apps-metadata/prefix. The/homepage/path will inherit this middleware, meaning the/homepage/prefix won't be stripped. If the homepage assets require prefix stripping (e.g., for CSS/JS relative paths), a separate middleware or per-path configuration is needed.
🤖 Fix all issues with AI agents
In `@docker-compose/common/config/homepage/homepage.js`:
- Around line 1-16: The buildMenu function currently constructs HTML via string
concatenation and assigns it with innerHTML (using innerHTML +=) which allows
XSS through app.url, app.appColor and app.name; replace this by creating
elements programmatically: get the container element with id
'gridcapa-homepage-menu', create a DocumentFragment, and for each app create an
li and a via document.createElement('li') and document.createElement('a'), set
the anchor's href using setAttribute('href', app.url), set its style.color via
element.style.color = app.appColor, set its visible text with textContent =
app.name.replace('Capa ', ''), then append the anchor to the li and the li to
the fragment; after the loop append the fragment to the container to avoid
repeated re-parsing and eliminate innerHTML usage (affects buildMenu and where
apps-metadata.json is consumed).
In `@docker-compose/nginx-dev/nginx-core-valid-day-ahead-conservative.conf`:
- Around line 48-54: The nginx config has an orphan "{ }" because the location /
block (using include mime.types, index homepage.html, alias
/usr/share/nginx/html/homepage/) was accidentally merged with the proxy_pass
block for filebrowser; fix by splitting into two valid location blocks: one
"location /" that contains only the include, index and alias directives (closed
properly), and a separate "location /utils/filebrowser/" (or the original
filebrowser path) that contains the proxy_pass http://filebrowser-upstream/
directive (opened and closed properly); remove the stray "{", ensure each
location block has matching braces, and keep the proxy_pass associated with the
filebrowser location.
In `@k8s/overlays/azure/dev/apps-metadata-server-configmap.yaml`:
- Around line 43-50: The CSS rule for the selector "a" sets display: block while
also declaring flex container properties justify-content and align-items which
have no effect; fix all copies of homepage.css inside the ConfigMaps by either
removing the justify-content and align-items declarations or changing display:
block to display: flex (update the "a" rule accordingly) so the intent is
preserved across apps-metadata-server-configmap.yaml and all other ConfigMap
copies.
In `@k8s/overlays/azure/test/apps-metadata-server-configmap.yaml`:
- Around line 115-131: Replace the unsafe innerHTML concatenation in buildMenu
with DOM API calls: for each app in buildMenu use document.createElement('li')
and document.createElement('a'), set the anchor's href via
anchor.setAttribute('href', app.url) or validate with new URL(...) to prevent
"javascript:" schemes, set visual color via anchor.style.color = app.appColor,
and assign display text with anchor.textContent = app.name.replace('Capa ', '');
then append the anchor to the li and li to menu; update the identical
homepage.js copies (buildMenu, menu, app.url, app.appColor, app.name) in the
other ConfigMaps accordingly.
In `@k8s/overlays/azure/test/gridcapa-ingress.yaml`:
- Around line 394-400: Add the missing Azure ingress annotations and rewrite
rule to mirror dev: insert the nginx.ingress.kubernetes.io/app-root annotation
with value /homepage/homepage.html on the same Ingress metadata/annotations
block and add the rewrite directive rewrite ^/homepage$ /homepage/ permanent;
(the rewrite that complements the existing rewrite-target: /$2) so requests to /
redirect to the homepage and /homepage (no trailing slash) is redirected; locate
the Ingress that routes to service apps-metadata-server and update its
annotations and/or server-snippet to include these entries.
In `@k8s/overlays/coreso/prod/ingress-gridcapa.yaml`:
- Around line 1768-1774: The YAML block for the /homepage/ path has broken
indentation: the keys backend, service, name, port, number, path and pathType
must be nested under the same parent as other path entries; fix by indenting
service (and its nested name/port/number) one level under backend so the
structure matches the earlier path entry (ensure path: /homepage/ and pathType:
ImplementationSpecific remain aligned with the other path entries).
In `@k8s/overlays/coreso/test/gridcapa-ingress.yaml`:
- Around line 1594-1601: Traefik won’t auto-redirect root to /homepage/, so add
a Traefik middleware (e.g., RedirectRegex or RewritePath) and attach it to the
ingress rule that handles path "/" to route "/" → "/homepage/"; also verify
backend expectations for apps-metadata-server: the existing middleware
default-gridcapa-apps-metadata only strips the "/apps-metadata/" prefix, so if
the apps-metadata backend expects to see "/" rather than "/homepage/" create and
attach a separate middleware (e.g., default-gridcapa-homepage) that strips
"/homepage/" before proxying to the apps-metadata-server; ensure the ingress
entry for path "/homepage/" references the correct middleware when routing to
service apps-metadata-server.
🧹 Nitpick comments (10)
docker-compose/nginx-dev/nginx-cse-valid-d2cc.conf (1)
51-55: Preferrootoveraliasforlocation /, and movemime.typesto thehttpblock.Two items on this homepage location block (applies identically to all ~17 nginx configs in this PR):
alias→root: Forlocation /,aliasandrootare functionally equivalent, butrootis the idiomatic choice and avoids the well-known nginx footgun wherealiason a prefix location can produce unexpected path concatenation when the location path changes later.
include mime.typesplacement: Includingmime.typesinside individuallocationblocks means it's parsed per-location. Moving it once to thehttp {}block ensures all locations (including future ones) benefit from correct MIME types without repetition.Suggested change (in every nginx config)
At the
httplevel (e.g., after line 5):http { + include mime.types; + # Common sectionIn the
location /block:location / { - include mime.types; index homepage.html; - alias /usr/share/nginx/html/homepage/; + root /usr/share/nginx/html/homepage; }And similarly remove
include mime.types;from the/apps-metadata.jsonlocation.docker-compose/nginx-dev/nginx.conf (1)
62-66:location /catch-all will serve homepage instead of 404 for unknown paths.Since
location /is the lowest-priority prefix match in nginx, any request to an undefined path (e.g.,/nonexistent) will servehomepage.htmlrather than returning a 404. This is acceptable if intentional (SPA-like routing), but it can mask misconfigurations and broken links during development.Consider whether a dedicated
/homepage/location block (matching the k8s ingress paths) would be more appropriate, paired with a redirect from/to/homepage/homepage.html.docker-compose/nginx-dev/nginx-core-cc.conf (1)
48-52: Preferrootoveraliasforlocation /.When using
location /, thealiasdirective is functionally equivalent torootbutrootis the idiomatic and recommended choice in nginx.aliasis designed for locations where the URI portion needs to be replaced (e.g.,location /images/), whereasrootsimply prepends the path. This same pattern is repeated across all nginx config files in this PR.♻️ Suggested change
location / { include mime.types; index homepage.html; - alias /usr/share/nginx/html/homepage/; + root /usr/share/nginx/html/homepage; }docker-compose/common/config/homepage/homepage.css (2)
22-29: Ineffective flex properties on block-level<a>element.
justify-contentandalign-itemsonly apply to flex/grid containers. Sincedisplay: blockis set, these properties have no effect. Either remove them or change todisplay: flex.♻️ Proposed fix
a { font-weight: bold; text-decoration: none; font-size: x-large; - display: block; - justify-content: center; - align-items: center; + display: flex; + justify-content: center; + align-items: center; }
1-50: Bare element selectors may cause unintended side effects if this stylesheet is ever shared.Selectors like
a,li,span, andmenutarget all instances globally. This is fine for a standalone page, but consider scoping under.gridcapa-homepageif the CSS might be loaded alongside other content in the future.k8s/overlays/coreso/prod/apps-metadata-server-configmap.yaml (1)
9-131: Homepage assets are duplicated betweendocker-compose/common/config/homepage/and this ConfigMap.The CSS, HTML, JS, and SVG content is copy-pasted into the ConfigMap rather than generated from a single source. Any fix (e.g., the XSS issue in
homepage.js) must be applied in multiple places. Consider using a build step or KustomizeconfigMapGeneratorwith file references to maintain a single source of truth.docker-compose/common/config/homepage/homepage.html (2)
9-10: Move<script>to end of<body>or adddeferattribute.The script is placed before any DOM elements it manipulates. It works today only because
fetch()is async and the DOM is ready by the time the callback fires. This is fragile — usedeferto make the dependency on DOM readiness explicit.♻️ Proposed fix
-<script src="homepage.js"></script> +<script src="homepage.js" defer></script>Or move the
<script>tag just before</body>.
1-36: Extensive use of inline styles.Most
styleattributes (lines 13, 14, 21, 28–30, etc.) could be moved intohomepage.cssfor better maintainability. This is a minor nit for a simple standalone page.k8s/overlays/azure/test/apps-metadata-server-configmap.yaml (2)
86-87: Move<script>to end of<body>or adddefer.The
<script src="homepage.js">is placed before the DOM elements it manipulates (gridcapa-homepage-menu). While the asyncfetchcallback will likely fire after DOM parsing, placing the script at the end of<body>(or addingdefer) is the standard pattern to avoid any race condition.♻️ Move script to end of body
<body> - <script src="homepage.js"></script> <div class="center gridcapa-homepage"> ... </div> + <script src="homepage.js"></script> </body>This applies to all three ConfigMap copies of
homepage.html.
9-150: Homepage assets are duplicated verbatim across multiple ConfigMaps.The
farao-logo.svg,homepage.css,homepage.html,homepage.js, andlogo-minio.svgcontent is copy-pasted identically into at least three overlay ConfigMaps (azure/dev,azure/test,coreso/test, and likelycoreso/prod). This makes maintenance error-prone — a fix in one file must be replicated everywhere.Consider extracting these assets into a shared Kustomize base ConfigMap and referencing it from each overlay, or using a
configMapGeneratorwith shared file sources.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docker-compose/common/config/homepage/homepage.css`:
- Around line 45-48: The .divider rule uses height: 100% which will collapse
unless its parent has an explicit height; either give the containing element a
defined height (e.g., set the parent container to a fixed height or 100vh) or
change the .divider to use a full-viewport approach (min-height: 100vh) or make
the parent a flex/container and let .divider stretch (e.g., parent display:flex
and align-items:stretch) so the vertical line renders as intended.
- Around line 1-9: The body rule sets a dark background (`#333333`) but lacks a
text color, causing default black text to be unreadable; update the body CSS
rule (the body selector that currently contains background-color: `#333333`) to
include an appropriate foreground color (e.g., color: `#ffffff` or a light gray)
so paragraphs, headings and spans inherit readable text color while leaving
anchor/link colors to be styled separately as needed.
In `@k8s/overlays/azure/test/gridcapa-ingress.yaml`:
- Line 5: The inline comment next to the annotation
nginx.ingress.kubernetes.io/app-root is inconsistent with the annotation value
(/homepage/homepage.html); update the comment to accurately reflect the redirect
target (e.g., "requests to '/' will be redirected to '/homepage/homepage.html'")
or, if the intended redirect is '/homepage.html', change the annotation value to
'/homepage.html' so the comment and annotation match; ensure the comment and the
value for nginx.ingress.kubernetes.io/app-root are consistent.
🧹 Nitpick comments (2)
docker-compose/common/config/homepage/homepage.css (1)
22-40: Broad element selectors may cause unintended side-effects.
a,li,span, andmenuare styled globally without scoping to a class or container. If this stylesheet is ever loaded alongside other content, these rules will bleed into unrelated markup. Consider scoping under.gridcapa-homepage(e.g.,.gridcapa-homepage a { … }).docker-compose/common/config/homepage/homepage.js (1)
1-19: Previous XSS concern has been addressed — looks good.The refactored code now uses
createElement,textContent, andDocumentFragment, which eliminates the innerHTML-based XSS vector and avoids repeated DOM re-parsing.One minor gap:
document.getElementById('gridcapa-homepage-menu')could returnnullif the element is missing, causing Line 18 to throw. A guard would make this more robust:let buildMenu = (apps) => { const menu = document.getElementById('gridcapa-homepage-menu'); + if (!menu) { + console.error('Menu element not found'); + return; + } const fragment = document.createDocumentFragment();
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
k8s/overlays/coreso/prod/ingress-middlewares.yaml (1)
667-676:⚠️ Potential issue | 🔴 CriticalInvalid Traefik Middleware: cannot combine
stripPrefixandredirectRegexin a single resource, andredirectRegexsyntax is incorrect.Two issues here:
One type per Middleware resource: Traefik's Middleware CRD allows only one middleware type per resource. Combining
stripPrefixandredirectRegexunder the samespecviolates the CRD specification. Create a separate Middleware resource for the redirect, or use a chain middleware to combine them.Incorrect
redirectRegexstructure: The fieldsregexandreplacementmust be direct keys underredirectRegex, not YAML list items. The current syntax with- regex:and- replacement:is invalid. Additionally, the regex"/"matches any URL containing a forward slash; use^/$or a more specific pattern to match only the intended path.Proposed fix: split into two Middleware resources
apiVersion: traefik.containo.us/v1alpha1 kind: Middleware metadata: creationTimestamp: null name: gridcapa-apps-metadata namespace: default spec: stripPrefix: prefixes: - /apps-metadata/ - /homepage/ - redirectRegex: - - regex: "/" - - replacement: "/homepage/homepage.html" +--- +apiVersion: traefik.containo.us/v1alpha1 +kind: Middleware +metadata: + creationTimestamp: null + name: gridcapa-homepage-redirect + namespace: default +spec: + redirectRegex: + regex: "^/$" + replacement: "/homepage/homepage.html"Reference the new
gridcapa-homepage-redirectmiddleware in the relevant IngressRoute annotations.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
k8s/overlays/coreso/prod/ingress-middlewares.yaml (1)
651-661:⚠️ Potential issue | 🟡 MinorPre-existing bug:
gridcapa-core-cc-task-notificationstrips the wrong prefix.The middleware named
gridcapa-core-cc-task-notification(Line 655) strips/cse/import/idcc/task-notification/(Line 660) instead of the expected/core/cc/task-notification/. This appears to be a copy-paste error from the CSE import IDCC block. Not introduced by this PR, but worth fixing while you're here.🐛 Suggested fix
spec: stripPrefix: prefixes: - - /cse/import/idcc/task-notification/ + - /core/cc/task-notification/
🤖 Fix all issues with AI agents
In `@k8s/overlays/azure/dev/apps-metadata-server-configmap.yaml`:
- Around line 88-91: The <br /> element is an invalid direct child of the <menu
id="gridcapa-homepage-menu">; remove the <br /> and instead apply spacing to the
first <li> (e.g., add a class or inline style such as margin-top/margin-bottom
to the <li> with "Processes :") so that the visual gap is preserved without
placing a <br /> inside the <menu>.
- Around line 66-77: The homepage.js script is loaded in the head and may run
before DOM elements like the element with id 'gridcapa-homepage-menu' exist (the
buildMenu function queries that id); fix by adding the defer attribute to the
<script src="homepage.js"> tag or move the <script> tag to just before </body>
in the homepage.html content of the ConfigMap so homepage.js always runs after
the DOM is parsed; apply the same change across all three ConfigMap copies
(azure/dev, coreso/test, coreso/prod).
🧹 Nitpick comments (2)
k8s/overlays/azure/dev/apps-metadata-server-configmap.yaml (1)
9-142: Homepage assets are duplicated verbatim across three ConfigMap overlays.
farao-logo.svg,homepage.css,homepage.html,homepage.js, andlogo-minio.svgare identical in azure/dev, coreso/test, and coreso/prod. Consider extracting them into a shared Kustomize base ConfigMap and patching only per-environment differences (e.g.,apps-metadata.json). This would eliminate triple-maintenance of these assets.k8s/overlays/coreso/prod/apps-metadata-server-configmap.yaml (1)
104-123:homepage.jsindentation is inconsistent with the azure/dev copy.The function body of
buildMenuand theforloop body are not indented relative to their enclosing blocks (compare with the azure/dev ConfigMap where they are properly indented). JS execution is unaffected, but this makes the embedded source harder to read and maintain, and diverges from the other overlay copies.
| menu { | ||
| margin:auto; | ||
| list-style-type:none; | ||
| } | ||
| li { | ||
| display:block; | ||
| padding:0.5rem; | ||
| text-align:left; | ||
| } |
| <head> | ||
| <meta charset="utf-8" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <link rel="stylesheet" href="homepage.css"> |
There was a problem hiding this comment.
This markup could be auto-closed
| <script src="homepage.js"></script> | ||
| <div class="center gridcapa-homepage"> | ||
| <!-- GridCapa Logo --> | ||
| <div style="align-self: center;width: 30%"> |
There was a problem hiding this comment.
There are many style attributes put directly in the HTML code. It would be cleaner to use the CSS file to store this information.
| <!-- List of processes (dynamic) --> | ||
| <div class="divider" style="align-self: flex-start;width: 35%"> | ||
| <menu id="gridcapa-homepage-menu" class="center"> | ||
| <li style="color: white;font-weight: lighter">Processes :</li> |
There was a problem hiding this comment.
"Processes" looks so small compared to everything else on the page, you should consider improving this
| <span style="color: white">Grid</span> | ||
| <span style="color: grey;">Capa</span> |
There was a problem hiding this comment.
I'm being nitpicky but there should be no space between "Grid" and "Capa". Furthermore, "Capa" should be thinner than "Grid" if possible.
Just in case it could be useful, I once worked on a full SVG version of the GridCapa logo which is available here: https://opensource.rte-france.com/assets/images/gridcapa/logo-gridcapa.svg. Unfortunately, it was designed for a fully horizontal use (see https://opensource.rte-france.com/projects/gridcapa) with a light-colored background and won't fit in your page as is.
| @@ -0,0 +1,19 @@ | |||
| fetch('apps-metadata.json') | |||
There was a problem hiding this comment.
I don't know by how but you managed to make apps-metadata.json available on /homepage/apps-metadata.json URL.
However, it is already available on /apps-metadata/apps-metadata.json URL (this URL is the one used by gridcapa-app to retrieve processes information). Shouldn't we consider using the same source of information rather than defining a new source?
| .center { | ||
| margin: 50px auto; | ||
| width: 90%; | ||
| padding: 10px; | ||
| text-align: center; | ||
| } |
There was a problem hiding this comment.
Your center class does much more than centring content. Could you clarify the responsabilities? (Maybe put some rules on the gridcapa-homepage class and some on gridcapa-homepage-menu class)
| </div> | ||
| <!-- List of processes (dynamic) --> | ||
| <div class="divider" style="align-self: flex-start;width: 35%"> | ||
| <menu id="gridcapa-homepage-menu" class="center"> |
There was a problem hiding this comment.
I don't really know why you need to add the center class here as content should already be centered through the center on the parent div on line 11.
| <!-- Tools (MinIO) --> | ||
| <div class="divider" style="align-self: center;width: 35%;"> | ||
| <a href="/minio/browser/gridcapa" style="color: white"> | ||
| <img src="logo-minio.svg" alt="icon" style="width:10%"> |
| <!-- Tools (MinIO) --> | ||
| <div class="divider" style="align-self: center;width: 35%;"> | ||
| <a href="/minio/browser/gridcapa" style="color: white"> | ||
| <img src="logo-minio.svg" alt="icon" style="width:10%"> |
There was a problem hiding this comment.
In the alternative text, be more precise: use "MinIO" rather than "logo"
| annotations: | ||
| nginx.ingress.kubernetes.io/app-root: /homepage/homepage.html # requests to "/" will be redirected to "/homepage/homepage.html" | ||
| nginx.ingress.kubernetes.io/configuration-snippet: | | ||
| rewrite ^/homepage$ /homepage/ permanent; |
There was a problem hiding this comment.
Content at /homepage/ looks like this...
| app: gridcapa | ||
| component: apps-metadata-server-configmap | ||
| data: | ||
| farao-logo.svg: "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<!-- Generator: Adobe |
There was a problem hiding this comment.
Maybe we can find a better alternative to avoid duplicating files content in apps-metadata-server configmaps on all platforms, as the files content will be the same regardless of the environment.
2 participants
Please check if the PR fulfills these requirements (please use
'[x]'to check the checkboxes, or submit the PR and then click the checkboxes)Does this PR already have an issue describing the problem ? If so, link to this issue using
'#XXX'and skip the restWhat kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
Other information:
(if any of the questions/checkboxes don't apply, please delete them entirely)
Summary by CodeRabbit
New Features
Chores