diff --git a/pkg/utils/overriding/merging.go b/pkg/utils/overriding/merging.go index 95fbeb80f..2ce6fc60c 100644 --- a/pkg/utils/overriding/merging.go +++ b/pkg/utils/overriding/merging.go @@ -25,20 +25,31 @@ func MergeDevWorkspaceTemplateSpec( parentFlattenedContent *workspaces.DevWorkspaceTemplateSpecContent, pluginFlattenedContents ...*workspaces.DevWorkspaceTemplateSpecContent) (*workspaces.DevWorkspaceTemplateSpecContent, error) { - allContents := []*workspaces.DevWorkspaceTemplateSpecContent{parentFlattenedContent} - allContents = append(allContents, pluginFlattenedContents...) - allContents = append(allContents, mainContent) - - if err := ensureNoConflictWithParent(mainContent, parentFlattenedContent); err != nil { - return nil, err + allContents := []*workspaces.DevWorkspaceTemplateSpecContent{} + if parentFlattenedContent != nil { + allContents = append(allContents, parentFlattenedContent) + } + if len(pluginFlattenedContents) > 0 { + allContents = append(allContents, pluginFlattenedContents...) } + allContents = append(allContents, mainContent) - if err := ensureNoConflictsWithPlugins(mainContent, pluginFlattenedContents...); err != nil { - return nil, err + // Check for conflicts + if parentFlattenedContent != nil { + if err := ensureNoConflictWithParent(mainContent, parentFlattenedContent); err != nil { + return nil, err + } } - // also need to ensure no conflict between parent and plugins - if err := ensureNoConflictsWithPlugins(parentFlattenedContent, pluginFlattenedContents...); err != nil { - return nil, err + if len(pluginFlattenedContents) > 0 { + if err := ensureNoConflictsWithPlugins(mainContent, pluginFlattenedContents...); err != nil { + return nil, err + } + if parentFlattenedContent != nil { + // also need to ensure no conflict between parent and plugins + if err := ensureNoConflictsWithPlugins(parentFlattenedContent, pluginFlattenedContents...); err != nil { + return nil, err + } + } } result := workspaces.DevWorkspaceTemplateSpecContent{} diff --git a/pkg/utils/overriding/overriding_test.go b/pkg/utils/overriding/overriding_test.go index df82e794c..4af4ab6fa 100644 --- a/pkg/utils/overriding/overriding_test.go +++ b/pkg/utils/overriding/overriding_test.go @@ -304,6 +304,75 @@ func TestMerging(t *testing.T) { }) } +func TestPluginOverrides(t *testing.T) { + originalFile := "test-fixtures/patches/override-just-plugin/original.yaml" + patchFile := "test-fixtures/patches/override-just-plugin/patch.yaml" + resultFile := "test-fixtures/patches/override-just-plugin/result.yaml" + + originalDWT := workspaces.DevWorkspaceTemplateSpecContent{} + patch := workspaces.PluginOverrides{} + expectedDWT := workspaces.DevWorkspaceTemplateSpecContent{} + + readFileToStruct(t, originalFile, &originalDWT) + readFileToStruct(t, patchFile, &patch) + readFileToStruct(t, resultFile, &expectedDWT) + + gotDWT, err := OverrideDevWorkspaceTemplateSpec(&originalDWT, patch) + if assert.NoError(t, err) { + assert.Equal(t, &expectedDWT, gotDWT) + } +} + +func TestMergingOnlyPlugins(t *testing.T) { + baseFile := "test-fixtures/merges/no-parent/main.yaml" + pluginFile := "test-fixtures/merges/no-parent/plugin.yaml" + resultFile := "test-fixtures/merges/no-parent/result.yaml" + + baseDWT := workspaces.DevWorkspaceTemplateSpecContent{} + pluginDWT := workspaces.DevWorkspaceTemplateSpecContent{} + expectedDWT := workspaces.DevWorkspaceTemplateSpecContent{} + + readFileToStruct(t, baseFile, &baseDWT) + readFileToStruct(t, pluginFile, &pluginDWT) + readFileToStruct(t, resultFile, &expectedDWT) + + gotDWT, err := MergeDevWorkspaceTemplateSpec(&baseDWT, nil, &pluginDWT) + if assert.NoError(t, err) { + assert.Equal(t, &expectedDWT, gotDWT) + } +} + +func TestMergingOnlyParent(t *testing.T) { + // Reuse only plugin case since it's compatible + baseFile := "test-fixtures/merges/no-parent/main.yaml" + parentFile := "test-fixtures/merges/no-parent/plugin.yaml" + resultFile := "test-fixtures/merges/no-parent/result.yaml" + + baseDWT := workspaces.DevWorkspaceTemplateSpecContent{} + parentDWT := workspaces.DevWorkspaceTemplateSpecContent{} + expectedDWT := workspaces.DevWorkspaceTemplateSpecContent{} + + readFileToStruct(t, baseFile, &baseDWT) + readFileToStruct(t, parentFile, &parentDWT) + readFileToStruct(t, resultFile, &expectedDWT) + + gotDWT, err := MergeDevWorkspaceTemplateSpec(&baseDWT, &parentDWT) + if assert.NoError(t, err) { + assert.Equal(t, &expectedDWT, gotDWT) + } +} + +func readFileToStruct(t *testing.T, path string, into interface{}) { + bytes, err := ioutil.ReadFile(path) + if err != nil { + t.Fatalf("Failed to read test file from %s: %s", path, err.Error()) + } + err = yaml.Unmarshal(bytes, into) + if err != nil { + t.Fatalf("Failed to unmarshal file into struct: %s", err.Error()) + } +} + // Since order of error message lines is not deterministic, it's necessary to compare // in a weaker way than asserting string equality. func compareErrorMessages(t *testing.T, expected, actual string, failReason string) { diff --git a/pkg/utils/overriding/test-fixtures/merges/no-parent/main.yaml b/pkg/utils/overriding/test-fixtures/merges/no-parent/main.yaml new file mode 100644 index 000000000..244a81a8b --- /dev/null +++ b/pkg/utils/overriding/test-fixtures/merges/no-parent/main.yaml @@ -0,0 +1,4 @@ +components: + - name: test-component + container: + image: test-image diff --git a/pkg/utils/overriding/test-fixtures/merges/no-parent/plugin.yaml b/pkg/utils/overriding/test-fixtures/merges/no-parent/plugin.yaml new file mode 100644 index 000000000..235ac56af --- /dev/null +++ b/pkg/utils/overriding/test-fixtures/merges/no-parent/plugin.yaml @@ -0,0 +1,4 @@ +components: + - name: test-plugin-component + container: + image: test-plugin-image diff --git a/pkg/utils/overriding/test-fixtures/merges/no-parent/result.yaml b/pkg/utils/overriding/test-fixtures/merges/no-parent/result.yaml new file mode 100644 index 000000000..03d13f269 --- /dev/null +++ b/pkg/utils/overriding/test-fixtures/merges/no-parent/result.yaml @@ -0,0 +1,7 @@ +components: + - name: test-plugin-component + container: + image: test-plugin-image + - name: test-component + container: + image: test-image diff --git a/pkg/utils/overriding/test-fixtures/patches/override-just-plugin/original.yaml b/pkg/utils/overriding/test-fixtures/patches/override-just-plugin/original.yaml new file mode 100644 index 000000000..5dd047f0d --- /dev/null +++ b/pkg/utils/overriding/test-fixtures/patches/override-just-plugin/original.yaml @@ -0,0 +1,20 @@ +projects: + - name: test-project + git: + remotes: + origin: "https://test-data.io" + +components: + - name: my-component + container: + image: testimg + +commands: + - id: test-command + exec: + component: my-component + commandLine: "echo 'just a test'" + +events: + preStart: + - test-command diff --git a/pkg/utils/overriding/test-fixtures/patches/override-just-plugin/patch.yaml b/pkg/utils/overriding/test-fixtures/patches/override-just-plugin/patch.yaml new file mode 100644 index 000000000..ce0b6aba8 --- /dev/null +++ b/pkg/utils/overriding/test-fixtures/patches/override-just-plugin/patch.yaml @@ -0,0 +1,10 @@ +components: + - name: my-component + container: + image: overridden-image + +commands: + - id: test-command + exec: + component: my-component + commandLine: "echo 'this bit is updated'" diff --git a/pkg/utils/overriding/test-fixtures/patches/override-just-plugin/result.yaml b/pkg/utils/overriding/test-fixtures/patches/override-just-plugin/result.yaml new file mode 100644 index 000000000..786ade288 --- /dev/null +++ b/pkg/utils/overriding/test-fixtures/patches/override-just-plugin/result.yaml @@ -0,0 +1,20 @@ +projects: + - name: test-project + git: + remotes: + origin: "https://test-data.io" + +components: + - name: my-component + container: + image: overridden-image + +commands: + - id: test-command + exec: + component: my-component + commandLine: "echo 'this bit is updated'" + +events: + preStart: + - test-command