elastic · azasypkin · Jan 16, 2025 · Jan 14, 2025 · Jan 16, 2025 · azasypkin
@@ -177,6 +177,14 @@ export interface KibanaFeatureConfig {
      * documentation.
      */
     notice: string;
+    /**
+     * An optional list of feature IDs representing the features that should _conceptually_ replace this deprecated
+     * feature. This is used, for example, in the Spaces feature visibility toggles UI to display the replacement
+     * feature(s) instead of the deprecated one. By default, the list of replacement features is derived from the
+     * `replacedBy` fields of the feature privileges. However, if the feature privileges are replaced by the privileges
+     * of multiple features, this behavior is not always desired and can be overridden here.
+     */
+    replacedBy?: readonly string[];
   }>;
 }
 

@@ -2702,10 +2702,12 @@ describe('FeatureRegistry', () => {
       }
 
       function createDeprecatedFeature({
+        deprecated,
         all,
         read,
         subAlpha,
       }: {
+        deprecated?: { notice: string; replacedBy?: string[] };
         all?: FeatureKibanaPrivilegesReference[];
         read?: {
           minimal: FeatureKibanaPrivilegesReference[];
@@ -2714,7 +2716,7 @@ describe('FeatureRegistry', () => {
         subAlpha?: FeatureKibanaPrivilegesReference[];
       } = {}): KibanaFeatureConfig {
         return {
-          deprecated: { notice: 'It was a mistake.' },
+          deprecated: deprecated ?? { notice: 'It was a mistake.' },
           id: 'feature-alpha',
           name: 'Feature Alpha',
           app: [],
@@ -3240,6 +3242,50 @@ describe('FeatureRegistry', () => {
           `"Cannot replace privilege \\"sub-alpha-1-1\\" of deprecated feature \\"feature-alpha\\" with disabled privilege \\"read\\" of feature \\"feature-delta\\"."`
         );
       });
+
+      it('requires correct list of feature IDs to be replaced by', () => {
+        // Case 1: empty list of feature IDs.
+        expect(() =>
+          createRegistry(
+            createDeprecatedFeature({ deprecated: { notice: 'some notice', replacedBy: [] } })
+          ).validateFeatures()
+        ).toThrowErrorMatchingInlineSnapshot(
+          `"Feature “feature-alpha” is deprecated and must have at least one feature ID added to the “replacedBy” property, or the property must be left out completely."`
+        );
+
+        // Case 2: invalid feature IDs.
+        expect(() =>
+          createRegistry(
+            createDeprecatedFeature({
+              deprecated: {
+                notice: 'some notice',
+                replacedBy: ['feature-beta', 'feature-gamma', 'feature-delta'],
+              },
+            })
+          ).validateFeatures()
+        ).toThrowErrorMatchingInlineSnapshot(
+          `"Cannot replace deprecated feature “feature-alpha” with the following features, as they aren’t used to replace feature privileges: feature-gamma, feature-delta."`
+        );
+
+        // Case 3: valid feature ID.
+        expect(() =>
+          createRegistry(
+            createDeprecatedFeature({
+              deprecated: { notice: 'some notice', replacedBy: ['feature-beta'] },
+            })
+          ).validateFeatures()
+        ).not.toThrow();
+
+        // Case 4: valid multiple feature IDs.
+        expect(() =>
+          createRegistry(
+            createDeprecatedFeature({
+              deprecated: { notice: 'some notice', replacedBy: ['feature-beta', 'feature-delta'] },
+              all: [{ feature: 'feature-delta', privileges: ['all'] }],
+            })
+          ).validateFeatures()
+        ).not.toThrow();
+      });
     });
   });
 

@@ -21,9 +21,9 @@ import { validateKibanaFeature, validateElasticsearchFeature } from './feature_s
 import type { ConfigOverridesType } from './config';
 
 /**
- * Describes parameters used to retrieve all Kibana features.
+ * Describes parameters used to retrieve all Kibana features (for internal consumers).
  */
-export interface GetKibanaFeaturesParams {
+export interface GetKibanaFeaturesParamsInternal {
   /**
    * If provided, the license will be used to filter out features that require a license higher than the specified one.
    * */
@@ -41,6 +41,17 @@ export interface GetKibanaFeaturesParams {
   omitDeprecated?: boolean;
 }
 
+/**
+ * Describes parameters used to retrieve all Kibana features (for public consumers).
+ */
+export interface GetKibanaFeaturesParams {
+  /**
+   * If true, deprecated features will be omitted. For backward compatibility reasons, deprecated features are included
+   * in the result by default.
+   */
+  omitDeprecated: boolean;
+}
+
 export class FeatureRegistry {
   private locked = false;
   private kibanaFeatures: Record<string, KibanaFeatureConfig> = {};
@@ -207,6 +218,7 @@ export class FeatureRegistry {
 
       // Iterate over all top-level and sub-feature privileges.
       const isFeatureDeprecated = !!feature.deprecated;
+      const replacementFeatureIds = new Set();
       for (const [privilegeId, privilege] of [
         ...Object.entries(feature.privileges),
         ...collectSubFeaturesPrivileges(feature),
@@ -263,6 +275,32 @@ export class FeatureRegistry {
               );
             }
           }
+
+          replacementFeatureIds.add(featureReference.feature);
+        }
+      }
+
+      const featureReplacedBy = feature.deprecated?.replacedBy;
+      if (featureReplacedBy) {
+        if (featureReplacedBy.length === 0) {
+          throw new Error(
+            `Feature “${feature.id}” is deprecated and must have at least one feature ID added to the “replacedBy” property, or the property must be left out completely.`
+          );
+        }
+
+        // The feature can be marked as replaced by another feature only if that feature is actually used to replace any
+        // of the deprecated feature’s privileges.
+        const invalidFeatureIds = featureReplacedBy.filter(
+          (featureId) => !replacementFeatureIds.has(featureId)
+        );
+        if (invalidFeatureIds.length > 0) {
+          throw new Error(
+            `Cannot replace deprecated feature “${
+              feature.id
+            }” with the following features, as they aren’t used to replace feature privileges: ${invalidFeatureIds.join(
+              ', '
+            )}.`
+          );
         }
       }
     }
@@ -272,7 +310,7 @@ export class FeatureRegistry {
     license,
     ignoreLicense = false,
     omitDeprecated = false,
-  }: GetKibanaFeaturesParams = {}): KibanaFeature[] {
+  }: GetKibanaFeaturesParamsInternal = {}): KibanaFeature[] {
     if (!this.locked) {
       throw new Error('Cannot retrieve Kibana features while registration is still open');
     }

@@ -285,7 +285,12 @@ const kibanaFeatureSchema = schema.object({
       ),
     })
   ),
-  deprecated: schema.maybe(schema.object({ notice: schema.string() })),
+  deprecated: schema.maybe(
+    schema.object({
+      notice: schema.string(),
+      replacedBy: schema.maybe(schema.arrayOf(schema.string())),
+    })
+  ),
 });
 
 const elasticsearchPrivilegeSchema = schema.object({

@@ -23,6 +23,7 @@ export type {
 } from '../common';
 export type { SubFeaturePrivilegeIterator } from './feature_privilege_iterator';
 export { KibanaFeature, ElasticsearchFeature } from '../common';
+export type { GetKibanaFeaturesParams } from './feature_registry';
 export type { FeaturesPluginSetup, FeaturesPluginStart } from './plugin';
 
 export const config: PluginConfigDescriptor<TypeOf<typeof ConfigSchema>> = { schema: ConfigSchema };

@@ -17,7 +17,7 @@ import {
   Capabilities as UICapabilities,
 } from '@kbn/core/server';
 import { ConfigType } from './config';
-import { FeatureRegistry } from './feature_registry';
+import { FeatureRegistry, GetKibanaFeaturesParams } from './feature_registry';
 import { uiCapabilitiesForFeatures } from './ui_capabilities_for_features';
 import { buildOSSFeatures } from './oss_features';
 import { defineRoutes } from './routes';
@@ -84,7 +84,11 @@ export interface FeaturesPluginSetup {
 export interface FeaturesPluginStart {
   getElasticsearchFeatures(): ElasticsearchFeature[];
 
-  getKibanaFeatures(): KibanaFeature[];
+  /**
+   * Returns all registered Kibana features.
+   * @param params Optional parameters to filter features.
+   */
+  getKibanaFeatures(params?: GetKibanaFeaturesParams): KibanaFeature[];
 }
 
 /**
@@ -147,7 +151,10 @@ export class FeaturesPlugin
       getElasticsearchFeatures: this.featureRegistry.getAllElasticsearchFeatures.bind(
         this.featureRegistry
       ),
-      getKibanaFeatures: this.featureRegistry.getAllKibanaFeatures.bind(this.featureRegistry),
+      getKibanaFeatures: (params) =>
+        this.featureRegistry.getAllKibanaFeatures(
+          params && { omitDeprecated: params.omitDeprecated }
+        ),
     });
   }
 

@@ -80,7 +80,7 @@ describe.skip('onPostAuthInterceptor', () => {
 
     const loggingMock = loggingSystemMock.create().asLoggerFactory().get('xpack', 'spaces');
 
-    const featuresPlugin = featuresPluginMock.createSetup();
+    const featuresPlugin = featuresPluginMock.createStart();
     featuresPlugin.getKibanaFeatures.mockReturnValue([
       {
         id: 'feature-1',
@@ -163,7 +163,7 @@ describe.skip('onPostAuthInterceptor', () => {
     initSpacesOnPostAuthRequestInterceptor({
       http: http as unknown as CoreSetup['http'],
       log: loggingMock,
-      features: featuresPlugin,
+      getFeatures: async () => featuresPlugin,
       getSpacesService: () => spacesServiceStart,
     });
 

@@ -6,25 +6,25 @@
  */
 
 import type { CoreSetup, Logger } from '@kbn/core/server';
+import type { FeaturesPluginStart } from '@kbn/features-plugin/server';
 
 import type { Space } from '../../../common';
 import { addSpaceIdToPath } from '../../../common';
 import { DEFAULT_SPACE_ID, ENTER_SPACE_PATH } from '../../../common/constants';
-import type { PluginsSetup } from '../../plugin';
-import type { SpacesServiceStart } from '../../spaces_service/spaces_service';
+import type { SpacesServiceStart } from '../../spaces_service';
 import { wrapError } from '../errors';
 import { getSpaceSelectorUrl } from '../get_space_selector_url';
 import { withSpaceSolutionDisabledFeatures } from '../utils/space_solution_disabled_features';
 
 export interface OnPostAuthInterceptorDeps {
   http: CoreSetup['http'];
-  features: PluginsSetup['features'];
+  getFeatures: () => Promise<FeaturesPluginStart>;
   getSpacesService: () => SpacesServiceStart;
   log: Logger;
 }
 
 export function initSpacesOnPostAuthRequestInterceptor({
-  features,
+  getFeatures,
   getSpacesService,
   log,
   http,
@@ -38,7 +38,7 @@ export function initSpacesOnPostAuthRequestInterceptor({
 
     const spaceId = spacesService.getSpaceId(request);
 
-    // The root of kibana is also the root of the defaut space,
+    // The root of kibana is also the root of the default space,
     // since the default space does not have a URL Identifier (i.e., `/s/foo`).
     const isRequestingKibanaRoot = path === '/' && spaceId === DEFAULT_SPACE_ID;
     const isRequestingSpaceRoot = path === '/' && spaceId !== DEFAULT_SPACE_ID;
@@ -106,7 +106,10 @@ export function initSpacesOnPostAuthRequestInterceptor({
         }
       }
 
-      const allFeatures = features.getKibanaFeatures();
+      // The Spaces client returns migrated feature IDs in `disabledFeatures`, so we need to omit
+      // deprecated features. Otherwise, apps granted by deprecated features will be considered
+      // available when they shouldn't be, since their IDs won't be present in `disabledFeatures`.
+      const allFeatures = (await getFeatures()).getKibanaFeatures({ omitDeprecated: true });
       const disabledFeatureKeys = withSpaceSolutionDisabledFeatures(
         allFeatures,
         space.disabledFeatures,

@@ -191,7 +191,7 @@ export class SpacesPlugin
       http: core.http,
       log: this.log,
       getSpacesService,
-      features: plugins.features,
+      getFeatures: async () => (await core.getStartServices())[1].features,
     });
 
     setupCapabilities(core, getSpacesService, this.log);

@@ -86,6 +86,37 @@ const features = [
       },
     },
   },
+  {
+    deprecated: { notice: 'It was another mistake.', replacedBy: ['feature_2'] },
+    id: 'feature_5_deprecated',
+    name: 'Another deprecated Feature',
+    app: ['feature2', 'feature3'],
+    catalogue: ['feature2Entry', 'feature3Entry'],
+    category: { id: 'deprecated', label: 'deprecated' },
+    scope: ['spaces', 'security'],
+    privileges: {
+      all: {
+        savedObject: { all: [], read: [] },
+        ui: [],
+        app: ['feature2', 'feature3'],
+        catalogue: ['feature2Entry', 'feature3Entry'],
+        replacedBy: [
+          { feature: 'feature_2', privileges: ['all'] },
+          { feature: 'feature_3', privileges: ['all'] },
+        ],
+      },
+      read: {
+        savedObject: { all: [], read: [] },
+        ui: [],
+        app: ['feature2', 'feature3'],
+        catalogue: ['feature2Entry', 'feature3Entry'],
+        replacedBy: [
+          { feature: 'feature_2', privileges: ['read'] },
+          { feature: 'feature_3', privileges: ['read'] },
+        ],
+      },
+    },
+  },
 ] as unknown as KibanaFeature[];
 const featuresStart = featuresPluginMock.createStart();
 
@@ -135,7 +166,7 @@ describe('#getAll', () => {
       },
     },
     {
-      // alpha only has deprecated disabled features
+      // alpha has deprecated disabled features
       id: 'alpha',
       type: 'space',
       references: [],
@@ -145,6 +176,17 @@ describe('#getAll', () => {
         disabledFeatures: ['feature_1', 'feature_4_deprecated'],
       },
     },
+    {
+      // beta has deprecated disabled features with specified `replacedBy` on feature level
+      id: 'beta',
+      type: 'space',
+      references: [],
+      attributes: {
+        name: 'beta-name',
+        description: 'beta-description',
+        disabledFeatures: ['feature_1', 'feature_5_deprecated'],
+      },
+    },
   ];
 
   const expectedSpaces: Space[] = [
@@ -178,6 +220,12 @@ describe('#getAll', () => {
       description: 'alpha-description',
       disabledFeatures: ['feature_1', 'feature_2', 'feature_3'],
     },
+    {
+      id: 'beta',
+      name: 'beta-name',
+      description: 'beta-description',
+      disabledFeatures: ['feature_1', 'feature_2'],
+    },
   ];
 
   test(`finds spaces using callWithRequestRepository`, async () => {

@@ -301,6 +301,14 @@ export class SpacesClient implements ISpacesClient {
         continue;
       }
 
+      // If the feature is deprecated and replacement features are explicitly defined, use them.
+      // Otherwise, use the replacement features defined in the feature privileges.
+      const featureReplacedBy = feature.deprecated?.replacedBy;
+      if (featureReplacedBy) {
+        deprecatedFeatureReferences.set(feature.id, new Set(featureReplacedBy));
+        continue;
+      }
+
       // Collect all feature privileges including the ones provided by sub-features, if any.
       const allPrivileges = Object.values(feature.privileges ?? {}).concat(
         feature.subFeatures?.flatMap((subFeature) =>