Repository: kubernetes/kubernetes (language: go)

Issue:
kubelet on Windows fails if a pod has SecurityContext with RunAsUser.
### What happened?

Install a cluster with v1.29.4 with linux node as master and worker node as windows
kubeadm upgrade apply v1.30.0 fails with

`[ERROR CreateJob]: Job \"upgrade-health-check-4rxpv\" in the namespace \"kube-system\" did not complete in 15s: no condition of type Complete [preflight] If you know what you are doing, you can make a check non-fatal with `--ignore-preflight-errors`

Describe Pod events:

`mountvolume.setup failed for volume "kube-api-access-rncjh" : chown c:\var\lib\kubelet\pods\c5a3db55-4ee9-4b99-8fd6-17da5799f34a\volumes\kubernetes.io~projected\kube-api-access-rncjh\..2024_05_08_20_28_34.3908400707\token: not supported by windows`

It seems that previously the pod was Pending as well, but this was ignored, because the jobs was successfully deleted in defer and return value was overridden with nil.

```
defer func() {
    lastError = deleteHealthCheckJob(client, ns, jobName)
}()
```

https://github.com/kubernetes/kubernetes/blob/v1.29.1/cmd/kubeadm/app/phases/upgrade/health.go#L151



### What did you expect to happen?

Upgrade health check job expected to be scheduled only on Linux nodes using node selectors, rather than on Windows nodes.

### How can we reproduce it (as minimally and precisely as possible)?

See **What happened?**

### Anything else we need to know?

_No response_

### Kubernetes version

<details>

kubeadm version (use kubeadm version): v1.30.0

Environment:

Kubernetes version (use kubectl version): v1.30.0
Cloud provider or hardware configuration:
OS (e.g. from /etc/os-release): Ubuntu 22.04.2 LTS, Windows Server 2022 Datacenter
Kernel (e.g. uname -a): 5.19.0-1025-aws
Container networking plugin (CNI): calico

</details>


### Cloud provider

<details>

</details>


### OS version

<details>

```console
# On Linux:
$ cat /etc/os-release
# paste output here
$ uname -a
# paste output here

# On Windows:
C:\> wmic os get Caption, Version, BuildNumber, OSArchitecture
# paste output here
```

</details>


### Install tools

<details>

</details>


### Container runtime (CRI) and version (if applicable)

<details>

</details>


### Related plugins (CNI, CSI, ...) and versions (if applicable)

<details>

</details>



Discussion / hints:
There are no sig labels on this issue. Please add an appropriate label by using one of the following commands:
- `/sig <group-name>`
- `/wg <group-name>`
- `/committee <group-name>`

Please see the [group list](https://git.k8s.io/community/sig-list.md) for a listing of the SIGs, working groups, and committees available.


<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>
> kubeadm version (use kubeadm version): v1.30.0

fixed in 1.30.1
https://github.com/kubernetes/kubeadm/issues/3050

/close
@neolit123: Closing this issue.

<details>

In response to [this](https://github.com/kubernetes/kubernetes/issues/125012):

>> kubeadm version (use kubeadm version): v1.30.0
>
>fixed in 1.30.1
>https://github.com/kubernetes/kubeadm/issues/3050
>
>/close


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>
/transfer kubeadm

@neolit123 we are seeing this issue for 1.30.1 version too with upgrade health check pods getting scheduled on windows worker nodes.
looks like I was mistaken and this issue is different.

but the health check uses the pause image, which is multi os, supposedly. the error is strange.

> Describe Pod events:

can you show the full log output.

@carlory  we might have to apply a node selector on the job spec and backport to 1.31 another bug fix.
@neolit123 I'm not sure if we should add a node selector on job spec. the job uses the pause image to create a container. It doesn't have specific requirements for the node.

After I look at the projected volume plugin, I'm confused why the above event occurs.

- On windows nodes, the `SetVolumeOwnership` function is empty. It doesn't do anything.
- On linux nodes, the `SetVolumeOwnership` function will set the owner of the volume.

FYI: 

- https://github.com/carlory/kubernetes/blob/7c48c2bd72b9bf5c44d21d7338cc7bea77d0ad2a/pkg/volume/projected/projected.go#L238
- https://github.com/carlory/kubernetes/blob/7c48c2bd72b9bf5c44d21d7338cc7bea77d0ad2a/pkg/volume/volume_unsupported.go#L27

If a log file from the kubelet is provided, it will be helpful to understand the issue.
thanks for checking @carlory 
I also think there should not be an error. maybe we are hitting a core kubelet bug.

@mani3196 is the kubelet on the windows node also 1.30?
@neolit123 please ignore the above comment, it's incorrect.

I may find the root cause. It's a bug in kubelet when kubelet is running on Windows and the pod configures the SecurityContext with a RunAsUser.

- https://github.com/carlory/kubernetes/blob/7c48c2bd72b9bf5c44d21d7338cc7bea77d0ad2a/pkg/securitycontext/util.go#L123
- https://github.com/carlory/kubernetes/blob/7c48c2bd72b9bf5c44d21d7338cc7bea77d0ad2a/pkg/volume/util/operationexecutor/operation_generator.go#L690
- https://github.com/carlory/kubernetes/blob/7c48c2bd72b9bf5c44d21d7338cc7bea77d0ad2a/pkg/volume/projected/projected.go#L356
- https://github.com/carlory/kubernetes/blob/7c48c2bd72b9bf5c44d21d7338cc7bea77d0ad2a/pkg/volume/util/atomic_writer.go#L447
/sig storage
/sig windows
/assign 

I'm trying to fix the kubelet bug.
so this is not a kubeadm bug and we can move it back to the k/k repo?
/transfer kubernetes

/retitle kubelet on Windows fails if a pod has SecurityContext with RunAsUser.

Because of historical reasons, the `runAsUser` and `runAsGroup` in `securityContext` is only designed for the Linux, not Windows. See [PodSpec](https://github.com/kubernetes/kubernetes/blob/62aae86899a7b780eb1e8805fbcaf0fb6a5baa14/pkg/apis/core/types.go#L3394) for more details.

[KEP-2802](https://github.com/kubernetes/enhancements/tree/master/keps/sig-windows/2802-identify-windows-pods-apiserver-admission) adds a new field `podOS` to the PodSpec, which can be used to identify the OS of the pod.

```go
	// Specifies the OS of the containers in the pod.
	// Some pod and container fields are restricted if this is set.
	//
	// If the OS field is set to linux, the following fields must be unset:
	// - securityContext.windowsOptions
	//
	// If the OS field is set to windows, following fields must be unset:
	// - spec.hostPID
	// - spec.hostIPC
	// - spec.hostUsers
	// - spec.securityContext.appArmorProfile
	// - spec.securityContext.seLinuxOptions
	// - spec.securityContext.seccompProfile
	// - spec.securityContext.fsGroup
	// - spec.securityContext.fsGroupChangePolicy
	// - spec.securityContext.sysctls
	// - spec.shareProcessNamespace
	// - spec.securityContext.runAsUser
	// - spec.securityContext.runAsGroup
	// - spec.securityContext.supplementalGroups
	// - spec.containers[*].securityContext.appArmorProfile
	// - spec.containers[*].securityContext.seLinuxOptions
	// - spec.containers[*].securityContext.seccompProfile
	// - spec.containers[*].securityContext.capabilities
	// - spec.containers[*].securityContext.readOnlyRootFilesystem
	// - spec.containers[*].securityContext.privileged
	// - spec.containers[*].securityContext.allowPrivilegeEscalation
	// - spec.containers[*].securityContext.procMount
	// - spec.containers[*].securityContext.runAsUser
	// - spec.containers[*].securityContext.runAsGroup
	// +optional
	OS *PodOS
```

It's an optional field and no default value. If unset, the above fields are not restricted by the api-server. In other words, the following job is allowed to be created.

```go
job := &batchv1.Job{
	ObjectMeta: metav1.ObjectMeta{
		Name:      jobName,
		Namespace: ns,
	},
	Spec: batchv1.JobSpec{
		BackoffLimit: ptr.To[int32](0),
		Template: v1.PodTemplateSpec{
			Spec: v1.PodSpec{
				RestartPolicy: v1.RestartPolicyNever,
				SecurityContext: &v1.PodSecurityContext{
					RunAsUser:    ptr.To[int64](999),        // windows does not support
					RunAsGroup:   ptr.To[int64](999),        // windows does not support
					RunAsNonRoot: ptr.To(true),
				},
				Tolerations: []v1.Toleration{
					{
						Key:    constants.LabelNodeRoleControlPlane,
						Effect: v1.TaintEffectNoSchedule,
					},
				},
				Containers: []v1.Container{
					{
						Name:  jobName,
						Image: images.GetPauseImage(cfg),
						Args:  []string{"-v"},
					},
				},
			},
		},
	},
}
```

But it will fails to start on Windows nodes. And the root cause is that the `os.Chown` is called by the [AtomicWriter](https://github.com/carlory/kubernetes/blob/7c48c2bd72b9bf5c44d21d7338cc7bea77d0ad2a/pkg/volume/util/atomic_writer.go#L447).

To fix this issue, the AtomicWriter can check the current OS of the node and skip the `os.Chown` if the OS is Windows. I want to confirm with sig-storage and sig-windows if this is the correct way to fix this issue.

I'm not sure if the failing job is expected or not. If it's expected, kubeadm has to change the job manifest to make it work on only Linux nodes. 

Why I ask this question is that I saw [volume.SetVolumeOwnership](https://github.com/carlory/kubernetes/blob/7c48c2bd72b9bf5c44d21d7338cc7bea77d0ad2a/pkg/volume/projected/projected.go#L238), which is used to set the ownership of the volume, if the os is Windows, it will become a no-op.


/cc @kubernetes/sig-windows @jsafrane @xing-yang @neolit123 

/cc @kubernetes/sig-windows-bugs 
/triage accepted
@mani3196 
slack discussion is happening here:
https://kubernetes.slack.com/archives/C0SJ4AFB7/p1716283930327109
but we have agreed that this is a kubelet problem.
on the kubeadm side you can use the --ignore-preflight-errors=CreateJob option to skip the check.

unclear when or if a kubelet fix will be added and to what versions it will be available.
>To fix this issue, the AtomicWriter can check the current OS of the node and skip the os.Chown if the OS is Windows. I want to confirm with sig-storage and sig-windows if this is the correct way to fix this issue.

@carlory please see:
- https://github.com/openshift/kubernetes/pull/804

@rphillips do you want to upstream your carry?

Fix patch:
diff --git a/pkg/volume/util/atomic_writer.go b/pkg/volume/util/atomic_writer.go
index 3ac4dcffe9a90..a6837fa65e9ba 100644
--- a/pkg/volume/util/atomic_writer.go
+++ b/pkg/volume/util/atomic_writer.go
@@ -444,7 +444,8 @@ func (w *AtomicWriter) writePayloadToDir(payload map[string]FileProjection, dir
 		if fileProjection.FsUser == nil {
 			continue
 		}
-		if err := os.Chown(fullPath, int(*fileProjection.FsUser), -1); err != nil {
+
+		if err := w.chown(fullPath, int(*fileProjection.FsUser), -1); err != nil {
 			klog.Errorf("%s: unable to change file %s with owner %v: %v", w.logContext, fullPath, int(*fileProjection.FsUser), err)
 			return err
 		}
diff --git a/pkg/volume/util/atomic_writer_linux.go b/pkg/volume/util/atomic_writer_linux.go
new file mode 100644
index 0000000000000..c12a0f4cd317e
--- /dev/null
+++ b/pkg/volume/util/atomic_writer_linux.go
@@ -0,0 +1,27 @@
+//go:build linux
+// +build linux
+
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+import "os"
+
+// chown changes the numeric uid and gid of the named file.
+func (w *AtomicWriter) chown(name string, uid, gid int) error {
+	return os.Chown(name, uid, gid)
+}
diff --git a/pkg/volume/util/atomic_writer_unsupported.go b/pkg/volume/util/atomic_writer_unsupported.go
new file mode 100644
index 0000000000000..cdfb83e639725
--- /dev/null
+++ b/pkg/volume/util/atomic_writer_unsupported.go
@@ -0,0 +1,33 @@
+//go:build !linux
+// +build !linux
+
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+import (
+	"runtime"
+
+	"k8s.io/klog/v2"
+)
+
+// chown changes the numeric uid and gid of the named file.
+// This is a no-op on unsupported platforms.
+func (w *AtomicWriter) chown(name string, uid, _ /* gid */ int) error {
+	klog.Warningf("%s: skipping change of Linux owner %v for file %s; unsupported on %s", w.logContext, uid, name, runtime.GOOS)
+	return nil
+}


Test patch:
diff --git a/test/e2e/windows/security_context.go b/test/e2e/windows/security_context.go
index 09ff4ae2a50dc..ebb62dbbaa077 100644
--- a/test/e2e/windows/security_context.go
+++ b/test/e2e/windows/security_context.go
@@ -28,15 +28,19 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/util/uuid"
+	"k8s.io/apimachinery/pkg/util/wait"
 	clientset "k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/kubernetes/pkg/kubelet/events"
 	"k8s.io/kubernetes/test/e2e/feature"
 	"k8s.io/kubernetes/test/e2e/framework"
 	e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
 	e2eoutput "k8s.io/kubernetes/test/e2e/framework/pod/output"
 	testutils "k8s.io/kubernetes/test/utils"
+	"k8s.io/kubernetes/test/utils/format"
 	imageutils "k8s.io/kubernetes/test/utils/image"
 	admissionapi "k8s.io/pod-security-admission/api"
+	"k8s.io/utils/ptr"
 )
 
 const runAsUserNameContainerName = "run-as-username-container"
@@ -193,6 +197,67 @@ var _ = sigDescribe(feature.Windows, "SecurityContext", skipUnlessWindows(func()
 	})
 }))
 
+var _ = sigDescribe(feature.Windows, "SecurityContext", skipUnlessWindows(func() {
+	f := framework.NewDefaultFramework("windows-with-unsupported-fields")
+	f.NamespacePodSecurityLevel = admissionapi.LevelPrivileged
+
+	ginkgo.It("should be able to create pod and run containers", func(ctx context.Context) {
+		ginkgo.By("Creating 1 pods: run with unsupported fields")
+
+		pod := &v1.Pod{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "run-ignore-unsupported-fields",
+				Namespace: f.Namespace.Name,
+			},
+			Spec: v1.PodSpec{
+				NodeSelector: map[string]string{"kubernetes.io/os": "windows"},
+				Containers: []v1.Container{
+					{
+						Name:  "test-container",
+						Image: imageutils.GetE2EImage(imageutils.Pause),
+					},
+				},
+				SecurityContext: &v1.PodSecurityContext{
+					RunAsUser:    ptr.To[int64](999), // windows does not support
+					RunAsGroup:   ptr.To[int64](999), // windows does not support
+					RunAsNonRoot: ptr.To(true),
+				},
+				RestartPolicy: v1.RestartPolicyNever,
+			},
+		}
+
+		pod, err := f.ClientSet.CoreV1().Pods(f.Namespace.Name).Create(ctx, pod, metav1.CreateOptions{})
+		framework.ExpectNoError(err, "Error creating pod")
+
+		podErr := e2epod.WaitForPodRunningInNamespace(ctx, f.ClientSet, pod)
+
+		// Get the logs and events before calling ExpectNoError, so we can debug any errors.
+		var logs string
+		var events *v1.EventList
+		if err := wait.PollUntilContextTimeout(ctx, 30*time.Second, 2*time.Minute, true, func(ctx context.Context) (done bool, err error) {
+			framework.Logf("polling logs")
+			logs, err = e2epod.GetPodLogs(ctx, f.ClientSet, f.Namespace.Name, pod.Name, pod.Spec.Containers[0].Name)
+			if err != nil {
+				framework.Logf("Error pulling logs: %v", err)
+				return false, nil
+			}
+
+			events, err = f.ClientSet.CoreV1().Events(pod.Namespace).Search(scheme.Scheme, pod)
+			if err != nil {
+				return false, fmt.Errorf("error in listing events: %w", err)
+			}
+			return true, nil
+		}); err != nil {
+			framework.Failf("Unexpected error getting pod logs/events: %v", err)
+		} else {
+			framework.Logf("Pod logs: \n%v", logs)
+			framework.Logf("Pod events: \n%v", format.Object(events, 1))
+		}
+
+		framework.ExpectNoError(podErr)
+	})
+}))
+
 func runAsUserNamePod(username *string) *v1.Pod {
 	podName := "run-as-username-" + string(uuid.NewUUID())
 	return &v1.Pod{