Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions internal/pkg/plan/rename/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,90 @@ func TestRenameAllPlan(t *testing.T) {
}, plan)
}

// TestRenameAllPlan_StatelessParentSkipped verifies that NewPlan does not panic
// when a config with remote state has a parent that is stateless in the registry.
// This reproduces the production scenario from PSGO-233: a scheduler config has
// a SchedulerForRelation pointing to an orchestrator whose manifest entry is
// orphaned (nil Local and nil Remote). State.All() returns the scheduler (has
// remote state) but not the orchestrator, so doUpdate reaches the parent via
// MustGet and must skip gracefully without calling LocalOrRemoteState().
func TestRenameAllPlan_StatelessParentSkipped(t *testing.T) {
t.Parallel()
_, testFile, _, _ := runtime.Caller(0)
testDir := filesystem.Dir(testFile)
fs := testFs(t, filesystem.Join(testDir, "..", "..", "fixtures", "local", "to-rename"))
d := dependencies.NewMocked(t, t.Context())

getGenericExResponder, err := httpmock.NewJsonResponder(200, map[string]any{
"id": "ex-generic-v2",
"type": "extractor",
"name": "Generic",
})
require.NoError(t, err)
getMySQLExResponder, err := httpmock.NewJsonResponder(200, map[string]any{
"id": "keboola.ex-db-mysql",
"type": "extractor",
"name": "MySQL",
})
require.NoError(t, err)
d.MockedHTTPTransport().RegisterResponder("GET", `=~/storage/components/ex-generic-v2`, getGenericExResponder.Once())
d.MockedHTTPTransport().RegisterResponder("GET", `=~/storage/components/keboola.ex-db-mysql`, getMySQLExResponder.Once())

projectState, err := d.MockedProject(fs).LoadState(t.Context(), loadState.Options{LoadLocalState: true}, d)
require.NoError(t, err)

// Stateless orchestrator: in the registry as an orphaned manifest entry, but
// both Local and Remote are nil — simulates a deleted orchestrator.
orchKey := model.ConfigKey{BranchID: 123, ComponentID: "keboola.orchestrator", ID: "orch-orphaned"}
orchState := &model.ConfigState{
ConfigManifest: &model.ConfigManifest{
ConfigKey: orchKey,
Paths: model.Paths{
AbsPath: model.NewAbsPath("my-main-branch", "other/keboola.orchestrator/orch-orphaned"),
},
},
}

// Scheduler with remote state whose SchedulerForRelation parent key points to
// the stateless orchestrator. State.All() will include this scheduler (it has
// remote state), so doUpdate will recurse into the stateless orchestrator parent.
schedulerKey := model.ConfigKey{BranchID: 123, ComponentID: "keboola.scheduler", ID: "sched-for-orch"}
schedulerState := &model.ConfigState{
ConfigManifest: &model.ConfigManifest{
ConfigKey: schedulerKey,
Paths: model.Paths{
AbsPath: model.NewAbsPath("my-main-branch", "other/keboola.scheduler/sched-for-orch"),
},
},
Remote: &model.Config{
ConfigKey: schedulerKey,
Relations: model.Relations{
&model.SchedulerForRelation{
ComponentID: "keboola.orchestrator",
ConfigID: "orch-orphaned",
},
},
},
}

require.NoError(t, projectState.State().Set(orchState))
require.NoError(t, projectState.State().Set(schedulerState))

// NewPlan must not panic; neither orphaned object should appear in rename actions.
plan, err := NewPlan(projectState.State())
require.NoError(t, err)

// The fixture produces 3 legitimate rename actions (branch, mysql config, mysql row).
// Asserting the count ensures the plan is not vacuously empty — an empty plan would
// make the loop below a no-op and the NotEqual assertions would never fire.
require.Len(t, plan.actions, 3, "plan must contain the fixture's legitimate rename actions")

for _, action := range plan.actions {
Comment thread
Matovidlo marked this conversation as resolved.
assert.NotEqual(t, action.Manifest.Key(), orchKey)
assert.NotEqual(t, action.Manifest.Key(), schedulerKey)
}
}

func testFs(t *testing.T, inputDir string) filesystem.Fs {
t.Helper()

Expand Down
13 changes: 13 additions & 0 deletions internal/pkg/state/local/paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ func (g *PathsGenerator) doUpdate(objectState model.ObjectState, origin model.Ke
return nil
}

// Orphaned manifest entries have no local or remote state; skip them so
// LocalOrRemoteState() does not panic and they are excluded from rename.
if !objectState.HasLocalState() && !objectState.HasRemoteState() {
g.processed[objectState.Key().String()] = true
return nil
}

Comment thread
Matovidlo marked this conversation as resolved.
// Detect cyclic relations
if origin != nil && objectState.Key().String() == origin.String() {
return errors.Errorf(`a cyclic relation was found when generating path to %s`, origin.Desc())
Expand All @@ -93,6 +100,12 @@ func (g *PathsGenerator) doUpdate(objectState model.ObjectState, origin model.Ke
return err
}

// Skip if the parent is stateless — its path cannot be resolved.
if !parent.HasLocalState() && !parent.HasRemoteState() {
Comment thread
Matovidlo marked this conversation as resolved.
g.processed[objectState.Key().String()] = true
return nil
}

// Set new parent path
manifest.SetParentPath(parent.Path())
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pull --force --storage-api-token %%TEST_KBC_STORAGE_API_TOKEN%%
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Manifest loaded with warnings (some records were skipped):
- %s
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Plan for "pull" operation:
* B main | changed: description
* C main/extractor/ex-generic-v2/wrong-name | changed: description, name
Plan for "rename" operation:
- main/extractor/ex-generic-v2/{wrong-name -> empty}
Rename done.
Pull done.
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
{
"version": 2,
"project": {
"id": %%TEST_KBC_PROJECT_ID%%,
"apiHost": "%%TEST_KBC_STORAGE_API_HOST%%"
},
"allowTargetEnv": false,
"sortBy": "path",
"naming": {
"branch": "{branch_name}",
"config": "{component_type}/{component_id}/{config_name}",
"configRow": "rows/{config_row_name}",
"schedulerConfig": "schedules/{config_name}",
"sharedCodeConfig": "_shared/{target_component_id}",
"sharedCodeConfigRow": "codes/{config_row_name}",
"variablesConfig": "variables",
"variablesValuesRow": "values/{config_row_name}",
"dataAppConfig": "app/{component_id}/{config_name}"
},
"allowedBranches": [
"__all__"
],
"ignoredComponents": [],
"templates": {
"repositories": [
{
"type": "git",
"name": "keboola",
"url": "https://github.com/keboola/keboola-as-code-templates.git",
"ref": "main"
}
]
},
"branches": [
{
"id": %%TEST_BRANCH_MAIN_ID%%,
"path": "main"
}
],
"configurations": [
{
"branchId": %%TEST_BRANCH_MAIN_ID%%,
"componentId": "keboola.scheduler",
"id": "456",
"path": "schedules/schedule1",
"relations": [
{
"componentId": "ex-generic-v2",
"configId": "123",
"type": "schedulerFor"
}
],
"rows": []
},
{
"branchId": %%TEST_BRANCH_MAIN_ID%%,
"componentId": "ex-generic-v2",
"id": "%%TEST_BRANCH_ALL_CONFIG_EMPTY_ID%%",
"path": "extractor/ex-generic-v2/wrong-name",
"rows": []
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "wrong-name"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "Main",
"isDefault": true
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"allBranchesConfigs": ["empty"],
"branches": [
{
"branch": {
"name": "Main",
"description": "my description",
"isDefault": true
}
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
{
"version": 2,
"project": {
"id": %%TEST_KBC_PROJECT_ID%%,
"apiHost": "%%TEST_KBC_STORAGE_API_HOST%%"
},
"allowTargetEnv": false,
"sortBy": "path",
"naming": {
"branch": "{branch_name}",
"config": "{component_type}/{component_id}/{config_name}",
"configRow": "rows/{config_row_name}",
"schedulerConfig": "schedules/{config_name}",
"sharedCodeConfig": "_shared/{target_component_id}",
"sharedCodeConfigRow": "codes/{config_row_name}",
"variablesConfig": "variables",
"variablesValuesRow": "values/{config_row_name}",
"dataAppConfig": "app/{component_id}/{config_name}"
},
"allowedBranches": [
"__all__"
],
"ignoredComponents": [],
"templates": {
"repositories": [
{
"type": "git",
"name": "keboola",
"url": "https://github.com/keboola/keboola-as-code-templates.git",
"ref": "main"
}
]
},
"branches": [
{
"id": %%TEST_BRANCH_MAIN_ID%%,
"path": "main"
}
],
"configurations": [
{
"branchId": %%TEST_BRANCH_MAIN_ID%%,
"componentId": "ex-generic-v2",
"id": "%%TEST_BRANCH_ALL_CONFIG_EMPTY_ID%%",
"path": "extractor/ex-generic-v2/empty",
"rows": []
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"backends": [
%A
],
"features": [
%A
],
"defaultBranchId": %A
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
my description
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test fixture
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "empty",
"isDisabled": false
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "Main",
"isDefault": true
}
Loading