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.

Sign up for GitHub

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

Jump to bottom

Add support for sharing saved objects to all spaces #76132

Merged
merged 29 commits into from
Oct 5, 2020
Merged

Add support for sharing saved objects to all spaces #76132

merged 29 commits into from
Oct 5, 2020

Conversation

jportner
Copy link
Contributor

@jportner jportner commented Aug 27, 2020 •
edited
Loading

Overview

Resolves #69808 and partially resolves #76992.

This PR allows saved objects to be shared to all current and future spaces. See linked issues for details. Primary changes:

Server side

  • Changed Saved Objects Repository and client wrappers
    • Queries support "all spaces"
    • Delete operation now deletes objects that are shared to multiple spaces (instead of removing the object from the current space)
    • Create and BulkCreate operations now support "initial namespaces" that consumers can specify

Client side

  • Changed "Shared spaces" column in "Saved Objects" management table to render all spaces correctly
  • Changed "Share to space" flyout substantially --
    • "Share options" provides choice whether to share to selected spaces or all spaces
    • Privilege checks conditionally disable options based on what the user is authorized to do
    • Additional UI text to allow the user to create a new space (if authorized and none exist), and to describe any hidden spaces that the object may be shared to

Screenshots

These screenshots depict different users:

  • "Global User" -- this user has access to all spaces
  • "Default + Alpha User" -- this user has access only to the Default and Alpha spaces
  • "Default User" -- this user has access only to the Default space
Scenario 1: sharing an object but no other spaces exist

The Global User sees additional text with a link to create a new space. The Default User does not.

image

Scenario 2: sharing an object after additional spaces have been created

Each user can only see spaces that they are authorized to create saved objects in.

image

Scenario 3: sharing an object after the object has already been shared to additional spaces

Each user can only see spaces that they are authorized to create saved objects in. The Default + Alpha User and the Default User each see an additional label with a count of hidden spaces, and they see additional text with a link explaining that they lack privileges to see those spaces.

image

Scenario 4: disabled options and tooltips

If a user does not have global access, they cannot A. share an object to all spaces, or B. unshare an object from all spaces. In each of these cases, UI elements are disabled, and a tooltip renders the appropriate message.

image

Scenario 5: "Saved Objects" management table

The table will now display an appropriate badge if an object is shared to all spaces. NOTE FOR REVIEWERS: this column is currently disabled by default, see "x-pack/plugins/spaces/public/share_saved_objects_to_space/share_saved_objects_to_space_service.ts" for details.

image

@jportner jportner force-pushed the issue-69808-share-to-all-spaces branch from b4dcae9 to f8cef00 Compare August 28, 2020 03:28
@jportner jportner force-pushed the issue-69808-share-to-all-spaces branch 2 times, most recently from 1d44a3e to ebaa032 Compare September 11, 2020 02:14
@jportner jportner force-pushed the issue-69808-share-to-all-spaces branch 4 times, most recently from fd152f9 to e93f563 Compare September 23, 2020 22:25
@jportner jportner linked an issue Sep 28, 2020 that may be closed by this pull request
Improve saved object Copy/Share flyouts when no spaces exist #76992
Closed
@jportner jportner force-pushed the issue-69808-share-to-all-spaces branch from e93f563 to 8ebcb3f Compare September 30, 2020 13:54
@jportner jportner removed a link to an issue Sep 30, 2020
Improve saved object Copy/Share flyouts when no spaces exist #76992
Closed
@jportner jportner mentioned this pull request Sep 30, 2020
Closed
@jportner
Change delete to always delete the object
8cdf171 
This method will no longer update a multi-namespace object to
remove it from its current namespace. Instead, it will always
delete the object, even if that object exists in multiple
namespaces.
Adds a new `force` option that is required if the object does exist
in multiple namespaces.
@jportner
Change SavedObjectsRepository to support '*' namespace string
d978bce 
This is treated as "all namespaces".
@jportner
Small refactor for SecureSavedObjectsClientWrapper unit tests
966f826 
Changed how options are passed into tests and how privilege checks
are tested so that tests are easier to understand and change.
@jportner
Change create and bulkCreate to allow initial namespaces
69f6491
@jportner
Change "share to space" initial warning callout
2e1dabb 
Removed "Make a copy" button, made "make a copy" inline text a
clickable link instead.
@jportner
Clean up "Shared spaces" column code, add unit tests
6012e05 
No functionality changes, just some refactoring.
@jportner
Change "Shared spaces" column to display "All spaces" badge
87ed4e8
@jportner
Clean up "Share to space" routes, add unit tests
8385cdb 
No functionality changes, just some refactoring.
@jportner
Remove dead code
b79aff1
@jportner
Clean up saved object authorization unit tests / errors
1deac86 
One error message was incorrect. Converted to get rid of snapshots.
@jportner
Change "Share to space" flyout to support sharing to all spaces
a15abff 
* Added new checkable card options
* Added privilege checks which conditionally disable options
* Added descriptive text when unknown spaces are selected
@jportner jportner force-pushed the issue-69808-share-to-all-spaces branch from 8ebcb3f to 473dc7d Compare September 30, 2020 17:44
@jportner
Change saved objects table to use force-delete
e68ecc4 
This is necessary when deleting saved objects that exist in
multiple namespaces.
@jportner
Fix "Copy to space" functional test
f855fcd 
Could not figure out why this broke, I did not change this flyout.
At any rate, clicking a different part of the radio button fixed
it.
@jportner jportner force-pushed the issue-69808-share-to-all-spaces branch from 473dc7d to f855fcd Compare September 30, 2020 18:12
@jportner jportner marked this pull request as ready for review September 30, 2020 18:15
@jportner jportner requested review from a team as code owners September 30, 2020 18:15
@jportner jportner added the release_note:skip Skip the PR/issue when compiling release notes label Sep 30, 2020
legrego
legrego reviewed Oct 1, 2020
View reviewed changes
Copy link
Member

@legrego legrego left a comment

Choose a reason for hiding this comment

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

This is working beautifully! I haven't evaluated all of the functional or UI unit tests yet, but I'll leave you with what I have so far

src/core/server/saved_objects/service/lib/search_dsl/query_params.ts Show resolved Hide resolved
src/core/server/saved_objects/service/saved_objects_client.ts Show resolved Hide resolved
x-pack/plugins/security/server/authorization/actions/saved_object.test.ts Show resolved Hide resolved
x-pack/plugins/spaces/public/share_saved_objects_to_space/components/no_spaces_available.tsx Outdated Show resolved Hide resolved
.../plugins/spaces/public/share_saved_objects_to_space/components/selectable_spaces_control.tsx Outdated Show resolved Hide resolved
x-pack/plugins/spaces/public/share_saved_objects_to_space/components/share_to_space_form.tsx Outdated Show resolved Hide resolved
x-pack/plugins/spaces/public/share_saved_objects_to_space/components/share_to_space_flyout.tsx Outdated Show resolved Hide resolved
x-pack/plugins/spaces/public/share_saved_objects_to_space/components/share_to_space_flyout.tsx Show resolved Hide resolved
x-pack/plugins/spaces/public/share_saved_objects_to_space/components/share_to_space_flyout.tsx Show resolved Hide resolved
.../plugins/spaces/public/share_saved_objects_to_space/components/selectable_spaces_control.tsx Outdated Show resolved Hide resolved
@legrego legrego self-requested a review October 2, 2020 14:43
@jportner
More changes for PR review feedback
e0ad362
@jportner
Rename initialNamespaces to namespaces
790a437
@jportner
Merge branch 'master' into issue-69808-share-to-all-spaces
57ad704
@jportner
Fix API integration tests
3c0aa6b 
A holdover from the legacy test suite checked to ensure that saved
objects could not be created with the `namespace` or `namespaces`
fields. This isn't necessary for an API integration test -- the
unit test suite covers this scenario -- and it's invalid now that
`namespaces` is a valid field. So I removed these test cases.
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

id before after diff
spaces 224 237 +13

async chunks size

id before after diff
savedObjectsManagement 206.9KB 206.9KB +13.0B

distributable file count

id before after diff
default 44116 44115 -1

page load bundle size

id before after diff
core 661.1KB 661.2KB +96.0B
spaces 343.9KB 364.2KB +20.3KB
total +20.3KB

History

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

legrego
legrego approved these changes Oct 4, 2020
View reviewed changes
Copy link
Member

@legrego legrego left a comment

Choose a reason for hiding this comment

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

LGTM!

@jportner jportner merged commit caa5da2 into elastic:master Oct 5, 2020
@jportner jportner mentioned this pull request Oct 5, 2020
Merged
jportner added a commit to jportner/kibana that referenced this pull request Oct 5, 2020
@jportner
Add support for sharing saved objects to all spaces (elastic#76132)
b6961d8
gmmorris added a commit to gmmorris/kibana that referenced this pull request Oct 5, 2020
@gmmorris
Merge branch 'master' into actions/decouple-actions-from-alertid
d99fe4b 
* master: (128 commits)
  add core-js production dependency (elastic#79395)
  Add support for sharing saved objects to all spaces (elastic#76132)
  [Alerting UI] Display a banner to users when some alerts have failures, added alert statuses column and filters (elastic#79038)
  load js-yaml lazily (elastic#79092)
  skip flaky suite (elastic#77278)
  Fix agentPolicyUpdateEventHandler() to use app context soClient for creation of actions (elastic#79341)
  [Security Solution] Untitled Timeline created when first action is to add note (elastic#78988)
  [Security Solutions][Detection Engine] Updates the edit rules page to:wq! only have what is selected for editing (elastic#79233)
  Cleanup yarn.lock from duplicates (elastic#66617)
  [kbn/optimizer] implement more efficient auto transpilation for node (elastic#79052)
  [Ingest Manager] Rename Fleet setup and requirement, Fleet => Central… (elastic#79291)
  [core/server/plugins] don't run discovery in dev server parent process (take 2) (elastic#79358)
  [babel/register] remove from build (take 2) (elastic#79379)
  [Security Solution] Changes rules table tag display (elastic#77102)
  define integrationTestRoot in config file and use to define screensho… (elastic#79247)
  Revert "[babel/register] remove from build (elastic#79176)"
  skip flaky suite (elastic#75241)
  [Uptime] Synthetics UI (elastic#77960)
  [Security Solution] [Detections] Only display actions options if user has "read" privileges (elastic#78812)
  [babel/register] remove from build (elastic#79176)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Oct 5, 2020
@gmmorris
Merge branch 'master' into task-manager/health
7b14dff 
* master: (288 commits)
  add core-js production dependency (elastic#79395)
  Add support for sharing saved objects to all spaces (elastic#76132)
  [Alerting UI] Display a banner to users when some alerts have failures, added alert statuses column and filters (elastic#79038)
  load js-yaml lazily (elastic#79092)
  skip flaky suite (elastic#77278)
  Fix agentPolicyUpdateEventHandler() to use app context soClient for creation of actions (elastic#79341)
  [Security Solution] Untitled Timeline created when first action is to add note (elastic#78988)
  [Security Solutions][Detection Engine] Updates the edit rules page to only have what is selected for editing (elastic#79233)
  Cleanup yarn.lock from duplicates (elastic#66617)
  [kbn/optimizer] implement more efficient auto transpilation for node (elastic#79052)
  [Ingest Manager] Rename Fleet setup and requirement, Fleet => Central… (elastic#79291)
  [core/server/plugins] don't run discovery in dev server parent process (take 2) (elastic#79358)
  [babel/register] remove from build (take 2) (elastic#79379)
  [Security Solution] Changes rules table tag display (elastic#77102)
  define integrationTestRoot in config file and use to define screensho… (elastic#79247)
  Revert "[babel/register] remove from build (elastic#79176)"
  skip flaky suite (elastic#75241)
  [Uptime] Synthetics UI (elastic#77960)
  [Security Solution] [Detections] Only display actions options if user has "read" privileges (elastic#78812)
  [babel/register] remove from build (elastic#79176)
  ...
jportner added a commit that referenced this pull request Oct 5, 2020
@jportner
[7.x] Add support for sharing saved objects to all spaces (#76132) (#…
bc114ae 
…79411)
@jportner jportner mentioned this pull request Oct 6, 2020
Merged
@jportner jportner deleted the issue-69808-share-to-all-spaces branch November 20, 2020 18:14
@jportner jportner mentioned this pull request Apr 29, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gchaps gchaps gchaps left review comments

@pgayvallet pgayvallet pgayvallet approved these changes

@rudolf rudolf rudolf left review comments

@mdefazio mdefazio mdefazio approved these changes

@legrego legrego legrego approved these changes

Assignees
No one assigned
Labels
backported release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0
Projects
None yet
Milestone
No milestone
7 participants
@jportner @kibanamachine @rudolf @pgayvallet @legrego @mdefazio @gchaps