digitalocean / clusterlint

A best practices checker for Kubernetes clusters. 🤠
Apache License 2.0
542 stars 45 forks source link

"Latest-tag" should be upgrade to "non-fixed-tag check" #63

Open DarthSlider opened 4 years ago

DarthSlider commented 4 years ago

We just found that one of our users used "nightly" tag in his manifest so I think that it should check for any non-fixed versions of images, not just latest

adamwg commented 4 years ago

Hm, this is an interesting suggestion. How would we reliably distinguish between "fixed" and "non-fixed" tags? latest is a special case since it's the default in the docker client if no tag is specified when building an image. While some projects use semantic versions or similar for images, there are also many that don't (e.g., debian:stretch, alpine:3). I'd be hesitant to have clusterlint throw a warning for a non-semver image tag.

timoreimann commented 4 years ago

I wonder if we could solve this by making certain checks more configurable (beyond turning checks entirely on or off); that is, allow users to specify what constitutes undesirable tags for the check at hand. I don't think it's something clusterlint fundamentally supports today though, so it might additional effort (in terms of design and implementation).

Curious what others think.

adamwg commented 4 years ago

I was talking with @nathanjsweet about this on Friday and he had a suggestion I liked. He suggested implementing a much stricter check, requiring that all images are specified by SHA rather than by tag - if you want to make sure your images don't change that is the best practice. I think we would want to put this check in a different group (maybe the security group, or maybe a new strict group or something like that) since specifying images by SHA is not that common despite being a good idea.

varshavaradarajan commented 4 years ago

@timoreimann - Tags are just these labels for versioning docker images. No matter what the tag is, if users push a new version of the image to the same tag it doesn't matter, even if it is semver. Clusterlint checks for the most probable case of tag "reuse" which is latest. While we can provide users an option to configure the other tags as "bad" (ones we need to check for) , I feel like this would involve just adding it for one time use before its some other tag that needs to be configured as bad, unlike latest which is more probable in appearing as a bad tag multiple times. WDYT?

timoreimann commented 4 years ago

@varshavaradarajan I agree that what constitutes a "bad" tag may vary, presumably depending on how strict a specific organization is about certain tags. My thinking was that we could offer a generic mechanism for clusterlint users to specify what these bad tags are, e.g.,

clusterlint -g basic --check-options-latest-tag bad-tags='latest,nightly,0.*'

That way, we (as clusterlint maintainers) do not need to bother with catching up on what the latest bad tags are. It'd up to the user to decide and specify per their individual needs.

This may also meet the needs of users who explicitly expect tags to be overridable. For instance, debian:stable is guaranteed to always point at the latest stable, and debian:stretch will always include the latest security updates for stretch; both is by (Docker Hub) design.

My suggestion would certainly involve more effort since we do not have a mechanism in place today to pass through arbitrary check arguments. The SHA-based approach also sounds interesting but seems to address a slightly different use case (i.e., super strict mode).

Maybe @DarthSlider has some additional thoughts on which approach might best suit their needs.

nathanjsweet commented 4 years ago

I opened an image check PR that I thought might address these issues.

DarthSlider commented 4 years ago

According to our local policies, every word-only tags are considered as bad (except stretch I think). It might not be the best practice ever, but I think it's safe enough. So, I assume that clusterlint could just give a warning for every tag without numbers in it, or, for more strict check for every tag without X.Y in it.