linter: add lint rule for required json arguments #4889
Conversation
| return hasShellCommand(ds.stage.BaseName, states) | ||
| } | ||
|
|
||
| func validateJsonArgs(stage instructions.Stage, allDispatchStates *dispatchStates, warn linter.LintWarnFunc) { |
There was a problem hiding this comment.
Linter wants this validateJSONArgs
jsternberg
force-pushed
the
lint-json-args-required-sans-shell
branch
from
37b454b
to
c306ac6
Compare
|
After an internal conversation, I've updated this to be more accurate by using the image configuration. As a result, this lint will not be run when a stage is not included in the build, but the accuracy will be 100% with no false positives. |
jsternberg
force-pushed
the
lint-json-args-required-sans-shell
branch
from
c306ac6
to
92bbd6a
Compare
jsternberg
force-pushed
the
lint-json-args-required-sans-shell
branch
from
92bbd6a
to
ecc09c1
Compare
| var shell []string | ||
| if len(img.Config.Shell) > 0 { | ||
| shell = append([]string{}, img.Config.Shell...) | ||
| } else { | ||
| if warn != nil { | ||
| msg := linter.RuleJSONArgsRequired.Format(cmd.Name()) |
There was a problem hiding this comment.
This doesn't look like a rule directly related to withShell helper, but related to how ENTRYPOINT/CMD work (or more how these properties will work in docker run afterward). I think it makes more sense if there is a
if PrependShell && len(img.Config.Shell) == 0 {
report()
}
In dispatchArg/Entrypoint.
| Warnings: []expectedLintWarning{ | ||
| { | ||
| RuleName: "JSONArgsRequired", | ||
| Description: "JSON arguments required unless a custom shell is specified", |
There was a problem hiding this comment.
"required" may be strong word here as build would work fine without it. "preferred/recommended" maybe. Not sure if we need to mention "custom shell" as well as unlikely that the user would know anything about custom shells (and Dockerfile does not contain any SHELL). The warning is already not shown if there is a custom shell.
jsternberg
force-pushed
the
lint-json-args-required-sans-shell
branch
from
ecc09c1
to
cd2038a
Compare
This lint requires json arguments to be used when the `SHELL` command isn't used previously for `ENTRYPOINT` and `CMD`. This is because using non-json arguments can block signals from properly being handled when used in shell mode. This uses the `Shell` attribute on the image configuration and therefore requires the stage to be included in the build for it to properly run. This is done because we don't want to force image config resolution on every stage even if the stage isn't part of the build, but the only way to definitively know if a custom shell is set is to look at the image configuration. Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
jsternberg
force-pushed
the
lint-json-args-required-sans-shell
branch
from
cd2038a
to
79b0c96
Compare
This lint requires json arguments to be used when the
SHELLcommand isn't used previously forENTRYPOINTandCMD. This is because using non-json arguments can block signals from properly being handled when used in shell mode.This uses the
Shellattribute on the image configuration and thereforerequires the stage to be included in the build for it to properly run.
This is done because we don't want to force image config resolution on
every stage even if the stage isn't part of the build, but the only way
to definitively know if a custom shell is set is to look at the image
configuration.