diff --git a/docs/resource-guide.md b/docs/resource-guide.md index cbd9e91c..e6eee670 100644 --- a/docs/resource-guide.md +++ b/docs/resource-guide.md @@ -31,11 +31,11 @@ along with this program. If not, see . 2. [Theory - Resource theory in mgmt](#theory) 3. [Resource API - Getting started with mgmt](#resource-api) * [Default - Get an empty resource with defaults](#default) + * [Validate - Validate the values of a resource struct](#validate) * [Init - Initialize the resource](#init) * [CheckApply - Check and apply resource state](#checkapply) * [Watch - Detect resource changes](#watch) * [Compare - Compare resource with another](#compare) - * [Validate - Validate the values of a resource struct](#validate) * [GetUIDs - Returns the list of resource UID's](#getuids) * [AutoEdges - Returns the autoedge interface matcher](#autoedges) * [CollectPattern - Currently a stub, API will change](#collectpattern) @@ -94,6 +94,17 @@ func (obj *FooRes) Default() Res { } ``` +###Validate +```golang +Validate() error +``` + +This method is used to validate if the populated resource struct is a valid +representation of the resource kind. If it does not conform to the resource +specifications, it should generate an error. If you notice that this method is +quite large, it might be an indication that you should reconsider the parameter +list and interface to this resource. This method is called _before_ `Init`. + ###Init ```golang Init() error @@ -116,6 +127,12 @@ func (obj *FooRes) Init() error { } ``` +This method is always called after `Validate` has run successfully, with the +exception that we can't prevent a malicious or buggy `libmgmt` user to not run +this. In other words, you should expect `Validate` to have run first, but you +shouldn't allow `Init` to dangerously `rm -rf /$the_world` if your code only +checks `$the_world` in `Validate`. Remember to always program safely! + ###CheckApply ```golang CheckApply(apply bool) (checkOK bool, err error) @@ -367,17 +384,6 @@ func (obj *FooRes) Compare(res Res) bool { } ``` -###Validate -```golang -Validate() error -``` - -This method is used to validate if the populated resource struct is a valid -representation of the resource kind. If it does not conform to the resource -specifications, it should generate an error. If you notice that this method is -quite large, it might be an indication that you might want to reconsider the -parameter list and interface to this resource. - ###GetUIDs ```golang GetUIDs() []ResUID diff --git a/pgraph/pgraph.go b/pgraph/pgraph.go index 826321c7..1d285e88 100644 --- a/pgraph/pgraph.go +++ b/pgraph/pgraph.go @@ -550,6 +550,9 @@ func (g *Graph) GraphSync(oldGraph *Graph) (*Graph, error) { vertex := oldGraph.GetVertexMatch(res) if vertex == nil { // no match found + if err := res.Validate(); err != nil { + return nil, errwrap.Wrapf(err, "could not Validate() resource") + } if err := res.Init(); err != nil { return nil, errwrap.Wrapf(err, "could not Init() resource") } diff --git a/resources/exec.go b/resources/exec.go index 445687f5..a9175634 100644 --- a/resources/exec.go +++ b/resources/exec.go @@ -74,14 +74,7 @@ func (obj *ExecRes) Default() Res { return &ExecRes{} } -// Init runs some startup code for this resource. -func (obj *ExecRes) Init() error { - obj.BaseRes.kind = "Exec" - return obj.BaseRes.Init() // call base init, b/c we're overriding -} - // Validate if the params passed in are valid data. -// FIXME: where should this get called ? func (obj *ExecRes) Validate() error { if obj.Cmd == "" { // this is the only thing that is really required return fmt.Errorf("Command can't be empty!") @@ -95,6 +88,12 @@ func (obj *ExecRes) Validate() error { return nil } +// Init runs some startup code for this resource. +func (obj *ExecRes) Init() error { + obj.BaseRes.kind = "Exec" + return obj.BaseRes.Init() // call base init, b/c we're overriding +} + // BufioChanScanner wraps the scanner output in a channel. func (obj *ExecRes) BufioChanScanner(scanner *bufio.Scanner) (chan string, chan error) { ch, errch := make(chan string), make(chan error) diff --git a/resources/file.go b/resources/file.go index 09376942..a1b1691f 100644 --- a/resources/file.go +++ b/resources/file.go @@ -84,6 +84,32 @@ func (obj *FileRes) Default() Res { } } +// Validate reports any problems with the struct definition. +func (obj *FileRes) Validate() error { + if obj.Dirname != "" && !strings.HasSuffix(obj.Dirname, "/") { + return fmt.Errorf("Dirname must end with a slash.") + } + + if strings.HasPrefix(obj.Basename, "/") { + return fmt.Errorf("Basename must not start with a slash.") + } + + if obj.Content != nil && obj.Source != "" { + return fmt.Errorf("Can't specify both Content and Source.") + } + + if obj.isDir && obj.Content != nil { // makes no sense + return fmt.Errorf("Can't specify Content when creating a Dir.") + } + + // XXX: should this specify that we create an empty directory instead? + //if obj.Source == "" && obj.isDir { + // return fmt.Errorf("Can't specify an empty source when creating a Dir.") + //} + + return nil +} + // Init runs some startup code for this resource. func (obj *FileRes) Init() error { obj.sha256sum = "" @@ -115,32 +141,6 @@ func (obj *FileRes) GetPath() string { return obj.Dirname + obj.Basename } -// Validate reports any problems with the struct definition. -func (obj *FileRes) Validate() error { - if obj.Dirname != "" && !strings.HasSuffix(obj.Dirname, "/") { - return fmt.Errorf("Dirname must end with a slash.") - } - - if strings.HasPrefix(obj.Basename, "/") { - return fmt.Errorf("Basename must not start with a slash.") - } - - if obj.Content != nil && obj.Source != "" { - return fmt.Errorf("Can't specify both Content and Source.") - } - - if obj.isDir && obj.Content != nil { // makes no sense - return fmt.Errorf("Can't specify Content when creating a Dir.") - } - - // XXX: should this specify that we create an empty directory instead? - //if obj.Source == "" && obj.isDir { - // return fmt.Errorf("Can't specify an empty source when creating a Dir.") - //} - - return nil -} - // Watch is the primary listener for this resource and it outputs events. // This one is a file watcher for files and directories. // Modify with caution, it is probably important to write some test cases first! diff --git a/resources/hostname.go b/resources/hostname.go index d0a5d6b7..95f31337 100644 --- a/resources/hostname.go +++ b/resources/hostname.go @@ -87,6 +87,14 @@ func (obj *HostnameRes) Default() Res { return &HostnameRes{} } +// Validate if the params passed in are valid data. +func (obj *HostnameRes) Validate() error { + if obj.PrettyHostname == "" && obj.StaticHostname == "" && obj.TransientHostname == "" { + return ErrResourceInsufficientParameters + } + return nil +} + // Init runs some startup code for this resource. func (obj *HostnameRes) Init() error { obj.BaseRes.kind = "Hostname" @@ -102,15 +110,6 @@ func (obj *HostnameRes) Init() error { return obj.BaseRes.Init() // call base init, b/c we're overriding } -// Validate if the params passed in are valid data. -// FIXME: where should this get called ? -func (obj *HostnameRes) Validate() error { - if obj.PrettyHostname == "" && obj.StaticHostname == "" && obj.TransientHostname == "" { - return ErrResourceInsufficientParameters - } - return nil -} - // Watch is the primary listener for this resource and it outputs events. func (obj *HostnameRes) Watch(processChan chan event.Event) error { cuid := obj.ConvergerUID() // get the converger uid used to report status diff --git a/resources/msg.go b/resources/msg.go index 72d1ac37..49a7eb06 100644 --- a/resources/msg.go +++ b/resources/msg.go @@ -78,13 +78,7 @@ func (obj *MsgRes) Default() Res { return &MsgRes{} } -// Init runs some startup code for this resource. -func (obj *MsgRes) Init() error { - obj.BaseRes.kind = "Msg" - return obj.BaseRes.Init() // call base init, b/c we're overrriding -} - -// Validate the params that are passed to MsgRes +// Validate the params that are passed to MsgRes. func (obj *MsgRes) Validate() error { invalidCharacters := regexp.MustCompile("[^a-zA-Z0-9_]") for field := range obj.Fields { @@ -98,6 +92,12 @@ func (obj *MsgRes) Validate() error { return nil } +// Init runs some startup code for this resource. +func (obj *MsgRes) Init() error { + obj.BaseRes.kind = "Msg" + return obj.BaseRes.Init() // call base init, b/c we're overrriding +} + // isAllStateOK derives a compound state from all internal cache flags that apply to this resource. func (obj *MsgRes) isAllStateOK() bool { if obj.Journal && !obj.journalStateOK { diff --git a/resources/noop.go b/resources/noop.go index a989665d..f2f8e915 100644 --- a/resources/noop.go +++ b/resources/noop.go @@ -51,18 +51,17 @@ func (obj *NoopRes) Default() Res { return &NoopRes{} } +// Validate if the params passed in are valid data. +func (obj *NoopRes) Validate() error { + return nil +} + // Init runs some startup code for this resource. func (obj *NoopRes) Init() error { obj.BaseRes.kind = "Noop" return obj.BaseRes.Init() // call base init, b/c we're overriding } -// Validate if the params passed in are valid data. -// FIXME: where should this get called ? -func (obj *NoopRes) Validate() error { - return nil -} - // Watch is the primary listener for this resource and it outputs events. func (obj *NoopRes) Watch(processChan chan event.Event) error { cuid := obj.ConvergerUID() // get the converger uid used to report status diff --git a/resources/nspawn.go b/resources/nspawn.go index 7fb02dfc..02f8fec3 100644 --- a/resources/nspawn.go +++ b/resources/nspawn.go @@ -57,26 +57,6 @@ type NspawnRes struct { svc *SvcRes } -// Default returns some sensible defaults for this resource. -func (obj *NspawnRes) Default() Res { - return &NspawnRes{ - State: running, - } -} - -// Init runs some startup code for this resource -func (obj *NspawnRes) Init() error { - var serviceName = fmt.Sprintf(nspawnServiceTmpl, obj.GetName()) - obj.svc = &SvcRes{} - obj.svc.Name = serviceName - obj.svc.State = obj.State - if err := obj.svc.Init(); err != nil { - return err - } - obj.BaseRes.kind = "Nspawn" - return obj.BaseRes.Init() -} - // NewNspawnRes is the constructor for this resource func NewNspawnRes(name string, state string) (*NspawnRes, error) { obj := &NspawnRes{ @@ -88,7 +68,14 @@ func NewNspawnRes(name string, state string) (*NspawnRes, error) { return obj, obj.Init() } -// Validate params +// Default returns some sensible defaults for this resource. +func (obj *NspawnRes) Default() Res { + return &NspawnRes{ + State: running, + } +} + +// Validate if the params passed in are valid data. func (obj *NspawnRes) Validate() error { // TODO: validStates should be an enum! validStates := map[string]struct{}{ @@ -101,6 +88,19 @@ func (obj *NspawnRes) Validate() error { return obj.svc.Validate() } +// Init runs some startup code for this resource. +func (obj *NspawnRes) Init() error { + var serviceName = fmt.Sprintf(nspawnServiceTmpl, obj.GetName()) + obj.svc = &SvcRes{} + obj.svc.Name = serviceName + obj.svc.State = obj.State + if err := obj.svc.Init(); err != nil { + return err + } + obj.BaseRes.kind = "Nspawn" + return obj.BaseRes.Init() +} + // Watch for state changes and sends a message to the bus if there is a change func (obj *NspawnRes) Watch(processChan chan event.Event) error { cuid := obj.ConvergerUID() // get the converger uid used to report status diff --git a/resources/password.go b/resources/password.go index 415889d3..20eb3e6b 100644 --- a/resources/password.go +++ b/resources/password.go @@ -74,6 +74,11 @@ func (obj *PasswordRes) Default() Res { } } +// Validate if the params passed in are valid data. +func (obj *PasswordRes) Validate() error { + return nil +} + // Init generates a new password for this resource if one was not provided. It // will save this into a local file. It will load it back in from previous runs. func (obj *PasswordRes) Init() error { @@ -88,12 +93,6 @@ func (obj *PasswordRes) Init() error { return obj.BaseRes.Init() // call base init, b/c we're overriding } -// Validate if the params passed in are valid data. -// FIXME: where should this get called ? -func (obj *PasswordRes) Validate() error { - return nil -} - func (obj *PasswordRes) read() (string, error) { file, err := os.Open(obj.path) // open a handle to read the file if err != nil { diff --git a/resources/pkg.go b/resources/pkg.go index 0f63a1dc..065fa993 100644 --- a/resources/pkg.go +++ b/resources/pkg.go @@ -67,6 +67,15 @@ func (obj *PkgRes) Default() Res { } } +// Validate checks if the resource data structure was populated correctly. +func (obj *PkgRes) Validate() error { + if obj.State == "" { + return fmt.Errorf("State cannot be empty!") + } + + return nil +} + // Init runs some startup code for this resource. func (obj *PkgRes) Init() error { obj.BaseRes.kind = "Pkg" @@ -102,15 +111,6 @@ func (obj *PkgRes) Init() error { return nil } -// Validate checks if the resource data structure was populated correctly. -func (obj *PkgRes) Validate() error { - if obj.State == "" { - return fmt.Errorf("State cannot be empty!") - } - - return nil -} - // Watch is the primary listener for this resource and it outputs events. // It uses the PackageKit UpdatesChanged signal to watch for changes. // TODO: https://github.com/hughsie/PackageKit/issues/109 diff --git a/resources/resources.go b/resources/resources.go index 573b339e..ff01c146 100644 --- a/resources/resources.go +++ b/resources/resources.go @@ -164,8 +164,8 @@ type Base interface { type Res interface { Base // include everything from the Base interface Default() Res // return a struct with sane defaults as a Res + Validate() error Init() error - //Validate() error // TODO: this might one day be added GetUIDs() []ResUID // most resources only return one Watch(chan event.Event) error // send on channel to signal process() events CheckApply(apply bool) (checkOK bool, err error) diff --git a/resources/svc.go b/resources/svc.go index abda0347..7ad734ee 100644 --- a/resources/svc.go +++ b/resources/svc.go @@ -61,12 +61,6 @@ func (obj *SvcRes) Default() Res { return &SvcRes{} } -// Init runs some startup code for this resource. -func (obj *SvcRes) Init() error { - obj.BaseRes.kind = "Svc" - return obj.BaseRes.Init() // call base init, b/c we're overriding -} - // Validate checks if the resource data structure was populated correctly. func (obj *SvcRes) Validate() error { if obj.State != "running" && obj.State != "stopped" && obj.State != "" { @@ -78,6 +72,12 @@ func (obj *SvcRes) Validate() error { return nil } +// Init runs some startup code for this resource. +func (obj *SvcRes) Init() error { + obj.BaseRes.kind = "Svc" + return obj.BaseRes.Init() // call base init, b/c we're overriding +} + // Watch is the primary listener for this resource and it outputs events. func (obj *SvcRes) Watch(processChan chan event.Event) error { cuid := obj.ConvergerUID() // get the converger uid used to report status diff --git a/resources/timer.go b/resources/timer.go index 2f29ca05..e073ae51 100644 --- a/resources/timer.go +++ b/resources/timer.go @@ -60,17 +60,17 @@ func (obj *TimerRes) Default() Res { return &TimerRes{} } +// Validate the params that are passed to TimerRes. +func (obj *TimerRes) Validate() error { + return nil +} + // Init runs some startup code for this resource. func (obj *TimerRes) Init() error { obj.BaseRes.kind = "Timer" return obj.BaseRes.Init() // call base init, b/c we're overrriding } -// Validate the params that are passed to TimerRes. -func (obj *TimerRes) Validate() error { - return nil -} - // newTicker creates a new ticker func (obj *TimerRes) newTicker() *time.Ticker { return time.NewTicker(time.Duration(obj.Interval) * time.Second)