Skip to content

Commit 0b86883

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 0b86883

2 files changed

Lines changed: 66 additions & 0 deletions

File tree

internal/pkg/plan/rename/plan_test.go

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

86+
// TestRenameAllPlan_StatelessObjectSkipped verifies that NewPlan does not panic
87+
// when the state contains a ConfigState with neither local nor remote data.
88+
// This reproduces the crash reported in PSGO-233 where an orphaned manifest
89+
// entry (e.g. a scheduler config whose target orchestrator was removed) caused
90+
// LocalOrRemoteState() to panic inside PathsGenerator.doUpdate.
91+
func TestRenameAllPlan_StatelessObjectSkipped(t *testing.T) {
92+
t.Parallel()
93+
_, testFile, _, _ := runtime.Caller(0)
94+
testDir := filesystem.Dir(testFile)
95+
fs := testFs(t, filesystem.Join(testDir, "..", "..", "fixtures", "local", "to-rename"))
96+
d := dependencies.NewMocked(t, t.Context())
97+
98+
getGenericExResponder, err := httpmock.NewJsonResponder(200, map[string]any{
99+
"id": "ex-generic-v2",
100+
"type": "extractor",
101+
"name": "Generic",
102+
})
103+
require.NoError(t, err)
104+
getMySQLExResponder, err := httpmock.NewJsonResponder(200, map[string]any{
105+
"id": "keboola.ex-db-mysql",
106+
"type": "extractor",
107+
"name": "MySQL",
108+
})
109+
require.NoError(t, err)
110+
d.MockedHTTPTransport().RegisterResponder("GET", `=~/storage/components/ex-generic-v2`, getGenericExResponder.Once())
111+
d.MockedHTTPTransport().RegisterResponder("GET", `=~/storage/components/keboola.ex-db-mysql`, getMySQLExResponder.Once())
112+
113+
projectState, err := d.MockedProject(fs).LoadState(t.Context(), loadState.Options{LoadLocalState: true}, d)
114+
require.NoError(t, err)
115+
116+
// Inject an orphaned ConfigState: manifest entry exists but both Local and
117+
// Remote are nil. This simulates a scheduler config referencing a deleted
118+
// orchestrator, leaving a state entry that has no actual data.
119+
orphanKey := model.ConfigKey{BranchID: 123, ComponentID: "keboola.scheduler", ID: "orphaned-01"}
120+
orphanState := &model.ConfigState{
121+
ConfigManifest: &model.ConfigManifest{
122+
ConfigKey: orphanKey,
123+
Paths: model.Paths{
124+
AbsPath: model.NewAbsPath("my-main-branch", "other/keboola.scheduler/orphaned-01"),
125+
},
126+
},
127+
}
128+
require.NoError(t, projectState.State().Set(orphanState))
129+
130+
// NewPlan must not panic and the orphaned entry must be silently excluded.
131+
plan, err := NewPlan(projectState.State())
132+
require.NoError(t, err)
133+
134+
// The orphaned config must not appear in the rename actions.
135+
for _, action := range plan.actions {
136+
assert.NotContains(t, action.OldPath, "orphaned-01")
137+
assert.NotContains(t, action.NewPath, "orphaned-01")
138+
}
139+
}
140+
86141
func testFs(t *testing.T, inputDir string) filesystem.Fs {
87142
t.Helper()
88143

internal/pkg/state/local/paths.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,12 @@ 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+
return nil
74+
}
75+
7076
// Detect cyclic relations
7177
if origin != nil && objectState.Key().String() == origin.String() {
7278
return errors.Errorf(`a cyclic relation was found when generating path to %s`, origin.Desc())
@@ -93,6 +99,11 @@ func (g *PathsGenerator) doUpdate(objectState model.ObjectState, origin model.Ke
9399
return err
94100
}
95101

102+
// Skip if the parent is stateless — its path cannot be resolved.
103+
if !parent.HasLocalState() && !parent.HasRemoteState() {
104+
return nil
105+
}
106+
96107
// Set new parent path
97108
manifest.SetParentPath(parent.Path())
98109
}

0 commit comments

Comments
 (0)