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

[Infra UI] Support Feature Controls for the Infrastructure and Logs UIs #31287

Closed
weltenwort opened this issue Feb 15, 2019 · 16 comments
Closed
Labels
Feature:Logs UI Logs UI feature Feature:Metrics UI Metrics UI feature Meta Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services

Comments

@weltenwort
Copy link
Member

The next phase of the RBAC effort includes the addition of Feature Controls (#20277), which allow for assignment of read/write permissions to features of Kibana and its plugins. The enforcement of these permissions takes place on several levels:

  • the UI via explicit checks that need to be added
  • the HTTP API via explicit checks that need to be added
  • the SavedObjectsClient internally
  • possibly loading of javascript bundles

Due to the facts that both the Infrastructure UI and Logs UI reside in the same plugins and furthermore share the same saved object type for their settings, implementing these Feature Controls properly require some changes. Two implementation options come to mind:

Option 1: Split the saved objects and GraphQL fields

  • The settings need to be split into separate saved object types for the Infrastructure UI feature and the Logs UI feature.
  • The graphql API needs to separate top-level fields for metric sources and log sources to enforce the appropriate permission checks.
  • The UIs each need to account for cases when the the user doesn't have permissions to...
    • access the respective part of the plugin by displaying an appropriate message
    • read the corresponding saved object by displaying an appropriate message
    • write the corresponding saved object by disabling changing of the source configuration

Option 2: Split off a separate logs plugin

  • The infra and logs plugins need to each define their own saved object types.
  • The infra and logs plugins need to each provide their own graphql API in which the source accesses their respective saved object type.
  • The UIs would each be disabled at the topmost routing level if the proper permissions for each were missing.
  • The UIs would have to account for a lack of write permissions to the saved objects by disabling changing of the source configuration.

I would judge the initial effort for both to be about the same.

Open questions

How can commonly used client- and server-side code be shared between the two plugins to avoid unnecessary duplication?

This is already a problem in regard to other plugins like apm, siem or beats-cm. In the context of the infra plugin the sharing would have to cover:

  • react components
  • client-side functions
  • server-side functions
@weltenwort weltenwort added discuss Meta [zube]: Backlog Feature:Metrics UI Metrics UI feature Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.2.0 labels Feb 15, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/infrastructure-ui

@weltenwort
Copy link
Member Author

/cc @makwarth @roncohen

@weltenwort
Copy link
Member Author

After some discussion it seems that we should go with option 1 without excluding option 2 in the future. Effectively this means:

  • Create a new saved object type for the log source configuration, which is similar to the current infra-ui configuration but without metric-specific fields.
  • Expose the two source configuration types via the GraphQL API.
  • Split the container into a metrics source and a logs source container which each access the corresponding GraphQL API.
  • Split the source configuration ui to edit the correct source configuration type.

@roncohen
Copy link
Contributor

sounds good, thanks @weltenwort

@tbragin
Copy link
Contributor

tbragin commented Jun 26, 2019

@weltenwort I saw that Feature Controls for all Infrastructure and Logs apps were added by @kobelb already: #31287 (thank you!!)

Can we close this issue, or is there something remaining to be done?

cc @roncohen @jasonrhodes @sgrodzicki

@weltenwort
Copy link
Member Author

@tbragin as far as the saved object permissions so, the Infrastructure and Logs UIs still share permissions. At the time I created this issue the flavor of the day was to have them both as independent as possible, which is why I proposed the two options. So this aims at full support of the feature controls as opposed to the limited support implemented so far.

@tbragin
Copy link
Contributor

tbragin commented Jun 26, 2019

@weltenwort Thanks for the quick response. What does "full" support mean vs the "limited" support we have in 7.2?

@weltenwort
Copy link
Member Author

The Infra UI and the Logs UI share the source configurations, so having permission to read or write the configuration in one of the them means having the ability to change the configuration of the other.

The HTTP API is similarly shared and all the endpoints can be accessed if permissions to access either of them is given.

@jasonrhodes
Copy link
Member

Just out of curiosity, could the feature control be constructed so that you can only turn on/off "infra" as a whole, meaning metrics and logging, since they reside in the same plugin? This may not be an optimal best experience, but I wonder if it solves the more buggy parts of the problems outlined above without needing to change much code until we better understand how we are going to construct these plugins/apps going forward.

In the end I think this work is the right thing to do so I'm all for separating things out. Just curious if we have a backup plan here if we need it.

@kobelb
Copy link
Contributor

kobelb commented Jul 1, 2019

could the feature control be constructed so that you can only turn on/off "infra" as a whole, meaning metrics and logging, since they reside in the same plugin?

This is theoretically possible, but since we've already shipped Kibana with these as separate features, it becomes more complicated at this point in time. We'd have to figure out how we'd want this to behave with spaces and roles created with the separate features.

@weltenwort
Copy link
Member Author

IIRC treating both UIs as one "feature" was considered at that time, but we chose to introduce separate permissions because that aligned better with the vision at that time.

@jasonrhodes
Copy link
Member

OK, thanks for the clarification, @kobelb and @weltenwort -- that makes sense.

We should rewrite this ticket now so that it just outlines the chosen option (or make a new ticket that does that and references this issue, if we want to preserve the discussion around those options).

@jasonrhodes
Copy link
Member

I think the answer here is for us to definitely move forward with "Option 1: Split the saved objects and GraphQL fields", so that the two apps are decoupled. Whether we split them into two separate Kibana plugins or not won't matter, either way we want to have de-coupled apps that don't share state like this.

@jasonrhodes
Copy link
Member

@weltenwort do you think this ticket is still relevant? I know we've done a lot to split the two apps but I don't know if we ever got so far as to de-tangle their RBAC permissions.

@weltenwort
Copy link
Member Author

Mostly yes 😇 Since we're still sharing the source config saved objects the permissions still overlap. Except for the GraphQL part this still applies.

@smith
Copy link
Contributor

smith commented Feb 27, 2023

I don't think we need to do anything here at this time.

@smith smith closed this as not planned Won't fix, can't repro, duplicate, stale Feb 27, 2023
@zube zube bot closed this as completed Feb 27, 2023
@zube zube bot removed the [zube]: Done label May 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Logs UI Logs UI feature Feature:Metrics UI Metrics UI feature Meta Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services
Projects
None yet
Development

No branches or pull requests

8 participants