Votre avis sur un script bash

**nenex73** · 29/06/2020, 12h33

Bonjour à tous,

J'essaye de faire un script pour configurer cron-apt.
Je débute en bash alors il y a sans doute plein de choses imparfaites/qui m'échappent.
N'hésitez pas à m'en faire part car je préfère tout de suite prendre les bonnes habitudes

Merci d'avance.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
#!/bin/sh
 
# Script vars 
actiondir="/etc/cron-apt/action.d"
preferencesfile="/etc/cron-apt/config.d/preferences"
configfile="/etc/cron-apt/config"
 
# Menage
echo "Suppression des anciennes configurations"
find ${actiondir} -type f | xargs rm -f
if [ -f ${preferencesfile} ]; then rm -f ${preferencesfile} ; fi
echo "" > ${configfile}
 
# Debut du programme
echo "Merci de specifier le type de profile à installer (full / security)"
read profile
 
# En fonction du profil choisi, on applique telle ou telle conf
case $profile in
    "full")
        echo "Creation de la regle full"
        # Creation de la règle full
        echo "autoclean -y" > ${actiondir}/0-FullProfile
        echo "dist-upgrade -y -o Dpkg::Options::=--force-confdef -o Dpkg::Options::=--force-confold -o APT::Get::Show-Upgraded=true" >> ${actiondir}/0-FullProfile
        echo "autoremove --purge" >> ${actiondir}/0-FullProfile
 
        echo "Update du fichier config pour prendre en compte le ACTIONDIR"
        # Modification du fichier config pour prise en compte de ce param
    ;;
    "security")
        echo "Blocage des paquets kernel tagues *linux-image*"
        # On bloque les packages kernel pour pas les upgrade
        echo "Package: dovecot* galera* glusterfs* haproxy* heartbeat* libc6* mariadb* mysql* postfix* proxmox* pve-* qemu-* linux-headers* linux-image* linux-libc*" > ${preferencesfile}
        echo "Pin: release *" >> ${preferencesfile}
        echo "Pin-Priority: -1" >> ${preferencesfile}
 
        echo "Creation de la source list dediee securite"
        grep -i security /etc/apt/sources.list > /etc/apt/sources.list.d/security.sources.list
 
        echo "Menage dans /etc/apt/sources.list"
        sed -i "/security/d" /etc/apt/sources.list
 
        echo "Creation de la regle security"
        # Creation de la règle security
        echo "autoclean -y" > ${actiondir}/0-SecurityProfile
        echo "upgrade -y -o Dpkg::Options::=--force-confdef -o Dpkg::Options::=--force-confold -o APT::Get::Show-Upgraded=true -o Dir::Etc::SourceList=/etc/apt/security.sources.list" >> ${actiondir}/0-SecurityProfile
        echo "upgrade -y telegraf grafana" >> ${actiondir}/0-SecurityProfile
        echo "autoremove --purge" >> ${actiondir}/0-SecurityProfile
    ;;
    *)
        echo "Parametre incorrect, vous devez entrer soit full soit security"
        exit 1
    ;;
esac
 
# Configuration globale
# Ajout d'apt en commande de reference
# Ajout des preferences d envoi de mail
echo "PTCOMMAND=/usr/bin/apt" >> ${configfile}
echo "MAILTO=\"***\"" >> ${configfile}
echo "MAILON=\"upgrade\"" >> ${configfile}
echo "RUNSLEEP=600" >> ${configfile}

**disedorgue** · 29/06/2020, 17h42

Rapidement:
-Le shebang est en ligne 2, or celui-ci doit obligatoirement être en ligne 1
-Si script bash alors on indique bash et non sh dans le shebang
-Avec rm -f il est inutile de vérifier au préalable l'existence d'un fichier.
-echo "" >fichier peut être remplacé par >fichier.
-Ici, ce n'est pas important, mais lorsque l'on traite des fichiers de grande capacité, on évite de faire plusieurs echo consécutif dessus quand un seul suffit (ou alors on passe par un descripteur de fichier pour éviter que le shell fasse autant d'ouverture/fermeture de fichier qu'il y a d'echo dessus)

**Flodelarab** · 29/06/2020, 18h03

Bonjour

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

find ${actiondir} -type f | xargs rm -f

Ah ! Une re-mouture de rm -rf *. Le jour où $actiondir sera égal à /, les larmes vont couler.

Sinon :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
$ shellcheck myscript
 
Line 10:
find ${actiondir} -type f | xargs rm -f
^-- SC2038: Use -print0/-0 or -exec + to allow for non-alphanumeric filenames.
 
Line 16:
read profile
^-- SC2162: read without -r will mangle backslashes.
 
Line 59:
echo "PTCOMMAND=/usr/bin/apt" >> ${configfile}
^-- SC2129: Consider using { cmd1; cmd2; } >> file instead of individual redirects.
 
$

Source : https://www.shellcheck.net/

**Jipété** · 29/06/2020, 18h29

Hello,

Envoyé par Flodelarab

Source : https://www.shellcheck.net/

Ah tiens, je ne connaissais pas du tout, et ça a l'air merveilleux, magnifique, toussa toussa, sauf que j'ai testé comme Flodelarab :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
#!/bin/bash
actiondir="/etc/cron-apt/action.d"
find ${actiondir} -type f | xargs rm -f

et l'outil m'a donc dit :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

^-- SC2038: Use -print0/-0 or -exec + to allow for non-alphanumeric filenames.

J'ai corrigé en

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

find -print0 ${actiondir} -type f | xargs rm -f

et j'ai eu droit à

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

^-- SC2185: Some finds don't have a default path. Specify '.' explicitly.

alors j'ai testé l'autre suggestion

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

find -0 ${actiondir} -type f | xargs rm -f

et là j'ai gagné

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

No issues detected!

Tout comme avec

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
find . -print0 ${actiondir} -type f | xargs rm -f
No issues detected!

Il y aurait donc une petite différence entre -print0 et -0 ?
-0 se comporterait comme . -print0…

**nenex73** · 29/06/2020, 19h28

Merci à tous les 2, sympa de m'aider.

Voici la nouvelle version de mon script :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
#!/bin/bash
 
# Script vars 
actiondir="/etc/cron-apt/action.d"
preferencesfile="/etc/cron-apt/config.d/preferences"
configfile="/etc/cron-apt/config"
 
# Menage
echo "Suppression des anciennes configurations"
find -0 ${actiondir} -type f | xargs rm -f
if [ -f ${preferencesfile} ]; then rm -f ${preferencesfile} ; fi
> ${configfile}
 
# Debut du programme
echo "Merci de specifier le type de profile à installer (full / security)"
read profile
 
# En fonction du profil choisi, on applique telle ou telle conf
case $profile in
    "full")
        echo "Creation de la regle full"
        # Creation de la règle full
        echo "autoclean -y" > ${actiondir}/0-FullProfile
        echo "dist-upgrade -y -o Dpkg::Options::=--force-confdef -o Dpkg::Options::=--force-confold -o APT::Get::Show-Upgraded=true" >> ${actiondir}/0-FullProfile
        echo "autoremove --purge" >> ${actiondir}/0-FullProfile
 
        echo "Update du fichier config pour prendre en compte le ACTIONDIR"
        # Modification du fichier config pour prise en compte de ce param
    ;;
    "security")
        echo "Blocage des paquets kernel tagues *linux-image*"
        # On bloque les packages kernel pour pas les upgrade
        echo "Package: dovecot* galera* glusterfs* haproxy* heartbeat* libc6* mariadb* mysql* postfix* proxmox* pve-* qemu-* linux-headers* linux-image* linux-libc*" > ${preferencesfile}
        echo "Pin: release *" >> ${preferencesfile}
        echo "Pin-Priority: -1" >> ${preferencesfile}
 
        echo "Creation de la source list dediee securite"
        grep -i security /etc/apt/sources.list > /etc/apt/sources.list.d/security.sources.list
 
        echo "Menage dans /etc/apt/sources.list"
        sed -i "/security/d" /etc/apt/sources.list
 
        echo "Creation de la regle security"
        # Creation de la règle security
        echo "autoclean -y" > ${actiondir}/0-SecurityProfile
        echo "upgrade -y -o Dpkg::Options::=--force-confdef -o Dpkg::Options::=--force-confold -o APT::Get::Show-Upgraded=true -o Dir::Etc::SourceList=/etc/apt/security.sources.list" >> ${actiondir}/0-SecurityProfile
        echo "upgrade -y telegraf grafana" >> ${actiondir}/0-SecurityProfile
        echo "autoremove --purge" >> ${actiondir}/0-SecurityProfile
    ;;
    *)
        echo "Parametre incorrect, vous devez entrer soit full soit security"
        exit 1
    ;;
esac
 
# Configuration globale
# Ajout d'apt en commande de reference
# Ajout des preferences d envoi de mail
echo "PTCOMMAND=/usr/bin/apt" >> ${configfile}
echo "MAILTO=\"sysadmin@antesis.team\"" >> ${configfile}
echo "MAILON=\"upgrade\"" >> ${configfile}
echo "RUNSLEEP=600" >> ${configfile}

**Flodelarab** · 29/06/2020, 19h52

Une fois passée la joie d'avoir des réponses, il faut les lire, les comprendre et agir.
Ton prétendu "nouveau" script contient toutes erreurs pointées par disedorgue et moi.

**nenex73** · 29/06/2020, 20h05

Ah m...

Moi qui croyais avoir fait le job !
Donc mes modifications lignes 10 et 12 ne suffisent pas !

OK vu une de plus en ligne 11 :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
if [ -f ${preferencesfile} ]; then rm -f ${preferencesfile} ; fi
devient
rm -f ${preferencesfile}#car pas nécessaire de tester l'existence du fichier préalablement

Après, je débute en bash.
Et je ne sais pas si tu as glissé un message subliminale dans "on évite de faire plusieurs echo consécutif dessus quand un seul suffit" ?
Si oui, je suis désolé mais je n'avais pas compris

Et donc :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
echo "PTCOMMAND=/usr/bin/apt" >> ${configfile}
echo "MAILTO=\"sysadmin@antesis.team\"" >> ${configfile}
echo "MAILON=\"upgrade\"" >> ${configfile}
echo "RUNSLEEP=600" >> ${configfile}
devient
echo { "PTCOMMAND=/usr/bin/apt"; MAILTO=\"sysadmin@antesis.team\"; "MAILON=\"upgrade\""; "RUNSLEEP=600" } >> ${configfile}

C'est ça ?

**N_BaH** · 29/06/2020, 21h07

les accolades sont inutiles autour des Développements de paramètres, mais pas les guillemets !

**disedorgue** · 29/06/2020, 21h40

Pas vraiment, la syntaxe sur un seul echo est du genre:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
echo "PTCOMMAND=/usr/bin/apt
MAILTO=\"sysadmin@antesis.team\"
MAILON=\"upgrade\"
RUNSLEEP=600" >fichier

et sinon, via un descripteur de fichier, tu as la version avec un choix spécifique de ta part du numéro de descripteur de fichier mais dans ce cas, c'est à toi de savoir si celui-ci n'est pas déjà utilisé (sachant que par défaut tu as stdin sur le 0, stdout sur le 1 et stderr sur le 2, donc tu peux utiliser le 3 pour un de tes fichiers, ce qui donnerait:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
exec 3>fichier #ouverture du fichier avec remise à zéro de celui-ci
echo "PTCOMMAND=/usr/bin/apt" >&3
echo "MAILTO=\"sysadmin@antesis.team\"" >&3
exec 3>&-  #fermeture du fichier

Sinon, tu as aussi la méthode où c'est le shell qui choisi tous seul le numéro (donc plus pratique) :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
exec {fd1}>fichier #ouverture du fichier avec remise à zéro de celui-ci
echo "PTCOMMAND=/usr/bin/apt" >&${fd1}
echo "MAILTO=\"sysadmin@antesis.team\"" >&${fd1}
exec {fd1}>&-  #fermeture du fichier

Attention, comme on peut le voir, dans le cas de l'ouverture et de la fermeture, on ne précise pas le '$' car on ne spécifie pas la valeur mais la variable elle même, par contre pour écrire dans le fichier, on précise la valeur de la variable, d'où le '$'.

**N_BaH** · 29/06/2020, 22h25

plus simple : un simple here-doc

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
cat <<MARK >fichier
bla
bla
MARK

**Flodelarab** · 30/06/2020, 00h13

Envoyé par nenex73

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
echo "PTCOMMAND=/usr/bin/apt" >> ${configfile}
echo "MAILTO=\"sysadmin@antesis.team\"" >> ${configfile}
echo "MAILON=\"upgrade\"" >> ${configfile}
echo "RUNSLEEP=600" >> ${configfile}
devient
echo { "PTCOMMAND=/usr/bin/apt"; MAILTO=\"sysadmin@antesis.team\"; "MAILON=\"upgrade\""; "RUNSLEEP=600" } >> ${configfile}

C'est ça ?

Cela devient plutôt ceci, vis-à-vis du regroupement de commandes :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
{ echo "PTCOMMAND=/usr/bin/apt" 
echo "MAILTO=\"sysadmin@antesis.team\"" 
echo "MAILON=\"upgrade\"" 
echo "RUNSLEEP=600"; } >> ${configfile}

Mais comme il a été dit, les "echo"s successifs sont maladroits. Il faut passer par un seul echo, ou un fichier texte externe, ou un here-doc.

Il reste la destruction automatique et récursive de toute vie dans le dossier $actiondir. Cela est dangereux et inarrêtable. Es-tu vraiment sure de vouloir cela ?

**nenex73** · 30/06/2020, 16h39

Envoyé par Flodelarab

Il reste la destruction automatique et récursive de toute vie dans le dossier $actiondir. Cela est dangereux et inarrêtable. Es-tu vraiment sure de vouloir cela ?

Je partage tes craintes mais comment faire autrement ?

Avec vos préconisations, voici la nouvelle version du script :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
#!/bin/bash
 
# Script vars 
actiondir="/etc/cron-apt/action.d"
preferencesfile="/etc/cron-apt/config.d/preferences"
configfile="/etc/cron-apt/config"
 
# Menage
echo "Suppression des anciennes configurations"
find -0 ${actiondir} -type f | xargs rm -f
if [ -f ${preferencesfile} ]; then rm -f ${preferencesfile} ; fi
 
# Debut du programme
echo "Merci de specifier le type de profile à installer (full / security)"
read profile
 
# En fonction du profil choisi, on applique telle ou telle conf
case $profile in
    "full")
        echo "Creation de la regle full"
        # Creation de la règle full
        echo "autoclean -y" > ${actiondir}/0-FullProfile
        echo "dist-upgrade -y -o Dpkg::Options::=--force-confdef -o Dpkg::Options::=--force-confold -o APT::Get::Show-Upgraded=true" >> ${actiondir}/0-FullProfile
        echo "autoremove --purge" >> ${actiondir}/0-FullProfile
 
        echo "Update du fichier config pour prendre en compte le ACTIONDIR"
        # Modification du fichier config pour prise en compte de ce param
    ;;
    "security")
        echo "Blocage des paquets kernel tagues *linux-image*"
        # On bloque les packages kernel pour pas les upgrade
        echo "Package: dovecot* galera* glusterfs* haproxy* heartbeat* libc6* mariadb* mysql* postfix* proxmox* pve-* qemu-* linux-headers* linux-image* linux-libc*" > ${preferencesfile}
        echo "Pin: release *" >> ${preferencesfile}
        echo "Pin-Priority: -1" >> ${preferencesfile}
 
        echo "Creation de la source list dediee securite"
        grep -i security /etc/apt/sources.list > /etc/apt/sources.list.d/security.sources.list
 
        echo "Menage dans /etc/apt/sources.list"
        sed -i "/security/d" /etc/apt/sources.list
 
        echo "Creation de la regle security"
        # Creation de la règle security
        echo "autoclean -y" > ${actiondir}/0-SecurityProfile
        echo "upgrade -y -o Dpkg::Options::=--force-confdef -o Dpkg::Options::=--force-confold -o APT::Get::Show-Upgraded=true -o Dir::Etc::SourceList=/etc/apt/security.sources.list" >> ${actiondir}/0-SecurityProfile
        echo "upgrade -y telegraf grafana" >> ${actiondir}/0-SecurityProfile
        echo "autoremove --purge" >> ${actiondir}/0-SecurityProfile
    ;;
    *)
        echo "Parametre incorrect, vous devez entrer soit full soit security"
        exit 1
    ;;
esac
 
# Configuration globale
# Ajout d'apt en commande de reference
# Ajout des preferences d envoi de mail
echo "PTCOMMAND=/usr/bin/apt
MAILTO=\"sysadmin@antesis.team\"
MAILON=\"upgrade\"
RUNSLEEP=600" > ${configfile}

**N_BaH** · 30/06/2020, 17h16

les accolades !
les guillemets !

**nenex73** · 30/06/2020, 17h21

Envoyé par N_BaH

les accolades !
les guillemets !

T'es pas clair !
Tu parles des "echo"s du bas ?

**Flodelarab** · 30/06/2020, 17h37

Combien de redirection dans "${actiondir}/0-SecurityProfile" ? Combien dans "${preferencesfile}" ? etc. Combien d'echo ?

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

sed -i "/security/d" /etc/apt/sources.list

Et y a jamais de apt-update ? ça marche, ça ?

comment faire autrement ?

En supprimant la variabilité malsaine. Pourquoi tous les fichiers ? Pourquoi un dossier variable ? Si tu mets un dossier fixe, ou si tu pointes des fichiers particuliers, tu es tout de suite plus précautionneux.

**N_BaH** · 30/06/2020, 17h59

je parle des Développements de paramètres, en général.
pas d'accolades inutiles,
et des guillemets, toujours.
partout.

parce que, par exemple test -n $nExistePas retourne toujours vrai, quand $nExistePas n'est pas définie, car test teste la seule chaîne présente (-n), qui n'est plus considérée comme une option.
il y a de nombreux autres cas dans lesquels l'absence de guillemets est problématique.

donc :

on met toujours les Développements de paramètres entre guillemets.
TOUJOURS !
à moins de savoir pourquoi.

**nenex73** · 30/06/2020, 19h37

Envoyé par Flodelarab

Combien de redirection dans "${actiondir}/0-SecurityProfile" ? Combien dans "${preferencesfile}" ? etc. Combien d'echo ?

Désolé mais qu'entends-tu par redirection ?

Envoyé par Flodelarab

Et y a jamais de apt-update ? ça marche, ça ?

Très juste !
Ca marche quand même mais je pense que c'est grâce à apticron.
Il faut que je résolve ça.

Envoyé par Flodelarab

En supprimant la variabilité malsaine. Pourquoi tous les fichiers ? Pourquoi un dossier variable ? Si tu mets un dossier fixe, ou si tu pointes des fichiers particuliers, tu es tout de suite plus précautionneux.

Le truc c'est que j'utilise ce dossier à plusieurs endroits donc le déclarer en variable est plus que nécessaire me semble-t-il ?
Et puis variable ou pas, rien n'empêchera le big finger