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

Connect to Elasticsearch via SSL when starting kibana with --ssl #42840

Merged
merged 8 commits into from
Aug 8, 2019

Conversation

mikecote
Copy link
Contributor

@mikecote mikecote commented Aug 7, 2019

In this PR, I'm making --ssl flag also connect to Elasticsearch via SSL. This will expect Elasticsearch to already be running with SSL enabled (yarn es snapshot --ssl). I'm also making the --ssl option only apply default configurations. It will throw an error if something is already configured for SSL.

@mikecote mikecote added Team:Operations Team label for Operations Team v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.4.0 labels Aug 7, 2019
@mikecote mikecote self-assigned this Aug 7, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations

@elasticmachine

This comment has been minimized.

@@ -94,6 +95,14 @@ function applyConfigOverrides(rawConfig, opts, extraCliOptions) {
set('server.ssl.certificate', DEV_SSL_CERT_PATH);
set('server.ssl.key', DEV_SSL_KEY_PATH);
}

if (opts.ssl && !opts.elasticsearch && !has('elasticsearch.hosts')) {
set('elasticsearch.hosts', 'https://localhost:9200');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't seem to be the best way to hardcode the host. Maybe there's a better way to do this? Or to findout if Elasticsearch is running on a different port?

Note the certificate within CA_CERT_PATH is bound to localhost.

Copy link
Contributor

Choose a reason for hiding this comment

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

If --elasticsearch or --elasticsearch.hosts is defined we should parse that url with url.parse(), throw if parsedUrl.hostname !== 'localhost', and if it is then use parsedUrl.port and default to 9200.

@mikecote mikecote marked this pull request as ready for review August 7, 2019 18:50
@mikecote mikecote requested review from spalger and a team August 7, 2019 18:50
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@@ -20,6 +20,7 @@
import _ from 'lodash';
import { statSync } from 'fs';
import { resolve } from 'path';
import { CA_CERT_PATH } from '@kbn/dev-utils';
Copy link
Member

@jbudz jbudz Aug 8, 2019

Choose a reason for hiding this comment

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

are we okay shipping dev certs? i know there's quite a bit in this file that makes it tough and extends beyond the scope of this PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, we should only be requiring this when opts.dev is true so that we don't have to ship @kbn/dev-utils in the distributable.

@jbudz
Copy link
Member

jbudz commented Aug 8, 2019

Out of scope of this PR for now but I think this makes a great example of committing a new config.*.yml for development. Other variations include disabled plugins for quicker refreshes and no base paths and so on.

@mikecote mikecote added the review label Aug 8, 2019
Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

LGTM

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@mikecote
Copy link
Contributor Author

mikecote commented Aug 8, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@mikecote mikecote merged commit 6d4191c into elastic:master Aug 8, 2019
mikecote added a commit to mikecote/kibana that referenced this pull request Aug 8, 2019
…lastic#42840)

* Initial work

* Add check for elasticsearch.hosts

* Make --ssl apply default config values only

* Move @kbn/dev-utils to devDependencies

* Check elasticsearch url for localhost

* Cleanup

* elasticsearch.hosts can be string too
mikecote added a commit that referenced this pull request Aug 9, 2019
…42840) (#42996)

* Initial work

* Add check for elasticsearch.hosts

* Make --ssl apply default config values only

* Move @kbn/dev-utils to devDependencies

* Check elasticsearch url for localhost

* Cleanup

* elasticsearch.hosts can be string too
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 9, 2019
…p-metrics-selectall

* 'master' of github.com:elastic/kibana: (306 commits)
  [ML] Adding job overrides to the module setup endpoint (elastic#42946)
  [APM] Fix missing RUM url (elastic#42940)
  close socket timeouts without message (elastic#42456)
  Upgrade elastic/charts to 8.1.6 (elastic#42518)
  [ML] Delete old AngularJS data visualizer and refactor folders (elastic#42962)
  Add custom formatting for Date Nanos Format (elastic#42445)
  [Vega] Shim new platform - vega_fn.js -> vega_fn.js , use ExpressionFunction (elastic#42582)
  add socket.getPeerCertificate to KibanaRequest (elastic#42929)
  [Automation] ISTANBUL PRESET PATH is not working fine with constructor(private foo) (elastic#42683)
  [ML] Data frames: Updated stats structure. (elastic#42923)
  [Code] fixed the issue that the repository can not be deleted in some cases. (elastic#42841)
  [kbn-es] Support for passing regex value to ES (elastic#42651)
  Connect to Elasticsearch via SSL when starting kibana with `--ssl` (elastic#42840)
  Add Elasticsearch SSL support for integration tests (elastic#41765)
  Fix duplicate fetch in Visualize (elastic#41204)
  [DOCS] TSVB and Timelion clean up (elastic#42953)
  [Maps] [File upload] Fix maps geojson upload hanging on index step (elastic#42623)
  [APM] Use rounded bucket sizes for transaction distribution (elastic#42830)
  [yarn.lock] consistent resolve domain (elastic#42969)
  [Uptime] [Test] Repurpose unit test assertions to avoid flakiness (elastic#40650)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes review Team:Operations Team label for Operations Team v7.4.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants