-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Logs onboarding] Removes dependency on apikey id in saved objects #159535
[Logs onboarding] Removes dependency on apikey id in saved objects #159535
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
core.setup.http.basePath.publicBaseUrl ?? // priority given to server.publicBaseUrl | ||
plugins.cloud?.setup?.kibanaUrl ?? // then cloud id | ||
getFallbackUrls(coreStart).kibanaUrl; // falls back to local network binding |
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 you manually test this out in the QA environment to make sure it works in the 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.
i tested it out locally by setting xpack.cloud.id: 'es-logs-onboarding-test:ZWFzdHV...4MjYz'
, which is the config set in cloud.
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.
You can create a serverless instance in the QA environment pretty easily (pastebin)
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.
lgtm
💚 Build Succeeded
Metrics [docs]Async chunks
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
@@ -29,7 +29,7 @@ export interface SavedObservabilityOnboardingState | |||
export const observabilityOnboardingState: SavedObjectsType = { | |||
name: OBSERVABILITY_ONBOARDING_STATE_SAVED_OBJECT_TYPE, | |||
hidden: false, | |||
namespaceType: 'multiple', |
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.
So... In theory that kind of change is absolutely unsupported on released types (types already in a production version).
Now... In practice, technically this should mostly be fine, as both multiple
and agnostic
types have the same id-generation logic. However there can be functional impacts from this change. Also, I'm not even sure how our system reacts when an agnostic
-type document is encountered with the namespaces
field present and populated.
So, are you sure about this 😄 ?
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.
In theory that kind of change is absolutely unsupported on released types (types already in a production version).
This is not yet released :)
(introduced in #158386)
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.
yes, this should be fine: These saved objects are:
- not yet released, so no users are impacted
- always referenced by id, regardless of namespace (setting
'multiple'
was an oversight) - mostly ephemeral, they exist for one step in a larger onboarding flow and then not referenced again after flow is completed
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.
Ok, great then, thanks for confirming
Closes #159381
Makes use of the auto-generated saved object ID to identify observability onboarding state saved objects, so the API key id is never persisted. In this change, the generated API key never has an explicit association with the saved object for the onboarding flow.