You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/10/10 19:57:32 UTC
[GitHub] [nifi] exceptionfactory opened a new pull request #4594: NIFI-3669 Add SSL Support to CaptureChangeMySQL
exceptionfactory opened a new pull request #4594:
URL: https://github.com/apache/nifi/pull/4594
#### Description of PR
NIFI-3669 Add SSL support to Binary Log Client in CaptureChangeMySQL processor. Introduces new SSL Mode property defaulted to DISABLED with allowed values provided from the binary log client library. Also introduces optional SSL Context Service property with custom validation to ensure that SSL Context Service is configured when SSL Mode is set to something other than DISABLED. Includes unit tests to verify the presence of an SSLSocketFactory on the mocked binary log client and the creation of an SSLSocket with appropriate exception handling.
PR #3456 includes the same conceptual functionality, but it contains unnecessary work to initialize a new SSLContext, does not include any new unit tests, and contains merge conflicts.
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 `main`)?
- [X] Is your initial contribution a single, squashed commit? _Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not `squash` or use `--force` when pushing to allow for clean monitoring of changes._
### 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?
- [X] Have you verified that the full build is successful on JDK 8?
- [X] Have you verified that the full build is successful on JDK 11?
- [ ] 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`?
- [X] If adding new Properties, have you added `.displayName` in addition to .name (programmatic access) for each of the new properties?
### For documentation related changes:
- [ ] 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 GitHub Actions CI for build issues and submit an update to your PR as soon as possible.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] asfgit closed pull request #4594: NIFI-3669 Add SSL Support to CaptureChangeMySQL
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #4594:
URL: https://github.com/apache/nifi/pull/4594
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] pvillard31 commented on a change in pull request #4594: NIFI-3669 Add SSL Support to CaptureChangeMySQL
Posted by GitBox <gi...@apache.org>.
pvillard31 commented on a change in pull request #4594:
URL: https://github.com/apache/nifi/pull/4594#discussion_r561646469
##########
File path: nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java
##########
@@ -368,6 +393,23 @@
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
.build();
+ public static final PropertyDescriptor SSL_CONTEXT_SERVICE = new PropertyDescriptor.Builder()
+ .name("SSL Context Service")
+ .displayName("SSL Context Service")
+ .description("SSL Context Service supporting encrypted socket communication")
+ .required(false)
+ .identifiesControllerService(SSLContextService.class)
+ .build();
+
+ public static final PropertyDescriptor SSL_MODE = new PropertyDescriptor.Builder()
Review comment:
Do we want to use the new .dependsOn functionality to display this property only if the SSL Context Service is set?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a change in pull request #4594: NIFI-3669 Add SSL Support to CaptureChangeMySQL
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4594:
URL: https://github.com/apache/nifi/pull/4594#discussion_r561903819
##########
File path: nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java
##########
@@ -368,6 +393,23 @@
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
.build();
+ public static final PropertyDescriptor SSL_CONTEXT_SERVICE = new PropertyDescriptor.Builder()
+ .name("SSL Context Service")
+ .displayName("SSL Context Service")
+ .description("SSL Context Service supporting encrypted socket communication")
+ .required(false)
+ .identifiesControllerService(SSLContextService.class)
+ .build();
+
+ public static final PropertyDescriptor SSL_MODE = new PropertyDescriptor.Builder()
Review comment:
@pvillard31 Thanks for following up with the recommendation. I updated the `SSL Context Service` property descriptor to depend on the relevant values from the `SSL Mode` property.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] pvillard31 commented on a change in pull request #4594: NIFI-3669 Add SSL Support to CaptureChangeMySQL
Posted by GitBox <gi...@apache.org>.
pvillard31 commented on a change in pull request #4594:
URL: https://github.com/apache/nifi/pull/4594#discussion_r503349223
##########
File path: nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java
##########
@@ -362,6 +377,23 @@
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
.build();
+ public static final PropertyDescriptor SSL_CONTEXT_SERVICE = new PropertyDescriptor.Builder()
+ .name("SSL Context Service")
+ .displayName("SSL Context Service")
+ .description("SSL Context Service supporting encrypted socket communication")
+ .required(false)
+ .identifiesControllerService(SSLContextService.class)
+ .build();
+
+ public static final PropertyDescriptor SSL_MODE = new PropertyDescriptor.Builder()
+ .name("SSL Mode")
+ .displayName("SSL Mode")
+ .description("SSL Mode used when SSL Context Service configured supporting certificate verification options")
+ .required(true)
+ .defaultValue(SSLMode.DISABLED.toString())
+ .allowableValues(SSL_MODES)
+ .build();
+
Review comment:
Yeah very good points, it makes sense. Can we add the comments from the library to the description of the allowable values? you will have to duplicate the enum on NiFi's side I'm afraid but will provide more clarity to our users. Thoughts?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a change in pull request #4594: NIFI-3669 Add SSL Support to CaptureChangeMySQL
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4594:
URL: https://github.com/apache/nifi/pull/4594#discussion_r528797353
##########
File path: nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java
##########
@@ -362,6 +377,23 @@
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
.build();
+ public static final PropertyDescriptor SSL_CONTEXT_SERVICE = new PropertyDescriptor.Builder()
+ .name("SSL Context Service")
+ .displayName("SSL Context Service")
+ .description("SSL Context Service supporting encrypted socket communication")
+ .required(false)
+ .identifiesControllerService(SSLContextService.class)
+ .build();
+
+ public static final PropertyDescriptor SSL_MODE = new PropertyDescriptor.Builder()
+ .name("SSL Mode")
+ .displayName("SSL Mode")
+ .description("SSL Mode used when SSL Context Service configured supporting certificate verification options")
+ .required(true)
+ .defaultValue(SSLMode.DISABLED.toString())
+ .allowableValues(SSL_MODES)
+ .build();
+
Review comment:
@pvillard31 Thanks again for the initial feedback, do you have any other recommendations following the change to include a detailed description of the SSL Mode properties?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] pvillard31 commented on pull request #4594: NIFI-3669 Add SSL Support to CaptureChangeMySQL
Posted by GitBox <gi...@apache.org>.
pvillard31 commented on pull request #4594:
URL: https://github.com/apache/nifi/pull/4594#issuecomment-783155749
Merged to main, thanks @exceptionfactory !
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a change in pull request #4594: NIFI-3669 Add SSL Support to CaptureChangeMySQL
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4594:
URL: https://github.com/apache/nifi/pull/4594#discussion_r503316088
##########
File path: nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/pom.xml
##########
@@ -45,24 +45,17 @@ language governing permissions and limitations under the License. -->
</dependency>
<dependency>
<groupId>org.apache.nifi</groupId>
- <artifactId>nifi-mock</artifactId>
- <version>1.13.0-SNAPSHOT</version>
- <scope>test</scope>
- </dependency>
- <dependency>
- <groupId>org.apache.nifi</groupId>
- <artifactId>nifi-distributed-cache-client-service</artifactId>
- <scope>test</scope>
+ <artifactId>nifi-ssl-context-service-api</artifactId>
</dependency>
<dependency>
<groupId>org.apache.nifi</groupId>
- <artifactId>nifi-ssl-context-service-api</artifactId>
Review comment:
This was a result of moving nifi-ssl-context-service-api before nifi-mock to keep the test dependencies listed together.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] pvillard31 commented on a change in pull request #4594: NIFI-3669 Add SSL Support to CaptureChangeMySQL
Posted by GitBox <gi...@apache.org>.
pvillard31 commented on a change in pull request #4594:
URL: https://github.com/apache/nifi/pull/4594#discussion_r561646469
##########
File path: nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java
##########
@@ -368,6 +393,23 @@
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
.build();
+ public static final PropertyDescriptor SSL_CONTEXT_SERVICE = new PropertyDescriptor.Builder()
+ .name("SSL Context Service")
+ .displayName("SSL Context Service")
+ .description("SSL Context Service supporting encrypted socket communication")
+ .required(false)
+ .identifiesControllerService(SSLContextService.class)
+ .build();
+
+ public static final PropertyDescriptor SSL_MODE = new PropertyDescriptor.Builder()
Review comment:
Do we want to use the new .dependsOn functionality to display this property only if the SSL Context Service is set?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] pvillard31 commented on a change in pull request #4594: NIFI-3669 Add SSL Support to CaptureChangeMySQL
Posted by GitBox <gi...@apache.org>.
pvillard31 commented on a change in pull request #4594:
URL: https://github.com/apache/nifi/pull/4594#discussion_r503265418
##########
File path: nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/pom.xml
##########
@@ -45,24 +45,17 @@ language governing permissions and limitations under the License. -->
</dependency>
<dependency>
<groupId>org.apache.nifi</groupId>
- <artifactId>nifi-mock</artifactId>
- <version>1.13.0-SNAPSHOT</version>
- <scope>test</scope>
- </dependency>
- <dependency>
- <groupId>org.apache.nifi</groupId>
- <artifactId>nifi-distributed-cache-client-service</artifactId>
- <scope>test</scope>
+ <artifactId>nifi-ssl-context-service-api</artifactId>
</dependency>
<dependency>
<groupId>org.apache.nifi</groupId>
- <artifactId>nifi-ssl-context-service-api</artifactId>
Review comment:
A bit confused by the diff here, shouldn't we just remove the ``test`` scope?
##########
File path: nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java
##########
@@ -738,6 +801,13 @@ protected void connect(List<InetSocketAddress> hosts, String username, String pa
binlogClient.setServerId(serverId);
}
+ binlogClient.setSSLMode(sslMode);
+ if (sslContextService != null) {
+ final SSLContext sslContext = sslContextService.createSSLContext(ClientAuth.NONE);
Review comment:
Why using ``ClientAuth.NONE`` here?
##########
File path: nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java
##########
@@ -362,6 +377,23 @@
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
.build();
+ public static final PropertyDescriptor SSL_CONTEXT_SERVICE = new PropertyDescriptor.Builder()
+ .name("SSL Context Service")
+ .displayName("SSL Context Service")
+ .description("SSL Context Service supporting encrypted socket communication")
+ .required(false)
+ .identifiesControllerService(SSLContextService.class)
+ .build();
+
+ public static final PropertyDescriptor SSL_MODE = new PropertyDescriptor.Builder()
+ .name("SSL Mode")
+ .displayName("SSL Mode")
+ .description("SSL Mode used when SSL Context Service configured supporting certificate verification options")
+ .required(true)
+ .defaultValue(SSLMode.DISABLED.toString())
+ .allowableValues(SSL_MODES)
+ .build();
+
Review comment:
I'm a bit concerned by this property as this is going to overlap with what is defined in the SSL Context Service for the client authentication.
SSL Context Service provides three options:
https://github.com/apache/nifi/blob/c2960998ac70009d9365326dce47b3d15ad299e4/nifi-commons/nifi-security-utils-api/src/main/java/org/apache/nifi/security/util/ClientAuth.java#L26-L28
SSL_MODES are providing the below options:
https://github.com/shyiko/mysql-binlog-connector-java/blob/dd710a5466381faa57442977b24fceff56a0820e/src/main/java/com/github/shyiko/mysql/binlog/network/SSLMode.java#L23-L48
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #4594: NIFI-3669 Add SSL Support to CaptureChangeMySQL
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on pull request #4594:
URL: https://github.com/apache/nifi/pull/4594#issuecomment-707229793
The most recent CI build failed for Ubuntu JDK 11 EN and Windows JDK 1.8 due to an HTTP 503 when attempting to retrieve a dependency. Other builds completed without errors.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a change in pull request #4594: NIFI-3669 Add SSL Support to CaptureChangeMySQL
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4594:
URL: https://github.com/apache/nifi/pull/4594#discussion_r561903819
##########
File path: nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java
##########
@@ -368,6 +393,23 @@
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
.build();
+ public static final PropertyDescriptor SSL_CONTEXT_SERVICE = new PropertyDescriptor.Builder()
+ .name("SSL Context Service")
+ .displayName("SSL Context Service")
+ .description("SSL Context Service supporting encrypted socket communication")
+ .required(false)
+ .identifiesControllerService(SSLContextService.class)
+ .build();
+
+ public static final PropertyDescriptor SSL_MODE = new PropertyDescriptor.Builder()
Review comment:
@pvillard31 Thanks for following up with the recommendation. I updated the `SSL Context Service` property descriptor to depend on the relevant values from the `SSL Mode` property.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a change in pull request #4594: NIFI-3669 Add SSL Support to CaptureChangeMySQL
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4594:
URL: https://github.com/apache/nifi/pull/4594#discussion_r503306827
##########
File path: nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java
##########
@@ -362,6 +377,23 @@
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
.build();
+ public static final PropertyDescriptor SSL_CONTEXT_SERVICE = new PropertyDescriptor.Builder()
+ .name("SSL Context Service")
+ .displayName("SSL Context Service")
+ .description("SSL Context Service supporting encrypted socket communication")
+ .required(false)
+ .identifiesControllerService(SSLContextService.class)
+ .build();
+
+ public static final PropertyDescriptor SSL_MODE = new PropertyDescriptor.Builder()
+ .name("SSL Mode")
+ .displayName("SSL Mode")
+ .description("SSL Mode used when SSL Context Service configured supporting certificate verification options")
+ .required(true)
+ .defaultValue(SSLMode.DISABLED.toString())
+ .allowableValues(SSL_MODES)
+ .build();
+
Review comment:
Although there is some similarity, the SSLMode options control the flexibility of the MySQL Binary Log client during TLS negotiation, whereas ClientAuth influences how an SSL Server Socket would respond. Since the Binary Log connection is a client, the ClientAuth options are not necessarily applicable.
SSLMode provides some more lenient connection options, such as PREFERRED, where the Binary Log client will request SSL, but will still connect and function if the MySQL server does not support SSL. The VERIFY_IDENTITY option is a special case that causes the Binary Log client to use a custom TlsHostnameVerifier to compare the network connected hostname to the certificate presented. The REQUIRED and VERIFY_CA options result in essentially the same behavior for the purposes of CaptureChangeMySQL since SSLContextService always provides the SSLSocketFactory instead of having the Binary Log client use it's own preconfigured factories.
Another option would be attempting to translate ClientAuth values to an applicable SSLMode, but there isn't a direct correlation, and I think that would end up being more confusing. Based on how the Binary Log client uses SSLMode, it might make sense to limit the allowable values to DISABLED, PREFERRED, REQUIRED, and VERIFY_IDENTITY. As mentioned, the VERIFY_IDENTITY option is a special case that would be more restrictive than REQUIRED due to the custom hostname verification. If that sounds too confusing, then it would be best to simply remove the SSL Mode property and use the presence of an SSLContextService to set REQUIRED.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a change in pull request #4594: NIFI-3669 Add SSL Support to CaptureChangeMySQL
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4594:
URL: https://github.com/apache/nifi/pull/4594#discussion_r503322190
##########
File path: nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java
##########
@@ -738,6 +801,13 @@ protected void connect(List<InetSocketAddress> hosts, String username, String pa
binlogClient.setServerId(serverId);
}
+ binlogClient.setSSLMode(sslMode);
+ if (sslContextService != null) {
+ final SSLContext sslContext = sslContextService.createSSLContext(ClientAuth.NONE);
Review comment:
Since the ClientAuth options influence the values of setNeedClientAuth() and setWantClientAuth() on the default SSLParameters, the value of ClientAuth is effectively ignored when the SSL Socket is acting in client mode, as in the case of the Binary Log client. In this PR, with multiple options for SSLMode, passing ClientAuth.NONE seemed to be the most intuitive default option, since SSLMode.PREFERRED does not mandate an SSL connection.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a change in pull request #4594: NIFI-3669 Add SSL Support to CaptureChangeMySQL
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4594:
URL: https://github.com/apache/nifi/pull/4594#discussion_r503357934
##########
File path: nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java
##########
@@ -362,6 +377,23 @@
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
.build();
+ public static final PropertyDescriptor SSL_CONTEXT_SERVICE = new PropertyDescriptor.Builder()
+ .name("SSL Context Service")
+ .displayName("SSL Context Service")
+ .description("SSL Context Service supporting encrypted socket communication")
+ .required(false)
+ .identifiesControllerService(SSLContextService.class)
+ .build();
+
+ public static final PropertyDescriptor SSL_MODE = new PropertyDescriptor.Builder()
+ .name("SSL Mode")
+ .displayName("SSL Mode")
+ .description("SSL Mode used when SSL Context Service configured supporting certificate verification options")
+ .required(true)
+ .defaultValue(SSLMode.DISABLED.toString())
+ .allowableValues(SSL_MODES)
+ .build();
+
Review comment:
Thanks for the feedback. Adding more description around each allowable value makes sense since the options are not necessarily intuitively obvious. Based on how CaptureChangeMySQL will provide the SSLSocketFactory, I will also remove VERIFY_CA as an option.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a change in pull request #4594: NIFI-3669 Add SSL Support to CaptureChangeMySQL
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4594:
URL: https://github.com/apache/nifi/pull/4594#discussion_r503357934
##########
File path: nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java
##########
@@ -362,6 +377,23 @@
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
.build();
+ public static final PropertyDescriptor SSL_CONTEXT_SERVICE = new PropertyDescriptor.Builder()
+ .name("SSL Context Service")
+ .displayName("SSL Context Service")
+ .description("SSL Context Service supporting encrypted socket communication")
+ .required(false)
+ .identifiesControllerService(SSLContextService.class)
+ .build();
+
+ public static final PropertyDescriptor SSL_MODE = new PropertyDescriptor.Builder()
+ .name("SSL Mode")
+ .displayName("SSL Mode")
+ .description("SSL Mode used when SSL Context Service configured supporting certificate verification options")
+ .required(true)
+ .defaultValue(SSLMode.DISABLED.toString())
+ .allowableValues(SSL_MODES)
+ .build();
+
Review comment:
Thanks for the feedback. Adding more description around each allowable value makes sense since the options or not necessarily intuitively obvious. Based on how CaptureChangeMySQL will provide the SSLSocketFactory, I will also remove VERIFY_CA as an option.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org