In ElasticSearchSetup class I see this:
for (String host : config.get(INDEX_HOSTS)) {
String[] hostparts = host.split(":");
String hostname = hostparts[0];
int hostport = defaultPort;
if (hostparts.length == 2) hostport = Integer.parseInt(hostparts[1]);
log.debug("Configured remote host: {} : {}", hostname, hostport);
hosts.add(new HttpHost(hostname, hostport, "http"));
}
I think we should have an option for HTTPS as well.
Included in #612
Well, while I do agree that it would be included in #612, the fix for this one would be literally 2 or 3 lines (just support for HTTPS). I have actually tested it and it worked fine with AWS ES 5.5. managed service. And #612 requires some kind of authentication, which is another problem and it is outside of the scope of HttpHost. Unless a PR is coming for #612, I would probably recommend addressing these problems separately.
Adding tests might be the bulk of the work here (and in #612). If it were me I'd do both SSL and authentication together but we can reopen this in case you or anyone want to try and add tests and implementation to support HTTPS separately.
I like @sjudeng idea to put both in the same PR
I would be very happy to provide first the PR for this one (we need it like now :) ) and then for #612.
@ngrigoriev — in parallel with developing your patch, please also sign the CLA and configure your git client accordingly. You may need to have your company sign a CCLA rather than signing an ICLA; please check with your legal department. Looking forward to your contributions!
How you plan to test your feature without using X-pack/shield ?
I did not explore yet the Shield part. I think am done with just plain HTTPS support. I tested it against AWS ES service, which only allows HTTPS. As for Shield, I was thinking about an HTTPS proxy with Basic authentication. It seems that's what X-Pack/Shield is using.
Update: actually, I never used x-pack but it seems to be simple to install :) So I can test it for real.
Will deal with CLA this morning.
I have just realized that I need one more thing to have in this PR :) AWS request authentication. I mean - in addition to HTTP Basic authentication needed for Shield/X-Pack I also need AWS authentication to be able to sign the requests for AWS ES service. This goes beyond the scope of both this issue and #612. What would you propose? Extend #612 or open another issue and link all together? I am developing the change that would support both auth methods (and others if needed).
I think we would want to avoid adding custom code (and the AWS SDK dependency) to support AWS-specific auth directly in janusgraph-es. My concern is it would be difficult to test and maintain the AWS client implementation. What would make sense to me though would be if we wanted to look at refactoring to expose higher-level abstractions to allow users to accomplish this on their end. For example maybe RestElasticSearchClient could be updated to support a plugable authentication mechanism or support could be added for custom user ElasticSearchClient implementations. Then the AWS example could be provided in documentation and/or janusgraph-examples.
What do others think?
The implementation turns out to be, in fact, about 5 lines of code. But it does require a dependency on aws-java-sdk-core - to implement the V4 signing. The required hook is already available in the RestClientBuilder. So it does not really require any modifications/extensions of ElasticSearchClient. However, dependency is a dependency....
Technically this signer requires no parameters. So, theoretically, it can be done in a way similar to how DynamoDB support is done for storage:
storage.dynamodb.client.credentials.class-name=com.amazonaws.auth.BasicAWSCredentials
storage.dynamodb.client.credentials.constructor-args=fake,fake
So, it could be like a custom authenticator implemented as HttpRequestInterceptor wrapped into some something. So it would get just "constructor args".
Maybe something like this:
public interface ElasticsearchRestAuthenticator {
public HttpRequestInterceptor getHttpRequestInterceptor();
}
index.<index-name>.http.auth_type=custom
index.<index-name>.http.auth_class=com.blah-blah.AWSAuthenticator
index.<index-name>.http.auth_custom_params=us-east-1
So this would keep out the AWS dependency but still support the use case (which hopefully you can document and/or add an example for)? This seems good to me. Would need some stub implementation to support unit testing. Also maybe the configuration should go under elasticsearch namespace (e.g. index.<index-name>.elasticsearch.http.auth_type etc.).
Thanks again for your work on this!
Yes, this would keep AWS dependency out, would allow (potentially) to develop something else similar to AWS auth. And it is not exactly cast in stone. If more external customization is ever needed, additional interfaces can be added to that "auth class" and discovered at runtime.
Oh, yes, absolutely, under "elasticsearch", this is ES-specific - I did not mean that config example to be exactly like this.
Ok, agreed, will go this route with an externalized/configurable interceptor and a separate mini-project implementing that thing for AWS.
At the same time, the HTTP Basic Auth - I would consider it a part of this module. I did that too, will test it tomorrow with x-pack (got it running).
The custom AWS "authenticator" would look like https://github.com/ngrigoriev/janusgraph-es-aws-authenticator.
I have just tested it with JG and it worked against AWS ES service with IAM-based authorization. It worked.
Most helpful comment
I would be very happy to provide first the PR for this one (we need it like now :) ) and then for #612.