symfony / recipes

Symfony Recipes Repository
https://github.com/symfony/recipes/blob/flex/main/RECIPES.md
MIT License
956 stars 472 forks source link

`doctrine/doctrine-bundle` causes hadolint to fail #1228

Closed jurchiks closed 11 months ago

jurchiks commented 11 months ago

I have a project generated using this template: https://github.com/dunglas/symfony-docker I've installed doctrine-bundle, which added the following block of code to the Dockerfile:

###> recipes ###
+###> doctrine/doctrine-bundle ###
+RUN apk add --no-cache --virtual .pgsql-deps postgresql-dev; \
+   docker-php-ext-install -j"$(nproc)" pdo_pgsql; \
+   apk add --no-cache --virtual .pgsql-rundeps so:libpq.so.5; \
+   apk del .pgsql-deps
+###< doctrine/doctrine-bundle ###
###< recipes ###

However, when running GitHub CI, hadolint complains about this command and fails: image

Adding # hadolint ignore=DL3018 immediately before the command fixes the issue, as has been done elsewhere in the Dockerfile of the symfony-docker repo, but since this is a recipe-generated block of code, that is not a fix for everyone and it should be added in this repo instead. OR the command should be fixed to not generate this error in the first place, if possible.

stof commented 11 months ago

you can actually edit those files after they have been created, as long you preserve the markers for package names that allow to know which part of the file should be removed when uninstalling the package.

@dunglas should this actually be added in the recipe ?

jurchiks commented 11 months ago

I know I can edit the files, but do you really think everyone who installs this recipe will want to do that manually? Because this will be an issue for everyone who uses that recipe, both for symfony-docker and the API platform.

stof commented 11 months ago

Not everyone that uses this recipe will run hadolint on the generated Dockerfile.

jurchiks commented 11 months ago

Sigh... https://github.com/api-platform/api-platform/blob/main/.github/workflows/ci.yml#L71 https://github.com/dunglas/symfony-docker/blob/main/.github/workflows/ci.yml#L72 You're arguing against adding ONE line that will make many people avoid a headache. Don't do that.

wouterj commented 11 months ago

I would be 👎 as well. The goal of recipes is to give you a starting point, but giving you full control of the end result. Adding more lines to fullfill the requirements of your application (such as making static analysers happy) is part of this.

At the same time, adding this line will add noise to anyone not using halolint (and I think that's probably in the 80% users). Whether one core team member uses that tool doesn't make a huge difference here. I can also imagine a scenario where person A might use another linter for their Dockerfile. We don't want to end up with a Dockerfile with 8 exclusion rules for all sorts of tools out there.

dunglas commented 11 months ago

This issue has been fixed by https://github.com/symfony/recipes/pull/1206. It's another benefit of using mlocati/docker-php-extension-installer 🙂

fabpot commented 11 months ago

Closing then.