Update Cloud plugin to handle new config in kibana.yml by ryankeairns · Pull Request #95569 · elastic/kibana

ryankeairns · 2021-03-26T18:31:46Z

Closes #93436
Relates to https://github.com/elastic/cloud/pull/77420 https://github.com/elastic/cloud/pull/77961

Caveat

⚠️ Since the original work here originated with Core UI, I felt obliged to move this along. It's in a functioning and passing state, but will likely need some engineering help to get over the line. In particular, there may be a need for additional tests here, but I'm not versed in writing them.

Summary

The Cloud plugin was designed to support the below config values in anticipation of them being provided by ESS/ECE. Now that the Cloud team has completed their work (PRs above), we need to adjust the plugin to handle the final names + additional values.

Old config

# Enable cloud plugin
xpack.cloud.id: 'eastus2.azure.elastic-cloud.com:9243$59ef636c6917463db140321484d63cfa$a8b109c08adc43279ef48f29af1a3911'
xpack.cloud.resetPasswordUrl: 'https://cloud.elastic.co/user/settings' # Cloud profile link
xpack.cloud.accountUrl: 'https://cloud.elastic.co/account/activity' # Account link
xpack.cloud.deploymentUrl: 'https://cloud.elastic.co/deployments/resolve/cluster/eastus2/9243$59ef636c6917463db140321484d63cfa/'

New config

# USER OVERRIDES
# other properties
xpack.cloud.id: 'eastus2.azure.elastic-cloud.com:9243$59ef636c6917463db140321484d63cfa$a8b109c08adc43279ef48f29af1a3911'
xpack.cloud.profile_url: "/user/settings/"
xpack.cloud.organization_url: "/account/"
xpack.cloud.cname: "cloud.elastic.co"
xpack.cloud.base_url: "https://cloud.elastic.co"
xpack.cloud.deployment_url: "/deployments/bfdad4ef99a24212a06d387593686d63"
# END USER OVERRIDES

Items of note

With the base url split apart, this PR does some concatenating to arrive at the full URLs
While the config values are snake_case, I've attempted to make the plugin return camelCase variables. Some plugins were already relying on a camelCase `deploymentUrl' and this format seems like the preferred convention for Kibana.
The examples show cloud.elastic.co as the cname. This value will change for ECE installations where the value is user-defined.
A few new, previously unanticipated yet welcome, values are now being passed. I've gone ahead and opened those up (i.e. have the Cloud plugin provide them) so as not to require additional, follow-on PRs to expose them.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)

For maintainers

This was checked for breaking API changes and was labeled appropriately

ryankeairns · 2021-03-27T19:16:46Z

@elasticmachine merge upstream

ryankeairns · 2021-03-29T21:02:28Z

@elasticmachine merge upstream

cee-chen

Enterprise Search changes LGTM - thanks for grabbing that!

yuliacech

Thanks a lot for working on this, @ryankeairns! ES UI changes LGTM 👍

legrego

Security changes LGTM

pgayvallet

Technically it's LGTM.

Now the real question is: which version is https://github.com/elastic/cloud/pull/77420 going to impact? Will cloud correctly deploy/use this new format only on Kibana 7.13 and higher?

x-pack/plugins/cloud/public/utils.ts

x-pack/plugins/cloud/public/plugin.ts

x-pack/plugins/cloud/public/user_menu_links.ts

ryankeairns · 2021-03-30T21:43:34Z

Technically it's LGTM.

Now the real question is: which version is elastic/cloud#77420 going to impact? Will cloud correctly deploy/use this new format only on Kibana 7.13 and higher?

I posed the same question and it is my understanding that, yes, it would target 7.13 and higher. I believe this to part of the reason that @yspotts reverted his PR. Anyway, one final confirmation would be good.

Also, thanks for the review.

yspotts · 2021-03-30T21:46:09Z

Technically it's LGTM.
Now the real question is: which version is elastic/cloud#77420 going to impact? Will cloud correctly deploy/use this new format only on Kibana 7.13 and higher?

I posed the same question and it is my understanding that, yes, it would target 7.13 and higher. I believe this to part of the reason that @yspotts reverted his PR. Anyway, one final confirmation would be good.

Also, thanks for the review.

yes, my (reverted) PR targets 7.13 and higher only. Once this is merged, I will merge the cloud side.

kibanamachine · 2021-03-31T00:43:16Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`cloud`	7	8	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`enterpriseSearch`	2.0MB	2.0MB	-5.0B
`indexLifecycleManagement`	240.1KB	240.1KB	-5.0B
total			-10.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`cloud`	4.5KB	4.9KB	+435.0B
`security`	122.9KB	122.8KB	-102.0B
total			+333.0B

History

💚 Build #115946 succeeded 804857d
💔 Build #115915 failed b00a553
💔 Build #115662 failed 71ab58e
💔 Build #115597 failed 8ebfa8e
💔 Build #115581 failed 57e97ac

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

yspotts · 2021-03-31T12:51:39Z

@ryankeairns now that this is merged, I guess we are ready re-merge my PR on the cloud side? Maybe we should wait a day until kibana builds are created to accept these new properties?

* Handle cloud urls from kibana.yml * Add types to utils params * Update utils * address nits Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* Handle cloud urls from kibana.yml * Add types to utils params * Update utils * address nits Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

ryankeairns · 2021-04-13T21:26:47Z

@ryankeairns now that this is merged, I guess we are ready re-merge my PR on the cloud side? Maybe we should wait a day until kibana builds are created to accept these new properties?

@yspotts Yes, we are good to go (I was out on vacation recently). Have you re-merged your PR?

cc:/ @alexfrancoeur

yspotts · 2021-04-13T22:36:31Z

@ryankeairns now that this is merged, I guess we are ready re-merge my PR on the cloud side? Maybe we should wait a day until kibana builds are created to accept these new properties?

@yspotts Yes, we are good to go (I was out on vacation recently). Have you re-merged your PR?

cc:/ @alexfrancoeur

thanks! Just merged today

ryankeairns added 2 commits March 26, 2021 13:27

Handle cloud urls from kibana.yml

57e97ac

Add types to utils params

8ebfa8e

kibanamachine and others added 2 commits March 27, 2021 15:16

Merge branch 'master' into rk/93436-cloud-urls

71ab58e

Update utils

b00a553

Merge branch 'master' into rk/93436-cloud-urls

804857d

ryankeairns marked this pull request as ready for review March 30, 2021 13:59

ryankeairns requested a review from a team March 30, 2021 13:59

ryankeairns requested review from a team as code owners March 30, 2021 13:59

ryankeairns added v7.13.0 v8.0.0 release_note:feature Makes this part of the condensed release notes labels Mar 30, 2021

cee-chen approved these changes Mar 30, 2021

View reviewed changes

yuliacech approved these changes Mar 30, 2021

View reviewed changes

legrego approved these changes Mar 30, 2021

View reviewed changes

pgayvallet reviewed Mar 30, 2021

View reviewed changes

pgayvallet approved these changes Mar 30, 2021

View reviewed changes

address nits

fd53d2d

ryankeairns merged commit b301d41 into elastic:master Mar 31, 2021

ryankeairns mentioned this pull request Mar 31, 2021

[7.x] Update Cloud plugin to handle new config in kibana.yml (#95569) #95921

Merged

yuliacech mentioned this pull request Apr 22, 2021

[Cloud plugin] Added README text #97999

Merged

KOTungseth added the Feature:Home Kibana home application label May 12, 2021

This was referenced Aug 20, 2021

Fix link to Cloud deployment URL in upgrade step. #109528

Merged

Add more details to and reformat the Cloud plugin README. #109529

Merged

matschaffer mentioned this pull request Dec 9, 2021

Kibana Stack Monitoring links should point at local ECE instead of Elastic Cloud #118592

Open

Conversation

ryankeairns commented Mar 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Caveat

Summary

Old config

New config

Items of note

Checklist

For maintainers

Uh oh!

ryankeairns commented Mar 27, 2021

Uh oh!

ryankeairns commented Mar 29, 2021

Uh oh!

cee-chen left a comment

Choose a reason for hiding this comment

Uh oh!

yuliacech left a comment

Choose a reason for hiding this comment

Uh oh!

legrego left a comment

Choose a reason for hiding this comment

Uh oh!

pgayvallet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ryankeairns commented Mar 30, 2021

Uh oh!

yspotts commented Mar 30, 2021

Uh oh!

kibanamachine commented Mar 31, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

Uh oh!

yspotts commented Mar 31, 2021

Uh oh!

ryankeairns commented Apr 13, 2021

Uh oh!

yspotts commented Apr 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ryankeairns commented Mar 26, 2021 •

edited

Loading