----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52399/#review151728 -----------------------------------------------------------
core/src/main/java/org/apache/oozie/util/Instrumentation.java (line 783) <https://reviews.apache.org/r/52399/#comment220201> Why is this change needed? core/src/main/resources/oozie-default.xml (line 2514) <https://reviews.apache.org/r/52399/#comment220202> I think we should make these property names consistent. Some start with oozie.ssl and some start with oozie.https but they're all related. I'd vote for oozie.https given that (a) we're using http(s) for everything and (b) ssl is technically outdated in favor of tls, though they're often used interchangebly; with oozie.https we don't have to think about that :) distro/pom.xml (lines 116 - 128) <https://reviews.apache.org/r/52399/#comment220203> What is this for? distro/src/main/bin/oozie-jetty-server.sh (line 1) <https://reviews.apache.org/r/52399/#comment220205> Please run all new/updated shell scripts through shellcheck (https://www.shellcheck.net/). It finds various problems and best practice violations (kind of like findbugs). You can even install it on your Mac with Homebrew or MacPorts. distro/src/main/bin/oozie-jetty-server.sh (lines 40 - 61) <https://reviews.apache.org/r/52399/#comment220204> IMO, we should move as many of these as possible to oozie-default/site. We had a bunch of these as JVM properties before because of Tomcat and other reasons, but it would be good to have them all in the same place as other config properties. At the very least, the https/ssl ones (the other ones are probably out of scope for this JIRA). distro/src/main/bin/oozie-jetty-server.sh (line 187) <https://reviews.apache.org/r/52399/#comment220206> Can we set this up in such that there is no war file when using Jetty? server/pom.xml (line 26) <https://reviews.apache.org/r/52399/#comment220207> Leave off the <version> in this pom, and instead define them all in the root pom. server/src/main/assemblies/empty.xml (lines 9 - 11) <https://reviews.apache.org/r/52399/#comment220208> Whitespace server/src/main/java/org/apache/oozie/server/EmbeddedOozieServer.java (lines 46 - 47) <https://reviews.apache.org/r/52399/#comment220209> Should these be configurable? IIRC, we had to increase one of these in Tomcat at one point for some reason. server/src/main/java/org/apache/oozie/server/EmbeddedOozieServer.java (line 84) <https://reviews.apache.org/r/52399/#comment220213> We now typically get the Configuration by doing ConfigurationService.get("property-name") server/src/main/java/org/apache/oozie/server/EmbeddedOozieServer.java (line 136) <https://reviews.apache.org/r/52399/#comment220211> It would be good to do a services.destroy() when shutting down because many Services have some sort of cleanup or disconnect that they do. server/src/main/java/org/apache/oozie/server/JspHandler.java (line 51) <https://reviews.apache.org/r/52399/#comment220212> This typically points to /tmp right? In a long running Oozie Server, might we run into problem where the OS cleans this up? - Robert Kanter On Oct. 6, 2016, 10:09 p.m., Attila Sasvari wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52399/ > ----------------------------------------------------------- > > (Updated Oct. 6, 2016, 10:09 p.m.) > > > Review request for oozie, András Piros, Peter Cseh, Peter Bacsko, and Robert > Kanter. > > > Repository: oozie-git > > > Description > ------- > > Embedding jetty into Oozie so that it can run as a standalone application. > The changes also try to address OOZIE-2317 (i.e. Tomcat 6 is EOL). > > New functionality > - New module (server) is added that sets up an embedded Jetty server and > start Oozie services. Servlet mapping is done by reading web.xml of webapp at > runtime. JSP is handled with custom code. Server version is not revealed in > server repsonses. > - SSL protocols and ciphers can be configured via system properties and > environment variables. Precedence: system properties, environment variables, > default values > > Changes in existing code > - Excluded jetty 6 dependencies from core and updated tests accordingly > > Packaging > - oozie.sh is modified so that it starts Oozie with embedded jetty by > default. If someone would like to use tomcat for any reason, they can set an > environment variable (e.g. OOZIE_USE_TOMCAT=1). > > TODO: > - Add more tests > - Add more documentation > - Code cleanup + refactoring in packaging and core parts > - Maven clean up > - Allow to tune more Jetty settings (for example threadpool) > - More security measures (e.g. protect against clickjacking, CSRF, etc.) > - Update Oozie Documentation > > > Diffs > ----- > > core/src/main/java/org/apache/oozie/util/Instrumentation.java > fa1e92a0f1fadfb66c5e66fea5f26f57579080e7 > core/src/main/resources/oozie-default.xml > e71ebe3b7a85e6b23176ef30713af63847144498 > distro/pom.xml c50572c57a376b28963d4e7da8ac7df777fe0480 > distro/src/main/bin/oozie-jetty-server.sh PRE-CREATION > distro/src/main/bin/oozie-setup.sh 79b049bccceb2690f8a673a885a615c8d4d9578c > distro/src/main/bin/oozie-sys.sh 97d55a2b69c34ede007d4f65cdfc66f1ac2cfd13 > distro/src/main/bin/oozied.sh a869c3da177c863a068f2af45c7bca9d5cb771ac > pom.xml 704a2eeee9f4e4805e3e08c2a547b2a375f6b1b2 > server/pom.xml PRE-CREATION > server/src/main/assemblies/empty.xml PRE-CREATION > server/src/main/java/org/apache/oozie/server/EmbeddedOozieServer.java > PRE-CREATION > server/src/main/java/org/apache/oozie/server/JspHandler.java PRE-CREATION > server/src/main/java/org/apache/oozie/server/SSLServerConnectorFactory.java > PRE-CREATION > server/src/main/resources/checkstyle-header.txt PRE-CREATION > server/src/main/resources/checkstyle.xml PRE-CREATION > server/src/test/java/org/apache/oozie/server/TestEmbeddedOozieServer.java > PRE-CREATION > > server/src/test/java/org/apache/oozie/server/TestSSLServerConnectorFactory.java > PRE-CREATION > src/main/assemblies/distro.xml 1ffbfd6d2ba33b390999e9094cbb336fbce45c21 > > Diff: https://reviews.apache.org/r/52399/diff/ > > > Testing > ------- > > - Tested basic functionality by executing a workflow that uses the sample > JavaAction > - without SSL - on a 2.4.0 pseudo Hadoop cluster > - SSL with Kerberos is using a test CDH cluster > - Added new unit tests that check > - If oozie.ssl.enabled is not specified, server starts without SSL > settings > - If oozie.ssl.enabled is specified, server starts with SSL settings > - SSL protocols and ciphers can be configured via system properties and > environment variables > - Ran subset of tests using Hadoop-2 profile > - mvn clean package assembly:single -DjavaVersion=1.8 > -DtargetVersion=1.7 -Dtest=TestJavaActionExecutor -Phadoop-2 > -Dhadoop.version=2.4.0 > > > Thanks, > > Attila Sasvari > >
