Skip to content

Commit

Permalink
Disable support for Exec-type commands in the preStart lifecycle
Browse files Browse the repository at this point in the history
As suggested by David, do not support preStart Exec-type commands in
devfiles and instead return an error.

Signed-off-by: Angel Misevski <[email protected]>
  • Loading branch information
amisevsk committed Oct 30, 2020
1 parent 7b2942a commit 690c15b
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 5 deletions.
26 changes: 26 additions & 0 deletions pkg/library/lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,14 @@
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
Expand All @@ -29,6 +34,10 @@ func GetInitContainers(devfile v1alpha1.DevWorkspaceTemplateSpecContent) (initCo
if err != nil {
return nil, nil, err
}
// Check that commands in PreStart lifecycle binding are supported
if err = checkEventCommandsValidity(initCommands); err != nil {
return nil, nil, err
}
initComponentKeys, err := commandListToComponentKeys(initCommands)
if err != nil {
return nil, nil, err
Expand Down Expand Up @@ -64,3 +73,20 @@ func GetInitContainers(devfile v1alpha1.DevWorkspaceTemplateSpecContent) (initCo

return initContainers, mainComponents, nil
}

func checkEventCommandsValidity(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.
return fmt.Errorf("only apply-type commands are supported in the prestart lifecycle binding")
}
}
return nil
}
4 changes: 2 additions & 2 deletions pkg/library/lifecycle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func loadTestCaseOrPanic(t *testing.T, testFilename string) testCase {
if err := yaml.Unmarshal(bytes, &test); err != nil {
t.Fatal(err)
}
t.Log(fmt.Printf("Read file:\n %+v \n\n", test))
t.Log(fmt.Sprintf("Read file:\n %+v \n\n", test))
return test
}

Expand All @@ -66,7 +66,7 @@ func TestGetInitContainers(t *testing.T) {
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")
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")
Expand Down
3 changes: 1 addition & 2 deletions pkg/library/testdata/lifecycle/init_and_main_container.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@ input:
name: test-container2
image: my-image
commands:
- exec:
- apply:
id: test_preStart_command
component: test-container1
command: "test_command"
- exec:
id: test_regular_command
component: test-container1
Expand Down
2 changes: 1 addition & 1 deletion pkg/library/testdata/lifecycle/prestart_exec_command.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ output:
- container:
name: test-container2
image: my-image
errRegexp:
errRegexp: "only apply-type commands are supported in the prestart lifecycle binding"

0 comments on commit 690c15b

Please sign in to comment.