Skip to content

Conversation

@jloleysens
Copy link
Contributor

@jloleysens jloleysens commented Jan 31, 2020

Summary

Fix #56450

Tested locally and 'none' is being passed through to elasticsearch_proxy_confg.

Setup to reproduce on master

  • Set elasticsearch.hosts: ['https://localhost:9200'] and elasticsearch.ssl.verificationMode: 'none' in your kibana.dev.yml
  • yarn es snapshot --ssl
  • yarn start --ssl

@jloleysens jloleysens added Feature:Console Dev Tools Console Feature Feature:Dev Tools v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Jan 31, 2020
@jloleysens jloleysens requested a review from rudolf January 31, 2020 08:00
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui (Team:Elasticsearch UI)

@sebelga sebelga self-requested a review January 31, 2020 10:43
Copy link
Contributor

@sebelga sebelga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM. Although I could not test locally as I couldn't reproduce the issue. If you want me to test it locally, can you provide the steps?

@jloleysens
Copy link
Contributor Author

Thanks for the review @sebelga !

From the original issue:

elasticsearch.ssl.verificationMode: 'none' configured in mykibana.dev.yml

I'll add the steps in the description in future!

@cjcenizal cjcenizal self-requested a review January 31, 2020 14:48
@jportner
Copy link
Contributor

@jloleysens I ran into this problem and spent some time trying to track down what was happening.
I'm glad to see there's already a PR to fix it.

I noticed the bug was introduced in #55690.
Can I suggest taking a look at elasticsearch_proxy_config.ts as well?
The arguments in the createAgent and getElasticsearchProxyConfig functions do not have the correct type definitions, those should be changed so the type checker will catch problems like this.

@jloleysens
Copy link
Contributor Author

@jportner Good point, regarding the typings downstream.

@jloleysens
Copy link
Contributor Author

Unfortunately the upstream object still comes from an any so the type guards won't help in all cases, will add them though!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@cjcenizal
Copy link
Contributor

@jloleysens I tried to first reproduce the reported problem but I wasn't able to:

  1. I set elasticsearch.ssl.verificationMode: 'none' in my kibana.dev.yml.
  2. I refreshed Kibana and navigated to Dev Tools.
  3. I created an index, a document, and searched it all with no problem (requests below).

What am I doing wrong?

PUT test6

PUT test6/_doc/1
{
  "foo": "a"
}

GET test6/_search

@jloleysens
Copy link
Contributor Author

jloleysens commented Feb 1, 2020

@cjcenizal assuming you are testing before the fix :), you can add a log statement in logging the value of

const verificationMode = legacyConfig.ssl && legacyConfig.ssl.verificationMode;
to see the value of the config coming through. If it’s master it should have ssl but verificationMode is undefined.

[UPDATE]: I have updated the PR description

@jloleysens
Copy link
Contributor Author

That should be throwing the error in the default case per request.

Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jloleysens! Code LGTM, tested locally and verified the verificationMode is being passed through as expected.

@jloleysens jloleysens merged commit 27a4fe2 into elastic:master Feb 1, 2020
@jloleysens jloleysens deleted the fix/console/legacy-config-ssl branch February 1, 2020 15:38
jloleysens added a commit to jloleysens/kibana that referenced this pull request Feb 1, 2020
* Fix use of legacy config

* Add types for ssl
jloleysens added a commit that referenced this pull request Feb 1, 2020
* Fix use of legacy config

* Add types for ssl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Console Dev Tools Console Feature Feature:Dev Tools release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v7.7.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Kibana Dev Console queries returning Internal Server Error

6 participants