Merge pull request #62375 from feiskyer/visible-files

Automatic merge from submit-queue (batch tested with PRs 62650, 62303, 62545, 62375). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Fix user visible files creation for windows

**What this PR does / why we need it**:

Fix user visible files creation for windows. Without this, [createUserVisibleFiles](https://github.com/kubernetes/kubernetes/blob/master/pkg/volume/util/atomic_writer.go#L415:24) will get linkname with subpath included, and then symlink will fail. This is because "/" is used in pod spec (e.g. `"new/path/data-1"`) while "\" is used on Windows to get linkname. 


**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #62338

**Special notes for your reviewer**:

Should also be cherry-picked to old releases.

**Release note**:

```release-note
Fix user visible files creation for windows
```

pkg/volume/util/atomic_writer.go

@@ -230,7 +230,7 @@ func validatePayload(payload map[string]FileProjection) (map[string]FileProjecti
 			return nil, err
 		}
 
-		cleanPayload[path.Clean(k)] = content
+		cleanPayload[filepath.Clean(k)] = content
 	}
 
 	return cleanPayload, nil

pkg/volume/util/atomic_writer_test.go

@@ -811,3 +811,176 @@ func checkVolumeContents(targetDir, tcName string, payload map[string]FileProjec
 		t.Errorf("%v: payload and observed payload do not match.", tcName)
 	}
 }
+
+func TestValidatePayload(t *testing.T) {
+	maxPath := strings.Repeat("a", maxPathLength+1)
+
+	cases := []struct {
+		name     string
+		payload  map[string]FileProjection
+		expected sets.String
+		valid    bool
+	}{
+		{
+			name: "valid payload",
+			payload: map[string]FileProjection{
+				"foo": {},
+				"bar": {},
+			},
+			valid:    true,
+			expected: sets.NewString("foo", "bar"),
+		},
+		{
+			name: "payload with path length > 4096 is invalid",
+			payload: map[string]FileProjection{
+				maxPath: {},
+			},
+			valid: false,
+		},
+		{
+			name: "payload with absolute path is invalid",
+			payload: map[string]FileProjection{
+				"/dev/null": {},
+			},
+			valid: false,
+		},
+		{
+			name: "payload with reserved path is invalid",
+			payload: map[string]FileProjection{
+				"..sneaky.txt": {},
+			},
+			valid: false,
+		},
+		{
+			name: "payload with doubledot path is invalid",
+			payload: map[string]FileProjection{
+				"foo/../etc/password": {},
+			},
+			valid: false,
+		},
+		{
+			name: "payload with empty path is invalid",
+			payload: map[string]FileProjection{
+				"": {},
+			},
+			valid: false,
+		},
+		{
+			name: "payload with unclean path should be cleaned",
+			payload: map[string]FileProjection{
+				"foo////bar": {},
+			},
+			valid:    true,
+			expected: sets.NewString("foo/bar"),
+		},
+	}
+	getPayloadPaths := func(payload map[string]FileProjection) sets.String {
+		paths := sets.NewString()
+		for path := range payload {
+			paths.Insert(path)
+		}
+		return paths
+	}
+
+	for _, tc := range cases {
+		real, err := validatePayload(tc.payload)
+		if !tc.valid && err == nil {
+			t.Errorf("%v: unexpected success", tc.name)
+		}
+
+		if tc.valid {
+			if err != nil {
+				t.Errorf("%v: unexpected failure: %v", tc.name, err)
+				continue
+			}
+
+			realPaths := getPayloadPaths(real)
+			if !realPaths.Equal(tc.expected) {
+				t.Errorf("%v: unexpected payload paths: %v is not equal to %v", tc.name, realPaths, tc.expected)
+			}
+		}
+
+	}
+}
+
+func TestCreateUserVisibleFiles(t *testing.T) {
+	cases := []struct {
+		name     string
+		payload  map[string]FileProjection
+		expected map[string]string
+	}{
+		{
+			name: "simple path",
+			payload: map[string]FileProjection{
+				"foo": {},
+				"bar": {},
+			},
+			expected: map[string]string{
+				"foo": "..data/foo",
+				"bar": "..data/bar",
+			},
+		},
+		{
+			name: "simple nested path",
+			payload: map[string]FileProjection{
+				"foo/bar":     {},
+				"foo/bar/txt": {},
+				"bar/txt":     {},
+			},
+			expected: map[string]string{
+				"foo": "..data/foo",
+				"bar": "..data/bar",
+			},
+		},
+		{
+			name: "unclean nested path",
+			payload: map[string]FileProjection{
+				"./bar":     {},
+				"foo///bar": {},
+			},
+			expected: map[string]string{
+				"bar": "..data/bar",
+				"foo": "..data/foo",
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		targetDir, err := utiltesting.MkTmpdir("atomic-write")
+		if err != nil {
+			t.Errorf("%v: unexpected error creating tmp dir: %v", tc.name, err)
+			continue
+		}
+		defer os.RemoveAll(targetDir)
+
+		dataDirPath := path.Join(targetDir, dataDirName)
+		err = os.MkdirAll(dataDirPath, 0755)
+		if err != nil {
+			t.Fatalf("%v: unexpected error creating data path: %v", tc.name, err)
+		}
+
+		writer := &AtomicWriter{targetDir: targetDir, logContext: "-test-"}
+		payload, err := validatePayload(tc.payload)
+		if err != nil {
+			t.Fatalf("%v: unexpected error validating payload: %v", tc.name, err)
+		}
+		err = writer.createUserVisibleFiles(payload)
+		if err != nil {
+			t.Fatalf("%v: unexpected error creating visible files: %v", tc.name, err)
+		}
+
+		for subpath, expectedDest := range tc.expected {
+			visiblePath := path.Join(targetDir, subpath)
+			destination, err := os.Readlink(visiblePath)
+			if err != nil && os.IsNotExist(err) {
+				t.Fatalf("%v: visible symlink does not exist: %v", tc.name, visiblePath)
+			} else if err != nil {
+				t.Fatalf("%v: unable to read symlink %v: %v", tc.name, dataDirPath, err)
+			}
+
+			if expectedDest != destination {
+				t.Fatalf("%v: symlink destination %q not same with expected data dir %q", tc.name, destination, expectedDest)
+			}
+		}
+	}
+}