From 94cebd59b819a5ad831fa3175a1f698876c6b374 Mon Sep 17 00:00:00 2001 From: Andy LoPresto Date: Thu, 3 May 2018 16:02:19 -0700 Subject: [PATCH 1/5] NIFI-5146 Added logic to check for simultaneous configuration of HTTP and HTTPS connectors in JettyServer. --- .../apache/nifi/web/server/JettyServer.java | 202 +++++++++++------- 1 file changed, 120 insertions(+), 82 deletions(-) diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java index de53a2449ecb..9fd7123849a5 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java @@ -601,99 +601,137 @@ private void configureConnectors(final Server server) throws ServerConfiguration httpConfiguration.setRequestHeaderSize(headerSize); httpConfiguration.setResponseHeaderSize(headerSize); - if (props.getPort() != null) { - final Integer port = props.getPort(); - if (port < 0 || (int) Math.pow(2, 16) <= port) { - throw new ServerConfigurationException("Invalid HTTP port: " + port); - } - - logger.info("Configuring Jetty for HTTP on port: " + port); + // Check if both HTTP and HTTPS connectors are configured and fail if both are configured + if (bothHttpAndHttpsConnectorsConfigured()) { + logger.error("NiFi only supports one mode of HTTP or HTTPS operation, not both simultaneously. Check the nifi.properties file and ensure that either the HTTP hostname and port or the HTTPS hostname and port are empty"); + startUpFailure(new IllegalStateException("Only one of the HTTP and HTTPS connectors can be configured at one time")); + } - final List serverConnectors = Lists.newArrayList(); + if (props.getSslPort() != null) { + configureHttpsConnector(server, httpConfiguration); + } else if (props.getPort() != null) { + configureHttpConnector(server, httpConfiguration); + } else { + logger.error("Neither the HTTP nor HTTPS connector was configured in nifi.properties"); + startUpFailure(new IllegalStateException("Must configure HTTP or HTTPS connector")); + } + } - final Map httpNetworkInterfaces = props.getHttpNetworkInterfaces(); - if (httpNetworkInterfaces.isEmpty() || httpNetworkInterfaces.values().stream().filter(value -> !Strings.isNullOrEmpty(value)).collect(Collectors.toList()).isEmpty()) { - // create the connector - final ServerConnector http = new ServerConnector(server, new HttpConnectionFactory(httpConfiguration)); - // set host and port - if (StringUtils.isNotBlank(props.getProperty(NiFiProperties.WEB_HTTP_HOST))) { - http.setHost(props.getProperty(NiFiProperties.WEB_HTTP_HOST)); - } - http.setPort(port); - serverConnectors.add(http); - } else { - // add connectors for all IPs from http network interfaces - serverConnectors.addAll(Lists.newArrayList(httpNetworkInterfaces.values().stream().map(ifaceName -> { - NetworkInterface iface = null; - try { - iface = NetworkInterface.getByName(ifaceName); - } catch (SocketException e) { - logger.error("Unable to get network interface by name {}", ifaceName, e); - } - if (iface == null) { - logger.warn("Unable to find network interface named {}", ifaceName); - } - return iface; - }).filter(Objects::nonNull).flatMap(iface -> Collections.list(iface.getInetAddresses()).stream()) - .map(inetAddress -> { - // create the connector - final ServerConnector http = new ServerConnector(server, new HttpConnectionFactory(httpConfiguration)); - // set host and port - http.setHost(inetAddress.getHostAddress()); - http.setPort(port); - return http; - }).collect(Collectors.toList()))); - } - // add all connectors - serverConnectors.forEach(server::addConnector); + // TODO: Extract common functionality to single method and provide lambda for configure connector + private void configureHttpsConnector(Server server, HttpConfiguration httpConfiguration) { + final Integer port = props.getSslPort(); + if (port < 0 || (int) Math.pow(2, 16) <= port) { + throw new ServerConfigurationException("Invalid HTTPs port: " + port); } - if (props.getSslPort() != null) { - final Integer port = props.getSslPort(); - if (port < 0 || (int) Math.pow(2, 16) <= port) { - throw new ServerConfigurationException("Invalid HTTPs port: " + port); + logger.info("Configuring Jetty for HTTPs on port: " + port); + + final List serverConnectors = Lists.newArrayList(); + + final Map httpsNetworkInterfaces = props.getHttpsNetworkInterfaces(); + if (httpsNetworkInterfaces.isEmpty() || httpsNetworkInterfaces.values().stream().filter(value -> !Strings.isNullOrEmpty(value)).collect(Collectors.toList()).isEmpty()) { + final ServerConnector https = createUnconfiguredSslServerConnector(server, httpConfiguration); + + // set host and port + if (StringUtils.isNotBlank(props.getProperty(NiFiProperties.WEB_HTTPS_HOST))) { + https.setHost(props.getProperty(NiFiProperties.WEB_HTTPS_HOST)); } + https.setPort(port); + serverConnectors.add(https); + } else { + // add connectors for all IPs from https network interfaces + serverConnectors.addAll(Lists.newArrayList(httpsNetworkInterfaces.values().stream().map(ifaceName -> { + NetworkInterface iface = null; + try { + iface = NetworkInterface.getByName(ifaceName); + } catch (SocketException e) { + logger.error("Unable to get network interface by name {}", ifaceName, e); + } + if (iface == null) { + logger.warn("Unable to find network interface named {}", ifaceName); + } + return iface; + }).filter(Objects::nonNull).flatMap(iface -> Collections.list(iface.getInetAddresses()).stream()) + .map(inetAddress -> { + final ServerConnector https = createUnconfiguredSslServerConnector(server, httpConfiguration); + + // set host and port + https.setHost(inetAddress.getHostAddress()); + https.setPort(port); + return https; + }).collect(Collectors.toList()))); + } + // add all connectors + serverConnectors.forEach(server::addConnector); + } - logger.info("Configuring Jetty for HTTPs on port: " + port); + private void configureHttpConnector(Server server, HttpConfiguration httpConfiguration) { + final Integer port = props.getPort(); + if (port < 0 || (int) Math.pow(2, 16) <= port) { + throw new ServerConfigurationException("Invalid HTTP port: " + port); + } - final List serverConnectors = Lists.newArrayList(); + logger.info("Configuring Jetty for HTTP on port: " + port); - final Map httpsNetworkInterfaces = props.getHttpsNetworkInterfaces(); - if (httpsNetworkInterfaces.isEmpty() || httpsNetworkInterfaces.values().stream().filter(value -> !Strings.isNullOrEmpty(value)).collect(Collectors.toList()).isEmpty()) { - final ServerConnector https = createUnconfiguredSslServerConnector(server, httpConfiguration); + final List serverConnectors = Lists.newArrayList(); - // set host and port - if (StringUtils.isNotBlank(props.getProperty(NiFiProperties.WEB_HTTPS_HOST))) { - https.setHost(props.getProperty(NiFiProperties.WEB_HTTPS_HOST)); - } - https.setPort(port); - serverConnectors.add(https); - } else { - // add connectors for all IPs from https network interfaces - serverConnectors.addAll(Lists.newArrayList(httpsNetworkInterfaces.values().stream().map(ifaceName -> { - NetworkInterface iface = null; - try { - iface = NetworkInterface.getByName(ifaceName); - } catch (SocketException e) { - logger.error("Unable to get network interface by name {}", ifaceName, e); - } - if (iface == null) { - logger.warn("Unable to find network interface named {}", ifaceName); - } - return iface; - }).filter(Objects::nonNull).flatMap(iface -> Collections.list(iface.getInetAddresses()).stream()) - .map(inetAddress -> { - final ServerConnector https = createUnconfiguredSslServerConnector(server, httpConfiguration); - - // set host and port - https.setHost(inetAddress.getHostAddress()); - https.setPort(port); - return https; - }).collect(Collectors.toList()))); + final Map httpNetworkInterfaces = props.getHttpNetworkInterfaces(); + if (httpNetworkInterfaces.isEmpty() || httpNetworkInterfaces.values().stream().filter(value -> !Strings.isNullOrEmpty(value)).collect(Collectors.toList()).isEmpty()) { + // create the connector + final ServerConnector http = new ServerConnector(server, new HttpConnectionFactory(httpConfiguration)); + // set host and port + if (StringUtils.isNotBlank(props.getProperty(NiFiProperties.WEB_HTTP_HOST))) { + http.setHost(props.getProperty(NiFiProperties.WEB_HTTP_HOST)); } - // add all connectors - serverConnectors.forEach(server::addConnector); + http.setPort(port); + serverConnectors.add(http); + } else { + // add connectors for all IPs from http network interfaces + serverConnectors.addAll(Lists.newArrayList(httpNetworkInterfaces.values().stream().map(ifaceName -> { + NetworkInterface iface = null; + try { + iface = NetworkInterface.getByName(ifaceName); + } catch (SocketException e) { + logger.error("Unable to get network interface by name {}", ifaceName, e); + } + if (iface == null) { + logger.warn("Unable to find network interface named {}", ifaceName); + } + return iface; + }).filter(Objects::nonNull).flatMap(iface -> Collections.list(iface.getInetAddresses()).stream()) + .map(inetAddress -> { + // create the connector + final ServerConnector http = new ServerConnector(server, new HttpConnectionFactory(httpConfiguration)); + // set host and port + http.setHost(inetAddress.getHostAddress()); + http.setPort(port); + return http; + }).collect(Collectors.toList()))); } + // add all connectors + serverConnectors.forEach(server::addConnector); + } + + /** + * Returns true if there are configured properties for both HTTP and HTTPS connectors (specifically port because the hostname can be left blank in the HTTP connector). Prints a warning log message with the relevant properties. + * + * @return true if both ports are present + */ + private boolean bothHttpAndHttpsConnectorsConfigured() { + Integer httpPort = props.getPort(); + String httpHostname = props.getProperty(NiFiProperties.WEB_HTTP_HOST); + + Integer httpsPort = props.getSslPort(); + String httpsHostname = props.getProperty(NiFiProperties.WEB_HTTPS_HOST); + + if (httpPort != null && httpsPort != null) { + logger.warn("Both the HTTP and HTTPS connectors are configured in nifi.properties. Only one of these connectors should be configured. See the NiFi Admin Guide for more details"); + logger.warn("HTTP connector: http://" + httpHostname + ":" + httpPort); + logger.warn("HTTPS connector: https://" + httpsHostname + ":" + httpsPort); + return true; + } + + return false; } private ServerConnector createUnconfiguredSslServerConnector(Server server, HttpConfiguration httpConfiguration) { From ce227cc2d3129f0b4d66dda2acdf0ce00f35a983 Mon Sep 17 00:00:00 2001 From: Andy LoPresto Date: Fri, 4 May 2018 18:16:34 -0700 Subject: [PATCH 2/5] NIFI-5146 Added test logging resources. Added unit tests. --- .../nifi-web/nifi-jetty/pom.xml | 26 ++ .../apache/nifi/web/server/JettyServer.java | 12 +- .../web/server/JettyServerGroovyTest.groovy | 228 ++++++++++++++++++ .../src/test/resources/log4j.properties | 27 +++ 4 files changed, 289 insertions(+), 4 deletions(-) create mode 100644 nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/groovy/org/apache/nifi/web/server/JettyServerGroovyTest.groovy create mode 100644 nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/resources/log4j.properties diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/pom.xml b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/pom.xml index 80a2a9cb5a52..3d1c6ba16ead 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/pom.xml +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/pom.xml @@ -22,7 +22,27 @@ nifi-jetty jar + + + + org.codehaus.groovy + groovy-all + 2.4.13 + compile + + + org.slf4j + slf4j-log4j12 + + + + + + + org.slf4j + slf4j-log4j12 + org.apache.nifi nifi-api @@ -168,6 +188,12 @@ nifi-framework-cluster compile + + com.github.stefanbirkner + system-rules + 1.16.0 + test + diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java index 9fd7123849a5..7cc4ddbc6b02 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java @@ -602,8 +602,9 @@ private void configureConnectors(final Server server) throws ServerConfiguration httpConfiguration.setResponseHeaderSize(headerSize); // Check if both HTTP and HTTPS connectors are configured and fail if both are configured - if (bothHttpAndHttpsConnectorsConfigured()) { - logger.error("NiFi only supports one mode of HTTP or HTTPS operation, not both simultaneously. Check the nifi.properties file and ensure that either the HTTP hostname and port or the HTTPS hostname and port are empty"); + if (bothHttpAndHttpsConnectorsConfigured(props)) { + logger.error("NiFi only supports one mode of HTTP or HTTPS operation, not both simultaneously. " + + "Check the nifi.properties file and ensure that either the HTTP hostname and port or the HTTPS hostname and port are empty"); startUpFailure(new IllegalStateException("Only one of the HTTP and HTTPS connectors can be configured at one time")); } @@ -713,11 +714,14 @@ private void configureHttpConnector(Server server, HttpConfiguration httpConfigu } /** - * Returns true if there are configured properties for both HTTP and HTTPS connectors (specifically port because the hostname can be left blank in the HTTP connector). Prints a warning log message with the relevant properties. + * Returns true if there are configured properties for both HTTP and HTTPS connectors (specifically port because the hostname can be left blank in the HTTP connector). + * Prints a warning log message with the relevant properties. + * + * @param props the NiFiProperties * * @return true if both ports are present */ - private boolean bothHttpAndHttpsConnectorsConfigured() { + static boolean bothHttpAndHttpsConnectorsConfigured(NiFiProperties props) { Integer httpPort = props.getPort(); String httpHostname = props.getProperty(NiFiProperties.WEB_HTTP_HOST); diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/groovy/org/apache/nifi/web/server/JettyServerGroovyTest.groovy b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/groovy/org/apache/nifi/web/server/JettyServerGroovyTest.groovy new file mode 100644 index 000000000000..a468c9803da6 --- /dev/null +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/groovy/org/apache/nifi/web/server/JettyServerGroovyTest.groovy @@ -0,0 +1,228 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.web.server + +import org.apache.log4j.AppenderSkeleton +import org.apache.log4j.spi.LoggingEvent +import org.apache.nifi.bundle.Bundle +import org.apache.nifi.properties.StandardNiFiProperties +import org.apache.nifi.util.NiFiProperties +import org.bouncycastle.jce.provider.BouncyCastleProvider +import org.junit.After +import org.junit.AfterClass +import org.junit.Before +import org.junit.BeforeClass +import org.junit.Rule +import org.junit.Test +import org.junit.contrib.java.lang.system.Assertion +import org.junit.contrib.java.lang.system.ExpectedSystemExit +import org.junit.contrib.java.lang.system.SystemErrRule +import org.junit.contrib.java.lang.system.SystemOutRule +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import org.slf4j.Logger +import org.slf4j.LoggerFactory + +import java.security.Security + +@RunWith(JUnit4.class) +class JettyServerGroovyTest extends GroovyTestCase { + private static final Logger logger = LoggerFactory.getLogger(JettyServerGroovyTest.class) + + @Rule + public final ExpectedSystemExit exit = ExpectedSystemExit.none() + + @Rule + public final SystemOutRule systemOutRule = new SystemOutRule().enableLog() + + @Rule + public final SystemErrRule systemErrRule = new SystemErrRule().enableLog() + + @BeforeClass + static void setUpOnce() throws Exception { + Security.addProvider(new BouncyCastleProvider()) + + logger.metaClass.methodMissing = { String name, args -> + logger.info("[${name?.toUpperCase()}] ${(args as List).join(" ")}") + } + } + + @AfterClass + static void tearDownOnce() throws Exception { + + } + + @Before + void setUp() throws Exception { + + } + + @After + void tearDown() throws Exception { + TestAppender.reset() + } + + @Test + void testShouldDetectHttpAndHttpsConfigurationsBothPresent() { + // Arrange + Map badProps = [ + (NiFiProperties.WEB_HTTP_HOST) : "localhost", + (NiFiProperties.WEB_HTTPS_HOST): "secure.host.com", + (NiFiProperties.WEB_THREADS) : NiFiProperties.DEFAULT_WEB_THREADS + ] + NiFiProperties mockProps = [ + getPort : { -> 8080 }, + getSslPort : { -> 8443 }, + getProperty: { String prop -> + String value = badProps[prop] ?: "no_value" + logger.mock("getProperty(${prop}) -> ${value}") + value + }, + ] as StandardNiFiProperties + + // Act + boolean bothConfigsPresent = JettyServer.bothHttpAndHttpsConnectorsConfigured(mockProps) + logger.info("Both configs present: ${bothConfigsPresent}") + def log = TestAppender.getLogLines() + + // Assert + assert bothConfigsPresent + assert !log.isEmpty() + assert log.first() =~ "Both the HTTP and HTTPS connectors are configured in nifi.properties. Only one of these connectors should be configured. See the NiFi Admin Guide for more details" + } + + @Test + void testDetectHttpAndHttpsConfigurationsShouldAllowEither() { + // Arrange + Map httpMap = [ + (NiFiProperties.WEB_HTTP_HOST) : "localhost", + (NiFiProperties.WEB_HTTPS_HOST): null, + ] + NiFiProperties httpProps = [ + getPort : { -> 8080 }, + getSslPort : { -> null }, + getProperty: { String prop -> + String value = httpMap[prop] ?: "no_value" + logger.mock("getProperty(${prop}) -> ${value}") + value + }, + ] as StandardNiFiProperties + + Map httpsMap = [ + (NiFiProperties.WEB_HTTP_HOST) : null, + (NiFiProperties.WEB_HTTPS_HOST): "secure.host.com", + ] + NiFiProperties httpsProps = [ + getPort : { -> null }, + getSslPort : { -> 8443 }, + getProperty: { String prop -> + String value = httpsMap[prop] ?: "no_value" + logger.mock("getProperty(${prop}) -> ${value}") + value + }, + ] as StandardNiFiProperties + + // Act + boolean bothConfigsPresentForHttp = JettyServer.bothHttpAndHttpsConnectorsConfigured(httpProps) + logger.info("Both configs present for HTTP properties: ${bothConfigsPresentForHttp}") + + boolean bothConfigsPresentForHttps = JettyServer.bothHttpAndHttpsConnectorsConfigured(httpsProps) + logger.info("Both configs present for HTTPS properties: ${bothConfigsPresentForHttps}") + def log = TestAppender.getLogLines() + + // Assert + assert !bothConfigsPresentForHttp + assert !bothConfigsPresentForHttps + + // Verifies that the warning was not logged + assert log.size() == 2 + assert log.first() == "Both configs present for HTTP properties: false" + assert log.last() == "Both configs present for HTTPS properties: false" + } + + @Test + void testShouldFailToStartWithHttpAndHttpsConfigurationsBothPresent() { + // Arrange + Map badProps = [ + (NiFiProperties.WEB_HTTP_HOST) : "localhost", + (NiFiProperties.WEB_HTTPS_HOST): "secure.host.com", + ] + NiFiProperties mockProps = [ + getPort : { -> 8080 }, + getSslPort : { -> 8443 }, + getProperty: { String prop -> + String value = badProps[prop] ?: "no_value" + logger.mock("getProperty(${prop}) -> ${value}") + value + }, + getWebThreads: { -> NiFiProperties.DEFAULT_WEB_THREADS }, + getWebMaxHeaderSize: { -> NiFiProperties.DEFAULT_WEB_MAX_HEADER_SIZE }, + isHTTPSConfigured: { -> true } + ] as StandardNiFiProperties + + // The web server should fail to start and exit Java + exit.expectSystemExitWithStatus(1) + exit.checkAssertionAfterwards(new Assertion() { + void checkAssertion() { + final String standardErr = systemErrRule.getLog() + List errLines = standardErr.split("\n") + + assert errLines.any { it =~ "Failed to start web server: "} + assert errLines.any { it =~ "Shutting down..."} + } + }) + + // Act + JettyServer jettyServer = new JettyServer(mockProps, [] as Set) + + // Assert + + // Assertions defined above + } +} + +class TestAppender extends AppenderSkeleton { + static final List events = new ArrayList<>() + + @Override + protected void append(LoggingEvent e) { + synchronized (events) { + events.add(e) + } + } + + static void reset() { + synchronized (events) { + events.clear() + } + } + + @Override + void close() { + } + + @Override + boolean requiresLayout() { + return false + } + + static List getLogLines() { + synchronized (events) { + events.collect { LoggingEvent le -> le.getRenderedMessage() } + } + } +} \ No newline at end of file diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/resources/log4j.properties b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/resources/log4j.properties new file mode 100644 index 000000000000..4608f9e6437d --- /dev/null +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/resources/log4j.properties @@ -0,0 +1,27 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +log4j.rootLogger=DEBUG,console,test + +log4j.appender.console=org.apache.log4j.ConsoleAppender +log4j.appender.console.Target=System.err +log4j.appender.console.layout=org.apache.log4j.PatternLayout +log4j.appender.console.layout.ConversionPattern=%d{yy/MM/dd HH:mm:ss} %p %c{2}: %m%n + +log4j.appender.test=org.apache.nifi.web.server.TestAppender +log4j.appender.test.layout=org.apache.log4j.PatternLayout +log4j.appender.test.layout.ConversionPattern=%d{yy/MM/dd HH:mm:ss} %p %c{2}: %m%n \ No newline at end of file From 7ab895c07b2bde4d557077aa8f2188abaf36637e Mon Sep 17 00:00:00 2001 From: Andy LoPresto Date: Fri, 4 May 2018 20:26:01 -0700 Subject: [PATCH 3/5] NIFI-5146 Refactored shared functionality to generic method which accepts lambdas. Fixed unit test with logging side effects. --- .../apache/nifi/web/server/JettyServer.java | 131 +++++++++--------- .../web/server/JettyServerGroovyTest.groovy | 2 + 2 files changed, 68 insertions(+), 65 deletions(-) diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java index 7cc4ddbc6b02..a67c37b4e72b 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java @@ -618,77 +618,73 @@ private void configureConnectors(final Server server) throws ServerConfiguration } } - // TODO: Extract common functionality to single method and provide lambda for configure connector + /** + * Configures an HTTPS connector and adds it to the server. + * + * @param server the Jetty server instance + * @param httpConfiguration the configuration object for the HTTPS protocol settings + */ private void configureHttpsConnector(Server server, HttpConfiguration httpConfiguration) { + String hostname = props.getProperty(NiFiProperties.WEB_HTTP_HOST); final Integer port = props.getSslPort(); - if (port < 0 || (int) Math.pow(2, 16) <= port) { - throw new ServerConfigurationException("Invalid HTTPs port: " + port); - } - - logger.info("Configuring Jetty for HTTPs on port: " + port); + String connectorLabel = "HTTPS"; + final Map httpNetworkInterfaces = props.getHttpsNetworkInterfaces(); + ServerConnectorCreator scc = (s, c) -> createUnconfiguredSslServerConnector(s, c, port); - final List serverConnectors = Lists.newArrayList(); - - final Map httpsNetworkInterfaces = props.getHttpsNetworkInterfaces(); - if (httpsNetworkInterfaces.isEmpty() || httpsNetworkInterfaces.values().stream().filter(value -> !Strings.isNullOrEmpty(value)).collect(Collectors.toList()).isEmpty()) { - final ServerConnector https = createUnconfiguredSslServerConnector(server, httpConfiguration); - - // set host and port - if (StringUtils.isNotBlank(props.getProperty(NiFiProperties.WEB_HTTPS_HOST))) { - https.setHost(props.getProperty(NiFiProperties.WEB_HTTPS_HOST)); - } - https.setPort(port); - serverConnectors.add(https); - } else { - // add connectors for all IPs from https network interfaces - serverConnectors.addAll(Lists.newArrayList(httpsNetworkInterfaces.values().stream().map(ifaceName -> { - NetworkInterface iface = null; - try { - iface = NetworkInterface.getByName(ifaceName); - } catch (SocketException e) { - logger.error("Unable to get network interface by name {}", ifaceName, e); - } - if (iface == null) { - logger.warn("Unable to find network interface named {}", ifaceName); - } - return iface; - }).filter(Objects::nonNull).flatMap(iface -> Collections.list(iface.getInetAddresses()).stream()) - .map(inetAddress -> { - final ServerConnector https = createUnconfiguredSslServerConnector(server, httpConfiguration); - - // set host and port - https.setHost(inetAddress.getHostAddress()); - https.setPort(port); - return https; - }).collect(Collectors.toList()))); - } - // add all connectors - serverConnectors.forEach(server::addConnector); + configureGenericConnector(server, httpConfiguration, hostname, port, connectorLabel, httpNetworkInterfaces, scc); } + /** + * Configures an HTTP connector and adds it to the server. + * + * @param server the Jetty server instance + * @param httpConfiguration the configuration object for the HTTP protocol settings + */ private void configureHttpConnector(Server server, HttpConfiguration httpConfiguration) { + String hostname = props.getProperty(NiFiProperties.WEB_HTTP_HOST); final Integer port = props.getPort(); + String connectorLabel = "HTTP"; + final Map httpNetworkInterfaces = props.getHttpNetworkInterfaces(); + ServerConnectorCreator scc = (s, c) -> new ServerConnector(s, new HttpConnectionFactory(c)); + + configureGenericConnector(server, httpConfiguration, hostname, port, connectorLabel, httpNetworkInterfaces, scc); + } + + /** + * Configures an HTTP(S) connector for the server given the provided parameters. The functionality between HTTP and HTTPS connectors is largely similar. + * Here the common behavior has been extracted into a shared method and the respective calling methods obtain the right values and a lambda function for the differing behavior. + * + * @param server the Jetty server instance + * @param configuration the HTTP/HTTPS configuration instance + * @param hostname the hostname from the nifi.properties file + * @param port the port to expose + * @param connectorLabel used for log output (e.g. "HTTP" or "HTTPS") + * @param networkInterfaces the map of network interfaces from nifi.properties + * @param serverConnectorCreator a function which accepts a {@code Server} and {@code HttpConnection} instance and returns a {@code ServerConnector} + */ + private void configureGenericConnector(Server server, HttpConfiguration configuration, String hostname, Integer port, String connectorLabel, Map networkInterfaces, + ServerConnectorCreator serverConnectorCreator) { if (port < 0 || (int) Math.pow(2, 16) <= port) { - throw new ServerConfigurationException("Invalid HTTP port: " + port); + throw new ServerConfigurationException("Invalid " + connectorLabel + " port: " + port); } - logger.info("Configuring Jetty for HTTP on port: " + port); + logger.info("Configuring Jetty for " + connectorLabel + " on port: " + port); final List serverConnectors = Lists.newArrayList(); - final Map httpNetworkInterfaces = props.getHttpNetworkInterfaces(); - if (httpNetworkInterfaces.isEmpty() || httpNetworkInterfaces.values().stream().filter(value -> !Strings.isNullOrEmpty(value)).collect(Collectors.toList()).isEmpty()) { - // create the connector - final ServerConnector http = new ServerConnector(server, new HttpConnectionFactory(httpConfiguration)); - // set host and port - if (StringUtils.isNotBlank(props.getProperty(NiFiProperties.WEB_HTTP_HOST))) { - http.setHost(props.getProperty(NiFiProperties.WEB_HTTP_HOST)); + // If the interfaces collection is empty or each element is empty + if (networkInterfaces.isEmpty() || networkInterfaces.values().stream().filter(value -> !Strings.isNullOrEmpty(value)).collect(Collectors.toList()).isEmpty()) { + final ServerConnector serverConnector = serverConnectorCreator.create(server, configuration); + + // Set host and port + if (StringUtils.isNotBlank(hostname)) { + serverConnector.setHost(hostname); } - http.setPort(port); - serverConnectors.add(http); + serverConnector.setPort(port); + serverConnectors.add(serverConnector); } else { - // add connectors for all IPs from http network interfaces - serverConnectors.addAll(Lists.newArrayList(httpNetworkInterfaces.values().stream().map(ifaceName -> { + // Add connectors for all IPs from network interfaces + serverConnectors.addAll(Lists.newArrayList(networkInterfaces.values().stream().map(ifaceName -> { NetworkInterface iface = null; try { iface = NetworkInterface.getByName(ifaceName); @@ -701,15 +697,15 @@ private void configureHttpConnector(Server server, HttpConfiguration httpConfigu return iface; }).filter(Objects::nonNull).flatMap(iface -> Collections.list(iface.getInetAddresses()).stream()) .map(inetAddress -> { - // create the connector - final ServerConnector http = new ServerConnector(server, new HttpConnectionFactory(httpConfiguration)); - // set host and port - http.setHost(inetAddress.getHostAddress()); - http.setPort(port); - return http; + final ServerConnector serverConnector = serverConnectorCreator.create(server, configuration); + + // Set host and port + serverConnector.setHost(inetAddress.getHostAddress()); + serverConnector.setPort(port); + return serverConnector; }).collect(Collectors.toList()))); } - // add all connectors + // Add all connectors serverConnectors.forEach(server::addConnector); } @@ -738,11 +734,11 @@ static boolean bothHttpAndHttpsConnectorsConfigured(NiFiProperties props) { return false; } - private ServerConnector createUnconfiguredSslServerConnector(Server server, HttpConfiguration httpConfiguration) { + private ServerConnector createUnconfiguredSslServerConnector(Server server, HttpConfiguration httpConfiguration, int port) { // add some secure config final HttpConfiguration httpsConfiguration = new HttpConfiguration(httpConfiguration); httpsConfiguration.setSecureScheme("https"); - httpsConfiguration.setSecurePort(props.getSslPort()); + httpsConfiguration.setSecurePort(port); httpsConfiguration.addCustomizer(new SecureRequestCustomizer()); // build the connector @@ -1032,3 +1028,8 @@ public void destroy() { } }; } + +@FunctionalInterface +interface ServerConnectorCreator { + ServerConnector create(Server server, HttpConfiguration httpConfiguration); +} \ No newline at end of file diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/groovy/org/apache/nifi/web/server/JettyServerGroovyTest.groovy b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/groovy/org/apache/nifi/web/server/JettyServerGroovyTest.groovy index a468c9803da6..bb080692bf81 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/groovy/org/apache/nifi/web/server/JettyServerGroovyTest.groovy +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/groovy/org/apache/nifi/web/server/JettyServerGroovyTest.groovy @@ -59,6 +59,8 @@ class JettyServerGroovyTest extends GroovyTestCase { logger.metaClass.methodMissing = { String name, args -> logger.info("[${name?.toUpperCase()}] ${(args as List).join(" ")}") } + + TestAppender.reset() } @AfterClass From 77487439db2f5d28476ee65bfc4c6f0bae7b4bb3 Mon Sep 17 00:00:00 2001 From: Andy LoPresto Date: Mon, 7 May 2018 09:40:58 -0700 Subject: [PATCH 4/5] NIFI-5146 Added note about exclusive HTTP/HTTPS behavior to Admin Guide. Fixed typos. --- nifi-docs/src/main/asciidoc/administration-guide.adoc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/nifi-docs/src/main/asciidoc/administration-guide.adoc b/nifi-docs/src/main/asciidoc/administration-guide.adoc index 64facc7e1439..b9857d64c34f 100644 --- a/nifi-docs/src/main/asciidoc/administration-guide.adoc +++ b/nifi-docs/src/main/asciidoc/administration-guide.adoc @@ -148,7 +148,7 @@ should run on. If it is desired that the HTTPS interface be accessible from all admins to configure the application to run only on specific network interfaces, `nifi.web.http.network.interface*` or `nifi.web.https.network.interface*` properties can be specified. -NOTE: It is important when enabling HTTPS that the `nifi.web.http.port` property be unset. +NOTE: It is important when enabling HTTPS that the `nifi.web.http.port` property be unset. NiFi only supports running on HTTP *or* HTTPS, not both simultaneously. Similar to `nifi.security.needClientAuth`, the web server can be configured to require certificate based client authentication for users accessing the User Interface. In order to do this it must be configured to not support username/password authentication using <> or <>. Either of these options @@ -1967,8 +1967,8 @@ they must be set the same on every instance in the cluster. For each Node, the minimum properties to configure are as follows: -* Under the _Web Properties_ section, set either the http or https port that you want the Node to run on. - Also, consider whether you need to set the http or https host property. +* Under the _Web Properties_ section, set either the HTTP or HTTPS port that you want the Node to run on. + Also, consider whether you need to set the HTTP or HTTPS host property. All nodes in the cluster should use the same protocol setting. * Under the _State Management section_, set the `nifi.state.management.provider.cluster` property to the identifier of the Cluster State Provider. Ensure that the Cluster State Provider has been configured in the _state-management.xml_ file. See <> for more information. @@ -2164,7 +2164,7 @@ In order to secure the communications, we need to ensure that both the client an NiFi ZooKeeper client and embedded ZooKeeper server to use Kerberos are provided below. If Kerberos is not already setup in your environment, you can find information on installing and setting up a Kerberos Server at -link:https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/6/html/Managing_Smart_Cards/Configuring_a_Kerberos_5_Server.html[https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/6/html/Managing_Smart_Cards/Configuring_a_Kerberos_5_Server.html^]. This guide assumes that Kerberos already has been installed in the environment in which NiFi is running. +https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/6/html/Managing_Smart_Cards/Configuring_a_Kerberos_5_Server.html[Red Hat Customer Portal: Configuring a Kerberos 5 Server]. This guide assumes that Kerberos already has been installed in the environment in which NiFi is running. Note, the following procedures for kerberizing an Embedded ZooKeeper server in your NiFi Node and kerberizing a ZooKeeper NiFi client will require that Kerberos client libraries be installed. This is accomplished in Fedora-based Linux distributions via: @@ -2652,7 +2652,7 @@ documentation of the proxy for guidance for your deployment environment and use ** By default, if NiFi is running securely it will only accept HTTP requests with a Host header matching the host[:port] that it is bound to. If NiFi is to accept requests directed to a different host[:port] the expected values need to be configured. This may be required when running behind a proxy or in a containerized environment. This is configured in a comma -separated list in _nifi.properties_ using the `nifi.web.proxy.host` property (e.g. localhost:18443, proxyhost:443). IPv6 addressed are accepted. Please refer to +separated list in _nifi.properties_ using the `nifi.web.proxy.host` property (e.g. localhost:18443, proxyhost:443). IPv6 addresses are accepted. Please refer to RFC 5952 Sections link:https://tools.ietf.org/html/rfc5952#section-4[4] and link:https://tools.ietf.org/html/rfc5952#section-6[6] for additional details. ** NiFi will only accept HTTP requests with a X-ProxyContextPath or X-Forwarded-Context header if the value is whitelisted in the `nifi.web.proxy.context.path` property in From 611306464d5bf1fc43bec686e52712167439dc55 Mon Sep 17 00:00:00 2001 From: Andy LoPresto Date: Tue, 8 May 2018 18:54:28 -0700 Subject: [PATCH 5/5] NIFI-5146 Fixed copy/paste error on property access. Added unit test. --- .../apache/nifi/web/server/JettyServer.java | 31 ++++++++++++------- .../web/server/JettyServerGroovyTest.groovy | 28 +++++++++++++++++ 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java index a67c37b4e72b..83b489f145bc 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java @@ -173,6 +173,14 @@ public JettyServer(final NiFiProperties props, final Set bundles) { server.setHandler(allHandlers); } + /** + * Instantiates this object but does not perform any configuration. Used for unit testing. + */ + JettyServer(Server server, NiFiProperties properties) { + this.server = server; + this.props = properties; + } + private Handler loadWars(final Set bundles) { // load WARs @@ -621,23 +629,23 @@ private void configureConnectors(final Server server) throws ServerConfiguration /** * Configures an HTTPS connector and adds it to the server. * - * @param server the Jetty server instance + * @param server the Jetty server instance * @param httpConfiguration the configuration object for the HTTPS protocol settings */ private void configureHttpsConnector(Server server, HttpConfiguration httpConfiguration) { - String hostname = props.getProperty(NiFiProperties.WEB_HTTP_HOST); + String hostname = props.getProperty(NiFiProperties.WEB_HTTPS_HOST); final Integer port = props.getSslPort(); String connectorLabel = "HTTPS"; - final Map httpNetworkInterfaces = props.getHttpsNetworkInterfaces(); + final Map httpsNetworkInterfaces = props.getHttpsNetworkInterfaces(); ServerConnectorCreator scc = (s, c) -> createUnconfiguredSslServerConnector(s, c, port); - configureGenericConnector(server, httpConfiguration, hostname, port, connectorLabel, httpNetworkInterfaces, scc); + configureGenericConnector(server, httpConfiguration, hostname, port, connectorLabel, httpsNetworkInterfaces, scc); } /** * Configures an HTTP connector and adds it to the server. * - * @param server the Jetty server instance + * @param server the Jetty server instance * @param httpConfiguration the configuration object for the HTTP protocol settings */ private void configureHttpConnector(Server server, HttpConfiguration httpConfiguration) { @@ -654,12 +662,12 @@ private void configureHttpConnector(Server server, HttpConfiguration httpConfigu * Configures an HTTP(S) connector for the server given the provided parameters. The functionality between HTTP and HTTPS connectors is largely similar. * Here the common behavior has been extracted into a shared method and the respective calling methods obtain the right values and a lambda function for the differing behavior. * - * @param server the Jetty server instance - * @param configuration the HTTP/HTTPS configuration instance - * @param hostname the hostname from the nifi.properties file - * @param port the port to expose - * @param connectorLabel used for log output (e.g. "HTTP" or "HTTPS") - * @param networkInterfaces the map of network interfaces from nifi.properties + * @param server the Jetty server instance + * @param configuration the HTTP/HTTPS configuration instance + * @param hostname the hostname from the nifi.properties file + * @param port the port to expose + * @param connectorLabel used for log output (e.g. "HTTP" or "HTTPS") + * @param networkInterfaces the map of network interfaces from nifi.properties * @param serverConnectorCreator a function which accepts a {@code Server} and {@code HttpConnection} instance and returns a {@code ServerConnector} */ private void configureGenericConnector(Server server, HttpConfiguration configuration, String hostname, Integer port, String connectorLabel, Map networkInterfaces, @@ -714,7 +722,6 @@ private void configureGenericConnector(Server server, HttpConfiguration configur * Prints a warning log message with the relevant properties. * * @param props the NiFiProperties - * * @return true if both ports are present */ static boolean bothHttpAndHttpsConnectorsConfigured(NiFiProperties props) { diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/groovy/org/apache/nifi/web/server/JettyServerGroovyTest.groovy b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/groovy/org/apache/nifi/web/server/JettyServerGroovyTest.groovy index bb080692bf81..174efc03daa1 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/groovy/org/apache/nifi/web/server/JettyServerGroovyTest.groovy +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/groovy/org/apache/nifi/web/server/JettyServerGroovyTest.groovy @@ -22,6 +22,10 @@ import org.apache.nifi.bundle.Bundle import org.apache.nifi.properties.StandardNiFiProperties import org.apache.nifi.util.NiFiProperties import org.bouncycastle.jce.provider.BouncyCastleProvider +import org.eclipse.jetty.server.Connector +import org.eclipse.jetty.server.HttpConfiguration +import org.eclipse.jetty.server.Server +import org.eclipse.jetty.server.ServerConnector import org.junit.After import org.junit.AfterClass import org.junit.Before @@ -195,6 +199,30 @@ class JettyServerGroovyTest extends GroovyTestCase { // Assertions defined above } + + @Test + void testShouldConfigureHTTPSConnector() { + // Arrange + NiFiProperties httpsProps = new StandardNiFiProperties(rawProperties: new Properties([ +// (NiFiProperties.WEB_HTTP_PORT): null, +// (NiFiProperties.WEB_HTTP_HOST): null, + (NiFiProperties.WEB_HTTPS_PORT): "8443", + (NiFiProperties.WEB_HTTPS_HOST): "secure.host.com", + ])) + + Server internalServer = new Server() + JettyServer jetty = new JettyServer(internalServer, httpsProps) + + // Act + jetty.configureHttpsConnector(internalServer, new HttpConfiguration()) + List connectors = Arrays.asList(internalServer.connectors) + + // Assert + assert connectors.size() == 1 + ServerConnector connector = connectors.first() as ServerConnector + assert connector.host == "secure.host.com" + assert connector.port == 8443 + } } class TestAppender extends AppenderSkeleton {