You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by harshach <gi...@git.apache.org> on 2015/03/24 23:49:31 UTC

[GitHub] storm pull request: STORM-721. Storm UI server should support SSL.

GitHub user harshach opened a pull request:

    https://github.com/apache/storm/pull/479

    STORM-721. Storm UI server should support SSL.

    

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

    $ git pull https://github.com/harshach/incubator-storm STORM-721

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

    https://github.com/apache/storm/pull/479.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 #479
    
----
commit e86b924e860bfe7722785b81f9647c5697a9461f
Author: Sriharsha Chintalapani <ma...@harsha.io>
Date:   2015-03-24T22:48:03Z

    STORM-721. Storm UI server should support SSL.

----


---
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] storm pull request: STORM-721. Storm UI server should support SSL.

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

    https://github.com/apache/storm/pull/479#discussion_r27926686
  
    --- Diff: storm-core/src/clj/backtype/storm/daemon/drpc.clj ---
    @@ -225,7 +225,13 @@
                   https-port (int (conf DRPC-HTTPS-PORT))
                   https-ks-path (conf DRPC-HTTPS-KEYSTORE-PATH)
                   https-ks-password (conf DRPC-HTTPS-KEYSTORE-PASSWORD)
    -              https-ks-type (conf DRPC-HTTPS-KEYSTORE-TYPE)]
    +              https-ks-type (conf DRPC-HTTPS-KEYSTORE-TYPE)
    +              https-key-password (conf DRPC-HTTPS-KEY-PASSWORD)
    +              https-ts-path (conf DRPC-HTTPS-TRUSTSTORE-PATH)
    +              https-ts-password (conf DRPC-HTTPS-TRUSTSTORE-PASSWORD)
    +              https-ts-type (conf DRPC-HTTPS-TRUSTSTORE-TYPE)
    +              https-want-client-auth (conf DRPC-HTTPS-WANT-CLIENT-AUTH)
    --- End diff --
    
    the documentation for DRPC-HTTPS-WANT-CLIENT-AUTH and DRPC-HTTPS-NEED-CLIENT-AUTH is missing from SECURITY.md.


---
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] storm pull request: STORM-721. Storm UI server should support SSL.

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

    https://github.com/apache/storm/pull/479#issuecomment-90751945
  
    Thanks for the review @Parth-Brahmbhatt  . Updated the security.md .


---
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] storm pull request: STORM-721. Storm UI server should support SSL.

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

    https://github.com/apache/storm/pull/479


---
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] storm pull request: STORM-721. Storm UI server should support SSL.

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

    https://github.com/apache/storm/pull/479#discussion_r27926806
  
    --- Diff: storm-core/src/clj/backtype/storm/ui/core.clj ---
    @@ -1078,10 +1078,32 @@
         (let [conf *STORM-CONF*
               header-buffer-size (int (.get conf UI-HEADER-BUFFER-BYTES))
               filters-confs [{:filter-class (conf UI-FILTER)
    -                          :filter-params (conf UI-FILTER-PARAMS)}]]
    +                          :filter-params (conf UI-FILTER-PARAMS)}]
    +          https-port (if (not-nil? (conf UI-HTTPS-PORT)) (conf UI-HTTPS-PORT) 0)
    +          https-ks-path (conf UI-HTTPS-KEYSTORE-PATH)
    +          https-ks-password (conf UI-HTTPS-KEYSTORE-PASSWORD)
    +          https-ks-type (conf UI-HTTPS-KEYSTORE-TYPE)
    +          https-key-password (conf UI-HTTPS-KEY-PASSWORD)
    +          https-ts-path (conf UI-HTTPS-TRUSTSTORE-PATH)
    +          https-ts-password (conf UI-HTTPS-TRUSTSTORE-PASSWORD)
    +          https-ts-type (conf UI-HTTPS-TRUSTSTORE-TYPE)
    +          https-want-client-auth (conf UI-HTTPS-WANT-CLIENT-AUTH)
    --- End diff --
    
    Same here, we should add the documentation to SECURITY.MD.


---
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] storm pull request: STORM-721. Storm UI server should support SSL.

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

    https://github.com/apache/storm/pull/479#issuecomment-90748269
  
    I am +1 once SECURITY.MD has documentation for all the configs.


---
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] storm pull request: STORM-721. Storm UI server should support SSL.

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

    https://github.com/apache/storm/pull/479#issuecomment-90316831
  
    @ptgoetz @Parth-Brahmbhatt @revans2 appreciate any feedback on this PR>


---
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] storm pull request: STORM-721. Storm UI server should support SSL.

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

    https://github.com/apache/storm/pull/479#discussion_r27090305
  
    --- Diff: storm-core/src/jvm/backtype/storm/Config.java ---
    @@ -567,6 +567,39 @@
         public static final Object UI_HEADER_BUFFER_BYTES_SCHEMA = Number.class;
     
         /**
    +     * This port is used by Storm DRPC for receiving HTTPS (SSL) DPRC requests from clients.
    +     */
    +    public static final String UI_HTTPS_PORT = "ui.https.port";
    --- End diff --
    
    Will be better if you could  modify file  
    ```
    storm-core/conf/defaults.yaml  
    ```
    and give these newly added configs default values.


---
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] storm pull request: STORM-721. Storm UI server should support SSL.

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

    https://github.com/apache/storm/pull/479#issuecomment-85788631
  
    @caofangkun this is not a manadatory fields as it its optional to configure ssl so no need for defaults here.


---
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] storm pull request: STORM-721. Storm UI server should support SSL.

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

    https://github.com/apache/storm/pull/479#issuecomment-90752115
  
    +1.


---
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.
---