From 42c4b8bc53c5e664ee45d1f8ea8d18d5c585b24d Mon Sep 17 00:00:00 2001 From: Jean-Philippe Evrard Date: Mon, 30 Sep 2024 21:36:29 +0200 Subject: [PATCH] Revert to use a constructor again Without this, we have no validation of the data in command/signal reboot. This was not a problem in the first refactor, as the constructor was a dummy one, without validation. However, as we refactoed, we now have code in the root method that is validation for the reboot command. This can now be encompassed in the constructor. Signed-off-by: Jean-Philippe Evrard --- cmd/kured/main.go | 12 +++--------- pkg/reboot/command.go | 15 ++++++++++++++- pkg/reboot/signal.go | 6 ++++++ 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/cmd/kured/main.go b/cmd/kured/main.go index c28a85f..abe2f53 100644 --- a/cmd/kured/main.go +++ b/cmd/kured/main.go @@ -35,7 +35,6 @@ import ( "github.com/kubereboot/kured/pkg/reboot" "github.com/kubereboot/kured/pkg/taints" "github.com/kubereboot/kured/pkg/timewindow" - "github.com/kubereboot/kured/pkg/util" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" ) @@ -722,19 +721,14 @@ func root(cmd *cobra.Command, args []string) { } log.Infof("Reboot schedule: %v", window) - restartCommand, err := shlex.Split(rebootCommand) - if err != nil { - log.Fatalf("Error parsing provided reboot command: %v", err) - } - var rebooter reboot.Rebooter switch { case rebootMethod == "command": - log.Infof("Reboot command: %s", restartCommand) - rebooter = reboot.CommandRebooter{RebootCommand: util.PrivilegedHostCommand(1, restartCommand)} + log.Infof("Reboot command: %s", rebootCommand) + rebooter = reboot.NewCommandRebooter(rebootCommand) case rebootMethod == "signal": log.Infof("Reboot signal: %v", rebootSignal) - rebooter = reboot.SignalRebooter{Signal: rebootSignal} + rebooter = reboot.NewSignalRebooter(rebootSignal) default: log.Fatalf("Invalid reboot-method configured: %s", rebootMethod) } diff --git a/pkg/reboot/command.go b/pkg/reboot/command.go index f252bd8..07c17f2 100644 --- a/pkg/reboot/command.go +++ b/pkg/reboot/command.go @@ -1,11 +1,12 @@ package reboot import ( + "github.com/google/shlex" "github.com/kubereboot/kured/pkg/util" log "github.com/sirupsen/logrus" ) -// CommandRebooter holds context-information for a command reboot. +// CommandRebooter holds context-information for a reboot with command type CommandRebooter struct { RebootCommand []string } @@ -17,3 +18,15 @@ func (c CommandRebooter) Reboot() { log.Fatalf("Error invoking reboot command: %v", err) } } + +// NewCommandRebooter is the constructor to create a CommandRebooter from a string not +// yet shell lexed. You can skip this constructor if you parse the data correctly first +// when instantiating a CommandRebooter instance. +func NewCommandRebooter(rebootCommand string) *CommandRebooter { + cmd, err := shlex.Split(rebootCommand) + if err != nil { + log.Fatalf("Error parsing provided reboot command: %v", err) + } + + return &CommandRebooter{RebootCommand: util.PrivilegedHostCommand(1, cmd)} +} diff --git a/pkg/reboot/signal.go b/pkg/reboot/signal.go index 2ec40f5..ab9adbd 100644 --- a/pkg/reboot/signal.go +++ b/pkg/reboot/signal.go @@ -28,3 +28,9 @@ func (c SignalRebooter) Reboot() { log.Fatalf("Signal of SIGRTMIN+5 failed: %v", err) } } + +// NewSignalRebooter is the constructor which sets the signal number. +// The constructor does not yet validate any input. It should be done in a later commit. +func NewSignalRebooter(sig int) *SignalRebooter { + return &SignalRebooter{Signal: sig} +}