usabilla/php-docker-template

Docker images for PHP applications, CLI and FPM with shared socket

php-docker-template's Issues

Move composer to dev images

Currently, we have Composer in our non-dev images, as shown below

php-docker-template/Dockerfile-cli

Lines 38 to 40 in f35fecd

	# Install composer
	COPY src/php/utils/install-composer /usr/local/bin/
	RUN install-composer && rm -rf /usr/local/bin/install-composer

We should move it to dev images, along with

php-docker-template/Dockerfile-cli

Lines 53 to 55 in f35fecd

	# Install Xdebug and development specific configuration
	RUN docker-php-dev-mode xdebug \
	&& docker-php-dev-mode config

This should be done in the next PHP circle release, as it will break our images.

Format PHP-FPM logs as JSON

Currently PHP-FPM outputs to stdout and stderr, but it does so in its own single-line format which stops projects from adopting the usabilla format.

We should change their format to json and adhere to ADR-23.

Question: this is pretty opinionated for Usabilla, how do we separate from OSS?

Migrate to Github Actions

This should take the ideas of #101 but port the entire thing to Actions so we can remove the external dependency.

Setup security tests

Security and container scan can be done with Clair

Example with GitlabCI: https://docs.gitlab.com/ee/ci/examples/container_scanning.html

Storage layer

Most of the time, you'll need a storage layer for your app: relational or non relational.

I'd make the assumption and say that probably, the most popular storage layers you'll find in the wild are MySQL and Redis. What about including them?

Enable recommended php configuration by default

Our PHP images are based on images from official repo.
As mentioned in the documentation, these images ship with the default ini configuration recommendations for both production and development.
These configurations, however, are not enabled by default in the official PHP images, as discussed here.

In order to have a sane baseline configuration guaranteed, we should enable the aforementioned ini recommendations for each environment (i.e. have the desired one available at this path $PHP_INI_DIR/php.ini). Docker images extending the ones in this repository will still have the flexibility to override (parts of) that configuration by placing customized configurations in the $PHP_INI_DIR/conf.d/ directory.

Improve test suite organisation

The test scripts (e.g. https://github.com/usabilla/php-docker-template/blob/master/test-cli.sh) control test suites that are run for different tags. This happens based on custom markers.

However the number of different markers currently used across multiple test files is causing confusion. The way the tests are organised and run could benefit from a better structured and transparent approach.

Introduce strategy for cleaning up outdated tags

We currently have no mechanism in place that can detect that we're building tags based on outdated images, e.g. an old alpine version.
From a security perspective it would be nice to have some tracking mechanism/automation to deal with this, so that we don't build on top of unsupported base images.

Add composer to our dev images

This was suggested by @rdohms, considering that we don't necessarily have an updated stack in our local env and sometimes we use the containers to run tests.

Include shush in image

To decrypt secrets we use shush (see also this comment). We should include it in the production image.

For versions in "Security Support" only build on new releases

Once a version of PHP enters "Security Support" we no longer need to build it every week, but should try to only build once a new version is actually released.

PHP's release schedule: https://php.net/releases.

Report Clair issues via Wall-e plugin

Wall-e can now handle broadcasting these problems to slack nicely, we just need to plug it in.

https://github.com/usabilla/botkit-bot/pull/105

Should we have nginx as a separate container?

Using kubernetes and docker-compose it's simple to achieve this but what are the trade-offs involved here?

Increase test range

Currently the tests are running only against the "default" images, being php 7.2 with alpine 3.7, this happens because that's the version being spin up by docker-compose.
My suggestion is to read from the tag files (in tmp) and do a text replace of all available tags, maybe just like the approach from build-php.sh. It seems like docker-compose also accepts the yml file from the stdin, making it simple and avoiding creating temporary files

Document new development script

A new development script for xdebug and configs was added #57

It needs to be documented and also explain we're moving away from the dev tags in the projects

Remove supervisord from cli image

Since we're moving to Kubernetes we don't need to have a single container running multiple commands.
This architectural decision reduces the size of the images, allows us to better scale the consumers, and makes logging much easier.

Add functional example with Symfony Project

The idea is to have a veeeery simple and functional example of a symfony project using our Docker images!

Just like this one: https://github.com/usabilla/php-docker-template/tree/master/docs/examples/hello-world-fpm
Which is a bare bones version on how to use it!

We'd like it to follow the same quality standards and look amazing!

Document Docker on k8s specifics

Context

There are too many things to consider when deploying a PHP Docker setup in Kubernetes, many of them related to good practices and others due to how PHP is designed, the intent of this issue is to list all of them and match whether we have both solved the issue and documented it.

THE list

• Starting with Docker best practices
  • Package a single app per container
  • Properly handle PID 1, signal handling, and zombie processes
    • PHP Cli doesn't come with pcntl by default.
      Which means SIGTERM and SIGINT will be ignored and the process will die non gracefully, also the application must know how to deal with the signal
    • PHP-FPM doesn't adhere to the standard posix signals IPC, where it'll terminate immediately upon SIGTERM and SIGINT
  • Optimize for the Docker build cache - Done in the official image
  • Remove unnecessary tools - I.e.:

    php-docker-template/Dockerfile-fpm

    Lines 20 to 21 in 7fd241a

    	# those deletions happen since the helper scripts nor the official image are removing them
    	&& docker-php-source-tarball clean && rm /usr/local/bin/phpdbg && rm -rf /tmp/pear ~/.pearrc \

  • Build the smallest image possible - Same as above
  • Use vulnerability scanning in Container Registry
    • How to patch those images?
    • Patch older versions of the image
  • Properly tag your images
  • Carefully consider whether to use a public image
• Nginx and PHP-FPM, after handling signals correctly, let's understand the relationship of those components.
  • Does the Nginx process finishes before the PHP-FPM one? I.e: A k8s preStop which checks if the Nginx is dead before PHP-FPM:

 # Considering you have a mount between the nginx and php-fpm containers on `/var/run`
          lifecycle:
            preStop:
              exec:
                command: ["/bin/sh","-c","while test -e /var/run/nginx.pid; do sleep 1; done"]

• Does Nginx need access to the code? When serving static files, you could for instance have another deployment only for that, which doesn't have you php files
• How do they communicate, advantages and disavantages of:
  • Via TCP connection
  • Via socket while running both in the same container
  • Via socket while running in different containers
• Test the relationship between the services
• On the best practices for operating containers, probing:
  • Should you warm up the cache of the application? I.e.: Opcode cache as file
  • Readiness probe for a non-interactive (CLI) container
  • Liveness probe for a non-interactive (CLI) container
  • Readiness probe for a PHP-FPM container - I.e.: https://github.com/renatomefi/php-fpm-healthcheck
  • Liveness probe for a PHP-FPM container - I.e.: https://github.com/renatomefi/php-fpm-healthcheck
• Observability, logging and monitoring
  • Logging to stdout and stderr
  • Log format
  • PHP-FPM prefixes all stdout with WARNING: [pool www] child 12 said into stdout :.
    7.3 has a fix for that, but what to do with 7.2? https://www.php.net/manual/en/migration73.new-features.php#migration73.new-features.fpm
  • Produce metrics
  • Make it possible to scrape or push metrics, is it non-blocking?
• Containers
  • Avoid privileged containers
  • Avoid running as root
  • Stateless
• Best practices for writing Dockerfiles
  • Lint the Dockerfile - I.e.:

    php-docker-template/Makefile

    Lines 67 to 68 in 436042e

    	lint:
    	docker run -v ${current_dir}:/project:ro --workdir=/project --rm -it hadolint/hadolint:latest-debian hadolint /project/Dockerfile-cli /project/Dockerfile-fpm /project/Dockerfile-http

Rename virtual name in PHP docker extension helper scripts to something specific

We sometimes use very generic names for the virtual name we hand APK for build dependencies, for example:

php-docker-template/src/php/utils/docker/docker-php-ext-rdkafka

Line 6 in 67fb7a3

apk add --no-cache --virtual .build-deps $PHPIZE_DEPS

There's a chance that something outside these scripts is using the same name so if, in the script, we do apk del .build-deps we also remove the packages that were assigned to that virtual name outside the script. We can solve this by using more specific virtual names.

Enable Docker buildkit

In order to improve build performance and enable more building features.

A good start: https://github.com/usabilla/php-docker-template/commits/enable-buildkit

The image should be shipped with `app` user

Context

Currently in the building process there's nothing that switches the user to the app, meaning currently the images are shipped with root user by default

Suggestion

Add USER app:app at the end of the Dockerfile-cli, but this is a breaking change!

Migrate to Gitlab

Enforce versions (Semantically) in the ext helper scripts

The problem:

For instance in the example below:

php-docker-template/src/php/utils/docker/docker-php-ext-rdkafka

Line 9 in 91c2a84

pecl install rdkafka

We see that we aren't locking the version pecl is installing, this can lead to bugs and major versions being upgraded without the user's consent.

A proposed solution would be:

The script should take an argument (.i.e.: $1 or ENV variable) which specifies the version to be installed.

A bonus is to be able to specify the version semantically, we have to check weather pecl supports it.

Introduce PHP Prometheus Exporter image

Context

Not every php process has a webserver which can expose prometheus metrics, thus a sidecar container can be a google solution for it.

Possible solution

Just like JMX exporter works as a sidecar container, we can have a container which exposes the php metrics to be fetch by prometheus.

A Nginx container which takes a file with the metrics or that connects to php directly could achieve this.

As first option I'd say the file is the easiest to implement

Add python linting

So we can have beautiful code :)

Use a PHP official images as starting point

So that we can reduce the compilation time of our images and reduce the amount of code we maintain.

Nodejs container

Given that in most modern apps nodejs is present through tools like webpack or various is linters, I would like to have that as an option for this stack setup.

Look into using nginx realip module for better logging

http://nginx.org/en/docs/http/ngx_http_realip_module.html

The nginx realip module allows you to specify a trusted list of proxies to parse X-Forwarded-For headers from so that the actual client IP address gets logged instead of the ingress controller's IP address. This is made a little complicated by the fact that the ingress controllers just have IP addresses on the pod network as for any other pod, and we don't necessarily want to treat everything running in Kubernetes as if it were a proxy, but it might be worth looking into at some stage.

Automate CVE patches removal

Context

When a new CVE comes up we tend to upgrade the package using apk upgrade --no-cache curl for instance.
Since our images are based on official ones, eventually they get the upgraded package which causes us to remove the patches from our Dockerfiles

Solution

• Embrace all CVE patches in blocks like:

###> CVE: 1234 ###

###< CVE: 1234 ###

• Write a script that removes them and tries to build the images and check for vulnerabilities, if it passes then automatically opens a PR with the CVEs removed

Build the container and run some tests

Otherwise we have no clue if the configuration is correct.

Get rid of `if` usage in nginx config

We tried to get rid of the newly-introduced if statement inside the cors.conf in order to avoid if, since it is generally discouraged by nginx.

With the help of @stevenjm we came to the following:

# /etc/nginx/location.d-enabled/cors.conf
# Allow CORS if environment variable is set
add_header 'Access-Control-Allow-Origin' "http://127.0.0.1:8888" always;
add_header 'Access-Control-Allow-Credentials' 'true' always;
add_header 'Access-Control-Allow-Methods' "*" always;
add_header 'Access-Control-Allow-Headers' "*" always;
add_header 'Access-Control-Expose-Headers' "*" always;

# /etc/nginx/conf.d/default.conf
map $request_method $rewrite_target {
    default /index.php;
    OPTIONS /NGINX_INTERNAL_HANDLE_OPTIONS;
}

server {
    listen 80;
    listen [::]:80;
    server_name  localhost;

    root    /opt/project/public;
    charset UTF-8;

    include /etc/nginx/location.d-enabled/*.conf;

    error_page   500 502 503 504  /50x.html;
    location = /50x.html {
        root   /usr/share/nginx/html;
    }
    
    location / {
        rewrite ^ $rewrite_target last;
    }

    location = /NGINX_INTERNAL_HANDLE_OPTIONS {
        return 204;
    }

    location ~ \.php$ {
        fastcgi_pass   unix:/var/run/php-fpm.sock;
        fastcgi_split_path_info ^(.+\.php)(/.*)$;

        include        fastcgi_params;

        fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name;
        fastcgi_param DOCUMENT_ROOT $document_root;
    }
}

The map could be moved into the cors.conf and that file could, for example, live in the conf.d directory, but I'll leave that to you guys to figure out.

Rebuild images off latest upstream to fix iconv bug

Hello,

upstream fixed a bug in march docker-library/php#1264

the last changes here were in Feb, would it be possible to rebuild and publish the images based off the latest upstream docker php to include this bug fix.

Let me know if theres anything I can do to make this process quicker and easier for you.

Thank you kindly.

Translate SIGTERM into SIGQUIT for php-fpm container

Context

PHP fpm doesn't respect the standard POSIX signals, thus we have to translate signal 15 into 3

Suggestions

Provide a new entrypoint which makes this change

Possible implementations

https://github.com/Yelp/dumb-init

Make GPG key download more reliable

Every time we have to install an extension, the GPG keys are downloaded and used. That fails more often than not so we need to find a way to make this more reliable.

The intermediate solution that is now implemented is that we cache the GPG keys (2 for each minor version) in the image itself. It'd be nicer if we could lift this to a higher level, maybe into GitLab?

Add functional example with Laravel Project

The idea is to have a veeeery simple and functional example of a Laravel project using our Docker images!

Just like this one: https://github.com/usabilla/php-docker-template/tree/master/docs/examples/hello-world-fpm
Which is a bare bones version on how to use it!

We'd like it to follow the same quality standards and look amazing!

Document how to replace the default vhost

Currently the default vhost is strict in the sense it forwards everything to fpm and doesn't handle static files for instance.
To replace it you can copy it from your project and replace the template one at: /etc/nginx/vhost.conf.template
Note it's still template since it'll have the variables replaced on the entrypoint

Also provide a working example

Allow to add extra tags into the images

Just like on the nginx builder we'd like to make it possible to add custom/extra tags to build.
One use case for instance is when a new project adopt a beta version of php, 7.3-rc let's say, they could already use 7.3 as base and when it's finally released the migration is seamless

template php-fpm config

Hi there,

Would you be open to a PR to allow templating php-fpm config via env vars?

Something like this, as these are the values that would mostly likely need scaling/configuring:

pm = ${PHP_FPM_PM}
pm.max_children = ${PHP_FPM_PM_MAX_CHILDREN}
pm.start_servers = ${PHP_FPM_PM_START_SERVERS}
pm.min_spare_servers = ${PHP_FPM_PM_MIN_SPARE_SERVERS}
pm.max_spare_servers = ${PHP_FPM_PM_MAX_SPARE_SERVERS}
pm.process_idle_timeout = ${PHP_FPM_PM_PROCESS_IDLE_TIMEOUT}
pm.max_requests = ${PHP_FPM_PM_MAX_REQUESTS}

Thanks.

Change PECL extension example

Context

Since we provide a helper to install and configure Xdebug it'd be good to replace the PECL example in the documentation to another extension, also document how to use the Xdebug helper

Reduce CircleCI configuration complexity

Context

CircleCI configuration can become really verbose and repetitive, here is a list of things that can be improved by either: Removing, downstream to Makefile, yaml anchors or CircleCI reusable commands

The list

• This checks if all images were built:

  php-docker-template/.circleci/config.yml

  Line 213 in 615c9f7

  - run: cat ./tmp/build-fpm.tags | xargs -I % docker inspect --format='%={{.Id}}:{{index .ContainerConfig.Env 8}}' %

• The whole build could be abstracted?

  php-docker-template/.circleci/config.yml

  Line 208 in 615c9f7

  build-fpm:

• Should Clair be running somewhere else?

  php-docker-template/.circleci/config.yml

  Line 176 in 615c9f7

  scan-vulnerability:

• Others? Please comment

Document images EOL strategy

My recommendation is that we follow PHP's own lifecycle, we don't delete images but we stop building them as the PHP counterpart version reaches EOL!