You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by GitBox <gi...@apache.org> on 2021/04/27 18:29:50 UTC
[GitHub] [atlas] vladhlinsky opened a new pull request #137: ATLAS-4263: Fix KafkaUtils to always enclose property values in double-quotes
vladhlinsky opened a new pull request #137:
URL: https://github.com/apache/atlas/pull/137
## What changes were proposed in this pull request?
Fix `KafkaUtils` to always enclose property values in double-quotes. This helps to avoid the following error with Spark Atlas Connector trying to configure Atlas client to use delegation tokens:
```
java.lang.IllegalArgumentException: Value not specified for key 'null' in JAAS config
at org.apache.kafka.common.security.JaasConfig.parseAppConfigurationEntry(JaasConfig.java:116)
at org.apache.kafka.common.security.JaasConfig.<init>(JaasConfig.java:63)
at org.apache.kafka.common.security.JaasContext.load(JaasContext.java:90)
at org.apache.kafka.common.security.JaasContext.loadClientContext(JaasContext.java:84)
```
The following configuration is not handled properly:
```
atlas.jaas.KafkaClient.option.username=30CQ4q1hQMy0dB6X0eXfxQ
atlas.jaas.KafkaClient.option.password=KdaUQ4FlKWlDxwQrAeFGUVbb6sR0P+zoqOZDZjtIRP1wseXbSbhiTjz3QI9Ur9o4LTYZSv8TE1QqUC4FSwnoTA==
```
[KafkaUtils](https://github.com/apache/atlas/blob/8d3c4ab0e8844f04e29f66acb3577e9d40de9a16/common/src/main/java/org/apache/atlas/utils/KafkaUtils.java#L195) should always enclose property values in double-quotes, since unenclosed digits and `+` sign can not be parsed by Kafka [JaasConfig](https://github.com/apache/kafka/blob/2.0.0/clients/src/main/java/org/apache/kafka/common/security/JaasConfig.java#L116).
## How was this patch tested?
* Manually deploying a modified version of `atlas-common-*.jar` on a cluster
* Updated existing unit tests and added a new one
--
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] [atlas] nixonrodrigues merged pull request #137: ATLAS-4263: Fix KafkaUtils to always enclose property values in double-quotes
Posted by GitBox <gi...@apache.org>.
nixonrodrigues merged pull request #137:
URL: https://github.com/apache/atlas/pull/137
--
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] [atlas] vladhlinsky commented on pull request #137: ATLAS-4263: Fix KafkaUtils to always enclose property values in double-quotes
Posted by GitBox <gi...@apache.org>.
vladhlinsky commented on pull request #137:
URL: https://github.com/apache/atlas/pull/137#issuecomment-827828603
@jayendrap, @nixonrodrigues could you please take a look?
--
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] [atlas] vladhlinsky commented on pull request #137: ATLAS-4263: Fix KafkaUtils to always enclose property values in double-quotes
Posted by GitBox <gi...@apache.org>.
vladhlinsky commented on pull request #137:
URL: https://github.com/apache/atlas/pull/137#issuecomment-830227783
@jayendrap, @nixonrodrigues could you please take a look?
--
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] [atlas] nixonrodrigues commented on pull request #137: ATLAS-4263: Fix KafkaUtils to always enclose property values in double-quotes
Posted by GitBox <gi...@apache.org>.
nixonrodrigues commented on pull request #137:
URL: https://github.com/apache/atlas/pull/137#issuecomment-831821119
+1 for the PR. Thanks @vladhlinsky
--
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] [atlas] vladhlinsky commented on a change in pull request #137: ATLAS-4263: Fix KafkaUtils to always enclose property values in double-quotes
Posted by GitBox <gi...@apache.org>.
vladhlinsky commented on a change in pull request #137:
URL: https://github.com/apache/atlas/pull/137#discussion_r621897707
##########
File path: common/src/main/java/org/apache/atlas/utils/KafkaUtils.java
##########
@@ -318,20 +318,9 @@ static String surroundWithQuotes(String optionVal) {
return optionVal;
}
- String ret = optionVal;
-
- // For property values which have special chars like "@" or "/", we need to enclose it in
- // double quotes, so that Kafka can parse it
- // If the property is already enclosed in double quotes, then do nothing.
- if (optionVal.indexOf(0) != '"' && optionVal.indexOf(optionVal.length() - 1) != '"') {
Review comment:
Probably, the `charAt` method could be used instead of `indexOf` , but I think we should omit this check at all, since the option value may contain double quotes at the beginning/at the end or in the middle of the string.
Thus, the following option values look valid to me:
```
user1
"user2
user3"
user"4
"user5" (the actual value is "user5", not user5)
```
--
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