From db5a56a640c8fb7434f5b02f382364eb17fd9e7e Mon Sep 17 00:00:00 2001 From: John Collier Date: Tue, 4 Aug 2020 08:22:11 -0400 Subject: [PATCH] Validate exec subcommands in a composite (#3658) * Validate exec subcommands in composite * Add integration test --- pkg/devfile/adapters/common/command.go | 13 +++-- pkg/devfile/adapters/common/command_test.go | 29 ++++++++++- .../devfileCompositeInvalidComponent.yaml | 52 +++++++++++++++++++ .../devfile/cmd_devfile_push_test.go | 11 ++++ 4 files changed, 100 insertions(+), 5 deletions(-) create mode 100644 tests/examples/source/devfiles/nodejs/devfileCompositeInvalidComponent.yaml diff --git a/pkg/devfile/adapters/common/command.go b/pkg/devfile/adapters/common/command.go index e4bc3ebfcee..dee06fc5175 100644 --- a/pkg/devfile/adapters/common/command.go +++ b/pkg/devfile/adapters/common/command.go @@ -7,6 +7,7 @@ import ( "github.com/openshift/odo/pkg/devfile/parser/data" "github.com/openshift/odo/pkg/devfile/parser/data/common" + "github.com/pkg/errors" "k8s.io/klog" ) @@ -143,7 +144,7 @@ func validateCommand(data data.DevfileData, command common.DevfileCommand) (err if command.Composite != nil { parentCommands := make(map[string]string) commandsMap := GetCommandsMap(data.GetCommands()) - return validateCompositeCommand(command.Composite, parentCommands, commandsMap) + return validateCompositeCommand(data, command.Composite, parentCommands, commandsMap) } // component must be specified @@ -173,7 +174,7 @@ func validateCommand(data data.DevfileData, command common.DevfileCommand) (err } // validateCompositeCommand checks that the specified composite command is valid -func validateCompositeCommand(compositeCommand *common.Composite, parentCommands map[string]string, devfileCommands map[string]common.DevfileCommand) error { +func validateCompositeCommand(data data.DevfileData, compositeCommand *common.Composite, parentCommands map[string]string, devfileCommands map[string]common.DevfileCommand) error { if compositeCommand.Group != nil && compositeCommand.Group.Kind == common.RunCommandGroupType { return fmt.Errorf("composite commands of run Kind are not supported currently") } @@ -200,12 +201,16 @@ func validateCompositeCommand(compositeCommand *common.Composite, parentCommands if subCommand.Composite != nil { // Recursively validate the composite subcommand - err := validateCompositeCommand(subCommand.Composite, parentCommands, devfileCommands) + err := validateCompositeCommand(data, subCommand.Composite, parentCommands, devfileCommands) if err != nil { // Don't wrap the error message here to make the error message more readable to the user return err } - + } else { + err := validateCommand(data, subCommand) + if err != nil { + return errors.Wrapf(err, "the composite command %q references an invalid command %q", compositeCommand.Id, subCommand.GetID()) + } } } return nil diff --git a/pkg/devfile/adapters/common/command_test.go b/pkg/devfile/adapters/common/command_test.go index 5005ad59b65..fa80129aee1 100644 --- a/pkg/devfile/adapters/common/command_test.go +++ b/pkg/devfile/adapters/common/command_test.go @@ -1868,6 +1868,33 @@ func TestValidateCompositeCommand(t *testing.T) { }, wantErr: true, }, + { + name: "Case 6: Invalid composite command, points to invalid exec command", + compositeCommands: []common.Composite{ + { + Id: id[3], + Commands: []string{id[0], id[1]}, + Group: &versionsCommon.Group{Kind: buildGroup}, + }, + }, + execCommands: []common.Exec{ + { + Id: id[0], + CommandLine: command[0], + Component: component, + Group: &common.Group{Kind: runGroup}, + WorkingDir: workDir[0], + }, + { + Id: id[1], + CommandLine: command[1], + Component: "some-fake-component", + Group: &common.Group{Kind: runGroup}, + WorkingDir: workDir[1], + }, + }, + wantErr: true, + }, } for _, tt := range tests { devObj := devfileParser.DevfileObj{ @@ -1882,7 +1909,7 @@ func TestValidateCompositeCommand(t *testing.T) { commandsMap := GetCommandsMap(devObj.Data.GetCommands()) parentCommands := make(map[string]string) - err := validateCompositeCommand(cmd.Composite, parentCommands, commandsMap) + err := validateCompositeCommand(devObj.Data, cmd.Composite, parentCommands, commandsMap) if !tt.wantErr == (err != nil) { t.Errorf("TestValidateAction unexpected error: %v", err) return diff --git a/tests/examples/source/devfiles/nodejs/devfileCompositeInvalidComponent.yaml b/tests/examples/source/devfiles/nodejs/devfileCompositeInvalidComponent.yaml new file mode 100644 index 00000000000..19d9d246b7b --- /dev/null +++ b/tests/examples/source/devfiles/nodejs/devfileCompositeInvalidComponent.yaml @@ -0,0 +1,52 @@ +schemaVersion: 2.0.0 +metadata: + name: nodejs + version: 1.0.0 +projects: + - name: nodejs-starter + git: + location: "https://github.com/odo-devfiles/nodejs-ex.git" +components: + - container: + name: runtime + image: registry.access.redhat.com/ubi8/nodejs-12:1-36 + memoryLimit: 1024Mi + mountSources: true + endpoints: + - name: http-3000 + targetPort: 3000 +commands: + - exec: + id: install + component: runtime + commandLine: npm install + workingDir: ${CHE_PROJECTS_ROOT}/nodejs-starter + group: + kind: build + isDefault: false + - exec: + id: mkdir + component: fakecomponent + commandLine: mkdir /projects/testfolder + workingDir: ${CHE_PROJECTS_ROOT}/nodejs-starter + group: + kind: build + isDefault: false + - exec: + id: run + component: runtime + commandLine: npm start + workingDir: ${CHE_PROJECTS_ROOT}/nodejs-starter + group: + kind: run + isDefault: true + - composite: + id: buildAndMkdir + label: Build and Mkdir + commands: + - mkdir + - install + parallel: false + group: + kind: build + isDefault: true diff --git a/tests/integration/devfile/cmd_devfile_push_test.go b/tests/integration/devfile/cmd_devfile_push_test.go index b8f651668ca..4515698ca33 100644 --- a/tests/integration/devfile/cmd_devfile_push_test.go +++ b/tests/integration/devfile/cmd_devfile_push_test.go @@ -241,6 +241,17 @@ var _ = Describe("odo devfile push command tests", func() { Expect(output).To(ContainSubstring("cannot indirectly reference itself")) }) + It("should throw a validation error for composite command that has invalid exec subcommand", func() { + helper.CmdShouldPass("odo", "create", "nodejs", "--project", namespace, cmpName) + + helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), context) + helper.CopyExampleDevFile(filepath.Join("source", "devfiles", "nodejs", "devfileCompositeInvalidComponent.yaml"), filepath.Join(context, "devfile.yaml")) + + // Verify odo push failed + output := helper.CmdShouldFail("odo", "push", "--context", context) + Expect(output).To(ContainSubstring("references an invalid command")) + }) + It("checks that odo push works outside of the context directory", func() { helper.Chdir(currentWorkingDirectory)