You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nifi.apache.org by pvillard31 <gi...@git.apache.org> on 2016/04/17 17:36:23 UTC

[GitHub] nifi pull request: NIFI-1197 Added SSL support for MongoDB process...

GitHub user pvillard31 opened a pull request:

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

    NIFI-1197 Added SSL support for MongoDB processors

    - Upgraded MongoDB java driver
    - Refactored code for properties
    - Added support for SSL

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

    $ git pull https://github.com/pvillard31/nifi NIFI-1197

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

    https://github.com/apache/nifi/pull/360.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 #360
    
----
commit d39f82b2c79d8ecba9fe6dd7ebf673b584d42f80
Author: Pierre Villard <pi...@gmail.com>
Date:   2016-04-17T15:24:59Z

    NIFI-1197 Added SSL support for MongoDB processors

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1197 Added SSL support for MongoDB process...

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

    https://github.com/apache/nifi/pull/360#discussion_r60159711
  
    --- Diff: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/AbstractMongoProcessor.java ---
    @@ -52,6 +62,32 @@
             .required(true)
             .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
             .build();
    +    public static final PropertyDescriptor SSL_CONTEXT_SERVICE = new PropertyDescriptor.Builder()
    +        .name("SSL Context Service")
    +        .description("The SSL Context Service used to provide client certificate information for TLS/SSL "
    +                + "connections.")
    +        .required(false)
    +        .identifiesControllerService(SSLContextService.class)
    +        .build();
    +    public static final PropertyDescriptor CLIENT_AUTH = new PropertyDescriptor.Builder()
    +        .name("Client Auth")
    --- End diff --
    
    Yes, the documentation does not sufficiently encourage this, and changing existing `PropertyDescriptors` will break backward-compatibility, but moving forward, we should explicitly conform to this standard. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1197 Added SSL support for MongoDB process...

Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on the pull request:

    https://github.com/apache/nifi/pull/360#issuecomment-212008237
  
    @olegz @alopresto PR updated. Let me know if you want me to squash commits.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1197 Added SSL support for MongoDB process...

Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on the pull request:

    https://github.com/apache/nifi/pull/360#issuecomment-211126729
  
    Changes and additions look good, testing adequate, +1. 
    Will let it sit for another day and unless there are any objections will merge.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1197 Added SSL support for MongoDB process...

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

    https://github.com/apache/nifi/pull/360#discussion_r60144583
  
    --- Diff: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/AbstractMongoProcessor.java ---
    @@ -52,6 +62,32 @@
             .required(true)
             .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
             .build();
    +    public static final PropertyDescriptor SSL_CONTEXT_SERVICE = new PropertyDescriptor.Builder()
    +        .name("SSL Context Service")
    +        .description("The SSL Context Service used to provide client certificate information for TLS/SSL "
    +                + "connections.")
    +        .required(false)
    +        .identifiesControllerService(SSLContextService.class)
    +        .build();
    +    public static final PropertyDescriptor CLIENT_AUTH = new PropertyDescriptor.Builder()
    +        .name("Client Auth")
    --- End diff --
    
    I agree and actually started using it myself on new components, so even though its not a common pattern in NiFi, I am certainly +1 into making it one.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1197 Added SSL support for MongoDB process...

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

    https://github.com/apache/nifi/pull/360#discussion_r60168086
  
    --- Diff: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/AbstractMongoProcessor.java ---
    @@ -52,6 +62,32 @@
             .required(true)
             .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
             .build();
    +    public static final PropertyDescriptor SSL_CONTEXT_SERVICE = new PropertyDescriptor.Builder()
    +        .name("SSL Context Service")
    +        .description("The SSL Context Service used to provide client certificate information for TLS/SSL "
    +                + "connections.")
    +        .required(false)
    +        .identifiesControllerService(SSLContextService.class)
    +        .build();
    +    public static final PropertyDescriptor CLIENT_AUTH = new PropertyDescriptor.Builder()
    +        .name("Client Auth")
    --- End diff --
    
    Not specific to this PR but rather this emerging guidance about machine safety.  We should encourage the use of displayName and better articulate the difference between name and displayName.  However, we should also be careful suggesting 'this-is-for-robots' is anymore machine safe than 'This is for Robots'.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1197 Added SSL support for MongoDB process...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1197 Added SSL support for MongoDB process...

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

    https://github.com/apache/nifi/pull/360#discussion_r60144254
  
    --- Diff: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/AbstractMongoProcessor.java ---
    @@ -52,6 +62,32 @@
             .required(true)
             .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
             .build();
    +    public static final PropertyDescriptor SSL_CONTEXT_SERVICE = new PropertyDescriptor.Builder()
    +        .name("SSL Context Service")
    +        .description("The SSL Context Service used to provide client certificate information for TLS/SSL "
    +                + "connections.")
    +        .required(false)
    +        .identifiesControllerService(SSLContextService.class)
    +        .build();
    +    public static final PropertyDescriptor CLIENT_AUTH = new PropertyDescriptor.Builder()
    +        .name("Client Auth")
    --- End diff --
    
    `.name` should be a machine-safe string (i.e. `ssl-client-auth`) which will remain constant over the life of the processor because it is used for object resolution when loading from the `flow.tar.gz` file. For the human-readable value to display in the UI, please use `.displayName`. This can be changed without affecting object resolution (for future renaming, internationalization, etc.). 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1197 Added SSL support for MongoDB process...

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

    https://github.com/apache/nifi/pull/360#discussion_r60144137
  
    --- Diff: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/AbstractMongoProcessor.java ---
    @@ -52,6 +62,32 @@
             .required(true)
             .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
             .build();
    +    public static final PropertyDescriptor SSL_CONTEXT_SERVICE = new PropertyDescriptor.Builder()
    +        .name("SSL Context Service")
    --- End diff --
    
    `.name` should be a machine-safe string (i.e. `ssl-context-service`) which will remain constant over the life of the processor because it is used for object resolution when loading from the `flow.tar.gz` file. For the human-readable value to display in the UI, please use `.displayName`. This can be changed without affecting object resolution (for future renaming, internationalization, etc.). 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1197 Added SSL support for MongoDB process...

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

    https://github.com/apache/nifi/pull/360#discussion_r60209844
  
    --- Diff: nifi-nar-bundles/nifi-mongodb-bundle/nifi-mongodb-processors/src/main/java/org/apache/nifi/processors/mongodb/AbstractMongoProcessor.java ---
    @@ -52,6 +62,32 @@
             .required(true)
             .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
             .build();
    +    public static final PropertyDescriptor SSL_CONTEXT_SERVICE = new PropertyDescriptor.Builder()
    +        .name("SSL Context Service")
    +        .description("The SSL Context Service used to provide client certificate information for TLS/SSL "
    +                + "connections.")
    +        .required(false)
    +        .identifiesControllerService(SSLContextService.class)
    +        .build();
    +    public static final PropertyDescriptor CLIENT_AUTH = new PropertyDescriptor.Builder()
    +        .name("Client Auth")
    --- End diff --
    
    Will update the PR shortly. And I will do the same for the PR regarding SSL over AMQP processors.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---