-
Notifications
You must be signed in to change notification settings - Fork 99
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Removing root requirement for download option from Wazuh installation assistant and other improvements #1465
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please review the requested changes
|
||
curl -so ${DEST_PATH}/wazuh-filebeat-module.tar.gz ${BASE_URL}/filebeat/wazuh-filebeat-0.1.tar.gz | ||
eval "chmod 500 ${BASE_DEST_FOLDER}" | ||
|
||
common_logger "The Configuration Files are in ${DEST_PATH}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why uppercase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done: c77ab55
if [ "$EUID" -eq 0 ]; then | ||
if [ -z "${debugLogger}" ] || ( [ -n "${debugLogger}" ] && [ -n "${debugEnabled}" ] ); then | ||
printf "${now} ${mtype} ${message}\n" | tee -a ${logfile} | ||
fi | ||
else | ||
if [ -z "${debugLogger}" ] || ( [ -n "${debugLogger}" ] && [ -n "${debugEnabled}" ] ); then | ||
printf "${now} ${mtype} ${message}\n" | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code can be simplified by:
if [ -z "${debugLogger}" ] || ( [ -n "${debugLogger}" ] && [ -n "${debugEnabled}" ] ); then
if [ "$EUID" -eq 0 ]; then
printf "${now} ${mtype} ${message}\n" | tee -a ${logfile}
else
printf "${now} ${mtype} ${message}\n"
fi
fi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done: c77ab55
@@ -85,13 +83,15 @@ function main() { | |||
case "${1}" in | |||
"-a"|"--all-in-one") | |||
AIO=1 | |||
common_checkRoot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of repeat common_checkRoot
make a function with a loop that finds the option -dw
inside the arguments array. If it doesn't exist, then make the common_checkRoot
call.
This option has to be attached with a flag check: -V
, -h
has to be used alone, and probably -dw
too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done: c77ab55
@@ -35,18 +35,18 @@ debug=">> ${logfile} 2>&1" | |||
## Offline Installation vars | |||
readonly BASE_DEST_FOLDER="wazuh-offline" | |||
readonly WAZUH_DEB_BASE_URL="${BASE_URL}/apt/pool/main/w/wazuh-manager" | |||
readonly WAZUH_DEB_PACKAGES=( "wazuh-manager_${wazuh_version}-${wazuh_revision}_amd64.deb" ) | |||
readonly WAZUH_DEB_PACKAGES="wazuh-manager_${wazuh_version}-${wazuh_revision}_amd64.deb" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead WAZUH_DEB_PACKAGES it should be MANAGER_DEB_PACKAGE, right?
Also, it could be a good idea to remove the plural of variable names that only have one element.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done: c77ab55
New tests:
|
New Tests:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
Logs example
root tests:
Tests
Ubuntu: https://devel.ci.wazuh.info/view/Tests/job/Test_unattended/523/console
Centos: https://devel.ci.wazuh.info/view/Tests/job/Test_unattended/526/console
Distributed: https://devel.ci.wazuh.info/view/Tests/job/Test_unattended_distributed/442/console