[GitHub] metron pull request #946: METRON-1465:Support for Elasticsearch X-pack

[simonellistonball]

Github user simonellistonball commented on a diff in the pull request:

https://github.com/apache/metron/pull/946#discussion_r173416554
  
--- Diff: 
metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/scripts/metron_service.py
 ---
@@ -70,6 +70,11 @@ def build_global_config_patch(params, patch_file):
 "path": "/es.date.format",
 "value": "{{es_date_format}}"
 },
+{
+"op": "add",
+"path": "/es.xpack.user",
+"value": "{{es_xpack_user}}"
--- End diff --

could we lean on the hadoop credentials apis? 


---

[GitHub] metron pull request #946: METRON-1465:Support for Elasticsearch X-pack

[wardbekker]

Github user wardbekker commented on a diff in the pull request:

https://github.com/apache/metron/pull/946#discussion_r173398330
  
--- Diff: 
metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/scripts/metron_service.py
 ---
@@ -70,6 +70,11 @@ def build_global_config_patch(params, patch_file):
 "path": "/es.date.format",
 "value": "{{es_date_format}}"
 },
+{
+"op": "add",
+"path": "/es.xpack.user",
+"value": "{{es_xpack_user}}"
--- End diff --

Good point. Is there an existing encryption mechanism that we can use to 
prevent plain text passwords here? 


---

[GitHub] metron pull request #946: METRON-1465:Support for Elasticsearch X-pack

[wardbekker]

Github user wardbekker commented on a diff in the pull request:

https://github.com/apache/metron/pull/946#discussion_r173397551
  
--- Diff: pom.xml ---
@@ -97,7 +97,7 @@
 
${base_hadoop_version}
 ${base_hbase_version}
 ${base_flume_version}
-5.6.2
+5.6.7
--- End diff --

There was no x-pack transport client build for 5.6.2


---

[GitHub] metron pull request #946: METRON-1465:Support for Elasticsearch X-pack

[mmiklavc]

Github user mmiklavc commented on a diff in the pull request:

https://github.com/apache/metron/pull/946#discussion_r172544606
  
--- Diff: 
metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/scripts/metron_service.py
 ---
@@ -70,6 +70,11 @@ def build_global_config_patch(params, patch_file):
 "path": "/es.date.format",
 "value": "{{es_date_format}}"
 },
+{
+"op": "add",
+"path": "/es.xpack.user",
+"value": "{{es_xpack_user}}"
--- End diff --

We can't store passwords like this in plain text because they'll end up in 
Zookeeper as plaintext without permissions locking it down.


---

[GitHub] metron pull request #946: METRON-1465:Support for Elasticsearch X-pack

[mmiklavc]

Github user mmiklavc commented on a diff in the pull request:

https://github.com/apache/metron/pull/946#discussion_r172351786
  
--- Diff: pom.xml ---
@@ -97,7 +97,7 @@
 
${base_hadoop_version}
 ${base_hbase_version}
 ${base_flume_version}
-5.6.2
+5.6.7
--- End diff --

Just noticed this - what's the motivation for changing ES versions as part 
of this PR?


---

[GitHub] metron pull request #946: METRON-1465:Support for Elasticsearch X-pack

[cestella]

Github user cestella commented on a diff in the pull request:

https://github.com/apache/metron/pull/946#discussion_r171624949
  
--- Diff: metron-platform/elasticsearch-shaded/pom.xml ---
@@ -31,7 +43,7 @@
 
 
 org.elasticsearch.client
-transport
+x-pack-transport
--- End diff --

Yep, I concede the point, that is not appropriately licensed via apache and 
we cannot bundle that dependency.


---

[GitHub] metron pull request #946: METRON-1465:Support for Elasticsearch X-pack

[mmiklavc]

Github user mmiklavc commented on a diff in the pull request:

https://github.com/apache/metron/pull/946#discussion_r171620565
  
--- Diff: metron-platform/elasticsearch-shaded/pom.xml ---
@@ -31,7 +43,7 @@
 
 
 org.elasticsearch.client
-transport
+x-pack-transport
--- End diff --

That project does not appear to contain `PreBuiltXPackTransportClient`. ES 
points users to this - 
https://www.elastic.co/guide/en/elasticsearch/reference/6.2/setup-xpack-client.html
 - which instructs users to add the ES Maven repo. The referenced jar's license 
file does not appear to dual license as Apache V2. 
https://artifacts.elastic.co/maven/org/elasticsearch/client/x-pack-transport/6.2.2/x-pack-transport-6.2.2.jar


---

[GitHub] metron pull request #946: METRON-1465:Support for Elasticsearch X-pack

[justinleet]

Github user justinleet commented on a diff in the pull request:

https://github.com/apache/metron/pull/946#discussion_r171619920
  
--- Diff: metron-platform/elasticsearch-shaded/pom.xml ---
@@ -31,7 +43,7 @@
 
 
 org.elasticsearch.client
-transport
+x-pack-transport
--- End diff --

The main Elastic stuff is (e.g. the plain transport client if you pull it 
down has Apache license), but the x-pack stuff doesn't seem to. The 
elastic/elasticsearch repo (from a quick search) does not contain the x-pack 
code from what I can tell.

Elastic's blog post from the other day also seems to imply it's not in 
there (and not ASLv2), but I could be reading too far into it. Check out 
https://www.elastic.co/products/x-pack/open

>We are creating a new X-Pack folder in each of these repositories that 
will be licensed under the Elastic EULA, which allows for some derivative works 
and contribution.




---

[GitHub] metron pull request #946: METRON-1465:Support for Elasticsearch X-pack

[cestella]

Github user cestella commented on a diff in the pull request:

https://github.com/apache/metron/pull/946#discussion_r171618015
  
--- Diff: metron-platform/elasticsearch-shaded/pom.xml ---
@@ -31,7 +43,7 @@
 
 
 org.elasticsearch.client
-transport
+x-pack-transport
--- End diff --

I believe this is dual licensed ES commercial and ASLv2.  IT comes from 
[here](https://github.com/elastic/elasticsearch#license).


---

[GitHub] metron pull request #946: METRON-1465:Support for Elasticsearch X-pack

[justinleet]

Github user justinleet commented on a diff in the pull request:

https://github.com/apache/metron/pull/946#discussion_r171617250
  
--- Diff: metron-platform/elasticsearch-shaded/pom.xml ---
@@ -31,7 +43,7 @@
 
 
 org.elasticsearch.client
-transport
+x-pack-transport
--- End diff --

For posterity, I pulled the jar with
```
{11:23}~ ➭ mvn org.apache.maven.plugins:maven-dependency-plugin:3.0.2:get 
\
-DremoteRepositories=https://artifacts.elastic.co/maven \
-Dartifact=org.elasticsearch.client:x-pack-api:5.6.7
```
and just opened up

`~/.m2/repository/org/elasticsearch/client/x-pack-transport/5.6.7/x-pack-transport-5.6.7.pom`

If someone knows more about how the ES licensing for this stuff works, let 
me know, because it' s definitely possible there's something I don't know about.


---

[GitHub] metron pull request #946: METRON-1465:Support for Elasticsearch X-pack

[justinleet]

Github user justinleet commented on a diff in the pull request:

https://github.com/apache/metron/pull/946#discussion_r171614360
  
--- Diff: metron-platform/elasticsearch-shaded/pom.xml ---
@@ -31,7 +43,7 @@
 
 
 org.elasticsearch.client
-transport
+x-pack-transport
--- End diff --

Unlike the plain transport client, this jar is NOT Apache licensed.  I 
pulled it down and it has this
```
  x-pack-transport
  5.6.7
  ...
  

  Elastic Commercial Software End User License Agreement
  https://www.elastic.co/eula/
  repo

  
```


---

[GitHub] metron pull request #946: METRON-1465:Support for Elasticsearch X-pack

[wardbekker]

Github user wardbekker commented on a diff in the pull request:

https://github.com/apache/metron/pull/946#discussion_r171599970
  
--- Diff: 
metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/utils/ElasticsearchUtils.java
 ---
@@ -111,19 +111,24 @@ public static TransportClient getClient(Map globalConfiguration,
 Settings.Builder settingsBuilder = Settings.builder();
 settingsBuilder.put("cluster.name", 
globalConfiguration.get("es.clustername"));
 settingsBuilder.put("client.transport.ping_timeout","500s");
+settingsBuilder.put("transport.type", "security4");
+Object xPackUser = globalConfiguration.get("es.xpackuser");
+if (xPackUser != null) {
+  settingsBuilder.put("xpack.security.user", xPackUser);
+}
 if (optionalSettings != null) {
   settingsBuilder.put(optionalSettings);
 }
 Settings settings = settingsBuilder.build();
-TransportClient client;
+PreBuiltXPackTransportClient client;
--- End diff --

yep. it might make sense to also check for a empty string for the 
"es.xpackuser" config, or does globalConfiguration.get return null also for 
empty strings?


---

[GitHub] metron pull request #946: METRON-1465:Support for Elasticsearch X-pack

[mmiklavc]

Github user mmiklavc commented on a diff in the pull request:

https://github.com/apache/metron/pull/946#discussion_r171596212
  
--- Diff: 
metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/utils/ElasticsearchUtils.java
 ---
@@ -111,19 +111,24 @@ public static TransportClient getClient(Map globalConfiguration,
 Settings.Builder settingsBuilder = Settings.builder();
 settingsBuilder.put("cluster.name", 
globalConfiguration.get("es.clustername"));
 settingsBuilder.put("client.transport.ping_timeout","500s");
+settingsBuilder.put("transport.type", "security4");
+Object xPackUser = globalConfiguration.get("es.xpackuser");
+if (xPackUser != null) {
+  settingsBuilder.put("xpack.security.user", xPackUser);
+}
 if (optionalSettings != null) {
   settingsBuilder.put(optionalSettings);
 }
 Settings settings = settingsBuilder.build();
-TransportClient client;
+PreBuiltXPackTransportClient client;
--- End diff --

First question I have is will this work with and without XPack enabled?


---

[GitHub] metron pull request #946: METRON-1465:Support for Elasticsearch X-pack

[wardbekker]

GitHub user wardbekker opened a pull request:

https://github.com/apache/metron/pull/946

METRON-1465:Support for Elasticsearch X-pack

## Contributor Comments
[Please place any comments here.  A description of the problem/enhancement, 
how to reproduce the issue, your testing methodology, etc.]


## Pull Request Checklist

Thank you for submitting a contribution to Apache Metron.  
Please refer to our [Development 
Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235)
 for the complete guide to follow for contributions.  
Please refer also to our [Build Verification 
Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview)
 for complete smoke testing guides.  


In order to streamline the review of the contribution we ask you follow 
these guidelines and ask you to double check the following:

### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? If not one needs to 
be created at [Metron 
Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
- [ ] Does your PR title start with METRON- where  is the JIRA 
number you are trying to resolve? Pay particular attention to the hyphen "-" 
character.
- [ ] Has your PR been rebased against the latest commit within the target 
branch (typically master)?


### For code changes:
- [ ] Have you included steps to reproduce the behavior or problem that is 
being changed or addressed?
- [ ] Have you included steps or a guide to how the change may be verified 
and tested manually?
- [ ] Have you ensured that the full suite of tests and checks have been 
executed in the root metron folder via:
  ```
  mvn -q clean integration-test install && 
dev-utilities/build-utils/verify_licenses.sh 
  ```

- [ ] Have you written or updated unit tests and or integration tests to 
verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] Have you verified the basic functionality of the build by building 
and running locally with Vagrant full-dev environment or the equivalent?

### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in 
which it is rendered by building and verifying the site-book? If not then run 
the following commands and the verify changes via 
`site-book/target/site/index.html`:

  ```
  cd site-book
  mvn site
  ```

 Note:
Please ensure that once the PR is submitted, you check travis-ci for build 
issues and submit an update to your PR as soon as possible.
It is also recommended that [travis-ci](https://travis-ci.org) is set up 
for your personal repository such that your branches are built there before 
submitting a pull request.


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

$ git pull https://github.com/wardbekker/metron xpack_support

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

https://github.com/apache/metron/pull/946.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 #946


commit 9978ba7151fb155fbf000af17650ca2a9784d258
Author: Ward Bekker 
Date:   2018-02-26T14:21:59Z

Squashed commit of xpack commits




---