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

Elasticsearch: Support REST URIs including userInfo #21291

Closed
murdos opened this issue May 1, 2020 · 4 comments
Closed

Elasticsearch: Support REST URIs including userInfo #21291

murdos opened this issue May 1, 2020 · 4 comments
Labels
status: superseded An issue that has been superseded by another

Comments

@murdos
Copy link

murdos commented May 1, 2020

Setting the spring.elasticsearch.rest.uris with value http://login:password@host:port generates UnknownHostException: login:password@host: Name or service not known

It would be nice if the userInfo part of the URL could also be used as credentials (that would be overridden if spring.elasticsearch.rest.username or spring.elasticsearch.rest.password are defined)

FYI I encountered this issue while trying to use Bonsai with Heroku that sets a variable BONSAI_URL including login and token.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 1, 2020
@philwebb
Copy link
Member

philwebb commented May 1, 2020

It looks like this should be possible, although it's a little involved since we support more than one URL in ElasticsearchRestClientProperties.

I think to implement it we'd need to change ElasticsearchRestClientConfigurations. I think we'd be able to parse the URL ourselves using java.net.URI to access the userInfo. The DefaultRestClientBuilderCustomizer would need to change to apply a custom CredentialsProvider that would be able to lookup the userinfo for a given host.

@philwebb philwebb added type: enhancement A general enhancement status: ideal-for-contribution An issue that a contributor can help us with and removed status: waiting-for-triage An issue we've not yet triaged labels May 1, 2020
@philwebb philwebb added this to the General Backlog milestone May 1, 2020
@evgeniycheban
Copy link
Contributor

Can I work on this issue?

evgeniycheban added a commit to evgeniycheban/spring-boot that referenced this issue May 11, 2020
evgeniycheban added a commit to evgeniycheban/spring-boot that referenced this issue May 11, 2020
evgeniycheban added a commit to evgeniycheban/spring-boot that referenced this issue May 12, 2020
evgeniycheban added a commit to evgeniycheban/spring-boot that referenced this issue May 12, 2020
evgeniycheban added a commit to evgeniycheban/spring-boot that referenced this issue May 12, 2020
@wilkinsona
Copy link
Member

While reviewing the PR from @evgeniycheban I've given some thought to the following:

It would be nice if the userInfo part of the URL could also be used as credentials (that would be overridden if spring.elasticsearch.rest.username or spring.elasticsearch.rest.password are defined)

I don't think the credentials from the URL should be overridden by the properties. The properties should only be used for URLs with no user info. This will align things with, for example, Boot's support for the addresses used to connect to RabbitMQ.

evgeniycheban added a commit to evgeniycheban/spring-boot that referenced this issue May 13, 2020
@philwebb
Copy link
Member

philwebb commented Jun 7, 2020

Closing in favor of PR #21381

@philwebb philwebb closed this as completed Jun 7, 2020
@philwebb philwebb added status: superseded An issue that has been superseded by another and removed status: ideal-for-contribution An issue that a contributor can help us with type: enhancement A general enhancement labels Jun 7, 2020
@snicoll snicoll removed this from the General Backlog milestone Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants