Skip to content

fix(ci): supprimer les effets de bord filesystem à l'import qui cassaient la collecte pytest - #3

Open
TiotBenjy wants to merge 3 commits into
repod-ce:mainfrom
TiotBenjy:fix/git-ci
Open

TiotBenjy wants to merge 3 commits into
repod-ce:mainfrom
TiotBenjy:fix/git-ci

Conversation

@TiotBenjy

Copy link
Copy Markdown
Contributor

Problème

La CI échouait avant qu'un seul test ne s'exécute : 7 erreurs de collecte, Interrupted: 7 errors during collection, exit code 2.

services/component_sbom.py:29: in <module>
    SBOM_DIR.mkdir(parents=True, exist_ok=True)
E   PermissionError: [Errno 13] Permission denied: '/repos'

Le ERROR: Coverage failure: total of 17 is less than fail-under=38 du même log est une conséquence, pas une seconde panne : aucun test n'ayant tourné, la couverture mesurée était celle des seuls modules importés. Aucun ajustement de seuil n'est nécessaire.

Cause racine

services/component_sbom.py créait SBOM_DIR (/repos/sboms par défaut) au niveau module. routers/upload.py importe ce module et routers/__init__.py importe upload : le moindre import routers.* tentait donc un mkdir sur /repos, non inscriptible pour l'utilisateur non-root d'un runner GitHub.

Six autres modules avaient le même motif (manifest, audit, indexer, pending_promotions, cve_enrichment, routers/upload). Ils ne cassaient pas encore parce que chaque fichier de test redéclare individuellement
MANIFEST_DIR, AUDIT_DIR, etc. via os.environ.setdefault en tête de module. SBOM_DIR était simplement plus récent que ces déclarations. La panne était donc structurelle : tout nouveau X_DIR ajouté au graphe d'import cassait la CI de la même façon.

Correctif

Convention appliquée à l'ensemble des modules concernés : importer un module ne doit jamais toucher au filesystem. Les répertoires sont créés à la première écriture, dans la fonction qui écrit.

Module Créé désormais par
services/component_sbom.py save_component_sbom()
services/manifest.py save_manifest()
services/audit.py log()
services/indexer.py _save_index()
services/pending_promotions.py create_pending()
services/cve_enrichment.py _save_json() (déjà présent, mkdir d'import redondant)
routers/upload.py _ensure_upload_dirs(), appelé par les deux handlers POST

L'alternative envisagée (centraliser les variables d'environnement dans tests/conftest.py) a été écartée : 29 des 37 fichiers de test utilisent leur _TMP local au-delà des appels setdefault, que conftest rendrait silencieusement inopérants. Le correctif côté modules ne touche aucun test existant.

Garde-fou

tests/test_no_import_side_effects.py (nouveau) :

  • analyse statique ast de services/, routers/, auth/, db/ : échoue si un mkdir/makedirs/touch/write_text/write_bytes réapparaît au niveau module. Descend dans les for/if/try de premier niveau (le cas routers/upload.py) mais jamais dans un corps de fonction. N'importe aucun module inspecté, ce qui serait précisément l'effet de bord testé ;
  • 3 tests du détecteur lui-même, sur les motifs exacts corrigés ici ;
  • 6 tests runtime qui pointent chaque répertoire sur un chemin absent et appellent le writer : sans le mkdir différé, ils lèvent FileNotFoundError. Ils couvrent notamment les deux cas sans mkdir dédié, _save_json() (unique point d'écriture du module) et update_pending() (sort en None avant l'écriture si le fichier n'existe pas, donc ne peut jamais être le premier writer).

Impact production

Aucun changement de comportement. Les répertoires sont créés au premier usage au lieu du démarrage. Vérifié :

  • tous les lecteurs passent par Path.glob/rglob/.exists(), qui renvoient vide sur un répertoire absent (pas de os.listdir, qui lèverait) ;
  • le backend ne monte aucun StaticFiles sur ces chemins (un montage échouerait au démarrage sur répertoire absent) ;
  • nginx reçoit pool/, dists/, conf/, db/ par bind mount docker-compose, créés par Docker indépendamment du backend.

Vérification

Exécuté dans un conteneur python:3.10-slim, utilisateur non-root (--user 1000:1000), reproduisant les conditions du runner. En root le bug ne se reproduit pas : /repos est créable.

  • avant correctif : les 7 mêmes erreurs de collecte que la CI ;
  • après correctif, commande CI exacte : 1586 passed, 3 skipped, 0 failed,
    Required test coverage of 38% reached. Total coverage: 50.19% ;
  • import routers et les 6 modules touchés avec zéro variable d'environnement sur un / non inscriptible : OK, /repos jamais créé ;
  • ruff check . : All checks passed.

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.

1 participant