[SO Management] Unable to search config type

[jloleysens]

Kibana version:

main on 0af9176d3de

Elasticsearch version:
8.8.0 snapshot

Describe the bug:
Unable to use the SO Management UI to search based on title of config type.

Steps to reproduce:

1. Start ES + Kibana
2. Install a sample of data to have some SOs loaded
3. Go to Saved Object Management in the Stack Management area and use search

Expected behavior:
Should match config SOs

Screenshots (if relevant):

Screen.Recording.2023-02-28.at.10.39.54.mov

Additional context
I am guessing the config type needs register a default searchable field.

[elasticmachine]

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

[elasticmachine]

Pinging @elastic/appex-sharedux (Team:SharedUX)

[vadimkibana]

Is there a reason this search should be possible? I'm thinking the Advanced Settings has its own UI where all the settings can be seen and its internal implementation details should not be exposed in the SO management screen.

[afharo]

@vadimkibana, isn't that true to all (most) SOs? Dashboards, dataviews, and visualizations (to name a few) have their own UIs where they can be managed. However, they are shown in the SOM listing.

I think the best use case is export/import: if I want to backup/copy my settings to "restore" them elsewhere, the SOM listing is the place to do that.

[vadimkibana]

@afharo yes, you are right most SOs have their own listing pages, showing them in SOM is actually harmful: it exposes internal storage details of those entities, which should never be done. Those SOs can be modified through a "backdoor", which means Kibana apps cannot even trust their own data in their database.

The only semi-valid use case for SOM, in my opinion, is to provide that export/import functionality, but that should be done somehow differently, without exposing internal details of our whole database. And when importing the objects should go through the same validation as they would when created through dedicated HTTP endpoints, otherwise it is a security hole right now.

---

Going back to the Advanced Settings. I don't think it should be exposed in SOM, in fact, it is actually harmful to expose it there.

Advanced Settings should not probably even be a SO in the first place. But they are.

The backing storage and schema of the Advanced Settings is internal implementation detail, Kibana users should not be exposed to that:

It should never be possible to delete Advanced Settings:

Advanced Settings is a global singleton, it is not a typical app data model, concept of "Relationships" makes little sense for it:

AFAIK, there is a single Advanced Settings object per space, so copying it to another space seems like an undesirable behaviour, which again touches on internal implementation details, which should be hidden:

Export/import is probably harmful for Advanced Settings, as it might just get the cluster into which it is imported into some undefined behavior. Advanced settings SOs are identified by Kibana version, here 8.10, so, if it is imported into another cluster with a different Kibana version, I don't know what would be the behaviour. Hopefully, it would not crash the server or crash later when migration to 8.10 happens, and there is already existing 8.10 document which somebody imported.

If export/import feature is necessary for the Advanced Settings, it should be done separately in a well thought out way (for example, at least the data should be validated, when imported).

To sum up, the Advanced Settings should not be SO in the first place, but it is, unfortunately. Given it is an SO now, what we can do is hide it from the SOM listing, because—as you see from above—pretty much every SOM feature (when applied to Advanced Settings) is harmful, it exposes internal domain details and allows users to modify things which might then break something.

[sebelga]

Agree with @vadimkibana. Unless there is any objection I will hide the config SO type from SO management 👍

[afharo]

@vadimkibana, apologies for the late response.

The backing storage and schema of the Advanced Settings is internal implementation detail, Kibana users should not be exposed to that:

IMO, this applies to any kind of SO. Here's an example with dashboards.

The solution to that would be removing the Inspect option in the SOM listing. Not necessarily hiding the SO.

It should never be possible to delete Advanced Settings:

Why not? The way we upgrade today (creating a new object with the ID matching the version), you may want to clean up older ones.
Or even, if you want to reset all the defaults in a space, you can quickly get that by deleting the Advanced Settings in that space and refresh the page.

It's an admin feature, for sure. But I don't think it's a reason to hide it.

Advanced Settings is a global singleton, it is not a typical app data model, concept of "Relationships" makes little sense for it:

That is true! For now... we may have features in the future that change that (like a banner pointing to an uploaded file), or the default Data View setting could be linked to the Data View, so that we update/remove the setting if the data view is removed.

AFAIK, there is a single Advanced Settings object per space, so copying it to another space seems like an undesirable behaviour, which again touches on internal implementation details, which should be hidden:

#162734 kind of requests a way to copy settings from one space to another. I just tested it, and selecting override existing in the "Copy to spaces" dialog works.

Export/import is probably harmful for Advanced Settings, as it might just get the cluster into which it is imported into some undefined behavior. Advanced settings SOs are identified by Kibana version, here 8.10, so, if it is imported into another cluster with a different Kibana version, I don't know what would be the behaviour. Hopefully, it would not crash the server or crash later when migration to 8.10 happens, and there is already existing 8.10 document which somebody imported.

I agree that it might be harmful when exporting from a recent version and importing it to an older one (if more recent migrations have been applied, it will even refuse to import. If not, it simply is skipped). But version-to-version or even older-to-newer works fine (this one requires removing the current version and refreshing, to force it to be recreated, but it imports and upgrades the versions).

If export/import feature is necessary for the Advanced Settings, it should be done separately in a well thought out way (for example, at least the data should be validated, when imported).

The data IS validated when imported: core validates the version of the migrations to refuse anything that won't be able to understand (when going newer-to-older). Additionally, uiSettings has accepted the schema option for 3 years now (#59694). If registered settings are not using it, that's the settings owners to blame.

To sum up, the Advanced Settings should not be SO in the first place, but it is, unfortunately.

What persistence alternative do you propose?

It exposes internal domain details and allows users to modify things which might then break something.

I think this is true for all SOs, not only config. As long as users can tamper with any SO, they will be able to break stuff. And, unfortunately, due to issues like duplicating Data Views when importing, we need to allow them to tamper raw SO (even when we limited it to only editing the exported NDJONs (they no-longer can edit the raw document in the SOM listing, nor write to the .kibana indices).

---

I want to remind us the title and description of this issue: searching for the advanced settings SOs doesn't work. We probably only need to add the appropriate management.defaultSearchField in https://github.com/elastic/kibana/blob/main/packages/core/ui-settings/core-ui-settings-server-internal/src/saved_objects/ui_settings.ts.

And, potentially, to avoid some of the issues mentioned above, use the other options like

• getEditUrl so Inspect takes to the advanced settings view.
• onExport to handle relationships like the defaultIndex option.
• onImport to warn when importing to a different version about the actions to take (remove the more recent one if older-to-newer, warn that it won't be used when newer-to-older)

[sebelga]

I want to remind us the title and description of this issue: searching for the advanced settings SOs doesn't work. We probably only need to add the appropriate management.defaultSearchField

Yes we can indeed fix it with what the current API allows, but I think it is good, when an issue is raised, to question ourselves why certain functionalities exist as they do today instead of quickly add a prop somewhere.

The fact that we expose our internal database data modeling openly to anyone is something we need to question. I haven't seen it anywhere else.

It seems much safer to control what can be done on our internals rather than allowing admins to do everything. Example: delete the config SO. I'd much prefer a clear action button, inside the Advanced settings page, that says "Reset to defaults" (and we, internally, delete something in the DB), than letting someone delete a row in our DB and hoping it all works after refreshing the page.

I agree that the discussion is deviating from the initial raised but I think it is good to ask ourselves: what happens if we don't surface certain SO in SO management? What functionality do we lose? Can we regain it differently? (e.g. allow "export" without surfacing the internal SO in the table? - probably yes by adding a "include advanced settings" checkbox in the "export" workflow)

And then take a decision based on time constraints and priorities 👍

[afharo]

I'm ++ to revisiting the status quo. We certainly can do things in a better way.

My only concern is that we are delaying a bug fix to discuss great and very much-needed improvements that depend on a larger effort:

1. Cleaning up old config SOs: uiSettings: old config objects should be cleaned up #159661
2. Provide an alternative way of exporting/copying configs from one deployment/space to another.
3. Add "Reset all defaults" top button in advanced settings.
4. ...?

But, most importantly, we were planning to hide the SO without providing our users with an alternative solution (and leaving us without a valid workaround when handling SDHs). I would even wonder if hiding this could be considered a breaking change for many of those users and would require advice from the BCC.

---

The fact that we expose our internal database data modeling openly to anyone is something we need to question. I haven't seen it anywhere else.

++ to improving this. That's done by providing the getEditUrl method (which should be the same as getInAppUrl in this case)
https://github.com/elastic/kibana/blob/ac86737e346ce1241bae1a01ecc8ba3f5af30d98/packages/core/ui-settings/core-ui-settings-server-internal/src/saved_objects/ui_settings.ts#L34C5-L39

And, as highlighted in my previous comment, it also applies to other SOs, such as dashboards.

We probably want to make getEditUrl mandatory in the API to avoid this situation for any other SOs.

[sebelga]

My only concern is that we are delaying a bug fix to discuss great and very much-needed improvements that depend on a larger effort

Agree 👍 Let's keep the focus on the initial issue and limit the scope to make the config searchable without creating a potential breaking change.

In the meantime we can discuss SO improvements with an holistic approach

[vadimkibana]

IMO, this applies to any kind of SO. Here's an example with dashboards.

Yes, all content types should not expose their internal implementation details. It is like, when you are editing Google Drive spreadsheet, you do not have access to the database record where it is stored. When you are using a Slack channel, Slack does not give you access to the internal database record that represents the channel. Etc.. But in Kibana the guts of our database are exposed to everyone, which is a source of many problems and SDHs.

[pgayvallet]

It's unclear to me reading the discussion, so did we come to an agreement regarding what should be done and by whom, or should we just close this issue?

[sebelga]

It's unclear to me reading the discussion, so did we come to an agreement regarding what should be done and by whom, or should we just close this issue?

We agreed that we were going to make the config type searchable, as SharedUX owns this SO type we will do it 👍

[sebelga]

So. I am fiiiinally back with this issue and was going to "quickly" fix it inspired by @afharo "...we probably only need to add the appropriate management.defaultSearchField".

And it results that the ui settings document does not have searchable title in ES. The title is generated at runtime (https://github.com/elastic/kibana/blob/main/packages/core/ui-settings/core-ui-settings-server-internal/src/saved_objects/ui_settings.ts#L42) and is thus not searchable in ES (https://github.com/elastic/kibana/blob/main/src/plugins/saved_objects_management/server/routes/find.ts#L78)

Before trying to find a way to merge runtime title search + DB search I'd like to confirm that we think it is worth the effort as this didn't come from any users/customers.

Was it a one time moment that @jloleysens could not find the advanced settings in the UI or is it a repetitive annoying bug that justifies the effort?

[sebelga]

It seems that what I would have to do is do 2 fetches, the current one and a second one with SO types marked as management.searchRuntimeTitle: true (new prop on the type definition) and then filter on the server.

[jloleysens]

Before trying to find a way to merge runtime title search + DB search...

Oh boy, I'd defer to @afharo if you want further technical engagement.

could not find the advanced settings in the UI or is it a repetitive annoying bug that justifies the effort?

This is bad UX/a bug that could have affected any number of users. Most (all?) of whom are not going through the trouble of creating an issue on GH for us.

My 2c: however, if it's high-hanging fruit we are not going to fix let's label it and close the issue. WDYT @sebelga ?

[jloleysens]

@sebelga Ah, wait, I see you have a fix here. I'll take a look!

[sebelga]

I presented my fix to this issue and discussed it with @rudolf offline and it seems that there will be an effort to include the "name" in audit logs which would allow searching for advanced settings.

My solution implies searching all SO, which for some users might be millions of docs. This is a big trade off for not so much value. We decided to not continue with my approach and wait for the "name" to be present.

Closing this issue in the meantime.