Cleanup Part 1 - First refactors

cmd/kured/main_test.go

@@ -3,61 +3,29 @@ package main
 import (
 	"reflect"
 	"testing"
-
-	log "github.com/sirupsen/logrus"
-	"github.com/spf13/cobra"
-	"github.com/kubereboot/kured/pkg/alerts"
-	assert "gotest.tools/v3/assert"
-
-	papi "github.com/prometheus/client_golang/api"
 )
 
-type BlockingChecker struct {
-	blocking bool
-}
-
-func (fbc BlockingChecker) isBlocked() bool {
-	return fbc.blocking
-}
-
-var _ RebootBlocker = BlockingChecker{}       // Verify that Type implements Interface.
-var _ RebootBlocker = (*BlockingChecker)(nil) // Verify that *Type implements Interface.
-
-func Test_flagCheck(t *testing.T) {
-	var cmd *cobra.Command
-	var args []string
-	slackHookURL = "https://hooks.slack.com/services/BLABLABA12345/IAM931A0VERY/COMPLICATED711854TOKEN1SET"
-	expected := "slack://BLABLABA12345/IAM931A0VERY/COMPLICATED711854TOKEN1SET"
-	flagCheck(cmd, args)
-	if notifyURL != expected {
-		t.Errorf("Slack URL Parsing is wrong: expecting %s  but got %s\n", expected, notifyURL)
-	}
+func TestValidateNotificationURL(t *testing.T) {
 
-	// validate that surrounding quotes are stripped
-	slackHookURL = "\"https://hooks.slack.com/services/BLABLABA12345/IAM931A0VERY/COMPLICATED711854TOKEN1SET\""
-	expected = "slack://BLABLABA12345/IAM931A0VERY/COMPLICATED711854TOKEN1SET"
-	flagCheck(cmd, args)
-	if notifyURL != expected {
-		t.Errorf("Slack URL Parsing is wrong: expecting %s  but got %s\n", expected, notifyURL)
-	}
-	slackHookURL = "'https://hooks.slack.com/services/BLABLABA12345/IAM931A0VERY/COMPLICATED711854TOKEN1SET'"
-	expected = "slack://BLABLABA12345/IAM931A0VERY/COMPLICATED711854TOKEN1SET"
-	flagCheck(cmd, args)
-	if notifyURL != expected {
-		t.Errorf("Slack URL Parsing is wrong: expecting %s  but got %s\n", expected, notifyURL)
-	}
-	slackHookURL = ""
-	notifyURL = "\"teams://79b4XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX@acd8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX/204cXXXXXXXXXXXXXXXXXXXXXXXXXXXX/a1f8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX?host=XXXX.webhook.office.com\""
-	expected = "teams://79b4XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX@acd8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX/204cXXXXXXXXXXXXXXXXXXXXXXXXXXXX/a1f8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX?host=XXXX.webhook.office.com"
-	flagCheck(cmd, args)
-	if notifyURL != expected {
-		t.Errorf("notifyURL Parsing is wrong: expecting %s  but got %s\n", expected, notifyURL)
+	tests := []struct {
+		name         string
+		slackHookURL string
+		notifyURL    string
+		expected     string
+	}{
+		{"slackHookURL only works fine", "https://hooks.slack.com/services/BLABLABA12345/IAM931A0VERY/COMPLICATED711854TOKEN1SET", "", "slack://BLABLABA12345/IAM931A0VERY/COMPLICATED711854TOKEN1SET"},
+		{"slackHookURL and notify URL together only keeps notifyURL", "\"https://hooks.slack.com/services/BLABLABA12345/IAM931A0VERY/COMPLICATED711854TOKEN1SET\"", "teams://79b4XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX@acd8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX/204cXXXXXXXXXXXXXXXXXXXXXXXXXXXX/a1f8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX?host=XXXX.webhook.office.com", "teams://79b4XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX@acd8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX/204cXXXXXXXXXXXXXXXXXXXXXXXXXXXX/a1f8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX?host=XXXX.webhook.office.com"},
+		{"slackHookURL removes extraneous double quotes", "\"https://hooks.slack.com/services/BLABLABA12345/IAM931A0VERY/COMPLICATED711854TOKEN1SET\"", "", "slack://BLABLABA12345/IAM931A0VERY/COMPLICATED711854TOKEN1SET"},
+		{"slackHookURL removes extraneous single quotes", "'https://hooks.slack.com/services/BLABLABA12345/IAM931A0VERY/COMPLICATED711854TOKEN1SET'", "", "slack://BLABLABA12345/IAM931A0VERY/COMPLICATED711854TOKEN1SET"},
+		{"notifyURL removes extraneous double quotes", "", "\"teams://79b4XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX@acd8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX/204cXXXXXXXXXXXXXXXXXXXXXXXXXXXX/a1f8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX?host=XXXX.webhook.office.com\"", "teams://79b4XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX@acd8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX/204cXXXXXXXXXXXXXXXXXXXXXXXXXXXX/a1f8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX?host=XXXX.webhook.office.com"},
+		{"notifyURL removes extraneous single quotes", "", "'teams://79b4XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX@acd8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX/204cXXXXXXXXXXXXXXXXXXXXXXXXXXXX/a1f8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX?host=XXXX.webhook.office.com'", "teams://79b4XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX@acd8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX/204cXXXXXXXXXXXXXXXXXXXXXXXXXXXX/a1f8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX?host=XXXX.webhook.office.com"},
 	}
-	notifyURL = "'teams://79b4XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX@acd8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX/204cXXXXXXXXXXXXXXXXXXXXXXXXXXXX/a1f8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX?host=XXXX.webhook.office.com'"
-	expected = "teams://79b4XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX@acd8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX/204cXXXXXXXXXXXXXXXXXXXXXXXXXXXX/a1f8XXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX?host=XXXX.webhook.office.com"
-	flagCheck(cmd, args)
-	if notifyURL != expected {
-		t.Errorf("notifyURL Parsing is wrong: expecting %s  but got %s\n", expected, notifyURL)
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := validateNotificationURL(tt.notifyURL, tt.slackHookURL); !reflect.DeepEqual(got, tt.expected) {
+				t.Errorf("validateNotificationURL() = %v, expected %v", got, tt.expected)
+			}
+		})
 	}
 }
 
@@ -106,205 +74,3 @@ func Test_stripQuotes(t *testing.T) {
 		})
 	}
 }
-
-func Test_rebootBlocked(t *testing.T) {
-	noCheckers := []RebootBlocker{}
-	nonblockingChecker := BlockingChecker{blocking: false}
-	blockingChecker := BlockingChecker{blocking: true}
-
-	// Instantiate a prometheusClient with a broken_url
-	promClient, err := alerts.NewPromClient(papi.Config{Address: "broken_url"})
-	if err != nil {
-		log.Fatal("Can't create prometheusClient: ", err)
-	}
-	brokenPrometheusClient := PrometheusBlockingChecker{promClient: promClient, filter: nil, firingOnly: false}
-
-	type args struct {
-		blockers []RebootBlocker
-	}
-	tests := []struct {
-		name string
-		args args
-		want bool
-	}{
-		{
-			name: "Do not block on no blocker defined",
-			args: args{blockers: noCheckers},
-			want: false,
-		},
-		{
-			name: "Ensure a blocker blocks",
-			args: args{blockers: []RebootBlocker{blockingChecker}},
-			want: true,
-		},
-		{
-			name: "Ensure a non-blocker doesn't block",
-			args: args{blockers: []RebootBlocker{nonblockingChecker}},
-			want: false,
-		},
-		{
-			name: "Ensure one blocker is enough to block",
-			args: args{blockers: []RebootBlocker{nonblockingChecker, blockingChecker}},
-			want: true,
-		},
-		{
-			name: "Do block on error contacting prometheus API",
-			args: args{blockers: []RebootBlocker{brokenPrometheusClient}},
-			want: true,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := rebootBlocked(tt.args.blockers...); got != tt.want {
-				t.Errorf("rebootBlocked() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
-func Test_buildHostCommand(t *testing.T) {
-	type args struct {
-		pid     int
-		command []string
-	}
-	tests := []struct {
-		name string
-		args args
-		want []string
-	}{
-		{
-			name: "Ensure command will run with nsenter",
-			args: args{pid: 1, command: []string{"ls", "-Fal"}},
-			want: []string{"/usr/bin/nsenter", "-m/proc/1/ns/mnt", "--", "ls", "-Fal"},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := buildHostCommand(tt.args.pid, tt.args.command); !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("buildHostCommand() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
-func Test_buildSentinelCommand(t *testing.T) {
-	type args struct {
-		rebootSentinelFile    string
-		rebootSentinelCommand string
-	}
-	tests := []struct {
-		name string
-		args args
-		want []string
-	}{
-		{
-			name: "Ensure a sentinelFile generates a shell 'test' command with the right file",
-			args: args{
-				rebootSentinelFile:    "/test1",
-				rebootSentinelCommand: "",
-			},
-			want: []string{"test", "-f", "/test1"},
-		},
-		{
-			name: "Ensure a sentinelCommand has priority over a sentinelFile if both are provided (because sentinelFile is always provided)",
-			args: args{
-				rebootSentinelFile:    "/test1",
-				rebootSentinelCommand: "/sbin/reboot-required -r",
-			},
-			want: []string{"/sbin/reboot-required", "-r"},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := buildSentinelCommand(tt.args.rebootSentinelFile, tt.args.rebootSentinelCommand); !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("buildSentinelCommand() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
-func Test_parseRebootCommand(t *testing.T) {
-	type args struct {
-		rebootCommand string
-	}
-	tests := []struct {
-		name string
-		args args
-		want []string
-	}{
-		{
-			name: "Ensure a reboot command is properly parsed",
-			args: args{
-				rebootCommand: "/sbin/systemctl reboot",
-			},
-			want: []string{"/sbin/systemctl", "reboot"},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := parseRebootCommand(tt.args.rebootCommand); !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("parseRebootCommand() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
-func Test_rebootRequired(t *testing.T) {
-	type args struct {
-		sentinelCommand []string
-	}
-	tests := []struct {
-		name string
-		args args
-		want bool
-	}{
-		{
-			name: "Ensure rc = 0 means reboot required",
-			args: args{
-				sentinelCommand: []string{"true"},
-			},
-			want: true,
-		},
-		{
-			name: "Ensure rc != 0 means reboot NOT required",
-			args: args{
-				sentinelCommand: []string{"false"},
-			},
-			want: false,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := rebootRequired(tt.args.sentinelCommand); got != tt.want {
-				t.Errorf("rebootRequired() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
-func Test_rebootRequired_fatals(t *testing.T) {
-	cases := []struct {
-		param       []string
-		expectFatal bool
-	}{
-		{
-			param:       []string{"true"},
-			expectFatal: false,
-		},
-		{
-			param:       []string{"./babar"},
-			expectFatal: true,
-		},
-	}
-
-	defer func() { log.StandardLogger().ExitFunc = nil }()
-	var fatal bool
-	log.StandardLogger().ExitFunc = func(int) { fatal = true }
-
-	for _, c := range cases {
-		fatal = false
-		rebootRequired(c.param)
-		assert.Equal(t, c.expectFatal, fatal)
-	}
-
-}

cmd/kured/pflags.go

@@ -5,27 +5,26 @@ import (
 )
 
 type regexpValue struct {
-	value **regexp.Regexp
+	*regexp.Regexp
 }
 
 func (rev *regexpValue) String() string {
-	if *rev.value == nil {
+	if rev.Regexp == nil {
 		return ""
 	}
-	return (*rev.value).String()
+	return rev.Regexp.String()
 }
 
 func (rev *regexpValue) Set(s string) error {
 	value, err := regexp.Compile(s)
 	if err != nil {
 		return err
 	}
-
-	*rev.value = value
-
+	rev.Regexp = value
 	return nil
 }
 
+// Type method returns the type of the flag as a string
 func (rev *regexpValue) Type() string {
-	return "regexp.Regexp"
+	return "regexp"
 }

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