fix: return true when one of analyzers strict field is true (#552)

* fix: return true when one of analyzers is true

* fix: return true when one of analyzers is true

* update: add unit test

* update: log errors while processing analyzers

* fix: return parse error instead of logging

* fix: update err message

* fix: evaluate strict check err separately

Author: Pavan Sokke Nagaraj

pkg/preflight/analyze.go

@@ -8,6 +8,7 @@ import (
 
 	analyze "github.com/replicatedhq/troubleshoot/pkg/analyze"
 	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
+	"github.com/replicatedhq/troubleshoot/pkg/logger"
 )
 
 // Analyze runs the analyze phase of preflight checks
@@ -77,7 +78,11 @@ func doAnalyze(allCollectedData map[string][]byte, analyzers []*troubleshootv1be
 	for _, analyzer := range analyzers {
 		analyzeResult, err := analyze.Analyze(analyzer, getCollectedFileContents, getChildCollectedFileContents)
 		if err != nil {
-			strict, _ := HasStrictAnalyzer(analyzer)
+			strict, strictErr := HasStrictAnalyzer(analyzer)
+			if strictErr != nil {
+				logger.Printf("failed to determine if analyzer %v is strict: %s", analyzer, strictErr)
+			}
+
 			analyzeResult = []*analyze.AnalyzeResult{
 				{
 					Strict:  strict,

pkg/preflight/util.go

@@ -26,7 +26,13 @@ func HasStrictAnalyzers(preflight *troubleshootv1beta2.Preflight) (bool, error)
 
 	// analyzerMap will ignore empty Analyzers and loop around Analyzer with data
 	for _, analyzers := range analyzersMap { // for each analyzer: map["clusterVersion": map[string]interface{} ["exclude": "", "strict": "true", "outcomes": nil]
-		return hasStrictAnalyzer(analyzers)
+		hasStrictAnalyzer, err := hasStrictAnalyzer(analyzers)
+		if err != nil {
+			return false, errors.Wrap(err, "failed to check if analyzer has strict:true")
+		}
+		if hasStrictAnalyzer {
+			return true, nil
+		}
 	}
 	return false, nil
 }
@@ -58,7 +64,9 @@ func hasStrictAnalyzer(analyzerMap map[string]interface{}) (bool, error) {
 		if err != nil {
 			return false, errors.Wrap(err, "error while un-marshalling marshalledAnalyzers")
 		}
-		return analyzeMeta.Strict.BoolOrDefaultFalse(), nil
+		if analyzeMeta.Strict.BoolOrDefaultFalse() {
+			return true, nil
+		}
 	}
 	return false, nil
 }

pkg/preflight/util_test.go

@@ -190,6 +190,53 @@ func TestHasStrictAnalyzers(t *testing.T) {
 			},
 			want:    false,
 			wantErr: false,
+		}, {
+			name: "expect true when preflight spec's analyzer has analyzer with strict true in one of multiple analyzers",
+			preflight: &troubleshootv1beta2.Preflight{
+				Spec: troubleshootv1beta2.PreflightSpec{
+					Analyzers: []*troubleshootv1beta2.Analyze{
+						{
+							ClusterVersion: &troubleshootv1beta2.ClusterVersion{AnalyzeMeta: analyzeMetaStrictFalseInt},
+						}, {
+							ClusterVersion: &troubleshootv1beta2.ClusterVersion{AnalyzeMeta: analyzeMetaStrictTrueBool},
+						},
+					},
+				},
+			},
+			want:    true,
+			wantErr: false,
+		}, {
+			name: "expect true when preflight spec's analyzer has analyzer with strict true in one of multiple analyzers",
+			preflight: &troubleshootv1beta2.Preflight{
+				Spec: troubleshootv1beta2.PreflightSpec{
+					Analyzers: []*troubleshootv1beta2.Analyze{
+						{
+							ClusterVersion: &troubleshootv1beta2.ClusterVersion{AnalyzeMeta: analyzeMetaStrictFalseInt},
+						}, {
+							ClusterVersion: &troubleshootv1beta2.ClusterVersion{AnalyzeMeta: analyzeMetaStrictTrueBool},
+						}, {
+							ClusterVersion: &troubleshootv1beta2.ClusterVersion{AnalyzeMeta: analyzeMetaStrictInvalidStr},
+						},
+					},
+				},
+			},
+			want:    true,
+			wantErr: false,
+		}, {
+			name: "expect true when preflight spec's analyzer has analyzer with strict true in one of multiple analyzers",
+			preflight: &troubleshootv1beta2.Preflight{
+				Spec: troubleshootv1beta2.PreflightSpec{
+					Analyzers: []*troubleshootv1beta2.Analyze{
+						{
+							ClusterVersion: &troubleshootv1beta2.ClusterVersion{AnalyzeMeta: analyzeMetaStrictFalseInt},
+							StorageClass:   &troubleshootv1beta2.StorageClass{AnalyzeMeta: analyzeMetaStrictFalseInt},
+							Secret:         &troubleshootv1beta2.AnalyzeSecret{AnalyzeMeta: analyzeMetaStrictTrueBool},
+						},
+					},
+				},
+			},
+			want:    true,
+			wantErr: false,
 		},
 	}
 	for _, tt := range tests {
@@ -298,7 +345,7 @@ func TestHasStrictAnalyzer(t *testing.T) {
 			want:    false,
 			wantErr: false,
 		}, {
-			name: "expect strict=false, err=nil when ClusterVersion analyzer has strict=true",
+			name: "expect strict=true, err=nil when ClusterVersion analyzer has strict=true",
 			analyzer: &troubleshootv1beta2.Analyze{
 				ClusterVersion: &troubleshootv1beta2.ClusterVersion{AnalyzeMeta: analyzeMetaStrictTrueInt},
 			},
@@ -311,6 +358,15 @@ func TestHasStrictAnalyzer(t *testing.T) {
 			},
 			want:    false,
 			wantErr: false,
+		}, {
+			name: "expect strict=true, err=nil when one of the analyzers has strict=true",
+			analyzer: &troubleshootv1beta2.Analyze{
+				ClusterVersion: &troubleshootv1beta2.ClusterVersion{AnalyzeMeta: analyzeMetaStrictFalseInt},
+				StorageClass:   &troubleshootv1beta2.StorageClass{AnalyzeMeta: analyzeMetaStrictFalseInt},
+				Secret:         &troubleshootv1beta2.AnalyzeSecret{AnalyzeMeta: analyzeMetaStrictTrueBool},
+			},
+			want:    true,
+			wantErr: false,
 		},
 	}
 	for _, tt := range tests {