Build Prototype Monkey Island AppImage#1069
Conversation
Adds the basic scripts for generating an AppImage. Code changes are required to allow Monkey Island to operate on a read-only filesystem.
1. Rewrote in pytest 2. Removed reduntant tests 3. Added tests for add_user() and get_users()
The `get_from_json()` and `get_from_dict()` static methods were really just used for testing. The `EnvironmentConfig` class needs to store its file path so it can wite to the file if needed. In practical usage, `EnvironmentConfig` objects are initialized from files, so a simpler interface is for its constructor to take a file path.
1. Adjusted some spacing and indentation 2. Reformatted with Black
…a todo, to move it to our fixture list
|
Are there any pre-requisites not mentioned? Or can I literraly just run |
| sudo apt-get install -y nodejs | ||
| } | ||
|
|
||
| install_build_prereqs() { |
There was a problem hiding this comment.
maybe add sudo apt update as well?
There was a problem hiding this comment.
The VM should be upgraded before building. I'll add details in a readme.
There was a problem hiding this comment.
Even if it's docker or something, how do you know that the repositories will be updated? I'd rather we have sudo apt update in scripts than specify in the readme that "you need to run sudo apt update".
| requirements_island="$ISLAND_PATH/requirements.txt" | ||
| # TODO: This is an ugly hack. PyInstaller is a build-time dependency and should | ||
| # not be installed as a runtime requirement. | ||
| sed '4d' $requirements_island | sponge $requirements_island |
There was a problem hiding this comment.
line_with_pyinstaller=$(cat $requirements_island | grep -i -n pyinstaller)
|
|
||
| requirements_island="$ISLAND_PATH/requirements.txt" | ||
| # TODO: This is an ugly hack. PyInstaller is a build-time dependency and should | ||
| # not be installed as a runtime requirement. |
There was a problem hiding this comment.
I think this applies to virtualenv as well
| # | ||
| # Note that appimage-builder replaces the interpreter on the monkey agent binaries | ||
| # when building the AppDir. This is unwanted as the monkey agents may execute in | ||
| # environments where the AppImage isn't loaded. |
There was a problem hiding this comment.
This means that monkeys can only be executed on the island or what?
There was a problem hiding this comment.
Ahh, this is the explanation to why you download the binaries after building the AppDir
There was a problem hiding this comment.
Yep. There are better solutions to this problem, but since we're not using AppImage Builder in the long run, they're not worth investing in.
| Version=1.10 | ||
| Type=Application | ||
| Name=Monkey Island | ||
| Comment=FILL ME IN |
There was a problem hiding this comment.
AppImage Builder doesn't actually use this file, but different packaging methods do. I'll fill this in in round 2 or remove this file.
| @@ -0,0 +1,41 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Can you explain when this file is used and when AppRun is used?
There was a problem hiding this comment.
AppRun is part of the AppImage spec. It's the entry point. AppImage Builder generates its own AppRun from the exec and exec_args fields in monkey_island_builder.yml. ATM, AppRun isn't used, so I'll just remove it.
VakarisZ
left a comment
VakarisZ
left a comment
There was a problem hiding this comment.
Phenomenal quality of bash scripts. Some duplication with linux deployment scripts and maybe that can be resolved in the future.
Left some questions, some arise due to the lack of knowledge about how appimage works, others are general suggestions.
| } | ||
|
|
||
| install_nodejs() { | ||
| NODE_SRC=https://deb.nodesource.com/setup_12.x |
There was a problem hiding this comment.
If you move links like these to the top of the file, it will be easier to fix once they get broken, since you won't have to scroll the whole file. Not worth holding up the PR though
There was a problem hiding this comment.
I like having it localized, as that function is the only place it's used, but I see your point. I'll leave it for now, as this may get reworked anyway in version 2.
| ## Building an AppImage | ||
|
|
||
| 1. Create a clean VM or LXC (not docker!) based on Ubuntu 18.04. | ||
| 1. Update the OS with `sudo apt update && sudo apt upgrade` |
There was a problem hiding this comment.
Any reason why this shouldn't be a part of build script?
There was a problem hiding this comment.
Arguably yes, but in its current form, no. I'll add it to the build script. We can revisit it later.
| 1. Execute `./build_appimage.sh`. This will pull all necessary dependencies | ||
| and build the AppImage. | ||
|
|
||
| NOTE: This script is intended to be run from a clean VM. You can also manually |
There was a problem hiding this comment.
Woulnd't it be easier in terms of integration testing, to NOT require a new machine each time?
There was a problem hiding this comment.
LXCs are very quick to spin up. So are VMs in the right environment. Regardless, the README contains instructions on how to clean up after the script runs so that you don't need to use a fresh environment each time.
VakarisZ
left a comment
VakarisZ
left a comment
There was a problem hiding this comment.
Small comments and questions, but can be solved later/ignored
3 participants
What does this PR do?
Adds a deployment script that builds Monkey Island as a prototype AppImage.
The changes to the python code have already been reviewed in the following PRs:
These changes were rebased on the develop branch.
The only changes that have not been reviewed are the ones in the
deployment_scriptsdirectory.PR Checklist
Testing Checklist