Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add apikey authentication support to quarkus-elasticsearch-rest-client #45490

Open
yujietan opened this issue Jan 10, 2025 · 8 comments · May be fixed by #45688
Open

add apikey authentication support to quarkus-elasticsearch-rest-client #45490

yujietan opened this issue Jan 10, 2025 · 8 comments · May be fixed by #45688

Comments

@yujietan
Copy link

Description

Hi right now only username/password auth is supported in the rest client. The elasticsearch cluster that i am connecting to is using apikey auth.
https://quarkus.io/guides/elasticsearch#quarkus-elasticsearch-rest-client_quarkus-elasticsearch-username

Can i request to add apikey auth support to this rest client?

Implementation ideas

No response

Copy link

quarkus-bot bot commented Jan 10, 2025

/cc @cescoffier (rest-client), @geoand (rest-client), @gsmet (elasticsearch), @loicmathieu (elasticsearch), @marko-bekhta (elasticsearch), @yrodiere (elasticsearch)

@yrodiere
Copy link
Member

yrodiere commented Jan 10, 2025

Hello,

The low-level REST client supports API key authentication by configuring it this way: https://www.elastic.co/guide/en/elasticsearch/client/java-api-client/current/_other_authentication_methods.html#_elasticsearch_api_keys

In Quarkus, you can set such configuration through a custom bean annotated with @ElasticsearchClientConfig and implementing RestClientBuilder.HttpClientConfigCallback: https://quarkus.io/guides/elasticsearch#programmatically-configuring-elasticsearch

If you want to add support for directly setting the API key through configuration, I think that's a good idea, and should not be too hard; pull requests welcome!

For anyone wanting to implement this, the implementation would boil down to:

  1. Adding the necessary configuration property (apiKey()?) in this class, below username/password:
    https://github.com/quarkusio/quarkus/blob/9df0cdb80b055658ce122b2c87289ecd083ae8aa/extensions/elasticsearch-rest-client/runtime/src/main/java/io/quarkus/elasticsearch/restclient/lowlevel/runtime/ElasticsearchConfig.java#L40-L39
  2. Adding the necessary logic here to propagate the configuration to the Elasticsearch Rest client as explained above:
    public HttpAsyncClientBuilder customizeHttpClient(HttpAsyncClientBuilder httpClientBuilder) {
    if (config.username().isPresent()) {
    if (!"https".equalsIgnoreCase(config.protocol())) {
    LOG.warn("Using Basic authentication in HTTP implies sending plain text passwords over the wire, " +
    "use the HTTPS protocol instead.");
    }
    CredentialsProvider credentialsProvider = new BasicCredentialsProvider();
    credentialsProvider.setCredentials(AuthScope.ANY,
    new UsernamePasswordCredentials(config.username().get(), config.password().orElse(null)));
    httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider);
    }

General information about how to start contributing to Quarkus is available here: https://github.com/quarkusio/quarkus/blob/main/CONTRIBUTING.md

@yrodiere yrodiere added the good first issue Good for newcomers label Jan 10, 2025
@sriram22
Copy link

I am wiling to take this up.

@yrodiere
Copy link
Member

Thank you, then we'll wait for your pull request!

@ShrutiShetty10
Copy link

Is this issue completed or actively worked upon? I see a PR raised but has review comments that needs to be worked upon. I can work on it if no one else is working on it.

@yrodiere
Copy link
Member

Is this issue completed or actively worked upon? I see a PR raised but has review comments that needs to be worked upon. I can work on it if no one else is working on it.

@marko-bekhta did the review, he'll be able to tell you what the status is exactly and how to proceed.

@sriram22
Copy link

i think the last review comment was deprecating existing username/password properties and creating new alternatives for them too and @marko-bekhta wanted to check with you (@yrodiere ) if its worth doing. so was waiting on a response on that

@yrodiere
Copy link
Member

yrodiere commented Mar 24, 2025

Sorry, I missed that. I just answered; IMO no further change is necessary. So that leaves only the other review comments, and I'll let @marko-bekhta see to those :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants