#67 General dockerlinter for Dockerfile.
Merged by jscotka. Opened by phracek.
dockerlinter  into  master

Download 67.patch

This Pull Request covers dockerlinter.
It checks these things:
- FROM baseruntime/baseruntime
- Dockerfile uses microdnf and not dnf
- ARCH exists in ENV directive and LABEL container architecture too.
- NAME exists in ENV directive and LABEL container name too.
- LABEL contains release.
- com.redhat.component exists in LABEL
- io.k8s.description exists in LABEL
- io.openshift.expose-services exists in LABEL
- io.openshift.tags exists in LABEL

1 new commit added

  • Add dependency into python2-dockerfile-parse

@jscotka @ttomecek I would be glad for any feedback.

probably this change also need to have same change in setup.py file for pip, to install this dependency

I don't understand why functions _get_from, _get_run, _get_expose are same, probably would be better to have one, just doing something like normalize, or whatever how to name it

in case of negative test, would be better to check all possibilities, also check that "yum" is not used too.

Please reconsider to merge it with DockerLint: https://pagure.io/modularity-testing-framework/blob/dockerlinter/f/tools/modulelint.py#_102
probably some of tests are same, or could be on this level. There is also check for labels against config. Could be here.

maybe there can be removed os.path.join(os.getcwd() and then there can be removed import os,
dockerlinter.DockerLinter("..")
should work well too I think

All functions *LabelExists seems to be very similar, reconsider to create one function in dockerlinter.DockerLinter class, and these function will be just wrapper like:

def testArchitectureInEnvAndLabelExists(self):
self.assertTrue(dp.label_check(label=architecture, env='ARCH='))

For me this line in every test does not make sense: if self.dp is not None
in case tests does not have sense, just add it into setUp like:

if not self.dp:
     self.skip()

2 new commits added

  • Fixes according to comments.
  • Add dockerfile-parse into deps.

Seems well now.
If you want I can merge it, after you rebase it, to be able to merge it.
Actually merging is blocked by some conflicts.

1 new commit added

  • Merge branch 'master' into dockerlinter

thanks a lot.
LGTM

Pull-Request has been merged by jscotka

Metadata