Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Support for Devfile V2 #3216

Merged
merged 21 commits into from
Jun 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
211 changes: 211 additions & 0 deletions docs/proposals/event-notification.md

Large diffs are not rendered by default.

192 changes: 104 additions & 88 deletions pkg/devfile/adapters/common/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,140 +7,145 @@ import (
"github.com/openshift/odo/pkg/devfile/parser/data"
"github.com/openshift/odo/pkg/devfile/parser/data/common"
"k8s.io/klog"

"github.com/pkg/errors"
)

// GetCommand iterates through the devfile commands and returns the associated devfile command
func getCommand(data data.DevfileData, commandName string, required bool) (supportedCommand common.DevfileCommand, err error) {
for _, command := range data.GetCommands() {
if command.Name == commandName {

// Get the supported actions
supportedCommandActions, err := getSupportedCommandActions(data, command)

// None of the actions are supported so the command cannot be run
if len(supportedCommandActions) == 0 {
return supportedCommand, errors.Wrapf(err, "\nThe command \"%v\" was found but its actions are not supported", commandName)
} else if err != nil {
klog.Warning(errors.Wrapf(err, "The command \"%v\" was found but some of its actions are not supported", commandName))
func getCommand(data data.DevfileData, commandName string, groupType common.DevfileCommandGroupType) (supportedCommand common.DevfileCommand, err error) {

commands := data.GetCommands()

for _, command := range commands {

// validate command
err = validateCommand(data, command)

if err != nil {
return common.DevfileCommand{}, err
}

// if command is specified via flags, it has the highest priority
// search through all commands to find the specified command name
// if not found fallback to error.
if commandName != "" {

// Update Group only custom commands (specified by odo flags)
command = updateGroupforCommand(groupType, command)

if command.Exec.Id == commandName {

// we have found the command with name, its groupType Should match to the flag
// e.g --build-command "mybuild"
// exec:
// id: mybuild
// group:
// kind: build
if command.Exec.Group.Kind != groupType {
return supportedCommand, fmt.Errorf("command group mismatched, command %s is of group %v in devfile.yaml", commandName, command.Exec.Group.Kind)
}
supportedCommand = command
return supportedCommand, nil
}
continue
}

// The command is supported, use it
supportedCommand.Name = command.Name
supportedCommand.Actions = supportedCommandActions
supportedCommand.Attributes = command.Attributes
// if no command specified via flag, default command has the highest priority
// We need to scan all the commands to find default command
// exec.Group is a pointer, to avoid null pointer
if command.Exec.Group != nil && command.Exec.Group.Kind == groupType && command.Exec.Group.IsDefault {
supportedCommand = command
return supportedCommand, nil
}
}

// The command was not found
msg := fmt.Sprintf("The command \"%v\" was not found in the devfile", commandName)
if required {
// Not found and required, return an error
err = fmt.Errorf(msg)
} else {
// Not found and optional, so just log it
klog.V(3).Info(msg)
}
if commandName == "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you convert this to a switch case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure about this, would try to do that

// if default command is not found return the first command found for the matching type.
for _, command := range commands {

return
}

// getSupportedCommandActions returns the supported actions for a given command and any errors
// If some actions are supported and others have errors both the supported actions and an aggregated error will be returned.
func getSupportedCommandActions(data data.DevfileData, command common.DevfileCommand) (supportedCommandActions []common.DevfileCommandAction, err error) {
klog.V(3).Infof("Validating actions for command: %v ", command.Name)

problemMsg := ""
for i, action := range command.Actions {
// Check if the command action is of type exec
err := validateAction(data, action)
if err == nil {
klog.V(3).Infof("Action %d maps to component %v", i+1, *action.Component)
supportedCommandActions = append(supportedCommandActions, action)
} else {
problemMsg += fmt.Sprintf("Problem with command \"%v\" action #%d: %v", command.Name, i+1, err)
if command.Exec.Group != nil && command.Exec.Group.Kind == groupType {
supportedCommand = command
return supportedCommand, nil
}
}
}

if len(problemMsg) > 0 {
err = fmt.Errorf(problemMsg)
// if any command specified via flag is not found in devfile then it is an error.
if commandName != "" {
err = fmt.Errorf("the command \"%v\" is not found in the devfile", commandName)
} else {
msg := fmt.Sprintf("the command type \"%v\" is not found in the devfile", groupType)
// if run command is not found in devfile then it is an error
if groupType == common.RunCommandGroupType {
err = fmt.Errorf(msg)
} else {
klog.V(4).Info(msg)
}
}

return
}

// validateAction validates the given action
// 1. action has to be of type exec
// validateCommand validates the given command
// 1. command has to be of type exec
// 2. component should be present
// 3. command should be present
func validateAction(data data.DevfileData, action common.DevfileCommandAction) (err error) {
// 4. command must have group
func validateCommand(data data.DevfileData, command common.DevfileCommand) (err error) {

// type must be exec
if *action.Type != common.DevfileCommandTypeExec {
return fmt.Errorf("Actions must be of type \"exec\"")
if command.Exec == nil {
return fmt.Errorf("command must be of type \"exec\"")
}

// component must be specified
if action.Component == nil || *action.Component == "" {
return fmt.Errorf("Actions must reference a component")
if command.Exec.Component == "" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we should handle this as part of the devfile validator, rather than here. Then it would save a bunch of time if the user was using a devfile with invalid commands

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is the other devfile validator invoked? Because if it is validated before push, a user can change the devfile and push would have to work with probably an invalid devfile. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnmcollier I also think validation should happen after parsing, but refactoring validation logic is not part of this PR :)
you can raise new issue for that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adisky Fair enough 😄

return fmt.Errorf("exec commands must reference a component")
}

// must specify a command
if action.Command == nil || *action.Command == "" {
return fmt.Errorf("Actions must have a command")
if command.Exec.CommandLine == "" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as before, wonder if we should move this into the devfile validator.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replied above

return fmt.Errorf("exec commands must have a command")
}

// must map to a supported component
components := GetSupportedComponents(data)

isActionValid := false
isComponentValid := false
for _, component := range components {
if *action.Component == *component.Alias && isComponentSupported(component) {
isActionValid = true
if command.Exec.Component == component.Container.Name {
isComponentValid = true
}
}
if !isActionValid {
return fmt.Errorf("The action does not map to a supported component")
if !isComponentValid {
return fmt.Errorf("the command does not map to a supported component")
}

return
}

// GetInitCommand iterates through the components in the devfile and returns the init command
func GetInitCommand(data data.DevfileData, devfileInitCmd string) (initCommand common.DevfileCommand, err error) {
if devfileInitCmd != "" {
// a init command was specified so if it is not found then it is an error
return getCommand(data, devfileInitCmd, true)
}
// a init command was not specified so if it is not found then it is not an error
return getCommand(data, string(DefaultDevfileInitCommand), false)

return getCommand(data, devfileInitCmd, common.InitCommandGroupType)
}

// GetBuildCommand iterates through the components in the devfile and returns the build command
func GetBuildCommand(data data.DevfileData, devfileBuildCmd string) (buildCommand common.DevfileCommand, err error) {
if devfileBuildCmd != "" {
// a build command was specified so if it is not found then it is an error
return getCommand(data, devfileBuildCmd, true)
}
// a build command was not specified so if it is not found then it is not an error
return getCommand(data, string(DefaultDevfileBuildCommand), false)

return getCommand(data, devfileBuildCmd, common.BuildCommandGroupType)
}

// GetRunCommand iterates through the components in the devfile and returns the run command
func GetRunCommand(data data.DevfileData, devfileRunCmd string) (runCommand common.DevfileCommand, err error) {
if devfileRunCmd != "" {
return getCommand(data, devfileRunCmd, true)
}
return getCommand(data, string(DefaultDevfileRunCommand), true)

return getCommand(data, devfileRunCmd, common.RunCommandGroupType)
}

// ValidateAndGetPushDevfileCommands validates the build and the run command,
// if provided through odo push or else checks the devfile for devBuild and devRun.
// It returns the build and run commands if its validated successfully, error otherwise.
func ValidateAndGetPushDevfileCommands(data data.DevfileData, devfileInitCmd, devfileBuildCmd, devfileRunCmd string) (pushDevfileCommands []common.DevfileCommand, err error) {
func ValidateAndGetPushDevfileCommands(data data.DevfileData, devfileInitCmd, devfileBuildCmd, devfileRunCmd string) (commandMap PushCommandsMap, err error) {
var emptyCommand common.DevfileCommand
commandMap = NewPushCommandMap()

isInitCommandValid, isBuildCommandValid, isRunCommandValid := false, false, false

initCommand, initCmdErr := GetInitCommand(data, devfileInitCmd)
Expand All @@ -149,11 +154,11 @@ func ValidateAndGetPushDevfileCommands(data data.DevfileData, devfileInitCmd, de
if isInitCmdEmpty && initCmdErr == nil {
// If there was no init command specified through odo push and no default init command in the devfile, default validate to true since the init command is optional
isInitCommandValid = true
klog.V(3).Infof("No init command was provided")
klog.V(4).Infof("No init command was provided")
} else if !isInitCmdEmpty && initCmdErr == nil {
isInitCommandValid = true
pushDevfileCommands = append(pushDevfileCommands, initCommand)
klog.V(3).Infof("Init command: %v", initCommand.Name)
commandMap[common.InitCommandGroupType] = initCommand
klog.V(4).Infof("Init command: %v", initCommand.Exec.Id)
}

buildCommand, buildCmdErr := GetBuildCommand(data, devfileBuildCmd)
Expand All @@ -162,18 +167,18 @@ func ValidateAndGetPushDevfileCommands(data data.DevfileData, devfileInitCmd, de
if isBuildCmdEmpty && buildCmdErr == nil {
// If there was no build command specified through odo push and no default build command in the devfile, default validate to true since the build command is optional
isBuildCommandValid = true
klog.V(3).Infof("No build command was provided")
klog.V(4).Infof("No build command was provided")
} else if !reflect.DeepEqual(emptyCommand, buildCommand) && buildCmdErr == nil {
isBuildCommandValid = true
pushDevfileCommands = append(pushDevfileCommands, buildCommand)
klog.V(3).Infof("Build command: %v", buildCommand.Name)
commandMap[common.BuildCommandGroupType] = buildCommand
klog.V(4).Infof("Build command: %v", buildCommand.Exec.Id)
}

runCommand, runCmdErr := GetRunCommand(data, devfileRunCmd)
if runCmdErr == nil && !reflect.DeepEqual(emptyCommand, runCommand) {
pushDevfileCommands = append(pushDevfileCommands, runCommand)
isRunCommandValid = true
klog.V(3).Infof("Run command: %v", runCommand.Name)
commandMap[common.RunCommandGroupType] = runCommand
klog.V(4).Infof("Run command: %v", runCommand.Exec.Id)
}

// If either command had a problem, return an empty list of commands and an error
Expand All @@ -188,8 +193,19 @@ func ValidateAndGetPushDevfileCommands(data data.DevfileData, devfileInitCmd, de
if runCmdErr != nil {
commandErrors += fmt.Sprintf(runCmdErr.Error(), "\n")
}
return []common.DevfileCommand{}, fmt.Errorf(commandErrors)
return commandMap, fmt.Errorf(commandErrors)
}

return pushDevfileCommands, nil
return commandMap, nil
}

// Need to update group on custom commands specified by odo flags
func updateGroupforCommand(groupType common.DevfileCommandGroupType, command common.DevfileCommand) common.DevfileCommand {
// Update Group only for exec commands
// Update Group only when Group is not nil, devfile v2 might contain group for custom commands.
if command.Exec != nil && command.Exec.Group == nil {
command.Exec.Group = &common.Group{Kind: groupType}
return command
}
return command
}
Loading