-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dockerfile-from-checker: initial commit #4
dockerfile-from-checker: initial commit #4
Conversation
@@ -0,0 +1,186 @@ | |||
package main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And please add a copyright and license line.
for i := 1; i < len(tfs); i++ { | ||
tf := tfs[i] | ||
if tf.tag != tfs[i-1].tag { | ||
fmt.Printf("tags differ for image %s in files %s and %s\n", tf.fullname, tf.file, tfs[i-1].file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this print all of the differences? It seems like it exits on the first one which would make it harder to use when we start with a mess, but also when we want to move some tag forward in one place and needing to realize that it is used (and needs to be updated) in N other places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It stops after the first one; it is a trick to hide a shortcoming of this code: if more than two "FROM"s exist for the same image but with different tags, it does not find all of them.
F.e. if we remove os.Exit(1)
and have the following files:
1/Dockerfile:FROM ubuntu:2
2/Dockerfile:FROM ubuntu:2
3/Dockerfile:FROM ubuntu:2
4/Dockerfile:FROM ubuntu:4
then the result is:
tags differ for image ubuntu:2 in files /.../eve-build-tools/src/dockerfile-from-checker/test/1/Dockerfile and /.../eve-build-tools/src/dockerfile-from-checker/test/4/Dockerfile
it misses file 2 and 3.
tf := tfs[i] | ||
if tf.tag != tfs[i-1].tag { | ||
fmt.Printf("tags differ for image %s in files %s and %s\n", tf.fullname, tf.file, tfs[i-1].file) | ||
os.Exit(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stylistically it would make more sense to move the exit to the main function. And I think exit(1) is normally for input errors (unknown option, input file does not exist).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This tool would be quite helpful.
Do you see use using it in the EVE build pipeline or just manual use?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is quite neat, I rather like it. It is a form of linting, checking that all the Dockerfiles it finds are unified in their FROM
. I am not sure if it will get confused with the multi-stage files, or ones that actually should FROM
elsewhere, but we can iterate on it.
The one change: there is no need for a Dockerfile in src/dockerfile-from-checker/
. We should include this in the single container image generated by this repo.
Instead, create a Makefile
in src/dockerfile-from-checker
that follows the Makefile rules here. Then it will get absorbed automatically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yetus and build failure to fix
dockerfile-checker allows to check that several Dockerfiles use images with the same image hash Signed-off-by: Christoph Ostarek <[email protected]>
137aca5
to
d22ef74
Compare
Thanks! For manual use I just used a combination of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
dockerfile-checker allows to check that several
Dockerfiles use images with the same image hash