diff --git a/README.md b/README.md --- a/README.md +++ b/README.md @@ -79,6 +79,22 @@ In windows, the `%LOCALAPPDATA%` environment variable is used instead of `XDG_CONFIG_HOME` +## Inline ignores + +It is also possible to ignore rules by using a special comment directly above the Dockerfile +instruction you want to make an exception for. Ignore rule comments look like +`# hadolint ignore=DL3001,SC1081`. For example: + +```dockerfile +# hadolint ignore=DL3006 +FROM ubuntu + +# hadolint ignore=DL3003,SC1035 +RUN cd /tmp && echo "hello!" +``` + +Inline ignores will only work if place directly above the instruction. + ## Integrations To get most of `hadolint` it is useful to integrate it as a check to your CI @@ -99,7 +115,7 @@ Please [create an issue][] if you have an idea for a good rule. | Rule | Description | -|--------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------| +|:-------------------------------------------------------------|:----------------------------------------------------------------------------------------------------------------------------------------------------| | [DL3000](https://github.com/hadolint/hadolint/wiki/DL3000) | Use absolute WORKDIR. | | [DL3001](https://github.com/hadolint/hadolint/wiki/DL3001) | For some bash commands it makes no sense running them in a Docker container like ssh, vim, shutdown, service, ps, free, top, kill, mount, ifconfig. | | [DL3002](https://github.com/hadolint/hadolint/wiki/DL3002) | Do not switch to root USER. | @@ -121,7 +137,7 @@ | [DL3018](https://github.com/hadolint/hadolint/wiki/DL3018) | Pin versions in apk add. Instead of `apk add ` use `apk add =`. | | [DL3019](https://github.com/hadolint/hadolint/wiki/DL3019) | Use the `--no-cache` switch to avoid the need to use `--update` and remove `/var/cache/apk/*` when done installing packages. | | [DL3020](https://github.com/hadolint/hadolint/wiki/DL3020) | Use `COPY` instead of `ADD` for files and folders. | -| [DL3021](https://github.com/hadolint/hadolint/wiki/DL3021) | `COPY` with more than 2 arguments requires the last argument to end with `/` | +| [DL3021](https://github.com/hadolint/hadolint/wiki/DL3021) | `COPY` with more than 2 arguments requires the last argument to end with `/` | | [DL3022](https://github.com/hadolint/hadolint/wiki/DL3022) | `COPY --from` should reference a previously defined `FROM` alias | | [DL3023](https://github.com/hadolint/hadolint/wiki/DL3023) | `COPY --from` cannot reference its own `FROM` alias | | [DL3024](https://github.com/hadolint/hadolint/wiki/DL3024) | `FROM` aliases (stage names) must be unique | @@ -209,8 +225,7 @@ ### AST Dockerfile syntax is fully described in the [Dockerfile reference][]. Just take -a look at [Syntax.hs](https://www.stackage.org/haddock/nightly-2018-01-07/language-docker-2.0.1/Language-Docker-Syntax.html) -in the `language-docker` project to see the AST definition. +a look at [Syntax.hs][] in the `language-docker` project to see the AST definition. ## Alternatives @@ -223,7 +238,7 @@ [appveyor-img]: https://ci.appveyor.com/api/projects/status//github/hadolint/hadolint?svg=true&branch=master [appveyor]: https://ci.appveyor.com/project/hadolint/hadolint/branch/master [license-img]: https://img.shields.io/badge/license-GPL--3-blue.svg -[license]: https://tldrlegal.com/license/gnu-general-public-license-v3-(gpl-3) +[license]: https://tldrlegal.com/l/gpl-3.0 [release-img]: https://img.shields.io/github/release/hadolint/hadolint.svg [release]: https://github.com/hadolint/hadolint/releases/latest [downloads-img]: https://img.shields.io/github/downloads/hadolint/hadolint/total.svg @@ -235,3 +250,4 @@ [integration]: docs/INTEGRATION.md [create an issue]: https://github.com/hadolint/hadolint/issues/new [dockerfile reference]: http://docs.docker.com/engine/reference/builder/ +[syntax.hs]: https://www.stackage.org/haddock/nightly-2018-01-07/language-docker-2.0.1/Language-Docker-Syntax.html diff --git a/hadolint.cabal b/hadolint.cabal --- a/hadolint.cabal +++ b/hadolint.cabal @@ -1,11 +1,11 @@ --- This file has been generated from package.yaml by hpack version 0.20.0. +-- This file has been generated from package.yaml by hpack version 0.21.2. -- -- see: https://github.com/sol/hpack -- --- hash: 0e73d59878f7e4b0a47b9ea2607d8a8de24ade73e92179f9c23870e7e0364c35 +-- hash: 8953f4990fda4e78c53418a6f7426bc5f60f367bf771ed9e7bf015eac0868247 name: hadolint -version: 1.5.0 +version: 1.6.0 synopsis: Dockerfile Linter JavaScript API description: A smarter Dockerfile linter that helps you build best practice Docker images. category: Development @@ -34,7 +34,7 @@ , base >=4.8 && <5 , bytestring , dlist - , language-docker >=2.0.0 + , language-docker >=3.0.0 , parsec >=3.1 , split >=0.2 , text @@ -60,8 +60,8 @@ , filepath , gitrev >=1.3.1 , hadolint - , language-docker >=2.0.0 - , optparse-applicative <=0.14.0.0 + , language-docker >=3.0.0 + , optparse-applicative , parsec >=3.1 , yaml if !(os(osx)) @@ -83,7 +83,7 @@ , bytestring >=0.10 , hadolint , hspec - , language-docker >=2.0.0 + , language-docker >=3.0.0 , parsec >=3.1 , split >=0.2 other-modules: diff --git a/src/Hadolint/Rules.hs b/src/Hadolint/Rules.hs --- a/src/Hadolint/Rules.hs +++ b/src/Hadolint/Rules.hs @@ -1,3 +1,5 @@ +{-# LANGUAGE NamedFieldPuns #-} + module Hadolint.Rules where import Control.Arrow ((&&&)) @@ -7,7 +9,10 @@ import Hadolint.Bash import Language.Docker.Syntax -import ShellCheck.Interface +import qualified ShellCheck.Interface +import ShellCheck.Interface (Severity(..)) +import qualified Text.Parsec as Parsec +import Text.Parsec ((<|>)) data Metadata = Metadata { code :: String @@ -70,7 +75,12 @@ -> Rule instructionRuleState code severity message = mapInstructions (Metadata code severity message) -dockerfileRule :: String -> Severity -> String -> ([Instruction] -> Bool) -> Rule +dockerfileRule :: + String -- ^ The rule code name + -> ShellCheck.Interface.Severity -- ^ The rule severity + -> String -- ^ The error message associated with the rule + -> ([Instruction] -> Bool) -- ^ The function that will check the rule + -> Rule dockerfileRule code severity message f = rule where rule dockerfile = @@ -82,8 +92,31 @@ analyze :: [Rule] -> Dockerfile -> [RuleCheck] analyze rules dockerfile = filter failed $ concat [r dockerfile | r <- rules] where - failed (RuleCheck _ _ _ success) = not success + failed RuleCheck {metadata = Metadata {code}, linenumber, success} = + not success && not (wasIgnored code linenumber) + wasIgnored c ln = not $ null [line | (line, codes) <- allIgnores, line == ln, c `elem` codes] + allIgnores = ignored dockerfile +ignored :: Dockerfile -> [(Linenumber, [String])] +ignored dockerfile = + [(l + 1, ignores) | (l, Just ignores) <- map (lineNumber &&& extractIgnored) dockerfile] + where + extractIgnored = ignoreFromInstruction . instruction + ignoreFromInstruction (Comment comment) = either (const Nothing) Just (parseComment comment) + ignoreFromInstruction _ = Nothing + -- | Parses the comment text and extracts the ignored rule names + parseComment :: String -> Either Parsec.ParseError [String] + parseComment = Parsec.parse commentParser "" + commentParser = + Parsec.skipMany space >> -- The parser for the ignored rules + Parsec.string "hadolint" >> + Parsec.skipMany1 space >> + Parsec.string "ignore=" >> + Parsec.skipMany space >> + Parsec.sepBy1 ruleName (Parsec.many space >> Parsec.char ',' >> Parsec.many space) + space = Parsec.char ' ' <|> Parsec.char '\t' + ruleName = Parsec.many1 (Parsec.choice $ map Parsec.char "DLSC0123456789") + rules = [ absoluteWorkdir , shellcheckBash @@ -124,7 +157,7 @@ shellcheckBash :: Dockerfile -> [RuleCheck] shellcheckBash = concatMap check where - check (InstructionPos (Run args) source linenumber) = + check (InstructionPos (Run (Arguments args)) source linenumber) = rmDup [RuleCheck m source linenumber False | m <- convert args] check _ = [] convert args = [commentMetadata c | c <- shellcheck $ unwords args] @@ -187,7 +220,7 @@ severity = WarningC message = "Multiple `CMD` instructions found. If you list more than one `CMD` then only the last \ - \`CMD` will take effect." + \`CMD` will take effect" check dockerfile = 1 >= length (filter (True ==) $ map isCmd dockerfile) isCmd (Cmd _) = True isCmd _ = False @@ -198,7 +231,7 @@ severity = ErrorC message = "Multiple `ENTRYPOINT` instructions found. If you list more than one `ENTRYPOINT` then \ - \only the last `ENTRYPOINT` will take effect." + \only the last `ENTRYPOINT` will take effect" check dockerfile = 1 >= length (filter (True ==) $ map isEntrypoint dockerfile) isEntrypoint (Entrypoint _) = True isEntrypoint _ = False @@ -211,7 +244,7 @@ check dockerfile = not $ anyCurl dockerfile && anyWget dockerfile anyCurl = any $ usingCmd "curl" anyWget = any $ usingCmd "wget" - usingCmd cmd (Run args) = cmd `elem` args + usingCmd cmd (Run (Arguments args)) = cmd `elem` args usingCmd _ _ = False invalidCmd = instructionRule code severity message check @@ -221,7 +254,7 @@ message = "For some bash commands it makes no sense running them in a Docker container like `ssh`, \ \`vim`, `shutdown`, `service`, `ps`, `free`, `top`, `kill`, `mount`, `ifconfig`" - check (Run args) = head args `notElem` invalidCmds + check (Run (Arguments args)) = head args `notElem` invalidCmds check _ = True invalidCmds = ["ssh", "vim", "shutdown", "service", "ps", "free", "top", "kill", "mount"] @@ -239,7 +272,7 @@ code = "DL3003" severity = WarningC message = "Use WORKDIR to switch to a directory" - check (Run args) = not $ usingProgram "cd" args + check (Run (Arguments args)) = not $ usingProgram "cd" args check _ = True noSudo = instructionRule code severity message check @@ -248,25 +281,26 @@ severity = ErrorC message = "Do not use sudo as it leads to unpredictable behavior. Use a tool like gosu to enforce \ - \root." - check (Run args) = not $ usingProgram "sudo" args + \root" + check (Run (Arguments args)) = not $ usingProgram "sudo" args check _ = True noAptGetUpgrade = instructionRule code severity message check where code = "DL3005" severity = ErrorC - message = "Do not use apt-get upgrade or dist-upgrade." - check (Run args) = not $ isInfixOf ["apt-get", "upgrade"] args + message = "Do not use apt-get upgrade or dist-upgrade" + check (Run (Arguments args)) = not $ isInfixOf ["apt-get", "upgrade"] args check _ = True noUntagged dockerfile = instructionRuleLine code severity message check dockerfile where code = "DL3006" severity = WarningC - message = "Always tag the version of an image explicitly." - check line (From (UntaggedImage "scratch" _)) = True - check line (From (UntaggedImage i _)) = i `elem` previouslyDefinedAliases line dockerfile + message = "Always tag the version of an image explicitly" + check line (From (UntaggedImage (Image _ "scratch") _)) = True + check line (From (UntaggedImage (Image _ i) _)) = + i `elem` previouslyDefinedAliases line dockerfile check _ _ = True noLatestTag = instructionRule code severity message check @@ -275,7 +309,7 @@ severity = WarningC message = "Using latest is prone to errors if the image will ever update. Pin the version explicitly \ - \to a release tag." + \to a release tag" check (From (TaggedImage _ tag _)) = tag /= "latest" check _ = True @@ -286,12 +320,17 @@ message = "Pin versions in apt get install. Instead of `apt-get install ` use `apt-get \ \install =`" - check (Run args) = and [versionFixed p | p <- aptGetPackages args] + check (Run (Arguments args)) = and [versionFixed p | p <- aptGetPackages args] check _ = True versionFixed package = "=" `isInfixOf` package aptGetPackages :: [String] -> [String] -aptGetPackages args = concat [filter noOption cmd | cmd <- bashCommands args, isAptGetInstall cmd] +aptGetPackages args = + concat + [ filter noOption (dropOptionsWithArg ["-t", "--target-release"] cmd) + | cmd <- bashCommands args + , isAptGetInstall cmd + ] where noOption arg = arg `notElem` options && not ("--" `isPrefixOf` arg) options = ["apt-get", "install", "-d", "-f", "-m", "-q", "-y", "-qq"] @@ -301,7 +340,7 @@ code = "DL3009" severity = InfoC message = "Delete the apt-get lists after installing something" - check (Run args) = not (hasUpdate args) || hasCleanup args + check (Run (Arguments args)) = not (hasUpdate args) || hasCleanup args check _ = True hasCleanup cmd = ["rm", "-rf", "/var/lib/apt/lists/*"] `isInfixOf` cmd hasUpdate cmd = ["apt-get", "update"] `isInfixOf` cmd @@ -317,7 +356,7 @@ code = "DL3017" severity = ErrorC message = "Do not use apk upgrade" - check (Run args) = not $ isInfixOf ["apk", "upgrade"] args + check (Run (Arguments args)) = not $ isInfixOf ["apk", "upgrade"] args check _ = True isApkAdd :: [String] -> Bool @@ -329,7 +368,7 @@ severity = WarningC message = "Pin versions in apk add. Instead of `apk add ` use `apk add =`" - check (Run args) = and [versionFixed p | p <- apkAddPackages args] + check (Run (Arguments args)) = and [versionFixed p | p <- apkAddPackages args] check _ = True versionFixed package = "=" `isInfixOf` package @@ -351,7 +390,7 @@ message = "Use the `--no-cache` switch to avoid the need to use `--update` and remove \ \`/var/cache/apk/*` when done installing packages" - check (Run args) = not (isApkAdd args) || hasNoCacheOption args + check (Run (Arguments args)) = not (isApkAdd args) || hasNoCacheOption args check _ = True hasNoCacheOption cmd = ["--no-cache"] `isInfixOf` cmd @@ -386,8 +425,9 @@ message = "Pin versions in pip. Instead of `pip install ` use `pip install \ \==`" - check (Run args) = - not (isPipInstall args && not (isRecursiveInstall args)) || all versionFixed (packages args) + check (Run (Arguments args)) = + not (isPipInstall args) || + (isRecursiveInstall args || isSetupPyInstall args || all versionFixed (packages args)) check _ = True isVersionedGit :: String -> Bool isVersionedGit package = "git+http" `isInfixOf` package && "@" `isInfixOf` package @@ -402,6 +442,8 @@ ["pip3", "install"] `isInfixOf` cmd || ["pip2", "install"] `isInfixOf` cmd isRecursiveInstall :: [String] -> Bool isRecursiveInstall cmd = ["-r"] `isInfixOf` cmd + isSetupPyInstall :: [String] -> Bool + isSetupPyInstall cmd = ["."] `isInfixOf` cmd -- | Returns all the packages after pip install packages :: [String] -> [String] packages args = concat [filter noOption cmd | cmd <- bashCommands args, isPipInstall cmd] @@ -428,7 +470,7 @@ message = "Pin versions in npm. Instead of `npm install ` use `npm install \ \@`" - check (Run args) = all versionFixed (packages args) + check (Run (Arguments args)) = all versionFixed (packages args) check _ = True packages :: [String] -> [String] packages args = concat [filter noOption cmd | cmd <- bashCommands args, isNpmInstall cmd] @@ -462,7 +504,7 @@ code = "DL3014" severity = WarningC message = "Use the `-y` switch to avoid manual input `apt-get -y install `" - check (Run args) = not (isAptGetInstall args) || hasYesOption args + check (Run (Arguments args)) = not (isAptGetInstall args) || hasYesOption args check _ = True hasYesOption cmd = ["-y"] `isInfixOf` cmd || @@ -474,7 +516,7 @@ code = "DL3015" severity = InfoC message = "Avoid additional packages by specifying `--no-install-recommends`" - check (Run args) = not (isAptGetInstall args) || hasNoRecommendsOption args + check (Run (Arguments args)) = not (isAptGetInstall args) || hasNoRecommendsOption args check _ = True hasNoRecommendsOption cmd = ["--no-install-recommends"] `isInfixOf` cmd @@ -559,6 +601,6 @@ code = "DL4005" severity = WarningC message = "Use SHELL to change the default shell" - check (Run args) = not $ any shellSymlink (bashCommands args) + check (Run (Arguments args)) = not $ any shellSymlink (bashCommands args) check _ = True shellSymlink args = usingProgram "ln" args && isInfixOf ["/bin/sh"] args diff --git a/test/Spec.hs b/test/Spec.hs --- a/test/Spec.hs +++ b/test/Spec.hs @@ -20,7 +20,7 @@ it "explicit tagged" $ ruleCatchesNot noLatestTag "FROM debian:jessie" it "explicit SHA" $ ruleCatchesNot noLatestTag - "FROM debian@sha256:\ + "FROM hub.docker.io/debian@sha256:\ \7959ed6f7e35f8b1aaa06d1d8259d4ee25aa85a086d5c125480c333183f9deeb" it "explicit tagged with name" $ ruleCatchesNot noLatestTag "FROM debian:jessie AS builder" @@ -145,6 +145,8 @@ ruleCatchesNot pipVersionPinned "RUN pip3 install MySQL_python==1.2.2" it "pip install requirements" $ ruleCatchesNot pipVersionPinned "RUN pip install -r requirements.txt" + it "pip install use setup.py" $ + ruleCatchesNot pipVersionPinned "RUN pip install ." it "pip version not pinned" $ ruleCatches pipVersionPinned "RUN pip install MySQL_python" it "pip version pinned" $ @@ -259,6 +261,18 @@ ruleCatchesNot aptGetVersionPinned "RUN apt-get -y --no-install-recommends install nodejs=0.10" + it "apt-get tolerate target-release" $ + let dockerfile = + [ "RUN set -e &&\\" + , " echo \"deb http://http.debian.net/debian jessie-backports main\" \ + \> /etc/apt/sources.list.d/jessie-backports.list &&\\" + , " apt-get update &&\\" + , " apt-get install -y --no-install-recommends -t jessie-backports \ + \openjdk-8-jdk=8u131-b11-1~bpo8+1 &&\\" + , " rm -rf /var/lib/apt/lists/*" + ] + in ruleCatchesNot aptGetVersionPinned $ unlines dockerfile + it "has maintainer" $ ruleCatches hasNoMaintainer "FROM debian\nMAINTAINER Lukas" it "has maintainer first" $ ruleCatches hasNoMaintainer "MAINTAINER Lukas\nFROM DEBIAN" it "has no maintainer" $ ruleCatchesNot hasNoMaintainer "FROM debian" @@ -349,6 +363,49 @@ case ast of Left err -> assertEqual "Unexpected error msg" expectedMsg (formatError err) Right _ -> assertFailure "AST should fail parsing" + -- + describe "Rules can be ignored with inline comments" $ do + it "ignores single rule" $ + let dockerfile = + [ "FROM ubuntu" + , "# hadolint ignore=DL3002" + , "USER root" + ] + in ruleCatchesNot noRootUser $ unlines dockerfile + it "ignores only the given rule" $ + let dockerfile = + [ "# hadolint ignore=DL3001" + , "USER root" + ] + in ruleCatches noRootUser $ unlines dockerfile + it "ignores only the given rule, when multiple passed" $ + let dockerfile = + [ "# hadolint ignore=DL3001,DL3002" + , "USER root" + ] + in ruleCatchesNot noRootUser $ unlines dockerfile + it "ignores the rule only if directly above the instruction" $ + let dockerfile = + [ "# hadolint ignore=DL3001,DL3002" + , "FROM ubuntu" + , "USER root" + ] + in ruleCatches noRootUser $ unlines dockerfile + it "won't ignore the rule if passed invalid rule names" $ + let dockerfile = + [ "# hadolint ignore=crazy,DL3002" + , "USER root" + ] + in ruleCatches noRootUser $ unlines dockerfile + it "ignores multiple rules correctly, even with some extra whitespace" $ + let dockerfile = + [ "FROM node as foo" + , "# hadolint ignore=DL3023, DL3021" + , "COPY --from=foo bar baz ." + ] + in do + ruleCatchesNot copyFromAnother $ unlines dockerfile + ruleCatchesNot copyEndingSlash $ unlines dockerfile assertChecks rule s f = case parseString (s ++ "\n") of