Skip to content

Commit 47dbade

Browse files
authored
Merge pull request kubernetes#130245 from marosset/windows-unit-tests-pkg-util-filesystem-fixes
Fixing k8s.io/kubernetes/pkg/util/filesystem unit tests for Windows
2 parents 0125bc1 + 5eb37b0 commit 47dbade

File tree

1 file changed

+60
-7
lines changed

1 file changed

+60
-7
lines changed

pkg/util/filesystem/util_windows_test.go

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -119,41 +119,88 @@ func TestWindowsChmod(t *testing.T) {
119119
require.NoError(t, err, "Failed to create temporary directory.")
120120
defer os.RemoveAll(tempDir)
121121

122-
// Set the file GROUP to BUILTIN\Administrators (BA) for test determinism and
122+
// Set the file OWNER to current user and GROUP to BUILTIN\Administrators (BA) for test determinism
123+
currentUserSID, err := getCurrentUserSID()
124+
require.NoError(t, err, "Failed to get current user SID")
125+
126+
err = setOwnerInfo(tempDir, currentUserSID)
127+
require.NoError(t, err, "Failed to set current owner SID")
128+
123129
err = setGroupInfo(tempDir, "S-1-5-32-544")
124130
require.NoError(t, err, "Failed to set group for directory.")
125131

126132
err = Chmod(tempDir, testCase.fileMode)
127133
require.NoError(t, err, "Failed to set permissions for directory.")
128134

129-
owner, descriptor, err := getPermissionsInfo(tempDir)
135+
owner, _, descriptor, err := getPermissionsInfo(tempDir)
130136
require.NoError(t, err, "Failed to get permissions for directory.")
131137

132138
expectedDescriptor := strings.ReplaceAll(testCase.expectedDescriptor, "OWNER", owner)
139+
// In cases where there is a single account in the Administrators group (which the case in CI)
140+
// the SDDL format will simply say LA (for Local Administrator) instead of the actual SID,
141+
// but we want to replace that with the actual SID for determinism
142+
descriptor = strings.ReplaceAll(descriptor, "LA", owner)
133143

134144
assert.Equal(t, expectedDescriptor, descriptor, "Unexpected DACL for directory. when setting permissions to %o", testCase.fileMode)
135145
}
136146
}
137147

138-
// Gets the owner and entire security descriptor of a file or directory in the SDDL format
148+
// Gets the SID for the current user
149+
func getCurrentUserSID() (string, error) {
150+
token := windows.GetCurrentProcessToken()
151+
user, err := token.GetTokenUser()
152+
if err != nil {
153+
return "", fmt.Errorf("Error getting user SID: %v", err)
154+
}
155+
156+
return user.User.Sid.String(), nil
157+
}
158+
159+
// Gets the owner, group, and entire security descriptor of a file or directory in the SDDL format
139160
// https://learn.microsoft.com/en-us/windows/win32/secauthz/security-descriptor-definition-language
140-
func getPermissionsInfo(path string) (string, string, error) {
161+
func getPermissionsInfo(path string) (string, string, string, error) {
141162
sd, err := windows.GetNamedSecurityInfo(
142163
path,
143164
windows.SE_FILE_OBJECT,
144165
windows.DACL_SECURITY_INFORMATION|windows.OWNER_SECURITY_INFORMATION|windows.GROUP_SECURITY_INFORMATION)
145166
if err != nil {
146-
return "", "", fmt.Errorf("Error getting security descriptor for file %s: %v", path, err)
167+
return "", "", "", fmt.Errorf("Error getting security descriptor for file %s: %v", path, err)
147168
}
148169

149170
owner, _, err := sd.Owner()
150171
if err != nil {
151-
return "", "", fmt.Errorf("Error getting owner SID for file %s: %v", path, err)
172+
return "", "", "", fmt.Errorf("Error getting owner SID for file %s: %v", path, err)
173+
}
174+
group, _, err := sd.Group()
175+
if err != nil {
176+
return "", "", "", fmt.Errorf("Error getting group SID for file %s: %v", path, err)
152177
}
153178

154179
sdString := sd.String()
155180

156-
return owner.String(), sdString, nil
181+
return owner.String(), group.String(), sdString, nil
182+
}
183+
184+
// Sets the OWNER of a file or a directory to the specific SID
185+
func setOwnerInfo(path, owner string) error {
186+
ownerSID, err := windows.StringToSid(owner)
187+
if err != nil {
188+
return fmt.Errorf("Error converting owner SID %s to SID: %v", owner, err)
189+
}
190+
191+
err = windows.SetNamedSecurityInfo(
192+
path, windows.SE_FILE_OBJECT,
193+
windows.OWNER_SECURITY_INFORMATION,
194+
ownerSID, // ownerSID
195+
nil, // Group SID
196+
nil, // DACL
197+
nil, // SACL
198+
)
199+
200+
if err != nil {
201+
return fmt.Errorf("Error setting owner SID for file %s: %v", path, err)
202+
}
203+
return nil
157204
}
158205

159206
// Sets the GROUP of a file or a directory to the specified group
@@ -184,6 +231,12 @@ func setGroupInfo(path, group string) error {
184231
// TestDeleteFilePermissions tests that when a folder's permissions are set to 0660, child items
185232
// cannot be deleted in the folder but when a folder's permissions are set to 0770, child items can be deleted.
186233
func TestDeleteFilePermissions(t *testing.T) {
234+
235+
// On Windows, connections under an SSH session acquire SeBackupPrivilege and SeRestorePrivilege
236+
// which allows you to delete a file bypassing ACLs (which invalidates this test)
237+
if sshConn := os.Getenv("SSH_CONNECTION"); sshConn != "" {
238+
t.Skip("Skipping test when running over SSH connection.")
239+
}
187240
tempDir, err := os.MkdirTemp("", "test-dir")
188241
require.NoError(t, err, "Failed to create temporary directory.")
189242

0 commit comments

Comments
 (0)