Templating of Server Config & Monitoring #168
Conversation
|
Hi Matt, Thanks a lot for your generous contribution! I'll review ASAP |
|
Ok so I changed my mind.. the |
|
Sure thing. Thanks a lot once again for your contribution |
|
No problem.. my router that I usually run my VPN on started acting up and I got to looking for a simple docker image that I could use and ran across yours... then of course I wanted to make modifications... and well one things leads to another and I might as well push them back up.. |
| CLIENT_ID="$(cat /dev/urandom | tr -dc 'a-zA-Z0-9' | fold -w 32 | head -n 1)" | ||
| ARG1=$1 | ||
| DEFAULT_ID="$(cat /dev/urandom | tr -dc 'a-zA-Z0-9' | fold -w 32 | head -n 1)" | ||
| CLIENT_ID="${ARG1:=$DEFAULT_ID}" |
There was a problem hiding this comment.
I more or less did what was mentioned here. I also modified how start.sh called this function.
| @@ -38,10 +40,14 @@ then | |||
| fi | |||
| ;; | |||
| o) | |||
| CLIENT_PATH="$(createConfig $2)" | |||
There was a problem hiding this comment.
Here and for all the other cases I moved the createConfig call to the cases and passed in the second/third optional argument which is mean to be the cert name. If its not passed the cert name will revert to the random name.
|
The last thing I am going to probably do is add |
|
Hi Matt, I really appreciate your commitment. Like I said I'll have a detailed look when I finalize the feature I'm currently working on. Generally speaking it's always better to raise smaller PRs for incremental improvements, but it's OK to have it as one lump too. I'll then cover with tests here https://github.com/dockovpn/dockovpn-it |
| CLIENT_PATH="$APP_PERSIST_DIR/clients/$CLIENT_ID" | ||
| [ -d $CLIENT_PATH ] && CLIENT_PATH=${CLIENT_PATH}_1 |
There was a problem hiding this comment.
I think if given path already exists it's better to return an error code. Otherwise, we will end up in a situation where we have two certificates with the same subject name (CLIENT_ID is present in generated certificate in subject field), which in its turn may lead to other problems. For instance, when we remove client with rmclient.sh we supply subject name as a parameter and certificate is revoked.
|
|
||
| WORKDIR ${APP_INSTALL_PATH} | ||
|
|
||
| COPY scripts . | ||
| COPY config ./config | ||
| COPY VERSION ./config | ||
|
|
||
| RUN apk add --no-cache openvpn easy-rsa bash netcat-openbsd zip dumb-init && \ | ||
| RUN apk add --no-cache python3 py-pip openvpn easy-rsa bash netcat-openbsd zip dumb-init && \ |
There was a problem hiding this comment.
I thoroughly reviewed the templating part and it's truly amazing! However it brings in another execution environment i.e python and package dependencies which leads to much greater image size. At the moment I'd like to keep resulting image as small as possible for the benefit of embedded and RISK architectures.
I'm still working out some bugs but I wanted to open this PR to make you aware of my modifications. I wanted to be able to modify some of the server configs a little easier and so I modified it to be more like a template. Instead of copying the
server.confto/etc/openvpnat build time, I am doing it as part of thestart.sh. I created variables that have defaults of the original values, and modified thedocker-compose.ymlto demo some of them.Additionally I ran across this project and thought it to be very useful and a simple thing to add to the
docker-compose.yml.If you want me to break these two things out into seperate PRs I can.
I also thought about using something like cookie to template the
server.confbut you're using analpineimage and I didn't really want to muck around trying to get it installed bloating the image more than was actually needed. Instead I just went with a simpleevalmethod that will inject variables into theserver.confI dunno..... thoughts?