Make all the internal validations ... internal

The main is doing flag validation through pflags, then
did further validation by involving the constructors.

With the recent refactor on the commit "Refactor constructors"
in this branch, we moved away from that pattern.

However, it means we reintroduced a log dependency into our
external API, and the external API now had extra validations
regardless of the type.

This is unnecessary, so I moved away from that pattern, and
moved back all the validation into a central place, internal,
which is only doing what kured would desire, without exposing
it to users. The users could still theoretically use the proper
constructors for each type, as they would validate just fine.

The only thing they would lose is the kured internal decision
of validation/precedence.

Signed-off-by: Jean-Philippe Evrard <[email protected]>

cmd/kured/main.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"github.com/kubereboot/kured/internal"
 	"github.com/kubereboot/kured/pkg/blockers"
 	"github.com/kubereboot/kured/pkg/checkers"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
@@ -17,7 +18,9 @@ import (
 	"strings"
 	"time"
 
+	"github.com/containrrr/shoutrrr"
 	papi "github.com/prometheus/client_golang/api"
+	"github.com/prometheus/client_golang/prometheus"
 	log "github.com/sirupsen/logrus"
 	flag "github.com/spf13/pflag"
 	v1 "k8s.io/api/core/v1"
@@ -27,14 +30,12 @@ import (
 	"k8s.io/client-go/rest"
 	kubectldrain "k8s.io/kubectl/pkg/drain"
 
-	shoutrrr "github.com/containrrr/shoutrrr"
 	"github.com/kubereboot/kured/pkg/alerts"
 	"github.com/kubereboot/kured/pkg/daemonsetlock"
 	"github.com/kubereboot/kured/pkg/delaytick"
 	"github.com/kubereboot/kured/pkg/reboot"
 	"github.com/kubereboot/kured/pkg/taints"
 	"github.com/kubereboot/kured/pkg/timewindow"
-	"github.com/prometheus/client_golang/prometheus"
 )
 
 var (
@@ -243,12 +244,12 @@ func main() {
 	log.Infof("Reboot schedule: %v", window)
 
 	log.Infof("Reboot method: %s", rebootMethod)
-	rebooter, err := reboot.NewRebooter(rebootMethod, rebootCommand, rebootSignal)
+	rebooter, err := internal.NewRebooter(rebootMethod, rebootCommand, rebootSignal)
 	if err != nil {
 		log.Fatalf("Failed to build rebooter: %v", err)
 	}
 
-	rebootChecker, err := checkers.NewRebootChecker(rebootSentinelCommand, rebootSentinelFile)
+	rebootChecker, err := internal.NewRebootChecker(rebootSentinelCommand, rebootSentinelFile)
 	if err != nil {
 		log.Fatalf("Failed to build reboot checker: %v", err)
 	}

internal/validators.go

@@ -0,0 +1,35 @@
+package internal
+
+import (
+	"fmt"
+	"github.com/kubereboot/kured/pkg/checkers"
+	"github.com/kubereboot/kured/pkg/reboot"
+	log "github.com/sirupsen/logrus"
+)
+
+// NewRebooter validates the rebootMethod, rebootCommand, and rebootSignal input,
+// then chains to the right constructor.
+func NewRebooter(rebootMethod string, rebootCommand string, rebootSignal int) (reboot.Rebooter, error) {
+	switch {
+	case rebootMethod == "command":
+		log.Infof("Reboot command: %s", rebootCommand)
+		return reboot.NewCommandRebooter(rebootCommand)
+	case rebootMethod == "signal":
+		log.Infof("Reboot signal: %d", rebootSignal)
+		return reboot.NewSignalRebooter(rebootSignal)
+	default:
+		return nil, fmt.Errorf("invalid reboot-method configured %s, expected signal or command", rebootMethod)
+	}
+}
+
+// NewRebootChecker validates the rebootSentinelCommand, rebootSentinelFile input,
+// then chains to the right constructor.
+func NewRebootChecker(rebootSentinelCommand string, rebootSentinelFile string) (checkers.Checker, error) {
+	// An override of rebootSentinelCommand means a privileged command
+	if rebootSentinelCommand != "" {
+		log.Infof("Sentinel checker is (privileged) user provided command: %s", rebootSentinelCommand)
+		return checkers.NewCommandChecker(rebootSentinelCommand)
+	}
+	log.Infof("Sentinel checker is (unprivileged) testing for the presence of: %s", rebootSentinelFile)
+	return checkers.NewFileRebootChecker(rebootSentinelFile)
+}

pkg/checkers/checker.go

@@ -23,16 +23,6 @@ type FileRebootChecker struct {
 	FilePath string
 }
 
-func NewRebootChecker(rebootSentinelCommand string, rebootSentinelFile string) (Checker, error) {
-	// An override of rebootSentinelCommand means a privileged command
-	if rebootSentinelCommand != "" {
-		log.Infof("Sentinel checker is (privileged) user provided command: %s", rebootSentinelCommand)
-		return NewCommandChecker(rebootSentinelCommand)
-	}
-	log.Infof("Sentinel checker is (unprivileged) testing for the presence of: %s", rebootSentinelFile)
-	return NewFileRebootChecker(rebootSentinelFile)
-}
-
 // RebootRequired checks the file presence
 // needs refactoring to also return an error, instead of leaking it inside the code.
 // This needs refactoring to get rid of NewCommand

pkg/reboot/reboot.go

@@ -1,30 +1,9 @@
 package reboot
 
-import (
-	"fmt"
-	log "github.com/sirupsen/logrus"
-)
-
 // Rebooter is the standard interface to use to execute
 // the reboot, after it has been considered as necessary.
 // The Reboot method does not expect any return, yet should
 // most likely be refactored in the future to return an error
 type Rebooter interface {
 	Reboot() error
 }
-
-// NewRebooter validates the rebootMethod, rebootCommand, and rebootSignal input,
-// then chains to the right constructor. This can be made internal later, as
-// only the real rebooters constructors should be public (by opposition to this one)
-func NewRebooter(rebootMethod string, rebootCommand string, rebootSignal int) (Rebooter, error) {
-	switch {
-	case rebootMethod == "command":
-		log.Infof("Reboot command: %s", rebootCommand)
-		return NewCommandRebooter(rebootCommand)
-	case rebootMethod == "signal":
-		log.Infof("Reboot signal: %d", rebootSignal)
-		return NewSignalRebooter(rebootSignal)
-	default:
-		return nil, fmt.Errorf("invalid reboot-method configured %s, expected signal or command", rebootMethod)
-	}
-}