ateapi: UpdateActor allows updating ActorTemplate.

cmd/ateapi/internal/controlapi/actor.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"slices"
 	"time"
 
 	"github.com/agent-substrate/substrate/cmd/ateapi/internal/store"
@@ -277,6 +278,34 @@ func (s *ServiceImpl) UpdateActor(ctx context.Context, actorRef resources.ActorR
 
 		// Do any further work on the resource.
 
+		// Update actor template is only allowed while the actor is suspended.
+		// The repointed ref must also resolve, mirroring CreateActor's
+		// check (same non-atomicity caveat; resume re-resolves and fails
+		// cleanly), and the replacement's sandbox class, volumes, and volume
+		// mounts must match the old template's.
+		if !proto.Equal(oldVal.GetActorTemplate(), newVal.GetActorTemplate()) {
+			if state := oldVal.GetStatus().GetState(); state != ateapipb.ActorState_ACTOR_STATE_SUSPENDED {
+				return status.Errorf(codes.FailedPrecondition,
+					"actor must be %s to change its actor template (got: %s)", ateapipb.ActorState_ACTOR_STATE_SUSPENDED, state)
+			}
+			newTemplate, err := resolveActorTemplate(ctx, s.store, newVal)
+			if err != nil {
+				return err
+			}
+			oldTemplate, err := resolveActorTemplate(ctx, s.store, oldVal)
+			if err == nil {
+				if err := validateTemplateSandboxClassUnchanged(oldTemplate, newTemplate); err != nil {
+					return err
+				}
+				if err := validateTemplateVolumesUnchanged(oldTemplate, newTemplate); err != nil {
+					return err
+				}
+			} else if !errors.Is(err, errActorTemplateNotFound) {
+				// Skip the validation if old template is not found
+				return err
+			}
+		}
+
 		// Validate the final value before storing it.
 		if errs := validateActorUpdate(ctx, field.NewPath("actor"), newVal, oldVal, true); len(errs) > 0 {
 			return toGRPCInternalError(errs)
@@ -302,6 +331,54 @@ func (s *ServiceImpl) UpdateActor(ctx context.Context, actorRef resources.ActorR
 	return storedActor, nil
 }
 
+// validateTemplateSandboxClassUnchanged rejects a template repoint that
+// changes the sandbox class: snapshots are not portable across sandbox
+// runtime families, so the actor's saved state could not be restored under
+// the new template.
+func validateTemplateSandboxClassUnchanged(oldTemplate, newTemplate *ateapipb.ActorTemplate) error {
+	oldClass := oldTemplate.GetSandboxConfig().GetSandboxClass()
+	newClass := newTemplate.GetSandboxConfig().GetSandboxClass()
+	if oldClass != newClass {
+		return status.Errorf(codes.FailedPrecondition,
+			"sandbox class differs between the current (%s) and the new (%s) actor template; the sandbox class must be identical to repoint an actor", oldClass, newClass)
+	}
+	return nil
+}
+
+// validateTemplateVolumesUnchanged rejects a template repoint that changes
+// the template's volumes or any container's volume mounts: an actor's
+// snapshot data is laid out per the volumes and mount paths it was captured
+// with, so a different layout would restore it to the wrong places. The
+// volumes list must be identical, and containers present in both templates
+// must keep identical mounts, order included; containers added or removed by
+// the new template are unconstrained.
+func validateTemplateVolumesUnchanged(oldTemplate, newTemplate *ateapipb.ActorTemplate) error {
+	if !slices.EqualFunc(oldTemplate.GetVolumes(), newTemplate.GetVolumes(), func(a, b *ateapipb.Volume) bool {
+		return proto.Equal(a, b)
+	}) {
+		return status.Error(codes.FailedPrecondition,
+			"volumes differ between the current and the new actor template; volumes must be identical to repoint an actor")
+	}
+
+	newContainers := make(map[string]*ateapipb.Container, len(newTemplate.GetContainers()))
+	for _, c := range newTemplate.GetContainers() {
+		newContainers[c.GetName()] = c
+	}
+	for _, oldC := range oldTemplate.GetContainers() {
+		newC, ok := newContainers[oldC.GetName()]
+		if !ok {
+			continue
+		}
+		if !slices.EqualFunc(oldC.GetVolumeMounts(), newC.GetVolumeMounts(), func(a, b *ateapipb.VolumeMount) bool {
+			return proto.Equal(a, b)
+		}) {
+			return status.Errorf(codes.FailedPrecondition,
+				"volume mounts of container %q differ between the current and the new actor template; volume mounts must be identical to repoint an actor", oldC.GetName())
+		}
+	}
+	return nil
+}
+
 func validateUpdateActorRequest(ctx context.Context, req *ateapipb.UpdateActorRequest) field.ErrorList {
 	// Call the generated validation.
 	// We model this as a create rather than an update because updates assume

cmd/ateapi/internal/controlapi/actor_test.go

@@ -867,6 +867,288 @@ func TestUpdateActor(t *testing.T) {
 	}
 }
 
+// TestUpdateActor_RepointTemplate covers the mutable actor_template ref: an
+// update may point a suspended actor at a different template (it takes effect
+// on the next ResumeActor), but the actor must be suspended, the new ref must
+// resolve, and the replacement's volumes and volume mounts must match the old
+// template's.
+func TestUpdateActor_RepointTemplate(t *testing.T) {
+	ctx := context.Background()
+	persistence, cleanup := storetest.SetupTestStore(t)
+	t.Cleanup(cleanup)
+
+	storetest.MustCreateAtespace(t, ctx, persistence, testAtespace)
+	// tmpl-a and tmpl-b are volume-compatible; tmpl-c mounts the data volume
+	// elsewhere, tmpl-d declares an extra volume, and tmpl-e runs on a
+	// different sandbox class.
+	dataVolume := &ateapipb.Volume{Name: "data", DurableDir: &ateapipb.DurableDirVolumeSource{}}
+	scratchVolume := &ateapipb.Volume{Name: "scratch", DurableDir: &ateapipb.DurableDirVolumeSource{}}
+	gvisorConfig := &ateapipb.SandboxConfig{SandboxClass: ateapipb.SandboxClass_SANDBOX_CLASS_GVISOR, ConfigName: "gvisor-default"}
+	microvmConfig := &ateapipb.SandboxConfig{SandboxClass: ateapipb.SandboxClass_SANDBOX_CLASS_MICROVM, ConfigName: "microvm"}
+	templates := map[string]struct {
+		mountPath     string
+		volumes       []*ateapipb.Volume
+		sandboxConfig *ateapipb.SandboxConfig
+	}{
+		"tmpl-a": {"/data", []*ateapipb.Volume{dataVolume}, gvisorConfig},
+		"tmpl-b": {"/data", []*ateapipb.Volume{dataVolume}, gvisorConfig},
+		"tmpl-c": {"/mnt/data", []*ateapipb.Volume{dataVolume}, gvisorConfig},
+		"tmpl-d": {"/data", []*ateapipb.Volume{dataVolume, scratchVolume}, gvisorConfig},
+		"tmpl-e": {"/data", []*ateapipb.Volume{dataVolume}, microvmConfig},
+	}
+	for name, tmpl := range templates {
+		if _, err := persistence.CreateActorTemplate(ctx, &ateapipb.ActorTemplate{
+			Metadata: &ateapipb.ResourceMetadata{Atespace: testAtespace, Name: name},
+			Containers: []*ateapipb.Container{{
+				Name:         "main",
+				Image:        "example.com/app:v1",
+				VolumeMounts: []*ateapipb.VolumeMount{{Name: "data", MountPath: tmpl.mountPath}},
+			}},
+			Volumes:         tmpl.volumes,
+			SnapshotsConfig: &ateapipb.SnapshotsConfig{StorageLocation: "gs://my-bucket/snapshots"},
+			SandboxConfig:   tmpl.sandboxConfig,
+		}); err != nil {
+			t.Fatalf("creating template %s: %v", name, err)
+		}
+	}
+
+	created := storetest.MustCreateActor(t, ctx, persistence, &ateapipb.Actor{
+		Metadata:      &ateapipb.ResourceMetadata{Atespace: testAtespace, Name: testActorID},
+		ActorTemplate: &ateapipb.ObjectRef{Atespace: testAtespace, Name: "tmpl-a"},
+		Status:        &ateapipb.ActorStatus{State: ateapipb.ActorState_ACTOR_STATE_SUSPENDED},
+	})
+	svc := &RPCService{impl: newServiceImpl(persistence, nil)}
+
+	// Repointing at a template that does not exist is rejected.
+	_, err := svc.UpdateActor(ctx, &ateapipb.UpdateActorRequest{Actor: &ateapipb.Actor{
+		Metadata:      created.GetMetadata(),
+		ActorTemplate: &ateapipb.ObjectRef{Atespace: testAtespace, Name: "absent"},
+	}})
+	if got := status.Code(err); got != codes.FailedPrecondition {
+		t.Fatalf("UpdateActor to an absent template = %v, want FailedPrecondition (err: %v)", got, err)
+	}
+
+	// Repointing at a template with different volume mounts is rejected.
+	_, err = svc.UpdateActor(ctx, &ateapipb.UpdateActorRequest{Actor: &ateapipb.Actor{
+		Metadata:      created.GetMetadata(),
+		ActorTemplate: &ateapipb.ObjectRef{Atespace: testAtespace, Name: "tmpl-c"},
+	}})
+	if got := status.Code(err); got != codes.FailedPrecondition {
+		t.Fatalf("UpdateActor to a template with different mounts = %v, want FailedPrecondition (err: %v)", got, err)
+	}
+
+	// Repointing at a template with different volumes is rejected.
+	_, err = svc.UpdateActor(ctx, &ateapipb.UpdateActorRequest{Actor: &ateapipb.Actor{
+		Metadata:      created.GetMetadata(),
+		ActorTemplate: &ateapipb.ObjectRef{Atespace: testAtespace, Name: "tmpl-d"},
+	}})
+	if got := status.Code(err); got != codes.FailedPrecondition {
+		t.Fatalf("UpdateActor to a template with different volumes = %v, want FailedPrecondition (err: %v)", got, err)
+	}
+
+	// Repointing at a template with a different sandbox class is rejected.
+	_, err = svc.UpdateActor(ctx, &ateapipb.UpdateActorRequest{Actor: &ateapipb.Actor{
+		Metadata:      created.GetMetadata(),
+		ActorTemplate: &ateapipb.ObjectRef{Atespace: testAtespace, Name: "tmpl-e"},
+	}})
+	if got := status.Code(err); got != codes.FailedPrecondition {
+		t.Fatalf("UpdateActor to a template with a different sandbox class = %v, want FailedPrecondition (err: %v)", got, err)
+	}
+
+	// Repointing at an existing template with identical volumes and mounts
+	// succeeds.
+	updated, err := svc.UpdateActor(ctx, &ateapipb.UpdateActorRequest{Actor: &ateapipb.Actor{
+		Metadata:      created.GetMetadata(),
+		ActorTemplate: &ateapipb.ObjectRef{Atespace: testAtespace, Name: "tmpl-b"},
+	}})
+	if err != nil {
+		t.Fatalf("UpdateActor failed: %v", err)
+	}
+	if got, want := updated.GetActorTemplate().GetName(), "tmpl-b"; got != want {
+		t.Errorf("updated actor_template.name = %q, want %q", got, want)
+	}
+
+	// Repointing an actor that is not suspended is rejected, even at a
+	// compatible template.
+	running := storetest.MustCreateActor(t, ctx, persistence, &ateapipb.Actor{
+		Metadata:      &ateapipb.ResourceMetadata{Atespace: testAtespace, Name: "running-actor"},
+		ActorTemplate: &ateapipb.ObjectRef{Atespace: testAtespace, Name: "tmpl-a"},
+		Status:        &ateapipb.ActorStatus{State: ateapipb.ActorState_ACTOR_STATE_RUNNING},
+	})
+	_, err = svc.UpdateActor(ctx, &ateapipb.UpdateActorRequest{Actor: &ateapipb.Actor{
+		Metadata:      running.GetMetadata(),
+		ActorTemplate: &ateapipb.ObjectRef{Atespace: testAtespace, Name: "tmpl-b"},
+	}})
+	if got := status.Code(err); got != codes.FailedPrecondition {
+		t.Fatalf("UpdateActor repointing a running actor = %v, want FailedPrecondition (err: %v)", got, err)
+	}
+
+	// An update that keeps the template ref is still allowed while running:
+	// the suspended-state gate applies only to repoints.
+	kept, err := svc.UpdateActor(ctx, &ateapipb.UpdateActorRequest{Actor: &ateapipb.Actor{
+		Metadata:       running.GetMetadata(),
+		ActorTemplate:  &ateapipb.ObjectRef{Atespace: testAtespace, Name: "tmpl-a"},
+		WorkerSelector: &ateapipb.Selector{MatchLabels: map[string]string{"tier": "paid"}},
+	}})
+	if err != nil {
+		t.Fatalf("UpdateActor keeping the template on a running actor failed: %v", err)
+	}
+	if got, want := kept.GetActorTemplate().GetName(), "tmpl-a"; got != want {
+		t.Errorf("updated actor_template.name = %q, want %q", got, want)
+	}
+}
+
+// TestValidateTemplateSandboxClassUnchanged exercises the sandbox class
+// comparison applied when an actor is repointed at a replacement template.
+func TestValidateTemplateSandboxClassUnchanged(t *testing.T) {
+	template := func(config *ateapipb.SandboxConfig) *ateapipb.ActorTemplate {
+		return &ateapipb.ActorTemplate{SandboxConfig: config}
+	}
+	gvisorDefault := &ateapipb.SandboxConfig{SandboxClass: ateapipb.SandboxClass_SANDBOX_CLASS_GVISOR, ConfigName: "gvisor-default"}
+	gvisorNightly := &ateapipb.SandboxConfig{SandboxClass: ateapipb.SandboxClass_SANDBOX_CLASS_GVISOR, ConfigName: "gvisor-nightly"}
+	microvm := &ateapipb.SandboxConfig{SandboxClass: ateapipb.SandboxClass_SANDBOX_CLASS_MICROVM, ConfigName: "microvm"}
+
+	tests := []struct {
+		name             string
+		oldTmpl, newTmpl *ateapipb.ActorTemplate
+		wantErr          bool
+	}{{
+		name:    "same sandbox class",
+		oldTmpl: template(gvisorDefault),
+		newTmpl: template(gvisorDefault),
+	}, {
+		name:    "same class with a different config name",
+		oldTmpl: template(gvisorDefault),
+		newTmpl: template(gvisorNightly),
+	}, {
+		name:    "class changed",
+		oldTmpl: template(gvisorDefault),
+		newTmpl: template(microvm),
+		wantErr: true,
+	}, {
+		name:    "class set on the new template only",
+		oldTmpl: template(nil),
+		newTmpl: template(microvm),
+		wantErr: true,
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateTemplateSandboxClassUnchanged(tt.oldTmpl, tt.newTmpl)
+			if gotErr := err != nil; gotErr != tt.wantErr {
+				t.Fatalf("validateTemplateSandboxClassUnchanged() error = %v, wantErr %v", err, tt.wantErr)
+			}
+			if err != nil {
+				if got := status.Code(err); got != codes.FailedPrecondition {
+					t.Errorf("status code = %v, want FailedPrecondition", got)
+				}
+			}
+		})
+	}
+}
+
+// TestValidateTemplateVolumesUnchanged exercises the volumes and
+// per-container mount comparison applied when an actor is repointed at a
+// replacement template.
+func TestValidateTemplateVolumesUnchanged(t *testing.T) {
+	dataVolume := &ateapipb.Volume{Name: "data", DurableDir: &ateapipb.DurableDirVolumeSource{}}
+	scratchVolume := &ateapipb.Volume{Name: "scratch", DurableDir: &ateapipb.DurableDirVolumeSource{}}
+	template := func(volumes []*ateapipb.Volume, containers ...*ateapipb.Container) *ateapipb.ActorTemplate {
+		return &ateapipb.ActorTemplate{Volumes: volumes, Containers: containers}
+	}
+	container := func(name string, mounts ...*ateapipb.VolumeMount) *ateapipb.Container {
+		return &ateapipb.Container{Name: name, Image: "example.com/app:v1", VolumeMounts: mounts}
+	}
+	dataMount := &ateapipb.VolumeMount{Name: "data", MountPath: "/data"}
+	scratchMount := &ateapipb.VolumeMount{Name: "scratch", MountPath: "/scratch"}
+
+	oneVolume := []*ateapipb.Volume{dataVolume}
+	twoVolumes := []*ateapipb.Volume{dataVolume, scratchVolume}
+
+	tests := []struct {
+		name             string
+		oldTmpl, newTmpl *ateapipb.ActorTemplate
+		wantErr          bool
+	}{{
+		name:    "identical volumes and mounts",
+		oldTmpl: template(oneVolume, container("main", dataMount)),
+		newTmpl: template(oneVolume, container("main", dataMount)),
+	}, {
+		name:    "no volumes or mounts on either side",
+		oldTmpl: template(nil, container("main")),
+		newTmpl: template(nil, container("other")),
+	}, {
+		name:    "volume added",
+		oldTmpl: template(oneVolume, container("main", dataMount)),
+		newTmpl: template(twoVolumes, container("main", dataMount)),
+		wantErr: true,
+	}, {
+		name:    "volume removed",
+		oldTmpl: template(twoVolumes, container("main", dataMount)),
+		newTmpl: template(oneVolume, container("main", dataMount)),
+		wantErr: true,
+	}, {
+		name:    "volume renamed",
+		oldTmpl: template(oneVolume, container("main", dataMount)),
+		newTmpl: template([]*ateapipb.Volume{{Name: "data2", DurableDir: &ateapipb.DurableDirVolumeSource{}}}, container("main", dataMount)),
+		wantErr: true,
+	}, {
+		name:    "volume source changed",
+		oldTmpl: template(oneVolume, container("main", dataMount)),
+		newTmpl: template([]*ateapipb.Volume{{Name: "data", Image: &ateapipb.ImageVolumeSource{Reference: "example.com/data@sha256:0f9c04b7387d13ba9d15ec50355f9ad533fee2e5ad25378753a30671f8f9b938"}}}, container("main", dataMount)),
+		wantErr: true,
+	}, {
+		name:    "volume order changed",
+		oldTmpl: template(twoVolumes, container("main", dataMount)),
+		newTmpl: template([]*ateapipb.Volume{scratchVolume, dataVolume}, container("main", dataMount)),
+		wantErr: true,
+	}, {
+		name:    "mount path changed",
+		oldTmpl: template(oneVolume, container("main", dataMount)),
+		newTmpl: template(oneVolume, container("main", &ateapipb.VolumeMount{Name: "data", MountPath: "/mnt/data"})),
+		wantErr: true,
+	}, {
+		name:    "mount added",
+		oldTmpl: template(twoVolumes, container("main", dataMount)),
+		newTmpl: template(twoVolumes, container("main", dataMount, scratchMount)),
+		wantErr: true,
+	}, {
+		name:    "mount removed",
+		oldTmpl: template(oneVolume, container("main", dataMount)),
+		newTmpl: template(oneVolume, container("main")),
+		wantErr: true,
+	}, {
+		name:    "mounted container renamed",
+		oldTmpl: template(oneVolume, container("main", dataMount)),
+		newTmpl: template(oneVolume, container("renamed", dataMount)),
+	}, {
+		name:    "container added with mounts",
+		oldTmpl: template(twoVolumes, container("main", dataMount)),
+		newTmpl: template(twoVolumes, container("main", dataMount), container("sidecar", scratchMount)),
+	}, {
+		name:    "mount order changed",
+		oldTmpl: template(twoVolumes, container("main", dataMount, scratchMount)),
+		newTmpl: template(twoVolumes, container("main", scratchMount, dataMount)),
+		wantErr: true,
+	}, {
+		name:    "mountless container renamed",
+		oldTmpl: template(oneVolume, container("main", dataMount), container("sidecar")),
+		newTmpl: template(oneVolume, container("main", dataMount), container("helper")),
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateTemplateVolumesUnchanged(tt.oldTmpl, tt.newTmpl)
+			if gotErr := err != nil; gotErr != tt.wantErr {
+				t.Fatalf("validateVolumeMountsUnchanged() error = %v, wantErr %v", err, tt.wantErr)
+			}
+			if err != nil {
+				if got := status.Code(err); got != codes.FailedPrecondition {
+					t.Errorf("status code = %v, want FailedPrecondition", got)
+				}
+			}
+		})
+	}
+}
+
 // TestUpdateActor_DeleteRecreateRace checks that an update is not applied
 // if an actor was deleted and recreated during the update operation.
 func TestUpdateActor_DeleteRecreateRace(t *testing.T) {