From 38cdfb1cdde099f30a4348fde1129623cc58a7ac Mon Sep 17 00:00:00 2001 From: Patryk Wasielewski Date: Wed, 5 Nov 2025 13:11:34 +0100 Subject: [PATCH 1/8] fix isAppAlreadyInstalledo error handling --- pkg/splunk/enterprise/afwscheduler.go | 39 ++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/pkg/splunk/enterprise/afwscheduler.go b/pkg/splunk/enterprise/afwscheduler.go index cc99eca0d..36f3847c6 100644 --- a/pkg/splunk/enterprise/afwscheduler.go +++ b/pkg/splunk/enterprise/afwscheduler.go @@ -785,12 +785,13 @@ func isAppAlreadyInstalled(ctx context.Context, cr splcommon.MetaObject, podExec scopedLog.Info("check app's installation state") - command := fmt.Sprintf("/opt/splunk/bin/splunk list app %s -auth admin:`cat /mnt/splunk-secrets/password`| grep ENABLED; echo -n $?", appTopFolder) + command := fmt.Sprintf("/opt/splunk/bin/splunk list app %s -auth admin:`cat /mnt/splunk-secrets/password`| grep ENABLED", appTopFolder) streamOptions := splutil.NewStreamOptionsObject(command) stdOut, stdErr, err := podExecClient.RunPodExecCommand(ctx, streamOptions, []string{"/bin/sh"}) + // Handle specific stderr cases first if strings.Contains(stdErr, "Could not find object") { // when app is not installed you will see something like on StdErr: // "Could not find object id=" @@ -798,15 +799,41 @@ func isAppAlreadyInstalled(ctx context.Context, cr splcommon.MetaObject, podExec return false, nil } - if stdErr != "" || err != nil { - return false, fmt.Errorf("could not get installed app status stdOut: %s, stdErr: %s, command: %s", stdOut, stdErr, command) + // Handle FIPS messages in stderr - these are informational, not errors + if stdErr != "" && strings.Contains(stdErr, "FIPS provider enabled") { + scopedLog.Info("FIPS provider informational message detected", "stderr", stdErr) + // Continue processing - FIPS messages don't indicate failure + } else if stdErr != "" { + // Log warning for any other stderr content we haven't seen before + scopedLog.Info("Unexpected stderr content detected - please review", "stderr", stdErr, "command", command) } - appInstallCheck, _ := strconv.Atoi(stdOut) + // Now check the actual command result + if err != nil { + // Kubernetes exec returns different error types for different exit codes + // For grep: exit code 1 = pattern not found, exit code 2+ = actual error + errMsg := err.Error() + + // Check for grep exit code 1 (pattern not found) + if strings.Contains(errMsg, "exit status 1") || strings.Contains(errMsg, "command terminated with exit code 1") { + // grep exit code 1 means "ENABLED" pattern not found - app is not enabled + scopedLog.Info("App not enabled - grep pattern not found", "stdout", stdOut, "stderr", stdErr) + return false, nil + } + + // Any other exit code indicates a real error (splunk command failed, etc.) + return false, fmt.Errorf("could not get installed app status stdOut: %s, stdErr: %s, error: %v, command: %s", stdOut, stdErr, err, command) + } - scopedLog.Info("Apps installation state", stdOut, stdOut) + // If we reach here, grep found "ENABLED" (exit code 0) + // stdOut should contain the app status line with "ENABLED" + if stdOut == "" { + // This shouldn't happen if grep succeeded, but let's be safe + return false, fmt.Errorf("command succeeded but no output received, command: %s", command) + } - return appInstallCheck == 0, nil + scopedLog.Info("App installation state check successful - app is enabled", "appStatus", strings.TrimSpace(stdOut)) + return true, nil } // get the name of top folder from the package. From d350504905bb59d9773849f8fe05ec099be2910b Mon Sep 17 00:00:00 2001 From: Patryk Wasielewski Date: Fri, 14 Nov 2025 14:15:29 +0100 Subject: [PATCH 2/8] add FIPS error handling and improve standalone error handling in afwscheduler --- pkg/splunk/enterprise/afwscheduler.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/pkg/splunk/enterprise/afwscheduler.go b/pkg/splunk/enterprise/afwscheduler.go index 36f3847c6..ee9cff7ef 100644 --- a/pkg/splunk/enterprise/afwscheduler.go +++ b/pkg/splunk/enterprise/afwscheduler.go @@ -766,6 +766,14 @@ func installApp(rctx context.Context, localCtx *localScopePlaybookContext, cr sp streamOptions := splutil.NewStreamOptionsObject(command) stdOut, stdErr, err := localCtx.podExecClient.RunPodExecCommand(rctx, streamOptions, []string{"/bin/sh"}) + + // Handle FIPS messages in stderr - these are informational, not errors + if stdErr != "" && strings.Contains(stdErr, "FIPS provider enabled") { + scopedLog.Info("FIPS provider informational message detected during app install", "stderr", stdErr) + // Continue processing - FIPS messages don't indicate failure + stdErr = "" // Clear stderr so it doesn't trigger error handling below + } + // if the app was already installed previously, then just mark it for install complete if stdErr != "" || err != nil { phaseInfo.FailCount++ @@ -1347,6 +1355,15 @@ installPhase: phaseInfo := getPhaseInfoByPhaseType(ctx, installWorker, enterpriseApi.PhaseInstall) if isPhaseMaxRetriesReached(ctx, phaseInfo, installWorker.afwConfig) { phaseInfo.Status = enterpriseApi.AppPkgInstallError + + // For fanout CRs, also update the main PhaseInfo to reflect the failure + if isFanOutApplicableToCR(installWorker.cr) { + scopedLog.Info("Max retries reached for fanout CR - updating main phase info", "app", installWorker.appDeployInfo.AppName, "failCount", phaseInfo.FailCount) + installWorker.appDeployInfo.PhaseInfo.Phase = enterpriseApi.PhaseInstall + installWorker.appDeployInfo.PhaseInfo.Status = enterpriseApi.AppPkgInstallError + installWorker.appDeployInfo.DeployStatus = enterpriseApi.DeployStatusError + } + ppln.deleteWorkerFromPipelinePhase(ctx, phaseInfo.Phase, installWorker) } else if isPhaseStatusComplete(phaseInfo) { ppln.deleteWorkerFromPipelinePhase(ctx, phaseInfo.Phase, installWorker) From 668a15509e5f67a489fb297d92fdd6827c67cff6 Mon Sep 17 00:00:00 2001 From: Patryk Wasielewski Date: Fri, 14 Nov 2025 15:37:33 +0100 Subject: [PATCH 3/8] add unit tests for enhanced FIPS error handlind --- pkg/splunk/enterprise/afwscheduler_test.go | 178 +++++++++++++++++++-- 1 file changed, 161 insertions(+), 17 deletions(-) diff --git a/pkg/splunk/enterprise/afwscheduler_test.go b/pkg/splunk/enterprise/afwscheduler_test.go index 87c5f2ba8..abfb7aa8d 100644 --- a/pkg/splunk/enterprise/afwscheduler_test.go +++ b/pkg/splunk/enterprise/afwscheduler_test.go @@ -3070,9 +3070,10 @@ func TestRunLocalScopedPlaybook(t *testing.T) { t.Errorf("Failed to detect not able to get installed app name: err") } - // Test4: get installed app command passes but installing app fails - mockPodExecReturnContexts[2].StdOut = "1" //app is not yet installed or it is not enabled - mockPodExecReturnContexts[2].StdErr = "" //no error thrown + // Test4: get installed app command passes but installing app fails (non-FIPS error) + mockPodExecReturnContexts[2].StdOut = "1" //app is not yet installed or it is not enabled + mockPodExecReturnContexts[2].StdErr = "" //no error thrown + mockPodExecReturnContexts[3].StdErr = "real installation error" // Non-FIPS error should still fail localInstallCtxt.sem <- struct{}{} waiter.Add(1) @@ -3091,15 +3092,14 @@ func TestRunLocalScopedPlaybook(t *testing.T) { t.Errorf("Expected app install failed") } - // Test5: install app should be successful - - mockPodExecReturnContexts[3].StdErr = "" //no error for app install + // Test5: FIPS message should be handled gracefully during install + mockPodExecReturnContexts[3].StdErr = "FIPS provider enabled. name: OpenSSL FIPS Provider, version: 3.0.9, buildinfo: 3.0.9, status: Success" // FIPS message should be ignored localInstallCtxt.sem <- struct{}{} waiter.Add(1) err = localInstallCtxt.runPlaybook(ctx) if err == nil { - t.Errorf("Expected app install succeeded but app arhive deletion failed") + t.Errorf("Expected app install succeeded but app archive deletion failed") } // Test6: successful scenario where everything succeeds @@ -3301,19 +3301,23 @@ func TestPremiumAppScopedPlaybook(t *testing.T) { t.Errorf("Failed to detect that steps to get installed app failed") } - // Test3: get installed app name passes but getting installed app name failed + // Test3: get installed app name passes but isAppAlreadyInstalled fails with non-FIPS error mockPodExecReturnContexts[1].StdErr = "" + mockPodExecReturnContexts[2].StdErr = "Could not find object id=app1" // Non-FIPS error + mockPodExecReturnContexts[2].Err = fmt.Errorf("exit status 2") // Real error, not grep exit code 1 localInstallCtxt.sem <- struct{}{} waiter.Add(1) err = pCtx.runPlaybook(ctx) if err == nil { - t.Errorf("Failed to detect not able to get installed app name: err") + t.Errorf("Failed to detect isAppAlreadyInstalled error") } - // Test4: get installed app command passes, it returns app is not enabled - // so app install will run and it should fail - mockPodExecReturnContexts[2].StdOut = "1" //app is not yet installed or it is not enabled - mockPodExecReturnContexts[2].StdErr = "" //no error thrown + // Test4: isAppAlreadyInstalled returns app is not enabled (grep exit code 1) + // so app install will run and it should fail with non-FIPS error + mockPodExecReturnContexts[2].StdOut = "" // No stdout means grep didn't find ENABLED + mockPodExecReturnContexts[2].StdErr = "" // No stderr + mockPodExecReturnContexts[2].Err = fmt.Errorf("exit status 1") // grep exit code 1 = pattern not found + mockPodExecReturnContexts[3].StdErr = "real installation error" // Non-FIPS error should still fail localInstallCtxt.sem <- struct{}{} waiter.Add(1) @@ -3322,9 +3326,9 @@ func TestPremiumAppScopedPlaybook(t *testing.T) { t.Errorf("Expected app install failed") } - // Test5: install app should be successful but es post install fails - - mockPodExecReturnContexts[3].StdErr = "" //no error for app install + // Test5: FIPS message during install should be handled gracefully, but es post install fails + mockPodExecReturnContexts[3].StdErr = "FIPS provider enabled. name: OpenSSL FIPS Provider, version: 3.0.9, buildinfo: 3.0.9, status: Success" // FIPS message should be ignored + mockPodExecReturnContexts[3].Err = nil // No actual error for install localInstallCtxt.sem <- struct{}{} waiter.Add(1) @@ -3343,7 +3347,24 @@ func TestPremiumAppScopedPlaybook(t *testing.T) { t.Errorf("Expected es post install succeeded but app arhive deletion failed") } - // Test7: successful scenario where everything succeeds + // Test7: App already installed with FIPS message - should skip installation + // Reset all mock contexts for this test + mockPodExecReturnContexts[0].Err = nil // File exists check passes + mockPodExecReturnContexts[1].StdErr = "" // Get app name passes + mockPodExecReturnContexts[1].StdOut = "app1" // App name is found + mockPodExecReturnContexts[2].StdOut = "app1 CONFIGURED ENABLED VISIBLE" // App is already enabled + mockPodExecReturnContexts[2].StdErr = "FIPS provider enabled. name: OpenSSL FIPS Provider, version: 3.0.9, buildinfo: 3.0.9, status: Success" + mockPodExecReturnContexts[2].Err = nil // No error - app is found and enabled + // Install step should be skipped, but cleanup should still work + mockPodExecReturnContexts[5].StdErr = "" // Cleanup should succeed + localInstallCtxt.sem <- struct{}{} + waiter.Add(1) + err = pCtx.runPlaybook(ctx) + if err != nil { + t.Errorf("runPlayBook should not have returned error when app is already installed with FIPS message. err=%s", err.Error()) + } + + // Test8: successful scenario where everything succeeds mockPodExecReturnContexts[5].StdErr = "" localInstallCtxt.sem <- struct{}{} waiter.Add(1) @@ -4462,3 +4483,126 @@ func TestAddTelAppCManager(t *testing.T) { // Negative testing addTelApp(ctx, mockPodExecClient, 2, &crNew) } + +func TestIsAppAlreadyInstalled(t *testing.T) { + ctx := context.TODO() + + tests := []struct { + name string + stdOut string + stdErr string + err error + expectedResult bool + expectedError bool + description string + }{ + { + name: "App is enabled - success case", + stdOut: "myapp CONFIGURED ENABLED VISIBLE", + stdErr: "", + err: nil, + expectedResult: true, + expectedError: false, + description: "App is found and enabled", + }, + { + name: "App not found - grep exit code 1", + stdOut: "", + stdErr: "", + err: fmt.Errorf("command terminated with exit code 1"), + expectedResult: false, + expectedError: false, + description: "App not enabled - grep pattern not found", + }, + { + name: "App not found - Could not find object", + stdOut: "", + stdErr: "Could not find object id=myapp", + err: nil, + expectedResult: false, + expectedError: false, + description: "App not installed at all", + }, + { + name: "FIPS provider message with app enabled", + stdOut: "myapp CONFIGURED ENABLED VISIBLE", + stdErr: "FIPS provider enabled. name: OpenSSL FIPS Provider, version: 3.0.9, buildinfo: 3.0.9, status: Success", + err: nil, + expectedResult: true, + expectedError: false, + description: "FIPS message should be ignored when app is enabled", + }, + { + name: "FIPS provider message with app not enabled", + stdOut: "", + stdErr: "FIPS provider enabled. name: OpenSSL FIPS Provider, version: 3.0.9, buildinfo: 3.0.9, status: Success", + err: fmt.Errorf("exit status 1"), + expectedResult: false, + expectedError: false, + description: "FIPS message should be ignored, grep exit code 1 means not enabled", + }, + { + name: "Real error - exit code 2", + stdOut: "", + stdErr: "Some real error occurred", + err: fmt.Errorf("exit status 2"), + expectedResult: false, + expectedError: true, + description: "Real error should be returned", + }, + { + name: "Command succeeded but no output", + stdOut: "", + stdErr: "", + err: nil, + expectedResult: false, + expectedError: true, + description: "Should error if command succeeds but no output", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a test CR + cr := &enterpriseApi.Standalone{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-standalone", + Namespace: "test", + }, + } + + // Create mock pod exec client with CR + mockPodExecClient := &spltest.MockPodExecClient{Cr: cr} + mockPodExecClient.SetTargetPodName(ctx, "test-pod") + + // Set up the mock return context + mockReturnContext := &spltest.MockPodExecReturnContext{ + StdOut: tt.stdOut, + StdErr: tt.stdErr, + Err: tt.err, + } + + // Add the mock command and return context - use the exact command pattern + command := "/opt/splunk/bin/splunk list app testapp -auth admin:`cat /mnt/splunk-secrets/password`| grep ENABLED" + mockPodExecClient.AddMockPodExecReturnContexts(ctx, []string{command}, mockReturnContext) + + // Call the function + result, err := isAppAlreadyInstalled(ctx, cr, mockPodExecClient, "testapp") + + // Check results + if tt.expectedError { + if err == nil { + t.Errorf("Expected error but got none for test: %s", tt.description) + } + } else { + if err != nil { + t.Errorf("Unexpected error for test '%s': %v", tt.description, err) + } + } + + if result != tt.expectedResult { + t.Errorf("Expected result %v but got %v for test: %s", tt.expectedResult, result, tt.description) + } + }) + } +} From 40f3b34b32567d7bdf3bb7b9f925041178529540 Mon Sep 17 00:00:00 2001 From: Patryk Wasielewski Date: Fri, 14 Nov 2025 16:01:32 +0100 Subject: [PATCH 4/8] add unit tests --- pkg/splunk/enterprise/afwscheduler_test.go | 26 +++++++++++++--------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/pkg/splunk/enterprise/afwscheduler_test.go b/pkg/splunk/enterprise/afwscheduler_test.go index abfb7aa8d..62d568269 100644 --- a/pkg/splunk/enterprise/afwscheduler_test.go +++ b/pkg/splunk/enterprise/afwscheduler_test.go @@ -3061,18 +3061,21 @@ func TestRunLocalScopedPlaybook(t *testing.T) { t.Errorf("Failed to detect that steps to get installed app failed") } - // Test3: get installed app name passes but getting installed app name failed + // Test3: get installed app name passes but isAppAlreadyInstalled fails with real error mockPodExecReturnContexts[1].StdErr = "" + mockPodExecReturnContexts[2].StdErr = "Could not find object id=app1" // Non-FIPS error + mockPodExecReturnContexts[2].Err = fmt.Errorf("exit status 2") // Real error, not grep exit code 1 localInstallCtxt.sem <- struct{}{} waiter.Add(1) err = localInstallCtxt.runPlaybook(ctx) if err == nil { - t.Errorf("Failed to detect not able to get installed app name: err") + t.Errorf("Failed to detect isAppAlreadyInstalled error") } - // Test4: get installed app command passes but installing app fails (non-FIPS error) - mockPodExecReturnContexts[2].StdOut = "1" //app is not yet installed or it is not enabled - mockPodExecReturnContexts[2].StdErr = "" //no error thrown + // Test4: isAppAlreadyInstalled returns app not enabled (grep exit code 1), then install fails + mockPodExecReturnContexts[2].StdOut = "" // No stdout means grep didn't find ENABLED + mockPodExecReturnContexts[2].StdErr = "" // No stderr + mockPodExecReturnContexts[2].Err = fmt.Errorf("exit status 1") // grep exit code 1 = pattern not found mockPodExecReturnContexts[3].StdErr = "real installation error" // Non-FIPS error should still fail localInstallCtxt.sem <- struct{}{} @@ -3082,18 +3085,21 @@ func TestRunLocalScopedPlaybook(t *testing.T) { t.Errorf("Expected app install failed") } - mockPodExecReturnContexts[2].StdOut = "1" //app is not yet installed or it is not enabled - mockPodExecReturnContexts[2].StdErr = "Could not find object" + // Test5: App not found scenario (Could not find object) + mockPodExecReturnContexts[2].StdOut = "" + mockPodExecReturnContexts[2].StdErr = "Could not find object id=app1" + mockPodExecReturnContexts[2].Err = nil // This should return false, nil (app not installed) localInstallCtxt.sem <- struct{}{} waiter.Add(1) err = localInstallCtxt.runPlaybook(ctx) if err == nil { - t.Errorf("Expected app install failed") + t.Errorf("Expected app install failed due to installation error") } - // Test5: FIPS message should be handled gracefully during install + // Test6: FIPS message should be handled gracefully during install mockPodExecReturnContexts[3].StdErr = "FIPS provider enabled. name: OpenSSL FIPS Provider, version: 3.0.9, buildinfo: 3.0.9, status: Success" // FIPS message should be ignored + mockPodExecReturnContexts[3].Err = nil // No actual error for install localInstallCtxt.sem <- struct{}{} waiter.Add(1) @@ -3102,7 +3108,7 @@ func TestRunLocalScopedPlaybook(t *testing.T) { t.Errorf("Expected app install succeeded but app archive deletion failed") } - // Test6: successful scenario where everything succeeds + // Test7: successful scenario where everything succeeds mockPodExecReturnContexts[4].StdErr = "" localInstallCtxt.sem <- struct{}{} waiter.Add(1) From b4772efdb7cdd305659c8abfdccd8ab19c31b95e Mon Sep 17 00:00:00 2001 From: Patryk Wasielewski Date: Fri, 14 Nov 2025 16:36:06 +0100 Subject: [PATCH 5/8] fix comment --- pkg/splunk/enterprise/afwscheduler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/splunk/enterprise/afwscheduler.go b/pkg/splunk/enterprise/afwscheduler.go index ee9cff7ef..efdf4af9e 100644 --- a/pkg/splunk/enterprise/afwscheduler.go +++ b/pkg/splunk/enterprise/afwscheduler.go @@ -818,7 +818,7 @@ func isAppAlreadyInstalled(ctx context.Context, cr splcommon.MetaObject, podExec // Now check the actual command result if err != nil { - // Kubernetes exec returns different error types for different exit codes + // The command pipeline ends with 'grep ENABLED', so exit codes follow grep semantics: // For grep: exit code 1 = pattern not found, exit code 2+ = actual error errMsg := err.Error() From 24563a24e56e0910df997cd12b7984cc5bf693c5 Mon Sep 17 00:00:00 2001 From: Patryk Wasielewski Date: Mon, 17 Nov 2025 13:43:49 +0100 Subject: [PATCH 6/8] remove stdErr output based decision if possible --- pkg/splunk/enterprise/afwscheduler.go | 25 +++++------ pkg/splunk/enterprise/afwscheduler_test.go | 51 ++++++++++++---------- 2 files changed, 39 insertions(+), 37 deletions(-) diff --git a/pkg/splunk/enterprise/afwscheduler.go b/pkg/splunk/enterprise/afwscheduler.go index efdf4af9e..2dd2fd667 100644 --- a/pkg/splunk/enterprise/afwscheduler.go +++ b/pkg/splunk/enterprise/afwscheduler.go @@ -767,15 +767,14 @@ func installApp(rctx context.Context, localCtx *localScopePlaybookContext, cr sp stdOut, stdErr, err := localCtx.podExecClient.RunPodExecCommand(rctx, streamOptions, []string{"/bin/sh"}) - // Handle FIPS messages in stderr - these are informational, not errors - if stdErr != "" && strings.Contains(stdErr, "FIPS provider enabled") { - scopedLog.Info("FIPS provider informational message detected during app install", "stderr", stdErr) - // Continue processing - FIPS messages don't indicate failure - stdErr = "" // Clear stderr so it doesn't trigger error handling below + // TODO(patrykw-splunk): remove this once we have confirm that we are not using stderr for error detection at all + // Log stderr content for debugging but don't use it for error detection + if stdErr != "" { + scopedLog.Info("App install command stderr output (informational only)", "stderr", stdErr) } - // if the app was already installed previously, then just mark it for install complete - if stdErr != "" || err != nil { + // Check only the actual command execution error, not stderr content + if err != nil { phaseInfo.FailCount++ scopedLog.Error(err, "local scoped app package install failed", "stdout", stdOut, "stderr", stdErr, "app pkg path", appPkgPathOnPod, "failCount", phaseInfo.FailCount) return fmt.Errorf("local scoped app package install failed. stdOut: %s, stdErr: %s, app pkg path: %s, failCount: %d", stdOut, stdErr, appPkgPathOnPod, phaseInfo.FailCount) @@ -807,13 +806,9 @@ func isAppAlreadyInstalled(ctx context.Context, cr splcommon.MetaObject, podExec return false, nil } - // Handle FIPS messages in stderr - these are informational, not errors - if stdErr != "" && strings.Contains(stdErr, "FIPS provider enabled") { - scopedLog.Info("FIPS provider informational message detected", "stderr", stdErr) - // Continue processing - FIPS messages don't indicate failure - } else if stdErr != "" { - // Log warning for any other stderr content we haven't seen before - scopedLog.Info("Unexpected stderr content detected - please review", "stderr", stdErr, "command", command) + // Log any other stderr content for debugging but don't use it for error detection + if stdErr != "" { + scopedLog.Info("Command stderr output (informational only)", "stderr", stdErr) } // Now check the actual command result @@ -824,7 +819,7 @@ func isAppAlreadyInstalled(ctx context.Context, cr splcommon.MetaObject, podExec // Check for grep exit code 1 (pattern not found) if strings.Contains(errMsg, "exit status 1") || strings.Contains(errMsg, "command terminated with exit code 1") { - // grep exit code 1 means "ENABLED" pattern not found - app is not enabled + // grep exit code 1 means "ENABLED" pattern not found - app exists but is not enabled scopedLog.Info("App not enabled - grep pattern not found", "stdout", stdOut, "stderr", stdErr) return false, nil } diff --git a/pkg/splunk/enterprise/afwscheduler_test.go b/pkg/splunk/enterprise/afwscheduler_test.go index 62d568269..e3b1f336c 100644 --- a/pkg/splunk/enterprise/afwscheduler_test.go +++ b/pkg/splunk/enterprise/afwscheduler_test.go @@ -3061,9 +3061,9 @@ func TestRunLocalScopedPlaybook(t *testing.T) { t.Errorf("Failed to detect that steps to get installed app failed") } - // Test3: get installed app name passes but isAppAlreadyInstalled fails with real error + // Test3: get installed app name passes but isAppAlreadyInstalled fails with real error (not "Could not find object") mockPodExecReturnContexts[1].StdErr = "" - mockPodExecReturnContexts[2].StdErr = "Could not find object id=app1" // Non-FIPS error + mockPodExecReturnContexts[2].StdErr = "Some other real error message" // Real error, not "Could not find object" mockPodExecReturnContexts[2].Err = fmt.Errorf("exit status 2") // Real error, not grep exit code 1 localInstallCtxt.sem <- struct{}{} waiter.Add(1) @@ -3076,7 +3076,8 @@ func TestRunLocalScopedPlaybook(t *testing.T) { mockPodExecReturnContexts[2].StdOut = "" // No stdout means grep didn't find ENABLED mockPodExecReturnContexts[2].StdErr = "" // No stderr mockPodExecReturnContexts[2].Err = fmt.Errorf("exit status 1") // grep exit code 1 = pattern not found - mockPodExecReturnContexts[3].StdErr = "real installation error" // Non-FIPS error should still fail + mockPodExecReturnContexts[3].StdErr = "real installation error" // This is just logged now + mockPodExecReturnContexts[3].Err = fmt.Errorf("install command failed") // This causes the actual failure localInstallCtxt.sem <- struct{}{} waiter.Add(1) @@ -3085,10 +3086,13 @@ func TestRunLocalScopedPlaybook(t *testing.T) { t.Errorf("Expected app install failed") } - // Test5: App not found scenario (Could not find object) + // Test5: App not found scenario (Could not find object) - should proceed to install but install fails mockPodExecReturnContexts[2].StdOut = "" mockPodExecReturnContexts[2].StdErr = "Could not find object id=app1" mockPodExecReturnContexts[2].Err = nil // This should return false, nil (app not installed) + // Keep the installation error from previous test to make install fail + mockPodExecReturnContexts[3].StdErr = "real installation error" // Install should fail + mockPodExecReturnContexts[3].Err = fmt.Errorf("install failed") localInstallCtxt.sem <- struct{}{} waiter.Add(1) @@ -3097,9 +3101,11 @@ func TestRunLocalScopedPlaybook(t *testing.T) { t.Errorf("Expected app install failed due to installation error") } - // Test6: FIPS message should be handled gracefully during install - mockPodExecReturnContexts[3].StdErr = "FIPS provider enabled. name: OpenSSL FIPS Provider, version: 3.0.9, buildinfo: 3.0.9, status: Success" // FIPS message should be ignored + // Test6: Install succeeds with stderr content (should be ignored), but cleanup fails + mockPodExecReturnContexts[3].StdErr = "Some informational message in stderr" // Stderr content should be ignored mockPodExecReturnContexts[3].Err = nil // No actual error for install + // Keep cleanup failure from previous test setup to make overall test fail + // mockPodExecReturnContexts[4] still has error from earlier localInstallCtxt.sem <- struct{}{} waiter.Add(1) @@ -3307,9 +3313,9 @@ func TestPremiumAppScopedPlaybook(t *testing.T) { t.Errorf("Failed to detect that steps to get installed app failed") } - // Test3: get installed app name passes but isAppAlreadyInstalled fails with non-FIPS error + // Test3: get installed app name passes but isAppAlreadyInstalled fails with real error (not "Could not find object") mockPodExecReturnContexts[1].StdErr = "" - mockPodExecReturnContexts[2].StdErr = "Could not find object id=app1" // Non-FIPS error + mockPodExecReturnContexts[2].StdErr = "Some other real error message" // Real error, not "Could not find object" mockPodExecReturnContexts[2].Err = fmt.Errorf("exit status 2") // Real error, not grep exit code 1 localInstallCtxt.sem <- struct{}{} waiter.Add(1) @@ -3319,11 +3325,12 @@ func TestPremiumAppScopedPlaybook(t *testing.T) { } // Test4: isAppAlreadyInstalled returns app is not enabled (grep exit code 1) - // so app install will run and it should fail with non-FIPS error + // so app install will run and it should fail with real error mockPodExecReturnContexts[2].StdOut = "" // No stdout means grep didn't find ENABLED mockPodExecReturnContexts[2].StdErr = "" // No stderr mockPodExecReturnContexts[2].Err = fmt.Errorf("exit status 1") // grep exit code 1 = pattern not found - mockPodExecReturnContexts[3].StdErr = "real installation error" // Non-FIPS error should still fail + mockPodExecReturnContexts[3].StdErr = "real installation error" // This is just logged now + mockPodExecReturnContexts[3].Err = fmt.Errorf("install command failed") // This causes the actual failure localInstallCtxt.sem <- struct{}{} waiter.Add(1) @@ -3332,9 +3339,9 @@ func TestPremiumAppScopedPlaybook(t *testing.T) { t.Errorf("Expected app install failed") } - // Test5: FIPS message during install should be handled gracefully, but es post install fails - mockPodExecReturnContexts[3].StdErr = "FIPS provider enabled. name: OpenSSL FIPS Provider, version: 3.0.9, buildinfo: 3.0.9, status: Success" // FIPS message should be ignored - mockPodExecReturnContexts[3].Err = nil // No actual error for install + // Test5: Install succeeds with stderr content (should be ignored), but post install fails + mockPodExecReturnContexts[3].StdErr = "Some informational message in stderr" // Stderr content should be ignored + mockPodExecReturnContexts[3].Err = nil // No actual error for install localInstallCtxt.sem <- struct{}{} waiter.Add(1) @@ -3353,13 +3360,13 @@ func TestPremiumAppScopedPlaybook(t *testing.T) { t.Errorf("Expected es post install succeeded but app arhive deletion failed") } - // Test7: App already installed with FIPS message - should skip installation + // Test7: App already installed with stderr content - should skip installation // Reset all mock contexts for this test mockPodExecReturnContexts[0].Err = nil // File exists check passes mockPodExecReturnContexts[1].StdErr = "" // Get app name passes mockPodExecReturnContexts[1].StdOut = "app1" // App name is found mockPodExecReturnContexts[2].StdOut = "app1 CONFIGURED ENABLED VISIBLE" // App is already enabled - mockPodExecReturnContexts[2].StdErr = "FIPS provider enabled. name: OpenSSL FIPS Provider, version: 3.0.9, buildinfo: 3.0.9, status: Success" + mockPodExecReturnContexts[2].StdErr = "Some informational message in stderr" mockPodExecReturnContexts[2].Err = nil // No error - app is found and enabled // Install step should be skipped, but cleanup should still work mockPodExecReturnContexts[5].StdErr = "" // Cleanup should succeed @@ -3367,7 +3374,7 @@ func TestPremiumAppScopedPlaybook(t *testing.T) { waiter.Add(1) err = pCtx.runPlaybook(ctx) if err != nil { - t.Errorf("runPlayBook should not have returned error when app is already installed with FIPS message. err=%s", err.Error()) + t.Errorf("runPlayBook should not have returned error when app is already installed with stderr content. err=%s", err.Error()) } // Test8: successful scenario where everything succeeds @@ -4530,22 +4537,22 @@ func TestIsAppAlreadyInstalled(t *testing.T) { description: "App not installed at all", }, { - name: "FIPS provider message with app enabled", + name: "App enabled with stderr content", stdOut: "myapp CONFIGURED ENABLED VISIBLE", - stdErr: "FIPS provider enabled. name: OpenSSL FIPS Provider, version: 3.0.9, buildinfo: 3.0.9, status: Success", + stdErr: "Some informational message in stderr", err: nil, expectedResult: true, expectedError: false, - description: "FIPS message should be ignored when app is enabled", + description: "Stderr content should be ignored when app is enabled", }, { - name: "FIPS provider message with app not enabled", + name: "App not enabled with stderr content", stdOut: "", - stdErr: "FIPS provider enabled. name: OpenSSL FIPS Provider, version: 3.0.9, buildinfo: 3.0.9, status: Success", + stdErr: "Some informational message in stderr", err: fmt.Errorf("exit status 1"), expectedResult: false, expectedError: false, - description: "FIPS message should be ignored, grep exit code 1 means not enabled", + description: "Stderr content should be ignored, grep exit code 1 means not enabled", }, { name: "Real error - exit code 2", From 6546a3fdf00a1c44a3b25b1e264e6ddc20a4b3fc Mon Sep 17 00:00:00 2001 From: Patryk Wasielewski Date: Tue, 18 Nov 2025 15:07:32 +0100 Subject: [PATCH 7/8] test branch against azure and gcp --- .github/workflows/int-test-azure-workflow.yml | 1 + .github/workflows/int-test-gcp-workflow.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/workflows/int-test-azure-workflow.yml b/.github/workflows/int-test-azure-workflow.yml index c8fccb5e2..a3ed3ad85 100644 --- a/.github/workflows/int-test-azure-workflow.yml +++ b/.github/workflows/int-test-azure-workflow.yml @@ -4,6 +4,7 @@ on: branches: - develop - main + - feature/fix-error-handling-in-afwscheduler jobs: build-operator-image: runs-on: ubuntu-latest diff --git a/.github/workflows/int-test-gcp-workflow.yml b/.github/workflows/int-test-gcp-workflow.yml index 7b7d6afef..596c7ce81 100644 --- a/.github/workflows/int-test-gcp-workflow.yml +++ b/.github/workflows/int-test-gcp-workflow.yml @@ -5,6 +5,7 @@ on: branches: - develop - main + - feature/fix-error-handling-in-afwscheduler jobs: build-operator-image: runs-on: ubuntu-latest From 85795c553bf07454e51759869c2aa3eb145fdee2 Mon Sep 17 00:00:00 2001 From: Patryk Wasielewski Date: Thu, 20 Nov 2025 18:46:55 +0100 Subject: [PATCH 8/8] revert workflow changes --- .github/workflows/int-test-azure-workflow.yml | 1 - .github/workflows/int-test-gcp-workflow.yml | 1 - 2 files changed, 2 deletions(-) diff --git a/.github/workflows/int-test-azure-workflow.yml b/.github/workflows/int-test-azure-workflow.yml index a3ed3ad85..c8fccb5e2 100644 --- a/.github/workflows/int-test-azure-workflow.yml +++ b/.github/workflows/int-test-azure-workflow.yml @@ -4,7 +4,6 @@ on: branches: - develop - main - - feature/fix-error-handling-in-afwscheduler jobs: build-operator-image: runs-on: ubuntu-latest diff --git a/.github/workflows/int-test-gcp-workflow.yml b/.github/workflows/int-test-gcp-workflow.yml index 596c7ce81..7b7d6afef 100644 --- a/.github/workflows/int-test-gcp-workflow.yml +++ b/.github/workflows/int-test-gcp-workflow.yml @@ -5,7 +5,6 @@ on: branches: - develop - main - - feature/fix-error-handling-in-afwscheduler jobs: build-operator-image: runs-on: ubuntu-latest