-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
Expose decoded cloudId components from the cloud plugin's contract #159442
Conversation
…-ref HEAD~1..HEAD --fix'
There was a problem hiding this 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 { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
export interface CloudStart { | ||
/** | ||
* A React component that provides a pre-wired `React.Context` which connects components to Cloud services. | ||
*/ | ||
CloudContextProvider: FC<{}>; |
There was a problem hiding this comment.
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.
/** | ||
* 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; |
There was a problem hiding this comment.
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), |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fleet LGTM
Pinging @elastic/kibana-core (Team:Core) |
Moving into review as the current failures are unrelated to the PR's changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Synthetics changes LGTM !!
Pinging @elastic/fleet (Team:Fleet) |
Pinging @elastic/uptime (Team:uptime) |
There was a problem hiding this 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!
* The full URL to the Kibana deployment. | ||
*/ | ||
kibanaUrl?: string; |
There was a problem hiding this comment.
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?)
There was a problem hiding this comment.
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
- only available on cloud (obviously)
- reflecting the "internal" way of accessing the deployment.
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
* 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) ...
…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>
…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>
…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>
## 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.
Summary
Fix #138813
cloud
plugin's browser and server-side contractsdecodeCloudId
helpers from the other plugins