fix: remove windows UNC paths from wait/init containers. Fixes #6583 (#…

…6704)

Signed-off-by: Anish Dangi <[email protected]>

workflow/controller/workflowpod.go

@@ -51,37 +51,25 @@ func (woc *wfOperationCtx) getVolumeMountDockerSock(tmpl *wfv1.Template) apiv1.V
 }
 
 func getDockerSockReadOnly(tmpl *wfv1.Template) bool {
- return !hasWindowsOSNodeSelector(tmpl.NodeSelector)
+ return !util.HasWindowsOSNodeSelector(tmpl.NodeSelector)
 }
 
 func getDockerSockPath(tmpl *wfv1.Template) string {
- if hasWindowsOSNodeSelector(tmpl.NodeSelector) {
+ if util.HasWindowsOSNodeSelector(tmpl.NodeSelector) {
  return "\\\\.\\pipe\\docker_engine"
  }
 
  return "/var/run/docker.sock"
 }
 
 func getVolumeHostPathType(tmpl *wfv1.Template) *apiv1.HostPathType {
- if hasWindowsOSNodeSelector(tmpl.NodeSelector) {
+ if util.HasWindowsOSNodeSelector(tmpl.NodeSelector) {
  return nil
  }
 
  return &hostPathSocket
 }
 
-func hasWindowsOSNodeSelector(nodeSelector map[string]string) bool {
- if nodeSelector == nil {
- return false
- }
-
- if os, keyExists := nodeSelector["kubernetes.io/os"]; keyExists && os == "windows" {
- return true
- }
-
- return false
-}
-
 func (woc *wfOperationCtx) getVolumeDockerSock(tmpl *wfv1.Template) apiv1.Volume {
  dockerSockPath := getDockerSockPath(tmpl)
 
@@ -906,6 +894,9 @@ func (woc *wfOperationCtx) addInputArtifactsVolumes(pod *apiv1.Pod, tmpl *wfv1.T
  // in case the executor needs to load artifacts to this volume
  // instead of the artifacts volume
  for _, mnt := range tmpl.GetVolumeMounts() {
+ if util.IsWindowsUNCPath(mnt.MountPath, tmpl) {
+ continue
+ }
  mnt.MountPath = filepath.Join(common.ExecutorMainFilesystemDir, mnt.MountPath)
  initCtr.VolumeMounts = append(initCtr.VolumeMounts, mnt)
  }
@@ -959,14 +950,8 @@ func addOutputArtifactsVolumes(pod *apiv1.Pod, tmpl *wfv1.Template) {
  return
  }
 
- waitCtrIndex := -1
- for i, ctr := range pod.Spec.Containers {
- switch ctr.Name {
- case common.WaitContainerName:
- waitCtrIndex = i
- }
- }
- if waitCtrIndex == -1 {
+ waitCtrIndex, err := util.FindWaitCtrIndex(pod)
+ if err != nil {
  log.Info("Could not find wait container in pod spec")
  return
  }
@@ -977,6 +962,9 @@ func addOutputArtifactsVolumes(pod *apiv1.Pod, tmpl *wfv1.Template) {
  continue
  }
  for _, mnt := range c.VolumeMounts {
+ if util.IsWindowsUNCPath(mnt.MountPath, tmpl) {
+ continue
+ }
  mnt.MountPath = filepath.Join(common.ExecutorMainFilesystemDir, mnt.MountPath)
  // ReadOnly is needed to be false for overlapping volume mounts
  mnt.ReadOnly = false

workflow/controller/workflowpod_test.go

@@ -18,6 +18,7 @@ import (
  "github.com/argoproj/argo-workflows/v3/test/util"
  armocks "github.com/argoproj/argo-workflows/v3/workflow/artifactrepositories/mocks"
  "github.com/argoproj/argo-workflows/v3/workflow/common"
+ wfutil "github.com/argoproj/argo-workflows/v3/workflow/util"
 )
 
 // Deprecated
@@ -1389,6 +1390,54 @@ func TestHybridWfVolumesWindows(t *testing.T) {
  assert.Equal(t, (*apiv1.HostPathType)(nil), pod.Spec.Volumes[0].HostPath.Type)
 }
 
+func TestWindowsUNCPathsAreRemoved(t *testing.T) {
+ wf := wfv1.MustUnmarshalWorkflow(helloWindowsWf)
+ uncVolume := apiv1.Volume{
+ Name: "unc",
+ VolumeSource: apiv1.VolumeSource{
+ HostPath: &apiv1.HostPathVolumeSource{
+ Path: "\\\\.\\pipe\\test",
+ },
+ },
+ }
+ uncMount := apiv1.VolumeMount{
+ Name: "unc",
+ MountPath: "\\\\.\\pipe\\test",
+ }
+
+ // Add artifacts so that initContainer volumeMount logic is run
+ inp := wfv1.Artifact{
+ Name: "kubectl",
+ Path: "C:\\kubectl",
+ ArtifactLocation: wfv1.ArtifactLocation{HTTP: &wfv1.HTTPArtifact{
+ URL: "https://dl.k8s.io/release/v1.22.0/bin/windows/amd64/kubectl.exe"},
+ },
+ }
+ wf.Spec.Volumes = append(wf.Spec.Volumes, uncVolume)
+ wf.Spec.Templates[0].Container.VolumeMounts = append(wf.Spec.Templates[0].Container.VolumeMounts, uncMount)
+ wf.Spec.Templates[0].Inputs.Artifacts = append(wf.Spec.Templates[0].Inputs.Artifacts, inp)
+ woc := newWoc(*wf)
+
+ ctx := context.Background()
+ mainCtr := woc.execWf.Spec.Templates[0].Container
+ pod, _ := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{})
+ waitCtrIdx, err := wfutil.FindWaitCtrIndex(pod)
+
+ if err != nil {
+ assert.Errorf(t, err, "could not find wait ctr index")
+ }
+ for _, mnt := range pod.Spec.Containers[waitCtrIdx].VolumeMounts {
+ assert.NotEqual(t, mnt.Name, "unc")
+ }
+ for _, initCtr := range pod.Spec.InitContainers {
+ if initCtr.Name == common.InitContainerName {
+ for _, mnt := range initCtr.VolumeMounts {
+ assert.NotEqual(t, mnt.Name, "unc")
+ }
+ }
+ }
+}
+
 func TestHybridWfVolumesLinux(t *testing.T) {
  wf := wfv1.MustUnmarshalWorkflow(helloLinuxWf)
  woc := newWoc(*wf)

workflow/util/util.go

@@ -11,6 +11,7 @@ import (
  "os"
  "path/filepath"
  "regexp"
+ nruntime "runtime"
  "strconv"
  "strings"
  "time"
@@ -1054,3 +1055,44 @@ func GetNodeType(tmpl *wfv1.Template) wfv1.NodeType {
  }
  return ""
 }
+
+// IsWindowsUNCPath checks if path is prefixed with \\
+// This can be used to skip any processing of paths
+// that point to SMB shares, local named pipes and local UNC path
+func IsWindowsUNCPath(path string, tmpl *wfv1.Template) bool {
+ if !HasWindowsOSNodeSelector(tmpl.NodeSelector) && nruntime.GOOS != "windows" {
+ return false
+ }
+ // Check for UNC prefix \\
+ if strings.HasPrefix(path, `\\`) {
+ return true
+ }
+ return false
+}
+
+func HasWindowsOSNodeSelector(nodeSelector map[string]string) bool {
+ if nodeSelector == nil {
+ return false
+ }
+
+ if platform, keyExists := nodeSelector["kubernetes.io/os"]; keyExists && platform == "windows" {
+ return true
+ }
+
+ return false
+}
+
+func FindWaitCtrIndex(pod *apiv1.Pod) (int, error) {
+ waitCtrIndex := -1
+ for i, ctr := range pod.Spec.Containers {
+ switch ctr.Name {
+ case common.WaitContainerName:
+ waitCtrIndex = i
+ }
+ }
+ if waitCtrIndex == -1 {
+ err := errors.Errorf("-1", "Could not find wait container in pod spec")
+ return -1, err
+ }
+ return waitCtrIndex, nil
+}