Skip to content

Commit d154382

Browse files
Matovidloclaude
andcommitted
fix(rename): skip stateless objects in PathsGenerator to prevent panic
When a manifest entry exists for a config that has neither local nor remote state (e.g. an orphaned scheduler config after its target orchestrator was deleted), PathsGenerator.doUpdate called LocalOrRemoteState() which panicked unconditionally. Two guards added to doUpdate: 1. Skip the object itself when both HasLocalState and HasRemoteState are false. 2. Skip the object when its parent is stateless, since a valid path cannot be resolved without a resolved parent. Reproduces the crash in PSGO-233 / SUPPORT-15941 where kbc pull panicked on "object Local or Remote state must be set" for clients with an existing local manifest containing an orphaned scheduler entry. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent d26de9b commit d154382

2 files changed

Lines changed: 92 additions & 0 deletions

File tree

internal/pkg/plan/rename/plan_test.go

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,85 @@ func TestRenameAllPlan(t *testing.T) {
8383
}, plan)
8484
}
8585

86+
// TestRenameAllPlan_StatelessParentSkipped verifies that NewPlan does not panic
87+
// when a config with remote state has a parent that is stateless in the registry.
88+
// This reproduces the production scenario from PSGO-233: a scheduler config has
89+
// a SchedulerForRelation pointing to an orchestrator whose manifest entry is
90+
// orphaned (nil Local and nil Remote). State.All() returns the scheduler (has
91+
// remote state) but not the orchestrator, so doUpdate reaches the parent via
92+
// MustGet and must skip gracefully without calling LocalOrRemoteState().
93+
func TestRenameAllPlan_StatelessParentSkipped(t *testing.T) {
94+
t.Parallel()
95+
_, testFile, _, _ := runtime.Caller(0)
96+
testDir := filesystem.Dir(testFile)
97+
fs := testFs(t, filesystem.Join(testDir, "..", "..", "fixtures", "local", "to-rename"))
98+
d := dependencies.NewMocked(t, t.Context())
99+
100+
getGenericExResponder, err := httpmock.NewJsonResponder(200, map[string]any{
101+
"id": "ex-generic-v2",
102+
"type": "extractor",
103+
"name": "Generic",
104+
})
105+
require.NoError(t, err)
106+
getMySQLExResponder, err := httpmock.NewJsonResponder(200, map[string]any{
107+
"id": "keboola.ex-db-mysql",
108+
"type": "extractor",
109+
"name": "MySQL",
110+
})
111+
require.NoError(t, err)
112+
d.MockedHTTPTransport().RegisterResponder("GET", `=~/storage/components/ex-generic-v2`, getGenericExResponder.Once())
113+
d.MockedHTTPTransport().RegisterResponder("GET", `=~/storage/components/keboola.ex-db-mysql`, getMySQLExResponder.Once())
114+
115+
projectState, err := d.MockedProject(fs).LoadState(t.Context(), loadState.Options{LoadLocalState: true}, d)
116+
require.NoError(t, err)
117+
118+
// Stateless orchestrator: in the registry as an orphaned manifest entry, but
119+
// both Local and Remote are nil — simulates a deleted orchestrator.
120+
orchKey := model.ConfigKey{BranchID: 123, ComponentID: "keboola.orchestrator", ID: "orch-orphaned"}
121+
orchState := &model.ConfigState{
122+
ConfigManifest: &model.ConfigManifest{
123+
ConfigKey: orchKey,
124+
Paths: model.Paths{
125+
AbsPath: model.NewAbsPath("my-main-branch", "other/keboola.orchestrator/orch-orphaned"),
126+
},
127+
},
128+
}
129+
130+
// Scheduler with remote state whose SchedulerForRelation parent key points to
131+
// the stateless orchestrator. State.All() will include this scheduler (it has
132+
// remote state), so doUpdate will recurse into the stateless orchestrator parent.
133+
schedulerKey := model.ConfigKey{BranchID: 123, ComponentID: "keboola.scheduler", ID: "sched-for-orch"}
134+
schedulerState := &model.ConfigState{
135+
ConfigManifest: &model.ConfigManifest{
136+
ConfigKey: schedulerKey,
137+
Paths: model.Paths{
138+
AbsPath: model.NewAbsPath("my-main-branch", "other/keboola.scheduler/sched-for-orch"),
139+
},
140+
},
141+
Remote: &model.Config{
142+
ConfigKey: schedulerKey,
143+
Relations: model.Relations{
144+
&model.SchedulerForRelation{
145+
ComponentID: "keboola.orchestrator",
146+
ConfigID: "orch-orphaned",
147+
},
148+
},
149+
},
150+
}
151+
152+
require.NoError(t, projectState.State().Set(orchState))
153+
require.NoError(t, projectState.State().Set(schedulerState))
154+
155+
// NewPlan must not panic; neither orphaned object should appear in rename actions.
156+
plan, err := NewPlan(projectState.State())
157+
require.NoError(t, err)
158+
159+
for _, action := range plan.actions {
160+
assert.NotEqual(t, action.Manifest.Key(), orchKey)
161+
assert.NotEqual(t, action.Manifest.Key(), schedulerKey)
162+
}
163+
}
164+
86165
func testFs(t *testing.T, inputDir string) filesystem.Fs {
87166
t.Helper()
88167

internal/pkg/state/local/paths.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,13 @@ func (g *PathsGenerator) doUpdate(objectState model.ObjectState, origin model.Ke
6767
return nil
6868
}
6969

70+
// Orphaned manifest entries have no local or remote state; skip them so
71+
// LocalOrRemoteState() does not panic and they are excluded from rename.
72+
if !objectState.HasLocalState() && !objectState.HasRemoteState() {
73+
g.processed[objectState.Key().String()] = true
74+
return nil
75+
}
76+
7077
// Detect cyclic relations
7178
if origin != nil && objectState.Key().String() == origin.String() {
7279
return errors.Errorf(`a cyclic relation was found when generating path to %s`, origin.Desc())
@@ -93,6 +100,12 @@ func (g *PathsGenerator) doUpdate(objectState model.ObjectState, origin model.Ke
93100
return err
94101
}
95102

103+
// Skip if the parent is stateless — its path cannot be resolved.
104+
if !parent.HasLocalState() && !parent.HasRemoteState() {
105+
g.processed[objectState.Key().String()] = true
106+
return nil
107+
}
108+
96109
// Set new parent path
97110
manifest.SetParentPath(parent.Path())
98111
}

0 commit comments

Comments
 (0)