Closed llaville closed 3 years ago
For a real example, see the Mega-Linter run under github action at https://github.com/llaville/docker-php-toolbox/runs/3400858164?check_suite_focus=true#step:4:127
BTW, I've a question about ERROR-DOCKERFILE_DOCKERFILELINT.log
report
In following results we got 4 issues, that it means it should be rather print ERROR - 4 error(s)
than
[dockerfilelint] Dockerfiles/mods/Dockerfile - ERROR - 1 error(s)
--Error detail:
File: /tmp/lint/Dockerfiles/mods/Dockerfile
Issues: 4
Line 7: FROM base as builder
Issue Category Title Description
1 Clarity Base Image Missing Base images should specify a tag to use.
Tag
Line 21: FROM builder as build-version-1
Issue Category Title Description
2 Clarity Base Image Missing Base images should specify a tag to use.
Tag
Line 27: FROM build-version-${BUILD_VERSION} as after-condition
Issue Category Title Description
3 Clarity Base Image Missing Base images should specify a tag to use.
Tag
Line 29: FROM after-condition
Issue Category Title Description
4 Clarity Base Image Missing Base images should specify a tag to use.
Tag
I don't kown Mega-Linter architecture as good as you, but if we can't override the default dockerfilelint config file See https://github.com/replicatedhq/dockerfilelint/blob/f7bdc892c28763cae835bc69ada55d20f65ed61e/lib/index.js#L93-L102 Introduced since v1.1.2 by https://github.com/replicatedhq/dockerfilelint/commit/b09bff92917adb77470605cd89bb43cc6627f8dd
And as dockerfilelint --config .dockerfilelintrc Dockerfiles/mods/Dockerfile
did not work
and as dockerfilelint --config . Dockerfiles/mods/Dockerfile
works
I suggest that descriptor should be changed from https://github.com/nvuillam/mega-linter/blob/v4.44.1/megalinter/descriptors/dockerfile.megalinter-descriptor.yml#L10-L19
to something like
cli_config_arg_name: "--config"
config_file_name: .
examples:
- "dockerfilelint Dockerfile"
- "dockerfilelint --config . Dockerfile"
In this case config_file_name
is the path (directories) to config file without filename .dockerfilelintrc
Screenshot from my docker container where I've installed dockerfilelint v1.8.0
What if you put an empty.dockerfilelintrc
at the root of your repository ?
In following results we got 4 issues, that it means it should be rather print ERROR - 4 error(s) than
BTW, not all linters have regex defined to get the number of errors, i'll arrange that for dockerfilelint :)
What if you put an empty.dockerfilelintrc at the root of your repository ?
Sorry I've tried lot of combinaisons, even your suggestion, but none works
No workaround are possible
DOCKERFILE_DOCKERFILELINT_CONFIG_FILE: "LINTER_DEFAULT"
- Rules config: [/.dockerfilelintrc]
DOCKERFILE_DOCKERFILELINT_RULES_PATH: "https://github.com/llaville/docker-php-toolbox/blob/master/"
- Rules config: [https://github.com/llaville/docker-php-toolbox/blob/master/.dockerfilelintrc]
FYI: I've opened a feature request https://github.com/replicatedhq/dockerfilelint/issues/186 to allow to change config file path
Their last commit is from one year ago, not sure they'll move quickly :/
Not really important, because I've disabled dockerfilelint linter (see https://github.com/llaville/docker-php-toolbox/commit/fa95a99a1868366cb29a4b388c82a3991b9b70e6) to avoid rules not ignored. And my GA results are fine as expected https://github.com/llaville/docker-php-toolbox/actions/runs/1161566009
I can check locally to see if something wrong are really important !
That's strange, because as you can see in Mega-Linter own repo, there is a .dockerfilelintrc that is taken in account, and without any reference to DOCKERFILELINT in .mega-linter.yml :/
But anyway... I think it's not a problem to disable dockerfilelint, as hadolint seems to be the reference now , usually the number of stars is a good indicator (700 vs 5800) :)
Ok I've just test it and found why it work in Mega-Linter source code context.
When Dockerfile(s) and .dockerfilelintrc
are in same location (directory) it works like a charm !
But not in my case when config file .dockerfilelintrc
is in root of project and Dockerfiles are in sub-folders !
Workaround in such condition is to have a .dockerfilelintrc
in each directory where they are a Dockerfile.
It's not elegant and worst to maintain source code, because symbolink link does not work (I've tested with it first)
I would need to implement a custom python linter Class for dockerfilelint, or update the architecture to be able to use config_folder_name... that seems a lot of work for very few benefit , especially for a soon-to-be deprecated linter , do you agree ? :angel: If someday dockerfilelint maintainers wake up, they'll take on account this request, and their new version will work on Mega-Linter with --config ... but if they do nothing, let's assume that their linter is dead ? πΌ
or update the architecture to be able to use config_folder_name
I'd prefer this solution
I don't say i'll never do it... but I have to prioritize , between my rent-paying job and my open-source requests... such not so small evolution for so few benefits, makes this task at the bottom of my list :/ But maybe you'd like to make a PR ? I'm not the only one to code here :)
I understand that you have other priority. As I'm not a PRO of python, perharps I may suggest a PR, and I'll be please if you have time to review !
I've almost a PR solution ready. I still want to make more tests before to submit the PR to review.
BTW, I've noticed some part of code that I think we should fix. Can you confirm following points please ?
https://github.com/nvuillam/mega-linter/blob/master/megalinter/Linter.py#L323-L326 => seem dead code (_FILE_NAME
, renamed to _CONFIG_FILE
since commit 1170817e1c5ba698b48f098431a924854a54fe0e
In console reporter we can see - Rules config: [/.github/linters]
; did it means we want to display short _RULES_PATH
version (than a directory separator is too much in front, due to a text.replace with value "/tmp/lint" instead of "/tmp/lint/") rather than full path version "/tmp/lint/.github/linters"
a typo error at https://github.com/nvuillam/mega-linter/blob/master/megalinter/Linter.py#L370 ( in user repo .github/linters folder
)
Finally, I've found a best compromise to solve the situation. No source code need to be modified.
Even if at the beginning I think that overwriting the default -c
option was a bug (see https://github.com/nvuillam/mega-linter/blob/v4.44.1/megalinter/descriptors/dockerfile.megalinter-descriptor.yml#L17), it's now a big help to solve the conflict with native dockerfilelint that accept only a directory as value of -c|--config
option.
When Dockerfiles to check are not it the same directory as the .dockerfilelintrc
config file, you just have to specify a DOCKERFILE_DOCKERFILELINT_ARGUMENTS
variable in your .meta-linter.yml
For examples:
DOCKERFILE_DOCKERFILELINT_ARGUMENTS: "-c /tmp/lint/.github/linters"
DOCKERFILE_DOCKERFILELINT_ARGUMENTS: "-c /tmp/lint/.rc"
Of course, when you change the DEFAULT_WORKSPACE
value, you must accordingly change it also in DOCKERFILE_DOCKERFILELINT_ARGUMENTS
https://github.com/nvuillam/mega-linter/blob/master/megalinter/Linter.py#L323-L326 => seem dead code (_FILE_NAME, renamed to _CONFIG_FILE since commit 1170817
I want to remain compliant with Super-Linter configuration (to ease switch to Mega-Linter ^^) so Mega-Linter continues to manage xxx_FILE_NAME anyway :)
In console reporter we can see - Rules config: [/.github/linters]; did it means we want to display short _RULES_PATH version (than a directory separator is too much in front, due to a text.replace with value "/tmp/lint" instead of "/tmp/lint/") rather than full path version "/tmp/lint/.github/linters"
Good catch, there is an extra / in the log :)
a typo error at https://github.com/nvuillam/mega-linter/blob/master/megalinter/Linter.py#L370 ( in user repo .github/linters folder )
What is the typo error ?
When Dockerfiles to check are not it the same directory as the .dockerfilelintrc config file, you just have to specify a DOCKERFILE_DOCKERFILELINT_ARGUMENTS variable in your .meta-linter.yml
May you update the documentation to explain that ? :) To do that, you can manage a linter_text
attribute on the descriptor, then call bash build.py --doc
I want to remain compliant with Super-Linter configuration (to ease switch to Mega-Linter ^^) so Mega-Linter continues to manage xxx_FILE_NAME anyway :)
Ok, got it !
a typo error at https://github.com/nvuillam/mega-linter/blob/master/megalinter/Linter.py#L370 ( in user repo .github/linters folder )
What is the typo error ?
As specified there is an extra / in path : compare # in user repo ./github/linters folder
and fix # in user repo .github/linters folder
When Dockerfiles to check are not it the same directory as the `.dockerfilelintrc` config file, you just have to specify a `DOCKERFILE_DOCKERFILELINT_ARGUMENTS` variable in your `.meta-linter.yml` For examples: DOCKERFILE_DOCKERFILELINT_ARGUMENTS: "-c /tmp/lint/.github/linters" DOCKERFILE_DOCKERFILELINT_ARGUMENTS: "-c /tmp/lint/.rc" Of course, when you change the `DEFAULT_WORKSPACE` value, you must accordingly change it also in `DOCKERFILE_DOCKERFILELINT_ARGUMENTS`
When I wrote these lines i didn't realized that it was so much important, because I fell in trap with Github Actions.
When you set a path to run locally (/tmp/lint
) it did not match with GA.
BTW thanks to ENV variables that solved the situation once again. Look at https://github.com/llaville/docker-php-toolbox/commit/4ffca062ebf4c7255f26c07640863dc41012bc47 and results now seems correct in first look (all green lights) :
https://github.com/llaville/docker-php-toolbox/actions/runs/1178555854
CAUTION I noticed something strange in GA context. Compares between this run 1178555854 and
EDITORCONFIG
descriptor is accordingly to my .mega-linter.yml
config file run as expected but not in GA run : Can you confirm that this is an issue ?
I confirm that GA https://github.com/llaville/docker-php-toolbox/runs/3453212069?check_suite_focus=true#step:2:156 and my local env run the same docker image
Local run is launch from same source code with following command :
llaville@DESKTOP-83207V7:~/devilbox_data/backups/github/docker-php-toolbox$ docker run -v $(pwd):/tmp/lint nvuillam/mega-linter-php:v4
EDITORCONFIG
and MARKDOWN
descriptors lint checks are missing in GA run.
Compare to local (and also files number)
Skipped linters: ANSIBLE_ANSIBLE_LINT, ARM_ARM_TTK, CLOJURE_CLJ_KONDO, CLOUDFORMATION_CFN_LINT, COFFEE_COFFEELINT, COPYPASTE_JSCPD,
CPP_CPPLINT, CREDENTIALS_SECRETLINT, CSHARP_DOTNET_FORMAT, CSS_SCSS_LINT, CSS_STYLELINT, C_CPPLINT, DART_DARTANALYZER,
ENV_DOTENV_LINTER, GHERKIN_GHERKIN_LINT, GIT_GIT_DIFF, GO_GOLANGCI_LINT, GO_REVIVE, GRAPHQL_GRAPHQL_SCHEMA_LINTER, GROOVY_NPM_GROOVY_LINT,
HTML_HTMLHINT, JAVASCRIPT_ES, JAVASCRIPT_PRETTIER, JAVASCRIPT_STANDARD, JAVA_CHECKSTYLE, JSON_V8R, JSX_ESLINT, KOTLIN_KTLINT,
KUBERNETES_KUBEVAL, LATEX_CHKTEX, LUA_LUACHECK, MARKDOWN_MARKDOWN_LINK_CHECK, MARKDOWN_MARKDOWN_TABLE_FORMATTER, MARKDOWN_REMARK_LINT,
OPENAPI_SPECTRAL, PERL_PERLCRITIC, PHP_PHPSTAN, PHP_PSALM, POWERSHELL_POWERSHELL, PROTOBUF_PROTOLINT, PUPPET_PUPPET_LINT, PYTHON_BANDIT,
PYTHON_BLACK, PYTHON_FLAKE8, PYTHON_ISORT, PYTHON_MYPY, PYTHON_PYLINT, RAKU_RAKU, RST_RSTCHECK, RST_RSTFMT, RST_RST_LINT, RUBY_RUBOCOP,
RUST_CLIPPY, R_LINTR, SALESFORCE_SFDX_SCANNER_APEX, SALESFORCE_SFDX_SCANNER_AURA, SALESFORCE_SFDX_SCANNER_LWC, SCALA_SCALAFIX,
SNAKEMAKE_LINT, SNAKEMAKE_SNAKEFMT, SPELL_CSPELL, SPELL_MISSPELL, SQL_SQLFLUFF, SQL_SQL_LINT, SQL_TSQLLINT, SWIFT_SWIFTLINT,
TEKTON_TEKTON_LINT, TERRAFORM_CHECKOV, TERRAFORM_TERRAFORM_FMT, TERRAFORM_TERRAGRUNT, TERRAFORM_TERRASCAN, TERRAFORM_TFLINT, TSX_ESLINT,
TYPESCRIPT_ES, TYPESCRIPT_PRETTIER, TYPESCRIPT_STANDARD, VBDOTNET_DOTNET_FORMAT, XML_XMLLINT, YAML_V8R
To receive reports as email, please set variable EMAIL_REPORTER_EMAIL
Listing all files in directory [/tmp/lint], then filter with:
- File extensions: *, .bash, .dash, .json, .json5, .jsonc, .ksh, .md, .php, .sh, .yaml, .yml
- File names (regex): Dockerfile
- Excluding regex: (vendor/)
Kept [251] files on [840] found files
+----MATCHING LINTERS-----------------+----------------------+----------------+------------+
| Descriptor | Linter | Criteria | Matching files | Format/Fix |
+--------------+----------------------+----------------------+----------------+------------+
| BASH | bash-exec | .sh|.bash|.dash|.ksh | 19 | no |
| BASH | shellcheck | .sh|.bash|.dash|.ksh | 19 | no |
| BASH | shfmt | .sh|.bash|.dash|.ksh | 19 | no |
| DOCKERFILE | dockerfilelint | Dockerfile | 4 | no |
| DOCKERFILE | hadolint | Dockerfile | 4 | no |
| EDITORCONFIG | editorconfig-checker | * | 237 | no |
| JSON | jsonlint | .json | 104 | no |
| JSON | eslint-plugin-jsonc | .json|.json5|.jsonc | 104 | no |
| JSON | prettier | .json | 102 | no |
| MARKDOWN | markdownlint | .md | 9 | no |
| PHP | php | .php | 31 | no |
| PHP | phpcs | .php | 31 | no |
| YAML | prettier | .yml|.yaml | 1 | no |
| YAML | yamllint | .yml|.yaml | 1 | no |
+--------------+----------------------+----------------------+----------------+------------+
@llaville your posts explain very well the issues , and with appropriate details to reproduce, but I'm starting to get lost in all the topics discussed :p
Please can you post one issue by problem ? :) So that I can process and close them one by one without having to re-read all everytime I work on mega-linter :p
Describe the bug Trying to ignore rules with Dockerfilelint config file
To Reproduce Run locally with command
docker run -v $(pwd):/tmp/lint nvuillam/mega-linter-php:latest mega-linter-runner --flavor php
on source code available at https://github.com/llaville/docker-php-toolboxExpected behavior No error detected (as ignored)
Screenshots
mega-linter.log
``` ### Processed [DOCKERFILE] files - Using [dockerfilelint v1.8.0] https://github.com/replicatedhq/dockerfilelint - Mega-Linter key: [DOCKERFILE_DOCKERFILELINT] - Rules config: [/.dockerfilelintrc] [dockerfilelint] Dockerfiles/base/Dockerfile - SUCCESS - 0 error(s) [dockerfilelint] Dockerfiles/mods/Dockerfile - ERROR - 1 error(s) --Error detail: File: /tmp/lint/Dockerfiles/mods/Dockerfile Issues: 4 Line 8: FROM base as builder Issue Category Title Description 1 Clarity Base Image Missing Base images should specify a tag to use. Tag Line 22: FROM builder as build-version-1 Issue Category Title Description 2 Clarity Base Image Missing Base images should specify a tag to use. Tag Line 28: FROM build-version-${BUILD_VERSION} as after-condition Issue Category Title Description 3 Clarity Base Image Missing Base images should specify a tag to use. Tag Line 30: FROM after-condition Issue Category Title Description 4 Clarity Base Image Missing Base images should specify a tag to use. Tag ```Additional context See Dockerfilelint source code that handle config file that does not allow other name than default : https://github.com/replicatedhq/dockerfilelint/blob/f7bdc892c28763cae835bc69ada55d20f65ed61e/lib/index.js#L93-L102
And my attempt to test it outside mega-linter with https://github.com/replicatedhq/dockerfilelint/issues/185