exceptionfactory commented on code in PR #5990:
URL: https://github.com/apache/nifi/pull/5990#discussion_r869237170
##########
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/storage/utils/AzureStorageUtils.java:
##########
@@ -311,4 +316,38 @@ public static void setProxy(final OperationContext
operationContext, final Proce
final ProxyConfiguration proxyConfig =
ProxyConfiguration.getConfiguration(processContext);
operationContext.setProxy(proxyConfig.createProxy());
}
+
+ public static void configureProxy(final NettyAsyncHttpClientBuilder
nettyClientBuilder, final PropertyContext propertyContext) {
+ final ProxyConfiguration proxyConfiguration =
ProxyConfiguration.getConfiguration(propertyContext);
+
+ if (proxyConfiguration != ProxyConfiguration.DIRECT_CONFIGURATION) {
+
+ final ProxyOptions proxyOptions = new ProxyOptions(
+ getProxyType(proxyConfiguration),
+ new
InetSocketAddress(proxyConfiguration.getProxyServerHost(),
proxyConfiguration.getProxyServerPort()));
+
+ final String proxyUserName = proxyConfiguration.getProxyUserName();
+ final String proxyUserPassword =
proxyConfiguration.getProxyUserPassword();
+ if (proxyUserName != null && proxyUserPassword != null) {
+ proxyOptions.setCredentials(proxyUserName, proxyUserPassword);
+ }
+
+ nettyClientBuilder.proxy(proxyOptions);
+ }
+ }
+
+ private static ProxyOptions.Type getProxyType(ProxyConfiguration
proxyConfiguration) {
+ if (proxyConfiguration.getProxyType() == Proxy.Type.HTTP) {
+ return ProxyOptions.Type.HTTP;
+ } else if (proxyConfiguration.getProxyType() == Proxy.Type.SOCKS) {
+ final SocksVersion socksVersion =
proxyConfiguration.getSocksVersion();
+ if (socksVersion != SocksVersion.NOT_SPECIFIED) {
+ return ProxyOptions.Type.valueOf(socksVersion.name());
+ } else {
+ throw new IllegalArgumentException("Version should be set when
SOCKS proxy type is used!");
Review Comment:
Use of exclamations in error messages should be avoided. Recommend adjusting
the wording for greater clarity:
```suggestion
throw new IllegalArgumentException("SOCKS Version is
required for SOCKS Proxy Type");
```
##########
nifi-nar-bundles/nifi-standard-services/nifi-proxy-configuration-api/src/main/java/org/apache/nifi/proxy/SocksVersion.java:
##########
@@ -0,0 +1,23 @@
+/*
+ * 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.proxy;
+
+public enum SocksVersion {
+ NOT_SPECIFIED,
Review Comment:
Recommend removing `NOT_SPECIFIED` as a value on this enum and defaulting to
`SOCKS5` since it should be more commonly used.
##########
nifi-nar-bundles/nifi-standard-services/nifi-proxy-configuration-bundle/nifi-proxy-configuration/src/main/java/org/apache/nifi/proxy/StandardProxyConfigurationService.java:
##########
@@ -46,31 +46,41 @@ public class StandardProxyConfigurationService extends
AbstractControllerService
.required(true)
.build();
- static final PropertyDescriptor PROXY_SERVER_HOST = new
PropertyDescriptor.Builder()
+ public static final PropertyDescriptor SOCKS_VERSION = new
PropertyDescriptor.Builder()
+ .name("socks-version")
+ .displayName("Socks version")
+ .description("Socks version.")
Review Comment:
Recommend expanding the description:
```suggestion
.description("SOCKS Protocol Version")
```
##########
nifi-nar-bundles/nifi-standard-services/nifi-proxy-configuration-bundle/nifi-proxy-configuration/src/main/java/org/apache/nifi/proxy/StandardProxyConfigurationService.java:
##########
@@ -46,31 +46,41 @@ public class StandardProxyConfigurationService extends
AbstractControllerService
.required(true)
.build();
- static final PropertyDescriptor PROXY_SERVER_HOST = new
PropertyDescriptor.Builder()
+ public static final PropertyDescriptor SOCKS_VERSION = new
PropertyDescriptor.Builder()
+ .name("socks-version")
+ .displayName("Socks version")
+ .description("Socks version.")
+ .allowableValues(SocksVersion.values())
+ .defaultValue(SocksVersion.NOT_SPECIFIED.name())
Review Comment:
Recommend changing the default value to `SOCKS5`.
##########
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/storage/utils/AzureStorageUtils.java:
##########
@@ -311,4 +316,38 @@ public static void setProxy(final OperationContext
operationContext, final Proce
final ProxyConfiguration proxyConfig =
ProxyConfiguration.getConfiguration(processContext);
operationContext.setProxy(proxyConfig.createProxy());
}
+
+ public static void configureProxy(final NettyAsyncHttpClientBuilder
nettyClientBuilder, final PropertyContext propertyContext) {
+ final ProxyConfiguration proxyConfiguration =
ProxyConfiguration.getConfiguration(propertyContext);
+
+ if (proxyConfiguration != ProxyConfiguration.DIRECT_CONFIGURATION) {
+
+ final ProxyOptions proxyOptions = new ProxyOptions(
+ getProxyType(proxyConfiguration),
+ new
InetSocketAddress(proxyConfiguration.getProxyServerHost(),
proxyConfiguration.getProxyServerPort()));
+
+ final String proxyUserName = proxyConfiguration.getProxyUserName();
+ final String proxyUserPassword =
proxyConfiguration.getProxyUserPassword();
+ if (proxyUserName != null && proxyUserPassword != null) {
+ proxyOptions.setCredentials(proxyUserName, proxyUserPassword);
+ }
+
+ nettyClientBuilder.proxy(proxyOptions);
+ }
+ }
+
+ private static ProxyOptions.Type getProxyType(ProxyConfiguration
proxyConfiguration) {
+ if (proxyConfiguration.getProxyType() == Proxy.Type.HTTP) {
+ return ProxyOptions.Type.HTTP;
+ } else if (proxyConfiguration.getProxyType() == Proxy.Type.SOCKS) {
+ final SocksVersion socksVersion =
proxyConfiguration.getSocksVersion();
+ if (socksVersion != SocksVersion.NOT_SPECIFIED) {
+ return ProxyOptions.Type.valueOf(socksVersion.name());
+ } else {
+ throw new IllegalArgumentException("Version should be set when
SOCKS proxy type is used!");
+ }
+ } else {
+ throw new IllegalArgumentException("Unsupported proxy type: " +
proxyConfiguration.getProxyType() + " please use HTTP or SOCKS type.");
Review Comment:
Recommend simplifying the wording, as this is an unlikely exception.
```suggestion
throw new IllegalArgumentException("Unsupported proxy type: " +
proxyConfiguration.getProxyType());
```
##########
nifi-nar-bundles/nifi-standard-services/nifi-proxy-configuration-bundle/nifi-proxy-configuration/src/main/java/org/apache/nifi/proxy/StandardProxyConfigurationService.java:
##########
@@ -46,31 +46,41 @@ public class StandardProxyConfigurationService extends
AbstractControllerService
.required(true)
.build();
- static final PropertyDescriptor PROXY_SERVER_HOST = new
PropertyDescriptor.Builder()
+ public static final PropertyDescriptor SOCKS_VERSION = new
PropertyDescriptor.Builder()
+ .name("socks-version")
+ .displayName("Socks version")
Review Comment:
`SOCKS` should be uppercase:
```suggestion
.displayName("SOCKS Version")
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
