features plugin: improve capabilities caching (#153743)

## Summary

- Avoid constructing the capabilities passed to core's capability
provider for each request
- Add proper *explicit* lock mechanism to the feature registry
- Some other minor cleanup, probably

#### Before:

(look at the `getFeatureUICapabilities` block)

<img width="1629" alt="CleanShot 2023-03-26 at 09 55 45@2x"
src="https://user-images.githubusercontent.com/1532934/227951476-1edbd2bc-f60b-4529-8c22-ff0d9511b2b8.png">

#### After

<img width="1640" alt="CleanShot 2023-03-27 at 14 58 31@2x"
src="https://user-images.githubusercontent.com/1532934/227951533-dbe675c3-7089-4b36-86a9-584a3e97c860.png">

x-pack/plugins/features/server/feature_registry.test.ts

@@ -22,6 +22,7 @@ describe('FeatureRegistry', () => {
 
       const featureRegistry = new FeatureRegistry();
       featureRegistry.registerKibanaFeature(feature);
+      featureRegistry.lockRegistration();
       const result = featureRegistry.getAllKibanaFeatures();
       expect(result).toHaveLength(1);
 
@@ -137,6 +138,7 @@ describe('FeatureRegistry', () => {
 
       const featureRegistry = new FeatureRegistry();
       featureRegistry.registerKibanaFeature(feature);
+      featureRegistry.lockRegistration();
       const result = featureRegistry.getAllKibanaFeatures();
       expect(result).toHaveLength(1);
 
@@ -278,6 +280,7 @@ describe('FeatureRegistry', () => {
 
       const featureRegistry = new FeatureRegistry();
       featureRegistry.registerKibanaFeature(feature);
+      featureRegistry.lockRegistration();
       const result = featureRegistry.getAllKibanaFeatures();
 
       expect(result[0].privileges).toHaveProperty('all');
@@ -313,6 +316,7 @@ describe('FeatureRegistry', () => {
 
       const featureRegistry = new FeatureRegistry();
       featureRegistry.registerKibanaFeature(feature);
+      featureRegistry.lockRegistration();
       const result = featureRegistry.getAllKibanaFeatures();
 
       expect(result[0].privileges).toHaveProperty('all');
@@ -350,6 +354,7 @@ describe('FeatureRegistry', () => {
 
       const featureRegistry = new FeatureRegistry();
       featureRegistry.registerKibanaFeature(feature);
+      featureRegistry.lockRegistration();
       const result = featureRegistry.getAllKibanaFeatures();
 
       const reservedPrivilege = result[0]!.reserved!.privileges[0].privilege;
@@ -383,6 +388,7 @@ describe('FeatureRegistry', () => {
 
       const featureRegistry = new FeatureRegistry();
       featureRegistry.registerKibanaFeature(feature);
+      featureRegistry.lockRegistration();
       const result = featureRegistry.getAllKibanaFeatures();
 
       expect(result[0].privileges).toHaveProperty('all');
@@ -1645,6 +1651,7 @@ describe('FeatureRegistry', () => {
 
       const featureRegistry = new FeatureRegistry();
       featureRegistry.registerKibanaFeature(feature);
+      featureRegistry.lockRegistration();
       const result = featureRegistry.getAllKibanaFeatures();
       expect(result).toHaveLength(1);
       expect(result[0].reserved?.privileges).toHaveLength(2);
@@ -1802,7 +1809,7 @@ describe('FeatureRegistry', () => {
       `);
     });
 
-    it('cannot register feature after getAll has been called', () => {
+    it('cannot register kibana feature after lockRegistration has been called', () => {
       const feature1: KibanaFeatureConfig = {
         id: 'test-feature',
         name: 'Test Feature',
@@ -1820,6 +1827,7 @@ describe('FeatureRegistry', () => {
 
       const featureRegistry = new FeatureRegistry();
       featureRegistry.registerKibanaFeature(feature1);
+      featureRegistry.lockRegistration();
       featureRegistry.getAllKibanaFeatures();
       expect(() => {
         featureRegistry.registerKibanaFeature(feature2);
@@ -1827,6 +1835,7 @@ describe('FeatureRegistry', () => {
         `"Features are locked, can't register new features. Attempt to register test-feature-2 failed."`
       );
     });
+
     describe('#getAllKibanaFeatures', () => {
       const features: KibanaFeatureConfig[] = [
         {
@@ -1879,6 +1888,7 @@ describe('FeatureRegistry', () => {
 
       const registry = new FeatureRegistry();
       features.forEach((f) => registry.registerKibanaFeature(f));
+      registry.lockRegistration();
 
       it('returns all features and sub-feature privileges by default', () => {
         const result = registry.getAllKibanaFeatures();
@@ -1926,6 +1936,7 @@ describe('FeatureRegistry', () => {
 
       const featureRegistry = new FeatureRegistry();
       featureRegistry.registerElasticsearchFeature(feature);
+      featureRegistry.lockRegistration();
       const result = featureRegistry.getAllElasticsearchFeatures();
       expect(result).toHaveLength(1);
 
@@ -1962,6 +1973,7 @@ describe('FeatureRegistry', () => {
 
       const featureRegistry = new FeatureRegistry();
       featureRegistry.registerElasticsearchFeature(feature);
+      featureRegistry.lockRegistration();
       const result = featureRegistry.getAllElasticsearchFeatures();
       expect(result).toHaveLength(1);
 
@@ -2015,6 +2027,27 @@ describe('FeatureRegistry', () => {
         featureRegistry.registerElasticsearchFeature(feature)
       ).toThrowErrorMatchingInlineSnapshot(`"Feature with id test-feature is already registered."`);
     });
+
+    it('cannot register elasticsearch feature after lockRegistration has been called', () => {
+      const feature: ElasticsearchFeatureConfig = {
+        id: 'test-feature',
+        privileges: [
+          {
+            requiredClusterPrivileges: ['all'],
+            ui: [],
+          },
+        ],
+      };
+
+      const featureRegistry = new FeatureRegistry();
+      featureRegistry.lockRegistration();
+
+      expect(() =>
+        featureRegistry.registerElasticsearchFeature(feature)
+      ).toThrowErrorMatchingInlineSnapshot(
+        `"Features are locked, can't register new features. Attempt to register test-feature failed."`
+      );
+    });
   });
 
   it('does not allow a Kibana feature to share an id with an Elasticsearch feature', () => {

x-pack/plugins/features/server/feature_registry.ts

@@ -21,6 +21,10 @@ export class FeatureRegistry {
   private kibanaFeatures: Record<string, KibanaFeatureConfig> = {};
   private esFeatures: Record<string, ElasticsearchFeatureConfig> = {};
 
+  public lockRegistration() {
+    this.locked = true;
+  }
+
   public registerKibanaFeature(feature: KibanaFeatureConfig) {
     if (this.locked) {
       throw new Error(
@@ -58,7 +62,10 @@ export class FeatureRegistry {
   }
 
   public getAllKibanaFeatures(license?: ILicense, ignoreLicense = false): KibanaFeature[] {
-    this.locked = true;
+    if (!this.locked) {
+      throw new Error('Cannot retrieve Kibana features while registration is still open');
+    }
+
     let features = Object.values(this.kibanaFeatures);
 
     const performLicenseCheck = license && !ignoreLicense;
@@ -84,7 +91,10 @@ export class FeatureRegistry {
   }
 
   public getAllElasticsearchFeatures(): ElasticsearchFeature[] {
-    this.locked = true;
+    if (!this.locked) {
+      throw new Error('Cannot retrieve elasticsearch features while registration is still open');
+    }
+
     return Object.values(this.esFeatures).map(
       (featureConfig) => new ElasticsearchFeature(featureConfig)
     );

x-pack/plugins/features/server/mocks.ts

@@ -15,7 +15,6 @@ const createSetup = (): jest.Mocked<PluginSetupContract> => {
   return {
     getKibanaFeatures: jest.fn(),
     getElasticsearchFeatures: jest.fn(),
-    getFeaturesUICapabilities: jest.fn(),
     registerKibanaFeature: jest.fn(),
     registerElasticsearchFeature: jest.fn(),
     enableReportingUiCapabilities: jest.fn(),

x-pack/plugins/features/server/plugin.ts

@@ -14,8 +14,8 @@ import {
   Logger,
   Plugin,
   PluginInitializerContext,
+  Capabilities as UICapabilities,
 } from '@kbn/core/server';
-import { Capabilities as UICapabilities } from '@kbn/core/server';
 import { FeatureRegistry } from './feature_registry';
 import { uiCapabilitiesForFeatures } from './ui_capabilities_for_features';
 import { buildOSSFeatures } from './oss_features';
@@ -40,7 +40,9 @@ import {
  */
 export interface PluginSetupContract {
   registerKibanaFeature(feature: KibanaFeatureConfig): void;
+
   registerElasticsearchFeature(feature: ElasticsearchFeatureConfig): void;
+
   /**
    * Calling this function during setup will crash Kibana.
    * Use start contract instead.
@@ -48,6 +50,7 @@ export interface PluginSetupContract {
    * @removeBy 8.8.0
    */
   getKibanaFeatures(): KibanaFeature[];
+
   /**
    * Calling this function during setup will crash Kibana.
    * Use start contract instead.
@@ -55,7 +58,6 @@ export interface PluginSetupContract {
    * @removeBy 8.8.0
    */
   getElasticsearchFeatures(): ElasticsearchFeature[];
-  getFeaturesUICapabilities(): UICapabilities;
 
   /**
    * In the future, OSS features should register their own subfeature
@@ -80,6 +82,7 @@ export interface PluginSetupContract {
 
 export interface PluginStartContract {
   getElasticsearchFeatures(): ElasticsearchFeature[];
+
   getKibanaFeatures(): KibanaFeature[];
 }
 
@@ -92,6 +95,7 @@ export class FeaturesPlugin
   private readonly logger: Logger;
   private readonly featureRegistry: FeatureRegistry = new FeatureRegistry();
   private isReportingEnabled: boolean = false;
+  private capabilities?: UICapabilities;
 
   constructor(private readonly initializerContext: PluginInitializerContext) {
     this.logger = this.initializerContext.logger.get();
@@ -103,13 +107,8 @@ export class FeaturesPlugin
       featureRegistry: this.featureRegistry,
     });
 
-    const getFeaturesUICapabilities = () =>
-      uiCapabilitiesForFeatures(
-        this.featureRegistry.getAllKibanaFeatures(),
-        this.featureRegistry.getAllElasticsearchFeatures()
-      );
-
-    core.capabilities.registerProvider(getFeaturesUICapabilities);
+    // capabilities provider wil never be called before plugin start
+    core.capabilities.registerProvider(() => this.capabilities!);
 
     return deepFreeze({
       registerKibanaFeature: this.featureRegistry.registerKibanaFeature.bind(this.featureRegistry),
@@ -120,7 +119,6 @@ export class FeaturesPlugin
       getElasticsearchFeatures: this.featureRegistry.getAllElasticsearchFeatures.bind(
         this.featureRegistry
       ),
-      getFeaturesUICapabilities,
       enableReportingUiCapabilities: this.enableReportingUiCapabilities.bind(this),
       featurePrivilegeIterator,
       subFeaturePrivilegeIterator,
@@ -129,6 +127,12 @@ export class FeaturesPlugin
 
   public start(core: CoreStart): RecursiveReadonly<PluginStartContract> {
     this.registerOssFeatures(core.savedObjects);
+    this.featureRegistry.lockRegistration();
+
+    this.capabilities = uiCapabilitiesForFeatures(
+      this.featureRegistry.getAllKibanaFeatures(),
+      this.featureRegistry.getAllElasticsearchFeatures()
+    );
 
     return deepFreeze({
       getElasticsearchFeatures: this.featureRegistry.getAllElasticsearchFeatures.bind(

x-pack/plugins/features/server/routes/index.test.ts

@@ -126,6 +126,8 @@ describe('GET /api/features', () => {
       privileges: null,
     });
 
+    featureRegistry.lockRegistration();
+
     const routerMock = httpServiceMock.createRouter();
     defineRoutes({
       router: routerMock,