Data separation, remove editing config ... #250
Conversation
|
Il est nécessaire de changer les messages anglais. |
Josue-T
left a comment
Josue-T
left a comment
There was a problem hiding this comment.
Merci pour la pull request, la partie gestion du repertoire data me paraît bien, par contre pour la partie de gestion des admin, il est vrais qu'en l'état c'est pas top, par contre je suis pas sûr que initialiser des donnée depuis une archive est une bonne solution d'un point de vue sécurité et maintenance long terme (peut être que a un moment donnée les donnée initiale peuvent changer). C'est pour ca que pour la gestion des admin je pense qu'il est mieux d'avoir un config panel. Ca permettra aussi par la suite de rajouter des paramètre de l'app depuis l'admin Yunohost lié a la config.
| */ | ||
| adminKeys: [ | ||
|
|
||
| "[cryptpad-admin@__DOMAIN__/2YOUhk2Fp5t5NmnW-NsN2mfTA65FGWj25KS4NPbcWco=]", |
There was a problem hiding this comment.
Je suis pas sûr que ca soie une bonne idée, de pre-créer un compte. Je préfèrerais garder la solution initiale, ou l'utilisateur crée lui même le compte et ensuite on ajoute dans la configuration les clefs publiques des admins.
There was a problem hiding this comment.
Je ne vois pas ou il y a risque au vu des droits de l'administrateur.
| # BACKUP THE DATA DIRECTORY | ||
| #================================================= | ||
|
|
||
| ynh_backup "$data_dir" |
There was a problem hiding this comment.
Il faut aussi restaurer le backup comme ici: https://github.com/YunoHost/example_ynh/blob/ac765f4e6eaf2600b46efd167adf4b16a637f873/scripts/restore#L28
There was a problem hiding this comment.
Je vais tester plusieurs scénarii et je fais un retour
| #================================================= | ||
| ynh_script_progression "Updating configuration..." | ||
|
|
||
| ynh_config_add --template="config.js" --destination="$install_dir/config/config.js" |
There was a problem hiding this comment.
Il est vrais qu'en l'était c'est pas top car les clefs des admins ne sont pas gardée lors de upgrade. Par contre pour moi il est important de continuer de gérer la config lors de l'upgrade. Je propose donc de créer un config panel pour cette app ou l'on peux saisir depuis l'admin la liste des clefs des admins, ainsi l'administrateur peux gérer cela proprement.
On a de la doc au sujet du config panel ici: https://doc.yunohost.org/fr/packaging/advanced/config_panels/
Et des example (assez complexe) sur l'app d'exemple: https://github.com/YunoHost/example_ynh/tree/main
Là en occurrence c'est juste d'avoir un binding d'un setting d'app par example admin_keys lié à la config et présenté dans le config panel. Pour un example simple de config panel qui fait ce que je pense il y a l'app element qui fait ca.
Pour pouvoir gérer une liste convenablement dans config.js, je recommande d'utiliser des templates jinja. Pour ca il suffit juste de rajouter le paramètre --jinja au helper ynh_config_add.
There was a problem hiding this comment.
L'idée du config panel me semble bonne elle permettrait d'agir sur plusieurs autres paramètres.
Je vais travailler sur le sujet.
| rm -rf "$install_dir/$item" | ||
| fi | ||
| chown -R $app:$app "$data_dir/$item" | ||
| ln -s "$data_dir/$item" "$install_dir/$item" |
There was a problem hiding this comment.
Il est nécessaire de gérer la migration des instances actuelles dans le script d'upgrade, donc déplacer les dossier actuelle, puis créée les symlink et finalement tester que la migration marche. Et idéalement ajouter un test d'upgrade automatique (mette l'ancien commit dans test.toml).
2 participants
Problem
Solution
PR Status
Automatic tests
Automatic tests can be triggered on https://ci-apps-dev.yunohost.org/ after creating the PR, by commenting "!testme", "!gogogadgetoci" or "By the power of systemd, I invoke The Great App CI to test this Pull Request!". (N.B. : for this to work you need to be a member of the Yunohost-Apps organization)