ci: construit et scanne les 3 images en parallèle + expose le dépôt CA - #11
Merged
Merged
Conversation
build passait CA, TSA et OCSP en séquentiel dans un seul job — environ 3x le temps d'une seule image (compilation Rust + scan Trivy). Reprend la même matrice que publish, sur 3 runners en parallèle. Renomme le check correspondant : « Construction des images (CA, TSA, répondeur OCSP) » devient trois checks « Construction et scan des images (CA/TSA/répondeur OCSP) » — mise à jour des rulesets dev et main dans le même geste (hors dépôt, via l'API).
La nouvelle page de dépôt public (GET /, PR précédente) n'était pas joignable de l'extérieur : le HTTPRoute ne routait que /download, /api/v1/ca.pem et /api/v1/conformance. Trouvé en testant pki.staging.open-eidas.eu juste après déploiement (404).
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The CI checks need distinct, non-cancelled results, and the CA server must implement the exposed root route.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This pull request parallelizes CA/TSA/OCSP image builds and scans and exposes the CA repository page through the public route.
Changes:
- Replaces sequential builds with a three-entry matrix.
- Adds an exact
/HTTPRoute. - Renames the image-build checks.
File summaries
| File | Review findings |
|---|---|
.github/workflows/ci.yml |
Matrix job names do not identify the image, and fail-fast may cancel sibling builds. |
deploy/helm/open-eidas/templates/ca/httproute.yaml |
The CA server lacks a GET / handler, so the new route still returns 404. |
Review details
Suppressed comments (2)
.github/workflows/ci.yml:98
- The matrix strategy defaults to
fail-fast: true, so a build or Trivy failure for one image cancels the other image jobs. With three per-image checks intended to be required, this leaves cancelled checks without independent results; disable fail-fast so all three builds and scans report their status.
strategy:
matrix:
include:
deploy/helm/open-eidas/templates/ca/httproute.yaml:48
- This forwards
/to the CA Service, but the currentca-serverrouter does not registerGET /(it only defines the API, download, and/healthzroutes inbin/ca-server/src/http.rs:167-179). The Gateway will therefore reach the pod and still return 404, so the public repository page remains inaccessible. Add the root handler/route to the server before exposing this path.
- matches:
- path:
type: Exact
value: /
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… OCSP) Sans référencer explicitement matrix.image dans le nom du job, GitHub affichait toutes les clés de la matrice (dockerfile, tag, pin_env compris) — illisible et fragile au moindre champ interne ajouté.
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Deux problèmes modérés restent à corriger avant approbation.
Review details
Suppressed comments (2)
.github/workflows/ci.yml:142
- Les trois legs de cette matrice utilisent le scope GHA par défaut (
buildkit). Aveccache-to, le dernier export écrase les caches exportés par les deux autres images ; leurs couches ne seront donc généralement pas réutilisées au prochain run, ce qui annule une partie du gain de parallélisation. Utilisez un scope stable par image (et le même scope danspublish).
cache-from: type=gha
cache-to: type=gha,mode=max
deploy/helm/open-eidas/templates/ca/httproute.yaml:48
- Cette règle ne rend pas encore le dépôt accessible : dans l’état de cette branche,
ca-serverne déclare aucun handlerGET /(bin/ca-server/src/http.rs:167-179), donc la Gateway transmettra cette requête au service qui répondra 404. Il faut intégrer le handler/page du dépôt dans cette branche (ou fusionner cette route avec ce changement) avant que l’exposition annoncée soit fonctionnelle.
- matches:
- path:
type: Exact
value: /
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Résumé
buildpassait CA/TSA/OCSP en séquentiel dans un seul job (~3x le temps d'une seule image). Reprend la matrice déjà utilisée parpublish, sur 3 runners en parallèle.HTTPRoutede la CA : la page de dépôt public (GET /, PR feat(ca): dépôt public des certificats + marquage STAGING #10) n'était pas joignable de l'extérieur, seuls/download,/api/v1/ca.pemet/api/v1/conformanceétaient routés. Trouvé en testantpki.staging.open-eidas.eujuste après déploiement (404).devetmainen conséquence (hors dépôt, via l'API) une fois cette PR mergée.Plan de test
helm lint/helm template: la nouvelle règle/est bien rendue