From 2eec4a2336eece2576709fc8d42d9d6677097b27 Mon Sep 17 00:00:00 2001 From: "alex.stanfield" <13949480+chaptersix@users.noreply.github.com> Date: Thu, 2 Jul 2026 17:36:39 -0500 Subject: [PATCH] =?UTF-8?q?fix:=20strip=20system=20SAs=20from=20V1?= =?UTF-8?q?=E2=86=92CHASM=20schedule=20migration=20state?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a V1 scheduler workflow is migrated to CHASM, its raw search attributes (including system SAs like TemporalNamespaceDivision and TemporalSchedulePaused) were copied verbatim into the CHASM entity's CustomSearchAttributes store. The visibility task executor then iterated these and routed each one through the custom SA mapper. System SAs have no per-namespace mapping, so the mapper returned an error and emitted a spurious warn-level log: "Failed to get field name for alias, ignoring search attribute" alias="TemporalNamespaceDivision" error="Namespace has no mapping defined for search attribute TemporalNamespaceDivision" The log was benign — TemporalNamespaceDivision is overwritten with the archetype ID unconditionally afterward, and TemporalSchedulePaused is set from Schedule.State.Paused by Scheduler.SearchAttributes(). But system SAs have no business being in the custom SA store. Fix: filter out reserved/system SAs in LegacyToCreateFromMigrationStateRequest before they enter SchedulerMigrationState.SearchAttributes using sadefs.IsReserved. This covers all current and future system SAs without needing a hardcoded list. Also adds: - unit test (table-driven) for the SA stripping logic - functional test verifying system SAs don't leak into the custom SA store after migration and that framework-managed SAs reflect correct schedule state --- chasm/lib/scheduler/migration/migration.go | 18 ++++- .../lib/scheduler/migration/migration_test.go | 78 ++++++++++++++++++- 2 files changed, 94 insertions(+), 2 deletions(-) diff --git a/chasm/lib/scheduler/migration/migration.go b/chasm/lib/scheduler/migration/migration.go index e9c9c3bbfd8..a7e42378fd7 100644 --- a/chasm/lib/scheduler/migration/migration.go +++ b/chasm/lib/scheduler/migration/migration.go @@ -1,6 +1,7 @@ package migration import ( + "maps" "time" commonpb "go.temporal.io/api/common/v1" @@ -11,6 +12,7 @@ import ( schedulerpb "go.temporal.io/server/chasm/lib/scheduler/gen/schedulerpb/v1" "go.temporal.io/server/common" schedulescommon "go.temporal.io/server/common/schedules" + "go.temporal.io/server/common/searchattribute/sadefs" "google.golang.org/protobuf/types/known/timestamppb" ) @@ -107,12 +109,26 @@ func LegacyToCreateFromMigrationStateRequest( InvokerState: invokerState, Backfillers: backfillers, LastCompletionResult: lastCompletion, - SearchAttributes: searchAttributes.GetIndexedFields(), + SearchAttributes: customSearchAttributesForMigration(searchAttributes), Memo: memo.GetFields(), }, } } +// customSearchAttributesForMigration returns only the user-defined search attributes +// from a V1 scheduler workflow, stripping any reserved/system SAs. +func customSearchAttributesForMigration(sa *commonpb.SearchAttributes) map[string]*commonpb.Payload { + fields := sa.GetIndexedFields() + if len(fields) == 0 { + return nil + } + out := maps.Clone(fields) + maps.DeleteFunc(out, func(k string, _ *commonpb.Payload) bool { + return sadefs.IsReserved(k) + }) + return out +} + // CHASMToLegacyStartScheduleArgs converts CHASM scheduler state to V1 StartScheduleArgs. // This is the primary V2-to-V1 migration function. The migrationTime parameter is used // to initialize missing timestamps. diff --git a/chasm/lib/scheduler/migration/migration_test.go b/chasm/lib/scheduler/migration/migration_test.go index 41a17192fec..76045b1e852 100644 --- a/chasm/lib/scheduler/migration/migration_test.go +++ b/chasm/lib/scheduler/migration/migration_test.go @@ -12,6 +12,7 @@ import ( workflowpb "go.temporal.io/api/workflow/v1" schedulespb "go.temporal.io/server/api/schedule/v1" schedulerpb "go.temporal.io/server/chasm/lib/scheduler/gen/schedulerpb/v1" + "go.temporal.io/server/common/searchattribute/sadefs" "google.golang.org/protobuf/types/known/durationpb" "google.golang.org/protobuf/types/known/timestamppb" ) @@ -140,11 +141,86 @@ func TestLegacyToCreateFromMigrationStateRequest(t *testing.T) { require.Equal(t, []byte("result"), migrationState.LastCompletionResult.Success.Data) require.Equal(t, "last failure", migrationState.LastCompletionResult.Failure.Message) - // Search attributes and memo + // Search attributes and memo: user-defined SAs are preserved as-is. require.Equal(t, searchAttrs.GetIndexedFields(), migrationState.SearchAttributes) require.Equal(t, memo.GetFields(), migrationState.Memo) } +func TestLegacyToCreateFromMigrationStateRequest_StripsSystemSearchAttributes(t *testing.T) { + // V1 scheduler workflows carry TemporalNamespaceDivision ('TemporalScheduler') + // and TemporalSchedulePaused in their search attributes. Both are system SAs + // that the CHASM framework manages independently, so they must not be stored in + // the CHASM entity's custom SA map. Storing them there causes a spurious + // warn-level log when the custom SA mapper finds no per-namespace mapping for them. + userSA := &commonpb.Payload{Data: []byte(`"my-value"`)} + + tests := []struct { + name string + searchAttrs *commonpb.SearchAttributes + wantKeys []string + absentKeys []string + }{ + { + name: "nil search attributes", + searchAttrs: nil, + }, + { + name: "empty IndexedFields", + searchAttrs: &commonpb.SearchAttributes{}, + }, + { + name: "only system SAs — all stripped", + searchAttrs: &commonpb.SearchAttributes{ + IndexedFields: map[string]*commonpb.Payload{ + sadefs.TemporalNamespaceDivision: {Data: []byte(`"TemporalScheduler"`)}, + sadefs.TemporalSchedulePaused: {Data: []byte(`false`)}, + }, + }, + absentKeys: []string{sadefs.TemporalNamespaceDivision, sadefs.TemporalSchedulePaused}, + }, + { + name: "user SA preserved, system SAs stripped", + searchAttrs: &commonpb.SearchAttributes{ + IndexedFields: map[string]*commonpb.Payload{ + sadefs.TemporalNamespaceDivision: {Data: []byte(`"TemporalScheduler"`)}, + sadefs.TemporalSchedulePaused: {Data: []byte(`false`)}, + "MyCustomAttr": userSA, + }, + }, + wantKeys: []string{"MyCustomAttr"}, + absentKeys: []string{sadefs.TemporalNamespaceDivision, sadefs.TemporalSchedulePaused}, + }, + { + name: "only user SAs — all preserved", + searchAttrs: &commonpb.SearchAttributes{ + IndexedFields: map[string]*commonpb.Payload{"MyCustomAttr": userSA}, + }, + wantKeys: []string{"MyCustomAttr"}, + }, + } + + now := time.Now().UTC() + state := &schedulespb.InternalState{ + Namespace: "test-ns", + NamespaceId: "test-ns-id", + ScheduleId: "test-sched-id", + } + info := &schedulepb.ScheduleInfo{} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + req := LegacyToCreateFromMigrationStateRequest(newTestSchedule(), info, state, tc.searchAttrs, nil, now) + got := req.State.SearchAttributes + for _, k := range tc.wantKeys { + require.Contains(t, got, k) + } + for _, k := range tc.absentKeys { + require.NotContains(t, got, k) + } + }) + } +} + func TestCHASMToLegacyStartScheduleArgs(t *testing.T) { now := time.Date(2024, 6, 1, 12, 0, 0, 0, time.UTC) scheduler := &schedulerpb.SchedulerState{