Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Docker] Fix fixture images not being available #885 #887

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
9 changes: 9 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,15 @@ COPY --from=sylius_node /srv/sylius/public/build public/build

FROM nginx:${NGINX_VERSION}-alpine AS sylius_nginx

ARG GID=82
ARG UID=82
ARG USERNAME="www-data"

RUN set -x; \
addgroup -g $GID -S $USERNAME; \
adduser -S -D -H -u $UID -h /var/cache/nginx -s /sbin/nologin -G $USERNAME -g $USERNAME $USERNAME

COPY docker/nginx/nginx.conf /etc/nginx/nginx.conf
COPY docker/nginx/conf.d/default.conf /etc/nginx/conf.d/

WORKDIR /srv/sylius
Expand Down
8 changes: 8 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,14 @@ services:
- DATABASE_URL=mysql://sylius:${MYSQL_PASSWORD:-nopassword}@mysql/sylius
- LOAD_FIXTURES=1
- PHP_DATE_TIMEZONE=${PHP_DATE_TIMEZONE:-UTC}
volumes:
- .:/srv/sylius:rw,cached
# if you develop on Linux, you may use a bind-mounted host directory instead
# - ./var:/srv/sylius/var:rw
- ./public:/srv/sylius/public:rw,delegated
# if you develop on Linux, you may use a bind-mounted host directory instead
# - ./public/media:/srv/sylius/public/media:rw
- public-media:/srv/sylius/public/media:rw

mysql:
container_name: mysql
Expand Down
3 changes: 3 additions & 0 deletions docker/migrations/docker-entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,7 @@ php bin/console doctrine:migrations:migrate --no-interaction

if [ "$LOAD_FIXTURES" = "1" ]; then
php bin/console sylius:fixtures:load --no-interaction

# make the image files created by fixtures accessible by fpm which runs with user www-data
chown -R www-data:www-data public/media/image
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Ferror this will fix the issue, but it's just like applying FlexTape... ideally we should adopt non-root containers and make GIDs and UIDs configurable via build args and env vars, like this: https://github.com/laradock/laradock/blob/96e0f2e92f6e5b128a71cc77a05b297ac4a0eadb/workspace/Dockerfile#L43-L47

Choose a reason for hiding this comment

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

I realized that if I launched the fixtures manually from the php container, I had to reset the rights each time afterwards.

php bin/console sylius:fixtures:load -n && chown -R www-data:www-data public/media/image

fi
31 changes: 31 additions & 0 deletions docker/nginx/nginx.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
user www-data;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we can merge both configurations?

https://github.com/Sylius/Sylius-Standard/blob/1.12/docker/nginx/conf.d/default.conf

if they can be separate, then I would rather have them moved to conf.d directory and let nginx handle loading it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The files from conf.d are included in the http section, they can't be merged and this file can't go to conf.d.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

worker_processes auto;

error_log /var/log/nginx/error.log notice;
pid /var/run/nginx.pid;


events {
worker_connections 1024;
}


http {
include /etc/nginx/mime.types;
default_type application/octet-stream;

log_format main '$remote_addr - $remote_user [$time_local] "$request" '
'$status $body_bytes_sent "$http_referer" '
'"$http_user_agent" "$http_x_forwarded_for"';

access_log /var/log/nginx/access.log main;

sendfile on;
#tcp_nopush on;

keepalive_timeout 65;

#gzip on;

include /etc/nginx/conf.d/*.conf;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Ferror this is where the files from conf.d are included. This config file is a copy of the one that was shipped with this image, I just changed the user.

}