codestyle, fmt, unit tests, etc

cmd/OliveTin/main.go

@@ -63,7 +63,7 @@ func reloadConfig() {
 		os.Exit(1)
 	}
 
-	config.Sanitize(cfg)
+	cfg.Sanitize()
 }
 
 func main() {

go.mod

@@ -12,6 +12,8 @@ require (
 	github.com/sirupsen/logrus v1.8.1
 	github.com/spf13/viper v1.8.1
 	github.com/stretchr/testify v1.7.0
+	golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 // indirect
+	golang.org/x/tools v0.1.7 // indirect
 	google.golang.org/genproto v0.0.0-20210617175327-b9e0b3197ced
 	google.golang.org/grpc v1.40.0-dev.0.20210708170655-30dfb4b933a5
 	google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0

go.sum

@@ -291,6 +291,7 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de
 github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
+github.com/yuin/goldmark v1.4.0/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
 go.etcd.io/etcd/api/v3 v3.5.0/go.mod h1:cbVKeC6lCfl7j/8jBhAK6aIYO9XOjdptoxU/nLQcPvs=
 go.etcd.io/etcd/client/pkg/v3 v3.5.0/go.mod h1:IJHfcCEKxYu1Os13ZdwCwIUTUVGYTSAM3YSwc9/Ac1g=
 go.etcd.io/etcd/client/v2 v2.305.0/go.mod h1:h9puh54ZTgAKtEbut2oe9P4L/oqKCVB6xsXlzd7alYQ=
@@ -392,6 +393,7 @@ golang.org/x/net v0.0.0-20210119194325-5f4716e94777/go.mod h1:m0MpNAwzfU5UDzcl9v
 golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
 golang.org/x/net v0.0.0-20210316092652-d523dce5a7f4/go.mod h1:RBQZq4jEuRlivfhVLdyRGr576XBO4/greRjx4P4O3yc=
 golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM=
+golang.org/x/net v0.0.0-20210805182204-aaa1db679c0d/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
 golang.org/x/net v0.0.0-20210825183410-e898025ed96a h1:bRuuGXV8wwSdGTB+CtJf+FjgO1APK1CoO39T4BN/XBw=
 golang.org/x/net v0.0.0-20210825183410-e898025ed96a/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
@@ -463,6 +465,7 @@ golang.org/x/sys v0.0.0-20210403161142-5e06dd20ab57/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.0.0-20210809222454-d867a43fc93e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20210823070655-63515b42dcdf h1:2ucpDCmfkl8Bd/FsLtiD653Wf96cW37s+iGx93zsu4k=
 golang.org/x/sys v0.0.0-20210823070655-63515b42dcdf/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
@@ -538,6 +541,8 @@ golang.org/x/tools v0.1.2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
 golang.org/x/tools v0.1.3/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
 golang.org/x/tools v0.1.5 h1:ouewzE6p+/VEB31YYnTbEJdi8pFqKp4P4n85vwo3DHA=
 golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
+golang.org/x/tools v0.1.7 h1:6j8CgantCy3yc8JGBqkDLMKWqZ0RDU2g1HVgacojGWQ=
+golang.org/x/tools v0.1.7/go.mod h1:LGqMHiF4EqQNHR1JncWGqT5BVaXmza+X+BDGol+dOxo=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

internal/acl/acl.go

@@ -6,10 +6,12 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
+// User respresents a person.
 type User struct {
 	Username string
 }
 
+// IsAllowedExec checks if a User is allowed to execute an Action
 func IsAllowedExec(cfg *config.Config, user *User, action *config.Action) bool {
 	canExec := cfg.DefaultPermissions.Exec
 
@@ -40,6 +42,7 @@ func IsAllowedExec(cfg *config.Config, user *User, action *config.Action) bool {
 	return canExec
 }
 
+// IsAllowedView checks if a User is allowed to view an Action
 func IsAllowedView(cfg *config.Config, user *User, action *config.Action) bool {
 	canView := cfg.DefaultPermissions.View
 
@@ -75,6 +78,8 @@ func isUserInGroup(user *User, usergroup string) bool {
 	return true
 }
 
+// UserFromContext tries to find a user from a grpc context - obviously this is
+// a stub at the moment.
 func UserFromContext(ctx context.Context) *User {
 	return &User{
 		Username: "Guest",

internal/config/config.go

@@ -1,7 +1,7 @@
 package config
 
-import ()
-
+// Action represents the core functionality of OliveTin - commands that show up
+// as buttons in the UI.
 type Action struct {
 	ID          string
 	Title       string
@@ -13,6 +13,7 @@ type Action struct {
 	Arguments   []ActionArgument
 }
 
+// ActionArgument objects appear on Actions.
 type ActionArgument struct {
 	Name    string
 	Title   string
@@ -21,6 +22,7 @@ type ActionArgument struct {
 	Choices []ActionArgumentChoice
 }
 
+// ActionArgumentChoice represents a predefined choice for an argument.
 type ActionArgumentChoice struct {
 	Value string
 	Title string
@@ -35,17 +37,20 @@ type Entity struct {
 	CSS     map[string]string
 }
 
+// PermissionsEntry defines what users can do with an action.
 type PermissionsEntry struct {
 	Usergroup string
 	View      bool
 	Exec      bool
 }
 
+// DefaultPermissions will be used when no PermissionsEntry overrides it.
 type DefaultPermissions struct {
 	View bool
 	Exec bool
 }
 
+// UserGroup is a group of users.
 type UserGroup struct {
 	Name    string
 	Members []string

internal/config/config_helpers.go

@@ -1,5 +1,6 @@
 package config
 
+// FindAction will return a action if there is a match on Title
 func (cfg *Config) FindAction(actionTitle string) *Action {
 	for _, action := range cfg.Actions {
 		if action.Title == actionTitle {
@@ -10,6 +11,7 @@ func (cfg *Config) FindAction(actionTitle string) *Action {
 	return nil
 }
 
+// FindArg will return an arg if there is a match on Name
 func (action *Action) FindArg(name string) *ActionArgument {
 	for _, arg := range action.Arguments {
 		if arg.Name == name {

internal/config/sanitize.go

@@ -4,36 +4,38 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
-func Sanitize(cfg *Config) {
-	sanitizeLogLevel(cfg)
+// Sanitize will look for common configuration issues, and fix them. For example,
+// populating undefined fields - name -> title, etc.
+func (cfg *Config) Sanitize() {
+	cfg.sanitizeLogLevel()
 
 	//log.Infof("cfg %p", cfg)
 
-	for idx, _ := range cfg.Actions {
-		sanitizeAction(&cfg.Actions[idx])
+	for idx := range cfg.Actions {
+		cfg.Actions[idx].sanitize()
 	}
 }
 
-func sanitizeLogLevel(cfg *Config) {
+func (cfg *Config) sanitizeLogLevel() {
 	if logLevel, err := log.ParseLevel(cfg.LogLevel); err == nil {
 		log.Info("Setting log level to ", logLevel)
 		log.SetLevel(logLevel)
 	}
 }
 
-func sanitizeAction(action *Action) {
+func (action *Action) sanitize() {
 	if action.Timeout < 3 {
 		action.Timeout = 3
 	}
 
 	action.Icon = lookupHTMLIcon(action.Icon)
 
-	for idx, _ := range action.Arguments {
-		sanitizeActionArgument(&action.Arguments[idx])
+	for idx := range action.Arguments {
+		action.Arguments[idx].sanitize()
 	}
 }
 
-func sanitizeActionArgument(arg *ActionArgument) {
+func (arg *ActionArgument) sanitize() {
 	if arg.Title == "" {
 		arg.Title = arg.Name
 	}
@@ -44,13 +46,13 @@ func sanitizeActionArgument(arg *ActionArgument) {
 		}
 	}
 
-	sanitizeActionArgumentNoType(arg)
+	arg.sanitizeNoType()
 
 	// TODO Validate the default against the type checker, but this creates a
 	// import loop
 }
 
-func sanitizeActionArgumentNoType(arg *ActionArgument) {
+func (arg *ActionArgument) sanitizeNoType() {
 	if len(arg.Choices) == 0 && arg.Type == "" {
 		log.WithFields(log.Fields{
 			"arg": arg.Name,

internal/config/sanitize_test.go

@@ -11,7 +11,7 @@ func TestSanitizeConfig(t *testing.T) {
 	a := Action{
 		Title: "Mr Waffles",
 		Arguments: []ActionArgument{
-			ActionArgument{
+			{
 				Name: "Carrots",
 				Choices: []ActionArgumentChoice{
 					{
@@ -26,8 +26,7 @@ func TestSanitizeConfig(t *testing.T) {
 	}
 
 	c.Actions = append(c.Actions, a)
-
-	Sanitize(c)
+	c.Sanitize()
 
 	a2 := c.FindAction("Mr Waffles")
 

internal/executor/executor.go

@@ -24,6 +24,9 @@ var (
 	}
 )
 
+// InternalLogEntry objects are created by an Executor, and represent the final
+// state of execution (even if the command is not executed). It's designed to be
+// easily serializable.
 type InternalLogEntry struct {
 	Datetime string
 	Stdout   string
@@ -40,6 +43,8 @@ type InternalLogEntry struct {
 	ActionIcon  string
 }
 
+// ExecutionRequest is a request to execute an action. It's passed to an
+// Executor. They're created from the grpcapi.
 type ExecutionRequest struct {
 	ActionName         string
 	Arguments          map[string]string
@@ -50,33 +55,37 @@ type ExecutionRequest struct {
 	finalParsedCommand string
 }
 
-type ExecutorStep interface {
+type executorStep interface {
 	Exec(*ExecutionRequest) bool
 }
 
+// Executor represents a helper class for executing commands. It's main method
+// is ExecRequest
 type Executor struct {
 	Logs []InternalLogEntry
 
-	chainOfCommand []ExecutorStep
+	chainOfCommand []executorStep
 }
 
+// DefaultExecutor returns an Executor, with a sensible "chain of command" for
+// executing actions.
 func DefaultExecutor() *Executor {
 	e := Executor{}
-	e.chainOfCommand = []ExecutorStep{
-		StepFindAction{},
-		StepAclCheck{},
-		StepParseArgs{},
-		StepLogStart{},
-		StepExec{},
-		StepLogFinish{},
+	e.chainOfCommand = []executorStep{
+		stepFindAction{},
+		stepACLCheck{},
+		stepParseArgs{},
+		stepLogStart{},
+		stepExec{},
+		stepLogFinish{},
 	}
 
 	return &e
 }
 
-type StepFindAction struct{}
+type stepFindAction struct{}
 
-func (s StepFindAction) Exec(req *ExecutionRequest) bool {
+func (s stepFindAction) Exec(req *ExecutionRequest) bool {
 	actualAction := req.Cfg.FindAction(req.ActionName)
 
 	if actualAction == nil {
@@ -95,9 +104,9 @@ func (s StepFindAction) Exec(req *ExecutionRequest) bool {
 	return true
 }
 
-type StepAclCheck struct{}
+type stepACLCheck struct{}
 
-func (s StepAclCheck) Exec(req *ExecutionRequest) bool {
+func (s stepACLCheck) Exec(req *ExecutionRequest) bool {
 	return acl.IsAllowedExec(req.Cfg, req.User, req.action)
 }
 
@@ -132,9 +141,9 @@ func (e *Executor) ExecRequest(req *ExecutionRequest) *pb.StartActionResponse {
 	}
 }
 
-type StepLogStart struct{}
+type stepLogStart struct{}
 
-func (e StepLogStart) Exec(req *ExecutionRequest) bool {
+func (e stepLogStart) Exec(req *ExecutionRequest) bool {
 	log.WithFields(log.Fields{
 		"title":   req.action.Title,
 		"timeout": req.action.Timeout,
@@ -143,9 +152,9 @@ func (e StepLogStart) Exec(req *ExecutionRequest) bool {
 	return true
 }
 
-type StepLogFinish struct{}
+type stepLogFinish struct{}
 
-func (e StepLogFinish) Exec(req *ExecutionRequest) bool {
+func (e stepLogFinish) Exec(req *ExecutionRequest) bool {
 	log.WithFields(log.Fields{
 		"title":    req.action.Title,
 		"stdout":   req.logEntry.Stdout,
@@ -157,9 +166,9 @@ func (e StepLogFinish) Exec(req *ExecutionRequest) bool {
 	return true
 }
 
-type StepParseArgs struct{}
+type stepParseArgs struct{}
 
-func (e StepParseArgs) Exec(req *ExecutionRequest) bool {
+func (e stepParseArgs) Exec(req *ExecutionRequest) bool {
 	var err error
 
 	req.finalParsedCommand, err = parseActionArguments(req.action.Shell, req.Arguments, req.action)
@@ -175,9 +184,9 @@ func (e StepParseArgs) Exec(req *ExecutionRequest) bool {
 	return true
 }
 
-type StepExec struct{}
+type stepExec struct{}
 
-func (e StepExec) Exec(req *ExecutionRequest) bool {
+func (e stepExec) Exec(req *ExecutionRequest) bool {
 	ctx, cancel := context.WithTimeout(context.Background(), time.Duration(req.action.Timeout)*time.Second)
 	defer cancel()
 
@@ -259,6 +268,9 @@ func typecheckChoice(value string, arg *config.ActionArgument) error {
 	return errors.New("Arg value is not one of the predefined choices")
 }
 
+// TypeSafetyCheck checks argument values match a specific type. The types are
+// defined in typecheckRegex, and, you guessed it, uses regex to check for allowed
+// characters.
 func TypeSafetyCheck(name string, value string, typ string) error {
 	pattern, found := typecheckRegex[typ]
 

internal/executor/executor_test.go

@@ -21,7 +21,7 @@ func testingExecutor() (*Executor, *config.Config) {
 		Title: "Do some tickles",
 		Shell: "echo 'Tickling {{ person }}'",
 		Arguments: []config.ActionArgument{
-			config.ActionArgument{
+			{
 				Name: "person",
 				Type: "ascii",
 			},
@@ -29,7 +29,7 @@ func testingExecutor() (*Executor, *config.Config) {
 	}
 
 	cfg.Actions = append(cfg.Actions, a1)
-	config.Sanitize(cfg)
+	cfg.Sanitize()
 
 	return e, cfg
 }

internal/updatecheck/updateCheck.go

@@ -21,7 +21,10 @@ type updateRequest struct {
 	MachineID      string
 }
 
+// AvailableVersion is updated when checking with the update service.
 var AvailableVersion = "none"
+
+// CurrentVersion is set by the main cmd (which is in tern set as a compile constant)
 var CurrentVersion = "?"
 
 func machineID() string {