bug(backend,sdk): Use a valid path separator for Modelcar imports (kubeflow#11767)

Forward slashes are invalid characters in a path and can't be escaped.

Signed-off-by: mprahl <[email protected]>
(cherry picked from commit 2694605)

Author: mprahl

backend/src/v2/component/launcher_v2.go

@@ -821,7 +821,7 @@ func LocalPathForURI(uri string) (string, error) {
 		return "/s3/" + strings.TrimPrefix(uri, "s3://"), nil
 	}
 	if strings.HasPrefix(uri, "oci://") {
-		return "/oci/" + strings.ReplaceAll(strings.TrimPrefix(uri, "oci://"), "/", "\\/") + "/models", nil
+		return "/oci/" + strings.ReplaceAll(strings.TrimPrefix(uri, "oci://"), "/", "_") + "/models", nil
 	}
 	return "", fmt.Errorf("failed to generate local path for URI %s: unsupported storage scheme", uri)
 }

backend/src/v2/driver/driver_test.go

@@ -439,15 +439,15 @@ func Test_initPodSpecPatch_modelcar_input_artifact(t *testing.T) {
 	assert.Len(t, podSpec.Containers, 2)
 	assert.Len(t, podSpec.Containers[0].VolumeMounts, 1)
 	assert.Equal(t, podSpec.Containers[0].VolumeMounts[0].Name, "oci-0")
-	assert.Equal(t, podSpec.Containers[0].VolumeMounts[0].MountPath, "/oci/registry.domain.local\\/my-model:latest")
-	assert.Equal(t, podSpec.Containers[0].VolumeMounts[0].SubPath, "registry.domain.local\\/my-model:latest")
+	assert.Equal(t, podSpec.Containers[0].VolumeMounts[0].MountPath, "/oci/registry.domain.local_my-model:latest")
+	assert.Equal(t, podSpec.Containers[0].VolumeMounts[0].SubPath, "registry.domain.local_my-model:latest")
 
 	assert.Equal(t, podSpec.Containers[1].Name, "oci-0")
 	assert.Equal(t, podSpec.Containers[1].Image, "registry.domain.local/my-model:latest")
 	assert.Len(t, podSpec.Containers[1].VolumeMounts, 1)
 	assert.Equal(t, podSpec.Containers[1].VolumeMounts[0].Name, "oci-0")
-	assert.Equal(t, podSpec.Containers[1].VolumeMounts[0].MountPath, "/oci/registry.domain.local\\/my-model:latest")
-	assert.Equal(t, podSpec.Containers[1].VolumeMounts[0].SubPath, "registry.domain.local\\/my-model:latest")
+	assert.Equal(t, podSpec.Containers[1].VolumeMounts[0].MountPath, "/oci/registry.domain.local_my-model:latest")
+	assert.Equal(t, podSpec.Containers[1].VolumeMounts[0].SubPath, "registry.domain.local_my-model:latest")
 }
 
 func Test_makeVolumeMountPatch(t *testing.T) {
@@ -925,7 +925,8 @@ func Test_extendPodSpecPatch_ConfigMap(t *testing.T) {
 						VolumeSource: k8score.VolumeSource{
 							ConfigMap: &k8score.ConfigMapVolumeSource{
 								LocalObjectReference: k8score.LocalObjectReference{Name: "cm1"},
-								Optional:             &[]bool{false}[0]},
+								Optional:             &[]bool{false}[0],
+							},
 						},
 					},
 				},
@@ -967,7 +968,8 @@ func Test_extendPodSpecPatch_ConfigMap(t *testing.T) {
 						VolumeSource: k8score.VolumeSource{
 							ConfigMap: &k8score.ConfigMapVolumeSource{
 								LocalObjectReference: k8score.LocalObjectReference{Name: "cm1"},
-								Optional:             &[]bool{false}[0]},
+								Optional:             &[]bool{false}[0],
+							},
 						},
 					},
 				},
@@ -1009,7 +1011,8 @@ func Test_extendPodSpecPatch_ConfigMap(t *testing.T) {
 						VolumeSource: k8score.VolumeSource{
 							ConfigMap: &k8score.ConfigMapVolumeSource{
 								LocalObjectReference: k8score.LocalObjectReference{Name: "cm1"},
-								Optional:             &[]bool{true}[0]},
+								Optional:             &[]bool{true}[0],
+							},
 						},
 					},
 				},

sdk/python/kfp/dsl/types/artifact_types.py

@@ -98,7 +98,7 @@ def _get_path(self) -> Optional[str]:
             return _S3_LOCAL_MOUNT_PREFIX + self.uri[len(S3_REMOTE_PREFIX):]
 
         elif self.uri.startswith(OCI_REMOTE_PREFIX):
-            escaped_uri = self.uri[len(OCI_REMOTE_PREFIX):].replace('/', '\\/')
+            escaped_uri = self.uri[len(OCI_REMOTE_PREFIX):].replace('/', '_')
             return _OCI_LOCAL_MOUNT_PREFIX + escaped_uri
         # uri == path for local execution
         return self.uri
@@ -115,7 +115,7 @@ def convert_local_path_to_remote_path(path: str) -> str:
     elif path.startswith(_S3_LOCAL_MOUNT_PREFIX):
         return S3_REMOTE_PREFIX + path[len(_S3_LOCAL_MOUNT_PREFIX):]
     elif path.startswith(_OCI_LOCAL_MOUNT_PREFIX):
-        remote_path = path[len(_OCI_LOCAL_MOUNT_PREFIX):].replace('\\/', '/')
+        remote_path = path[len(_OCI_LOCAL_MOUNT_PREFIX):].replace('_', '/')
         if remote_path.endswith("/models"):
             remote_path = remote_path[:-len("/models")]
 

sdk/python/kfp/dsl/types/artifact_types_test.py

@@ -138,9 +138,9 @@ class TestConvertLocalPathToRemotePath(parameterized.TestCase):
         ('/gcs/foo/bar', 'gs://foo/bar'),
         ('/minio/foo/bar', 'minio://foo/bar'),
         ('/s3/foo/bar', 's3://foo/bar'),
-        ('/oci/quay.io\\/org\\/repo:latest/models',
+        ('/oci/quay.io_org_repo:latest/models',
          'oci://quay.io/org/repo:latest'),
-        ('/oci/quay.io\\/org\\/repo:latest', 'oci://quay.io/org/repo:latest'),
+        ('/oci/quay.io_org_repo:latest', 'oci://quay.io/org/repo:latest'),
         ('/tmp/kfp_outputs', '/tmp/kfp_outputs'),
         ('/some/random/path', '/some/random/path'),
     ]])