feat: add dockerfile support to skaffold lint and top 2 dockerfile rules

cmd/skaffold/app/cmd/lint.go

@@ -23,6 +23,7 @@ import (
 	"github.com/spf13/cobra"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/lint"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/output/log"
 )
 
 var outFormat string
@@ -35,10 +36,16 @@ func NewCmdLint() *cobra.Command {
 			{Value: &outFormat, Name: "format", DefValue: lint.PlainTextOutput,
 				Usage: "Output format. One of: plain-text(default) or json"}}).
 		Hidden().
-		NoArgs(lintCmd)
+		NoArgs(doLint)
 }
 
-func lintCmd(ctx context.Context, out io.Writer) error {
+func doLint(ctx context.Context, out io.Writer) error {
+	// createRunner initializes state for objects (Docker client, etc.) lint uses
+	_, _, runCtx, err := createRunner(ctx, out, opts)
+	log.Entry(ctx).Debugf("starting skaffold lint with runCtx: %v", runCtx)
+	if err != nil {
+		return err
+	}
 	return lint.Lint(ctx, out, lint.Options{
 		Filename:     opts.ConfigurationFile,
 		RepoCacheDir: opts.RepoCacheDir,

pkg/skaffold/lint/dockerfiles.go

@@ -0,0 +1,169 @@
+/*
+Copyright 2021 The Skaffold Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package lint
+
+import (
+	"context"
+	"io/ioutil"
+	"path/filepath"
+
+	"github.com/moby/buildkit/frontend/dockerfile/command"
+
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/output/log"
+)
+
+// for testing
+var getDockerDependencies = docker.GetDependencies
+var dockerfileRules = &dockerfileLintRules
+
+var DockerfileLinters = []Linter{
+	&RegExpLinter{},
+	&DockerfileCommandLinter{},
+}
+
+var dockerfileLintRules = []Rule{
+	{
+		RuleType: DockerfileCommandLintRule,
+		Filter: DockerCommandFilter{
+			DockerCommand:          command.Copy,
+			DockerCopySourceRegExp: ".*",
+		},
+		RuleID: DockerfileCopyOver1000Files,
+		ExplanationTemplate: `Found 'COPY {{index .FieldMap "src"}} {{index .FieldMap "dest"}}', where the source dir: "{{index .FieldMap "src"}}" has over 1000 files.  This has the potential to dramatically slow 'skaffold dev' down ` +
+			`as skaffold watches all sources files referenced in dockerfile COPY directives for changes. ` +
+			`If you notice skaffold rebuilding images unnecessarily when non-image-critical files are ` +
+			`modified, consider changing this to 'COPY $REQUIRED_SOURCE_FILE(s) {{index .FieldMap "dest"}}' for each required source file instead of ` +
+			`or adding a .dockerignore file (https://docs.docker.com/engine/reference/builder/#dockerignore-file) ignoring non-image-critical files.  skaffold respects files ignored via the .dockerignore`,
+		ExplanationPopulator: func(params InputParams) (explanationInfo, error) {
+			return explanationInfo{
+				FieldMap: map[string]interface{}{
+					"src":  params.DockerCopyCommandInfo.src,
+					"dest": params.DockerCopyCommandInfo.dest,
+				},
+			}, nil
+		},
+		LintConditions: []func(InputParams) bool{func(params InputParams) bool {
+			files := 0
+			for range params.DockerfileToDepMap[params.ConfigFile.AbsPath] {
+				files++
+			}
+			return files > 1000
+		}},
+	},
+	{
+		RuleType: DockerfileCommandLintRule,
+		Filter: DockerCommandFilter{
+			DockerCommand:          command.Copy,
+			DockerCopySourceRegExp: ".*",
+		},
+		RuleID: DockerfileCopyContainsGitDir,
+		// TODO(aaron-prindle) suggest a full .dockerignore sample - .dockerignore:**/.git
+		ExplanationTemplate: `Found 'COPY {{index .FieldMap "src"}} {{index .FieldMap "dest"}}', where the source dir: "{{index .FieldMap "src"}}" contains a '.git' directory at {{index .FieldMap "gitDirectoryAbsPath"}}.  This has the potential to dramatically slow 'skaffold dev' down ` +
+			`as skaffold whill watch all of the files in the .git directory as skaffold watches all sources files referenced in dockerfile COPY directives for changes. ` +
+			`skaffold will likely rebuild images unnecessarily when non-image-critical files are ` +
+			`modified during any git related operation. Consider adding a .dockerignore file (https://docs.docker.com/engine/reference/builder/#dockerignore-file) removing the '.git' directory. skaffold respects files ignored via the .dockerignore`,
+		ExplanationPopulator: func(params InputParams) (explanationInfo, error) {
+			var gitDirectoryAbsPath string
+			for _, dep := range params.DockerfileToDepMap[params.ConfigFile.AbsPath] {
+				dir := filepath.Dir(dep)
+				if dir == ".git" {
+					gitDirectoryAbsPath = filepath.Join(params.WorkspacePath, dir)
+				}
+			}
+			return explanationInfo{
+				FieldMap: map[string]interface{}{
+					"src":                 params.DockerCopyCommandInfo.src,
+					"dest":                params.DockerCopyCommandInfo.dest,
+					"gitDirectoryAbsPath": gitDirectoryAbsPath,
+				},
+			}, nil
+		},
+		LintConditions: []func(InputParams) bool{func(params InputParams) bool {
+			for _, dep := range params.DockerfileToDepMap[params.ConfigFile.AbsPath] {
+				if filepath.Dir(dep) == ".git" {
+					return true
+				}
+			}
+			return false
+		}},
+	},
+}
+
+func GetDockerfilesLintResults(ctx context.Context, opts Options) (*[]Result, error) {
+	cfgs, err := getConfigSet(ctx, config.SkaffoldOptions{
+		ConfigurationFile:   opts.Filename,
+		ConfigurationFilter: opts.Modules,
+		RepoCacheDir:        opts.RepoCacheDir,
+		Profiles:            opts.Profiles,
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	l := []Result{}
+	seen := map[string]bool{}
+	dockerfileDepMap := map[string][]string{}
+	workdir, err := realWorkDir()
+	if err != nil {
+		return nil, err
+	}
+
+	for _, c := range cfgs {
+		for _, a := range c.Build.Artifacts {
+			if a.DockerArtifact != nil {
+				ws := filepath.Join(workdir, a.Workspace)
+				fp := filepath.Join(ws, a.DockerArtifact.DockerfilePath)
+				if _, ok := seen[fp]; ok {
+					continue
+				}
+				seen[fp] = true
+				b, err := ioutil.ReadFile(fp)
+				if err != nil {
+					return nil, err
+				}
+				dockerfile := ConfigFile{
+					AbsPath: fp,
+					RelPath: filepath.Join(a.Workspace, a.DockerArtifact.DockerfilePath),
+					Text:    string(b),
+				}
+				// TODO(aaron-prindle) currently this dep map is computed twice; here and in skaffoldyamls.go, make a singleton/share-the-info
+				deps, err := getDockerDependencies(context.TODO(),
+					docker.NewBuildConfig(ws, a.ImageName, fp, map[string]*string{}), nil)
+				if err != nil {
+					return nil, err
+				}
+				log.Entry(context.TODO()).Infof("dockerfile: %s, deps: %v", fp, deps)
+				dockerfileDepMap[fp] = deps
+				for _, r := range DockerfileLinters {
+					recs, err := r.Lint(InputParams{
+						ConfigFile:         dockerfile,
+						SkaffoldConfig:     c,
+						DockerfileToDepMap: dockerfileDepMap,
+						WorkspacePath:      ws,
+					}, &dockerfileLintRules)
+					if err != nil {
+						return nil, err
+					}
+					l = append(l, *recs...)
+				}
+			}
+		}
+	}
+	return &l, nil
+}