Validate exec subcommands in a composite #3658

johnmcollier · 2020-07-29T22:51:14Z

What type of PR is this?
/kind bug

What does does this PR do / why we need it:
This PR fixes an issue with composite commands where exec subcommands are not validated (such as exec commands that reference an invalid component). With this PR, when we recursively validate the command-tree in a composite command, we now make sure to validate any exec commands.

Composite subcommands were already being validated.

Which issue(s) this PR fixes:

Fixes #3656

PR acceptance criteria:

Unit test
Integration test

How to test changes / Special notes to the reviewer:

Run integreation and unit tests
Test with sample devfile:

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

This devfile has a composite command whose exec subcommand points to a non-existent component. Odo will spit out the following error when you try to push: the composite command "buildandmkdir" references an invalid command "mkdir": the command does not map to a supported component

codecov · 2020-07-30T02:00:28Z

Codecov Report

Merging #3658 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3658      +/-   ##
==========================================
+ Coverage   45.49%   45.50%   +0.01%     
==========================================
  Files         122      122              
  Lines       12286    12290       +4     
==========================================
+ Hits         5589     5593       +4     
  Misses       6146     6146              
  Partials      551      551

Impacted Files	Coverage Δ
pkg/devfile/adapters/common/command.go	`96.44% <100.00%> (+0.08%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e6801f9...a6f8fd3. Read the comment docs.

mik-dass

Works for me 👍
/lgtm

kadel · 2020-07-30T11:22:01Z

/approve

openshift-ci-robot · 2020-07-30T11:22:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kadel

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/devfile/OWNERS~~ [kadel]
~~tests/OWNERS~~ [kadel]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2020-07-31T02:02:24Z

/retest