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

Sharing saved-objects in multiple spaces: phase 3 #67380

Closed
7 tasks
jportner opened this issue May 26, 2020 · 9 comments · Fixed by #94383
Closed
7 tasks

Sharing saved-objects in multiple spaces: phase 3 #67380

jportner opened this issue May 26, 2020 · 9 comments · Fixed by #94383
Assignees
Labels
Feature:Saved Objects Feature:Security/Sharing Saved Objects Platform Security - Sharing Saved Objects feature release_note:enhancement Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!

Comments

@jportner
Copy link
Contributor

jportner commented May 26, 2020

Phase 3 from #27004

Currently blocked on #71409

Draft server-side tasks

  • Change export to expose object namespaces to internal consumers (still omit these from HTTP responses), and change copySavedObjectsToSpaces to exclude objects if they already exist in a given namespace
  • Abstract out export's includeReferencesDeep functionality to a public method in SavedObjectsClient
    • Note: Performance improvement opportunity (reduce multiple authorization checks)
  • Change SOC addToNamespaces and deleteFromNamespaces methods to add option that includes references (and update HTTP APIs to match)
  • Change usage data to include information on resolved aliases

Draft client-side tasks

- [ ] Update index pattern delete confirmation modal: add "This/these is/are shared in X spaces" banner, change "Delete" button to "Remove from space" (primary) and "Delete everywhere" (secondary)

  • Update Saved Objects Management Screen delete confirmation modal (?)
  • Update Share to Space flyout to support sharing related objects and/or related tags
  • Core SO abstraction changes

Mockup of updated Share flyout:

image

@jportner jportner added release_note:enhancement Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! labels May 26, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

@kobelb
Copy link
Contributor

kobelb commented Apr 20, 2021

👋 I noticed in the mockups that we're classifying saved-object references as either "relatives" or "tags". From the technical perspective, they're all "references". This classification will introduce some inconsistency from saved-object management, as all referenced saved-objects are considered "related saved-objects":

Screen Shot 2021-04-20 at 12 35 20 PM

tags-are-normal

The whole "are tags normal saved-objects and a result should be in references" resulted in a bunch of prior discussions: #74571 (comment)

I'd really like for us to be consistent here. If our prior conclusions were unsatisfactory, I'm all for us revisiting this. However, I don't think we should introduce inconsistencies between the sharing saved-object UIs and the rest of Kibana.

@jportner
Copy link
Contributor Author

The whole "are tags normal saved-objects and a result should be in references" resulted in a bunch of prior discussions: #74571 (comment)

Thanks, I wasn't aware of that discussion! Just following the mockups.

I'd really like for us to be consistent here. If our prior conclusions were unsatisfactory, I'm all for us revisiting this. However, I don't think we should introduce inconsistencies between the sharing saved-object UIs and the rest of Kibana.

Sure thing. FWIW, in the implementation, I intended to leave the "Include related objects" box checked-and-disabled, so it's just a visual indicator that additional objects will be shared (similar to the existing Copy to Space flyout today).

I do think it could be useful to optionally omit tags from the rest of the related objects. Even though they rely on the same implementation under the hood (references), they are just metadata and that might not make sense to share all tags to all spaces.

So what do you think the best path forward is here?

  • Get rid of the "Include all tags" checkbox
  • Change "Include all related objects" to something else like "Include all non-tag related objects" (🤮 )
  • ???

@kobelb
Copy link
Contributor

kobelb commented Apr 20, 2021

Sure thing. FWIW, in the implementation, I intended to leave the "Include related objects" box checked-and-disabled, so it's just a visual indicator that additional objects will be shared (similar to the existing Copy to Space flyout today).

Good to know, thanks!

I do think it could be useful to optionally omit tags from the rest of the related objects. Even though they rely on the same implementation under the hood (references), they are just metadata and that might not make sense to share all tags to all spaces.

Good point and valid use-case :)

So what do you think the best path forward is here?

Get rid of the "Include all tags" checkbox
Change "Include all related objects" to something else like "Include all non-tag related objects" (🤮 )
???

After you explained the valid use-case for wanting to share saved-objects and exclude the tags, I've started to feel like my concerns are rather pedantic. The original design conveyed to me that "tags aren't related objects", which is what made me think that we were doing something wrong.

What if we had an unchecked by default checkbox that said: "Exclude tags"? That way if both check-boxes were checked, we'd logically be including all related saved-objects but excluding tags, which is logically valid even if tags are related objects?

@pgayvallet
Copy link
Contributor

pgayvallet commented Apr 22, 2021

What if we had an unchecked by default checkbox that said: "Exclude tags"? That way if both check-boxes were checked, we'd logically be including all related saved-objects but excluding tags, which is logically valid even if tags are related objects?

I'm not sure we'll be avoid to dodge the fact that tags, even if using the same underlying implementation, are not functionally the same than other referenced objects much longer to be honest.

Now, I don't think that switching the includeTags checkbox to an excludeTags one causes much changes in the POC implementation of https://github.com/elastic/kibana/pull/94383/files (@jportner correct me if I'm wrong here), so this may be an easy small victory. So here, get my upvote.

@jportner
Copy link
Contributor Author

Agree we can make that a checkbox to exclude instead of a checkbox to include. It probably won't make much sense to have the first checkbox say "include" and the second one say "exclude", but perhaps we don't need the first to be a checkbox at all, since we don't plan to allow users to opt out of including (exclude) non-tag references anyway. I'll see what our designers think!

@kobelb
Copy link
Contributor

kobelb commented Apr 22, 2021

I'm not sure we'll be avoid to dodge the fact that tags, even if using the same underlying implementation, are not functionally the same than other referenced objects much longer to be honest.

Are there other situations or use-cases for tags that make them behave differently from other saved-object references?

IIRC, my original reasoning for treating tags like normal saved-object references was that it made implementing sharing to spaces, and everything else implicitly work. However, if we're now saying that when a saved-object is shared to a space we really don't need to share the tags, that contradicts my prior reasoning.

@pgayvallet
Copy link
Contributor

Are there other situations or use-cases for tags that make them behave differently from other saved-object references?

The initial specs mockups for tagging had the SO export modal modified to dissociate tags from other relations too. But apart from the SO management section, I don't think so.

@kobelb
Copy link
Contributor

kobelb commented Apr 22, 2021

In the short-term, we could also add a three-way toggle: "All | Exclude Tags | None". I defer to design though, they have 100x better ideas than myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Saved Objects Feature:Security/Sharing Saved Objects Platform Security - Sharing Saved Objects feature release_note:enhancement Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants