Added optional parameter for deployment script #570
Conversation
deployment_scripts/deploy_linux.sh
Outdated
| if exists wget; then | ||
| wget -c -N -P "${MONKEY_BIN_DIR}" ${SAMBACRY_64_BINARY_URL} | ||
| wget -c -N -P "${MONKEY_BIN_DIR}" ${SAMBACRY_32_BINARY_URL} | ||
| read -p "Do you want to use the latest binaries, Press Y, otherwise specify the version to downloaded " Resp |
There was a problem hiding this comment.
This should be a command line parameter, not interactive.
|
Code looks decent. It's important to note usability wise, this should be a command line parameter and not intereactive. |
deployment_scripts/deploy_linux.sh
Outdated
| tlink_64='https://github.com/guardicore/monkey/releases/download/' | ||
| tlink_64+=$Resp | ||
| tlink_64+='/traceroute64' |
There was a problem hiding this comment.
You can do this in a cleaner way using
tlink_64="https://github.com/guardicore/monkey/releases/download/$resp/traceroute64"
Do the same for all the other things?
There was a problem hiding this comment.
Ok then, I will rectify it..
and the command line parameter..
|
@danielguardicore Can you review now? |
deployment_scripts/deploy_linux.sh
Outdated
| wget -c -N -P "${MONKEY_BIN_DIR}" ${SAMBACRY_64_BINARY_URL} | ||
| wget -c -N -P "${MONKEY_BIN_DIR}" ${SAMBACRY_32_BINARY_URL} | ||
|
|
||
| if [ $# = 0 ] ; then |
There was a problem hiding this comment.
Why not explicitly check what the argument is?
deployment_scripts/deploy_linux.sh
Outdated
| curl -o ${MONKEY_BIN_DIR}/sc_monkey_runner64.so ${SAMBACRY_64_BINARY_URL} | ||
| curl -o ${MONKEY_BIN_DIR}/sc_monkey_runner32.so ${SAMBACRY_32_BINARY_URL} | ||
| log_message "Downloading sambacry binaries" | ||
| slink_64="https://github.com/guardicore/monkey/releases/download/$1/sc_monkey_runner64.so" |
There was a problem hiding this comment.
Why are you downloading using the first argument and not a variable?
|
@ashish493 could you better explain what the command line interface is now, for both powershell and bash? |
|
At present, Same logic goes for the powershell Script, If no arguments, download latest, In both the cases, I'm using only the first argument (as a temporary solution) for downloading, I'm still finding for a way around it, as it may cause some security concerns.. |
|
Can you add to the PR the updated docs? |
|
@danielguardicore Can u review now? |
Codecov Report
@@ Coverage Diff @@
## develop #570 +/- ##
==========================================
Coverage ? 56.15%
==========================================
Files ? 105
Lines ? 3588
Branches ? 0
==========================================
Hits ? 2015
Misses ? 1573
Partials ? 0 Continue to review full report at Codecov.
|
deployment_scripts/README.md
Outdated
| - `./deploy_linux.sh "" "" "v1.7.1" ` (If you want to download any specific version of the releases, Provide the version in the 3rd argument, Else by Default, it will download the latest release of binaries) |
There was a problem hiding this comment.
lowercase p, e and D, or change the grammer.
| @@ -8,6 +8,9 @@ param( | |||
| [Parameter(Mandatory = $false, Position = 2)] | |||
| [Bool] | |||
| $agents = $true | |||
| [Parameter(Mandatory = $false, Position = 3)] | |||
| [System.String] | |||
| $version = "v1.7.0", | |||
There was a problem hiding this comment.
Why 1.7? Why are we not querying for latest release?
There was a problem hiding this comment.
I made the changes in the config file to fetch the latest, although "v1.7.0" isn't a parameter, I have just set it to define the argument type. And when we specifically provide the version argument, it will then only download that particular version. Otherwise it's going to download the latest.
Fixes for the issue #552
Added the optional parameter for the linux deployment scripts and modified the Url's in the config file for the latest relese without using GH API's.
I will be modifying the windows deployment script soon, but it's config file need a lot of changes which will take some time.