Skip to content

Commit 24563a2

Browse files
author
Patryk Wasielewski
committed
remove stdErr output based decision if possible
1 parent b4772ef commit 24563a2

File tree

2 files changed

+39
-37
lines changed

2 files changed

+39
-37
lines changed

pkg/splunk/enterprise/afwscheduler.go

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -767,15 +767,14 @@ func installApp(rctx context.Context, localCtx *localScopePlaybookContext, cr sp
767767

768768
stdOut, stdErr, err := localCtx.podExecClient.RunPodExecCommand(rctx, streamOptions, []string{"/bin/sh"})
769769

770-
// Handle FIPS messages in stderr - these are informational, not errors
771-
if stdErr != "" && strings.Contains(stdErr, "FIPS provider enabled") {
772-
scopedLog.Info("FIPS provider informational message detected during app install", "stderr", stdErr)
773-
// Continue processing - FIPS messages don't indicate failure
774-
stdErr = "" // Clear stderr so it doesn't trigger error handling below
770+
// TODO(patrykw-splunk): remove this once we have confirm that we are not using stderr for error detection at all
771+
// Log stderr content for debugging but don't use it for error detection
772+
if stdErr != "" {
773+
scopedLog.Info("App install command stderr output (informational only)", "stderr", stdErr)
775774
}
776775

777-
// if the app was already installed previously, then just mark it for install complete
778-
if stdErr != "" || err != nil {
776+
// Check only the actual command execution error, not stderr content
777+
if err != nil {
779778
phaseInfo.FailCount++
780779
scopedLog.Error(err, "local scoped app package install failed", "stdout", stdOut, "stderr", stdErr, "app pkg path", appPkgPathOnPod, "failCount", phaseInfo.FailCount)
781780
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
807806
return false, nil
808807
}
809808

810-
// Handle FIPS messages in stderr - these are informational, not errors
811-
if stdErr != "" && strings.Contains(stdErr, "FIPS provider enabled") {
812-
scopedLog.Info("FIPS provider informational message detected", "stderr", stdErr)
813-
// Continue processing - FIPS messages don't indicate failure
814-
} else if stdErr != "" {
815-
// Log warning for any other stderr content we haven't seen before
816-
scopedLog.Info("Unexpected stderr content detected - please review", "stderr", stdErr, "command", command)
809+
// Log any other stderr content for debugging but don't use it for error detection
810+
if stdErr != "" {
811+
scopedLog.Info("Command stderr output (informational only)", "stderr", stdErr)
817812
}
818813

819814
// Now check the actual command result
@@ -824,7 +819,7 @@ func isAppAlreadyInstalled(ctx context.Context, cr splcommon.MetaObject, podExec
824819

825820
// Check for grep exit code 1 (pattern not found)
826821
if strings.Contains(errMsg, "exit status 1") || strings.Contains(errMsg, "command terminated with exit code 1") {
827-
// grep exit code 1 means "ENABLED" pattern not found - app is not enabled
822+
// grep exit code 1 means "ENABLED" pattern not found - app exists but is not enabled
828823
scopedLog.Info("App not enabled - grep pattern not found", "stdout", stdOut, "stderr", stdErr)
829824
return false, nil
830825
}

pkg/splunk/enterprise/afwscheduler_test.go

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3061,9 +3061,9 @@ func TestRunLocalScopedPlaybook(t *testing.T) {
30613061
t.Errorf("Failed to detect that steps to get installed app failed")
30623062
}
30633063

3064-
// Test3: get installed app name passes but isAppAlreadyInstalled fails with real error
3064+
// Test3: get installed app name passes but isAppAlreadyInstalled fails with real error (not "Could not find object")
30653065
mockPodExecReturnContexts[1].StdErr = ""
3066-
mockPodExecReturnContexts[2].StdErr = "Could not find object id=app1" // Non-FIPS error
3066+
mockPodExecReturnContexts[2].StdErr = "Some other real error message" // Real error, not "Could not find object"
30673067
mockPodExecReturnContexts[2].Err = fmt.Errorf("exit status 2") // Real error, not grep exit code 1
30683068
localInstallCtxt.sem <- struct{}{}
30693069
waiter.Add(1)
@@ -3076,7 +3076,8 @@ func TestRunLocalScopedPlaybook(t *testing.T) {
30763076
mockPodExecReturnContexts[2].StdOut = "" // No stdout means grep didn't find ENABLED
30773077
mockPodExecReturnContexts[2].StdErr = "" // No stderr
30783078
mockPodExecReturnContexts[2].Err = fmt.Errorf("exit status 1") // grep exit code 1 = pattern not found
3079-
mockPodExecReturnContexts[3].StdErr = "real installation error" // Non-FIPS error should still fail
3079+
mockPodExecReturnContexts[3].StdErr = "real installation error" // This is just logged now
3080+
mockPodExecReturnContexts[3].Err = fmt.Errorf("install command failed") // This causes the actual failure
30803081

30813082
localInstallCtxt.sem <- struct{}{}
30823083
waiter.Add(1)
@@ -3085,10 +3086,13 @@ func TestRunLocalScopedPlaybook(t *testing.T) {
30853086
t.Errorf("Expected app install failed")
30863087
}
30873088

3088-
// Test5: App not found scenario (Could not find object)
3089+
// Test5: App not found scenario (Could not find object) - should proceed to install but install fails
30893090
mockPodExecReturnContexts[2].StdOut = ""
30903091
mockPodExecReturnContexts[2].StdErr = "Could not find object id=app1"
30913092
mockPodExecReturnContexts[2].Err = nil // This should return false, nil (app not installed)
3093+
// Keep the installation error from previous test to make install fail
3094+
mockPodExecReturnContexts[3].StdErr = "real installation error" // Install should fail
3095+
mockPodExecReturnContexts[3].Err = fmt.Errorf("install failed")
30923096

30933097
localInstallCtxt.sem <- struct{}{}
30943098
waiter.Add(1)
@@ -3097,9 +3101,11 @@ func TestRunLocalScopedPlaybook(t *testing.T) {
30973101
t.Errorf("Expected app install failed due to installation error")
30983102
}
30993103

3100-
// Test6: FIPS message should be handled gracefully during install
3101-
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
3104+
// Test6: Install succeeds with stderr content (should be ignored), but cleanup fails
3105+
mockPodExecReturnContexts[3].StdErr = "Some informational message in stderr" // Stderr content should be ignored
31023106
mockPodExecReturnContexts[3].Err = nil // No actual error for install
3107+
// Keep cleanup failure from previous test setup to make overall test fail
3108+
// mockPodExecReturnContexts[4] still has error from earlier
31033109

31043110
localInstallCtxt.sem <- struct{}{}
31053111
waiter.Add(1)
@@ -3307,9 +3313,9 @@ func TestPremiumAppScopedPlaybook(t *testing.T) {
33073313
t.Errorf("Failed to detect that steps to get installed app failed")
33083314
}
33093315

3310-
// Test3: get installed app name passes but isAppAlreadyInstalled fails with non-FIPS error
3316+
// Test3: get installed app name passes but isAppAlreadyInstalled fails with real error (not "Could not find object")
33113317
mockPodExecReturnContexts[1].StdErr = ""
3312-
mockPodExecReturnContexts[2].StdErr = "Could not find object id=app1" // Non-FIPS error
3318+
mockPodExecReturnContexts[2].StdErr = "Some other real error message" // Real error, not "Could not find object"
33133319
mockPodExecReturnContexts[2].Err = fmt.Errorf("exit status 2") // Real error, not grep exit code 1
33143320
localInstallCtxt.sem <- struct{}{}
33153321
waiter.Add(1)
@@ -3319,11 +3325,12 @@ func TestPremiumAppScopedPlaybook(t *testing.T) {
33193325
}
33203326

33213327
// Test4: isAppAlreadyInstalled returns app is not enabled (grep exit code 1)
3322-
// so app install will run and it should fail with non-FIPS error
3328+
// so app install will run and it should fail with real error
33233329
mockPodExecReturnContexts[2].StdOut = "" // No stdout means grep didn't find ENABLED
33243330
mockPodExecReturnContexts[2].StdErr = "" // No stderr
33253331
mockPodExecReturnContexts[2].Err = fmt.Errorf("exit status 1") // grep exit code 1 = pattern not found
3326-
mockPodExecReturnContexts[3].StdErr = "real installation error" // Non-FIPS error should still fail
3332+
mockPodExecReturnContexts[3].StdErr = "real installation error" // This is just logged now
3333+
mockPodExecReturnContexts[3].Err = fmt.Errorf("install command failed") // This causes the actual failure
33273334

33283335
localInstallCtxt.sem <- struct{}{}
33293336
waiter.Add(1)
@@ -3332,9 +3339,9 @@ func TestPremiumAppScopedPlaybook(t *testing.T) {
33323339
t.Errorf("Expected app install failed")
33333340
}
33343341

3335-
// Test5: FIPS message during install should be handled gracefully, but es post install fails
3336-
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
3337-
mockPodExecReturnContexts[3].Err = nil // No actual error for install
3342+
// Test5: Install succeeds with stderr content (should be ignored), but post install fails
3343+
mockPodExecReturnContexts[3].StdErr = "Some informational message in stderr" // Stderr content should be ignored
3344+
mockPodExecReturnContexts[3].Err = nil // No actual error for install
33383345

33393346
localInstallCtxt.sem <- struct{}{}
33403347
waiter.Add(1)
@@ -3353,21 +3360,21 @@ func TestPremiumAppScopedPlaybook(t *testing.T) {
33533360
t.Errorf("Expected es post install succeeded but app arhive deletion failed")
33543361
}
33553362

3356-
// Test7: App already installed with FIPS message - should skip installation
3363+
// Test7: App already installed with stderr content - should skip installation
33573364
// Reset all mock contexts for this test
33583365
mockPodExecReturnContexts[0].Err = nil // File exists check passes
33593366
mockPodExecReturnContexts[1].StdErr = "" // Get app name passes
33603367
mockPodExecReturnContexts[1].StdOut = "app1" // App name is found
33613368
mockPodExecReturnContexts[2].StdOut = "app1 CONFIGURED ENABLED VISIBLE" // App is already enabled
3362-
mockPodExecReturnContexts[2].StdErr = "FIPS provider enabled. name: OpenSSL FIPS Provider, version: 3.0.9, buildinfo: 3.0.9, status: Success"
3369+
mockPodExecReturnContexts[2].StdErr = "Some informational message in stderr"
33633370
mockPodExecReturnContexts[2].Err = nil // No error - app is found and enabled
33643371
// Install step should be skipped, but cleanup should still work
33653372
mockPodExecReturnContexts[5].StdErr = "" // Cleanup should succeed
33663373
localInstallCtxt.sem <- struct{}{}
33673374
waiter.Add(1)
33683375
err = pCtx.runPlaybook(ctx)
33693376
if err != nil {
3370-
t.Errorf("runPlayBook should not have returned error when app is already installed with FIPS message. err=%s", err.Error())
3377+
t.Errorf("runPlayBook should not have returned error when app is already installed with stderr content. err=%s", err.Error())
33713378
}
33723379

33733380
// Test8: successful scenario where everything succeeds
@@ -4530,22 +4537,22 @@ func TestIsAppAlreadyInstalled(t *testing.T) {
45304537
description: "App not installed at all",
45314538
},
45324539
{
4533-
name: "FIPS provider message with app enabled",
4540+
name: "App enabled with stderr content",
45344541
stdOut: "myapp CONFIGURED ENABLED VISIBLE",
4535-
stdErr: "FIPS provider enabled. name: OpenSSL FIPS Provider, version: 3.0.9, buildinfo: 3.0.9, status: Success",
4542+
stdErr: "Some informational message in stderr",
45364543
err: nil,
45374544
expectedResult: true,
45384545
expectedError: false,
4539-
description: "FIPS message should be ignored when app is enabled",
4546+
description: "Stderr content should be ignored when app is enabled",
45404547
},
45414548
{
4542-
name: "FIPS provider message with app not enabled",
4549+
name: "App not enabled with stderr content",
45434550
stdOut: "",
4544-
stdErr: "FIPS provider enabled. name: OpenSSL FIPS Provider, version: 3.0.9, buildinfo: 3.0.9, status: Success",
4551+
stdErr: "Some informational message in stderr",
45454552
err: fmt.Errorf("exit status 1"),
45464553
expectedResult: false,
45474554
expectedError: false,
4548-
description: "FIPS message should be ignored, grep exit code 1 means not enabled",
4555+
description: "Stderr content should be ignored, grep exit code 1 means not enabled",
45494556
},
45504557
{
45514558
name: "Real error - exit code 2",

0 commit comments

Comments
 (0)