Skip to content

Commit

Permalink
use string join to join & split errors
Browse files Browse the repository at this point in the history
Signed-off-by: Stephanie <yangcao@redhat.com>
  • Loading branch information
yangcao77 committed May 6, 2021
1 parent 7dc0398 commit 7cc978e
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 42 deletions.
17 changes: 10 additions & 7 deletions pkg/validation/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,16 @@ func ValidateCommands(commands []v1alpha2.Command, components []v1alpha2.Compone
}
}

groupErrors := ""
var groupErrorsList []string
for groupKind, commands := range groupKindCommandMap {
if err = validateGroup(commands); err != nil {
groupErrors += fmt.Sprintf("\ncommand group %s error - %s", groupKind, err.Error())
groupErrorsList = append(groupErrorsList, fmt.Sprintf("command group %s error - %s", groupKind, err.Error()))
}
}

if len(groupErrors) > 0 {
err = fmt.Errorf("%s", groupErrors)
if len(groupErrorsList) > 0 {
groupErrors := strings.Join(groupErrorsList, "\n")
err = fmt.Errorf("\n%s", groupErrors)
}

return err
Expand Down Expand Up @@ -83,13 +84,15 @@ func validateGroup(commands []v1alpha2.Command) error {
if defaultCommandCount == 0 {
return fmt.Errorf("there should be exactly one default command, currently there is no default command")
} else if defaultCommandCount > 1 {
var commandsReference string
var commandsReferenceList []string
for _, command := range defaultCommands {
commandsReference += resolveErrorMessageWithImportAttributes(fmt.Errorf("; command: %s", command.Id), command.Attributes).Error()
commandsReferenceList = append(commandsReferenceList,
resolveErrorMessageWithImportAttributes(fmt.Errorf("command: %s", command.Id), command.Attributes).Error())
}
commandsReference := strings.Join(commandsReferenceList, "; ")
// example: there should be exactly one default command, currently there is more than one default command;
// command: <id1>; command: <id2>, imported from uri: http://127.0.0.1:8080, in parent overrides from main devfile"
return fmt.Errorf("there should be exactly one default command, currently there is more than one default command%s", commandsReference)
return fmt.Errorf("there should be exactly one default command, currently there is more than one default command; %s", commandsReference)
}

return nil
Expand Down
10 changes: 6 additions & 4 deletions pkg/validation/components.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package validation

import (
"fmt"
"strings"

"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"k8s.io/apimachinery/pkg/api/resource"
Expand Down Expand Up @@ -102,18 +103,19 @@ func ValidateComponents(components []v1alpha2.Component) error {
}

// Check if the volume mounts mentioned in the containers are referenced by a volume component
var invalidVolumeMountsErr string
var invalidVolumeMountsErrList []string
for componentName, volumeMountNames := range processedVolumeMounts {
for _, volumeMountName := range volumeMountNames {
if !processedVolumes[volumeMountName] {
missingVolumeMountErr := fmt.Errorf("\nvolume mount %s belonging to the container component %s", volumeMountName, componentName)
missingVolumeMountErr := fmt.Errorf("volume mount %s belonging to the container component %s", volumeMountName, componentName)
newErr := resolveErrorMessageWithImportAttributes(missingVolumeMountErr, processedComponentWithVolumeMounts[componentName].Attributes)
invalidVolumeMountsErr += newErr.Error()
invalidVolumeMountsErrList = append(invalidVolumeMountsErrList, newErr.Error())
}
}
}

if len(invalidVolumeMountsErr) > 0 {
if len(invalidVolumeMountsErrList) > 0 {
invalidVolumeMountsErr := fmt.Sprintf("\n%s", strings.Join(invalidVolumeMountsErrList, "\n"))
return &MissingVolumeMountError{errMsg: invalidVolumeMountsErr}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/validation/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type InvalidCommandTypeError struct {
}

func (e *InvalidCommandTypeError) Error() string {
return fmt.Sprintf("command %s type is invalid", e.commandId)
return fmt.Sprintf("command %s has invalid type", e.commandId)
}

// ReservedEnvError returns an error if the user attempts to customize a reserved ENV in a container
Expand Down
25 changes: 13 additions & 12 deletions pkg/validation/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,34 +17,34 @@ const (
// ValidateEvents validates all the devfile events
func ValidateEvents(events v1alpha2.Events, commands []v1alpha2.Command) error {

eventErrors := ""

commandMap := getCommandsMap(commands)
var eventErrorsList []string

switch {
case len(events.PreStart) > 0:
if preStartErr := isEventValid(events.PreStart, preStart, commandMap); preStartErr != nil {
eventErrors += fmt.Sprintf("\n%s", preStartErr.Error())
eventErrorsList = append(eventErrorsList, preStartErr.Error())
}
fallthrough
case len(events.PostStart) > 0:
if postStartErr := isEventValid(events.PostStart, postStart, commandMap); postStartErr != nil {
eventErrors += fmt.Sprintf("\n%s", postStartErr.Error())
eventErrorsList = append(eventErrorsList, postStartErr.Error())
}
fallthrough
case len(events.PreStop) > 0:
if preStopErr := isEventValid(events.PreStop, preStop, commandMap); preStopErr != nil {
eventErrors += fmt.Sprintf("\n%s", preStopErr.Error())
eventErrorsList = append(eventErrorsList, preStopErr.Error())
}
fallthrough
case len(events.PostStop) > 0:
if postStopErr := isEventValid(events.PostStop, postStop, commandMap); postStopErr != nil {
eventErrors += fmt.Sprintf("\n%s", postStopErr.Error())
eventErrorsList = append(eventErrorsList, postStopErr.Error())
}
}

// if there is any validation error, return it
if len(eventErrors) > 0 {
if len(eventErrorsList) > 0 {
eventErrors := fmt.Sprintf("\n%s", strings.Join(eventErrorsList, "\n"))
return fmt.Errorf("devfile events validation error: %s", eventErrors)
}

Expand Down Expand Up @@ -83,22 +83,23 @@ func isEventValid(eventNames []string, eventType string, commandMap map[string]v
}
}

var eventErrors string
var err error
var eventErrorsList []string

if len(invalidCommand) > 0 {
eventErrors = fmt.Sprintf("\n%s does not map to a valid devfile command", strings.Join(invalidCommand, ", "))
eventErrorsList = append(eventErrorsList, fmt.Sprintf("%s does not map to a valid devfile command", strings.Join(invalidCommand, ", ")))
}

if len(invalidApplyEvents) > 0 {
eventErrors += fmt.Sprintf("\n%s should either map to an apply command or a composite command with apply commands", strings.Join(invalidApplyEvents, ", "))
eventErrorsList = append(eventErrorsList, fmt.Sprintf("%s should either map to an apply command or a composite command with apply commands", strings.Join(invalidApplyEvents, ", ")))
}

if len(invalidExecEvents) > 0 {
eventErrors += fmt.Sprintf("\n%s should either map to an exec command or a composite command with exec commands", strings.Join(invalidExecEvents, ", "))
eventErrorsList = append(eventErrorsList, fmt.Sprintf("%s should either map to an exec command or a composite command with exec commands", strings.Join(invalidExecEvents, ", ")))
}

if len(eventErrors) != 0 {
if len(eventErrorsList) != 0 {
eventErrors := fmt.Sprintf("\n%s", strings.Join(eventErrorsList, "\n"))
err = &InvalidEventError{eventType: eventType, errorMsg: eventErrors}
}

Expand Down
38 changes: 20 additions & 18 deletions pkg/validation/projects.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package validation

import (
"fmt"
"strings"

"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
)
Expand All @@ -10,7 +11,7 @@ import (
// and if the checkout remote matches the renote configured
func ValidateStarterProjects(starterProjects []v1alpha2.StarterProject) error {

var errString string
var projectErrorsList []string
for _, starterProject := range starterProjects {
var gitSource v1alpha2.GitLikeProjectSource
if starterProject.Git != nil {
Expand All @@ -23,27 +24,28 @@ func ValidateStarterProjects(starterProjects []v1alpha2.StarterProject) error {

switch len(gitSource.Remotes) {
case 0:
starterProjectErr := fmt.Errorf("\nstarterProject %s should have at least one remote", starterProject.Name)
starterProjectErr := fmt.Errorf("starterProject %s should have at least one remote", starterProject.Name)
newErr := resolveErrorMessageWithImportAttributes(starterProjectErr, starterProject.Attributes)
errString += newErr.Error()
projectErrorsList = append(projectErrorsList, newErr.Error())
case 1:
if gitSource.CheckoutFrom != nil && gitSource.CheckoutFrom.Remote != "" {
err := validateRemoteMap(gitSource.Remotes, gitSource.CheckoutFrom.Remote, starterProject.Name)
if err != nil {
newErr := resolveErrorMessageWithImportAttributes(err, starterProject.Attributes)
errString += fmt.Sprintf("\n%s", newErr.Error())
projectErrorsList = append(projectErrorsList, newErr.Error())
}
}
default: // len(gitSource.Remotes) >= 2
starterProjectErr := fmt.Errorf("\nstarterProject %s should have one remote only", starterProject.Name)
starterProjectErr := fmt.Errorf("starterProject %s should have one remote only", starterProject.Name)
newErr := resolveErrorMessageWithImportAttributes(starterProjectErr, starterProject.Attributes)
errString += newErr.Error()
projectErrorsList = append(projectErrorsList, newErr.Error())
}
}

var err error
if len(errString) > 0 {
err = fmt.Errorf("error validating starter projects:%s", errString)
if len(projectErrorsList) > 0 {
projectErrors := fmt.Sprintf("\n%s", strings.Join(projectErrorsList, "\n"))
err = fmt.Errorf("error validating starter projects:%s", projectErrors)
}

return err
Expand All @@ -53,7 +55,7 @@ func ValidateStarterProjects(starterProjects []v1alpha2.StarterProject) error {
// remote is mandatory and if the checkout remote matches the renote configured
func ValidateProjects(projects []v1alpha2.Project) error {

var errString string
var projectErrorsList []string
for _, project := range projects {
var gitSource v1alpha2.GitLikeProjectSource
if project.Git != nil {
Expand All @@ -63,36 +65,36 @@ func ValidateProjects(projects []v1alpha2.Project) error {
} else {
continue
}

switch len(gitSource.Remotes) {
case 0:
projectErr := fmt.Errorf("\nprojects %s should have at least one remote", project.Name)
projectErr := fmt.Errorf("projects %s should have at least one remote", project.Name)
newErr := resolveErrorMessageWithImportAttributes(projectErr, project.Attributes)
errString += newErr.Error()
projectErrorsList = append(projectErrorsList, newErr.Error())
case 1:
if gitSource.CheckoutFrom != nil && gitSource.CheckoutFrom.Remote != "" {
if err := validateRemoteMap(gitSource.Remotes, gitSource.CheckoutFrom.Remote, project.Name); err != nil {
newErr := resolveErrorMessageWithImportAttributes(err, project.Attributes)
errString += fmt.Sprintf("\n%s", newErr.Error())
projectErrorsList = append(projectErrorsList, newErr.Error())
}
}
default: // len(gitSource.Remotes) >= 2
if gitSource.CheckoutFrom == nil || gitSource.CheckoutFrom.Remote == "" {
projectErr := fmt.Errorf("\nproject %s has more than one remote defined, but has no checkoutfrom remote defined", project.Name)
projectErr := fmt.Errorf("project %s has more than one remote defined, but has no checkoutfrom remote defined", project.Name)
newErr := resolveErrorMessageWithImportAttributes(projectErr, project.Attributes)
errString += newErr.Error()
projectErrorsList = append(projectErrorsList, newErr.Error())
continue
}
if err := validateRemoteMap(gitSource.Remotes, gitSource.CheckoutFrom.Remote, project.Name); err != nil {
newErr := resolveErrorMessageWithImportAttributes(err, project.Attributes)
errString += fmt.Sprintf("\n%s", newErr.Error())
projectErrorsList = append(projectErrorsList, newErr.Error())
}
}
}

var err error
if len(errString) > 0 {
err = fmt.Errorf("error validating projects:%s", errString)
if len(projectErrorsList) > 0 {
projectErrors := fmt.Sprintf("\n%s", strings.Join(projectErrorsList, "\n"))
err = fmt.Errorf("error validating projects:%s", projectErrors)
}

return err
Expand Down

0 comments on commit 7cc978e

Please sign in to comment.