You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Abhay Kulkarni <ak...@hortonworks.com> on 2016/05/19 02:51:51 UTC
Review Request 47571: JAAS configuration needed for Kafka interaction
via Atlas config file
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47571/
-----------------------------------------------------------
Review request for atlas, Madhan Neethiraj, Ramesh Mani, Shwetha GS, Suma Shivaprasad, and Hemanth Yamijala.
Bugs: ATLAS-809
https://issues.apache.org/jira/browse/ATLAS-809
Repository: atlas
Description
-------
In-Memory JAAS configurator obviates need for passing -Djava.security.auth.login.config command line argument to JVMs when starting various components that may interact with Kafka.
Diffs (updated)
-----
common/pom.xml 614b3f6
common/src/main/java/org/apache/atlas/ApplicationProperties.java 6a4dca3
common/src/main/java/org/apache/atlas/security/InMemoryJAASConfiguration.java PRE-CREATION
common/src/test/java/org/apache/atlas/security/InMemoryJAASConfigurationTest.java PRE-CREATION
common/src/test/resources/atlas-jaas.properties PRE-CREATION
distro/src/conf/atlas-application.properties d4722fb
typesystem/src/main/resources/atlas-application.properties a8e77bb
Diff: https://reviews.apache.org/r/47571/diff/
Testing
-------
Passed unit tests.
Thanks,
Abhay Kulkarni
Re: Review Request 47571: JAAS configuration needed for Kafka
interaction via Atlas config file
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47571/#review133863
-----------------------------------------------------------
common/src/main/java/org/apache/atlas/security/InMemoryJAASConfiguration.java (line 146)
<https://reviews.apache.org/r/47571/#comment198510>
Please ensure that the opened input stream is closed.
common/src/main/java/org/apache/atlas/security/InMemoryJAASConfiguration.java (line 153)
<https://reviews.apache.org/r/47571/#comment198515>
This would return only the configurations that start with "option". Perhaps you meant to use JAAS_CONFIG_PREFIX_PARAM ("atlas.jass.") instead of JAAS_CONFIG_LOGIN_OPTIONS_PREFIX ("option"). Please review.
Also, initilize() method processes only properties that start with JAAS_CONFIG_PREFIX_PARAM (line #205). However, subset() call here would strip the given prefix. Given that necessary filtering is done in initialize() method, is it necessary to get a subset here?
Ref: http://commons.apache.org/proper/commons-configuration/javadocs/v1.10/apidocs/index.html
common/src/main/java/org/apache/atlas/security/InMemoryJAASConfiguration.java (line 168)
<https://reviews.apache.org/r/47571/#comment198512>
Looks like properties loading is done in the constructor - line #167. Please review and move the logs accordingly.
Better yet, consider moving these logs to InMemoryJAASConfiguration.initialize() method.
common/src/main/java/org/apache/atlas/security/InMemoryJAASConfiguration.java (line 180)
<https://reviews.apache.org/r/47571/#comment198513>
Please add entry/exit debug logs. These will be handy while troubleshooting.
- Madhan Neethiraj
On May 19, 2016, 2:51 a.m., Abhay Kulkarni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47571/
> -----------------------------------------------------------
>
> (Updated May 19, 2016, 2:51 a.m.)
>
>
> Review request for atlas, Madhan Neethiraj, Ramesh Mani, Shwetha GS, Suma Shivaprasad, and Hemanth Yamijala.
>
>
> Bugs: ATLAS-809
> https://issues.apache.org/jira/browse/ATLAS-809
>
>
> Repository: atlas
>
>
> Description
> -------
>
> In-Memory JAAS configurator obviates need for passing -Djava.security.auth.login.config command line argument to JVMs when starting various components that may interact with Kafka.
>
>
> Diffs
> -----
>
> common/pom.xml 614b3f6
> common/src/main/java/org/apache/atlas/ApplicationProperties.java 6a4dca3
> common/src/main/java/org/apache/atlas/security/InMemoryJAASConfiguration.java PRE-CREATION
> common/src/test/java/org/apache/atlas/security/InMemoryJAASConfigurationTest.java PRE-CREATION
> common/src/test/resources/atlas-jaas.properties PRE-CREATION
> distro/src/conf/atlas-application.properties d4722fb
> typesystem/src/main/resources/atlas-application.properties a8e77bb
>
> Diff: https://reviews.apache.org/r/47571/diff/
>
>
> Testing
> -------
>
> Passed unit tests.
>
>
> Thanks,
>
> Abhay Kulkarni
>
>
Re: Review Request 47571: JAAS configuration needed for Kafka
interaction via Atlas config file
Posted by Shwetha GS <ss...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47571/#review134112
-----------------------------------------------------------
common/src/main/java/org/apache/atlas/security/InMemoryJAASConfiguration.java (line 155)
<https://reviews.apache.org/r/47571/#comment198764>
close() should be in finally clause
common/src/main/java/org/apache/atlas/security/InMemoryJAASConfiguration.java (line 173)
<https://reviews.apache.org/r/47571/#comment198765>
Use ConfigurationConverter.getProperties()
common/src/main/java/org/apache/atlas/security/InMemoryJAASConfiguration.java (line 188)
<https://reviews.apache.org/r/47571/#comment198766>
I guess the if check is to avoid string concatenation if the log level is not enabled.
You can avoid this check by using org.slf4j.Logger.debug(String format, Object... arguments). This does string format only if the log level is enabled
For example,
if (LOG.isDebugEnabled()) {
LOG.debug("==> InMemoryJAASConfiguration.getAppConfigurationEntry(" + name + ")");
}
can be simplified to
LOG.debug("==> InMemoryJAASConfiguration.getAppConfigurationEntry({})", name);
The changed code looks cleaner
common/src/main/java/org/apache/atlas/security/InMemoryJAASConfiguration.java (line 205)
<https://reviews.apache.org/r/47571/#comment198767>
This and the next debug statements are printed continuosly in application.log every 5 secs even when these is no activity in atlas server. Can you move these statements to TRACE level?
distro/src/conf/atlas-application.properties (line 94)
<https://reviews.apache.org/r/47571/#comment198775>
These configs are required only if kafka security is enabled. In the default setup, kafka is started as unsecured embedded server and hence these configs are not required. We should just comment these for reference
Can you also update the documentation at docs/src/site/twiki/security.twiki
- Shwetha GS
On May 19, 2016, 5:45 p.m., Abhay Kulkarni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47571/
> -----------------------------------------------------------
>
> (Updated May 19, 2016, 5:45 p.m.)
>
>
> Review request for atlas, Madhan Neethiraj, Ramesh Mani, Shwetha GS, Suma Shivaprasad, and Hemanth Yamijala.
>
>
> Bugs: ATLAS-809
> https://issues.apache.org/jira/browse/ATLAS-809
>
>
> Repository: atlas
>
>
> Description
> -------
>
> In-Memory JAAS configurator obviates need for passing -Djava.security.auth.login.config command line argument to JVMs when starting various components that may interact with Kafka.
>
>
> Diffs
> -----
>
> common/pom.xml 614b3f6
> common/src/main/java/org/apache/atlas/ApplicationProperties.java 6a4dca3
> common/src/main/java/org/apache/atlas/security/InMemoryJAASConfiguration.java PRE-CREATION
> common/src/test/java/org/apache/atlas/security/InMemoryJAASConfigurationTest.java PRE-CREATION
> common/src/test/resources/atlas-jaas.properties PRE-CREATION
> distro/src/conf/atlas-application.properties d4722fb
> pom.xml 30fb95a
> typesystem/src/main/resources/atlas-application.properties a8e77bb
>
> Diff: https://reviews.apache.org/r/47571/diff/
>
>
> Testing
> -------
>
> Passed unit tests.
>
>
> Thanks,
>
> Abhay Kulkarni
>
>
Re: Review Request 47571: JAAS configuration needed for Kafka
interaction via Atlas config file
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47571/#review134006
-----------------------------------------------------------
Ship it!
Ship It!
- Madhan Neethiraj
On May 19, 2016, 5:45 p.m., Abhay Kulkarni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47571/
> -----------------------------------------------------------
>
> (Updated May 19, 2016, 5:45 p.m.)
>
>
> Review request for atlas, Madhan Neethiraj, Ramesh Mani, Shwetha GS, Suma Shivaprasad, and Hemanth Yamijala.
>
>
> Bugs: ATLAS-809
> https://issues.apache.org/jira/browse/ATLAS-809
>
>
> Repository: atlas
>
>
> Description
> -------
>
> In-Memory JAAS configurator obviates need for passing -Djava.security.auth.login.config command line argument to JVMs when starting various components that may interact with Kafka.
>
>
> Diffs
> -----
>
> common/pom.xml 614b3f6
> common/src/main/java/org/apache/atlas/ApplicationProperties.java 6a4dca3
> common/src/main/java/org/apache/atlas/security/InMemoryJAASConfiguration.java PRE-CREATION
> common/src/test/java/org/apache/atlas/security/InMemoryJAASConfigurationTest.java PRE-CREATION
> common/src/test/resources/atlas-jaas.properties PRE-CREATION
> distro/src/conf/atlas-application.properties d4722fb
> pom.xml 30fb95a
> typesystem/src/main/resources/atlas-application.properties a8e77bb
>
> Diff: https://reviews.apache.org/r/47571/diff/
>
>
> Testing
> -------
>
> Passed unit tests.
>
>
> Thanks,
>
> Abhay Kulkarni
>
>
Re: Review Request 47571: JAAS configuration needed for Kafka
interaction via Atlas config file
Posted by Shwetha GS <ss...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47571/#review134312
-----------------------------------------------------------
Ship it!
Ship It!
- Shwetha GS
On May 21, 2016, 12:18 a.m., Abhay Kulkarni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47571/
> -----------------------------------------------------------
>
> (Updated May 21, 2016, 12:18 a.m.)
>
>
> Review request for atlas, Madhan Neethiraj, Ramesh Mani, Shwetha GS, Suma Shivaprasad, and Hemanth Yamijala.
>
>
> Bugs: ATLAS-809
> https://issues.apache.org/jira/browse/ATLAS-809
>
>
> Repository: atlas
>
>
> Description
> -------
>
> In-Memory JAAS configurator obviates need for passing -Djava.security.auth.login.config command line argument to JVMs when starting various components that may interact with Kafka.
>
>
> Diffs
> -----
>
> common/pom.xml ba1210a
> common/src/main/java/org/apache/atlas/ApplicationProperties.java 877d7db
> common/src/main/java/org/apache/atlas/security/InMemoryJAASConfiguration.java PRE-CREATION
> common/src/test/java/org/apache/atlas/security/InMemoryJAASConfigurationTest.java PRE-CREATION
> common/src/test/resources/atlas-jaas.properties PRE-CREATION
> distro/src/conf/atlas-application.properties d1deae4
> docs/src/site/twiki/security.twiki 88e35d2
> pom.xml 98184ae
> typesystem/src/main/resources/atlas-application.properties a8e77bb
>
> Diff: https://reviews.apache.org/r/47571/diff/
>
>
> Testing
> -------
>
> Passed unit tests.
>
>
> Thanks,
>
> Abhay Kulkarni
>
>
Re: Review Request 47571: JAAS configuration needed for Kafka
interaction via Atlas config file
Posted by Abhay Kulkarni <ak...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47571/
-----------------------------------------------------------
(Updated May 21, 2016, 12:18 a.m.)
Review request for atlas, Madhan Neethiraj, Ramesh Mani, Shwetha GS, Suma Shivaprasad, and Hemanth Yamijala.
Changes
-------
Updated with review comments
Bugs: ATLAS-809
https://issues.apache.org/jira/browse/ATLAS-809
Repository: atlas
Description
-------
In-Memory JAAS configurator obviates need for passing -Djava.security.auth.login.config command line argument to JVMs when starting various components that may interact with Kafka.
Diffs (updated)
-----
common/pom.xml ba1210a
common/src/main/java/org/apache/atlas/ApplicationProperties.java 877d7db
common/src/main/java/org/apache/atlas/security/InMemoryJAASConfiguration.java PRE-CREATION
common/src/test/java/org/apache/atlas/security/InMemoryJAASConfigurationTest.java PRE-CREATION
common/src/test/resources/atlas-jaas.properties PRE-CREATION
distro/src/conf/atlas-application.properties d1deae4
docs/src/site/twiki/security.twiki 88e35d2
pom.xml 98184ae
typesystem/src/main/resources/atlas-application.properties a8e77bb
Diff: https://reviews.apache.org/r/47571/diff/
Testing
-------
Passed unit tests.
Thanks,
Abhay Kulkarni
Re: Review Request 47571: JAAS configuration needed for Kafka
interaction via Atlas config file
Posted by Abhay Kulkarni <ak...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47571/
-----------------------------------------------------------
(Updated May 19, 2016, 5:45 p.m.)
Review request for atlas, Madhan Neethiraj, Ramesh Mani, Shwetha GS, Suma Shivaprasad, and Hemanth Yamijala.
Changes
-------
Addressed review comments.
Bugs: ATLAS-809
https://issues.apache.org/jira/browse/ATLAS-809
Repository: atlas
Description
-------
In-Memory JAAS configurator obviates need for passing -Djava.security.auth.login.config command line argument to JVMs when starting various components that may interact with Kafka.
Diffs (updated)
-----
common/pom.xml 614b3f6
common/src/main/java/org/apache/atlas/ApplicationProperties.java 6a4dca3
common/src/main/java/org/apache/atlas/security/InMemoryJAASConfiguration.java PRE-CREATION
common/src/test/java/org/apache/atlas/security/InMemoryJAASConfigurationTest.java PRE-CREATION
common/src/test/resources/atlas-jaas.properties PRE-CREATION
distro/src/conf/atlas-application.properties d4722fb
pom.xml 30fb95a
typesystem/src/main/resources/atlas-application.properties a8e77bb
Diff: https://reviews.apache.org/r/47571/diff/
Testing
-------
Passed unit tests.
Thanks,
Abhay Kulkarni