feat(preflight): adding warning message when validating the content of preflight and host preflight spec (#1250)

pkg/preflight/run.go

@@ -47,6 +47,13 @@ func RunPreflights(interactive bool, output string, format string, args []string
 		return err
 	}
 
+	warning := validatePreflight(specs)
+
+	if warning != nil {
+		fmt.Println(warning.Warning())
+		return nil
+	}
+
 	var collectResults []CollectResult
 	var uploadCollectResults []CollectResult
 	preflightSpecName := ""

pkg/preflight/validate_specs.go

@@ -0,0 +1,152 @@
+package preflight
+
+import (
+	"reflect"
+
+	"github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
+	"github.com/replicatedhq/troubleshoot/pkg/multitype"
+	"github.com/replicatedhq/troubleshoot/pkg/types"
+)
+
+// validatePreflight validates the preflight spec and returns a warning if there is any
+func validatePreflight(specs PreflightSpecs) *types.ExitCodeWarning {
+
+	if specs.PreflightSpec == nil && specs.HostPreflightSpec == nil {
+		return types.NewExitCodeWarning("no preflight or host preflight spec was found")
+	}
+
+	if specs.PreflightSpec != nil {
+		warning := validatePreflightSpecItems(specs.PreflightSpec.Spec.Collectors, specs.PreflightSpec.Spec.Analyzers)
+		if warning != nil {
+			return warning
+		}
+	}
+
+	if specs.HostPreflightSpec != nil {
+		warning := validateHostPreflightSpecItems(specs.HostPreflightSpec.Spec.Collectors, specs.HostPreflightSpec.Spec.Analyzers)
+		if warning != nil {
+			return warning
+		}
+	}
+
+	return nil
+}
+
+// validatePreflightSpecItems validates the preflight spec items and returns a warning if there is any
+// clusterResources and clusterInfo collectors are added automatically to the preflight spec, cannot be excluded
+func validatePreflightSpecItems(collectors []*v1beta2.Collect, analyzers []*v1beta2.Analyze) *types.ExitCodeWarning {
+	var numberOfExcludedCollectors, numberOfExcludedAnalyzers int
+	var numberOfExcludedDefaultCollectors int
+	numberOfCollectors := len(collectors) + 2
+	numberOfAnalyzers := len(analyzers)
+
+	if numberOfCollectors >= 0 {
+		collectorsInterface := make([]interface{}, len(collectors))
+		for i, v := range collectors {
+			if v.ClusterInfo != nil || v.ClusterResources != nil {
+				numberOfExcludedDefaultCollectors++
+			}
+			collectorsInterface[i] = v
+		}
+
+		numberOfExcludedCollectors += countExcludedItems(collectorsInterface)
+
+		if numberOfExcludedCollectors+numberOfExcludedDefaultCollectors == numberOfCollectors {
+			return types.NewExitCodeWarning("All collectors were excluded by the applied values")
+		}
+	}
+
+	// if there are no analyzers, return a warning
+	// else check if all analyzers are excluded
+	if numberOfAnalyzers == 0 {
+		return types.NewExitCodeWarning("No analyzers found")
+	} else {
+		analyzersInterface := make([]interface{}, len(analyzers))
+		for i, v := range analyzers {
+			analyzersInterface[i] = v
+		}
+
+		numberOfExcludedAnalyzers = countExcludedItems(analyzersInterface)
+
+		if numberOfExcludedAnalyzers == numberOfAnalyzers {
+			return types.NewExitCodeWarning("All analyzers were excluded by the applied values")
+		}
+	}
+	return nil
+}
+
+// validateHostPreflightSpecItems validates the host preflight spec items and returns a warning if there is any
+// no collectors are added or excluded automatically to the host preflight spec
+func validateHostPreflightSpecItems(collectors []*v1beta2.HostCollect, analyzers []*v1beta2.HostAnalyze) *types.ExitCodeWarning {
+	var numberOfExcludedCollectors, numberOfExcludedAnalyzers int
+	numberOfCollectors := len(collectors)
+	numberOfAnalyzers := len(analyzers)
+
+	// if there are no collectors, return a warning
+	if numberOfCollectors == 0 {
+		return types.NewExitCodeWarning("No collectors found")
+	}
+
+	// if there are no analyzers, return a warning
+	if numberOfAnalyzers == 0 {
+		return types.NewExitCodeWarning("No analyzers found")
+	}
+
+	collectorsInterface := make([]interface{}, len(collectors))
+	for i, v := range collectors {
+		collectorsInterface[i] = v
+	}
+
+	analyzersInterface := make([]interface{}, len(analyzers))
+	for i, v := range analyzers {
+		analyzersInterface[i] = v
+	}
+
+	numberOfExcludedCollectors = countExcludedItems(collectorsInterface)
+	numberOfExcludedAnalyzers = countExcludedItems(analyzersInterface)
+
+	if numberOfExcludedCollectors == numberOfCollectors {
+		return types.NewExitCodeWarning("All collectors were excluded by the applied values")
+	}
+
+	if numberOfExcludedAnalyzers == numberOfAnalyzers {
+		return types.NewExitCodeWarning("All analyzers were excluded by the applied values")
+	}
+
+	return nil
+}
+
+// countExcludedItems counts and returns the number of excluded items in the given items slice.
+// Items are assumed to be structures that may have an "Exclude" field as bool
+// If the "Exclude" field is true, the item is considered excluded.
+func countExcludedItems(items []interface{}) int {
+	numberOfExcludedItems := 0
+	for _, item := range items {
+		itemElem := reflect.ValueOf(item).Elem()
+
+		// Loop over all fields of the current item.
+		for i := 0; i < itemElem.NumField(); i++ {
+			// Get the value of the current field.
+			itemValue := itemElem.Field(i)
+			// If the current field is a pointer to a struct, check if it has an "Exclude" field.
+			if !itemValue.IsNil() {
+				elem := itemValue.Elem()
+				if elem.Kind() == reflect.Struct {
+					// Look for a field named "Exclude" in the struct.
+					excludeField := elem.FieldByName("Exclude")
+					if excludeField.IsValid() {
+						// Try to get the field's value as a *multitype.BoolOrString.
+						excludeValue, ok := excludeField.Interface().(*multitype.BoolOrString)
+						// If the field's value was successfully obtained and is not nil, and the value is true
+						if ok && excludeValue != nil {
+							if excludeValue.BoolOrDefaultFalse() {
+								numberOfExcludedItems++
+							}
+						}
+					}
+				}
+			}
+		}
+	}
+	return numberOfExcludedItems
+}

pkg/preflight/validate_specs_test.go

@@ -0,0 +1,97 @@
+package preflight
+
+import (
+	"path/filepath"
+	"testing"
+
+	"github.com/replicatedhq/troubleshoot/internal/testutils"
+	"github.com/replicatedhq/troubleshoot/pkg/types"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestValidatePreflight(t *testing.T) {
+	testingFiles := map[string]string{
+		"noCollectorsPreflightFile":                 "troubleshoot_v1beta2_preflight_validate_empty_collectors_gotest.yaml",
+		"noAnalyzersPreflightFile":                  "troubleshoot_v1beta2_preflight_validate_empty_analyzers_gotest.yaml",
+		"excludedAllDefaultCollectorsPreflightFile": "troubleshoot_v1beta2_preflight_validate_excluded_all_default_collectors_gotest.yaml",
+		"excludedOneDefaultCollectorsPreflightFile": "troubleshoot_v1beta2_preflight_validate_excluded_one_default_collectors_gotest.yaml",
+		"excludedAllNonCollectorsPreflightFile":     "troubleshoot_v1beta2_preflight_validate_excluded_all_non_default_collectors_gotest.yaml",
+		"excludedAnalyzersPreflightFile":            "troubleshoot_v1beta2_preflight_validate_excluded_analyzers_gotest.yaml",
+		"noCollectorsHostPreflightFile":             "troubleshoot_v1beta2_host_preflight_validate_empty_collectors_gotest.yaml",
+		"noAnalyzersHostPreflightFile":              "troubleshoot_v1beta2_host_preflight_validate_empty_analyzers_gotest.yaml",
+		"excludedHostCollectorsPreflightFile":       "troubleshoot_v1beta2_host_preflight_validate_excluded_collectors_gotest.yaml",
+		"excludedHostAnalyzersPreflightFile":        "troubleshoot_v1beta2_host_preflight_validate_excluded_analyzers_gotest.yaml",
+	}
+
+	tests := []struct {
+		name          string
+		preflightSpec string
+		wantWarning   *types.ExitCodeWarning
+	}{
+		{
+			name:          "empty-preflight",
+			preflightSpec: "",
+			wantWarning:   types.NewExitCodeWarning("no preflight or host preflight spec was found"),
+		},
+		{
+			name:          "no-collectores",
+			preflightSpec: testingFiles["noCollectorsPreflightFile"],
+			wantWarning:   nil,
+		},
+		{
+			name:          "no-analyzers",
+			preflightSpec: testingFiles["noAnalyzersPreflightFile"],
+			wantWarning:   types.NewExitCodeWarning("No analyzers found"),
+		},
+		{
+			name:          "excluded-all-default-collectors",
+			preflightSpec: testingFiles["excludedAllDefaultCollectorsPreflightFile"],
+			wantWarning:   types.NewExitCodeWarning("All collectors were excluded by the applied values"),
+		},
+		{
+			name:          "excluded-one-default-collectors",
+			preflightSpec: testingFiles["excludedOneDefaultCollectorsPreflightFile"],
+			wantWarning:   nil,
+		},
+		{
+			name:          "excluded-all-non-default-collectors",
+			preflightSpec: testingFiles["excludedAllNonCollectorsPreflightFile"],
+			wantWarning:   nil,
+		},
+		{
+			name:          "excluded-analyzers",
+			preflightSpec: testingFiles["excludedAnalyzersPreflightFile"],
+			wantWarning:   types.NewExitCodeWarning("All analyzers were excluded by the applied values"),
+		},
+		{
+			name:          "no-host-preflight-collectores",
+			preflightSpec: testingFiles["noCollectorsHostPreflightFile"],
+			wantWarning:   types.NewExitCodeWarning("No collectors found"),
+		},
+		{
+			name:          "no-host-preflight-analyzers",
+			preflightSpec: testingFiles["noAnalyzersHostPreflightFile"],
+			wantWarning:   types.NewExitCodeWarning("No analyzers found"),
+		},
+		{
+			name:          "excluded-host-preflight-collectors",
+			preflightSpec: testingFiles["excludedHostCollectorsPreflightFile"],
+			wantWarning:   types.NewExitCodeWarning("All collectors were excluded by the applied values"),
+		},
+		{
+			name:          "excluded-host-preflight-analyzers",
+			preflightSpec: testingFiles["excludedHostAnalyzersPreflightFile"],
+			wantWarning:   types.NewExitCodeWarning("All analyzers were excluded by the applied values"),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			testFilePath := filepath.Join(testutils.FileDir(), "../../testdata/preflightspec/"+tt.preflightSpec)
+			specs := PreflightSpecs{}
+			specs.Read([]string{testFilePath})
+			gotWarning := validatePreflight(specs)
+			assert.Equal(t, tt.wantWarning, gotWarning)
+		})
+	}
+}

pkg/types/types.go

@@ -1,5 +1,7 @@
 package types
 
+import "fmt"
+
 type NotFoundError struct {
 	Name string
 }
@@ -18,6 +20,10 @@ type ExitCodeError struct {
 	Code int
 }
 
+type ExitCodeWarning struct {
+	Msg string
+}
+
 func (e *ExitCodeError) Error() string {
 	return e.Msg
 }
@@ -33,3 +39,11 @@ func NewExitCodeError(exitCode int, theErr error) *ExitCodeError {
 	}
 	return &ExitCodeError{Msg: useErr, Code: exitCode}
 }
+
+func NewExitCodeWarning(theErrMsg string) *ExitCodeWarning {
+	return &ExitCodeWarning{Msg: theErrMsg}
+}
+
+func (e *ExitCodeWarning) Warning() string {
+	return fmt.Sprintf("Warning: %s", e.Msg)
+}

testdata/preflightspec/troubleshoot_v1beta2_host_preflight_validate_empty_analyzers_gotest.yaml

@@ -0,0 +1,8 @@
+apiVersion: troubleshoot.sh/v1beta2
+kind: HostPreflight
+metadata:
+  name: sample
+spec:
+  collectors:
+    - cpu: {}
+  analyzers: []

testdata/preflightspec/troubleshoot_v1beta2_host_preflight_validate_empty_collectors_gotest.yaml

@@ -0,0 +1,14 @@
+apiVersion: troubleshoot.sh/v1beta2
+kind: HostPreflight
+metadata:
+  name: sample
+spec:
+  analyzers:
+    - hostOS:
+        outcomes:
+          - fail:
+              when: "ubuntu-16.04-kernel < 4.15"
+              message: unsupported distribution
+          - pass:
+              when: "ubuntu-18.04-kernel >= 4.15"
+              message: supported distribution

testdata/preflightspec/troubleshoot_v1beta2_host_preflight_validate_excluded_analyzers_gotest.yaml

@@ -0,0 +1,50 @@
+apiVersion: troubleshoot.sh/v1beta2
+kind: HostPreflight
+metadata:
+  name: sample
+spec:
+  collectors:
+    - cpu: {}
+    - diskUsage:
+        collectorName: ephemeral
+        path: /var/lib/kubelet
+  analyzers:
+    - cpu:
+        exclude: true
+        outcomes:
+          - fail:
+              when: "physical < 4"
+              message: At least 4 physical CPU cores are required
+          - fail:
+              when: "logical < 8"
+              message: At least 8 CPU cores are required
+          - warn:
+              when: "count < 16"
+              message: At least 16 CPU cores preferred
+          - pass:
+              message: This server has sufficient CPU cores
+    - diskUsage:
+        exclude: true
+        collectorName: ephemeral
+        outcomes:
+          - fail:
+              when: "total < 20Gi"
+              message: /var/lib/kubelet has less than 20Gi of total space
+          - fail:
+              when: "available < 10Gi"
+              message: /var/lib/kubelet has less than 10Gi of disk space available
+          - fail:
+              when: "used/total > 70%"
+              message: /var/lib/kubelet is more than 70% full
+          - warn:
+              when: "total < 40Gi"
+              message: /var/lib/kubelet has less than 40Gi of total space
+          - warn:
+              when: "used/total > 60%"
+              message: /var/lib/kubelet is more than 60% full
+          - pass:
+              when: "available/total >= 90%"
+              message: /var/lib/kubelet has more than 90% available
+          - pass:
+              message: /var/lib/kubelet has sufficient disk space available
+

testdata/preflightspec/troubleshoot_v1beta2_host_preflight_validate_excluded_collectors_gotest.yaml

@@ -0,0 +1,50 @@
+apiVersion: troubleshoot.sh/v1beta2
+kind: HostPreflight
+metadata:
+  name: sample
+spec:
+  collectors:
+    - cpu:
+        exclude: true
+    - diskUsage:
+        exclude: true
+        collectorName: ephemeral
+        path: /var/lib/kubelet
+  analyzers:
+    - cpu:
+        outcomes:
+          - fail:
+              when: "physical < 4"
+              message: At least 4 physical CPU cores are required
+          - fail:
+              when: "logical < 8"
+              message: At least 8 CPU cores are required
+          - warn:
+              when: "count < 16"
+              message: At least 16 CPU cores preferred
+          - pass:
+              message: This server has sufficient CPU cores
+    - diskUsage:
+        collectorName: ephemeral
+        outcomes:
+          - fail:
+              when: "total < 20Gi"
+              message: /var/lib/kubelet has less than 20Gi of total space
+          - fail:
+              when: "available < 10Gi"
+              message: /var/lib/kubelet has less than 10Gi of disk space available
+          - fail:
+              when: "used/total > 70%"
+              message: /var/lib/kubelet is more than 70% full
+          - warn:
+              when: "total < 40Gi"
+              message: /var/lib/kubelet has less than 40Gi of total space
+          - warn:
+              when: "used/total > 60%"
+              message: /var/lib/kubelet is more than 60% full
+          - pass:
+              when: "available/total >= 90%"
+              message: /var/lib/kubelet has more than 90% available
+          - pass:
+              message: /var/lib/kubelet has sufficient disk space available
+

testdata/preflightspec/troubleshoot_v1beta2_preflight_validate_empty_analyzers_gotest.yaml

@@ -0,0 +1,8 @@
+apiVersion: troubleshoot.sh/v1beta2
+kind: Preflight
+metadata:
+  name: sample
+spec:
+  collectors:
+    - clusterInfo: {}
+  analyzers: []

testdata/preflightspec/troubleshoot_v1beta2_preflight_validate_empty_collectors_gotest.yaml

@@ -0,0 +1,11 @@
+apiVersion: troubleshoot.sh/v1beta2
+kind: Preflight
+metadata:
+  name: sample
+spec:
+  collectors: []
+  analyzers:
+    - clusterVersion:
+        outcomes:
+        - pass:
+            message: Cluster is up to date

testdata/preflightspec/troubleshoot_v1beta2_preflight_validate_excluded_all_default_collectors_gotest.yaml

@@ -0,0 +1,15 @@
+apiVersion: troubleshoot.sh/v1beta2
+kind: Preflight
+metadata:
+  name: sample
+spec:
+  collectors:
+    - clusterInfo: 
+        exclude: true
+    - clusterResources: 
+        exclude: true
+  analyzers:
+    - clusterVersion:
+        outcomes:
+        - pass:
+            message: Cluster is up to date

testdata/preflightspec/troubleshoot_v1beta2_preflight_validate_excluded_all_non_default_collectors_gotest.yaml

@@ -0,0 +1,30 @@
+apiVersion: troubleshoot.sh/v1beta2
+kind: Preflight
+metadata:
+  name: sample
+spec:
+  collectors:
+    - http:
+        exclude: true
+        name: healthz
+        get:
+          url: http://api:3000/healthz
+    - data:
+        exclude: true
+        collectorName: my-password-dump
+        name: data
+        data: |
+          my super secret password is abc123
+          another redaction will go here
+  analyzers:
+    - distribution:
+        outcomes:
+          - pass:
+              when: "== k3s"
+              message: K3S is a supported distribution
+          - warn:
+              message: Unable to determine the distribution of Kubernetes
+    - clusterVersion:
+        outcomes:
+        - pass:
+            message: Cluster is up to date

testdata/preflightspec/troubleshoot_v1beta2_preflight_validate_excluded_analyzers_gotest.yaml

@@ -0,0 +1,22 @@
+apiVersion: troubleshoot.sh/v1beta2
+kind: Preflight
+metadata:
+  name: sample
+spec:
+  collectors:
+    - clusterInfo: {}
+    - clusterResources: {}
+  analyzers:
+    - distribution:
+        exclude: true
+        outcomes:
+          - pass:
+              when: "== k3s"
+              message: K3S is a supported distribution
+          - warn:
+              message: Unable to determine the distribution of Kubernetes
+    - clusterVersion:
+        exclude: true
+        outcomes:
+        - pass:
+            message: Cluster is up to date

testdata/preflightspec/troubleshoot_v1beta2_preflight_validate_excluded_one_default_collectors_gotest.yaml

@@ -0,0 +1,13 @@
+apiVersion: troubleshoot.sh/v1beta2
+kind: Preflight
+metadata:
+  name: sample
+spec:
+  collectors:
+    - clusterInfo: 
+        exclude: true
+  analyzers:
+    - clusterVersion:
+        outcomes:
+        - pass:
+            message: Cluster is up to date