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 phase 3.5 #100424

Merged
merged 37 commits into from
Jun 28, 2021

Conversation

jportner
Copy link
Contributor

@jportner jportner commented May 20, 2021

Resolves: #100686 and #81605.

Changes

Primary changes are:

  • Updated Share to Space flyout:
    • Changed design
    • Now shares the target object and all of its references (excluding tags)
    • Now disables legacy URL aliases when they exist in target space(s)
  • Updated Saved Object Management "Delete" modal to indicate when objects are shared
  • Add usage data collection:
    core.services.savedObjects.legacyUrlAliases.activeCount
    core.services.savedObjects.legacyUrlAliases.inactiveCount
    core.services.savedObjects.legacyUrlAliases.disabledCount
    core.services.savedObjects.legacyUrlAliases.totalCount
    core.savedObjectsRepository.resolvedOutcome.exactMatch
    core.savedObjectsRepository.resolvedOutcome.aliasMatch
    core.savedObjectsRepository.resolvedOutcome.conflict
    core.savedObjectsRepository.resolvedOutcome.notFound
    core.savedObjectsRepository.resolvedOutcome.total
    spaces.apiCalls.disableLegacyUrlAliases.total
    
  • Updated client-side SavedObjectsClient to support resolve API and namespaces field (will be needed for future consumers)

Screenshots

Click to expand

Share with relatives:

share-with-relatives

Share and disable aliases:

share-disable-aliases

Delete modal:

image

Manual testing procedures

A. Setup:

  1. Uncomment code to enable Spaces column in Saved Object Mgmt page:

    Click to show diff
    diff --git a/x-pack/plugins/spaces/public/share_saved_objects_to_space/share_saved_objects_to_space_service.ts b/x-pack/plugins/spaces/public/share_saved_objects_to_space/share_saved_objects_to_space_service.ts
    index bc703477604..c59c857beb9 100644
    --- a/x-pack/plugins/spaces/public/share_saved_objects_to_space/share_saved_objects_to_space_service.ts
    +++ b/x-pack/plugins/spaces/public/share_saved_objects_to_space/share_saved_objects_to_space_service.ts
    @@ -9,7 +9,7 @@ import type { SavedObjectsManagementPluginSetup } from 'src/plugins/saved_object
     import type { SpacesApiUi } from 'src/plugins/spaces_oss/public';
     
     import { ShareToSpaceSavedObjectsManagementAction } from './share_saved_objects_to_space_action';
    -// import { ShareToSpaceSavedObjectsManagementColumn } from './share_saved_objects_to_space_column';
    +import { ShareToSpaceSavedObjectsManagementColumn } from './share_saved_objects_to_space_column';
     
     interface SetupDeps {
       savedObjectsManagementSetup: SavedObjectsManagementPluginSetup;
    @@ -21,7 +21,7 @@ export class ShareSavedObjectsToSpaceService {
         const action = new ShareToSpaceSavedObjectsManagementAction(spacesApiUi);
         savedObjectsManagementSetup.actions.register(action);
         // Note: this column is hidden for now because no saved objects are shareable. It should be uncommented when at least one saved object type is multi-namespace.
    -    // const column = new ShareToSpaceSavedObjectsManagementColumn(spacesApiUi);
    -    // savedObjectsManagementSetup.columns.register(column);
    +    const column = new ShareToSpaceSavedObjectsManagementColumn(spacesApiUi);
    +    savedObjectsManagementSetup.columns.register(column);
       }
     }
  2. Install the Sharing Saved Objects Test Plugin

  3. Start Kibana

  4. Navigate to the "barney" app and generate new toys

  5. Navigate to Kibana Dev Tools and create the 26 test spaces:

    Click to show command
    POST .kibana/_bulk
    {"index":{"_id":"space:alpha"}}
    {"space":{"name":"Alpha","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:01.830Z"}
    {"index":{"_id":"space:bravo"}}
    {"space":{"name":"Bravo","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:02.591Z"}
    {"index":{"_id":"space:charlie"}}
    {"space":{"name":"Charlie","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:03.587Z"}
    {"index":{"_id":"space:delta"}}
    {"space":{"name":"Delta","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:04.604Z"}
    {"index":{"_id":"space:echo"}}
    {"space":{"name":"Echo","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:05.616Z"}
    {"index":{"_id":"space:foxtrot"}}
    {"space":{"name":"Foxtrot","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:06.630Z"}
    {"index":{"_id":"space:golf"}}
    {"space":{"name":"Golf","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:07.650Z"}
    {"index":{"_id":"space:hotel"}}
    {"space":{"name":"Hotel","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:08.664Z"}
    {"index":{"_id":"space:india"}}
    {"space":{"name":"India","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:09.688Z"}
    {"index":{"_id":"space:juliett"}}
    {"space":{"name":"Juliett","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:10.711Z"}
    {"index":{"_id":"space:kilo"}}
    {"space":{"name":"Kilo","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:11.727Z"}
    {"index":{"_id":"space:lima"}}
    {"space":{"name":"Lima","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:12.743Z"}
    {"index":{"_id":"space:mike"}}
    {"space":{"name":"Mike","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:13.755Z"}
    {"index":{"_id":"space:november"}}
    {"space":{"name":"November","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:14.770Z"}
    {"index":{"_id":"space:oscar"}}
    {"space":{"name":"Oscar","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:15.791Z"}
    {"index":{"_id":"space:papa"}}
    {"space":{"name":"Papa","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:16.802Z"}
    {"index":{"_id":"space:quebec"}}
    {"space":{"name":"Quebec","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:17.820Z"}
    {"index":{"_id":"space:romeo"}}
    {"space":{"name":"Romeo","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:18.842Z"}
    {"index":{"_id":"space:sierra"}}
    {"space":{"name":"Sierra","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:19.849Z"}
    {"index":{"_id":"space:tango"}}
    {"space":{"name":"Tango","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:20.868Z"}
    {"index":{"_id":"space:uniform"}}
    {"space":{"name":"Uniform","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:21.876Z"}
    {"index":{"_id":"space:victor"}}
    {"space":{"name":"Victor","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:22.904Z"}
    {"index":{"_id":"space:whiskey"}}
    {"space":{"name":"Whiskey","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:23.921Z"}
    {"index":{"_id":"space:x-ray"}}
    {"space":{"name":"X-ray","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:24.937Z"}
    {"index":{"_id":"space:yankee"}}
    {"space":{"name":"Yankee","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:25.948Z"}
    {"index":{"_id":"space:zulu"}}
    {"space":{"name":"Zulu","disabledFeatures":[]},"type":"space","references":[],"migrationVersion":{"space":"6.6.0"},"coreMigrationVersion":"8.0.0","updated_at":"2021-06-17T14:50:26.963Z"}
    
  6. Navigate to the Saved Objects Management screen

B. Share an object with references:

Each of the four Army Men objects has a reference to the other three. If you share one of them, the other three should be shared too.

  1. Share one of the Army Men objects to any other space
  2. Note that at the bottom of the Share flyout, three other related objects (the other Army Men) are included when you share
  3. Select other space(s) and click Save & Continue
  4. Note each of the Army Men have been updated on the screen

C. Share an object and disable legacy URL aliases

The Sheriff Woody object has legacy URL aliases in 27 spaces: 26 for the alphabet (Alpha, Bravo, Charlie, etc.), and one in an additional space (---).

  1. Open your browser's developer tools to track network requests
  2. Share the Sheriff Woody object, select the Alpha space, and click Continue
  3. Note that on the next screen, the flyout will indicate that an alias will be disabled in the Alpha space; click Save & Close
  4. In your browser's developer tools, note that the alias in the Alpha space was disabled
  5. Share the Sheriff Woody object again, this time select "All spaces" and click Continue
  6. Note that on the next screen, the flyout will indicate that an alias will be disabled in 25 spaces (Bravo, Charlie, ..., Zulu); click Save & Close
  7. In your browser's developer tools, note that 26 aliases were disabled; these are the 25 aliases for Bravo through Zulu, and additionally the alias for the --- space which does not exist

@jportner jportner force-pushed the sharing-saved-objects-phase-3.5 branch 4 times, most recently from d82b858 to 64c8014 Compare May 31, 2021 14:03
This is used in feature-specific contexts for SpaceList and
ShareToSpaceFlyout components, so the rendered avatar for a space is in
a disabled state if the feature is disabled in that space.
…ient

Add support for `resolve` method to the public SOC client abstraction.
This was overlooked in Sharing Saved Objects Phase 2 because it does not
use the same type definitions as the server-side SOC client.
Also added the `namespaces` field to the SimpleSavedObject type, which
is always present as of several releases ago.
A user is authorized to change a legacy URL alias if they are have
privileges to modify the target object.

TODO: add an HTTP route for this API
Before, only the target object would be updated.
When sharing an object, we include all of its references *except* for
tags. Note, this does not preclude tags from being shared directly.
@jportner jportner force-pushed the sharing-saved-objects-phase-3.5 branch from 64c8014 to bc7e2fa Compare June 1, 2021 23:51
@jportner
Copy link
Contributor Author

jportner commented Jun 2, 2021

@legrego @watson this is ready for a draft review whenever you are ready.

What changes are already included:

  • Several design changes to the Share flyout
  • Addition of "disableLegacyUrlAliases" API (and HTTP route) for SpacesClient
  • Change Share flyout to update spaces for the target object and all of its references (excluding tags)

Next I have to add:

  • The additional Flyout screen to prevent alias conflicts while sharing
  • Small update to the SavedObjectsManagement "Delete" modal to allow shared objects to be deleted warn users when they are about to delete shared objects
  • Usage data for aliases

For the usage data, I think we could just add another counter value for the "disableLegacyUrlAliases" HTTP API route and call it a day. Thoughts?

@jportner jportner requested review from legrego and watson June 2, 2021 00:00
@jportner jportner force-pushed the sharing-saved-objects-phase-3.5 branch from bc7e2fa to ce1a020 Compare June 2, 2021 00:19
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.

General code/structure looks good to me. I haven't manually tested this yet, but I'll do so once you have the rest of the UI changes in place.

For the usage data, I think we could just add another counter value for the "disableLegacyUrlAliases" HTTP API route and call it a day. Thoughts?

What types of questions do we want to answer with this information? If I understand correctly, this would just tell us how often the endpoint was called, but not how many aliases have been disabled.

@jportner
Copy link
Contributor Author

jportner commented Jun 10, 2021

What types of questions do we want to answer with this information? If I understand correctly, this would just tell us how often the endpoint was called, but not how many aliases have been disabled.

After thinking on it some more, we probably want to be able to answer:

  1. How many aliases exist?
  2. How many aliases are disabled?
  3. How many aliases are being used (!disabled && resolveCounter > 0)?
  4. How often are users experiencing the different resolve alias outcomes? (exact match, alias match, conflict)
  5. How often do users need to disable aliases (in other words: how often are we preventing alias conflicts from occurring)?

We already have standard Core usage stats fields for the /internal/saved_objects/resolve/{type}/{id} HTTP route. However, it's very likely that consumers will implement their own routes to resolve their objects with the SOC.

For questions 1-3: we should update the Core usage data service to fetch alias data from the Kibana index to answer the first three questions.

For question 4: we should add the following Core usage stats fields that are incremented whenever SOR.resolve is called:

core.savedObjects.resolved.exactMatchOutcome
core.savedObjects.resolved.aliasMatchOutcome
core.savedObjects.resolved.conflictOutcome
core.savedObjects.resolved.total

For question 5: we should add the following Spaces usage stats field that is incremented whenever the /api/spaces/_disable_legacy_url_aliases HTTP route is called:

spaces.apiCalls.disableLegacyUrlAliases.total

Thoughts?

@jportner jportner linked an issue Jun 11, 2021 that may be closed by this pull request
Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

You can never be sure with SO features, but LGTM

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
Copy link
Contributor Author

@elasticmachine merge upstream

@jportner jportner enabled auto-merge (squash) June 28, 2021 16:55
@jportner
Copy link
Contributor Author

@elasticmachine merge upstream

@jportner
Copy link
Contributor Author

@elasticmachine merge upstream

@jportner
Copy link
Contributor Author

@elasticmachine merge upstream

@jportner
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
savedObjectsManagement 78 79 +1
spaces 249 251 +2
total +3

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
core 1055 1056 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
ml 5.9MB 5.9MB +9.0B
savedObjectsManagement 138.9KB 141.9KB +3.0KB
spaces 272.8KB 279.8KB +6.9KB
total +10.0KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
core 417.2KB 417.7KB +531.0B
spaces 41.6KB 42.3KB +712.0B
total +1.2KB

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/development-plugin-saved-objects.html#_mappings

id before after diff
legacy-url-alias 4 5 +1
Unknown metric groups

API count

id before after diff
core 2284 2296 +12
spaces 96 106 +10
spacesOss 71 72 +1
total +23

History

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

@jportner jportner merged commit 1965315 into elastic:master Jun 28, 2021
@jportner jportner deleted the sharing-saved-objects-phase-3.5 branch June 28, 2021 22:23
@jportner jportner added the auto-backport Deprecated - use backport:version if exact versions are needed label Jun 28, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 28, 2021
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Jun 29, 2021
Co-authored-by: Joe Portner <5295965+jportner@users.noreply.github.com>
@jportner jportner added the Feature:Security/Sharing Saved Objects Platform Security - Sharing Saved Objects feature label Oct 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Security/Sharing Saved Objects Platform Security - Sharing Saved Objects feature release_note:skip Skip the PR/issue when compiling release notes v7.14.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sharing saved-objects in multiple spaces: phase 3.5 Deleting shared saved objects
8 participants