You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@oozie.apache.org by as...@apache.org on 2020/03/11 10:06:27 UTC

[oozie] branch master updated: OOZIE-3592 Do not print misleading SecurityException for successful jobs (matijhs via asalamon74)

This is an automated email from the ASF dual-hosted git repository.

asalamon74 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/oozie.git


The following commit(s) were added to refs/heads/master by this push:
     new 417305c  OOZIE-3592 Do not print misleading SecurityException for successful jobs (matijhs via asalamon74)
417305c is described below

commit 417305c36cfe0b7158bf4f7ff94a25813164e694
Author: Andras Salamon <as...@apache.org>
AuthorDate: Wed Mar 11 11:06:02 2020 +0100

    OOZIE-3592 Do not print misleading SecurityException for successful jobs (matijhs via asalamon74)
---
 release-log.txt                                    |  1 +
 .../apache/oozie/action/hadoop/ErrorHolder.java    | 11 ++-
 .../org/apache/oozie/action/hadoop/LauncherAM.java | 79 ++++++++++++----------
 .../apache/oozie/action/hadoop/TestLauncherAM.java | 27 +++++++-
 4 files changed, 79 insertions(+), 39 deletions(-)

diff --git a/release-log.txt b/release-log.txt
index 090899c..a78cbbe 100644
--- a/release-log.txt
+++ b/release-log.txt
@@ -1,5 +1,6 @@
 -- Oozie 5.3.0 release (trunk - unreleased)
 
+OOZIE-3592 Do not print misleading SecurityException for successful jobs (matijhs via asalamon74)
 OOZIE-3584 Fork-join action issue when action param cannot be resolved (jmakai via asalamon74)
 OOZIE-3589 Avoid calling copyActionData method multiple times in ReRunXCommand (zuston via asalamon74)
 OOZIE-3590 Fix missing log expression parameter in SLACalculatorMemory (zuston via asalamon74)
diff --git a/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/ErrorHolder.java b/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/ErrorHolder.java
index 6a755db..6f561ea 100644
--- a/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/ErrorHolder.java
+++ b/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/ErrorHolder.java
@@ -22,6 +22,7 @@ public class ErrorHolder {
     private Throwable errorCause = null;
     private String errorMessage = null;
     private boolean populated = false;
+    private boolean errorIgnorable = false;
 
     public int getErrorCode() {
         return errorCode;
@@ -50,7 +51,15 @@ public class ErrorHolder {
         this.populated = true;
     }
 
+    public void markErrorIgnorable() {
+        errorIgnorable = true;
+    }
+
+    public boolean isErrorIgnorable() {
+        return errorIgnorable;
+    }
+
     public boolean isPopulated() {
         return populated;
     }
-}
\ No newline at end of file
+}
diff --git a/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherAM.java b/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherAM.java
index 41e5445..bc91959 100644
--- a/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherAM.java
+++ b/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherAM.java
@@ -415,42 +415,9 @@ public class LauncherAM {
             System.out.println();
             actionMainExecutedProperly = true;
         } catch (InvocationTargetException ex) {
-            ex.printStackTrace(System.out);
-            // Get what actually caused the exception
-            Throwable cause = ex.getCause();
-            // If we got a JavaMainException from JavaMain, then we need to unwrap it
-            if (JavaMain.JavaMainException.class.isInstance(cause)) {
-                cause = cause.getCause();
-            }
-            if (LauncherMainException.class.isInstance(cause)) {
-                int errorCode = ((LauncherMainException) ex.getCause()).getErrorCode();
-                String mainClass = launcherConf.get(CONF_OOZIE_ACTION_MAIN_CLASS);
-                eHolder.setErrorMessage("Main Class [" + mainClass + "], exit code [" +
-                        errorCode + "]");
-                eHolder.setErrorCode(errorCode);
-            } else if (SecurityException.class.isInstance(cause)) {
-                if (launcherSecurityManager.getExitInvoked()) {
-                    final int exitCode = launcherSecurityManager.getExitCode();
-                    System.out.println("Intercepting System.exit(" + exitCode + ")");
-                    // if 0 main() method finished successfully
-                    // ignoring
-                    if (exitCode != 0) {
-                        eHolder.setErrorCode(exitCode);
-                        String mainClass = launcherConf.get(CONF_OOZIE_ACTION_MAIN_CLASS);
-                        eHolder.setErrorMessage("Main Class [" + mainClass + "],"
-                                + " exit code [" + eHolder.getErrorCode() + "]");
-                    } else {
-                        actionMainExecutedProperly = true;
-                    }
-                } else {
-                    // just SecurityException, no exit was invoked
-                    eHolder.setErrorCode(0);
-                    eHolder.setErrorCause(cause);
-                    eHolder.setErrorMessage(cause.getMessage());
-                }
-            } else {
-                eHolder.setErrorMessage(cause.getMessage());
-                eHolder.setErrorCause(cause);
+            actionMainExecutedProperly = handleInvocationError(eHolder, ex);
+            if(!eHolder.isErrorIgnorable()){
+                ex.printStackTrace(System.out);
             }
         } catch (Throwable t) {
             t.printStackTrace();
@@ -463,6 +430,46 @@ public class LauncherAM {
         return actionMainExecutedProperly;
     }
 
+    private boolean handleInvocationError(ErrorHolder eHolder, InvocationTargetException ex) {
+        boolean actionMainExecutedProperly = false;
+        // Get what actually caused the exception
+        Throwable cause = ex.getCause();
+        // If we got a JavaMainException from JavaMain, then we need to unwrap it
+        if (cause instanceof JavaMain.JavaMainException) {
+            cause = cause.getCause();
+        }
+        if (cause instanceof LauncherMainException) {
+            int errorCode = ((LauncherMainException) ex.getCause()).getErrorCode();
+            String mainClass = launcherConf.get(CONF_OOZIE_ACTION_MAIN_CLASS);
+            eHolder.setErrorMessage("Main Class [" + mainClass + "], exit code [" + errorCode + "]");
+            eHolder.setErrorCode(errorCode);
+        } else if (cause instanceof SecurityException) {
+            if (launcherSecurityManager.getExitInvoked()) {
+                final int exitCode = launcherSecurityManager.getExitCode();
+                System.out.println("Intercepting System.exit(" + exitCode + ")\n");
+                // if 0 main() method finished successfully
+                // ignoring
+                if (exitCode != 0) {
+                    eHolder.setErrorCode(exitCode);
+                    String mainClass = launcherConf.get(CONF_OOZIE_ACTION_MAIN_CLASS);
+                    eHolder.setErrorMessage("Main Class [" + mainClass + "], exit code [" + eHolder.getErrorCode() + "]");
+                } else {
+                    eHolder.markErrorIgnorable();
+                    actionMainExecutedProperly = true;
+                }
+            } else {
+                // just SecurityException, no exit was invoked
+                eHolder.setErrorCode(0);
+                eHolder.setErrorCause(cause);
+                eHolder.setErrorMessage(cause.getMessage());
+            }
+        } else {
+            eHolder.setErrorMessage(cause.getMessage());
+            eHolder.setErrorCause(cause);
+        }
+        return actionMainExecutedProperly;
+    }
+
     private void setRecoveryId() throws LauncherException {
         try {
             ApplicationId applicationId = containerId.getApplicationAttemptId().getApplicationId();
diff --git a/sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java b/sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java
index e87693b..e0fed9b 100644
--- a/sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java
+++ b/sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java
@@ -55,6 +55,8 @@ import static org.mockito.Mockito.verifyZeroInteractions;
 import java.io.File;
 import java.io.IOException;
 import java.io.StringReader;
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
 import java.text.MessageFormat;
 import java.util.Map;
 import java.util.Properties;
@@ -66,11 +68,13 @@ import org.apache.hadoop.yarn.client.api.async.AMRMClientAsync;
 import org.apache.hadoop.yarn.util.ConverterUtils;
 import org.apache.oozie.action.hadoop.security.LauncherSecurityManager;
 import org.apache.oozie.action.hadoop.LauncherAM.OozieActionResult;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.junit.runner.RunWith;
+import org.junit.Assert;
 import org.mockito.Mock;
 import static org.mockito.Mockito.when;
 import org.mockito.runners.MockitoJUnitRunner;
@@ -133,6 +137,9 @@ public class TestLauncherAM {
 
     private ExpectedFailureDetails failureDetails = new ExpectedFailureDetails();
 
+    private final ByteArrayOutputStream outContent = new ByteArrayOutputStream();
+    private final PrintStream originalOut = System.out;
+
     @Before
     public void setup() throws Exception {
         configureMocksForHappyPath();
@@ -141,6 +148,18 @@ public class TestLauncherAM {
         instantiateLauncher();
     }
 
+    /**
+     * Setup method to assert for stack trace printed to stdout
+     */
+    public void setupStreams() {
+        System.setOut(new PrintStream(outContent));
+    }
+
+    @After
+    public void restoreStreams() {
+        System.setOut(originalOut);
+    }
+
     @Test
     public void testMainIsSuccessfullyInvokedWithActionData() throws Exception {
         setupActionOutputContents();
@@ -281,18 +300,21 @@ public class TestLauncherAM {
     }
 
     @Test
-    public void testActionThrowsSecurityExceptionWithExitCode0() throws Exception {
+    public void testActionThrowsSecurityExceptionWithExitCode0AndDoesNotPrintStackTrace() throws Exception {
+        setupStreams();
         setupArgsForMainClass(SECURITY_EXCEPTION);
         given(launcherSecurityManagerMock.getExitInvoked()).willReturn(true);
         given(launcherSecurityManagerMock.getExitCode()).willReturn(0);
 
         executeLauncher();
 
+        Assert.assertFalse(outContent.toString().contains("java.lang.SecurityException"));
         assertSuccessfulExecution(OozieActionResult.SUCCEEDED);
     }
 
     @Test
-    public void testActionThrowsSecurityExceptionWithExitCode1() throws Exception {
+    public void testActionThrowsSecurityExceptionWithExitCode1AndPrintsStackTrace() throws Exception {
+        setupStreams();
         setupArgsForMainClass(SECURITY_EXCEPTION);
         given(launcherSecurityManagerMock.getExitInvoked()).willReturn(true);
         given(launcherSecurityManagerMock.getExitCode()).willReturn(1);
@@ -304,6 +326,7 @@ public class TestLauncherAM {
             .expectedErrorReason("exit code ["+ EXIT_CODE_1 + "]")
             .withoutStackTrace();
 
+        Assert.assertTrue(outContent.toString().contains("java.lang.SecurityException"));
         assertFailedExecution();
     }