You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Vishal Suvagia via Review Board <no...@reviews.apache.org> on 2022/04/07 16:41:22 UTC

Review Request 73936: RANGER-3695 : Ranger Keystore alias should be configurable

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73936/
-----------------------------------------------------------

Review request for ranger, Ankita Sinha, Dhaval Shah, Dineshkumar Yadav, Gautam Borad, Jayendra Parab, Kishor Gollapalliwar, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Pradeep Agrawal, Ramesh Mani, Sailaja Polavarapu, and Velmurugan Periasamy.


Bugs: RANGER-3695
    https://issues.apache.org/jira/browse/RANGER-3695


Repository: ranger


Description
-------

Ranger requires keystore alias for TLS, However keystore alias should be  an optional parameter, hence should be only configured
if provided by the user.
Fix contains changes to make the keystore alias optional.


Diffs
-----

  embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java cae9075a7b7726ad5abf2b52f53f612d4223f712 


Diff: https://reviews.apache.org/r/73936/diff/1/


Testing
-------

Validated changes on a local VM with TLS enabled.


Thanks,

Vishal Suvagia


Re: Review Request 73936: RANGER-3695 : Ranger Keystore alias should be configurable

Posted by bhavik patel <bh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73936/#review225326
-----------------------------------------------------------


Ship it!




Ship It!

- bhavik patel


On April 7, 2022, 4:41 p.m., Vishal Suvagia wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73936/
> -----------------------------------------------------------
> 
> (Updated April 7, 2022, 4:41 p.m.)
> 
> 
> Review request for ranger, Ankita Sinha, Dhaval Shah, Dineshkumar Yadav, Gautam Borad, Jayendra Parab, Kishor Gollapalliwar, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Pradeep Agrawal, Ramesh Mani, Sailaja Polavarapu, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-3695
>     https://issues.apache.org/jira/browse/RANGER-3695
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Ranger requires keystore alias for TLS, However keystore alias should be  an optional parameter, hence should be only configured
> if provided by the user.
> Fix contains changes to make the keystore alias optional.
> 
> 
> Diffs
> -----
> 
>   embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java cae9075a7b7726ad5abf2b52f53f612d4223f712 
> 
> 
> Diff: https://reviews.apache.org/r/73936/diff/1/
> 
> 
> Testing
> -------
> 
> Validated changes on a local VM with TLS enabled.
> 
> 
> Thanks,
> 
> Vishal Suvagia
> 
>


Re: Review Request 73936: RANGER-3695 : Ranger Keystore alias should be configurable

Posted by Vishal Suvagia via Review Board <no...@reviews.apache.org>.

> On April 8, 2022, 4 a.m., bhavik patel wrote:
> > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java
> > Line 167 (original), 167 (patched)
> > <https://reviews.apache.org/r/73936/diff/1/?file=2267229#file2267229line167>
> >
> >     default should be "rangeradmin".

Default is not required, it should be on the user to define the alias value as it is configurable.


- Vishal


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73936/#review224269
-----------------------------------------------------------


On April 7, 2022, 4:41 p.m., Vishal Suvagia wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73936/
> -----------------------------------------------------------
> 
> (Updated April 7, 2022, 4:41 p.m.)
> 
> 
> Review request for ranger, Ankita Sinha, Dhaval Shah, Dineshkumar Yadav, Gautam Borad, Jayendra Parab, Kishor Gollapalliwar, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Pradeep Agrawal, Ramesh Mani, Sailaja Polavarapu, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-3695
>     https://issues.apache.org/jira/browse/RANGER-3695
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Ranger requires keystore alias for TLS, However keystore alias should be  an optional parameter, hence should be only configured
> if provided by the user.
> Fix contains changes to make the keystore alias optional.
> 
> 
> Diffs
> -----
> 
>   embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java cae9075a7b7726ad5abf2b52f53f612d4223f712 
> 
> 
> Diff: https://reviews.apache.org/r/73936/diff/1/
> 
> 
> Testing
> -------
> 
> Validated changes on a local VM with TLS enabled.
> 
> 
> Thanks,
> 
> Vishal Suvagia
> 
>


Re: Review Request 73936: RANGER-3695 : Ranger Keystore alias should be configurable

Posted by Vishal Suvagia via Review Board <no...@reviews.apache.org>.

> On April 8, 2022, 4 a.m., bhavik patel wrote:
> > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java
> > Line 167 (original), 167 (patched)
> > <https://reviews.apache.org/r/73936/diff/1/?file=2267229#file2267229line167>
> >
> >     default should be "rangeradmin".
> 
> Vishal Suvagia wrote:
>     Default is not required, it should be on the user to define the alias value as it is configurable.
> 
> bhavik patel wrote:
>     yeah, but if user doesn’t define then from the code it should set the default value

Without the alias value also Ranger comes up fine. Hardcoding a value necessicates the keystore to be configured with that hard coded value.
This should not be the case and need to remove the hard coded value, only configure it if user defines it ?
Do you see any use case where this value is required mandatorily ?


- Vishal


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73936/#review224269
-----------------------------------------------------------


On April 7, 2022, 4:41 p.m., Vishal Suvagia wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73936/
> -----------------------------------------------------------
> 
> (Updated April 7, 2022, 4:41 p.m.)
> 
> 
> Review request for ranger, Ankita Sinha, Dhaval Shah, Dineshkumar Yadav, Gautam Borad, Jayendra Parab, Kishor Gollapalliwar, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Pradeep Agrawal, Ramesh Mani, Sailaja Polavarapu, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-3695
>     https://issues.apache.org/jira/browse/RANGER-3695
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Ranger requires keystore alias for TLS, However keystore alias should be  an optional parameter, hence should be only configured
> if provided by the user.
> Fix contains changes to make the keystore alias optional.
> 
> 
> Diffs
> -----
> 
>   embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java cae9075a7b7726ad5abf2b52f53f612d4223f712 
> 
> 
> Diff: https://reviews.apache.org/r/73936/diff/1/
> 
> 
> Testing
> -------
> 
> Validated changes on a local VM with TLS enabled.
> 
> 
> Thanks,
> 
> Vishal Suvagia
> 
>


Re: Review Request 73936: RANGER-3695 : Ranger Keystore alias should be configurable

Posted by bhavik patel <bh...@gmail.com>.

> On April 8, 2022, 4 a.m., bhavik patel wrote:
> > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java
> > Line 167 (original), 167 (patched)
> > <https://reviews.apache.org/r/73936/diff/1/?file=2267229#file2267229line167>
> >
> >     default should be "rangeradmin".
> 
> Vishal Suvagia wrote:
>     Default is not required, it should be on the user to define the alias value as it is configurable.

yeah, but if user doesn’t define then from the code it should set the default value


- bhavik


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73936/#review224269
-----------------------------------------------------------


On April 7, 2022, 4:41 p.m., Vishal Suvagia wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73936/
> -----------------------------------------------------------
> 
> (Updated April 7, 2022, 4:41 p.m.)
> 
> 
> Review request for ranger, Ankita Sinha, Dhaval Shah, Dineshkumar Yadav, Gautam Borad, Jayendra Parab, Kishor Gollapalliwar, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Pradeep Agrawal, Ramesh Mani, Sailaja Polavarapu, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-3695
>     https://issues.apache.org/jira/browse/RANGER-3695
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Ranger requires keystore alias for TLS, However keystore alias should be  an optional parameter, hence should be only configured
> if provided by the user.
> Fix contains changes to make the keystore alias optional.
> 
> 
> Diffs
> -----
> 
>   embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java cae9075a7b7726ad5abf2b52f53f612d4223f712 
> 
> 
> Diff: https://reviews.apache.org/r/73936/diff/1/
> 
> 
> Testing
> -------
> 
> Validated changes on a local VM with TLS enabled.
> 
> 
> Thanks,
> 
> Vishal Suvagia
> 
>


Re: Review Request 73936: RANGER-3695 : Ranger Keystore alias should be configurable

Posted by bhavik patel <bh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73936/#review224269
-----------------------------------------------------------




embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java
Line 167 (original), 167 (patched)
<https://reviews.apache.org/r/73936/#comment313136>

    default should be "rangeradmin".


- bhavik patel


On April 7, 2022, 4:41 p.m., Vishal Suvagia wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73936/
> -----------------------------------------------------------
> 
> (Updated April 7, 2022, 4:41 p.m.)
> 
> 
> Review request for ranger, Ankita Sinha, Dhaval Shah, Dineshkumar Yadav, Gautam Borad, Jayendra Parab, Kishor Gollapalliwar, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Pradeep Agrawal, Ramesh Mani, Sailaja Polavarapu, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-3695
>     https://issues.apache.org/jira/browse/RANGER-3695
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Ranger requires keystore alias for TLS, However keystore alias should be  an optional parameter, hence should be only configured
> if provided by the user.
> Fix contains changes to make the keystore alias optional.
> 
> 
> Diffs
> -----
> 
>   embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java cae9075a7b7726ad5abf2b52f53f612d4223f712 
> 
> 
> Diff: https://reviews.apache.org/r/73936/diff/1/
> 
> 
> Testing
> -------
> 
> Validated changes on a local VM with TLS enabled.
> 
> 
> Thanks,
> 
> Vishal Suvagia
> 
>