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

[Monitoring] Fix issue with singular host config #67575

Closed
wants to merge 2 commits into from

Conversation

chrisronline
Copy link
Contributor

@chrisronline chrisronline commented May 28, 2020

Fixes #67373
Fixes #67668

The setup is something like this in your config:

monitoring.ui.elasticsearch.hosts: https://localhost:8200

If an array structure is not used, this code is not executed meaning the connection will use the default localhost:9200.

This seems like a bug with NP, but it also sounds like something we went through in the NP upgrade but I can't recall what came of that.

cc @restrry

@chrisronline chrisronline requested a review from a team May 28, 2020 02:27
@chrisronline chrisronline self-assigned this May 28, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/stack-monitoring (Team:Monitoring)

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@mshustov
Copy link
Contributor

This seems like a bug with NP,

I don't think it's a problem with KP. createClient explicitly requires string[] as a valid interface. It has been discussed several times. 1 2
I'd suggest not to break type checking logic with any here

Since ElasticsearchConfig is not used in the monitoring plugin, createClient could have problems with ssl property initialized in the config class.

@chrisronline
Copy link
Contributor Author

I synced with @restrry offline and we're going to wait until #67357 is in and update this PR to leverage the existing elasticsearch config class instead of defining anything custom.

@chrisronline
Copy link
Contributor Author

Closing in favor of #68389

@chrisronline chrisronline deleted the monitoring/hosts branch June 5, 2020 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Monitoring] Share Elasticsearch config Monitoring plugin is not respecting the declared config interface
4 participants