diff --git a/pkg/analyze/secret.go b/pkg/analyze/secret.go index 32486064..7f7a6178 100644 --- a/pkg/analyze/secret.go +++ b/pkg/analyze/secret.go @@ -30,6 +30,9 @@ func (a *AnalyzeSecret) Analyze(getFile getCollectedFileContents, findFiles getC if err != nil { return nil, err } + if result == nil { + return nil, nil + } result.Strict = a.analyzer.Strict.BoolOrDefaultFalse() return []*AnalyzeResult{result}, nil } @@ -54,42 +57,68 @@ func (a *AnalyzeSecret) analyzeSecret(analyzer *troubleshootv1beta2.AnalyzeSecre return nil, err } + // The secret analyzer only supports fail (not found) and pass (found) outcomes + // per https://troubleshoot.sh/docs/analyze/secrets. If the spec contains + // neither, return an explicit error: returning (nil, nil) is swallowed by the + // Analyze wrapper into an empty result slice, so the misconfiguration would + // surface as neither a result nor an error. + // Capture fail and pass independently: a single outcome object may set both, + // so an else-if here would silently drop the second one. + var failOutcome, passOutcome *troubleshootv1beta2.SingleOutcome + for _, outcome := range analyzer.Outcomes { + if outcome.Fail != nil { + failOutcome = outcome.Fail + } + if outcome.Pass != nil { + passOutcome = outcome.Pass + } + } + if failOutcome == nil && passOutcome == nil { + return nil, fmt.Errorf("secret analyzer %s/%s must define at least one pass or fail outcome", analyzer.Namespace, analyzer.SecretName) + } + result := AnalyzeResult{ Title: a.Title(), IconKey: "kubernetes_analyze_secret", IconURI: "https://troubleshoot.sh/images/analyzer-icons/secret.svg?w=13&h=16", } - var failOutcome *troubleshootv1beta2.Outcome - for _, outcome := range analyzer.Outcomes { - if outcome.Fail != nil { - failOutcome = outcome - } + secretFound := foundSecret.SecretExists + if secretFound && analyzer.Key != "" { + secretFound = foundSecret.Key == analyzer.Key && foundSecret.KeyExists } - - if !foundSecret.SecretExists { + // Use the matched branch's configured outcome verbatim, tracking whether one + // was actually present. A configured outcome with an intentionally empty + // message (e.g. a URI-only outcome) is preserved as-is. But when the matched + // branch has NO configured outcome at all — e.g. a pass-only spec that took + // the fail path, or a fail-only spec that passed — the empty message is not + // an intentional choice, so fall back to a default diagnostic. An absent + // outcome is not the same as an intentionally empty one. + outcomeConfigured := false + if secretFound { + result.IsPass = true + if passOutcome != nil { + result.Message = passOutcome.Message + result.URI = passOutcome.URI + outcomeConfigured = true + } + } else { result.IsFail = true - result.Message = failOutcome.Fail.Message - result.URI = failOutcome.Fail.URI - - return &result, nil - } - - if analyzer.Key != "" { - if foundSecret.Key != analyzer.Key || !foundSecret.KeyExists { - result.IsFail = true - result.Message = failOutcome.Fail.Message - result.URI = failOutcome.Fail.URI - - return &result, nil + if failOutcome != nil { + result.Message = failOutcome.Message + result.URI = failOutcome.URI + outcomeConfigured = true } } - result.IsPass = true - for _, outcome := range analyzer.Outcomes { - if outcome.Pass != nil { - result.Message = outcome.Pass.Message - result.URI = outcome.Pass.URI + if !outcomeConfigured { + switch { + case result.IsPass: + result.Message = fmt.Sprintf("Secret %s was found in namespace %s", analyzer.SecretName, analyzer.Namespace) + case analyzer.Key != "" && foundSecret.SecretExists: + result.Message = fmt.Sprintf("Key %s was not found in secret %s/%s", analyzer.Key, analyzer.Namespace, analyzer.SecretName) + default: + result.Message = fmt.Sprintf("Secret %s was not found in namespace %s", analyzer.SecretName, analyzer.Namespace) } } diff --git a/pkg/analyze/secret_test.go b/pkg/analyze/secret_test.go index f8945f1e..a4f4bd8c 100644 --- a/pkg/analyze/secret_test.go +++ b/pkg/analyze/secret_test.go @@ -166,6 +166,118 @@ func Test_analyzeSecret(t *testing.T) { IconURI: "https://troubleshoot.sh/images/analyzer-icons/secret.svg?w=13&h=16", }, }, + { + name: "not found with no fail outcome falls back to a default message (no configured outcome for this branch)", + analyzer: &troubleshootv1beta2.AnalyzeSecret{ + AnalyzeMeta: troubleshootv1beta2.AnalyzeMeta{ + CheckName: "Optional Secret", + }, + Namespace: "default", + SecretName: "does-not-exist", + Outcomes: []*troubleshootv1beta2.Outcome{ + { + Pass: &troubleshootv1beta2.SingleOutcome{ + Message: "secret found", + }, + }, + }, + }, + mockFiles: map[string][]byte{ + "secrets/default/does-not-exist.json": mustJSONMarshalIndent(t, collect.SecretOutput{ + Namespace: "default", + Name: "does-not-exist", + SecretExists: false, + }), + }, + want: &AnalyzeResult{ + IsFail: true, + Message: "Secret does-not-exist was not found in namespace default", + Title: "Optional Secret", + IconKey: "kubernetes_analyze_secret", + IconURI: "https://troubleshoot.sh/images/analyzer-icons/secret.svg?w=13&h=16", + }, + }, + { + name: "key not found with no fail outcome falls back to a default message (no configured outcome for this branch)", + analyzer: &troubleshootv1beta2.AnalyzeSecret{ + AnalyzeMeta: troubleshootv1beta2.AnalyzeMeta{ + CheckName: "Optional Secret Key", + }, + Namespace: "test-namespace", + SecretName: "test-secret", + Key: "missing-key", + Outcomes: []*troubleshootv1beta2.Outcome{ + { + Pass: &troubleshootv1beta2.SingleOutcome{ + Message: "key found", + }, + }, + }, + }, + mockFiles: map[string][]byte{ + "secrets/test-namespace/test-secret/missing-key.json": mustJSONMarshalIndent(t, collect.SecretOutput{ + Namespace: "test-namespace", + Name: "test-secret", + Key: "missing-key", + SecretExists: true, + KeyExists: false, + }), + }, + want: &AnalyzeResult{ + IsFail: true, + Message: "Key missing-key was not found in secret test-namespace/test-secret", + Title: "Optional Secret Key", + IconKey: "kubernetes_analyze_secret", + IconURI: "https://troubleshoot.sh/images/analyzer-icons/secret.svg?w=13&h=16", + }, + }, + { + name: "found with only fail outcome configured falls back to the default pass message, not the fail outcome's message", + analyzer: &troubleshootv1beta2.AnalyzeSecret{ + Namespace: "test-namespace", + SecretName: "test-secret", + Outcomes: []*troubleshootv1beta2.Outcome{ + { + Fail: &troubleshootv1beta2.SingleOutcome{ + Message: "Not found", + }, + }, + }, + }, + mockFiles: map[string][]byte{ + "secrets/test-namespace/test-secret.json": mustJSONMarshalIndent(t, collect.SecretOutput{ + Namespace: "test-namespace", + Name: "test-secret", + SecretExists: true, + }), + }, + want: &AnalyzeResult{ + IsPass: true, + Message: "Secret test-secret was found in namespace test-namespace", + Title: "Secret test-secret", + IconKey: "kubernetes_analyze_secret", + IconURI: "https://troubleshoot.sh/images/analyzer-icons/secret.svg?w=13&h=16", + }, + }, + { + name: "spec with neither fail nor pass outcome returns an error so the framework surfaces the misconfiguration", + analyzer: &troubleshootv1beta2.AnalyzeSecret{ + AnalyzeMeta: troubleshootv1beta2.AnalyzeMeta{ + CheckName: "Misconfigured", + }, + Namespace: "default", + SecretName: "does-not-exist", + Outcomes: []*troubleshootv1beta2.Outcome{}, + }, + mockFiles: map[string][]byte{ + "secrets/default/does-not-exist.json": mustJSONMarshalIndent(t, collect.SecretOutput{ + Namespace: "default", + Name: "does-not-exist", + SecretExists: false, + }), + }, + wantErr: true, + }, { name: "key not found secret not found", analyzer: &troubleshootv1beta2.AnalyzeSecret{ @@ -190,6 +302,128 @@ func Test_analyzeSecret(t *testing.T) { }, wantErr: true, // TODO: should this be a not found error? This will not work with selectors. }, + { + name: "combined fail and pass in a single outcome, secret found uses the pass outcome", + analyzer: &troubleshootv1beta2.AnalyzeSecret{ + Namespace: "test-namespace", + SecretName: "test-secret", + Outcomes: []*troubleshootv1beta2.Outcome{ + { + Fail: &troubleshootv1beta2.SingleOutcome{ + Message: "Not found", + }, + Pass: &troubleshootv1beta2.SingleOutcome{ + Message: "Found", + URI: "https://example.com/found", + }, + }, + }, + }, + mockFiles: map[string][]byte{ + "secrets/test-namespace/test-secret.json": mustJSONMarshalIndent(t, collect.SecretOutput{ + Namespace: "test-namespace", + Name: "test-secret", + SecretExists: true, + }), + }, + want: &AnalyzeResult{ + IsPass: true, + Message: "Found", + URI: "https://example.com/found", + Title: "Secret test-secret", + IconKey: "kubernetes_analyze_secret", + IconURI: "https://troubleshoot.sh/images/analyzer-icons/secret.svg?w=13&h=16", + }, + }, + { + name: "combined fail and pass in a single outcome, secret not found uses the fail outcome", + analyzer: &troubleshootv1beta2.AnalyzeSecret{ + Namespace: "test-namespace", + SecretName: "test-secret", + Outcomes: []*troubleshootv1beta2.Outcome{ + { + Fail: &troubleshootv1beta2.SingleOutcome{ + Message: "Not found", + }, + Pass: &troubleshootv1beta2.SingleOutcome{ + Message: "Found", + }, + }, + }, + }, + mockFiles: map[string][]byte{ + "secrets/test-namespace/test-secret.json": mustJSONMarshalIndent(t, collect.SecretOutput{ + Namespace: "test-namespace", + Name: "test-secret", + SecretExists: false, + }), + }, + want: &AnalyzeResult{ + IsFail: true, + Message: "Not found", + Title: "Secret test-secret", + IconKey: "kubernetes_analyze_secret", + IconURI: "https://troubleshoot.sh/images/analyzer-icons/secret.svg?w=13&h=16", + }, + }, + { + name: "secret found with URI-only pass outcome preserves the empty message and URI", + analyzer: &troubleshootv1beta2.AnalyzeSecret{ + Namespace: "test-namespace", + SecretName: "test-secret", + Outcomes: []*troubleshootv1beta2.Outcome{ + { + Pass: &troubleshootv1beta2.SingleOutcome{ + URI: "https://example.com/pass", + }, + }, + }, + }, + mockFiles: map[string][]byte{ + "secrets/test-namespace/test-secret.json": mustJSONMarshalIndent(t, collect.SecretOutput{ + Namespace: "test-namespace", + Name: "test-secret", + SecretExists: true, + }), + }, + want: &AnalyzeResult{ + IsPass: true, + Message: "", + URI: "https://example.com/pass", + Title: "Secret test-secret", + IconKey: "kubernetes_analyze_secret", + IconURI: "https://troubleshoot.sh/images/analyzer-icons/secret.svg?w=13&h=16", + }, + }, + { + name: "secret not found with URI-only fail outcome preserves the empty message and URI", + analyzer: &troubleshootv1beta2.AnalyzeSecret{ + Namespace: "test-namespace", + SecretName: "test-secret", + Outcomes: []*troubleshootv1beta2.Outcome{ + { + Fail: &troubleshootv1beta2.SingleOutcome{ + URI: "https://example.com/fail", + }, + }, + }, + }, + mockFiles: map[string][]byte{ + "secrets/test-namespace/test-secret.json": mustJSONMarshalIndent(t, collect.SecretOutput{ + Namespace: "test-namespace", + Name: "test-secret", + SecretExists: false, + }), + }, + want: &AnalyzeResult{ + IsFail: true, + Message: "", + URI: "https://example.com/fail", + Title: "Secret test-secret", + IconKey: "kubernetes_analyze_secret", + IconURI: "https://troubleshoot.sh/images/analyzer-icons/secret.svg?w=13&h=16", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {