passbolt / passbolt_help

Passbolt help and knowledge base site for the open source password manager for teams!
https://help.passbolt.com
GNU Affero General Public License v3.0
52 stars 183 forks source link

Improvement suggestions for SLES setup instructions #108

Closed dirkmueller closed 2 months ago

dirkmueller commented 2 months ago

Following the instructions at https://passbolt.com/docs/hosting/install/ce/sles/ I have the following suggestions:

avoid use of wget

wget is not a standard installed tool in the minimal system. you can achieve pretty much the same outcome by using curl -O instead, so

-wget https://download.passbolt.com/ce/installer/passbolt-repo-setup.ce.sh
+curl -O https://download.passbolt.com/ce/installer/passbolt-repo-setup.ce.sh

then, the repo-setup script is using which(1) to determine whether zypper or other tools are available. which is sort of deprecated and might not be installed in really very minimal systems. There is an equivalent bash builtin, which would work the same way:

--- passbolt-repo-setup.ce.sh-20240503  2024-05-03 11:46:47.838522592 +0000
+++ passbolt-repo-setup.ce.sh   2024-05-03 11:49:06.370671699 +0000
@@ -141,13 +141,13 @@
       then
           CODENAME="focal"
       fi
-  elif which zypper > /dev/null 2>&1
+  elif test -e "$(type -p zypper)" > /dev/null 2>&1
   then
       PACKAGE_MANAGER=zypper
-  elif which dnf > /dev/null 2>&1
+  elif test -e "$(type -p dnf)" > /dev/null 2>&1
   then
       PACKAGE_MANAGER=dnf
-  elif which yum > /dev/null 2>&1
+  elif test -e "$(type -p yum)" > /dev/null 2>&1
   then
       PACKAGE_MANAGER=yum
   else
@@ -349,4 +349,4 @@
qntoni commented 2 months ago

Hello @dirkmueller, Thanks a lot for your feedback, we really appreciate the time you've taken to help us.

Regarding curl instead of wget, we chose to use curl -LO to make curl follow HTTPS redirects, which is crucial when the target file has been moved to a different location. This is especially important when performing operations like SHA checksum verification, as the correct file must be retrieved for accurate validation. (ref. PB-33239)

With the repo setup script, we chose to use command -v over test -e for checking command availability because command -v is more versatile in verifying all executable types the shell can run, including built-ins, functions, and aliases—not merely executables in the PATH. This ensures a thorough check for command presence. Additionally, using command -v avoids issues like ShellCheck's SC2065 warning, which flags the misuse of redirections in conditional tests where a command check is intended. (ref PB-33238)

Thanks again.

Best regards, Antony

dirkmueller commented 2 months ago

both works for me. I've validated that this works now. thank you for the quick fix!