FROM

[jaknoll]

https://github.com/projectatomic/dockerfile_lint/blob/a36b3008822a0b44303ddf1ad645e3327942450c/test/data/rules/basic.yaml#L7

The regex for the FROM rule causes false errors if the docker repo is on a non-standard port, for example:

privaterepo.mine.com:1234/repo/name:tag

This is a valid FROM parameter, but fails the linter.

[lphiri]

@jaknoll the rules in test/data directory are meant for functional and unit testing and not maintained for accuracy. Is there a reason you are using those instead of the rules in config or sample_rules directories?

[jaknoll]

Sorry, I didn't look closely at the path there. Checked the regex here: https://github.com/projectatomic/dockerfile_lint/blob/master/config/default_rules.yaml#L96

and it seems to work, but I get failures -

Pulling docker image projectatomic/dockerfile-lint ...
Running on runner-a762da71-project-1142-concurrent-0 via runner-a762da71-auto-scale-1480342053-7d3a3165...
Fetching changes...
HEAD is now at 1ad2e4e updates to Dockerfile(s)
From <github repo>
   1ad2e4e..2d8c2b6  master     -> origin/master
Checking out 2d8c2b64 as master..
$ dockerfile_lint -f tests/Dockerfile

--------ERRORS---------

Line 8: -> FROM privaterepo.mine.com:1234/repo/name:tag
ERROR: Invalid parameters for command.. 
Reference -> https://docs.docker.com/reference/builder/

--------INFO---------

Line 8: -> FROM privaterepo.mine.com:1234/repo/name:tag
INFO: using a specified registry in the FROM line. using a specified registry may supply invalid or unexpected base images. 
Reference -> https://docs.docker.com/reference/builder/#entrypoint


INFO: There is no 'EXPOSE' instruction. Without exposed ports how will the service of the container be accessed?. 
Reference -> https://docs.docker.com/reference/builder/#expose

[dav1x]

@jaknoll I've submit a PR to address this issue.

https://github.com/projectatomic/dockerfile_lint/pull/58/commits/887c8dc87d3b2f589db30a1d1cd8064ca3730094

[lostintangent]

@lphiri Does it make sense for syntax rules to be enforced via rules files as opposed to the parser? I just noticed that the parser doesn't validate the FROM instruction at all, and it seems like it should, as opposed to relying on a rule file to do it.

[lphiri]

@lostintangent - yes, that is where we are transitioning to. I did not have a parser initially (everything was rule file and adhoc syntax checked) and added the parser later to check syntax. Just haven't gotten around to migrating everything out of the base file yet. You are welcome to help in that area :)

[lostintangent]

@lphiri Sounds good. Now that the shell and healthcheck commands are supported in the parser, it actually seems like we could get rid of the base yank file, since that only defines the set of valid instructions and specifies that they need to have arbitrary content.

Regarding the default rules file, I will port over all of the syntax rules to the parser, but it would seem like any best practices should stay in a rules file that a user can control. In fact, I'd love to discuss being able to disable the base rules so that consumers such as editors can use this lib purely for syntax linting, and then allow users to add best practices by means of their own custom rules.

[lphiri]

@lostintangent - the base rule file gives you an indirect way of doing syntax only checks (dockerfile_lint -r config/base_rules.yml) - but agree it would be nice to just have a "syntax only" switch/shortcut. I suspect some users have been using the base file for that purpose, and for backward compatibility reasons we probably should just keep it even if we add a "syntax only" switch.

And yes, the parser is strictly for syntax checks only -things that would make "docker build" fail - everything else must be in a rule file that a user can control - that's what makes this linter different from the others.

[lostintangent]

@lphiri I believe this can be closed now?

[roadSurfer]

It's almost working now, although it's not quite clear to be how to add unit tests for the FROM rules. The problem I have noted is that some.registry:1134/some/repo will pass the 'no_tag' rule when it should in fact fail. It's not clear to me how to add further unit tests for this, should I just create a series of test Docker files in test/data/dockerfiles and update test/integration/exec_spec.js to parse them? Note that the file test/data/rules/basic.yaml fails to parse the more complex image reference; would it not be better to have tests use config/default_rules.yaml, rather than maintain separate files?

[roadSurfer]

I have created a pull request which adds to the change but making "no_tag" detection more discrete. Pull 122