feat(lib): helper ini_set idempotent pour merge de fichiers INI #24

Closed

Damien wants to merge 3 commits from feat/lib-ini-set into main

pull from: feat/lib-ini-set

merge into: :main

:main

Conversation 3 Commits 3 Files Changed 1

+107

Damien commented 2026-07-31 15:35:01 +00:00

Owner

Copy Link

Copy Source

Contexte

Le pattern openbao (« écrire la config seulement si absente ») entre en collision avec la règle d'idempotence du dépôt dès qu'un fichier de config gagne de nouvelles clés au fil des versions du script (cas de l'app.ini de Gitea en #19). Il faut un merge clé par clé, scopé à la section, pas un tout-ou-rien au niveau du fichier.

Implémentation

ini_set <file> <section> <key> <value> dans lib/common.sh, en bash + awk pur (pas de crudini) :

• Fichier absent → créé
• Section absente → ajoutée en fin de fichier
• Clé absente dans la section → insérée dans la section
• Clé présente, même valeur → no-op (sortie strictement identique)
• Clé présente, valeur différente → remplacée en place
• Le matching est scopé à la section courante (via un flag in_section remis à jour à chaque en-tête [...]), donc une même clé dans deux sections différentes (ENABLED dans [metrics] et [actions]) n'interfère jamais.
• Écriture atomique : mktemp + mv.
• Commentaires et lignes vides hors de la ligne modifiée sont préservés tels quels.

Documenté dans l'en-tête de lib/common.sh comme premier helper OS-agnostique, à côté du contrat Alpine-only existant.

Idempotence — comment vérifié

Testé manuellement (voir les 6 cas du tableau de l'issue) sur un app.ini de test :

ini_set app.ini server DOMAIN new.example.com
cp app.ini after1.ini
ini_set app.ini server DOMAIN new.example.com
diff after1.ini app.ini   # vide → deux appels identiques laissent le fichier inchangé

Vérifié aussi : fichier absent (création), section absente (ajout en fin de fichier), clé absente dans section existante (insertion), clé présente avec valeur différente (remplacement en place sans toucher au reste), indépendance [metrics] ENABLED / [actions] ENABLED.

Vérification

• shellcheck lib/common.sh : aucune nouvelle alerte (SC2148 préexistant, lié à l'absence de shebang sur un fichier sourcé — confirmé identique sur main avant ce changement).

Closes #18

## Contexte Le pattern openbao (« écrire la config seulement si absente ») entre en collision avec la règle d'idempotence du dépôt dès qu'un fichier de config gagne de nouvelles clés au fil des versions du script (cas de l'`app.ini` de Gitea en #19). Il faut un merge clé par clé, scopé à la section, pas un tout-ou-rien au niveau du fichier. ## Implémentation `ini_set <file> <section> <key> <value>` dans `lib/common.sh`, en bash + awk pur (pas de `crudini`) : - Fichier absent → créé - Section absente → ajoutée en fin de fichier - Clé absente dans la section → insérée dans la section - Clé présente, même valeur → no-op (sortie strictement identique) - Clé présente, valeur différente → remplacée en place - Le matching est scopé à la section courante (via un flag `in_section` remis à jour à chaque en-tête `[...]`), donc une même clé dans deux sections différentes (`ENABLED` dans `[metrics]` et `[actions]`) n'interfère jamais. - Écriture atomique : `mktemp` + `mv`. - Commentaires et lignes vides hors de la ligne modifiée sont préservés tels quels. Documenté dans l'en-tête de `lib/common.sh` comme premier helper OS-agnostique, à côté du contrat Alpine-only existant. ## Idempotence — comment vérifié Testé manuellement (voir les 6 cas du tableau de l'issue) sur un `app.ini` de test : ```bash ini_set app.ini server DOMAIN new.example.com cp app.ini after1.ini ini_set app.ini server DOMAIN new.example.com diff after1.ini app.ini # vide → deux appels identiques laissent le fichier inchangé ``` Vérifié aussi : fichier absent (création), section absente (ajout en fin de fichier), clé absente dans section existante (insertion), clé présente avec valeur différente (remplacement en place sans toucher au reste), indépendance `[metrics] ENABLED` / `[actions] ENABLED`. ## Vérification - `shellcheck lib/common.sh` : aucune nouvelle alerte (SC2148 préexistant, lié à l'absence de shebang sur un fichier sourcé — confirmé identique sur `main` avant ce changement). Closes #18

Damien added 1 commit 2026-07-31 15:35:02 +00:00

feat(lib): add ini_set idempotent INI merge helper ... 66f981af81

openbao's "write config only if absent" pattern would silently skip
newly-required keys on a rejoué script. ini_set() merges key by key
instead: creates file/section/key as needed, replaces an existing
key's value in place, no-ops when already correct, and never touches
other sections — so a duplicate key name across sections (e.g.
[metrics] ENABLED vs [actions] ENABLED) stays scoped correctly.
Atomic write via tmpfile + mv. First OS-agnostic helper in the file,
documented as such in the header contract.

Refs #18

Damien added 1 commit 2026-07-31 16:46:38 +00:00

fix(lib): preserve file mode/ownership across ini_set's atomic write ... 905166d1cc

mktemp defaults to 0600 root:root. The tmpfile+mv swap in ini_set was
carrying that over onto the replaced config, silently locking out
whatever service account owned the original file (e.g. gitea:www-data
on Gitea's app.ini — the service failed to start with a permission
denied on its own config after the very first ini_set call). Restore
the original file's mode and ownership on the tmpfile before the mv.

Verified on both BusyBox (Alpine) and GNU coreutils (Debian) stat -c.

Refs #18

Damien commented 2026-07-31 16:48:27 +00:00

Author

Owner

Copy Link

Copy Source

Découvert en implémentant #19 (gitea/install.sh consomme ini_set sur app.ini, possédé par gitea:www-data) : le mktemp de l'écriture atomique crée le fichier temporaire en 0600 root:root, et le mv faisait donc atterrir ces permissions sur le fichier final — le service Gitea se retrouvait immédiatement à permission denied sur son propre app.ini dès le premier ini_set.

Correctif ajouté (905166d) : stat -c sur le fichier original avant le mv, puis chmod/chown du tmpfile en conséquence. Revérifié :

• Les 6 cas + test d'idempotence, inchangés (diff vide sur appels successifs identiques).
• Permissions/propriétaire préservés, testé à la fois sous Alpine/BusyBox et Debian/coreutils (stat -c se comporte identiquement sur les deux).
• shellcheck toujours sans nouvelle alerte.

Découvert en implémentant #19 (gitea/install.sh consomme `ini_set` sur `app.ini`, possédé par `gitea:www-data`) : le `mktemp` de l'écriture atomique crée le fichier temporaire en `0600 root:root`, et le `mv` faisait donc atterrir ces permissions sur le fichier final — le service Gitea se retrouvait immédiatement à `permission denied` sur son propre `app.ini` dès le premier `ini_set`. Correctif ajouté (905166d) : `stat -c` sur le fichier original avant le `mv`, puis `chmod`/`chown` du tmpfile en conséquence. Revérifié : - Les 6 cas + test d'idempotence, inchangés (`diff` vide sur appels successifs identiques). - Permissions/propriétaire préservés, testé à la fois sous Alpine/BusyBox et Debian/coreutils (`stat -c` se comporte identiquement sur les deux). - `shellcheck` toujours sans nouvelle alerte.

Damien commented 2026-08-01 08:41:35 +00:00

Author

Owner

Copy Link

Copy Source

Bonne implémentation dans l'ensemble : le scoping par section est correct ([metrics] ENABLED ne touche pas [actions] ENABLED), l'insertion avant la section suivante fonctionne, le contrat OS-agnostique est bien documenté dans l'en-tête, et la préservation chmod/chown avant le mv est une bonne prise — mktemp en 0600 root:root aurait effectivement verrouillé l'utilisateur gitea hors de son propre app.ini.

Un point bloquant et un point cosmétique.

Bloquant — le mv s'exécute même si awk a échoué

awk '...' "$file" > "$tmp"
mv "$tmp" "$file"

La redirection crée $tmp avant qu'awk ne réussisse. Si awk meurt en cours de route (OOM, signal, erreur de syntaxe sur une valeur exotique), $tmp est tronqué ou vide — et le mv l'installe quand même par-dessus l'app.ini. On perd la config entière, silencieusement.

L'appelant a set -e, ce qui couvre le cas nominal, mais pas si ini_set est un jour invoqué dans un if, un || ou un $(...) — contextes où set -e est désactivé. Pour une fonction dont le rôle est de protéger un fichier de config, la garde mérite d'être dans la fonction :

if ! awk '...' "$file" > "$tmp"; then
  rm -f "$tmp"
  log_error "ini_set: awk failed on ${file} (${section}.${key}), config left untouched."
  return 1
fi
mv "$tmp" "$file"

Cosmétique — insertion après la ligne vide de fin de section

Sur un app.ini typique :

[metrics]
ENABLED = true

[actions]

l'insertion se fait à la rencontre de [actions], donc après la ligne vide :

[metrics]
ENABLED = true

TOKEN = xyz
[actions]

Sémantiquement correct (la clé reste dans [metrics]), mais après les ~20 appels de configure_app_ini le fichier devient difficile à lire, et les nouvelles clés se collent à l'en-tête de la section suivante. Bufferiser les lignes vides de fin de section et les réémettre après l'insertion réglerait ça.

Note sur le critère d'acceptation « byte-identique »

À vérifier dans le bon ordre : le premier appel normalise l'espacement (KEY=val → KEY = val), donc le diff vide s'observe entre le 1ᵉʳ et le 2ᵉ appel, pas entre l'original et le 1ᵉʳ. C'est le comportement voulu, mais le test doit en tenir compte.

Bonne implémentation dans l'ensemble : le scoping par section est correct (`[metrics] ENABLED` ne touche pas `[actions] ENABLED`), l'insertion avant la section suivante fonctionne, le contrat OS-agnostique est bien documenté dans l'en-tête, et la préservation `chmod`/`chown` avant le `mv` est une bonne prise — `mktemp` en 0600 root:root aurait effectivement verrouillé l'utilisateur `gitea` hors de son propre `app.ini`. Un point bloquant et un point cosmétique. ## Bloquant — le `mv` s'exécute même si `awk` a échoué ```bash awk '...' "$file" > "$tmp" mv "$tmp" "$file" ``` La redirection crée `$tmp` avant qu'`awk` ne réussisse. Si `awk` meurt en cours de route (OOM, signal, erreur de syntaxe sur une valeur exotique), `$tmp` est tronqué ou vide — et le `mv` l'installe quand même par-dessus l'`app.ini`. On perd la config entière, silencieusement. L'appelant a `set -e`, ce qui couvre le cas nominal, mais pas si `ini_set` est un jour invoqué dans un `if`, un `||` ou un `$(...)` — contextes où `set -e` est désactivé. Pour une fonction dont le rôle est de protéger un fichier de config, la garde mérite d'être dans la fonction : ```bash if ! awk '...' "$file" > "$tmp"; then rm -f "$tmp" log_error "ini_set: awk failed on ${file} (${section}.${key}), config left untouched." return 1 fi mv "$tmp" "$file" ``` ## Cosmétique — insertion après la ligne vide de fin de section Sur un `app.ini` typique : ```ini [metrics] ENABLED = true [actions] ``` l'insertion se fait à la rencontre de `[actions]`, donc *après* la ligne vide : ```ini [metrics] ENABLED = true TOKEN = xyz [actions] ``` Sémantiquement correct (la clé reste dans `[metrics]`), mais après les ~20 appels de `configure_app_ini` le fichier devient difficile à lire, et les nouvelles clés se collent à l'en-tête de la section suivante. Bufferiser les lignes vides de fin de section et les réémettre après l'insertion réglerait ça. ## Note sur le critère d'acceptation « byte-identique » À vérifier dans le bon ordre : le premier appel **normalise** l'espacement (`KEY=val` → `KEY = val`), donc le `diff` vide s'observe entre le 1ᵉʳ et le 2ᵉ appel, pas entre l'original et le 1ᵉʳ. C'est le comportement voulu, mais le test doit en tenir compte.

Damien referenced this issue from a commit 2026-08-01 09:18:54 +00:00

fix(lib): ini_set n'écrase plus app.ini si awk échoue, insertion avant les lignes vides

Damien added 1 commit 2026-08-01 09:18:54 +00:00

fix(lib): ini_set n'écrase plus app.ini si awk échoue, insertion avant les lignes vides ... 1217b26ff9

Deux corrections dans ini_set() suite à la review de #24 :

- Le mv s'exécutait même si awk avait échoué en cours de route (OOM,
  signal, valeur exotique) : $tmp, créé par la redirection avant
  qu'awk ne tourne, pouvait être tronqué ou vide et venait quand même
  écraser le fichier original. mv est maintenant gaté sur le code de
  retour d'awk : en cas d'échec, $tmp est supprimé, une erreur est
  logguée, et la fonction retourne 1 sans toucher au fichier d'origine.

- L'insertion d'une clé juste avant l'en-tête de section suivante
  atterrissait après la ou les lignes vides de fin de section plutôt
  qu'avant, rendant le fichier plus difficile à lire au fil des appels
  répétés. Les lignes vides rencontrées section active/clé pas encore
  trouvée sont désormais bufferisées et réémises juste après la clé
  insérée (ou en fin de fichier si la section n'est jamais refermée),
  sans toucher à l'ordre dans tous les autres cas.

Damien commented 2026-08-01 09:19:08 +00:00

Author

Owner

Copy Link

Copy Source

Corrigé (1217b26), les deux points.

mv gaté sur la réussite d'awk

if ! awk ... > "$tmp"; then rm -f "$tmp"; log_error ...; return 1; fi avant le mv. Vérifié en forçant un échec réel d'awk (fichier cible remplacé par un répertoire, awk ne peut pas l'ouvrir) :

$ ini_set not-a-file.ini server KEY val
awk: cmd. line:54: Is a directory
[ERROR] ini_set: awk failed on not-a-file.ini (server.KEY), config left untouched.
$ echo $?
1

• Aucun fichier *.tmp.* résiduel (le rm -f fait son travail).
• Un app.ini valide passé juste après au même ini_set fonctionne normalement (pas de régression sur le chemin nominal).
• Testé sous Alpine/BusyBox et Debian/coreutils.

Insertion avant les lignes vides de fin de section

Les lignes vides rencontrées tant que la section est active et la clé pas encore trouvée sont désormais bufferisées et réémises juste après l'insertion (fonction flush_blanks()), au lieu d'être imprimées immédiatement. Revérifié sur les cas limites :

• Une ligne vide avant la section suivante → la clé s'insère avant, la ligne vide après :

  PROTOCOL = http
  ROOT_URL = https://new.example.com
  
  [metrics]
  

  (avant le correctif : ROOT_URL atterrissait après la ligne vide, collé à [metrics])
• Plusieurs lignes vides consécutives → toutes préservées, dans l'ordre, après la clé insérée.
• Ligne vide au milieu d'une section (clé trouvée après) → comportement inchangé, la ligne vide reste à sa place d'origine (le buffer se vide dès qu'une ligne non-vide arrive, qu'elle corresponde à la clé ou non).
• Section jamais refermée (clé insérée en fin de fichier avec des lignes vides traînantes) → même logique, clé avant les lignes vides.

Sur la note « byte-identique »

Bien vu — le test que j'ai relancé compare maintenant le résultat du 1ᵉʳ appel à celui du 2ᵉ (diff vide), pas l'original au 1ᵉʳ appel (qui normalise l'espacement, comme prévu). Les 6 cas du tableau de l'issue #18 repassent tous, shellcheck lib/common.sh toujours sans nouvelle alerte (SC2148 préexistant).

Corrigé (1217b26), les deux points. ## `mv` gaté sur la réussite d'`awk` `if ! awk ... > "$tmp"; then rm -f "$tmp"; log_error ...; return 1; fi` avant le `mv`. Vérifié en forçant un échec réel d'`awk` (fichier cible remplacé par un répertoire, awk ne peut pas l'ouvrir) : ``` $ ini_set not-a-file.ini server KEY val awk: cmd. line:54: Is a directory [ERROR] ini_set: awk failed on not-a-file.ini (server.KEY), config left untouched. $ echo $? 1 ``` - Aucun fichier `*.tmp.*` résiduel (le `rm -f` fait son travail). - Un `app.ini` valide passé juste après au même `ini_set` fonctionne normalement (pas de régression sur le chemin nominal). - Testé sous Alpine/BusyBox et Debian/coreutils. ## Insertion avant les lignes vides de fin de section Les lignes vides rencontrées tant que la section est active et la clé pas encore trouvée sont désormais bufferisées et réémises juste après l'insertion (fonction `flush_blanks()`), au lieu d'être imprimées immédiatement. Revérifié sur les cas limites : - Une ligne vide avant la section suivante → la clé s'insère avant, la ligne vide après : ``` PROTOCOL = http ROOT_URL = https://new.example.com [metrics] ``` (avant le correctif : `ROOT_URL` atterrissait après la ligne vide, collé à `[metrics]`) - Plusieurs lignes vides consécutives → toutes préservées, dans l'ordre, après la clé insérée. - Ligne vide **au milieu** d'une section (clé trouvée après) → comportement inchangé, la ligne vide reste à sa place d'origine (le buffer se vide dès qu'une ligne non-vide arrive, qu'elle corresponde à la clé ou non). - Section jamais refermée (clé insérée en fin de fichier avec des lignes vides traînantes) → même logique, clé avant les lignes vides. ## Sur la note « byte-identique » Bien vu — le test que j'ai relancé compare maintenant le résultat du 1ᵉʳ appel à celui du 2ᵉ (`diff` vide), pas l'original au 1ᵉʳ appel (qui normalise l'espacement, comme prévu). Les 6 cas du tableau de l'issue #18 repassent tous, `shellcheck lib/common.sh` toujours sans nouvelle alerte (SC2148 préexistant).

Damien closed this pull request 2026-08-01 17:44:07 +00:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

Damien (Damien Arnodo)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Damien/infra-scripts#24