You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by ijokarumawak <gi...@git.apache.org> on 2018/03/05 06:51:06 UTC

[GitHub] nifi pull request #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

GitHub user ijokarumawak opened a pull request:

    https://github.com/apache/nifi/pull/2510

    NIFI-4932: Enable S2S work behind a Reverse Proxy

    Adding S2S endpoint Reverse Proxy mapping capability.
    
    Thank you for submitting a contribution to Apache NiFi.
    
    In order to streamline the review of the contribution we ask you
    to ensure the following steps have been taken:
    
    ### For all changes:
    - [x] Is there a JIRA ticket associated with this PR? Is it referenced 
         in the commit message?
    
    - [x] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
    
    - [x] Has your PR been rebased against the latest commit within the target branch (typically master)?
    
    - [x] Is your initial contribution a single, squashed commit?
    
    ### For code changes:
    - [x] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
    - [x] Have you written or updated unit tests to verify your changes?
    - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? 
    - [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
    - [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
    - [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?
    
    ### For documentation related changes:
    - [x] Have you ensured that format looks appropriate for the output in which it is rendered?
    
    ### Note:
    Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ijokarumawak/nifi nifi-4932

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/nifi/pull/2510.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2510
    
----
commit 9c86dde9089392f13afc20c1e2a91e62230efa6b
Author: Koji Kawamura <ij...@...>
Date:   2018-02-06T02:37:06Z

    NIFI-4932: Enable S2S work behind a Reverse Proxy
    
    Adding S2S endpoint Reverse Proxy mapping capability.

----


---

[GitHub] nifi issue #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:

    https://github.com/apache/nifi/pull/2510
  
    @mcgilman Thanks for reviewing. I've incorporated all feedback. Please take a look it again. Thanks!


---

[GitHub] nifi pull request #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/2510#discussion_r177925669
  
    --- Diff: nifi-docs/src/main/asciidoc/administration-guide.adoc ---
    @@ -3058,6 +3062,258 @@ responses from the remote system for `30 secs`. This allows NiFi to avoid consta
     has many instances of Remote Process Groups.
     |====
     
    +[[site_to_site_reverse_proxy_properties]]
    +=== Site to Site Routing Properties for Reverse Proxies
    +
    +Site-to-Site requires peer-to-peer communication between a client and a remote NiFi node. E.g. if a remote NiFi cluster has 3 nodes, nifi0, nifi1 and nifi2, then a client requests have to be reachable to each of those remote node.
    +
    +If a NiFi cluster is planned to receive/transfer data from/to Site-to-Site clients over the internet or a company firewall, a reverse proxy server can be deployed in front of the NiFi cluster nodes as a gateway to route client requests to upstream NiFi nodes, to reduce number of servers and ports those have to be exposed.
    +
    +In such environment, the same NiFi cluster would also be expected to be accessed by Site-to-Site clients within the same network. Sending FlowFiles to itself for load distribution among NiFi cluster nodes can be a typical example. In this case, client requests should be routed directly to a node without going through the reverse proxy.
    +
    +In order to support such deployments, remote NiFi clusters need to expose its Site-to-Site endpoints dynamically based on client request contexts. Following properties configure how peers should be exposed to clients. A routing definition consists of 4 properties, 'when', 'hostname', 'port', and 'secure', grouped by 'protocol' and 'name'. Multiple routing definitions can be configured. 'protocol' represents Site-to-Site transport protocol, i.e. raw or http.
    +
    +|====
    +|*Property*|*Description*
    +|nifi.remote.route.{protocol}.{name}.when|Boolean value, 'true' or 'false'. Controls whether the routing definition for this name should be used.
    +|nifi.remote.route.{protocol}.{name}.hostname|Specify hostname that will be introduced to Site-to-Site clients for further communications.
    +|nifi.remote.route.{protocol}.{name}.port|Specify port number that will be introduced to Site-to-Site clients for further communications.
    +|nifi.remote.route.{protocol}.{name}.secure|Boolean value, 'true' or 'false'. Specify whether the remote peer should be accessed via secure protocol.
    +|====
    +
    +All of above routing properties can use NiFi Expression Language to compute target peer description from request context. Available variables are:
    +
    +|===
    +|*Variable name*|*Description*
    +|s2s.{source\|target}.hostname|Hostname of the source where the request came from, and the original target.
    +|s2s.{source\|target}.port|Same as above, for ports. Source port may not be useful as it is just a client side TCP port.
    +|s2s.{source\|target}.secure|Same as above, for secure or not.
    +|s2s.protocol|The name of Site-to-Site protocol being used, RAW or HTTP.
    +|s2s.request|The name of current request type, SiteToSiteDetail or Peers. See Site-to-Site protocol sequence below for detail.
    +|HTTP request headers|HTTP request header values can be referred by its name.
    +|===
    +
    +==== Site to Site protocol sequence
    +
    +Configuring these properties correctly would require some understandings on Site-to-Site protocol sequence.
    +
    +1. A client initiates Site-to-Site protocol by sending a HTTP(S) request to the specified remote URL to get remote cluster Site-to-Site information. Specifically, to '/nifi-api/site-to-site'. This request is called 'SiteToSiteDetail'.
    +2. A remote NiFi node responds with its input and output ports, and TCP port numbers for RAW and TCP transport protocols.
    +3. The client sends another request to get remote peers using the TCP port number returned at #2. From this request, raw socket communication is used for RAW transport protocol, while HTTP keeps using HTTP(S). This request is called 'Peers'.
    +4. A remote NiFi node responds with list of available remote peers containing hostname, port, secure and workload such as the number of queued FlowFiles. From this point, further communication is done between the client and the remote NiFi node.
    +5. The client decides which peer to transfer data from/to, based on workload information.
    +6. The client sends a request to create a transaction to a remote NiFi node.
    +7. The remote NiFi node accepts the transaction.
    +8. Data is sent to the target peer. Multiple Data packets can be sent in batch manner.
    +9. When there is no more data to send, or reached to batch limit, the transaction is confirmed on both end by calculating CRC32 hash of sent data.
    +10. The transaction is committed on both end.
    +
    +==== Reverse Proxy Configurations
    +
    +Most reverse proxy software implement HTTP and TCP proxy mode. For NiFi RAW Site-to-Site protocol, both HTTP and TCP proxy configurations are required, and at least 2 ports needed to be opened. NiFi HTTP Site-to-Site protocol can minimize the required number of open ports at the reverse proxy to 1.
    +
    +Setting correct HTTP headers at reverse proxies are crucial for NiFi to work correctly, not only routing requests but also authorize client requests. See also <<proxy_configuration>> for details.
    +
    +There are two types of requests-to-NiFi-node mapping techniques those can be applied at reverse proxy servers. One is 'Server name to Node' and the other is 'Port number to Node'.
    +
    +With 'Server name to Node', the same port can be used to route requests to different upstream NiFi nodes based on the requested server name (e.g. nifi0.example.com, nifi1.example.com). Host name resolution should be configured to map different host names to the same reverse proxy address, that can be done by adding /etc/hosts file or DNS server entries. Also, if clients to reverse proxy uses HTTPS, reverse proxy server certificate should have wildcard common name or SAN to be accessed by different host names.
    +
    +Some reverse proxy technologies do not support server name routing rules, in such case, use 'Port number to Node' technique. 'Port number to Node' mapping requires N open port at a reverse proxy for a NiFi cluster consists of N nodes.
    +
    +Refer following examples for actual configurations.
    +
    +==== Site to Site and Reverse Proxy Examples
    +
    +Here are some example reverse proxy and NiFi setups to illustrate how configuration files look like.
    +
    +Client1 in the following diagrams represents a client that does not have direct access to NiFi nodes, and it accesses through the reverse proxy, while Client2 has direct access.
    +
    +In this example, Nginx is used as a reverse proxy.
    +
    +===== Example 1: RAW - Server name to Node mapping
    +
    +image:s2s-rproxy-servername.svg["Server name to Node mapping"]
    +
    +1. Client1 initiates Site-to-Site protocol, the request is routed to one of upstream NiFi nodes. The NiFi node computes Site-to-Site port for RAW. By routing 'example1', port 10443 is returned.
    +2. Client1 asks peers to 'nifi.example.com:10443', the request is routed to 'nifi0:8081'. The NiFi node computes available peers, by 'example1' routing rule, 'nifi0:8081' is converted to 'nifi0.example.com:10443', so are nifi1 and nifi2. As a result, 'nifi0.eample.com:10443', 'nifi1.example.com:10443' and 'nifi2.example.com:10443' are returned.
    +3. Client1 decides to use 'nifi2.example.com:10443' for further communication.
    +4. On the other hand, Client2 has two URIs for Site-to-Site bootstrap URIs, and initiates the protocol using one of them. The 'example1' routing does not match this for this request, and port 8081 is returned.
    +5. Client2 asks peers from 'nifi1:8081'. The 'example1' does not match, so the original 'nifi0:8081', 'nifi1:8081' and 'nifi2:8081' are returned as they are.
    +6. Client2 decides to use 'nifi2:8081' for further communication.
    +
    +nifi.properties (all node has the same routing configuration)
    --- End diff --
    
    Good point. I added more wording to the listed texts so that readers can be guided to the nifi.properties example. I wanted to make the diagram and the steps closer together.


---

[GitHub] nifi pull request #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/2510#discussion_r178174688
  
    --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-site-to-site/src/main/java/org/apache/nifi/remote/PeerDescriptionModifier.java ---
    @@ -0,0 +1,165 @@
    +/*
    + * 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.remote;
    +
    +import org.apache.nifi.attribute.expression.language.PreparedQuery;
    +import org.apache.nifi.attribute.expression.language.Query;
    +import org.apache.nifi.remote.protocol.SiteToSiteTransportProtocol;
    +import org.apache.nifi.util.NiFiProperties;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.List;
    +import java.util.Map;
    +import java.util.stream.Collectors;
    +
    +import static java.lang.String.format;
    +import static org.apache.commons.lang3.StringUtils.isBlank;
    +
    +public class PeerDescriptionModifier {
    +
    +    private static final Logger logger = LoggerFactory.getLogger(PeerDescriptionModifier.class);
    +
    +    public enum RequestType {
    +        SiteToSiteDetail,
    +        Peers
    +    }
    +
    +    private static class Route {
    +        private String name;
    +        private SiteToSiteTransportProtocol protocol;
    +        private PreparedQuery predicate;
    +        private PreparedQuery hostname;
    +        private PreparedQuery port;
    +        private PreparedQuery secure;
    +
    +        private Route validate() {
    +            if (hostname == null) {
    +                throw new IllegalArgumentException(
    +                        format("Found an invalid Site-to-Site route definition [%s] 'hostname' is not specified.", name));
    +            }
    +            if (port == null) {
    +                throw new IllegalArgumentException(
    +                        format("Found an invalid Site-to-Site route definition [%s] 'port' is not specified.", name));
    +            }
    +            return this;
    +        }
    +
    +        private PeerDescription getTarget(final Map<String, String> variables) {
    +            final String targetHostName = hostname.evaluateExpressions(variables, null);
    +            if (isBlank(targetHostName)) {
    +                throw new IllegalStateException("Target hostname was not resolved for the route definition " + name);
    +            }
    +
    +            final String targetPortStr = port.evaluateExpressions(variables, null);
    +            if (isBlank(targetPortStr)) {
    +                throw new IllegalStateException("Target port was not resolved for the route definition " + name);
    +            }
    +
    +            final String targetIsSecure = secure == null ? null : secure.evaluateExpressions(variables, null);
    +            return new PeerDescription(targetHostName, Integer.valueOf(targetPortStr), Boolean.valueOf(targetIsSecure));
    +        }
    +    }
    +
    +    private Map<SiteToSiteTransportProtocol, List<Route>> routes;
    +
    +
    +    private static final String PROPERTY_PREFIX = "nifi.remote.route.";
    +
    +    public PeerDescriptionModifier(final NiFiProperties properties) {
    +        final Map<String, List<String>> routeDefinitions = properties.getPropertyKeys().stream()
    +                .filter(propertyKey -> propertyKey.startsWith(PROPERTY_PREFIX))
    +                .collect(Collectors.groupingBy(propertyKey -> propertyKey.substring(PROPERTY_PREFIX.length(), propertyKey.lastIndexOf('.'))));
    +
    +        routes = routeDefinitions.entrySet().stream().map(routeDefinition -> {
    +            final Route route = new Route();
    +            // E.g. raw.example1, http.example2
    +            final String[] protocolAndRoutingName = routeDefinition.getKey().split("\\.");
    +            route.protocol = SiteToSiteTransportProtocol.valueOf(protocolAndRoutingName[0].toUpperCase());
    +            route.name = protocolAndRoutingName[1];
    +            routeDefinition.getValue().forEach(propertyKey -> {
    +                final String routingConfigName = propertyKey.substring(propertyKey.lastIndexOf('.') + 1);
    --- End diff --
    
    I believe we should still better handle the property parse. I indirectly mentioned this in the last review with my comment to IndexOutOfBoundsException. Sorry if that wasn't clear previously. If a user misconfigures their properties the error isn't helpful:
    
    
    > Caused by: java.lang.ArrayIndexOutOfBoundsException: 1
    > 	at org.apache.nifi.remote.PeerDescriptionModifier.lambda$new$4(PeerDescriptionModifier.java:93)
    > 	at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
    > 	at java.util.HashMap$EntrySpliterator.forEachRemaining(HashMap.java:1683)
    
    There is another `substring` method above that is likely susceptible to the same issue. 


---

[GitHub] nifi issue #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:

    https://github.com/apache/nifi/pull/2510
  
    Thanks @ijokarumawak! This has been merged to master.


---

[GitHub] nifi pull request #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/2510#discussion_r177924170
  
    --- Diff: nifi-docs/src/main/asciidoc/administration-guide.adoc ---
    @@ -3058,6 +3062,258 @@ responses from the remote system for `30 secs`. This allows NiFi to avoid consta
     has many instances of Remote Process Groups.
     |====
     
    +[[site_to_site_reverse_proxy_properties]]
    +=== Site to Site Routing Properties for Reverse Proxies
    +
    +Site-to-Site requires peer-to-peer communication between a client and a remote NiFi node. E.g. if a remote NiFi cluster has 3 nodes, nifi0, nifi1 and nifi2, then a client requests have to be reachable to each of those remote node.
    +
    +If a NiFi cluster is planned to receive/transfer data from/to Site-to-Site clients over the internet or a company firewall, a reverse proxy server can be deployed in front of the NiFi cluster nodes as a gateway to route client requests to upstream NiFi nodes, to reduce number of servers and ports those have to be exposed.
    +
    +In such environment, the same NiFi cluster would also be expected to be accessed by Site-to-Site clients within the same network. Sending FlowFiles to itself for load distribution among NiFi cluster nodes can be a typical example. In this case, client requests should be routed directly to a node without going through the reverse proxy.
    +
    +In order to support such deployments, remote NiFi clusters need to expose its Site-to-Site endpoints dynamically based on client request contexts. Following properties configure how peers should be exposed to clients. A routing definition consists of 4 properties, 'when', 'hostname', 'port', and 'secure', grouped by 'protocol' and 'name'. Multiple routing definitions can be configured. 'protocol' represents Site-to-Site transport protocol, i.e. raw or http.
    +
    +|====
    +|*Property*|*Description*
    +|nifi.remote.route.{protocol}.{name}.when|Boolean value, 'true' or 'false'. Controls whether the routing definition for this name should be used.
    +|nifi.remote.route.{protocol}.{name}.hostname|Specify hostname that will be introduced to Site-to-Site clients for further communications.
    +|nifi.remote.route.{protocol}.{name}.port|Specify port number that will be introduced to Site-to-Site clients for further communications.
    +|nifi.remote.route.{protocol}.{name}.secure|Boolean value, 'true' or 'false'. Specify whether the remote peer should be accessed via secure protocol.
    +|====
    +
    +All of above routing properties can use NiFi Expression Language to compute target peer description from request context. Available variables are:
    +
    +|===
    +|*Variable name*|*Description*
    +|s2s.{source\|target}.hostname|Hostname of the source where the request came from, and the original target.
    +|s2s.{source\|target}.port|Same as above, for ports. Source port may not be useful as it is just a client side TCP port.
    +|s2s.{source\|target}.secure|Same as above, for secure or not.
    +|s2s.protocol|The name of Site-to-Site protocol being used, RAW or HTTP.
    +|s2s.request|The name of current request type, SiteToSiteDetail or Peers. See Site-to-Site protocol sequence below for detail.
    +|HTTP request headers|HTTP request header values can be referred by its name.
    +|===
    +
    +==== Site to Site protocol sequence
    +
    +Configuring these properties correctly would require some understandings on Site-to-Site protocol sequence.
    +
    +1. A client initiates Site-to-Site protocol by sending a HTTP(S) request to the specified remote URL to get remote cluster Site-to-Site information. Specifically, to '/nifi-api/site-to-site'. This request is called 'SiteToSiteDetail'.
    +2. A remote NiFi node responds with its input and output ports, and TCP port numbers for RAW and TCP transport protocols.
    +3. The client sends another request to get remote peers using the TCP port number returned at #2. From this request, raw socket communication is used for RAW transport protocol, while HTTP keeps using HTTP(S). This request is called 'Peers'.
    +4. A remote NiFi node responds with list of available remote peers containing hostname, port, secure and workload such as the number of queued FlowFiles. From this point, further communication is done between the client and the remote NiFi node.
    +5. The client decides which peer to transfer data from/to, based on workload information.
    +6. The client sends a request to create a transaction to a remote NiFi node.
    +7. The remote NiFi node accepts the transaction.
    +8. Data is sent to the target peer. Multiple Data packets can be sent in batch manner.
    +9. When there is no more data to send, or reached to batch limit, the transaction is confirmed on both end by calculating CRC32 hash of sent data.
    +10. The transaction is committed on both end.
    +
    +==== Reverse Proxy Configurations
    +
    +Most reverse proxy software implement HTTP and TCP proxy mode. For NiFi RAW Site-to-Site protocol, both HTTP and TCP proxy configurations are required, and at least 2 ports needed to be opened. NiFi HTTP Site-to-Site protocol can minimize the required number of open ports at the reverse proxy to 1.
    +
    +Setting correct HTTP headers at reverse proxies are crucial for NiFi to work correctly, not only routing requests but also authorize client requests. See also <<proxy_configuration>> for details.
    +
    +There are two types of requests-to-NiFi-node mapping techniques those can be applied at reverse proxy servers. One is 'Server name to Node' and the other is 'Port number to Node'.
    +
    +With 'Server name to Node', the same port can be used to route requests to different upstream NiFi nodes based on the requested server name (e.g. nifi0.example.com, nifi1.example.com). Host name resolution should be configured to map different host names to the same reverse proxy address, that can be done by adding /etc/hosts file or DNS server entries. Also, if clients to reverse proxy uses HTTPS, reverse proxy server certificate should have wildcard common name or SAN to be accessed by different host names.
    +
    +Some reverse proxy technologies do not support server name routing rules, in such case, use 'Port number to Node' technique. 'Port number to Node' mapping requires N open port at a reverse proxy for a NiFi cluster consists of N nodes.
    +
    +Refer following examples for actual configurations.
    +
    +==== Site to Site and Reverse Proxy Examples
    +
    +Here are some example reverse proxy and NiFi setups to illustrate how configuration files look like.
    +
    +Client1 in the following diagrams represents a client that does not have direct access to NiFi nodes, and it accesses through the reverse proxy, while Client2 has direct access.
    +
    +In this example, Nginx is used as a reverse proxy.
    +
    +===== Example 1: RAW - Server name to Node mapping
    +
    +image:s2s-rproxy-servername.svg["Server name to Node mapping"]
    +
    +1. Client1 initiates Site-to-Site protocol, the request is routed to one of upstream NiFi nodes. The NiFi node computes Site-to-Site port for RAW. By routing 'example1', port 10443 is returned.
    +2. Client1 asks peers to 'nifi.example.com:10443', the request is routed to 'nifi0:8081'. The NiFi node computes available peers, by 'example1' routing rule, 'nifi0:8081' is converted to 'nifi0.example.com:10443', so are nifi1 and nifi2. As a result, 'nifi0.eample.com:10443', 'nifi1.example.com:10443' and 'nifi2.example.com:10443' are returned.
    --- End diff --
    
    Thanks, fixed.


---

[GitHub] nifi issue #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:

    https://github.com/apache/nifi/pull/2510
  
    Dear reviewers, I've tested this improvement with Nginx and AWS ALB (Application Load Balancer). I used docker to run different Nginx configurations to test, and those docker environments are available here in my github project, which may be useful for reviewing, too. https://github.com/ijokarumawak/nifi-reverseproxy
    
    Please build this PR and see updated administration-guide.html for details. Thank you!


---

[GitHub] nifi issue #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on the issue:

    https://github.com/apache/nifi/pull/2510
  
    Thanks Koji, this is really important work. I can't review immediately but when I get a couple of my tickets knocked out, I'll take a look. Hopefully we get multiple sets of eyes on this one. 


---

[GitHub] nifi pull request #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/2510#discussion_r177538563
  
    --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-site-to-site/src/main/java/org/apache/nifi/remote/PeerDescriptionModifier.java ---
    @@ -0,0 +1,160 @@
    +/*
    + * 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.remote;
    +
    +import org.apache.nifi.attribute.expression.language.PreparedQuery;
    +import org.apache.nifi.attribute.expression.language.Query;
    +import org.apache.nifi.remote.protocol.SiteToSiteTransportProtocol;
    +import org.apache.nifi.util.NiFiProperties;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.List;
    +import java.util.Map;
    +import java.util.stream.Collectors;
    +
    +import static java.lang.String.format;
    +import static org.apache.commons.lang3.StringUtils.isBlank;
    +
    +public class PeerDescriptionModifier {
    +
    +    private static final Logger logger = LoggerFactory.getLogger(PeerDescriptionModifier.class);
    +
    +    public enum RequestType {
    +        SiteToSiteDetail,
    +        Peers
    +    }
    +
    +    private static class Route {
    +        private String name;
    +        private SiteToSiteTransportProtocol protocol;
    +        private PreparedQuery predicate;
    +        private PreparedQuery hostname;
    +        private PreparedQuery port;
    +        private PreparedQuery secure;
    +
    +        private boolean isValid() {
    +            if (hostname == null) {
    +                logger.warn("Ignore invalid route definition {} because 'hostname' is not specified.", name);return false;
    +            }
    +            if (port == null) {
    +                logger.warn("Ignore invalid route definition {} because 'port' is not specified.", name);
    +                return false;
    +            }
    +            return true;
    +        }
    +
    +        private PeerDescription getTarget(final Map<String, String> variables) {
    +            final String targetHostName = hostname.evaluateExpressions(variables, null);
    +            if (isBlank(targetHostName)) {
    +                throw new IllegalStateException("Target hostname was not resolved for the route definition " + name);
    +            }
    +
    +            final String targetPortStr = port.evaluateExpressions(variables, null);
    +            if (isBlank(targetPortStr)) {
    +                throw new IllegalStateException("Target port was not resolved for the route definition " + name);
    +            }
    +
    +            String targetIsSecure = secure == null ? null : secure.evaluateExpressions(variables, null);
    +            if (isBlank(targetIsSecure)) {
    +                targetIsSecure = "false";
    +            }
    +            return new PeerDescription(targetHostName, Integer.valueOf(targetPortStr), Boolean.valueOf(targetIsSecure));
    +        }
    +    }
    +
    +    private Map<SiteToSiteTransportProtocol, List<Route>> routes;
    +
    +
    +    private static final String PROPERTY_PREFIX = "nifi.remote.route.";
    +
    +    public PeerDescriptionModifier(final NiFiProperties properties) {
    +        final Map<String, List<String>> routeDefinitions = properties.getPropertyKeys().stream()
    +                .filter(k -> k.startsWith(PROPERTY_PREFIX))
    +                .collect(Collectors.groupingBy(k -> k.substring(PROPERTY_PREFIX.length(), k.lastIndexOf('.'))));
    +
    +        routes = routeDefinitions.entrySet().stream().map(r -> {
    +            final Route route = new Route();
    +            final String[] key = r.getKey().split("\\.");
    +            route.protocol = SiteToSiteTransportProtocol.valueOf(key[0].toUpperCase());
    +            route.name = key[1];
    +            r.getValue().forEach(k -> {
    +                final String name = k.substring(k.lastIndexOf('.') + 1);
    +                final String value = properties.getProperty(k);
    +                switch (name) {
    +                    case "when":
    +                        route.predicate = Query.prepare(value);
    +                        break;
    +                    case "hostname":
    +                        route.hostname = Query.prepare(value);
    +                        break;
    +                    case "port":
    +                        route.port = Query.prepare(value);
    +                        break;
    +                    case "secure":
    +                        route.secure = Query.prepare(value);
    +                        break;
    +                }
    +            });
    +            return route;
    +        }).filter(Route::isValid).collect(Collectors.groupingBy(r -> r.protocol));
    --- End diff --
    
    If a route is invalid, would it be better to fail startup or ignore the route? I have some concern that a user wouldn't notice the WARN message in the log when filtering an invalid route.


---

[GitHub] nifi pull request #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/2510#discussion_r177541745
  
    --- Diff: nifi-docs/src/main/asciidoc/administration-guide.adoc ---
    @@ -3058,6 +3062,258 @@ responses from the remote system for `30 secs`. This allows NiFi to avoid consta
     has many instances of Remote Process Groups.
     |====
     
    +[[site_to_site_reverse_proxy_properties]]
    +=== Site to Site Routing Properties for Reverse Proxies
    +
    +Site-to-Site requires peer-to-peer communication between a client and a remote NiFi node. E.g. if a remote NiFi cluster has 3 nodes, nifi0, nifi1 and nifi2, then a client requests have to be reachable to each of those remote node.
    +
    +If a NiFi cluster is planned to receive/transfer data from/to Site-to-Site clients over the internet or a company firewall, a reverse proxy server can be deployed in front of the NiFi cluster nodes as a gateway to route client requests to upstream NiFi nodes, to reduce number of servers and ports those have to be exposed.
    +
    +In such environment, the same NiFi cluster would also be expected to be accessed by Site-to-Site clients within the same network. Sending FlowFiles to itself for load distribution among NiFi cluster nodes can be a typical example. In this case, client requests should be routed directly to a node without going through the reverse proxy.
    +
    +In order to support such deployments, remote NiFi clusters need to expose its Site-to-Site endpoints dynamically based on client request contexts. Following properties configure how peers should be exposed to clients. A routing definition consists of 4 properties, 'when', 'hostname', 'port', and 'secure', grouped by 'protocol' and 'name'. Multiple routing definitions can be configured. 'protocol' represents Site-to-Site transport protocol, i.e. raw or http.
    +
    +|====
    +|*Property*|*Description*
    +|nifi.remote.route.{protocol}.{name}.when|Boolean value, 'true' or 'false'. Controls whether the routing definition for this name should be used.
    +|nifi.remote.route.{protocol}.{name}.hostname|Specify hostname that will be introduced to Site-to-Site clients for further communications.
    +|nifi.remote.route.{protocol}.{name}.port|Specify port number that will be introduced to Site-to-Site clients for further communications.
    +|nifi.remote.route.{protocol}.{name}.secure|Boolean value, 'true' or 'false'. Specify whether the remote peer should be accessed via secure protocol.
    +|====
    +
    +All of above routing properties can use NiFi Expression Language to compute target peer description from request context. Available variables are:
    +
    +|===
    +|*Variable name*|*Description*
    +|s2s.{source\|target}.hostname|Hostname of the source where the request came from, and the original target.
    +|s2s.{source\|target}.port|Same as above, for ports. Source port may not be useful as it is just a client side TCP port.
    +|s2s.{source\|target}.secure|Same as above, for secure or not.
    +|s2s.protocol|The name of Site-to-Site protocol being used, RAW or HTTP.
    +|s2s.request|The name of current request type, SiteToSiteDetail or Peers. See Site-to-Site protocol sequence below for detail.
    +|HTTP request headers|HTTP request header values can be referred by its name.
    +|===
    +
    +==== Site to Site protocol sequence
    +
    +Configuring these properties correctly would require some understandings on Site-to-Site protocol sequence.
    +
    +1. A client initiates Site-to-Site protocol by sending a HTTP(S) request to the specified remote URL to get remote cluster Site-to-Site information. Specifically, to '/nifi-api/site-to-site'. This request is called 'SiteToSiteDetail'.
    +2. A remote NiFi node responds with its input and output ports, and TCP port numbers for RAW and TCP transport protocols.
    +3. The client sends another request to get remote peers using the TCP port number returned at #2. From this request, raw socket communication is used for RAW transport protocol, while HTTP keeps using HTTP(S). This request is called 'Peers'.
    +4. A remote NiFi node responds with list of available remote peers containing hostname, port, secure and workload such as the number of queued FlowFiles. From this point, further communication is done between the client and the remote NiFi node.
    +5. The client decides which peer to transfer data from/to, based on workload information.
    +6. The client sends a request to create a transaction to a remote NiFi node.
    +7. The remote NiFi node accepts the transaction.
    +8. Data is sent to the target peer. Multiple Data packets can be sent in batch manner.
    +9. When there is no more data to send, or reached to batch limit, the transaction is confirmed on both end by calculating CRC32 hash of sent data.
    +10. The transaction is committed on both end.
    +
    +==== Reverse Proxy Configurations
    +
    +Most reverse proxy software implement HTTP and TCP proxy mode. For NiFi RAW Site-to-Site protocol, both HTTP and TCP proxy configurations are required, and at least 2 ports needed to be opened. NiFi HTTP Site-to-Site protocol can minimize the required number of open ports at the reverse proxy to 1.
    +
    +Setting correct HTTP headers at reverse proxies are crucial for NiFi to work correctly, not only routing requests but also authorize client requests. See also <<proxy_configuration>> for details.
    +
    +There are two types of requests-to-NiFi-node mapping techniques those can be applied at reverse proxy servers. One is 'Server name to Node' and the other is 'Port number to Node'.
    +
    +With 'Server name to Node', the same port can be used to route requests to different upstream NiFi nodes based on the requested server name (e.g. nifi0.example.com, nifi1.example.com). Host name resolution should be configured to map different host names to the same reverse proxy address, that can be done by adding /etc/hosts file or DNS server entries. Also, if clients to reverse proxy uses HTTPS, reverse proxy server certificate should have wildcard common name or SAN to be accessed by different host names.
    +
    +Some reverse proxy technologies do not support server name routing rules, in such case, use 'Port number to Node' technique. 'Port number to Node' mapping requires N open port at a reverse proxy for a NiFi cluster consists of N nodes.
    +
    +Refer following examples for actual configurations.
    +
    +==== Site to Site and Reverse Proxy Examples
    +
    +Here are some example reverse proxy and NiFi setups to illustrate how configuration files look like.
    +
    +Client1 in the following diagrams represents a client that does not have direct access to NiFi nodes, and it accesses through the reverse proxy, while Client2 has direct access.
    +
    +In this example, Nginx is used as a reverse proxy.
    +
    +===== Example 1: RAW - Server name to Node mapping
    +
    +image:s2s-rproxy-servername.svg["Server name to Node mapping"]
    +
    +1. Client1 initiates Site-to-Site protocol, the request is routed to one of upstream NiFi nodes. The NiFi node computes Site-to-Site port for RAW. By routing 'example1', port 10443 is returned.
    +2. Client1 asks peers to 'nifi.example.com:10443', the request is routed to 'nifi0:8081'. The NiFi node computes available peers, by 'example1' routing rule, 'nifi0:8081' is converted to 'nifi0.example.com:10443', so are nifi1 and nifi2. As a result, 'nifi0.eample.com:10443', 'nifi1.example.com:10443' and 'nifi2.example.com:10443' are returned.
    --- End diff --
    
    Type: 'eample'


---

[GitHub] nifi pull request #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/nifi/pull/2510


---

[GitHub] nifi pull request #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/2510#discussion_r178175082
  
    --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-site-to-site/src/main/java/org/apache/nifi/remote/PeerDescriptionModifier.java ---
    @@ -0,0 +1,165 @@
    +/*
    + * 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.remote;
    +
    +import org.apache.nifi.attribute.expression.language.PreparedQuery;
    +import org.apache.nifi.attribute.expression.language.Query;
    +import org.apache.nifi.remote.protocol.SiteToSiteTransportProtocol;
    +import org.apache.nifi.util.NiFiProperties;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.List;
    +import java.util.Map;
    +import java.util.stream.Collectors;
    +
    +import static java.lang.String.format;
    +import static org.apache.commons.lang3.StringUtils.isBlank;
    +
    +public class PeerDescriptionModifier {
    +
    +    private static final Logger logger = LoggerFactory.getLogger(PeerDescriptionModifier.class);
    +
    +    public enum RequestType {
    +        SiteToSiteDetail,
    +        Peers
    +    }
    +
    +    private static class Route {
    +        private String name;
    +        private SiteToSiteTransportProtocol protocol;
    +        private PreparedQuery predicate;
    +        private PreparedQuery hostname;
    +        private PreparedQuery port;
    +        private PreparedQuery secure;
    +
    +        private Route validate() {
    +            if (hostname == null) {
    +                throw new IllegalArgumentException(
    +                        format("Found an invalid Site-to-Site route definition [%s] 'hostname' is not specified.", name));
    +            }
    +            if (port == null) {
    +                throw new IllegalArgumentException(
    +                        format("Found an invalid Site-to-Site route definition [%s] 'port' is not specified.", name));
    +            }
    +            return this;
    +        }
    +
    +        private PeerDescription getTarget(final Map<String, String> variables) {
    +            final String targetHostName = hostname.evaluateExpressions(variables, null);
    +            if (isBlank(targetHostName)) {
    +                throw new IllegalStateException("Target hostname was not resolved for the route definition " + name);
    +            }
    +
    +            final String targetPortStr = port.evaluateExpressions(variables, null);
    +            if (isBlank(targetPortStr)) {
    +                throw new IllegalStateException("Target port was not resolved for the route definition " + name);
    +            }
    +
    +            final String targetIsSecure = secure == null ? null : secure.evaluateExpressions(variables, null);
    +            return new PeerDescription(targetHostName, Integer.valueOf(targetPortStr), Boolean.valueOf(targetIsSecure));
    +        }
    +    }
    +
    +    private Map<SiteToSiteTransportProtocol, List<Route>> routes;
    +
    +
    +    private static final String PROPERTY_PREFIX = "nifi.remote.route.";
    +
    +    public PeerDescriptionModifier(final NiFiProperties properties) {
    +        final Map<String, List<String>> routeDefinitions = properties.getPropertyKeys().stream()
    +                .filter(propertyKey -> propertyKey.startsWith(PROPERTY_PREFIX))
    +                .collect(Collectors.groupingBy(propertyKey -> propertyKey.substring(PROPERTY_PREFIX.length(), propertyKey.lastIndexOf('.'))));
    +
    +        routes = routeDefinitions.entrySet().stream().map(routeDefinition -> {
    +            final Route route = new Route();
    +            // E.g. raw.example1, http.example2
    +            final String[] protocolAndRoutingName = routeDefinition.getKey().split("\\.");
    +            route.protocol = SiteToSiteTransportProtocol.valueOf(protocolAndRoutingName[0].toUpperCase());
    +            route.name = protocolAndRoutingName[1];
    +            routeDefinition.getValue().forEach(propertyKey -> {
    +                final String routingConfigName = propertyKey.substring(propertyKey.lastIndexOf('.') + 1);
    +                final String routingConfigValue = properties.getProperty(propertyKey);
    +                switch (routingConfigName) {
    +                    case "when":
    +                        route.predicate = Query.prepare(routingConfigValue);
    --- End diff --
    
    I also could not make `prepare` throw but the JavaDocs state that it can. Could we add a try/catch around this switch to gracefully handle if it does throw?


---

[GitHub] nifi issue #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:

    https://github.com/apache/nifi/pull/2510
  
    @mcgilman Thanks for clarifying the String manipulation exception, I just didn't have enough imagination to come up with such invalid inputs. I switched to use regex to check and parse property keys. Now it should be more robust and provide more user friendly error messages.
    
    Also, added try/catch for EL parse failure. Thanks!


---

[GitHub] nifi issue #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:

    https://github.com/apache/nifi/pull/2510
  
    @alopresto Thank you! The RAT check failed with SVG files for docs. I didn't run contrib check after I added docs, my bad. I'll update it.


---

[GitHub] nifi pull request #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/2510#discussion_r177538075
  
    --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-site-to-site/src/main/java/org/apache/nifi/remote/PeerDescriptionModifier.java ---
    @@ -0,0 +1,160 @@
    +/*
    + * 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.remote;
    +
    +import org.apache.nifi.attribute.expression.language.PreparedQuery;
    +import org.apache.nifi.attribute.expression.language.Query;
    +import org.apache.nifi.remote.protocol.SiteToSiteTransportProtocol;
    +import org.apache.nifi.util.NiFiProperties;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.List;
    +import java.util.Map;
    +import java.util.stream.Collectors;
    +
    +import static java.lang.String.format;
    +import static org.apache.commons.lang3.StringUtils.isBlank;
    +
    +public class PeerDescriptionModifier {
    +
    +    private static final Logger logger = LoggerFactory.getLogger(PeerDescriptionModifier.class);
    +
    +    public enum RequestType {
    +        SiteToSiteDetail,
    +        Peers
    +    }
    +
    +    private static class Route {
    +        private String name;
    +        private SiteToSiteTransportProtocol protocol;
    +        private PreparedQuery predicate;
    +        private PreparedQuery hostname;
    +        private PreparedQuery port;
    +        private PreparedQuery secure;
    +
    +        private boolean isValid() {
    +            if (hostname == null) {
    +                logger.warn("Ignore invalid route definition {} because 'hostname' is not specified.", name);return false;
    +            }
    +            if (port == null) {
    +                logger.warn("Ignore invalid route definition {} because 'port' is not specified.", name);
    +                return false;
    +            }
    +            return true;
    +        }
    +
    +        private PeerDescription getTarget(final Map<String, String> variables) {
    +            final String targetHostName = hostname.evaluateExpressions(variables, null);
    +            if (isBlank(targetHostName)) {
    +                throw new IllegalStateException("Target hostname was not resolved for the route definition " + name);
    +            }
    +
    +            final String targetPortStr = port.evaluateExpressions(variables, null);
    +            if (isBlank(targetPortStr)) {
    +                throw new IllegalStateException("Target port was not resolved for the route definition " + name);
    +            }
    +
    +            String targetIsSecure = secure == null ? null : secure.evaluateExpressions(variables, null);
    +            if (isBlank(targetIsSecure)) {
    +                targetIsSecure = "false";
    +            }
    +            return new PeerDescription(targetHostName, Integer.valueOf(targetPortStr), Boolean.valueOf(targetIsSecure));
    +        }
    +    }
    +
    +    private Map<SiteToSiteTransportProtocol, List<Route>> routes;
    +
    +
    +    private static final String PROPERTY_PREFIX = "nifi.remote.route.";
    +
    +    public PeerDescriptionModifier(final NiFiProperties properties) {
    --- End diff --
    
    A couple comments on this method...
    
    - Can we make this code a little more readable by not using single letter variable names? It made it difficult to understand what was happening here.
    - Can we add better error handling throughout this method? For instance, if the properties are misconfigured it's likely this could fail exceptionally with IndexOutOfBounds or EL Parsing Exception. Can we handle those specifics and log the underlying issue and then continue to fail. This we should be able to identify when the routes are misconfigured and help drive the user to the exact issue.


---

[GitHub] nifi pull request #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/2510#discussion_r177941458
  
    --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-site-to-site/src/main/java/org/apache/nifi/remote/PeerDescriptionModifier.java ---
    @@ -0,0 +1,160 @@
    +/*
    + * 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.remote;
    +
    +import org.apache.nifi.attribute.expression.language.PreparedQuery;
    +import org.apache.nifi.attribute.expression.language.Query;
    +import org.apache.nifi.remote.protocol.SiteToSiteTransportProtocol;
    +import org.apache.nifi.util.NiFiProperties;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.List;
    +import java.util.Map;
    +import java.util.stream.Collectors;
    +
    +import static java.lang.String.format;
    +import static org.apache.commons.lang3.StringUtils.isBlank;
    +
    +public class PeerDescriptionModifier {
    +
    +    private static final Logger logger = LoggerFactory.getLogger(PeerDescriptionModifier.class);
    +
    +    public enum RequestType {
    +        SiteToSiteDetail,
    +        Peers
    +    }
    +
    +    private static class Route {
    +        private String name;
    +        private SiteToSiteTransportProtocol protocol;
    +        private PreparedQuery predicate;
    +        private PreparedQuery hostname;
    +        private PreparedQuery port;
    +        private PreparedQuery secure;
    +
    +        private boolean isValid() {
    +            if (hostname == null) {
    +                logger.warn("Ignore invalid route definition {} because 'hostname' is not specified.", name);return false;
    +            }
    +            if (port == null) {
    +                logger.warn("Ignore invalid route definition {} because 'port' is not specified.", name);
    +                return false;
    +            }
    +            return true;
    +        }
    +
    +        private PeerDescription getTarget(final Map<String, String> variables) {
    +            final String targetHostName = hostname.evaluateExpressions(variables, null);
    +            if (isBlank(targetHostName)) {
    +                throw new IllegalStateException("Target hostname was not resolved for the route definition " + name);
    +            }
    +
    +            final String targetPortStr = port.evaluateExpressions(variables, null);
    +            if (isBlank(targetPortStr)) {
    +                throw new IllegalStateException("Target port was not resolved for the route definition " + name);
    +            }
    +
    +            String targetIsSecure = secure == null ? null : secure.evaluateExpressions(variables, null);
    +            if (isBlank(targetIsSecure)) {
    +                targetIsSecure = "false";
    +            }
    +            return new PeerDescription(targetHostName, Integer.valueOf(targetPortStr), Boolean.valueOf(targetIsSecure));
    +        }
    +    }
    +
    +    private Map<SiteToSiteTransportProtocol, List<Route>> routes;
    +
    +
    +    private static final String PROPERTY_PREFIX = "nifi.remote.route.";
    +
    +    public PeerDescriptionModifier(final NiFiProperties properties) {
    +        final Map<String, List<String>> routeDefinitions = properties.getPropertyKeys().stream()
    +                .filter(k -> k.startsWith(PROPERTY_PREFIX))
    +                .collect(Collectors.groupingBy(k -> k.substring(PROPERTY_PREFIX.length(), k.lastIndexOf('.'))));
    +
    +        routes = routeDefinitions.entrySet().stream().map(r -> {
    +            final Route route = new Route();
    +            final String[] key = r.getKey().split("\\.");
    +            route.protocol = SiteToSiteTransportProtocol.valueOf(key[0].toUpperCase());
    +            route.name = key[1];
    +            r.getValue().forEach(k -> {
    +                final String name = k.substring(k.lastIndexOf('.') + 1);
    +                final String value = properties.getProperty(k);
    +                switch (name) {
    +                    case "when":
    +                        route.predicate = Query.prepare(value);
    +                        break;
    +                    case "hostname":
    +                        route.hostname = Query.prepare(value);
    +                        break;
    +                    case "port":
    +                        route.port = Query.prepare(value);
    +                        break;
    +                    case "secure":
    +                        route.secure = Query.prepare(value);
    +                        break;
    +                }
    +            });
    +            return route;
    +        }).filter(Route::isValid).collect(Collectors.groupingBy(r -> r.protocol));
    --- End diff --
    
    I changed it to throw Exceptions if invalid.


---

[GitHub] nifi issue #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:

    https://github.com/apache/nifi/pull/2510
  
    Will review...


---

[GitHub] nifi issue #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:

    https://github.com/apache/nifi/pull/2510
  
    @ijokarumawak Just wanted to add that I have verified this capability running standalone and clustered and everything seems to be working nicely. Just a couple more minor error handling cases. Thanks!


---

[GitHub] nifi pull request #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/2510#discussion_r177542298
  
    --- Diff: nifi-docs/src/main/asciidoc/administration-guide.adoc ---
    @@ -3058,6 +3062,258 @@ responses from the remote system for `30 secs`. This allows NiFi to avoid consta
     has many instances of Remote Process Groups.
     |====
     
    +[[site_to_site_reverse_proxy_properties]]
    +=== Site to Site Routing Properties for Reverse Proxies
    +
    +Site-to-Site requires peer-to-peer communication between a client and a remote NiFi node. E.g. if a remote NiFi cluster has 3 nodes, nifi0, nifi1 and nifi2, then a client requests have to be reachable to each of those remote node.
    +
    +If a NiFi cluster is planned to receive/transfer data from/to Site-to-Site clients over the internet or a company firewall, a reverse proxy server can be deployed in front of the NiFi cluster nodes as a gateway to route client requests to upstream NiFi nodes, to reduce number of servers and ports those have to be exposed.
    +
    +In such environment, the same NiFi cluster would also be expected to be accessed by Site-to-Site clients within the same network. Sending FlowFiles to itself for load distribution among NiFi cluster nodes can be a typical example. In this case, client requests should be routed directly to a node without going through the reverse proxy.
    +
    +In order to support such deployments, remote NiFi clusters need to expose its Site-to-Site endpoints dynamically based on client request contexts. Following properties configure how peers should be exposed to clients. A routing definition consists of 4 properties, 'when', 'hostname', 'port', and 'secure', grouped by 'protocol' and 'name'. Multiple routing definitions can be configured. 'protocol' represents Site-to-Site transport protocol, i.e. raw or http.
    +
    +|====
    +|*Property*|*Description*
    +|nifi.remote.route.{protocol}.{name}.when|Boolean value, 'true' or 'false'. Controls whether the routing definition for this name should be used.
    +|nifi.remote.route.{protocol}.{name}.hostname|Specify hostname that will be introduced to Site-to-Site clients for further communications.
    +|nifi.remote.route.{protocol}.{name}.port|Specify port number that will be introduced to Site-to-Site clients for further communications.
    +|nifi.remote.route.{protocol}.{name}.secure|Boolean value, 'true' or 'false'. Specify whether the remote peer should be accessed via secure protocol.
    +|====
    +
    +All of above routing properties can use NiFi Expression Language to compute target peer description from request context. Available variables are:
    +
    +|===
    +|*Variable name*|*Description*
    +|s2s.{source\|target}.hostname|Hostname of the source where the request came from, and the original target.
    +|s2s.{source\|target}.port|Same as above, for ports. Source port may not be useful as it is just a client side TCP port.
    +|s2s.{source\|target}.secure|Same as above, for secure or not.
    +|s2s.protocol|The name of Site-to-Site protocol being used, RAW or HTTP.
    +|s2s.request|The name of current request type, SiteToSiteDetail or Peers. See Site-to-Site protocol sequence below for detail.
    +|HTTP request headers|HTTP request header values can be referred by its name.
    +|===
    +
    +==== Site to Site protocol sequence
    +
    +Configuring these properties correctly would require some understandings on Site-to-Site protocol sequence.
    +
    +1. A client initiates Site-to-Site protocol by sending a HTTP(S) request to the specified remote URL to get remote cluster Site-to-Site information. Specifically, to '/nifi-api/site-to-site'. This request is called 'SiteToSiteDetail'.
    +2. A remote NiFi node responds with its input and output ports, and TCP port numbers for RAW and TCP transport protocols.
    +3. The client sends another request to get remote peers using the TCP port number returned at #2. From this request, raw socket communication is used for RAW transport protocol, while HTTP keeps using HTTP(S). This request is called 'Peers'.
    +4. A remote NiFi node responds with list of available remote peers containing hostname, port, secure and workload such as the number of queued FlowFiles. From this point, further communication is done between the client and the remote NiFi node.
    +5. The client decides which peer to transfer data from/to, based on workload information.
    +6. The client sends a request to create a transaction to a remote NiFi node.
    +7. The remote NiFi node accepts the transaction.
    +8. Data is sent to the target peer. Multiple Data packets can be sent in batch manner.
    +9. When there is no more data to send, or reached to batch limit, the transaction is confirmed on both end by calculating CRC32 hash of sent data.
    +10. The transaction is committed on both end.
    +
    +==== Reverse Proxy Configurations
    +
    +Most reverse proxy software implement HTTP and TCP proxy mode. For NiFi RAW Site-to-Site protocol, both HTTP and TCP proxy configurations are required, and at least 2 ports needed to be opened. NiFi HTTP Site-to-Site protocol can minimize the required number of open ports at the reverse proxy to 1.
    +
    +Setting correct HTTP headers at reverse proxies are crucial for NiFi to work correctly, not only routing requests but also authorize client requests. See also <<proxy_configuration>> for details.
    +
    +There are two types of requests-to-NiFi-node mapping techniques those can be applied at reverse proxy servers. One is 'Server name to Node' and the other is 'Port number to Node'.
    +
    +With 'Server name to Node', the same port can be used to route requests to different upstream NiFi nodes based on the requested server name (e.g. nifi0.example.com, nifi1.example.com). Host name resolution should be configured to map different host names to the same reverse proxy address, that can be done by adding /etc/hosts file or DNS server entries. Also, if clients to reverse proxy uses HTTPS, reverse proxy server certificate should have wildcard common name or SAN to be accessed by different host names.
    +
    +Some reverse proxy technologies do not support server name routing rules, in such case, use 'Port number to Node' technique. 'Port number to Node' mapping requires N open port at a reverse proxy for a NiFi cluster consists of N nodes.
    +
    +Refer following examples for actual configurations.
    +
    +==== Site to Site and Reverse Proxy Examples
    +
    +Here are some example reverse proxy and NiFi setups to illustrate how configuration files look like.
    +
    +Client1 in the following diagrams represents a client that does not have direct access to NiFi nodes, and it accesses through the reverse proxy, while Client2 has direct access.
    +
    +In this example, Nginx is used as a reverse proxy.
    +
    +===== Example 1: RAW - Server name to Node mapping
    +
    +image:s2s-rproxy-servername.svg["Server name to Node mapping"]
    +
    +1. Client1 initiates Site-to-Site protocol, the request is routed to one of upstream NiFi nodes. The NiFi node computes Site-to-Site port for RAW. By routing 'example1', port 10443 is returned.
    +2. Client1 asks peers to 'nifi.example.com:10443', the request is routed to 'nifi0:8081'. The NiFi node computes available peers, by 'example1' routing rule, 'nifi0:8081' is converted to 'nifi0.example.com:10443', so are nifi1 and nifi2. As a result, 'nifi0.eample.com:10443', 'nifi1.example.com:10443' and 'nifi2.example.com:10443' are returned.
    +3. Client1 decides to use 'nifi2.example.com:10443' for further communication.
    +4. On the other hand, Client2 has two URIs for Site-to-Site bootstrap URIs, and initiates the protocol using one of them. The 'example1' routing does not match this for this request, and port 8081 is returned.
    +5. Client2 asks peers from 'nifi1:8081'. The 'example1' does not match, so the original 'nifi0:8081', 'nifi1:8081' and 'nifi2:8081' are returned as they are.
    +6. Client2 decides to use 'nifi2:8081' for further communication.
    +
    +nifi.properties (all node has the same routing configuration)
    --- End diff --
    
    Would it make sense to move this example configuration above these 1 - 6 steps? The steps reference 'example1' which was confusing until I realized that 'example1' was used as the route name in the configuration.


---

[GitHub] nifi pull request #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/2510#discussion_r177922327
  
    --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-site-to-site/src/main/java/org/apache/nifi/remote/PeerDescriptionModifier.java ---
    @@ -0,0 +1,160 @@
    +/*
    + * 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.remote;
    +
    +import org.apache.nifi.attribute.expression.language.PreparedQuery;
    +import org.apache.nifi.attribute.expression.language.Query;
    +import org.apache.nifi.remote.protocol.SiteToSiteTransportProtocol;
    +import org.apache.nifi.util.NiFiProperties;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.List;
    +import java.util.Map;
    +import java.util.stream.Collectors;
    +
    +import static java.lang.String.format;
    +import static org.apache.commons.lang3.StringUtils.isBlank;
    +
    +public class PeerDescriptionModifier {
    +
    +    private static final Logger logger = LoggerFactory.getLogger(PeerDescriptionModifier.class);
    +
    +    public enum RequestType {
    +        SiteToSiteDetail,
    +        Peers
    +    }
    +
    +    private static class Route {
    +        private String name;
    +        private SiteToSiteTransportProtocol protocol;
    +        private PreparedQuery predicate;
    +        private PreparedQuery hostname;
    +        private PreparedQuery port;
    +        private PreparedQuery secure;
    +
    +        private boolean isValid() {
    +            if (hostname == null) {
    +                logger.warn("Ignore invalid route definition {} because 'hostname' is not specified.", name);return false;
    +            }
    +            if (port == null) {
    +                logger.warn("Ignore invalid route definition {} because 'port' is not specified.", name);
    +                return false;
    +            }
    +            return true;
    +        }
    +
    +        private PeerDescription getTarget(final Map<String, String> variables) {
    +            final String targetHostName = hostname.evaluateExpressions(variables, null);
    +            if (isBlank(targetHostName)) {
    +                throw new IllegalStateException("Target hostname was not resolved for the route definition " + name);
    +            }
    +
    +            final String targetPortStr = port.evaluateExpressions(variables, null);
    +            if (isBlank(targetPortStr)) {
    +                throw new IllegalStateException("Target port was not resolved for the route definition " + name);
    +            }
    +
    +            String targetIsSecure = secure == null ? null : secure.evaluateExpressions(variables, null);
    +            if (isBlank(targetIsSecure)) {
    +                targetIsSecure = "false";
    +            }
    +            return new PeerDescription(targetHostName, Integer.valueOf(targetPortStr), Boolean.valueOf(targetIsSecure));
    +        }
    +    }
    +
    +    private Map<SiteToSiteTransportProtocol, List<Route>> routes;
    +
    +
    +    private static final String PROPERTY_PREFIX = "nifi.remote.route.";
    +
    +    public PeerDescriptionModifier(final NiFiProperties properties) {
    +        final Map<String, List<String>> routeDefinitions = properties.getPropertyKeys().stream()
    +                .filter(k -> k.startsWith(PROPERTY_PREFIX))
    +                .collect(Collectors.groupingBy(k -> k.substring(PROPERTY_PREFIX.length(), k.lastIndexOf('.'))));
    +
    +        routes = routeDefinitions.entrySet().stream().map(r -> {
    +            final Route route = new Route();
    +            final String[] key = r.getKey().split("\\.");
    +            route.protocol = SiteToSiteTransportProtocol.valueOf(key[0].toUpperCase());
    +            route.name = key[1];
    +            r.getValue().forEach(k -> {
    +                final String name = k.substring(k.lastIndexOf('.') + 1);
    +                final String value = properties.getProperty(k);
    +                switch (name) {
    +                    case "when":
    +                        route.predicate = Query.prepare(value);
    +                        break;
    +                    case "hostname":
    +                        route.hostname = Query.prepare(value);
    +                        break;
    +                    case "port":
    +                        route.port = Query.prepare(value);
    +                        break;
    +                    case "secure":
    +                        route.secure = Query.prepare(value);
    +                        break;
    +                }
    +            });
    +            return route;
    +        }).filter(Route::isValid).collect(Collectors.groupingBy(r -> r.protocol));
    +
    +    }
    +
    +    private void addVariables(Map<String, String> map, String prefix, PeerDescription peer) {
    +        map.put(format("%s.hostname", prefix), peer.getHostname());
    +        map.put(format("%s.port", prefix), String.valueOf(peer.getPort()));
    +        map.put(format("%s.secure", prefix), String.valueOf(peer.isSecure()));
    +    }
    +
    +    public boolean isModificationNeeded(final SiteToSiteTransportProtocol protocol) {
    --- End diff --
    
    @mcgilman The reason of this method being separated is to avoid unnecessary object creations at the caller. I imagine most of deployments will not use this configuration, hence if caller side (e.g. SiteToSiteResource) always create objects to call modify unconditionally, but doing nothing actually because not having modification definitions, that would be waste  or resources.


---

[GitHub] nifi pull request #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/2510#discussion_r177942356
  
    --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-site-to-site/src/main/java/org/apache/nifi/remote/PeerDescriptionModifier.java ---
    @@ -0,0 +1,160 @@
    +/*
    + * 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.remote;
    +
    +import org.apache.nifi.attribute.expression.language.PreparedQuery;
    +import org.apache.nifi.attribute.expression.language.Query;
    +import org.apache.nifi.remote.protocol.SiteToSiteTransportProtocol;
    +import org.apache.nifi.util.NiFiProperties;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.List;
    +import java.util.Map;
    +import java.util.stream.Collectors;
    +
    +import static java.lang.String.format;
    +import static org.apache.commons.lang3.StringUtils.isBlank;
    +
    +public class PeerDescriptionModifier {
    +
    +    private static final Logger logger = LoggerFactory.getLogger(PeerDescriptionModifier.class);
    +
    +    public enum RequestType {
    +        SiteToSiteDetail,
    +        Peers
    +    }
    +
    +    private static class Route {
    +        private String name;
    +        private SiteToSiteTransportProtocol protocol;
    +        private PreparedQuery predicate;
    +        private PreparedQuery hostname;
    +        private PreparedQuery port;
    +        private PreparedQuery secure;
    +
    +        private boolean isValid() {
    +            if (hostname == null) {
    +                logger.warn("Ignore invalid route definition {} because 'hostname' is not specified.", name);return false;
    +            }
    +            if (port == null) {
    +                logger.warn("Ignore invalid route definition {} because 'port' is not specified.", name);
    +                return false;
    +            }
    +            return true;
    +        }
    +
    +        private PeerDescription getTarget(final Map<String, String> variables) {
    +            final String targetHostName = hostname.evaluateExpressions(variables, null);
    +            if (isBlank(targetHostName)) {
    +                throw new IllegalStateException("Target hostname was not resolved for the route definition " + name);
    +            }
    +
    +            final String targetPortStr = port.evaluateExpressions(variables, null);
    +            if (isBlank(targetPortStr)) {
    +                throw new IllegalStateException("Target port was not resolved for the route definition " + name);
    +            }
    +
    +            String targetIsSecure = secure == null ? null : secure.evaluateExpressions(variables, null);
    +            if (isBlank(targetIsSecure)) {
    +                targetIsSecure = "false";
    +            }
    +            return new PeerDescription(targetHostName, Integer.valueOf(targetPortStr), Boolean.valueOf(targetIsSecure));
    +        }
    +    }
    +
    +    private Map<SiteToSiteTransportProtocol, List<Route>> routes;
    +
    +
    +    private static final String PROPERTY_PREFIX = "nifi.remote.route.";
    +
    +    public PeerDescriptionModifier(final NiFiProperties properties) {
    --- End diff --
    
    @mcgilman Agreed. I've changed variable names for better readability. Also changed validation logic to throw Exceptions with detailed error message.
    
    For EL parsing, I tried several invalid ELs, but couldn't get EL parsing exception when it's compiled. Instead it errors out when it is evaluated.
    
    More unit tests are added as well for invalid configuration scenarios.
    I confirmed NiFi does not start with invalid configs.


---

[GitHub] nifi pull request #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/2510#discussion_r177539731
  
    --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-site-to-site/src/main/java/org/apache/nifi/remote/PeerDescriptionModifier.java ---
    @@ -0,0 +1,160 @@
    +/*
    + * 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.remote;
    +
    +import org.apache.nifi.attribute.expression.language.PreparedQuery;
    +import org.apache.nifi.attribute.expression.language.Query;
    +import org.apache.nifi.remote.protocol.SiteToSiteTransportProtocol;
    +import org.apache.nifi.util.NiFiProperties;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.List;
    +import java.util.Map;
    +import java.util.stream.Collectors;
    +
    +import static java.lang.String.format;
    +import static org.apache.commons.lang3.StringUtils.isBlank;
    +
    +public class PeerDescriptionModifier {
    +
    +    private static final Logger logger = LoggerFactory.getLogger(PeerDescriptionModifier.class);
    +
    +    public enum RequestType {
    +        SiteToSiteDetail,
    +        Peers
    +    }
    +
    +    private static class Route {
    +        private String name;
    +        private SiteToSiteTransportProtocol protocol;
    +        private PreparedQuery predicate;
    +        private PreparedQuery hostname;
    +        private PreparedQuery port;
    +        private PreparedQuery secure;
    +
    +        private boolean isValid() {
    +            if (hostname == null) {
    +                logger.warn("Ignore invalid route definition {} because 'hostname' is not specified.", name);return false;
    +            }
    +            if (port == null) {
    +                logger.warn("Ignore invalid route definition {} because 'port' is not specified.", name);
    +                return false;
    +            }
    +            return true;
    +        }
    +
    +        private PeerDescription getTarget(final Map<String, String> variables) {
    +            final String targetHostName = hostname.evaluateExpressions(variables, null);
    +            if (isBlank(targetHostName)) {
    +                throw new IllegalStateException("Target hostname was not resolved for the route definition " + name);
    +            }
    +
    +            final String targetPortStr = port.evaluateExpressions(variables, null);
    +            if (isBlank(targetPortStr)) {
    +                throw new IllegalStateException("Target port was not resolved for the route definition " + name);
    +            }
    +
    +            String targetIsSecure = secure == null ? null : secure.evaluateExpressions(variables, null);
    +            if (isBlank(targetIsSecure)) {
    +                targetIsSecure = "false";
    --- End diff --
    
    Is there a reason we are defaulting this to false here? If so, is this something that should be called out in the documentation?


---

[GitHub] nifi pull request #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/2510#discussion_r177536829
  
    --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-site-to-site/src/main/java/org/apache/nifi/remote/PeerDescriptionModifier.java ---
    @@ -0,0 +1,160 @@
    +/*
    + * 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.remote;
    +
    +import org.apache.nifi.attribute.expression.language.PreparedQuery;
    +import org.apache.nifi.attribute.expression.language.Query;
    +import org.apache.nifi.remote.protocol.SiteToSiteTransportProtocol;
    +import org.apache.nifi.util.NiFiProperties;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.List;
    +import java.util.Map;
    +import java.util.stream.Collectors;
    +
    +import static java.lang.String.format;
    +import static org.apache.commons.lang3.StringUtils.isBlank;
    +
    +public class PeerDescriptionModifier {
    +
    +    private static final Logger logger = LoggerFactory.getLogger(PeerDescriptionModifier.class);
    +
    +    public enum RequestType {
    +        SiteToSiteDetail,
    +        Peers
    +    }
    +
    +    private static class Route {
    +        private String name;
    +        private SiteToSiteTransportProtocol protocol;
    +        private PreparedQuery predicate;
    +        private PreparedQuery hostname;
    +        private PreparedQuery port;
    +        private PreparedQuery secure;
    +
    +        private boolean isValid() {
    +            if (hostname == null) {
    +                logger.warn("Ignore invalid route definition {} because 'hostname' is not specified.", name);return false;
    +            }
    +            if (port == null) {
    +                logger.warn("Ignore invalid route definition {} because 'port' is not specified.", name);
    +                return false;
    +            }
    +            return true;
    +        }
    +
    +        private PeerDescription getTarget(final Map<String, String> variables) {
    +            final String targetHostName = hostname.evaluateExpressions(variables, null);
    +            if (isBlank(targetHostName)) {
    +                throw new IllegalStateException("Target hostname was not resolved for the route definition " + name);
    +            }
    +
    +            final String targetPortStr = port.evaluateExpressions(variables, null);
    +            if (isBlank(targetPortStr)) {
    +                throw new IllegalStateException("Target port was not resolved for the route definition " + name);
    +            }
    +
    +            String targetIsSecure = secure == null ? null : secure.evaluateExpressions(variables, null);
    +            if (isBlank(targetIsSecure)) {
    +                targetIsSecure = "false";
    +            }
    +            return new PeerDescription(targetHostName, Integer.valueOf(targetPortStr), Boolean.valueOf(targetIsSecure));
    +        }
    +    }
    +
    +    private Map<SiteToSiteTransportProtocol, List<Route>> routes;
    +
    +
    +    private static final String PROPERTY_PREFIX = "nifi.remote.route.";
    +
    +    public PeerDescriptionModifier(final NiFiProperties properties) {
    +        final Map<String, List<String>> routeDefinitions = properties.getPropertyKeys().stream()
    +                .filter(k -> k.startsWith(PROPERTY_PREFIX))
    +                .collect(Collectors.groupingBy(k -> k.substring(PROPERTY_PREFIX.length(), k.lastIndexOf('.'))));
    +
    +        routes = routeDefinitions.entrySet().stream().map(r -> {
    +            final Route route = new Route();
    +            final String[] key = r.getKey().split("\\.");
    +            route.protocol = SiteToSiteTransportProtocol.valueOf(key[0].toUpperCase());
    +            route.name = key[1];
    +            r.getValue().forEach(k -> {
    +                final String name = k.substring(k.lastIndexOf('.') + 1);
    +                final String value = properties.getProperty(k);
    +                switch (name) {
    +                    case "when":
    +                        route.predicate = Query.prepare(value);
    +                        break;
    +                    case "hostname":
    +                        route.hostname = Query.prepare(value);
    +                        break;
    +                    case "port":
    +                        route.port = Query.prepare(value);
    +                        break;
    +                    case "secure":
    +                        route.secure = Query.prepare(value);
    +                        break;
    +                }
    +            });
    +            return route;
    +        }).filter(Route::isValid).collect(Collectors.groupingBy(r -> r.protocol));
    +
    +    }
    +
    +    private void addVariables(Map<String, String> map, String prefix, PeerDescription peer) {
    +        map.put(format("%s.hostname", prefix), peer.getHostname());
    +        map.put(format("%s.port", prefix), String.valueOf(peer.getPort()));
    +        map.put(format("%s.secure", prefix), String.valueOf(peer.isSecure()));
    +    }
    +
    +    public boolean isModificationNeeded(final SiteToSiteTransportProtocol protocol) {
    --- End diff --
    
    Does it make sense to make this method private and invoke it from within the modify method? It seems like the general usage is to check if modification is needed and then invoke modify. Alternatively, we could invoke modify unconditionally and check if modification is needed within. If modification is not needed, then target could just be returned.


---

[GitHub] nifi pull request #2510: NIFI-4932: Enable S2S work behind a Reverse Proxy

Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/2510#discussion_r177924123
  
    --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-site-to-site/src/main/java/org/apache/nifi/remote/PeerDescriptionModifier.java ---
    @@ -0,0 +1,160 @@
    +/*
    + * 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.remote;
    +
    +import org.apache.nifi.attribute.expression.language.PreparedQuery;
    +import org.apache.nifi.attribute.expression.language.Query;
    +import org.apache.nifi.remote.protocol.SiteToSiteTransportProtocol;
    +import org.apache.nifi.util.NiFiProperties;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.List;
    +import java.util.Map;
    +import java.util.stream.Collectors;
    +
    +import static java.lang.String.format;
    +import static org.apache.commons.lang3.StringUtils.isBlank;
    +
    +public class PeerDescriptionModifier {
    +
    +    private static final Logger logger = LoggerFactory.getLogger(PeerDescriptionModifier.class);
    +
    +    public enum RequestType {
    +        SiteToSiteDetail,
    +        Peers
    +    }
    +
    +    private static class Route {
    +        private String name;
    +        private SiteToSiteTransportProtocol protocol;
    +        private PreparedQuery predicate;
    +        private PreparedQuery hostname;
    +        private PreparedQuery port;
    +        private PreparedQuery secure;
    +
    +        private boolean isValid() {
    +            if (hostname == null) {
    +                logger.warn("Ignore invalid route definition {} because 'hostname' is not specified.", name);return false;
    +            }
    +            if (port == null) {
    +                logger.warn("Ignore invalid route definition {} because 'port' is not specified.", name);
    +                return false;
    +            }
    +            return true;
    +        }
    +
    +        private PeerDescription getTarget(final Map<String, String> variables) {
    +            final String targetHostName = hostname.evaluateExpressions(variables, null);
    +            if (isBlank(targetHostName)) {
    +                throw new IllegalStateException("Target hostname was not resolved for the route definition " + name);
    +            }
    +
    +            final String targetPortStr = port.evaluateExpressions(variables, null);
    +            if (isBlank(targetPortStr)) {
    +                throw new IllegalStateException("Target port was not resolved for the route definition " + name);
    +            }
    +
    +            String targetIsSecure = secure == null ? null : secure.evaluateExpressions(variables, null);
    +            if (isBlank(targetIsSecure)) {
    +                targetIsSecure = "false";
    --- End diff --
    
    @mcgilman The blank check is not needed since Boolean.valueOf returns `false` for null/empty string. I will remove these lines and also callout that default is 'false' in the docs.


---