diff --git a/controllers/workspace/restapis/configmap.go b/controllers/workspace/restapis/configmap.go index 052994cb5..2f2da51bd 100644 --- a/controllers/workspace/restapis/configmap.go +++ b/controllers/workspace/restapis/configmap.go @@ -23,7 +23,6 @@ import ( "github.com/devfile/devworkspace-operator/pkg/config" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "gopkg.in/yaml.v2" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -31,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/types" runtimeClient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/yaml" ) var configmapDiffOpts = cmp.Options{ diff --git a/go.mod b/go.mod index e910b65ba..0b1fbfcf4 100644 --- a/go.mod +++ b/go.mod @@ -14,11 +14,12 @@ require ( github.com/onsi/gomega v1.10.1 github.com/openshift/api v0.0.0-20200205133042-34f0ec8dab87 github.com/operator-framework/operator-sdk v0.17.0 - gopkg.in/yaml.v2 v2.3.0 + github.com/stretchr/testify v1.6.1 k8s.io/api v0.18.6 k8s.io/apimachinery v0.18.6 k8s.io/client-go v12.0.0+incompatible sigs.k8s.io/controller-runtime v0.6.2 + sigs.k8s.io/yaml v1.2.0 ) // devfile/api requires v12.0.0+incompatible but this causes issues with go commands diff --git a/go.sum b/go.sum index 8e284a41a..a78b5980e 100644 --- a/go.sum +++ b/go.sum @@ -246,6 +246,7 @@ github.com/fsouza/fake-gcs-server v1.7.0/go.mod h1:5XIRs4YvwNbNoz+1JF8j6KLAyDh7R github.com/garyburd/redigo v0.0.0-20150301180006-535138d7bcd7/go.mod h1:NR3MbYisc3/PwhQ00EMzDiPmrwpPxAn5GI05/YaO1SY= github.com/ghodss/yaml v0.0.0-20150909031657-73d445a93680/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= +github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32 h1:Mn26/9ZMNWSw9C9ERFA1PUxfmGpolnw2v0bKOREu5ew= github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32/go.mod h1:GIjDIg/heH5DOkXY3YJ/wNhfHsQHoXGjl8G8amsYQ1I= github.com/globalsign/mgo v0.0.0-20180905125535-1ca0a4f7cbcb/go.mod h1:xkRDCp4j0OGD1HRkm4kmhM+pmpv3AKq5SU7GMg4oO/Q= github.com/globalsign/mgo v0.0.0-20181015135952-eeefdecb41b8/go.mod h1:xkRDCp4j0OGD1HRkm4kmhM+pmpv3AKq5SU7GMg4oO/Q= diff --git a/pkg/config/cmd_terminal.go b/pkg/config/cmd_terminal.go index 732c9be17..f0cbf3483 100644 --- a/pkg/config/cmd_terminal.go +++ b/pkg/config/cmd_terminal.go @@ -19,7 +19,7 @@ import ( devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha1" - "gopkg.in/yaml.v2" + "sigs.k8s.io/yaml" ) const ( diff --git a/pkg/internal_registry/registry.go b/pkg/internal_registry/registry.go index 0bf750d4e..6222c5805 100644 --- a/pkg/internal_registry/registry.go +++ b/pkg/internal_registry/registry.go @@ -21,8 +21,8 @@ import ( "github.com/eclipse/che-plugin-broker/model" brokerModel "github.com/eclipse/che-plugin-broker/model" - "gopkg.in/yaml.v2" logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/yaml" ) const ( diff --git a/pkg/library/command.go b/pkg/library/command.go new file mode 100644 index 000000000..785a2c084 --- /dev/null +++ b/pkg/library/command.go @@ -0,0 +1,89 @@ +// +// Copyright (c) 2019-2020 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package library + +import ( + "fmt" + + "github.com/devfile/api/pkg/apis/workspaces/v1alpha1" +) + +func getCommandType(command v1alpha1.Command) (v1alpha1.CommandType, error) { + err := command.Normalize() + if err != nil { + return "", err + } + return command.CommandType, nil +} + +func getCommandsForKeys(key []string, commands []v1alpha1.Command) ([]v1alpha1.Command, error) { + var resolvedCommands []v1alpha1.Command + + for _, id := range key { + resolvedCommand, err := getCommandByKey(id, commands) + if err != nil { + return nil, err + } + resolvedCommands = append(resolvedCommands, *resolvedCommand) + } + + return resolvedCommands, nil +} + +func getCommandByKey(key string, commands []v1alpha1.Command) (*v1alpha1.Command, error) { + for _, command := range commands { + commandKey, err := command.Key() + if err != nil { + return nil, err + } + if commandKey == key { + return &command, nil + } + } + return nil, fmt.Errorf("no command with ID %s is defined", key) +} + +func commandListToComponentKeys(commands []v1alpha1.Command) (map[string]bool, error) { + componentKeys := map[string]bool{} + for _, command := range commands { + commandType, err := getCommandType(command) + if err != nil { + return nil, err + } + switch commandType { + case v1alpha1.ApplyCommandType: + componentKeys[command.Apply.Component] = true + case v1alpha1.ExecCommandType: + // TODO: This will require special handling (how do we handle prestart exec?) + componentKeys[command.Exec.Component] = true + case v1alpha1.CompositeCommandType: + // TODO: Handle composite commands: what if an init command is composite and refers to other commands + default: // Ignore + } + } + return componentKeys, nil +} + +func removeCommandsByKeys(keys []string, commands []v1alpha1.Command) ([]v1alpha1.Command, error) { + var filtered []v1alpha1.Command + for _, command := range commands { + key, err := command.Key() + if err != nil { + return nil, err + } + if !listContains(key, keys) { + filtered = append(filtered, command) + } + } + return filtered, nil +} diff --git a/pkg/library/common.go b/pkg/library/common.go new file mode 100644 index 000000000..435995070 --- /dev/null +++ b/pkg/library/common.go @@ -0,0 +1,22 @@ +// +// Copyright (c) 2019-2020 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package library + +func listContains(query string, list []string) bool { + for _, elem := range list { + if query == elem { + return true + } + } + return false +} diff --git a/pkg/library/lifecycle.go b/pkg/library/lifecycle.go new file mode 100644 index 000000000..164d3da06 --- /dev/null +++ b/pkg/library/lifecycle.go @@ -0,0 +1,93 @@ +// +// Copyright (c) 2019-2020 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package library + +import ( + "fmt" + + "github.com/devfile/api/pkg/apis/workspaces/v1alpha1" +) + +// GetInitContainers partitions the components in a devfile's flattened spec into initContainer and non-initContainer lists +// based off devfile lifecycle bindings and commands. Note that a component can appear in both lists, if e.g. it referred to +// in a preStart command and in a regular command. +func GetInitContainers(devfile v1alpha1.DevWorkspaceTemplateSpecContent) (initContainers, mainComponents []v1alpha1.Component, err error) { + components := devfile.Components + commands := devfile.Commands + events := devfile.Events + if events == nil || commands == nil { + // All components should be run in the main deployment + return nil, components, nil + } + + initCommands, err := getCommandsForKeys(events.PreStart, commands) + if err != nil { + return nil, nil, err + } + // Check that commands in PreStart lifecycle binding are supported + if err = checkPreStartEventCommandsValidity(initCommands); err != nil { + return nil, nil, err + } + initComponentKeys, err := commandListToComponentKeys(initCommands) + if err != nil { + return nil, nil, err + } + + // Need to also consider components that are *both* init containers and in the main deployment + // Example: component is referenced in both a prestart event and a regular, non-prestart command + // TODO: Figure out details of handling postStop commands, since they should not be included in main deployment + nonInitCommands, err := removeCommandsByKeys(events.PreStart, commands) + if err != nil { + return nil, nil, err + } + mainComponentKeys, err := commandListToComponentKeys(nonInitCommands) + if err != nil { + return nil, nil, err + } + + for _, component := range components { + componentID, err := component.Key() + if err != nil { + return nil, nil, err + } + if initComponentKeys[componentID] { + initContainers = append(initContainers, component) + if mainComponentKeys[componentID] { + // Component is *also* a main component. + mainComponents = append(mainComponents, component) + } + } else { + mainComponents = append(mainComponents, component) + } + } + + return initContainers, mainComponents, nil +} + +func checkPreStartEventCommandsValidity(initCommands []v1alpha1.Command) error { + for _, cmd := range initCommands { + commandType, err := getCommandType(cmd) + if err != nil { + return err + } + switch commandType { + case v1alpha1.ApplyCommandType: + continue + default: + // How a prestart exec command should be implemented is undefined currently, so we reject it. + // Other types of commands cannot be included in the preStart event hook. + return fmt.Errorf("only apply-type commands are supported in the prestart lifecycle binding") + } + } + return nil +} diff --git a/pkg/library/lifecycle_test.go b/pkg/library/lifecycle_test.go new file mode 100644 index 000000000..060348587 --- /dev/null +++ b/pkg/library/lifecycle_test.go @@ -0,0 +1,73 @@ +// +// Copyright (c) 2019-2020 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package library + +import ( + "fmt" + "io/ioutil" + "path/filepath" + "testing" + + "github.com/devfile/api/pkg/apis/workspaces/v1alpha1" + "github.com/stretchr/testify/assert" + "sigs.k8s.io/yaml" +) + +type testCase struct { + Name string `json:"name,omitempty"` + Input v1alpha1.DevWorkspaceTemplateSpecContent `json:"input,omitempty"` + Output testOutput `json:"output,omitempty"` +} + +type testOutput struct { + InitContainers []v1alpha1.Component `json:"initContainers,omitempty"` + MainContainers []v1alpha1.Component `json:"mainContainers,omitempty"` + ErrRegexp *string `json:"errRegexp,omitempty"` +} + +func loadTestCaseOrPanic(t *testing.T, testFilename string) testCase { + testPath := filepath.Join("./testdata/lifecycle", testFilename) + bytes, err := ioutil.ReadFile(testPath) + if err != nil { + t.Fatal(err) + } + var test testCase + if err := yaml.Unmarshal(bytes, &test); err != nil { + t.Fatal(err) + } + t.Log(fmt.Sprintf("Read file:\n%+v\n\n", test)) + return test +} + +func TestGetInitContainers(t *testing.T) { + tests := []testCase{ + loadTestCaseOrPanic(t, "no_events.yaml"), + loadTestCaseOrPanic(t, "prestart_exec_command.yaml"), + loadTestCaseOrPanic(t, "prestart_apply_command.yaml"), + loadTestCaseOrPanic(t, "init_and_main_container.yaml"), + } + + for _, tt := range tests { + t.Run(tt.Name, func(t *testing.T) { + // sanity check that file reads correctly. + assert.True(t, len(tt.Input.Components) > 0, "Input defines no components") + gotInitContainers, gotMainComponents, err := GetInitContainers(tt.Input) + if tt.Output.ErrRegexp != nil && assert.Error(t, err) { + assert.Regexp(t, *tt.Output.ErrRegexp, err.Error(), "Error message should match") + } else { + assert.Equal(t, tt.Output.InitContainers, gotInitContainers, "Init containers should match expected") + assert.Equal(t, tt.Output.MainContainers, gotMainComponents, "Main containers should match expected") + } + }) + } +} diff --git a/pkg/library/testdata/lifecycle/init_and_main_container.yaml b/pkg/library/testdata/lifecycle/init_and_main_container.yaml new file mode 100644 index 000000000..6e8936117 --- /dev/null +++ b/pkg/library/testdata/lifecycle/init_and_main_container.yaml @@ -0,0 +1,35 @@ +name: "Should use container as both init and main when multiple commands apply" + +input: + components: + - container: + name: test-container1 + image: my-image + - container: + name: test-container2 + image: my-image + commands: + - apply: + id: test_preStart_command + component: test-container1 + - exec: + id: test_regular_command + component: test-container1 + command: "test_command" + events: + preStart: + - "test_preStart_command" + +output: + initContainers: + - container: + name: test-container1 + image: my-image + mainContainers: + - container: + name: test-container1 + image: my-image + - container: + name: test-container2 + image: my-image + errRegexp: diff --git a/pkg/library/testdata/lifecycle/no_events.yaml b/pkg/library/testdata/lifecycle/no_events.yaml new file mode 100644 index 000000000..fa2c07a67 --- /dev/null +++ b/pkg/library/testdata/lifecycle/no_events.yaml @@ -0,0 +1,25 @@ +name: "Should return all components when devfile contains no events" + +input: + components: + - container: + name: test-container1 + image: my-image + - container: + name: test-container2 + image: my-image + commands: + - exec: + component: test-container1 + command: "test_command" + +output: + initContainers: + mainContainers: + - container: + name: test-container1 + image: my-image + - container: + name: test-container2 + image: my-image + errRegexp: diff --git a/pkg/library/testdata/lifecycle/prestart_apply_command.yaml b/pkg/library/testdata/lifecycle/prestart_apply_command.yaml new file mode 100644 index 000000000..5465056f3 --- /dev/null +++ b/pkg/library/testdata/lifecycle/prestart_apply_command.yaml @@ -0,0 +1,28 @@ +name: "Should return init container with prestart apply command" + +input: + components: + - container: + name: test-container1 + image: my-image + - container: + name: test-container2 + image: my-image + commands: + - apply: + id: test_apply_command + component: test-container1 + events: + preStart: + - "test_apply_command" + +output: + initContainers: + - container: + name: test-container1 + image: my-image + mainContainers: + - container: + name: test-container2 + image: my-image + errRegexp: diff --git a/pkg/library/testdata/lifecycle/prestart_exec_command.yaml b/pkg/library/testdata/lifecycle/prestart_exec_command.yaml new file mode 100644 index 000000000..71aeaa5f8 --- /dev/null +++ b/pkg/library/testdata/lifecycle/prestart_exec_command.yaml @@ -0,0 +1,21 @@ +name: "Should return init container with prestart exec command" + +input: + components: + - container: + name: test-container1 + image: my-image + - container: + name: test-container2 + image: my-image + commands: + - exec: + id: test_command + component: test-container1 + command: "test_command" + events: + preStart: + - "test_command" + +output: + errRegexp: "only apply-type commands are supported in the prestart lifecycle binding"