resources: Officially add Validate method

This officially adds the Validate method to the resource API, and also
cleans up the ordering in existing resources.

docs/resource-guide.md

@@ -31,11 +31,11 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 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

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")
 			}

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)

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!

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

resources/msg.go

@@ -78,13 +78,7 @@ func (obj *MsgRes) Default() Res {
 	return &MsgRes{}
 }
 
-// Init runs some startup code for this resource.
+// Validate the params that are passed to MsgRes.
-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
 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 {

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

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

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 {

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

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)

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

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)