From b067a5c2a9429f5d1c3bfcf98368ae00aece8cec Mon Sep 17 00:00:00 2001 From: Yiqing Wang Date: Sat, 15 Aug 2026 00:40:39 -0700 Subject: [PATCH] ateapi: align ActorSnapshot request validation Signed-off-by: Yiqing Wang --- .../internal/controlapi/actor_snapshot.go | 14 +- .../controlapi/actor_snapshot_test.go | 259 ++++++++++++++++++ .../internal/controlapi/functional_test.go | 4 +- 3 files changed, 269 insertions(+), 8 deletions(-) diff --git a/cmd/ateapi/internal/controlapi/actor_snapshot.go b/cmd/ateapi/internal/controlapi/actor_snapshot.go index 7e34d8de0..2683f97b0 100644 --- a/cmd/ateapi/internal/controlapi/actor_snapshot.go +++ b/cmd/ateapi/internal/controlapi/actor_snapshot.go @@ -133,9 +133,6 @@ func (s *Service) CreateActorSnapshotTag(ctx context.Context, req *ateapipb.Crea return nil, toGRPCStatusError(errs) } ref := req.GetActorSnapshotTag().GetSnapshot() - if req.GetActorSnapshotTag().GetMetadata().GetAtespace() != ref.GetAtespace() { - return nil, status.Error(codes.FailedPrecondition, "ActorSnapshot tags must belong to the snapshot's Atespace") - } tag, err := s.persistence.CreateActorSnapshotTag(ctx, ref.GetAtespace(), ref.GetName(), req.GetActorSnapshotTag()) if errors.Is(err, store.ErrNotFound) { return nil, status.Error(codes.NotFound, "ActorSnapshot not found") @@ -156,7 +153,7 @@ func validateCreateActorSnapshotTagRequest(req *ateapipb.CreateActorSnapshotTagR var fldPath *field.Path var errs field.ErrorList - tag := req.ActorSnapshotTag + tag := req.GetActorSnapshotTag() tagPath := fldPath.Child("actor_snapshot_tag") if tag == nil { errs = append(errs, field.Required(tagPath, "")) @@ -165,7 +162,7 @@ func validateCreateActorSnapshotTagRequest(req *ateapipb.CreateActorSnapshotTagR errs = append(errs, resources.ValidateObjectRef(&ateapipb.ObjectRef{Atespace: tag.GetMetadata().GetAtespace(), Name: tag.GetMetadata().GetName()}, tagPath.Child("metadata"))...) - if val, p := tag.Snapshot, tagPath.Child("snapshot"); val == nil { + if val, p := tag.GetSnapshot(), tagPath.Child("snapshot"); val == nil { errs = append(errs, field.Required(p, "")) } else { errs = append(errs, resources.ValidateObjectRef(val, p)...) @@ -173,6 +170,11 @@ func validateCreateActorSnapshotTagRequest(req *ateapipb.CreateActorSnapshotTagR errs = append(errs, validateActorSnapshotTagScope(tag.GetScope(), tagPath.Child("scope"))...) + metaAtespace, snapAtespace := tag.GetMetadata().GetAtespace(), tag.GetSnapshot().GetAtespace() + if metaAtespace != snapAtespace && resources.IsValidResourceName(metaAtespace) && resources.IsValidResourceName(snapAtespace) { + errs = append(errs, field.Invalid(tagPath.Child("metadata").Child("atespace"), metaAtespace, "must match snapshot.atespace")) + } + return errs } @@ -218,7 +220,7 @@ func validateUpdateActorSnapshotTagRequest(req *ateapipb.UpdateActorSnapshotTagR errs = append(errs, resources.ValidateResourceMetadataRef(tag.GetMetadata(), tagPath.Child("metadata"))...) - errs = append(errs, fieldmask.Validate(req.GetUpdateMask(), actorSnapshotTagMutableFields, field.NewPath("update_mask"))...) + errs = append(errs, fieldmask.Validate(req.GetUpdateMask(), actorSnapshotTagMutableFields, fldPath.Child("update_mask"))...) errs = append(errs, validateActorSnapshotTagScope(tag.GetScope(), tagPath.Child("scope"))...) diff --git a/cmd/ateapi/internal/controlapi/actor_snapshot_test.go b/cmd/ateapi/internal/controlapi/actor_snapshot_test.go index 0193adeed..350e28a47 100644 --- a/cmd/ateapi/internal/controlapi/actor_snapshot_test.go +++ b/cmd/ateapi/internal/controlapi/actor_snapshot_test.go @@ -29,6 +29,244 @@ import ( "github.com/agent-substrate/substrate/pkg/proto/ateapipb" ) +func TestValidateGetActorSnapshotRequest(t *testing.T) { + tests := []struct { + name string + req *ateapipb.GetActorSnapshotRequest + want field.ErrorList + }{{ + "valid", + &ateapipb.GetActorSnapshotRequest{Snapshot: &ateapipb.ObjectRef{Atespace: "ns1", Name: "snap1"}}, + nil, + }, { + "missing snapshot", + &ateapipb.GetActorSnapshotRequest{}, + field.ErrorList{field.Required(field.NewPath("snapshot"), "")}, + }, { + "missing snapshot.atespace", + &ateapipb.GetActorSnapshotRequest{Snapshot: &ateapipb.ObjectRef{Name: "snap1"}}, + field.ErrorList{field.Required(field.NewPath("snapshot", "atespace"), "")}, + }, { + "invalid snapshot.atespace", + &ateapipb.GetActorSnapshotRequest{Snapshot: &ateapipb.ObjectRef{Atespace: "NS1", Name: "snap1"}}, + field.ErrorList{field.Invalid(field.NewPath("snapshot", "atespace"), "NS1", "")}, + }, { + "missing snapshot.name", + &ateapipb.GetActorSnapshotRequest{Snapshot: &ateapipb.ObjectRef{Atespace: "ns1"}}, + field.ErrorList{field.Required(field.NewPath("snapshot", "name"), "")}, + }, { + "invalid snapshot.name", + &ateapipb.GetActorSnapshotRequest{Snapshot: &ateapipb.ObjectRef{Atespace: "ns1", Name: "SNAP1"}}, + field.ErrorList{field.Invalid(field.NewPath("snapshot", "name"), "SNAP1", "")}, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assertValidateErr(t, validateGetActorSnapshotRequest(tt.req), tt.want) + }) + } +} + +func TestValidateGetActorSnapshotTagRequest(t *testing.T) { + tests := []struct { + name string + req *ateapipb.GetActorSnapshotTagRequest + want field.ErrorList + }{{ + "valid", + &ateapipb.GetActorSnapshotTagRequest{Tag: &ateapipb.ObjectRef{Atespace: "ns1", Name: "tag1"}}, + nil, + }, { + "missing tag", + &ateapipb.GetActorSnapshotTagRequest{}, + field.ErrorList{field.Required(field.NewPath("tag"), "")}, + }, { + "missing tag.atespace", + &ateapipb.GetActorSnapshotTagRequest{Tag: &ateapipb.ObjectRef{Name: "tag1"}}, + field.ErrorList{field.Required(field.NewPath("tag", "atespace"), "")}, + }, { + "invalid tag.atespace", + &ateapipb.GetActorSnapshotTagRequest{Tag: &ateapipb.ObjectRef{Atespace: "NS1", Name: "tag1"}}, + field.ErrorList{field.Invalid(field.NewPath("tag", "atespace"), "NS1", "")}, + }, { + "missing tag.name", + &ateapipb.GetActorSnapshotTagRequest{Tag: &ateapipb.ObjectRef{Atespace: "ns1"}}, + field.ErrorList{field.Required(field.NewPath("tag", "name"), "")}, + }, { + "invalid tag.name", + &ateapipb.GetActorSnapshotTagRequest{Tag: &ateapipb.ObjectRef{Atespace: "ns1", Name: "TAG1"}}, + field.ErrorList{field.Invalid(field.NewPath("tag", "name"), "TAG1", "")}, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assertValidateErr(t, validateGetActorSnapshotTagRequest(tt.req), tt.want) + }) + } +} + +func TestValidateListActorSnapshotsRequest(t *testing.T) { + tests := []struct { + name string + req *ateapipb.ListActorSnapshotsRequest + want field.ErrorList + }{{ + "valid, atespace scoped", + &ateapipb.ListActorSnapshotsRequest{Atespace: "ns1"}, + nil, + }, { + // Empty atespace means "all atespaces". + "valid, empty atespace means all atespaces", + &ateapipb.ListActorSnapshotsRequest{}, + nil, + }, { + "invalid atespace", + &ateapipb.ListActorSnapshotsRequest{Atespace: "NS1"}, + field.ErrorList{field.Invalid(field.NewPath("atespace"), "NS1", "")}, + }, { + "valid, positive page_size", + &ateapipb.ListActorSnapshotsRequest{Atespace: "ns1", PageSize: 10}, + nil, + }, { + "negative page_size", + &ateapipb.ListActorSnapshotsRequest{Atespace: "ns1", PageSize: -1}, + field.ErrorList{field.Invalid(field.NewPath("page_size"), int32(-1), "")}, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assertValidateErr(t, validateListActorSnapshotsRequest(tt.req), tt.want) + }) + } +} + +func TestValidateCreateActorSnapshotTagRequest(t *testing.T) { + scopes := []string{ + ateapipb.ActorSnapshotTagScope_ACTOR_SNAPSHOT_TAG_SCOPE_ATESPACE.String(), + ateapipb.ActorSnapshotTagScope_ACTOR_SNAPSHOT_TAG_SCOPE_PUBLISHED.String(), + } + validTag := func(mutate func(*ateapipb.ActorSnapshotTag)) *ateapipb.CreateActorSnapshotTagRequest { + tag := &ateapipb.ActorSnapshotTag{ + Metadata: &ateapipb.ResourceMetadata{Atespace: "ns1", Name: "tag1"}, + Snapshot: &ateapipb.ObjectRef{Atespace: "ns1", Name: "snap1"}, + Scope: ateapipb.ActorSnapshotTagScope_ACTOR_SNAPSHOT_TAG_SCOPE_ATESPACE, + } + if mutate != nil { + mutate(tag) + } + return &ateapipb.CreateActorSnapshotTagRequest{ActorSnapshotTag: tag} + } + + tests := []struct { + name string + req *ateapipb.CreateActorSnapshotTagRequest + want field.ErrorList + }{{ + "valid", + validTag(nil), + nil, + }, { + "missing actor_snapshot_tag", + &ateapipb.CreateActorSnapshotTagRequest{}, + field.ErrorList{field.Required(field.NewPath("actor_snapshot_tag"), "")}, + }, { + "missing metadata.atespace", + validTag(func(tag *ateapipb.ActorSnapshotTag) { tag.Metadata.Atespace = "" }), + field.ErrorList{field.Required(field.NewPath("actor_snapshot_tag", "metadata", "atespace"), "")}, + }, { + "invalid metadata.atespace", + validTag(func(tag *ateapipb.ActorSnapshotTag) { tag.Metadata.Atespace = "NS1" }), + field.ErrorList{field.Invalid(field.NewPath("actor_snapshot_tag", "metadata", "atespace"), "NS1", "")}, + }, { + "missing metadata.name", + validTag(func(tag *ateapipb.ActorSnapshotTag) { tag.Metadata.Name = "" }), + field.ErrorList{field.Required(field.NewPath("actor_snapshot_tag", "metadata", "name"), "")}, + }, { + "invalid metadata.name", + validTag(func(tag *ateapipb.ActorSnapshotTag) { tag.Metadata.Name = "TAG1" }), + field.ErrorList{field.Invalid(field.NewPath("actor_snapshot_tag", "metadata", "name"), "TAG1", "")}, + }, { + "missing snapshot", + validTag(func(tag *ateapipb.ActorSnapshotTag) { tag.Snapshot = nil }), + field.ErrorList{field.Required(field.NewPath("actor_snapshot_tag", "snapshot"), "")}, + }, { + "missing snapshot.atespace", + validTag(func(tag *ateapipb.ActorSnapshotTag) { tag.Snapshot.Atespace = "" }), + field.ErrorList{field.Required(field.NewPath("actor_snapshot_tag", "snapshot", "atespace"), "")}, + }, { + "invalid snapshot.atespace", + validTag(func(tag *ateapipb.ActorSnapshotTag) { tag.Snapshot.Atespace = "NS1" }), + field.ErrorList{field.Invalid(field.NewPath("actor_snapshot_tag", "snapshot", "atespace"), "NS1", "")}, + }, { + "missing snapshot.name", + validTag(func(tag *ateapipb.ActorSnapshotTag) { tag.Snapshot.Name = "" }), + field.ErrorList{field.Required(field.NewPath("actor_snapshot_tag", "snapshot", "name"), "")}, + }, { + "invalid snapshot.name", + validTag(func(tag *ateapipb.ActorSnapshotTag) { tag.Snapshot.Name = "SNAP1" }), + field.ErrorList{field.Invalid(field.NewPath("actor_snapshot_tag", "snapshot", "name"), "SNAP1", "")}, + }, { + "atespace mismatch", + validTag(func(tag *ateapipb.ActorSnapshotTag) { tag.Metadata.Atespace = "other" }), + field.ErrorList{field.Invalid(field.NewPath("actor_snapshot_tag", "metadata", "atespace"), "other", "")}, + }, { + "unset scope", + validTag(func(tag *ateapipb.ActorSnapshotTag) { + tag.Scope = ateapipb.ActorSnapshotTagScope_ACTOR_SNAPSHOT_TAG_SCOPE_UNSPECIFIED + }), + field.ErrorList{field.Required(field.NewPath("actor_snapshot_tag", "scope"), "")}, + }, { + "scope outside the enum", + validTag(func(tag *ateapipb.ActorSnapshotTag) { tag.Scope = ateapipb.ActorSnapshotTagScope(7) }), + field.ErrorList{field.NotSupported(field.NewPath("actor_snapshot_tag", "scope"), "7", scopes)}, + }, { + "published scope", + validTag(func(tag *ateapipb.ActorSnapshotTag) { + tag.Scope = ateapipb.ActorSnapshotTagScope_ACTOR_SNAPSHOT_TAG_SCOPE_PUBLISHED + }), + nil, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assertValidateErr(t, validateCreateActorSnapshotTagRequest(tt.req), tt.want) + }) + } +} + +func TestValidateDeleteActorSnapshotTagRequest(t *testing.T) { + tests := []struct { + name string + req *ateapipb.DeleteActorSnapshotTagRequest + want field.ErrorList + }{{ + "valid", + &ateapipb.DeleteActorSnapshotTagRequest{Tag: &ateapipb.ObjectRef{Atespace: "ns1", Name: "tag1"}}, + nil, + }, { + "missing tag", + &ateapipb.DeleteActorSnapshotTagRequest{}, + field.ErrorList{field.Required(field.NewPath("tag"), "")}, + }, { + "missing tag.atespace", + &ateapipb.DeleteActorSnapshotTagRequest{Tag: &ateapipb.ObjectRef{Name: "tag1"}}, + field.ErrorList{field.Required(field.NewPath("tag", "atespace"), "")}, + }, { + "invalid tag.atespace", + &ateapipb.DeleteActorSnapshotTagRequest{Tag: &ateapipb.ObjectRef{Atespace: "NS1", Name: "tag1"}}, + field.ErrorList{field.Invalid(field.NewPath("tag", "atespace"), "NS1", "")}, + }, { + "missing tag.name", + &ateapipb.DeleteActorSnapshotTagRequest{Tag: &ateapipb.ObjectRef{Atespace: "ns1"}}, + field.ErrorList{field.Required(field.NewPath("tag", "name"), "")}, + }, { + "invalid tag.name", + &ateapipb.DeleteActorSnapshotTagRequest{Tag: &ateapipb.ObjectRef{Atespace: "ns1", Name: "TAG1"}}, + field.ErrorList{field.Invalid(field.NewPath("tag", "name"), "TAG1", "")}, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assertValidateErr(t, validateDeleteActorSnapshotTagRequest(tt.req), tt.want) + }) + } +} + func TestValidateUpdateActorSnapshotTagRequest(t *testing.T) { mutableFields := []string{"scope"} scopes := []string{ @@ -352,6 +590,27 @@ func TestCreateActorSnapshotTag_RejectsUnsetScope(t *testing.T) { } } +// TestCreateActorSnapshotTag_RejectsAtespaceMismatch checks that a tag cannot +// be created in a different Atespace than the snapshot it points at. +func TestCreateActorSnapshotTag_RejectsAtespaceMismatch(t *testing.T) { + ctx := context.Background() + svc, stored := serviceWithActorSnapshotTag(t, &ateapipb.ActorSnapshotTag{ + Metadata: &ateapipb.ResourceMetadata{Atespace: testAtespace, Name: "tag1"}, + Scope: ateapipb.ActorSnapshotTagScope_ACTOR_SNAPSHOT_TAG_SCOPE_ATESPACE, + }) + + _, err := svc.CreateActorSnapshotTag(ctx, &ateapipb.CreateActorSnapshotTagRequest{ + ActorSnapshotTag: &ateapipb.ActorSnapshotTag{ + Metadata: &ateapipb.ResourceMetadata{Atespace: "other", Name: "tag2"}, + Snapshot: stored.GetSnapshot(), + Scope: ateapipb.ActorSnapshotTagScope_ACTOR_SNAPSHOT_TAG_SCOPE_ATESPACE, + }, + }) + if code := status.Code(err); code != codes.InvalidArgument { + t.Errorf("CreateActorSnapshotTag error = %v (code %v), want code InvalidArgument", err, code) + } +} + // serviceWithActorSnapshotTag seeds an ActorSnapshot and a tag pointing at it // in a miniredis-backed store, and returns a Service over it. func serviceWithActorSnapshotTag(t *testing.T, tag *ateapipb.ActorSnapshotTag) (*Service, *ateapipb.ActorSnapshotTag) { diff --git a/cmd/ateapi/internal/controlapi/functional_test.go b/cmd/ateapi/internal/controlapi/functional_test.go index 0087b0da1..894488b4b 100644 --- a/cmd/ateapi/internal/controlapi/functional_test.go +++ b/cmd/ateapi/internal/controlapi/functional_test.go @@ -2135,8 +2135,8 @@ func TestSuspendActor(t *testing.T) { Snapshot: snapshotRef, Scope: ateapipb.ActorSnapshotTagScope_ACTOR_SNAPSHOT_TAG_SCOPE_ATESPACE, }, - }); status.Code(err) != codes.FailedPrecondition { - t.Fatalf("cross-atespace CreateActorSnapshotTag status = %v, want FailedPrecondition", status.Code(err)) + }); status.Code(err) != codes.InvalidArgument { + t.Fatalf("cross-atespace CreateActorSnapshotTag status = %v, want InvalidArgument", status.Code(err)) } if _, err := tc.client.CreateActor(context.Background(), &ateapipb.CreateActorRequest{ Actor: &ateapipb.Actor{