Skip to content
This repository has been archived by the owner on Aug 29, 2022. It is now read-only.

added new version of Dockerfile and convenience script #2

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 36 additions & 26 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,26 +1,36 @@
FROM php:7.4

# Install npm
# libzip-dev: for the PHP zip extension (used by Composer)
RUN curl -sL https://deb.nodesource.com/setup_12.x | bash - \
&& apt update && apt install -y git ssh apache2-utils nodejs python3-pip libzip-dev zip && rm -rf /var/lib/apt/lists/*

# Install Composer
RUN curl -sS https://getcomposer.org/installer | php \
&& mv composer.phar /usr/local/bin/composer \
&& chmod +x /usr/local/bin/composer

# Install the `aws` CLI tool
RUN pip3 install --upgrade --user awscli && echo 'export PATH=/root/.local/bin:$PATH'>/root/.bashrc

# Install serverless
RUN npm install -g serverless

RUN docker-php-ext-install zip pdo_mysql

RUN mkdir -p /var/task

# Register the Serverless and AWS bin directories
ENV PATH="/root/.serverless/bin:/root/.local/bin:${PATH}"

WORKDIR '/var/task'
FROM composer:1.10.4
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the start I chose to use Alpine-based image for the following reasons:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
FROM composer:1.10.4
# This image is based on Linux Alpine, a lighter image than debian/ubuntu
FROM composer:1.10.4

By the way, why not use the latest version?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the comment really necessary? I mean yes, it's smaller than something else, but we could pick many other facts. Should we state them too? And how would we know which ones?

And for the latest. You really shouldn't use it when you need to do something that depends on the system version. For example we know that Alpine 3.11 has docker=19.03.5-r0 in repository, but what will the latest a month from now have?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the comment really necessary? I mean yes, it's smaller than something else, but we could pick many other facts. Should we state them too? And how would we know which ones?

This comment is to make my life, as maintainer, easier. In 1 year, I will stumble on that code and not remember why we chose that image over a classic linux distribution.

And for the latest. You really shouldn't use it when you need to do something that depends on the system version. For example we know that Alpine 3.11 has docker=19.03.5-r0 in repository, but what will the latest a month from now have?

Good point.

So does that mean that I will have to regularly update the version here, and then update the Docker version below too? How can I find the Docker version for the Alpine version that is referenced here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the latest through again and I think we can use it in this situation eventually. The only things we might require specific versions of are only serverless and awscli, and maybe even those not. So if we leave out specifying package versions we can have the base system as latest.
I'll push the changes.

BTW, you can find the available packages per Alpine release at https://pkgs.alpinelinux.org/packages?name=docker&branch=v3.11&arch=x86_64


# composer:1.10.4
# https://github.com/composer/docker/blob/master/1.10/Dockerfile
# based on php:7-alpine
# https://github.com/docker-library/php/blob/master/7.4/alpine3.11/cli/Dockerfile
# based on alpine:3.11

# Package Dependencies:
# serverless
# docker
# awscli
# groff
mnapoli marked this conversation as resolved.
Show resolved Hide resolved
# hirak/prestissimo: accelerates installing Composer dependencies
# yarn: npm alternative
# groff: needed by the AWS cli
RUN adduser -D -G users -h /home/app app \
meridius marked this conversation as resolved.
Show resolved Hide resolved
&& composer global require hirak/prestissimo \
mnapoli marked this conversation as resolved.
Show resolved Hide resolved
&& apk add --no-cache \
docker=19.03.5-r0 \
mnapoli marked this conversation as resolved.
Show resolved Hide resolved
yarn=1.19.2-r0 \
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yarn = better npm ;)
Also necessary for installing serverless.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All Bref docs mention npm, could we install it? (we can also have yarn, why not)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the docs mention it exactly 3 times:

But I guess we can have npm too, if you insist. I don't really mind.

groff=1.22.4-r0 \
mnapoli marked this conversation as resolved.
Show resolved Hide resolved
&& yarn global add [email protected] \
mnapoli marked this conversation as resolved.
Show resolved Hide resolved
&& pip3 install \
--progress-bar off \
--no-cache-dir \
--disable-pip-version-check \
awscli==1.18.39
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Installing awscli as in the original file, except with different pip3 options so the resulting image is smaller (--no-cache-dir), build quicker (--disable-pip-version-check) and output void of unnecessary stuff (--progress-bar off).

Original --upgrade is not necessary since we don't want to deal with its upgrade anyway.
Original --user is nor necessary since we're still root.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 thanks

Could you add this information into a comment?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
&& pip3 install \
--progress-bar off \
--no-cache-dir \
--disable-pip-version-check \
awscli==1.18.39
&& pip3 install \
--progress-bar off \
--no-cache-dir \
--disable-pip-version-check \
awscli==1.18.39
##
# pip3 install options explained:
# --no-cache-dir makes resulting image is smaller
# --disable-pip-version-check makes build quicker
# --progress-bar off makes output cleaner
# --upgrade is not necessary since we don't want to deal with its upgrade anyway
# --user is not necessary since we're still root
##

However I'm against explaining options since user should be able to read it easily from their names (that's why i don't use short variants, if at all possible) and if not they should read docs of given program where more information and related info could be.


USER app
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned above, I'm not sure if this is necessary, but it works.
I'm for removing both lines (create and activate of user) if it proves that there is no need for that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the bit I'm cautious about. If we can do without that would be great, the last thing I would want is debugging these kind of issues 😄

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • TODO: try to not create the app user

ENV PATH=$PATH:/tmp/vendor/bin \
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

COMPOSER_HOME is originally set to /tmp so we have to add its vendor/bin location to the PATH of user to be able to use installed packages.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure which packages you have in mind?

Do you have an example of a use case?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might be for Prestissimo to work.

COMPOSER_HOME=/var/task/.composer
VOLUME /var/task
VOLUME /home/app/.aws
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The location to AWS settings directory. Located under created user, but it also "might" work under /root/.aws.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • TODO: try if it works without app user created

VOLUME /var/run/docker.sock
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docker socket file location necessary for successfully working Docker in Docker.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
VOLUME /var/run/docker.sock
# Change the docker socket file location for successfully running Docker in Docker.
VOLUME /var/run/docker.sock

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
VOLUME /var/run/docker.sock
# Mount the Docker socket file from host for successfully running Docker in Docker.
VOLUME /var/run/docker.sock

WORKDIR /var/task
20 changes: 16 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,14 +1,26 @@
This repository provides a development environment for Bref via a Docker image.
This repository provides a development environment for Bref via single Docker image and convenience script.
mnapoli marked this conversation as resolved.
Show resolved Hide resolved

Thanks to that image you can easily get started with Bref without having to install its tools: `serverless`, `aws`, `php`, `composer`…

## Usage

Run any command in the container:
Run any command in the container using `./dev-env.sh`:

```bash
docker run --rm -it bref/dev-env <command>
./dev-env <command>

# For example:
docker run --rm -it bref/dev-env php -v
./dev-env php -v
mnapoli marked this conversation as resolved.
Show resolved Hide resolved
```

You can also clone this repo and make a symlink of `dev-env` script to somewhere in your `$PATH`.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be a mention on other way to download this script (e.g. from Bref Doc), once we decide which/how.


## Explanations

Path to your `docker.sock` file is required for `serverless invoke local --docker -f myFunction` to work.

The `HOST_AWS_CONF_DIR` ENV variable is needed for running `bref/dashboard`. It's because we can't simply use volume mappings from inside of `bref/dev-env` container since in case of Docker in Docker mappings are from host instead.

Bref is NOT preinstalled in `bref/dev-env` image. For it to work you have to first install it via `./dev-env composer require bref/bref`. This will create not only typical `vendor` directory in your project but also `.composer` directory for Composer cache, which you can add to your `.gitignore` file.

The Serverless util uses `.serverless` directory also located in your project. You can ignore that as well.
meridius marked this conversation as resolved.
Show resolved Hide resolved
40 changes: 40 additions & 0 deletions dev-env
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#!/usr/bin/env bash
mnapoli marked this conversation as resolved.
Show resolved Hide resolved

set -eu
mnapoli marked this conversation as resolved.
Show resolved Hide resolved

# This is an array of where the Docker socket usually is on most distributions
declare -a POSSIBLE_DOCKER_SOCK_PATHS=(
meridius marked this conversation as resolved.
Show resolved Hide resolved
/run/docker.sock
/var/run/docker.sock
meridius marked this conversation as resolved.
Show resolved Hide resolved
)
DOCKER_SOCK_PATH=
meridius marked this conversation as resolved.
Show resolved Hide resolved
readonly DOCKER_GROUP_ID=$(getent group docker | cut -d: -f3)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need GID of a user able to run Docker, so we can pass it to the container. This is because docker commands run in container will still be run via Docker socket so user running this container needs to be able to work with it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add it above the docker run command instead.

readonly AWS_CONF_DIR="${HOME}/.aws"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where I assume AWS configuration will be


# Find out which of the possible paths for Docker socket is valid
for path in "${POSSIBLE_DOCKER_SOCK_PATHS[@]}" ; do
meridius marked this conversation as resolved.
Show resolved Hide resolved
if [[ -S "${path}" ]]; then
DOCKER_SOCK_PATH="${path}"
break
fi
done
mnapoli marked this conversation as resolved.
Show resolved Hide resolved

if [[ -z "${DOCKER_SOCK_PATH}" ]]; then
echo "Unable to locate Docker socket."
echo "Please make sure Docker service is installed."
exit 1
fi
mnapoli marked this conversation as resolved.
Show resolved Hide resolved

if [[ ! -d "${AWS_CONF_DIR}" ]]; then
echo "AWS settings directory is not found in ${AWS_CONF_DIR}"
exit 1
fi
mnapoli marked this conversation as resolved.
Show resolved Hide resolved

docker run --rm --interactive --tty \
meridius marked this conversation as resolved.
Show resolved Hide resolved
--user "$(id -u)":"$(id -g)" \
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

necessary so all files created in container are actually owned by the user running this container

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
--user "$(id -u)":"$(id -g)" \
# So that all files created in container are actually owned by the user running this container
--user "$(id -u)":"$(id -g)" \

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment breaks the multiline command, so I'd put the explanation under Explanations in README.md instead.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add it above the docker run command instead.

--group-add "${DOCKER_GROUP_ID}" \
meridius marked this conversation as resolved.
Show resolved Hide resolved
--env HOST_AWS_CONF_DIR="${AWS_CONF_DIR}" \
meridius marked this conversation as resolved.
Show resolved Hide resolved
--volume "${DOCKER_SOCK_PATH}:/var/run/docker.sock" \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
--volume "${DOCKER_SOCK_PATH}:/var/run/docker.sock" \
# Necessary for Docker in Docker
--volume "${DOCKER_SOCK_PATH}:/var/run/docker.sock" \

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment breaks the multiline command. In addition the explanation is already mentioned under Explanations in README.md.

--volume "${AWS_CONF_DIR}:/home/app/.aws" \
--volume "${PWD}:/app" \
mnapoli marked this conversation as resolved.
Show resolved Hide resolved
"bref/dev-env" "$@"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and final one - to pass all commands to the container

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"bref/dev-env" "$@"
# "$@" forwards all the arguments to the container
"bref/dev-env" "$@"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The $@ is pretty basic Bash knowledge, but if you really want to have it described it'd have to be put somewhere else (above, below, in README.md) than in the middle of multiline command.