From 3422d5572af36c70c7875e4861703cb1c8588548 Mon Sep 17 00:00:00 2001 From: Taylor Barrella Date: Tue, 23 Jul 2019 18:10:22 -0400 Subject: [PATCH] Misc. small changes/refactoring (#712) --- cmd/executor/cmd/root.go | 2 +- pkg/commands/add.go | 2 +- pkg/commands/expose.go | 2 +- pkg/commands/volume.go | 2 +- pkg/dockerfile/dockerfile.go | 2 +- pkg/snapshot/layered_map.go | 4 ++-- pkg/snapshot/snapshot.go | 6 +++--- pkg/util/command_util.go | 26 ++++++++++++-------------- pkg/util/fs_util.go | 9 ++------- 9 files changed, 24 insertions(+), 31 deletions(-) diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index 1fc2672bc2..0dd717f86a 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -208,12 +208,12 @@ func resolveSourceContext() error { } if opts.Bucket != "" { if !strings.Contains(opts.Bucket, "://") { + // if no prefix use Google Cloud Storage as default for backwards compatibility opts.SrcContext = constants.GCSBuildContextPrefix + opts.Bucket } else { opts.SrcContext = opts.Bucket } } - // if no prefix use Google Cloud Storage as default for backwards compatibility contextExecutor, err := buildcontext.GetBuildContext(opts.SrcContext) if err != nil { return err diff --git a/pkg/commands/add.go b/pkg/commands/add.go index 72f97653c9..27d6666461 100644 --- a/pkg/commands/add.go +++ b/pkg/commands/add.go @@ -43,7 +43,7 @@ type AddCommand struct { // - If remote file has HTTP Last-Modified header, we set the mtime of the file to that timestamp // - If dest doesn't end with a slash, the filepath is inferred to be / // 2. If is a local tar archive: -// -If is a local tar archive, it is unpacked at the dest, as 'tar -x' would +// - it is unpacked at the dest, as 'tar -x' would func (a *AddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { replacementEnvs := buildArgs.ReplacementEnvs(config.Env) diff --git a/pkg/commands/expose.go b/pkg/commands/expose.go index fa497f17d4..9d56ee3eab 100644 --- a/pkg/commands/expose.go +++ b/pkg/commands/expose.go @@ -54,7 +54,7 @@ func (r *ExposeCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile. } protocol := strings.Split(p, "/")[1] if !validProtocol(protocol) { - return fmt.Errorf("Invalid protocol: %s", protocol) + return fmt.Errorf("invalid protocol: %s", protocol) } logrus.Infof("Adding exposed port: %s", p) existingPorts[p] = struct{}{} diff --git a/pkg/commands/volume.go b/pkg/commands/volume.go index 2ec0fcb2bf..b6d92bf53a 100644 --- a/pkg/commands/volume.go +++ b/pkg/commands/volume.go @@ -54,7 +54,7 @@ func (v *VolumeCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile. if _, err := os.Stat(volume); os.IsNotExist(err) { logrus.Infof("Creating directory %s", volume) if err := os.MkdirAll(volume, 0755); err != nil { - return fmt.Errorf("Could not create directory for volume %s: %s", volume, err) + return fmt.Errorf("could not create directory for volume %s: %s", volume, err) } } } diff --git a/pkg/dockerfile/dockerfile.go b/pkg/dockerfile/dockerfile.go index 3312191590..8865b17d00 100644 --- a/pkg/dockerfile/dockerfile.go +++ b/pkg/dockerfile/dockerfile.go @@ -111,7 +111,7 @@ func Parse(b []byte) ([]instructions.Stage, []instructions.ArgCommand, error) { if err != nil { return nil, nil, err } - return stages, metaArgs, err + return stages, metaArgs, nil } // targetStage returns the index of the target stage kaniko is trying to build diff --git a/pkg/snapshot/layered_map.go b/pkg/snapshot/layered_map.go index ad2bee4d22..56e8da4f09 100644 --- a/pkg/snapshot/layered_map.go +++ b/pkg/snapshot/layered_map.go @@ -103,13 +103,13 @@ func (l *LayeredMap) Add(s string) error { // Use hash function and add to layers newV, err := l.hasher(s) if err != nil { - return fmt.Errorf("Error creating hash for %s: %v", s, err) + return fmt.Errorf("error creating hash for %s: %v", s, err) } l.layers[len(l.layers)-1][s] = newV return nil } -// CheckFileChange checkes whether a given file changed +// CheckFileChange checks whether a given file changed // from the current layered map by its hashing function. // Returns true if the file is changed. func (l *LayeredMap) CheckFileChange(s string) (bool, error) { diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go index 90638168d1..02f5ffcc7d 100644 --- a/pkg/snapshot/snapshot.go +++ b/pkg/snapshot/snapshot.go @@ -80,7 +80,7 @@ func (s *Snapshotter) TakeSnapshot(files []string) (string, error) { // Add files to the layered map for _, file := range filesToAdd { if err := s.l.Add(file); err != nil { - return "", fmt.Errorf("Unable to add file %s to layered map: %s", file, err) + return "", fmt.Errorf("unable to add file %s to layered map: %s", file, err) } } @@ -183,13 +183,13 @@ func (s *Snapshotter) scanFullFilesystem() ([]string, []string, error) { } } - // Also add parent directories to keep the permission of them correctly. + // Also add parent directories to keep their permissions correctly. filesToAdd = filesWithParentDirs(filesToAdd) // Add files to the layered map for _, file := range filesToAdd { if err := s.l.Add(file); err != nil { - return nil, nil, fmt.Errorf("Unable to add file %s to layered map: %s", file, err) + return nil, nil, fmt.Errorf("unable to add file %s to layered map: %s", file, err) } } diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index a38972ced7..6d9471ebd5 100644 --- a/pkg/util/command_util.go +++ b/pkg/util/command_util.go @@ -55,7 +55,7 @@ func ResolveEnvironmentReplacementList(values, envs []string, isFilepath bool) ( // ResolveEnvironmentReplacement resolves replacing env variables in some text from envs // It takes in a string representation of the command, the value to be resolved, and a list of envs (config.Env) -// Ex: fp = $foo/newdir, envs = [foo=/foodir], then this should return /foodir/newdir +// Ex: value = $foo/newdir, envs = [foo=/foodir], then this should return /foodir/newdir // The dockerfile/shell package handles processing env values // It handles escape characters and supports expansion from the config.Env array // Shlex handles some of the following use cases (these and more are tested in integration tests) @@ -325,13 +325,12 @@ func GetUserFromUsername(userStr string, groupStr string) (string, string, error // Lookup by username userObj, err := user.Lookup(userStr) if err != nil { - if _, ok := err.(user.UnknownUserError); ok { - // Lookup by id - userObj, err = user.LookupId(userStr) - if err != nil { - return "", "", err - } - } else { + if _, ok := err.(user.UnknownUserError); !ok { + return "", "", err + } + // Lookup by id + userObj, err = user.LookupId(userStr) + if err != nil { return "", "", err } } @@ -341,12 +340,11 @@ func GetUserFromUsername(userStr string, groupStr string) (string, string, error if groupStr != "" { group, err = user.LookupGroup(groupStr) if err != nil { - if _, ok := err.(user.UnknownGroupError); ok { - group, err = user.LookupGroupId(groupStr) - if err != nil { - return "", "", err - } - } else { + if _, ok := err.(user.UnknownGroupError); !ok { + return "", "", err + } + group, err = user.LookupGroupId(groupStr) + if err != nil { return "", "", err } } diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 5f72c7784e..d22dcb4aa4 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -188,7 +188,7 @@ func extractFile(dest string, hdr *tar.Header, tr io.Reader) error { switch hdr.Typeflag { case tar.TypeReg: logrus.Debugf("creating file %s", path) - // It's possible a file is in the tar before it's directory. + // It's possible a file is in the tar before its directory. if _, err := os.Stat(dir); os.IsNotExist(err) { logrus.Debugf("base %s for file %s does not exist. Creating.", base, path) if err := os.MkdirAll(dir, 0755); err != nil { @@ -288,12 +288,7 @@ func checkWhitelistRoot(root string) bool { if root == constants.RootDir { return false } - for _, wl := range whitelist { - if HasFilepathPrefix(root, wl.Path, wl.PrefixMatchOnly) { - return true - } - } - return false + return CheckWhitelist(root) } // Get whitelist from roots of mounted files