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

Expose decoded cloudId components from the cloud plugin's contract #159442

Merged
merged 18 commits into from Jun 13, 2023

Conversation

pgayvallet
Copy link
Contributor

@pgayvallet pgayvallet commented Jun 12, 2023

Summary

Fix #138813

  • decode the cloudId and expose the id components from the cloud plugin's browser and server-side contracts
  • remove the existing decodeCloudId helpers from the other plugins
  • adapt the usages accordingly

@pgayvallet pgayvallet added v8.9.0 Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc release_note:skip Skip the PR/issue when compiling release notes labels Jun 12, 2023
Copy link
Contributor Author

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

self-review

kibanaUrl: string;
}
| undefined {
export function decodeCloudId(cid: string): DecodedCloudId | undefined {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved, pretty much without any modifications, to the cloud plugin. Tell me if you think anything within the method should be changed.

Copy link
Member

Choose a reason for hiding this comment

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

Can't add a comment in that line. Should we use a different logger to replace all console.debug entries?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh right, we have the client-side loggers now! will do.

Comment on lines +10 to +14
export interface CloudStart {
/**
* A React component that provides a pre-wired `React.Context` which connects components to Cloud services.
*/
CloudContextProvider: FC<{}>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Took this opportunity to move the browser-side contracts to a dedicated file, given they started to become quite big.

Comment on lines +78 to +93
/**
* The full URL to the elasticsearch cluster.
*/
elasticsearchUrl?: string;
/**
* The full URL to the Kibana deployment.
*/
kibanaUrl?: string;
/**
* {host} from the deployment url https://<deploymentId>.<application>.<host><?:port>
*/
cloudHost?: string;
/**
* {port} from the deployment url https://<deploymentId>.<application>.<host><?:port>
*/
cloudDefaultPort?: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Naming is hard. I went with those, but I'm open to better suggestions (or better TSDoc)

return {
cloudId: id,
deploymentId: parseDeploymentIdFromDeploymentUrl(this.config.deployment_url),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the deploymentId (which was only exposed on the server-side contract) while I was at it, given some plugins recoded their helper to access it.

Copy link
Contributor

@juliaElastic juliaElastic left a comment

Choose a reason for hiding this comment

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

Fleet LGTM

@pgayvallet pgayvallet marked this pull request as ready for review June 12, 2023 12:40
@pgayvallet pgayvallet requested review from a team as code owners June 12, 2023 12:40
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@pgayvallet
Copy link
Contributor Author

Moving into review as the current failures are unrelated to the PR's changes

Copy link
Contributor

@shahzad31 shahzad31 left a comment

Choose a reason for hiding this comment

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

Synthetics changes LGTM !!

@botelastic botelastic bot added Team:Fleet Team label for Observability Data Collection Fleet team Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability labels Jun 12, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

Copy link
Contributor

@ogupte ogupte left a comment

Choose a reason for hiding this comment

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

Observability onboarding changes LGTM. Thank you for this, it's so much better!

Comment on lines +40 to +42
* The full URL to the Kibana deployment.
*/
kibanaUrl?: string;
Copy link
Member

Choose a reason for hiding this comment

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

Just out of curiosity, is there a difference between this and server.publicBaseUrl? (Other than the fact that this will only be available in a cloud environment?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is. server.publicBaseUrl is an arbitrary URL eventually set by the user, that is supposed to reflect the "public" way of accessing Kibana.

The url decrypted from the cloudId is

  1. only available on cloud (obviously)
  2. reflecting the "internal" way of accessing the deployment.

@pgayvallet pgayvallet enabled auto-merge (squash) June 13, 2023 06:50
@pgayvallet pgayvallet merged commit 2b42341 into elastic:main Jun 13, 2023
20 checks passed
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Fleet Cypress Tests / View agents list Bulk actions should allow to bulk upgrade agents and cancel that upgrade

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
cloud 9 10 +1
enterpriseSearch 2103 2102 -1
fleet 814 813 -1
serverlessSearch 85 84 -1
total -2

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
fleet 1068 1066 -2

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.4MB 2.4MB -2.5KB
fleet 970.2KB 970.2KB -2.0B
serverlessSearch 52.6KB 51.9KB -784.0B
total -3.3KB

Page load bundle

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

id before after diff
cloud 3.6KB 4.6KB +1.0KB
fleet 131.6KB 130.9KB -728.0B
total +317.0B
Unknown metric groups

API count

id before after diff
cloud 41 52 +11
fleet 1184 1182 -2
total +9

ESLint disabled line counts

id before after diff
enterpriseSearch 19 18 -1
fleet 49 46 -3
securitySolution 410 414 +4
serverlessSearch 7 4 -3
total -3

Total ESLint disabled count

id before after diff
enterpriseSearch 20 19 -1
fleet 59 56 -3
securitySolution 491 495 +4
serverlessSearch 7 4 -3
total -3

History

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

@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jun 13, 2023
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jun 13, 2023
* main: (199 commits)
  [Lens] Add custom formatter within the Lens editor (elastic#158468)
  [Uptime] Hide app if no data is available (elastic#159118)
  [CodeEditor] Add grok highlighting (elastic#159334)
  Expose decoded cloudId components from the cloud plugin's contract (elastic#159442)
  [Profiling] Use collector and symbolizer integrations in the add data page (elastic#159481)
  [Infrastructure UI] Hosts View: Unified Search bar with auto-refresh enabled (elastic#157011)
  [APM] Add feature flag for not available apm schema (elastic#158911)
  [Lens] Remove deprecated componentWillReceiveProps usage (elastic#159502)
  [api-docs] 2023-06-13 Daily api_docs build (elastic#159536)
  [data views] Use versioned router for REST routes (elastic#158608)
  [maps] fix geo line source not loaded unless maps application is opened (elastic#159432)
  [Enterprise Search][Search application]Fix Create Api key url (elastic#159519)
  [Security Solution] Increase timeout for indexing hosts (elastic#159518)
  dashboard Reset button disable (elastic#159430)
  [Security Solution] Unskip Endpoint API tests after package fix (elastic#159484)
  [Synthetics] adjust alert timing (elastic#159511)
  [ResponseOps][rule registry] Remove usages of `refresh: true` (elastic#159252)
  Revert "Remove unused package (elastic#158597)"
  [Serverless] Adding config to disable authentication on task manager background worker utilization API (elastic#159505)
  Remove unused package (elastic#158597)
  ...
saarikabhasi pushed a commit to saarikabhasi/kibana that referenced this pull request Jun 13, 2023
…lastic#159442)

## Summary

Fix elastic#138813

- decode the cloudId and expose the id components from the `cloud`
plugin's browser and server-side contracts
- remove the existing `decodeCloudId` helpers from the other plugins
- adapt the usages accordingly

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
saarikabhasi pushed a commit to saarikabhasi/kibana that referenced this pull request Jun 13, 2023
…lastic#159442)

## Summary

Fix elastic#138813

- decode the cloudId and expose the id components from the `cloud`
plugin's browser and server-side contracts
- remove the existing `decodeCloudId` helpers from the other plugins
- adapt the usages accordingly

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
saarikabhasi pushed a commit to saarikabhasi/kibana that referenced this pull request Jun 14, 2023
…lastic#159442)

## Summary

Fix elastic#138813

- decode the cloudId and expose the id components from the `cloud`
plugin's browser and server-side contracts
- remove the existing `decodeCloudId` helpers from the other plugins
- adapt the usages accordingly

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
TattdCodeMonkey added a commit that referenced this pull request Jul 12, 2023
## Summary

#159442 updated the decoding of the cloud id and added
`elasticsearchUrl` & `kibanaUrl` to the `CloudStart` type, but it only
set them on the `CloudSetup` result.

This change will also add them to the `CloudStart` so they are available
to code that is trying to read the values from `CloudStart` , mainly
[serverless_search](https://github.com/elastic/kibana/blob/main/x-pack/plugins/serverless_search/public/application/components/overview.tsx#L49-L51)
is what I'm concerned with.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Fleet Team label for Observability Data Collection Fleet team Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Common function to decode Cloud ID into constituent URLs