visiblevc / wordpress-starter

A slightly less shitty wordpress development workflow
688 stars 167 forks source link

Require SYS_ADMIN only in DEV context not PROD. #142

Open jmatsushita opened 6 years ago

jmatsushita commented 6 years ago

Hi there,

Thanks a lot for the articles and this repo! I wanted to share that I've been hesitant to upgrade to 0.19.0 because I'm using this container in kubernetes but the required SYS_ADMIN cap is great for development, but not something I want to enable in k8s.

Let me know if this doesn't make sense!

Jun

dsifford commented 6 years ago

You're welcome to fork this at version 0.18 and continue using from there.

The reason why we now require the new caps is because we're mounting the webroot using a bindfs mount. This has several useful implications that you can find described in greater detail here.

I'm not interested in reverting back because the benefits gained by using bindfs far outweigh the risks of adding the cap.

jmatsushita commented 6 years ago

Hi again,

Sorry I didn't mean to suggest reverting. What I've done in my dockerfile is the following:

# This requires cap SYS_ADMIN. Should only be activated in DEV
ONBUILD ARG PROD=${PROD:-"true"}
ONBUILD ENV PROD=${PROD}
ONBUILD RUN if ${PROD} -eq "true"; then \
      echo "Not a development environment, building with symlink" && \
      sudo mkdir -p /app \
      && sudo rm -fr /var/www/html \
      && sudo chown admin:admin -R /app \
      && sudo chmod 0766 -R /app \
      && sudo chmod a+X /app \
      && sudo ln -s /app /var/www/html \
      && sudo chown apache:admin -h /var/www/html; \
    else \
      echo "Development environment, building with bindfs" && \
      printf '%s\t%s\t%s\t%s%s%s%s%s%s%s%s\t%d\t%d\n' \
      '/app' \
      '/var/www/html' \
      'fuse.bindfs' \
          'force-user=apache,' \
          'force-group=apache,' \
          'create-for-user=admin,' \
          'create-for-group=admin,' \
          'create-with-perms=0644:a+X,' \
          'chgrp-ignore,' \
          'chown-ignore,' \
          'chmod-ignore' \
      0 \
      0 | sudo tee -a /etc/fstab; \
    fi

You might not need the ONBUILD (which I use to configure dependent images). This takes a build arg PROD which is set to false in the docker-compose file. This means that anyone using docker-compose to manage their container will have the bindfs feature, but when the container is built elsewhere (for me in CI) then bindfs is not activated and instead the older method of symlinking is used (the permissions are probably too lax right now in my code snippet). This ensures that when deployed in production (in kubernetes for me) SYS_ADMIN or privileged is not needed, which is a big no no in production clusters, but in development there is no permissions headaches thanks to your bindfs solution.

Hope this makes it clearer. By the way, indeed I'm already using a fork, but gladly now of 0.20.0 if only because I use CentOS. In fact, I'll temporarily hijack this issue thread to also ask if you'd be interested in a PR for adding CentOS Dockerfile.

Cheers,

Jun

dsifford commented 6 years ago

SYS_ADMIN or privileged is not needed, which is a big no no in production clusters

Can you expand on this? (the SYS_ADMIN part, privileged I understand)

Also, having the conditional in the Dockerfile is not going to be possible because it will require everyone who uses this project to build from source, which I think most are not doing currently.