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 more validation to service struct #1110

Merged
merged 1 commit into from
Jun 26, 2019
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
4 changes: 2 additions & 2 deletions service/dependency.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type Dependency struct {
VolumesFrom []string `hash:"name:4" validate:"unique,dive,printascii"`

// Ports holds ports configuration for container.
Ports []string `hash:"name:5"`
Ports []string `hash:"name:5" validate:"unique,dive,portmap"`

// Command is the Docker command which will be executed when container started.
Command string `hash:"name:6" validate:"printascii"`
Expand All @@ -25,5 +25,5 @@ type Dependency struct {
Args []string `hash:"name:7" validate:"dive,printascii"`

// Env is a slice of environment variables in key=value format.
Env []string `hash:"name:8" validate:"unique,dive,printascii"`
Env []string `hash:"name:8" validate:"unique,dive,env"`
}
2 changes: 1 addition & 1 deletion service/event.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ type Event struct {
Description string `hash:"name:3" validate:"printascii"`

// Data holds the input parameters of event.
Data []*Parameter `hash:"name:4"`
Data []*Parameter `hash:"name:4" validate:"dive,required"`
}

// GetEvent returns event eventKey of service.
Expand Down
2 changes: 1 addition & 1 deletion service/parameter.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,5 @@ type Parameter struct {
Repeated bool `hash:"name:6"`

// Definition of the structure of the object when the type is object
Object []*Parameter `hash:"name:7"`
Object []*Parameter `hash:"name:7" validate:"unique,dive,required"`
Copy link
Member

Choose a reason for hiding this comment

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

why this one is unique?
does it actually works?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it won't be used here, but shouldn't exist

}
10 changes: 5 additions & 5 deletions service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,19 @@ type Service struct {
Sid string `hash:"name:1" validate:"omitempty,printascii,max=63,domain"`

// Name is the service name.
Name string `hash:"name:2" validate:"required,printascii,min=1"`
Name string `hash:"name:2" validate:"required,printascii"`

// Description is service description.
Description string `hash:"name:3" validate:"printascii"`

// Tasks are the list of tasks that service can execute.
Tasks []*Task `hash:"name:4"`
Tasks []*Task `hash:"name:4" validate:"dive,required"`

// Events are the list of events that service can emit.
Events []*Event `hash:"name:5"`
Events []*Event `hash:"name:5" validate:"dive,required"`

// Dependencies are the Docker containers that service can depend on.
Dependencies []*Dependency `hash:"name:6"`
Dependencies []*Dependency `hash:"name:6" validate:"dive,required"`

// Configuration of the service
Configuration *Dependency `hash:"name:8"`
Expand All @@ -53,7 +53,7 @@ type Service struct {
Repository string `hash:"name:7" validate:"omitempty,uri"`

// Source is the hash id of service's source code on IPFS.
Source string `hash:"name:9"`
Source string `hash:"name:9" validate:"required,printascii"`
}

// StatusType of the service.
Expand Down
57 changes: 57 additions & 0 deletions service/service_validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ func ValidateService(service *Service) error {
}
}

if err := isServiceKeysUnique(service); err != nil {
errs = append(errs, err)
}

// validate configuration image
if service.Configuration.Image != "" {
errs = append(errs, errors.New("configuration.image is not allowed"))
Expand Down Expand Up @@ -82,6 +86,59 @@ func ValidateService(service *Service) error {
return errs.ErrorOrNil()
}

// isServiceKeysUnique checks uniqueness of service deps/tasks/events/params keys.
func isServiceKeysUnique(service *Service) error {
var errs xerrors.Errors
exist := make(map[string]bool)
for _, dep := range service.Dependencies {
if exist[dep.Key] {
errs = append(errs, fmt.Errorf("dependencies[%s] already exist", dep.Key))
}
exist[dep.Key] = true
}

exist = make(map[string]bool)
for _, task := range service.Tasks {
if exist[task.Key] {
errs = append(errs, fmt.Errorf("tasks[%s] already exist", task.Key))
}
exist[task.Key] = true

existparam := make(map[string]bool)
for _, param := range task.Inputs {
if existparam[param.Key] {
errs = append(errs, fmt.Errorf("tasks[%s].inputs[%s] already exist", task.Key, param.Key))
}
existparam[param.Key] = true
}

existparam = make(map[string]bool)
for _, param := range task.Outputs {
if existparam[param.Key] {
errs = append(errs, fmt.Errorf("tasks[%s].outputs[%s] already exist", task.Key, param.Key))
}
existparam[param.Key] = true
}
}

exist = make(map[string]bool)
for _, event := range service.Events {
if exist[event.Key] {
errs = append(errs, fmt.Errorf("events[%s] already exist", event.Key))
}
exist[event.Key] = true

existparam := make(map[string]bool)
for _, param := range event.Data {
if existparam[param.Key] {
Copy link
Member

@NicolasMahe NicolasMahe Jun 26, 2019

Choose a reason for hiding this comment

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

params.Object keys are not tested (task inputs, task outputs and event)
maybe we could use the validation lib with something like:

Object []*Parameter `hash:"name:7" validate:"dive,(key)unique,required"`

otherwise a function that validate keys of type []*Parameter that can be called recursively.

Copy link
Member

Choose a reason for hiding this comment

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

otherwise a new custom validation like PortMapping

errs = append(errs, fmt.Errorf("events[%s].data[%s] already exist", event.Key, param.Key))
}
existparam[param.Key] = true
}
}
return errs.ErrorOrNil()
}

func newValidator() (*validator.Validate, ut.Translator) {
en := en.New()
uni := ut.New(en, en)
Expand Down
4 changes: 2 additions & 2 deletions service/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ type Task struct {
Description string `hash:"name:3" validate:"printascii"`

// Inputs are the definition of the execution inputs of task.
Inputs []*Parameter `hash:"name:4"`
Inputs []*Parameter `hash:"name:4" validate:"dive,required"`

// Outputs are the definition of the execution results of task.
Outputs []*Parameter `hash:"name:5"`
Outputs []*Parameter `hash:"name:5" validate:"dive,required"`
}

// GetTask returns task taskKey of service.
Expand Down