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

[Upgrade Assistant] Provide ways to surface deprecation warnings for multiple spaces #127341

Closed
TinaHeiligers opened this issue Mar 9, 2022 · 10 comments
Labels
enhancement New value added to drive a business result Feature:Upgrade Assistant impact:needs-assessment Product and/or Engineering needs to evaluate the impact of the change. loe:needs-research This issue requires some research before it can be worked on or estimated Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more

Comments

@TinaHeiligers
Copy link
Contributor

Access to the Upgrade Assistant UI isn't restricted to administrators, meaning that a user visiting the UA may not be authorized to read/write objects across all spaces.

If a user only has access to a subset of items and spaces, then any deprecations they handle from within that space won't be applied to all spaces for all users.

This can be misguiding because the user wouldn't know that the deprecation still applies in other spaces.

Ideally, we should provide a way to resolve a deprecation across all spaces but that would mean having to restrict access to the UA to admins only, or roles that have read/write privileges across all spaces and saved objects.

As a first step, we should investigate ways of surfacing a warning when a deprecation applies to more than one space.

@TinaHeiligers TinaHeiligers added enhancement New value added to drive a business result loe:needs-research This issue requires some research before it can be worked on or estimated Feature:Upgrade Assistant impact:needs-assessment Product and/or Engineering needs to evaluate the impact of the change. labels Mar 9, 2022
@botelastic botelastic bot added the needs-team Issues missing a team label label Mar 9, 2022
@TinaHeiligers
Copy link
Contributor Author

cc @elastic/kibana-core for visibility and to discuss our options for supporting this from the deprecations-service.

@TinaHeiligers TinaHeiligers added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more labels Mar 9, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/platform-deployment-management (Team:Deployment Management)

@botelastic botelastic bot removed the needs-team Issues missing a team label label Mar 9, 2022
@TinaHeiligers TinaHeiligers added needs-team Issues missing a team label and removed Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Mar 9, 2022
@botelastic botelastic bot removed the needs-team Issues missing a team label label Mar 9, 2022
@alisonelizabeth
Copy link
Contributor

Thanks @TinaHeiligers for starting this discussion!

The Upgrade Assistant currently only requires that the user has the manage cluster privilege. Other parts of the UI may not work without certain index privileges (i.e., deprecation logs).

Ideally, we should provide a way to resolve a deprecation across all spaces but that would mean having to restrict access to the UA to admins only, or roles that have read/write privileges across all spaces and saved objects.

I think we should consider restricting access to UA. I imagine only admins would be responsible for the upgrade process, and it seems like we potentially put a user with insufficient privileges in a bad situation by letting them think they are ready to upgrade, when in reality they are not.

As a first step, we should investigate ways of surfacing a warning when a deprecation applies to more than one space.

Would we be able to surface a warning though, if a user doesn't have sufficient privileges?

Per @jportner:

However, if a user doesn't have access to All Spaces, they wouldn't see deprecations for affected saved objects in spaces they don't have access to.

@jportner
Copy link
Contributor

jportner commented Mar 9, 2022

I think we should consider restricting access to UA. I imagine only admins would be responsible for the upgrade process, and it seems like we potentially put a user with insufficient privileges in a bad situation by letting them think they are ready to upgrade, when in reality they are not.

Agreed.

Would we be able to surface a warning though, if a user doesn't have sufficient privileges?

Basically what we want to do is to check if the user has the global All base privilege -- that means they have access to all Kibana features in all spaces.
The kibana_admin role has this base privilege.

However, you can't check if a user has a base privilege (that's basically a set of individual privileges), you have to check if they have access to a specific privilege - what we call a "privilege action".

You could check to see if the user is allowed to manage Spaces, which they are only allowed to do if they are a global admin.
Or, you could add a separate privilege for the upgrade assistant.
The global All base privilege is defined here:

global: {
all: [
actions.login,
actions.version,
actions.api.get('decryptedTelemetry'),
actions.api.get('features'),
actions.api.get('taskManager'),
actions.space.manage,
actions.ui.get('spaces', 'manage'),
actions.ui.get('management', 'kibana', 'spaces'),
actions.ui.get('catalogue', 'spaces'),
actions.ui.get('enterpriseSearch', 'all'),
...allActions,
],

Here's an example where we prevent the Space Management page from loading if the user doesn't have the spaces.manage privilege action for the UI (AKA UI capability):

public async componentDidMount() {
if (!this.props.capabilities.spaces.manage) {
return;
}

There are other privilege actions registered (management.kibana.spaces, catalogue.spaces) to hide the links to the Space Management page too.

@pgayvallet
Copy link
Contributor

pgayvallet commented Mar 9, 2022

we should provide a way to resolve a deprecation across all spaces

Do we have examples of deprecations that are, or should be, space-specific? config type deprecations are not, by nature, so I'm assuming it's some feature type deprecations?

And to discuss our options for supporting this from the deprecation service

The deprecation service is just the global registry of each registered deprecations. The actual deprecations logic is managed by the owning plugin that registered the associated deprecation provider.

I don't think we can do anything ourselves within this service. The best we can do is properly restrict the deprecation endpoint's access to administrators (NOTE: we can do that exclusively via the access tag on the route definition, as the route is registered from core which cannot use the security plugin's API) to make sure that the scoped clients provided via the GetDeprecationsContext to deprecation providers have the correct permissions to have unrestricted access to all spaces.

But the logic to resolve (and, eventually, fix) deprecations for 'all spaces' will still have to be manually implemented in the individual deprecation providers.

As a first step, we should investigate ways of surfacing a warning when a deprecation applies to more than one space

Following my previous point, we could add an optional impactedSpaces (or any better name) to BaseDeprecationDetails, to allow deprecation providers to eventually specify the space(s) impacted by the deprecation when the deprecation is 'space-scoped'

@jportner
Copy link
Contributor

jportner commented Mar 9, 2022

Do we have examples of deprecations that are, or should be, space-specific? config type deprecations are not, by nature, so I'm assuming it's some feature type deprecations?

Copying my response in an internal repo:

It appears that the linked issue is concerning a deprecation for the infrastructure-ui-source saved object type. I'm not aware of any other deprecations for saved objects that require some manual intervention like this, seems like a one-off.

That saved object type is not global, it is isolated to a single space, and the deprecation code only attempts to find objects in the active space (getInfraDeprecationsFactory -> getAllSavedSourceConfigurations)

This deprecation could be changed to take all spaces into account -- in this case, the call to savedObjectsClient.find can be changed to use the namespaces: ['*'] option. However, if a user doesn't have access to All Spaces, they wouldn't see deprecations for affected saved objects in spaces they don't have access to.


I don't think we can do anything ourselves within this service. The best we can do is properly restrict the deprecation endpoint's access to administrators (NOTE: we can do that exclusively via the access tag on the route definition, as the route is registered from core which cannot use the security plugin's API) to make sure that the scoped clients provided via the GetDeprecationsContext to deprecation providers have the correct permissions to have unrestricted access to all spaces.

We can restrict the endpoint access, yes, but I think it's more important to restrict the UI in a meaningful way so that users know they need an admin account to properly use the Upgrade Assistant.

But the logic to resolve (and, eventually, fix) deprecations for 'all spaces' will still have to be manually implemented in the individual deprecation providers.

Agreed.

Following my previous point, we could add an optional impactedSpaces (or any better name) to BaseDeprecationDetails, to allow deprecation providers to eventually specify the space(s) impacted by the deprecation when the deprecation is 'space-scoped'

I like this idea 😄

I'm curious to see if any other consumers are impacted by this though.

@pgayvallet
Copy link
Contributor

We can restrict the endpoint access, yes, but I think it's more important to restrict the UI in a meaningful way

Sorry for being unclear. Restricting the UI is the priority of course. Limiting the endpoint's access to the same roles/permission for consistency would be great.

@alisonelizabeth
Copy link
Contributor

Thanks @jportner and @pgayvallet!

It sounds like there are 3 potential action items here:

  1. Restrict access to UA based on the guidelines in [Upgrade Assistant] Provide ways to surface deprecation warnings for multiple spaces #127341 (comment).
  2. Evaluate the current deprecations and determine if there are any others besides the mentioned infra deprecation that are space-specific. Since this is managed by each plugin owner, perhaps it’s best to send out an email regarding this?
  3. [Nice to have] Restrict access to the core deprecations endpoint.

Does that sound right to everyone? I can open up separate issues to track this work if we're all in agreement.

@jportner
Copy link
Contributor

jportner commented Mar 10, 2022

Yeah I think that makes sense

edit: whoops I hit the wrong comment button!

@alisonelizabeth
Copy link
Contributor

I've opened the following issues based on the discussion above:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:Upgrade Assistant impact:needs-assessment Product and/or Engineering needs to evaluate the impact of the change. loe:needs-research This issue requires some research before it can be worked on or estimated Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more
Projects
None yet
Development

No branches or pull requests

5 participants