Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Resolve relative paths to absolute paths in command line arguments #736

Merged
merged 1 commit into from
Oct 4, 2019
Merged

Resolve relative paths to absolute paths in command line arguments #736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions cmd/executor/cmd/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ var RootCmd = &cobra.Command{
if err := executor.CheckPushPermissions(opts); err != nil {
exit(errors.Wrap(err, "error checking push permissions -- make sure you entered the correct tag name, and that you are authenticated correctly, and try again"))
}
if err := resolveRelativePaths(); err != nil {
exit(errors.Wrap(err, "error resolving relative paths to absolute paths"))
}
if err := os.Chdir("/"); err != nil {
exit(errors.Wrap(err, "error changing to root dir"))
}
Expand Down Expand Up @@ -227,6 +230,37 @@ func resolveSourceContext() error {
return nil
}

func resolveRelativePaths() error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@huguesalary Thank you for your contributions. This looks really good. However can we please some unit test for this?

Can you refactor resolveRelativePaths() to accept opts? That ways we can easily write unit tests

Suggested change
func resolveRelativePaths() error {
func resolveRelativePaths(opts config.KanikoOptions{}) error {

Now, you can creates tests where

func TestResolveRelativePaths(t *testing.T) {
	tests := []struct {
		description  string
		opts config.KanikoOptions
               expected config.KanikoOptions
               shdErr  bool 
	}{
            {
                description: "dockerfile path  is absolute",
                opts: config.KanikoOptions{
                           DockerfilePath: "/abs/dockerfile",
                        },
               expected: config.KanikoOptions{
                           DockerfilePath: "/abs/dockerfile",
                        },
            }

	}
	for _, tt := range tests {
           actual := resolveRelativePaths(tt.opts)
           t.CheckDeepEqualAndError(tt.shdErr, actual, tt.expected, tt.opts)
	}
}

optsPaths := []*string{
&opts.DockerfilePath,
&opts.SrcContext,
&opts.CacheDir,
&opts.TarPath,
&opts.DigestFile,
}

for _, p := range optsPaths {
// Skip empty path
if *p == "" {
continue
}
// Skip path that is already absolute
if filepath.IsAbs(*p) {
logrus.Debugf("Path %s is absolute, skipping", *p)
continue
}

// Resolve relative path to absolute path
var err error
relp := *p // save original relative path
if *p, err = filepath.Abs(*p); err != nil {
return errors.Wrapf(err, "Couldn't resolve relative path %s to an absolute path", *p)
}
logrus.Debugf("Resolved relative path %s to %s", relp, *p)
}
return nil
}

func exit(err error) {
fmt.Println(err)
os.Exit(1)
Expand Down
30 changes: 30 additions & 0 deletions integration/images.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,3 +286,33 @@ func (d *DockerFileBuilder) buildCachedImages(imageRepo, cacheRepo, dockerfilesP
}
return nil
}

// buildRelativePathsImage builds the images for testing passing relatives paths to Kaniko
func (d *DockerFileBuilder) buildRelativePathsImage(imageRepo, dockerfile string) error {
_, ex, _, _ := runtime.Caller(0)
cwd := filepath.Dir(ex)

buildContextPath := "./relative-subdirectory"
kanikoImage := GetKanikoImage(imageRepo, dockerfile)

kanikoCmd := exec.Command("docker",
append([]string{"run",
"-v", os.Getenv("HOME") + "/.config/gcloud:/root/.config/gcloud",
"-v", cwd + ":/workspace",
ExecutorImage,
"-f", dockerfile,
"-d", kanikoImage,
"--digest-file", "./digest",
"-c", buildContextPath,
})...,
)

timer := timing.Start(dockerfile + "_kaniko_relative_paths")
_, err := RunCommandWithoutTest(kanikoCmd)
timing.DefaultRun.Stop(timer)
if err != nil {
return fmt.Errorf("Failed to build relative path image %s with kaniko command \"%s\": %s", kanikoImage, kanikoCmd.Args, err)
}

return nil
}
24 changes: 24 additions & 0 deletions integration/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,30 @@ func TestCache(t *testing.T) {
}
}

func TestRelativePaths(t *testing.T) {

dockerfile := "Dockerfile_test_copy"

t.Run("test_relative_"+dockerfile, func(t *testing.T) {
t.Parallel()
imageBuilder.buildRelativePathsImage(config.imageRepo, dockerfile)

dockerImage := GetDockerImage(config.imageRepo, dockerfile)
kanikoImage := GetKanikoImage(config.imageRepo, dockerfile)

// container-diff
daemonDockerImage := daemonPrefix + dockerImage
containerdiffCmd := exec.Command("container-diff", "diff", "--no-cache",
daemonDockerImage, kanikoImage,
"-q", "--type=file", "--type=metadata", "--json")
diff := RunCommand(containerdiffCmd, t)
t.Logf("diff = %s", string(diff))

expected := fmt.Sprintf(emptyContainerDiff, dockerImage, kanikoImage, dockerImage, kanikoImage)
checkContainerDiffOutput(t, diff, expected)
})
}

type fileDiff struct {
Name string
Size int
Expand Down