Skip to content

Commit

Permalink
fix(test): windows tests fixes. Fixes #11994 (#12071)
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Weibel <michael@helio.exchange>
  • Loading branch information
mweibel authored Oct 7, 2024
1 parent 734b5b6 commit 15f2170
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 26 deletions.
4 changes: 1 addition & 3 deletions cmd/argo/commands/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,7 @@ spec:
defer func() { os.Stdin = oldStdin }() // Restore original Stdin
os.Stdin, err = os.Open(clusterWftmplPath)
require.NoError(t, err)
defer func() {
_ = os.Stdin.Close() // close previously opened path to avoid errors trying to remove the file.
}()
defer func() { _ = os.Stdin.Close() }() // close previously opened path to avoid errors trying to remove the file.

err = runLint(context.Background(), []string{workflowPath, wftmplPath, "-"}, true, nil, "pretty", true)

Expand Down
9 changes: 7 additions & 2 deletions cmd/argo/lint/formatter_pretty_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
//go:build !windows

package lint

import (
"fmt"
"runtime"
"testing"

"github.com/TwiN/go-color"
Expand Down Expand Up @@ -41,6 +40,9 @@ func TestPrettyFormat(t *testing.T) {
Linted: true,
})
expected := "\x1b[4mtest1\x1b[0m:\n \x1b[31m✖\x1b[0m some error\n \x1b[31m✖\x1b[0m some error2\n\n"
if runtime.GOOS == "windows" {
expected = "test1:\n ✖ some error\n ✖ some error2\n\n"
}
assert.Equal(t, expected, msg)
})

Expand All @@ -53,6 +55,9 @@ func TestPrettyFormat(t *testing.T) {
Linted: true,
})
expected := "\x1b[4mtest2\x1b[0m:\n \x1b[31m✖\x1b[0m some error\n\n"
if runtime.GOOS == "windows" {
expected = "test2:\n ✖ some error\n\n"
}
assert.Equal(t, expected, msg)
})

Expand Down
6 changes: 6 additions & 0 deletions pkg/apis/workflow/v1alpha1/workflow_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"path/filepath"
"reflect"
"regexp"
"runtime"
"sort"
"strings"
"time"
Expand Down Expand Up @@ -1003,6 +1004,11 @@ func (a *Artifact) CleanPath() error {
// Any resolved path should always be within the /tmp/safe/ directory.
safeDir := ""
slashDotDotRe := regexp.MustCompile(fmt.Sprintf(`%c..$`, os.PathSeparator))
if runtime.GOOS == "windows" {
// windows PathSeparator is \ and needs escaping
slashDotDotRe = regexp.MustCompile(fmt.Sprintf(`\%c..$`, os.PathSeparator))
}

slashDotDotSlash := fmt.Sprintf(`%c..%c`, os.PathSeparator, os.PathSeparator)
if strings.Contains(path, slashDotDotSlash) {
safeDir = path[:strings.Index(path, slashDotDotSlash)]
Expand Down
29 changes: 18 additions & 11 deletions pkg/apis/workflow/v1alpha1/workflow_types_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
//go:build !windows

package v1alpha1

import (
"fmt"
"os"
"path/filepath"
"sort"
"testing"
"time"
Expand Down Expand Up @@ -255,56 +255,63 @@ func TestArtifact_ValidatePath(t *testing.T) {
a3 := Artifact{Name: "a3", Path: "../.."}
err = a3.CleanPath()
require.NoError(t, err)
assert.Equal(t, "../..", a3.Path)
assert.Equal(t, filepath.Join("..", ".."), a3.Path)

a4 := Artifact{Name: "a4", Path: "../etc/passwd"}
err = a4.CleanPath()
require.NoError(t, err)
assert.Equal(t, "../etc/passwd", a4.Path)
assert.Equal(t, filepath.Join("..", "etc", "passwd"), a4.Path)
})

t.Run("directory traversal ending within safe base dir succeeds", func(t *testing.T) {
a1 := Artifact{Name: "a1", Path: "/tmp/../tmp/abcd"}
err := a1.CleanPath()
require.NoError(t, err)
assert.Equal(t, "/tmp/abcd", a1.Path)
assert.Equal(t, ensureRootPathSeparator(filepath.Join("tmp", "abcd")), a1.Path)

a2 := Artifact{Name: "a2", Path: "/tmp/subdir/../../tmp/subdir/abcd"}
err = a2.CleanPath()
require.NoError(t, err)
assert.Equal(t, "/tmp/subdir/abcd", a2.Path)
assert.Equal(t, ensureRootPathSeparator(filepath.Join("tmp", "subdir", "abcd")), a2.Path)
})

t.Run("artifact path filenames are allowed to contain double dots", func(t *testing.T) {
a1 := Artifact{Name: "a1", Path: "/tmp/..artifact.txt"}
err := a1.CleanPath()
require.NoError(t, err)
assert.Equal(t, "/tmp/..artifact.txt", a1.Path)
assert.Equal(t, ensureRootPathSeparator(filepath.Join("tmp", "..artifact.txt")), a1.Path)

a2 := Artifact{Name: "a2", Path: "/tmp/artif..t.txt"}
err = a2.CleanPath()
require.NoError(t, err)
assert.Equal(t, "/tmp/artif..t.txt", a2.Path)
assert.Equal(t, ensureRootPathSeparator(filepath.Join("tmp", "artif..t.txt")), a2.Path)
})

t.Run("normal artifact path succeeds", func(t *testing.T) {
a1 := Artifact{Name: "a1", Path: "/tmp"}
err := a1.CleanPath()
require.NoError(t, err)
assert.Equal(t, "/tmp", a1.Path)
assert.Equal(t, ensureRootPathSeparator("tmp"), a1.Path)

a2 := Artifact{Name: "a2", Path: "/tmp/"}
err = a2.CleanPath()
require.NoError(t, err)
assert.Equal(t, "/tmp", a2.Path)
assert.Equal(t, ensureRootPathSeparator("tmp"), a2.Path)

a3 := Artifact{Name: "a3", Path: "/tmp/abcd/some-artifact.txt"}
err = a3.CleanPath()
require.NoError(t, err)
assert.Equal(t, "/tmp/abcd/some-artifact.txt", a3.Path)
assert.Equal(t, ensureRootPathSeparator(filepath.Join("tmp", "abcd", "some-artifact.txt")), a3.Path)
})
}

func ensureRootPathSeparator(p string) string {
if p[0] == os.PathSeparator {
return p
}
return string(os.PathSeparator) + p
}

func TestArtifactLocation_IsArchiveLogs(t *testing.T) {
var l *ArtifactLocation
assert.False(t, l.IsArchiveLogs())
Expand Down
6 changes: 4 additions & 2 deletions workflow/artifacts/common/load_to_stream_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
//go:build !windows

package common

import (
"fmt"
"io"
"os"
"runtime"
"strings"
"testing"

Expand Down Expand Up @@ -74,6 +73,9 @@ func filteredFiles(t *testing.T) ([]os.DirEntry, error) {
}

func TestLoadToStream(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("disabled on windows because artifacts server isn't run in windows and the test fails.")
}
tests := map[string]struct {
artifactDriver ArtifactDriver
errMsg string
Expand Down
8 changes: 0 additions & 8 deletions workflow/executor/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,10 +234,6 @@ func TestDefaultParametersEmptyString(t *testing.T) {
}

func TestIsTarball(t *testing.T) {
if runtime.GOOS == "windows" {
// TODO: fix this test in windows
t.Skip("test not working in windows - temp disable")
}
tests := []struct {
path string
isTarball bool
Expand All @@ -264,10 +260,6 @@ func TestIsTarball(t *testing.T) {
}

func TestUnzip(t *testing.T) {
if runtime.GOOS == "windows" {
// TODO: fix this test in windows
t.Skip("test not working in windows - temp disable")
}
zipPath := "testdata/file.zip"
destPath := "testdata/unzippedFile"

Expand Down

0 comments on commit 15f2170

Please sign in to comment.