Skip to content

Commit

Permalink
check command casing after parsing the ast
Browse files Browse the repository at this point in the history
Signed-off-by: Talon Bowler <[email protected]>
  • Loading branch information
daghack committed Aug 20, 2024
1 parent 664c2b4 commit e7779a5
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 16 deletions.
48 changes: 32 additions & 16 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,8 +251,6 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
}
}

validateCommandCasing(dockerfile, lint)

proxyEnv := proxyEnvFromBuildArgs(opt.BuildArgs)

stages, metaArgs, err := instructions.Parse(dockerfile.AST, lint)
Expand All @@ -263,6 +261,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
return nil, errors.New("dockerfile contains no stages to build")
}
validateStageNames(stages, lint)
validateCommandCasing(stages, lint)

shlex := shell.NewLex(dockerfile.EscapeToken)
outline := newOutlineCapture()
Expand Down Expand Up @@ -2199,32 +2198,49 @@ func isSelfConsistentCasing(s string) bool {
return s == strings.ToLower(s) || s == strings.ToUpper(s)
}

func validateCommandCasing(dockerfile *parser.Result, lint *linter.Linter) {
func validateCaseMatch(name string, isMajorityLower bool, location []parser.Range, lint *linter.Linter) {
var correctCasing string
if isMajorityLower && strings.ToLower(name) != name {
correctCasing = "lowercase"
} else if !isMajorityLower && strings.ToUpper(name) != name {
correctCasing = "uppercase"
}
if correctCasing != "" {
msg := linter.RuleConsistentInstructionCasing.Format(name, correctCasing)
lint.Run(&linter.RuleConsistentInstructionCasing, location, msg)
}
}

func validateCommandCasing(stages []instructions.Stage, lint *linter.Linter) {
var lowerCount, upperCount int
for _, node := range dockerfile.AST.Children {
if isSelfConsistentCasing(node.Value) {
if strings.ToLower(node.Value) == node.Value {
for _, stage := range stages {
if isSelfConsistentCasing(stage.OrigCmd) {
if strings.ToLower(stage.OrigCmd) == stage.OrigCmd {
lowerCount++
} else {
upperCount++
}
}
for _, cmd := range stage.Commands {
cmdName := cmd.Name()
if isSelfConsistentCasing(cmdName) {
if strings.ToLower(cmdName) == cmdName {
lowerCount++
} else {
upperCount++
}
}
}
}

isMajorityLower := lowerCount > upperCount
for _, node := range dockerfile.AST.Children {
for _, stage := range stages {
// Here, we check both if the command is consistent per command (ie, "CMD" or "cmd", not "Cmd")
// as well as ensuring that the casing is consistent throughout the dockerfile by comparing the
// command to the casing of the majority of commands.
var correctCasing string
if isMajorityLower && strings.ToLower(node.Value) != node.Value {
correctCasing = "lowercase"
} else if !isMajorityLower && strings.ToUpper(node.Value) != node.Value {
correctCasing = "uppercase"
}
if correctCasing != "" {
msg := linter.RuleConsistentInstructionCasing.Format(node.Value, correctCasing)
lint.Run(&linter.RuleConsistentInstructionCasing, node.Location(), msg)
validateCaseMatch(stage.OrigCmd, isMajorityLower, stage.Location, lint)
for _, cmd := range stage.Commands {
validateCaseMatch(cmd.Name(), isMajorityLower, cmd.Location(), lint)
}
}
}
Expand Down
8 changes: 8 additions & 0 deletions frontend/dockerfile/dockerfile_lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,14 @@ copy Dockerfile /bar
FROM scratch
COPY Dockerfile /foo
COPY Dockerfile /bar
`)
checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})

dockerfile = []byte(`
FROM alpine
RUN <<'EOT'
env
EOT
`)
checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
}
Expand Down
1 change: 1 addition & 0 deletions frontend/dockerfile/instructions/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,7 @@ type ShellCommand struct {
type Stage struct {
Name string // name of the stage
Commands []Command // commands contained within the stage
OrigCmd string // original FROM command, used for rule checks
BaseName string // name of the base stage or source
Platform string // platform of base source to use

Expand Down
1 change: 1 addition & 0 deletions frontend/dockerfile/instructions/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ func parseFrom(req parseRequest) (*Stage, error) {
code := strings.TrimSpace(req.original)
return &Stage{
BaseName: req.args[0],
OrigCmd: req.command,
Name: stageName,
SourceCode: code,
Commands: []Command{},
Expand Down

0 comments on commit e7779a5

Please sign in to comment.