Skip to content

Commit

Permalink
feat: add dockerfile support to skaffold lint and top 2 dockerfile ru…
Browse files Browse the repository at this point in the history
…les (#6793)
  • Loading branch information
aaron-prindle authored Nov 9, 2021
1 parent 3a57cc8 commit fe0b543
Show file tree
Hide file tree
Showing 13 changed files with 688 additions and 62 deletions.
13 changes: 10 additions & 3 deletions cmd/skaffold/app/cmd/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -35,15 +36,21 @@ 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,
OutFormat: outFormat,
Modules: opts.ConfigurationFilter,
Profiles: opts.Profiles,
})
}, runCtx)
}
73 changes: 71 additions & 2 deletions pkg/skaffold/docker/dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,18 @@ func GetDependencies(ctx context.Context, buildCfg BuildConfig, cfg Config) ([]s
return resultPair(result)
}

// GetDependencies finds the sources dependency for the given docker artifact.
// it caches the results for the computed dependency which can be used by `GetDependenciesCached`
// All paths are relative to the workspace.
func GetDependenciesByDockerCopyFromTo(ctx context.Context, buildCfg BuildConfig, cfg Config) (map[string][]string, error) {
absDockerfilePath, err := NormalizeDockerfilePath(buildCfg.workspace, buildCfg.dockerfilePath)
if err != nil {
return nil, fmt.Errorf("normalizing dockerfilePath path: %w", err)
}
ftToDependencies := getDependenciesByDockerCopyFromTo(ctx, buildCfg.workspace, buildCfg.dockerfilePath, absDockerfilePath, buildCfg.args, cfg)
return resultPairForDockerCopyFromTo(ftToDependencies)
}

// GetDependenciesCached reads from cache finds the sources dependency for the given docker artifact.
// All paths are relative to the workspace.
func GetDependenciesCached(ctx context.Context, buildCfg BuildConfig, cfg Config) ([]string, error) {
Expand All @@ -103,6 +115,17 @@ func resultPair(deps interface{}) ([]string, error) {
}
}

func resultPairForDockerCopyFromTo(deps interface{}) (map[string][]string, error) {
switch t := deps.(type) {
case error:
return nil, t
case map[string][]string:
return t, nil
default:
return nil, fmt.Errorf("internal error when retrieving cache result of type %T", t)
}
}

func getDependencies(ctx context.Context, workspace string, dockerfilePath string, absDockerfilePath string, buildArgs map[string]*string, cfg Config) interface{} {
// If the Dockerfile doesn't exist, we can't compute the dependency.
// But since we know the Dockerfile is a dependency, let's return a list
Expand All @@ -112,7 +135,7 @@ func getDependencies(ctx context.Context, workspace string, dockerfilePath strin
return []string{dockerfilePath}
}

fts, err := readCopyCmdsFromDockerfile(ctx, false, absDockerfilePath, workspace, buildArgs, cfg)
fts, err := ReadCopyCmdsFromDockerfile(ctx, false, absDockerfilePath, workspace, buildArgs, cfg)
if err != nil {
return err
}
Expand All @@ -124,7 +147,7 @@ func getDependencies(ctx context.Context, workspace string, dockerfilePath strin

deps := make([]string, 0, len(fts))
for _, ft := range fts {
deps = append(deps, ft.from)
deps = append(deps, ft.From)
}

files, err := WalkWorkspace(workspace, excludes, deps)
Expand All @@ -151,6 +174,52 @@ func getDependencies(ctx context.Context, workspace string, dockerfilePath strin
return dependencies
}

func getDependenciesByDockerCopyFromTo(ctx context.Context, workspace string, dockerfilePath string, absDockerfilePath string, buildArgs map[string]*string, cfg Config) interface{} {
// If the Dockerfile doesn't exist, we can't compute the dependency.
// But since we know the Dockerfile is a dependency, let's return a list
// with only that file. It makes errors down the line more actionable
// than returning an error now.
if _, err := os.Stat(absDockerfilePath); os.IsNotExist(err) {
return []string{dockerfilePath}
}

fts, err := ReadCopyCmdsFromDockerfile(ctx, false, absDockerfilePath, workspace, buildArgs, cfg)
if err != nil {
return err
}

excludes, err := readDockerignore(workspace, absDockerfilePath)
if err != nil {
return fmt.Errorf("reading .dockerignore: %w", err)
}

ftToDependencies := map[string][]string{}
for _, ft := range fts {
files, err := WalkWorkspace(workspace, excludes, []string{ft.From})
if err != nil {
return fmt.Errorf("walking workspace: %w", err)
}

// Always add dockerfile even if it's .dockerignored. The daemon will need it anyways.
if !filepath.IsAbs(dockerfilePath) {
files[dockerfilePath] = true
} else {
files[absDockerfilePath] = true
}

// Ignore .dockerignore
delete(files, ".dockerignore")

var dependencies []string
for file := range files {
dependencies = append(dependencies, file)
}
sort.Strings(dependencies)
ftToDependencies[ft.String()] = dependencies
}
return ftToDependencies
}

// readDockerignore reads patterns to ignore
func readDockerignore(workspace string, absDockerfilePath string) ([]string, error) {
var excludes []string
Expand Down
77 changes: 62 additions & 15 deletions pkg/skaffold/docker/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,23 @@ import (
"github.com/GoogleContainerTools/skaffold/proto/v1"
)

type FromTo struct {
// From is the relative path (wrt. the skaffold root directory) of the dependency on the host system.
From string
// To is the destination location in the container. Must use slashes as path separator.
To string
// ToIsDir indicates if the `to` path must be treated as directory
ToIsDir bool
// StartLine indicates the starting line in the dockerfile of the copy command
StartLine int
// EndLine indiciates the ending line in the dockerfile of the copy command
EndLine int
}

func (f *FromTo) String() string {
return fmt.Sprintf("From:%s, To:%s, ToIsDir:%t, StartLine: %d, EndLine: %d", f.From, f.To, f.ToIsDir, f.StartLine, f.EndLine)
}

type from struct {
image string
as string
Expand All @@ -55,23 +72,20 @@ type copyCommand struct {
dest string
// destIsDir indicates if dest must be treated as directory.
destIsDir bool
}

type fromTo struct {
// from is the relative path (wrt. the skaffold root directory) of the dependency on the host system.
from string
// to is the destination location in the container. Must use slashes as path separator.
to string
// toIsDir indicates if the `to` path must be treated as directory
toIsDir bool
// startLine indicates the starting line in the dockerfile of the copy command
startLine int
// endLine indiciates the ending line in the dockerfile of the copy command
endLine int
}

var (
// RetrieveImage is overridden for unit testing
RetrieveImage = retrieveImage
)

func readCopyCmdsFromDockerfile(ctx context.Context, onlyLastImage bool, absDockerfilePath, workspace string, buildArgs map[string]*string, cfg Config) ([]fromTo, error) {
// ReadCopyCmdsFromDockerfile parses a given dockerfile for COPY commands accounting for build args, env vars, globs, etc
// and returns an array of FromTos specifying the files that will be copied 'from' local dirs 'to' container dirs in the COPY statements
func ReadCopyCmdsFromDockerfile(ctx context.Context, onlyLastImage bool, absDockerfilePath, workspace string, buildArgs map[string]*string, cfg Config) ([]FromTo, error) {
r, err := ioutil.ReadFile(absDockerfilePath)
if err != nil {
return nil, err
Expand Down Expand Up @@ -105,6 +119,38 @@ func readCopyCmdsFromDockerfile(ctx context.Context, onlyLastImage bool, absDock
return expandSrcGlobPatterns(workspace, cpCmds)
}

func ExtractOnlyCopyCommands(absDockerfilePath string) ([]FromTo, error) {
r, err := ioutil.ReadFile(absDockerfilePath)
if err != nil {
return nil, err
}

res, err := parser.Parse(bytes.NewReader(r))
if err != nil {
return nil, fmt.Errorf("parsing dockerfile %q: %w", absDockerfilePath, err)
}

var copied []FromTo
workdir := "/"
envs := make([]string, 0)
for _, node := range res.AST.Children {
switch node.Value {
case command.Add, command.Copy:
cpCmd, err := readCopyCommand(node, envs, workdir)
if err != nil {
return nil, err
}

if cpCmd != nil && len(cpCmd.srcs) > 0 {
for _, src := range cpCmd.srcs {
copied = append(copied, FromTo{From: src, To: cpCmd.dest, ToIsDir: cpCmd.destIsDir, StartLine: cpCmd.startLine, EndLine: cpCmd.endLine})
}
}
}
}
return copied, nil
}

// filterUnusedBuildArgs removes entries from the build arguments map that are not found in the dockerfile
func filterUnusedBuildArgs(dockerFile io.Reader, buildArgs map[string]*string) (map[string]*string, error) {
res, err := parser.Parse(dockerFile)
Expand Down Expand Up @@ -162,15 +208,15 @@ func expandBuildArgs(nodes []*parser.Node, buildArgs map[string]*string) error {
return nil
}

func expandSrcGlobPatterns(workspace string, cpCmds []*copyCommand) ([]fromTo, error) {
var fts []fromTo
func expandSrcGlobPatterns(workspace string, cpCmds []*copyCommand) ([]FromTo, error) {
var fts []FromTo
for _, cpCmd := range cpCmds {
matchesOne := false

for _, p := range cpCmd.srcs {
path := filepath.Join(workspace, p)
if _, err := os.Stat(path); err == nil {
fts = append(fts, fromTo{from: filepath.Clean(p), to: cpCmd.dest, toIsDir: cpCmd.destIsDir})
fts = append(fts, FromTo{From: filepath.Clean(p), To: cpCmd.dest, ToIsDir: cpCmd.destIsDir, StartLine: cpCmd.startLine, EndLine: cpCmd.endLine})
matchesOne = true
continue
}
Expand All @@ -189,7 +235,7 @@ func expandSrcGlobPatterns(workspace string, cpCmds []*copyCommand) ([]fromTo, e
return nil, fmt.Errorf("getting relative path of %s", f)
}

fts = append(fts, fromTo{from: rel, to: cpCmd.dest, toIsDir: cpCmd.destIsDir})
fts = append(fts, FromTo{From: rel, To: cpCmd.dest, ToIsDir: cpCmd.destIsDir, StartLine: cpCmd.startLine, EndLine: cpCmd.endLine})
}
matchesOne = true
}
Expand Down Expand Up @@ -266,7 +312,6 @@ func extractCopyCommands(ctx context.Context, nodes []*parser.Node, onlyLastImag
}
}
}

return copied, nil
}

Expand Down Expand Up @@ -311,6 +356,8 @@ func readCopyCommand(value *parser.Node, envs []string, workdir string) (*copyCo
srcs: srcs,
dest: dest,
destIsDir: destIsDir,
startLine: value.StartLine,
endLine: value.EndLine,
}, nil
}

Expand Down
18 changes: 9 additions & 9 deletions pkg/skaffold/docker/syncmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func SyncMap(ctx context.Context, workspace string, dockerfilePath string, build
}

// only the COPY/ADD commands from the last image are syncable
fts, err := readCopyCmdsFromDockerfile(ctx, true, absDockerfilePath, workspace, buildArgs, cfg)
fts, err := ReadCopyCmdsFromDockerfile(ctx, true, absDockerfilePath, workspace, buildArgs, cfg)
if err != nil {
return nil, err
}
Expand All @@ -57,7 +57,7 @@ func SyncMap(ctx context.Context, workspace string, dockerfilePath string, build
// walkWorkspaceWithDestinations walks the given host directories and determines their
// location in the container. It returns a map of host path by container destination.
// Note: if you change this function, you might also want to modify `WalkWorkspace`.
func walkWorkspaceWithDestinations(workspace string, excludes []string, fts []fromTo) (map[string]string, error) {
func walkWorkspaceWithDestinations(workspace string, excludes []string, fts []FromTo) (map[string]string, error) {
dockerIgnored, err := NewDockerIgnorePredicate(workspace, excludes)
if err != nil {
return nil, err
Expand All @@ -66,7 +66,7 @@ func walkWorkspaceWithDestinations(workspace string, excludes []string, fts []fr
// Walk the workspace
srcByDest := make(map[string]string)
for _, ft := range fts {
absFrom := filepath.Join(workspace, ft.from)
absFrom := filepath.Join(workspace, ft.From)

fi, err := os.Stat(absFrom)
if err != nil {
Expand Down Expand Up @@ -100,23 +100,23 @@ func walkWorkspaceWithDestinations(workspace string, excludes []string, fts []fr
return err
}

srcByDest[path.Join(ft.to, filepath.ToSlash(relBase))] = relPath
srcByDest[path.Join(ft.To, filepath.ToSlash(relBase))] = relPath
return nil
}); err != nil {
return nil, fmt.Errorf("walking %q: %w", absFrom, err)
}
case mode.IsRegular():
ignored, err := dockerIgnored(filepath.Join(workspace, ft.from), fi)
ignored, err := dockerIgnored(filepath.Join(workspace, ft.From), fi)
if err != nil {
return nil, err
}

if !ignored {
if ft.toIsDir {
base := filepath.Base(ft.from)
srcByDest[path.Join(ft.to, base)] = ft.from
if ft.ToIsDir {
base := filepath.Base(ft.From)
srcByDest[path.Join(ft.To, base)] = ft.From
} else {
srcByDest[ft.to] = ft.from
srcByDest[ft.To] = ft.From
}
}
}
Expand Down
Loading

0 comments on commit fe0b543

Please sign in to comment.