Add support for dockerfiles #81

lachmanfrantisek · 2018-04-12T08:18:58Z

Resolves #66
Resolves #82

Support for dockerfiles (using DockerfileParser as a target instance) -- Input is filepath or file-like object.
Refactoring of check loading.
Check loading extended:
- Allows new format (subdirs in the check list can specify target type(s))
- Allows tag/severity filtering. (Not present yet in the API.)
Using same definition for one label check for images, containers and dockerfiles.
Abstract checks for dockerfile:
- instruction counter (compare wit min/max)
- instruction regex (Check regex for given instruction type)

Signed-off-by: lachmanfrantisek <[email protected]>

jpopelka · 2018-04-13T09:50:58Z

colin/checks/abstract/dockerfile.py

+    for inst in dfp.structure:
+        if inst["instruction"] == instruction:
+            result.append(inst)
+    return result


I know you know list comprehensions, I'm just wondering why you don't use it here:

return [inst for inst in dfp.structure if inst["instruction"] == instruction]

Neither am I...;-) Thanks!

Signed-off-by: lachmanfrantisek <[email protected]>

lachmanfrantisek · 2018-04-15T22:12:21Z

I've also moved some code between files to avoid recursive dependencies. The structure started to be a little complicated:

lachmanfrantisek · 2018-04-15T22:14:05Z

It needs some polishing but I'm prepared for some review.

TomasTomecek

Too long for me doing a code review.

Tested locally on a bunch of Dockerfiles and worked like a charm, very nice work!

TomasTomecek · 2018-04-16T06:54:19Z

colin/checks/dockerfile/from_tag.py

                         message="",
                         description="",
                         reference_url="https://docs.docker.com/engine/reference/builder/#from",
                         tags=["from", "dockerfile", "latest"],
                         instruction="FROM",
-                         regex=".*/latest$",
+                         value_regex=".*/latest$",


FROM fedora is also bad, we should catch that one as well

Yes, there is a TODO for this on the next line:

TODO: Does not check if there is no tag => use ImageName parsing.

It will not be hard to do it.

Signed-off-by: lachmanfrantisek <[email protected]>

TomasTomecek

LGTM, Doing PoC with Zdravo? @dhodovsk

dhodovsk · 2018-04-16T11:57:27Z

@TomasTomecek on it

jpopelka

The CLI doesn't mention 'dockerfile', so I think you want to add 'dockerfile' also here

jpopelka · 2018-04-16T13:59:07Z

colin/core/ruleset/ruleset.py

+            if tags:
+                for t in tags:
+                    if t not in check_instance.tags:
+                        logger.debug("Check not passed the tag control: {}".format(r))


r -> t ?

Yes, would be better.

jpopelka · 2018-04-16T14:02:54Z

colin/core/target.py

            return target
+        if os.path.exists(target):


Maybe more specifically if os.path.isfile(target):

I don't know, why I always choose the wrong one..;-) Thanks

Signed-off-by: lachmanfrantisek <[email protected]>

lachmanfrantisek · 2018-04-16T17:26:29Z

@jpopelka I've corrected the docs and add also support for the dockerfile as a file-like object -- We can use it for easier testing.

jpopelka

Awesome 👍

lachmanfrantisek · 2018-04-16T19:28:07Z

@TomasTomecek Thanks for the testing.
@jpopelka Thanks for the review.

lachmanfrantisek added 7 commits April 12, 2018 10:11

Add support for dockerfiles

ee4b275

Signed-off-by: lachmanfrantisek <[email protected]>

Implement dockerfile abstract check for instruction count and label

4401373

Signed-off-by: lachmanfrantisek <[email protected]>

Correct dockerfile checks in rulesets

9239d5b

Signed-off-by: lachmanfrantisek <[email protected]>

Add check for maintainer label in dockerfile

25cb7bf

Signed-off-by: lachmanfrantisek <[email protected]>

Refactor check loading

ecc1034

Signed-off-by: lachmanfrantisek <[email protected]>

Merge code for dockerfile/image checks

3581176

Signed-off-by: lachmanfrantisek <[email protected]>

Use new format for ruleset

b390612

Signed-off-by: lachmanfrantisek <[email protected]>

jpopelka reviewed Apr 13, 2018

View reviewed changes

lachmanfrantisek added 3 commits April 14, 2018 18:21

Use list comprehension

0d06abf

Signed-off-by: lachmanfrantisek <[email protected]>

Reformat ruleset and target

d25a336

Signed-off-by: lachmanfrantisek <[email protected]>

Correct, reformat, sort imports

4424896

Signed-off-by: lachmanfrantisek <[email protected]>

lachmanfrantisek changed the title ~~WIP: Add support for dockerfiles~~ Add support for dockerfiles Apr 15, 2018

TomasTomecek reviewed Apr 16, 2018

View reviewed changes

Make better check for the latest tag in the dockerfile

b77caa4

Signed-off-by: lachmanfrantisek <[email protected]>

TomasTomecek approved these changes Apr 16, 2018

View reviewed changes

jpopelka reviewed Apr 16, 2018

View reviewed changes

lachmanfrantisek added 2 commits April 16, 2018 19:23

Add support for the dockerfile as file-like object

dd18945

Signed-off-by: lachmanfrantisek <[email protected]>

Correct docs

2e1a08d

Signed-off-by: lachmanfrantisek <[email protected]>

jpopelka approved these changes Apr 16, 2018

View reviewed changes

jpopelka merged commit 5a33b88 into user-cont:master Apr 16, 2018

lachmanfrantisek deleted the dockerfile-support branch April 16, 2018 19:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for dockerfiles #81

Add support for dockerfiles #81

lachmanfrantisek commented Apr 12, 2018 •

edited

Loading

jpopelka Apr 13, 2018

lachmanfrantisek Apr 13, 2018

lachmanfrantisek commented Apr 15, 2018

lachmanfrantisek commented Apr 15, 2018

TomasTomecek left a comment

TomasTomecek Apr 16, 2018

lachmanfrantisek Apr 16, 2018 •

edited

Loading

lachmanfrantisek Apr 16, 2018

TomasTomecek left a comment

dhodovsk commented Apr 16, 2018

jpopelka left a comment

jpopelka Apr 16, 2018

lachmanfrantisek Apr 16, 2018

jpopelka Apr 16, 2018

lachmanfrantisek Apr 16, 2018

lachmanfrantisek commented Apr 16, 2018

jpopelka left a comment

lachmanfrantisek commented Apr 16, 2018

Add support for dockerfiles #81

Add support for dockerfiles #81

Conversation

lachmanfrantisek commented Apr 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lachmanfrantisek commented Apr 15, 2018

lachmanfrantisek commented Apr 15, 2018

TomasTomecek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lachmanfrantisek Apr 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomasTomecek left a comment

Choose a reason for hiding this comment

dhodovsk commented Apr 16, 2018

jpopelka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lachmanfrantisek commented Apr 16, 2018

jpopelka left a comment

Choose a reason for hiding this comment

lachmanfrantisek commented Apr 16, 2018

lachmanfrantisek commented Apr 12, 2018 •

edited

Loading

lachmanfrantisek Apr 16, 2018 •

edited

Loading