actorTemplate(fix): update volume mount validation

cmd/ateapi/internal/controlapi/actor_template.go

@@ -357,3 +357,58 @@ func actorTemplateObjectRef(actor *ateapipb.Actor) *ateapipb.ObjectRef {
 	}
 	return &ateapipb.ObjectRef{Atespace: ref.GetAtespace(), Name: ref.GetName()}
 }
+
+// ValidateCustom_Container_VolumeMounts rejects two mounts at the same path
+// within one container. The list is keyed by volume name (one mount per
+// volume), so path uniqueness cannot come from the list-map key
+func ValidateCustom_Container_VolumeMounts(_ context.Context, _ operation.Operation, fldPath *field.Path, value, _ []*ateapipb.VolumeMount) field.ErrorList {
+	var errs field.ErrorList
+	seen := make(map[string]bool, len(value))
+	for i, m := range value {
+		path := m.GetMountPath()
+		if path == "" {
+			continue // required is enforced by tags
+		}
+		if seen[path] {
+			errs = append(errs, field.Duplicate(fldPath.Index(i).Child("mount_path"), path))
+		}
+		// Nested mounts are unsupported (volumes cannot mount onto
+		// other volumes).
+		for j := 0; j < i; j++ {
+			prior := value[j].GetMountPath()
+			if prior == "" || prior == path {
+				continue
+			}
+			if strings.HasPrefix(path, prior+"/") || strings.HasPrefix(prior, path+"/") {
+				errs = append(errs, field.Invalid(fldPath.Index(i).Child("mount_path"), path,
+					fmt.Sprintf("must not nest under or over another mount (%q)", prior)))
+			}
+		}
+		seen[path] = true
+	}
+	return errs
+}
+
+// ValidateCustom_CreateActorTemplateRequest_ActorTemplate rejects container
+// volume mounts that reference volumes the template does not declare.
+func ValidateCustom_CreateActorTemplateRequest_ActorTemplate(_ context.Context, _ operation.Operation, fldPath *field.Path, value, _ *ateapipb.ActorTemplate) field.ErrorList {
+	declared := make(map[string]bool, len(value.GetVolumes()))
+	for _, vol := range value.GetVolumes() {
+		declared[vol.GetName()] = true
+	}
+	var errs field.ErrorList
+	for i, ctr := range value.GetContainers() {
+		for j, mount := range ctr.GetVolumeMounts() {
+			name := mount.GetName()
+			if name == "" {
+				continue // required is enforced by tags
+			}
+			if !declared[name] {
+				errs = append(errs, field.Invalid(
+					fldPath.Child("containers").Index(i).Child("volume_mounts").Index(j).Child("name"),
+					name, "must reference a volume declared in the template"))
+			}
+		}
+	}
+	return errs
+}

cmd/ateapi/internal/controlapi/actor_template_test.go

@@ -121,6 +121,19 @@ func TestValidateCreateActorTemplateRequest(t *testing.T) {
 			tmpl.Containers[0].Image = ""
 		})},
 		field.ErrorList{field.Required(field.NewPath("actor_template", "containers").Index(0).Child("image"), "")},
+	}, {
+		"volume mount referencing a declared volume",
+		&ateapipb.CreateActorTemplateRequest{ActorTemplate: validActorTemplate(func(tmpl *ateapipb.ActorTemplate) {
+			tmpl.Volumes = []*ateapipb.Volume{{Name: "data", DurableDir: &ateapipb.DurableDirVolumeSource{}}}
+			tmpl.Containers[0].VolumeMounts = []*ateapipb.VolumeMount{{Name: "data", MountPath: "/var/data"}}
+		})},
+		nil,
+	}, {
+		"volume mount referencing an undeclared volume",
+		&ateapipb.CreateActorTemplateRequest{ActorTemplate: validActorTemplate(func(tmpl *ateapipb.ActorTemplate) {
+			tmpl.Containers[0].VolumeMounts = []*ateapipb.VolumeMount{{Name: "ghost-vol", MountPath: "/var/data"}}
+		})},
+		field.ErrorList{field.Invalid(field.NewPath("actor_template", "containers").Index(0).Child("volume_mounts").Index(0).Child("name"), "ghost-vol", "")},
 	}, {
 		"missing snapshots_config",
 		&ateapipb.CreateActorTemplateRequest{ActorTemplate: validActorTemplate(func(tmpl *ateapipb.ActorTemplate) {
@@ -487,6 +500,73 @@ func TestValidateActorTemplate(t *testing.T) {
 			}
 		},
 		want: field.ErrorList{field.Duplicate(field.NewPath("containers").Index(0).Child("volume_mounts").Index(1), nil)},
+	}, {
+		name: "two volumes at the same path are rejected",
+		mutate: func(tmpl *ateapipb.ActorTemplate) {
+			tmpl.Containers[0].VolumeMounts = []*ateapipb.VolumeMount{
+				{Name: "data", MountPath: "/var/data"},
+				{Name: "other", MountPath: "/var/data"},
+			}
+		},
+		want: field.ErrorList{field.Duplicate(field.NewPath("containers").Index(0).Child("volume_mounts").Index(1).Child("mount_path"), nil)},
+	}, {
+		name: "nested mount paths are rejected",
+		mutate: func(tmpl *ateapipb.ActorTemplate) {
+			tmpl.Containers[0].VolumeMounts = []*ateapipb.VolumeMount{
+				{Name: "data", MountPath: "/data"},
+				{Name: "config", MountPath: "/data/config"},
+			}
+		},
+		want: field.ErrorList{field.Invalid(field.NewPath("containers").Index(0).Child("volume_mounts").Index(1).Child("mount_path"), nil, "")},
+	}, {
+		name: "deeply nested mount path is rejected",
+		mutate: func(tmpl *ateapipb.ActorTemplate) {
+			tmpl.Containers[0].VolumeMounts = []*ateapipb.VolumeMount{
+				{Name: "data", MountPath: "/data"},
+				{Name: "deep", MountPath: "/data/a/b/c"},
+			}
+		},
+		want: field.ErrorList{field.Invalid(field.NewPath("containers").Index(0).Child("volume_mounts").Index(1).Child("mount_path"), nil, "")},
+	}, {
+		name: "nesting is rejected regardless of listing order",
+		mutate: func(tmpl *ateapipb.ActorTemplate) {
+			tmpl.Containers[0].VolumeMounts = []*ateapipb.VolumeMount{
+				{Name: "deep", MountPath: "/data/a/b/c"},
+				{Name: "data", MountPath: "/data"},
+			}
+		},
+		want: field.ErrorList{field.Invalid(field.NewPath("containers").Index(0).Child("volume_mounts").Index(1).Child("mount_path"), nil, "")},
+	}, {
+		name: "sibling subpaths under an unmounted ancestor are allowed",
+		mutate: func(tmpl *ateapipb.ActorTemplate) {
+			tmpl.Containers[0].VolumeMounts = []*ateapipb.VolumeMount{
+				{Name: "data", MountPath: "/data/a"},
+				{Name: "other", MountPath: "/data/b"},
+			}
+		},
+	}, {
+		name: "shared segment prefix below the root is allowed",
+		mutate: func(tmpl *ateapipb.ActorTemplate) {
+			tmpl.Containers[0].VolumeMounts = []*ateapipb.VolumeMount{
+				{Name: "data", MountPath: "/data/a"},
+				{Name: "other", MountPath: "/data/ab"},
+			}
+		},
+	}, {
+		name: "the same path in different containers is allowed",
+		mutate: func(tmpl *ateapipb.ActorTemplate) {
+			tmpl.Containers = append(tmpl.Containers, &ateapipb.Container{Name: "sidecar", Image: "example.com/side:v1"})
+			tmpl.Containers[0].VolumeMounts = []*ateapipb.VolumeMount{{Name: "data", MountPath: "/var/data"}}
+			tmpl.Containers[1].VolumeMounts = []*ateapipb.VolumeMount{{Name: "data", MountPath: "/var/data"}}
+		},
+	}, {
+		name: "shared path prefix without nesting is allowed",
+		mutate: func(tmpl *ateapipb.ActorTemplate) {
+			tmpl.Containers[0].VolumeMounts = []*ateapipb.VolumeMount{
+				{Name: "data", MountPath: "/data"},
+				{Name: "other", MountPath: "/database"},
+			}
+		},
 	}, {
 		name: "two volumes at distinct paths are allowed",
 		mutate: func(tmpl *ateapipb.ActorTemplate) {

pkg/proto/ateapipb/ateapi.proto

@@ -923,17 +923,15 @@ message Container {
   // +k8s:optional
   ContainerReadyz readyz = 6;
 
-  // Each volume may be mounted at most once per container.
-  //
-  // TODO: Kubernetes permits mounting a single volume at multiple paths (which
-  // requires keying by mountPath). We restrict it to one mount per volume (keyed
-  // by name). Note that keying by name means DV will not catch
-  // two different volumes mounted to the same path.
+  // TODO: Kubernetes permits mounting a single volume at multiple paths
+  // (which requires keying by mountPath). We restrict it to one mount per
+  // volume (keyed by name).
   //
   // +k8s:optional
   // +k8s:maxItems=32
   // +k8s:listType=map
   // +k8s:listMapKey=name
+  // +k8s:customValidation # mount_path must be unique within the container
   repeated VolumeMount volume_mounts = 7;
 
   // security_context adjusts the container's security settings. Unset leaves
@@ -1236,6 +1234,7 @@ message CreateActorTemplateRequest {
   // version, timestamps) is ignored, as are the status fields.
   //
   // +k8s:required
+  // +k8s:customValidation # volume_mounts must reference declared volumes
   ActorTemplate actor_template = 1;
 }