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

[Discuss] New Management Sections API #43499

Closed
lukeelmers opened this issue Aug 16, 2019 · 8 comments
Closed

[Discuss] New Management Sections API #43499

lukeelmers opened this issue Aug 16, 2019 · 8 comments
Assignees
Labels
discuss Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages

Comments

@lukeelmers
Copy link
Member

lukeelmers commented Aug 16, 2019

Background

Management is one of the four primary "domains" covered by @elastic/kibana-app-arch (along with Data, Embeddables, and Visualizations). There are two main purposes for this service:

  1. Own the management "framework" -- the UI that displays the management sidebar nav, the landing page, and handles rendering each of the sections
  2. Expose a registry for other plugins to add their own registry sections to the UI and add nested links to them in the sidebar.

Purpose

The purpose of this discussion is to consider item 2 above -- the service for registering sections to the nav & loading them up. Below is a proposed design for how this would look in the new platform. The reason for having this discussion now is because some plugins will be blocked on fully moving to the new platform by the management plugin.

Design Goals

  • Remove another usage of IndexedArray & uiRegistry (required for migration)
  • Remove dependency on ui/routes (required for migration)
  • Remove management section uiExport (required for migration)
  • Simple API that is designed in keeping with new platform principles
    • This includes being rendering-framework-agnostic... You should be able to build your management section UI however you'd like
  • Clear separation of app/UI code and service code, even if both live within the same plugin
  • Flexibility to potentially support alternate layouts in the future (see mockups in reference section below)

Reference

Overview of the legacy service

Example usage of how this looks today:

// myplugin/index
new Kibana.Plugin({
  uiExports: {
    managementSections: ['myplugin/management'],
  }
});
 
// myplugin/public/management
import { management } from 'ui/management';
 
// completely new section
const newSection = management.register('mypluginsection', {
  name: 'mypluginsection',
  order: 10,
  display: 'My Plugin',
  icon: 'iconName',
});
newSection.register('mypluginlink', {
  name: 'mypluginlink',
  order: 10,
  display: 'My sublink',
  url: `#/management/myplugin`,
});
 
// new link in existing section
const kibanaSection = management.getSection('kibana');
kibanaSection.register('mypluginlink', {
  name: 'mypluginlink',
  order: 10,
  display: 'My sublink',
  url: `#/management/myplugin`,
});
 
// use ui/routes to render component
import routes from 'ui/routes';
 
const renderReact = (elem) => {
  render(<MyApp />, elem);
};
 
routes.when('management/myplugin', {
  controller($scope, $http, kbnUrl) {
    $scope.$on('$destroy', () => {
      const elem = document.getElementById('usersReactRoot');
      if (elem) unmountComponentAtNode(elem);
    });
    $scope.$$postDigest(() => {
      const elem = document.getElementById('usersReactRoot');
      const changeUrl = (url) => {
        kbnUrl.change(url);
        $scope.$apply();
      };
      renderReact(elem, $http, changeUrl);
    });
  },
});

Current public contracts owned by the legacy service:

// ui/management/index
interface API {
  PAGE_TITLE_COMPONENT: string; // actually related to advanced settings?
  PAGE_SUBTITLE_COMPONENT: string; // actually related to advanced settings?
  PAGE_FOOTER_COMPONENT: string; // actually related to advanced settings?
  SidebarNav: React.SFC<any>;
  registerSettingsComponent: (
    id: string,
    component: string | React.SFC<any>,
    allowOverride: boolean
  ) => void;
  management: new ManagementSection();
  MANAGEMENT_BREADCRUMB: {
    text: string;
    href: string;
  };
}

// ui/management/section
class ManagementSection {
  get visibleItems,
  addListener: (fn: function) => void,
  register: (id: string, options: Options) => ManagementSection | Error,
  deregister: (id: string) => void,
  hasItem: (id: string) => boolean,
  getSection: (id: string) => ManagementSection,
  hide: () => void,
  show: () => void,
  disable: () => void,
  enable: () => void,
}
 
interface Options {
  order: number | null;
  display: string | null; // defaults to id
  url: string | null; // defaults to ''
  visible: boolean | null; // defaults to true
  disabled: boolean | null; // defaults to false
  tooltip: string | null; // defaults to ''
  icon: string | null; // defaults to ''
}

Proposed API

This API is influenced heavily by the design of Core's application service mounting. The intent is to make the experience consistent with that service; the Management section is basically one big app with a bunch of registered "subapps".

Example usage:

// my_plugin/public/plugin.ts

export class MyPlugin {
  setup(core, { management }) {
    management.sections.register({
      id: 'my-section',
      title: 'My Main Section', // display name
      description: 'hello', // not used in current UI, but possibly in future
      order: 10,
      euiIconType: 'iconName',
    });
    management.sections.registerLink('my-section', {
      id: 'my-link',
      title: 'My Link', // display name
      description: 'hello', // not used in current UI, but possibly in future
      order: 20,
      async mount(context, params) {
        const { renderApp } = await import('./my-section');
        return renderApp(context, params);
      }
    });
  }
}
 
// my_plugin/public/my-section.tsx

export function renderApp(context, { basePath, element }) {
  ReactDOM.render(
    // `basePath` would be `/app/management/my-section/my-link`
    <MyApp basename={basePath} />,
    element
  );
 
  // return value must be a function that unmounts (just like Core Application Service)
  return () => ReactDOM.unmountComponentAtNode(element);
}

Detailed design:

interface ManagementSetup {
  sections: SectionsService;
}
 
interface SectionsService {
  register: Register;
  registerLink: RegisterLink;
  getAvailable: () => Section[]; // filtered based on capabilities
  get: (id: string) => Section | undefined;
  getLink: (id: string) => Link | undefined;
}
 
type Register = ({
  id: string;
  title: string;
  description: string; // not used in current UI, but possibly in future
  order?: number;
  euiIconType?: string; // takes precedence over `icon` property.
  icon?: string; // URL to image file; fallback if no `euiIconType`
}) => Section | Error;
 
type RegisterLink = (sectionId: string, {
  id: string;
  title: string;
  description: string; // not used in current UI, but possibly in future
  order?: number;
  mount: ManagementSectionMount;
}) => Link | Error;
 
type Unmount = () => Promise<void> | void;
 
type ManagementSectionMount = (
  context: AppMountContext, // provided by core.ApplicationService
  params: MountParams,
) => Unmount | Promise<Unmount>;
 
interface MountParams {
  basePath: string; // base path for setting up your router
  element: HTMLElement; // element the section should render into
}
 
interface Section {
  id: string;
  title: string;
  description: string;
  baseBath: string;
  links: Link[];
  order?: number;
  euiIconType?: string;
  icon?: string;
  destroy: () => Promise<void> | void; // de-registers and destroys all links
}
 
interface Link {
  id: string;
  title: string;
  description: string;
  basePath: string;
  sectionId: string;
  order?: number;
  destroy: () => Promise<void> | void; // de-registers & calls unmount()
}

Caveats, Questions, & Discussion Points:

  • This removes the ability to infinitely nest sections within each other by making a distinction between a section header and a nav link.
    • So far we didn't seem to be using this feature anyway, but would like feedback on any use cases for it.
  • The hide/show/disable/enable options were dropped with the assumption that we will be working with uiCapabilities to determine this instead... so people shouldn't need to manage it manually as they can look up a pre-filtered list of sections.
  • This was updated to add flexibility for custom (non-EUI) icons as outlined in Allow custom icons for management sections #32661. Much like the Core Application Service, you either choose an EUI icon, or provide a URL to an icon.
  • Do individual links within a section need the ability to specify icons? Currently only the section headers have icons.
  • Added field for a description, though this isn't currently used in the UI.

Next Steps

We will keep this issue open for feedback for one week (until August 23, 2019), at which point we will seek any final comments before closing the discussion in order to make plans for future implementation.


Copying a few folks here who either have knowledge of the legacy management framework, or who are actively managing plugins that register management sections:

@bmcconaghy @cjcenizal @jen-huang @mattkime @kobelb @legrego @alvarezmelissa87 @peteharverson @mattapperson @stacey-gammon

@lukeelmers lukeelmers added discuss Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages Team:AppArch labels Aug 16, 2019
@lukeelmers lukeelmers self-assigned this Aug 16, 2019
@epixa
Copy link
Contributor

epixa commented Aug 16, 2019

I am out next week so won’t be able to give you comprehensive feedback, but at a high level this sounds great. The app mounting pattern seems super appropriate here, and if you use the context service and such (as the app service does), then you’ll get management page level extensibility as well.

My one suggestion is to re-do this issue as a formal RFC PR. The platform team has had success with that format for its service design, it allows for discussions on individual details of the proposal rather than only a single flat thread of comments, and it becomes a sort of permanent documentation to guide development and allow folks to understand how it works and why it works the way it does long after the actual change goes in.

@cjcenizal
Copy link
Contributor

The Management product vision

CC @AlonaNadler @alexfrancoeur @arisonl @skearns64

Historically the Management section has been a kitchen sink. Because we haven't had a clear definition of its purpose or what should go into it, it's accumulated apps over time:

alt text

Long-term, the ES UI is planning on grouping its apps together into domain-specific "super-apps" (e.g. a Cluster app for cluster management) which will live in the nav drawer, and not in Management.

I could imagine something similar done with the other apps here, e.g. an Ingest app that contains Beats Management and Logstash Pipelines, a Security app, perhaps ML jobs could be moved into the ML app, and then you'd just be left with a Kibana Management app. This would be a breaking change for third party apps that ended up in Management, so we would want to target this change for 8.0.

The big question is: what's our long-term product vision for the Management app? If it's along the lines of what I've suggested, then we could redirect our efforts into attaining that vision instead of rebuilding Management to support the state it's in now. This would save the AppArch team a lot of work, shifting some work to other teams, and it would advance Kibana as a product.

@cjcenizal
Copy link
Contributor

cjcenizal commented Aug 17, 2019

As an aside that's a bit contradictory to my above suggestion, Dev Tools functions in a similar way to Management, where it's essentially a shell app with a navigation component that can route to any child app that get registered. It would be amazing if we could create a generic app with this functionality that we could reuse in both places (and possibly others). Even some reusable code (components and services) would be helpful.

@kobelb
Copy link
Contributor

kobelb commented Aug 19, 2019

@cjcenizal some of what you've discussed starts to overlap with #37285 and #37283. /cc @peterschretlen

@lukeelmers
Copy link
Member Author

It would be amazing if we could create a generic app with this functionality that we could reuse in both places (and possibly others)

That's an interesting point; I'll definitely think about whether there's an easy way to separate "subapp mounting" concerns from management-specific ones. I'm sure there's some stuff in there that could be reuseable.

we could redirect our efforts into attaining that vision instead of rebuilding Management to support the state it's in now.

I totally agree @cjcenizal, I think keeping an eye on the long-term product goals is important, and the design here is taking into consideration the links that @kobelb posted above.

One question I probably should have answered in more detail is why now?

The main driver for considering this now is that the Management API moving to the new platform is going to block other teams from completing migration, so we need to have an answer to what the new platform version of the API looks like as soon as possible in 7.x. For example, I believe this is the only thing currently blocking Spaces from completing migration to NP.

The other question to answer is why not just keep the current API and defer this question until later?

The answer to that has to do with the items that are currently used in the management implementation which must be removed in order to migrate to NP: the framework currently registers a uiExport, and relies on IndexedArray, uiRegistry, and ui/routes.

This means that we will basically need to rebuild the service anyway in order to migrate to the new platform. So if we are going to invest that time, we might as well invest it in building the API the way we want it to be longer term, rather than creating more work for ourselves later.

My one suggestion is to re-do this issue as a formal RFC PR.

Thanks @epixa, I'll see if I can open a PR using that format instead. In the meantime we can keep the conversation flowing here; I'll post a link to the RFC PR when it is ready.

@lukeelmers
Copy link
Member Author

I also have some thoughts around product use-cases for management vs "super-apps", but that's probably a discussion for a different thread.

@AlonaNadler
Copy link

Long-term, the ES UI is planning on grouping its apps together into domain-specific "super-apps" (e.g. a Cluster app for cluster management) which will live in the nav drawer, and not in Management.

I agree with @cjcenizal that our management is currently a kitchen sink for all capabilities related to stack management alongside space setting and reporting (which is per user), this situation is not ideal and we should definitely improve it.
Separating this to multiple dedicated apps in navigation (one for ES, one for Beats or ingest etc..) is not ideal. At most companies only a very few people manage the stack ( typically 1-3 people), commonly these folks are the ones who manage the indices but also manage beats configuration and responsible for the use-role configuration. Separating them to different apps will not benefit them but will cause them to move around Kibana and loss context. It will be better to keep the tools to manage the stack in common application and make it more accessible than it currently is.

In the past, we briefly bounced ideas about the admin space concept but there was never an urgency so it was postponed. Perhaps now with management API and the new platform, there is a forcing function to decide that.

From my perspective, the management app has several goals:

  • the management application should be a place to manage kibana currently it is a combination of multiple things, as we add more and more to it, it makes it more confusing and harder to find what users are actually looking for. Hence users setting and users' reports/alerts should be somewhere else (perhaps under the user avatar).
  • As @lukeelmers mentioned,expose a registry for other plugins to add their own registry sections to the UI and add nested links to them
  • When working with Security, management app should be hidden to regular users, we currently have a few challenges with that mainly around cluster privileges, admins cannot hide/restrict management application from end users, it makes an odd situation where every user sees all the capabilities that require cluster privileges - privileges which they don't have. Hiding management is also one of the common expectations of feature control.
  • Management should have an aspect of space-specific vs global - at minimum the clarity for admins which pages are global vs space-specific, having space-specific items next to global cause a situation that admins move from space to space in order to configure specific things while other management capabilities are global and effect all of kibana. Long term preferably admins can manage everything from one place without the need to move around between spaces, copy to space will improve that but will not solve this issue.
  • Ability to find and use new capabilities for management of the stack, the list under management is getting long and perhaps there are better ways to organize it based on value or functionality instead of only the stack components (e.g. backup, storage etc...) and highlight new capabilities that were added

@lukeelmers
Copy link
Member Author

Thanks for the input @AlonaNadler! Based on the goals you have described, it seems this proposal is still mostly in line with them, since the scope is really only related to that second bullet point.

Management should have an aspect of space-specific vs global

This is the one thing we might want to consider, especially if it is something we envision happening nearer-term (by 8.0). If we can reach a decision now on how we want to approach this, we could incorporate the concept of space-specific vs global in the design of the registry.

If we think it's a longer-term thing (8.x and beyond), we can always move forward building this to be as flexible as possible, so that this can be added as an enhancement sometime down the road.

This topic might warrant a separate offline discussion with app arch, security, product, and whomever else is interested.

I'm going to go ahead and close this issue now, as a proper RFC has been opened which should make reviewing the design easier. Let's continue this conversation over there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages
Projects
None yet
Development

No branches or pull requests

5 participants