Merge pull request #30546 from backstage/rugvip/do-you-have-a-flag

catalog-backend: switch flags to new default for 3.0

.changeset/clever-windows-judge.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-backend': major
+---
+
+**BREAKING**: The default `catalog.stitchingStrategy` has been switched to `{ mode: 'deferred' }`.

.changeset/cute-sloths-smile.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-backend': major
+---
+
+**BREAKING**: The relations compatibility mode is no longer enabled by default, and the `disableRelationsCompatiblity` flag has been removed. To re-enable relations compatibility, the new `enableRelationsCompatibility` flag can be used instead.

.changeset/loud-trains-take.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-backend': major
+---
+
+**BREAKING**: The default `catalog.orphanStrategy` has been switched to `'delete'`.

.changeset/silent-gifts-retire.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-backend': major
+---
+
+**BREAKING**: The default `catalog.orphanProviderStrategy` has been switched to `'delete'`.

plugins/catalog-backend/config.d.ts

@@ -139,14 +139,14 @@ export interface Config {
     }>;
 
     /**
-     * Disables the compatibility layer for relations in returned entities that
+     * Enables the compatibility layer for relations in returned entities that
      * ensures that all relations objects have both `target` and `targetRef`.
      *
-     * Enabling this option significantly reduces the memory usage of the
-     * catalog, and slightly increases performance, but may break consumers that
+     * Enabling this option significantly increases the memory usage of the
+     * catalog, and slightly reduces performance, but may avoid breaking consumers that
      * rely on the existence of `target` in the relations objects.
      */
-    disableRelationsCompatibility?: boolean;
+    enableRelationsCompatibility?: boolean;
 
     /**
      * Disables the default backstage processors.
@@ -160,19 +160,19 @@ export interface Config {
     /**
      * The strategy to use for entities that are orphaned, i.e. no longer have
      * any other entities or providers referencing them. The default value is
-     * "keep".
+     * "delete".
      */
     orphanStrategy?: 'keep' | 'delete';
 
     /**
      * The strategy to use for entities that are referenced by providers that are orphaned,
      * i.e. entities with no providers currently configured in the catalog. The default value is
-     * "keep".
+     * "delete".
      */
     orphanProviderStrategy?: 'keep' | 'delete';
 
     /**
-     * The strategy to use when stitching together the final entities.
+     * The strategy to use when stitching together the final entities. The default mode is "deferred".
      */
     stitchingStrategy?:
       | {

plugins/catalog-backend/src/processing/DefaultCatalogProcessingEngine.ts

@@ -344,7 +344,7 @@ export class DefaultCatalogProcessingEngine {
 
   private startOrphanCleanup(): () => void {
     const orphanStrategy =
-      this.config.getOptionalString('catalog.orphanStrategy') ?? 'keep';
+      this.config.getOptionalString('catalog.orphanStrategy') ?? 'delete';
     if (orphanStrategy !== 'delete') {
       return () => {};
     }

plugins/catalog-backend/src/service/CatalogBuilder.ts

@@ -463,8 +463,8 @@ export class CatalogBuilder {
       httpAuth,
     } = this.env;
 
-    const disableRelationsCompatibility = config.getOptionalBoolean(
-      'catalog.disableRelationsCompatibility',
+    const enableRelationsCompatibility = Boolean(
+      config.getOptionalBoolean('catalog.enableRelationsCompatibility'),
     );
 
     const policy = this.buildEntityPolicy();
@@ -503,7 +503,7 @@ export class CatalogBuilder {
       database: dbClient,
       logger,
       stitcher,
-      disableRelationsCompatibility,
+      enableRelationsCompatibility,
     });
 
     let permissionsService: PermissionsService;
@@ -619,12 +619,10 @@ export class CatalogBuilder {
       httpAuth,
       permissionsService,
       auditor,
-      disableRelationsCompatibility,
+      enableRelationsCompatibility,
     });
 
-    if (
-      config.getOptionalString('catalog.orphanProviderStrategy') === 'delete'
-    ) {
+    if (config.getOptionalString('catalog.orphanProviderStrategy') !== 'keep') {
       await evictEntitiesFromOrphanedProviders({
         db: providerDatabase,
         providers: entityProviders,

plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts

@@ -525,7 +525,7 @@ describe('DefaultEntitiesCatalog', () => {
     );
 
     it.each(databases.eachSupportedId())(
-      'should return both target and targetRef for entities',
+      'should return both target and targetRef for entities in compat mode',
       async databaseId => {
         await createDatabase(databaseId);
         await addEntity(
@@ -557,6 +557,7 @@ describe('DefaultEntitiesCatalog', () => {
           database: knex,
           logger: mockServices.logger.mock(),
           stitcher,
+          enableRelationsCompatibility: true,
         });
 
         const res = await catalog.entities();

plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts

@@ -105,19 +105,19 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog {
   private readonly database: Knex;
   private readonly logger: LoggerService;
   private readonly stitcher: Stitcher;
-  private readonly disableRelationsCompatibility: boolean;
+  private readonly enableRelationsCompatibility: boolean;
 
   constructor(options: {
     database: Knex;
     logger: LoggerService;
     stitcher: Stitcher;
-    disableRelationsCompatibility?: boolean;
+    enableRelationsCompatibility?: boolean;
   }) {
     this.database = options.database;
     this.logger = options.logger;
     this.stitcher = options.stitcher;
-    this.disableRelationsCompatibility = Boolean(
-      options.disableRelationsCompatibility,
+    this.enableRelationsCompatibility = Boolean(
+      options.enableRelationsCompatibility,
     );
   }
 
@@ -199,15 +199,15 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog {
     return {
       entities: processRawEntitiesResult(
         rows.map(r => r.final_entity!),
-        this.disableRelationsCompatibility
-          ? request?.fields
-          : e => {
+        this.enableRelationsCompatibility
+          ? e => {
               expandLegacyCompoundRelationsInEntity(e);
               if (request?.fields) {
                 return request.fields(e);
               }
               return e;
-            },
+            }
+          : request?.fields,
       ),
       pageInfo,
     };

plugins/catalog-backend/src/service/createRouter.test.ts

@@ -136,38 +136,6 @@ describe('createRouter readonly disabled', () => {
         { apiVersion: 'a', kind: 'b', metadata: { name: 'n' } },
       ];
 
-      entitiesCatalog.entities.mockResolvedValueOnce({
-        entities: { type: 'object', entities: [entities[0]] },
-        pageInfo: { hasNextPage: false },
-      });
-
-      const response = await request(app).get('/entities');
-
-      expect(response.status).toEqual(200);
-      expect(response.body).toEqual(entities);
-    });
-
-    it('happy path: lists entities when by-entities emulation is enabled', async () => {
-      const router = await createRouter({
-        entitiesCatalog,
-        locationService,
-        orchestrator,
-        logger: mockServices.logger.mock(),
-        refreshService,
-        config: new ConfigReader(undefined),
-        permissionIntegrationRouter: express.Router(),
-        auth: mockServices.auth(),
-        httpAuth: mockServices.httpAuth(),
-        locationAnalyzer,
-        permissionsService,
-        disableRelationsCompatibility: true, // added
-      });
-      app = await wrapServer(express().use(router));
-
-      const entities: Entity[] = [
-        { apiVersion: 'a', kind: 'b', metadata: { name: 'n' } },
-      ];
-
       entitiesCatalog.queryEntities.mockResolvedValueOnce({
         items: { type: 'object', entities: [entities[0]] },
         pageInfo: {},
@@ -180,34 +148,7 @@ describe('createRouter readonly disabled', () => {
       expect(response.body).toEqual(entities);
     });
 
-    it('parses single and multiple request parameters and passes them down', async () => {
-      entitiesCatalog.entities.mockResolvedValueOnce({
-        entities: { type: 'object', entities: [] },
-        pageInfo: { hasNextPage: false },
-      });
-      const response = await request(app).get(
-        '/entities?filter=a=1,a=2,b=3&filter=c=4',
-      );
-
-      expect(response.status).toEqual(200);
-      expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1);
-      expect(entitiesCatalog.entities).toHaveBeenCalledWith({
-        filter: {
-          anyOf: [
-            {
-              allOf: [
-                { key: 'a', values: ['1', '2'] },
-                { key: 'b', values: ['3'] },
-              ],
-            },
-            { key: 'c', values: ['4'] },
-          ],
-        },
-        credentials: mockCredentials.user(),
-      });
-    });
-
-    it('parses single and multiple request parameters and passes them down when by-entities emulation is enabled', async () => {
+    it('happy path: lists entities with relations compat', async () => {
       const router = await createRouter({
         entitiesCatalog,
         locationService,
@@ -220,10 +161,26 @@ describe('createRouter readonly disabled', () => {
         httpAuth: mockServices.httpAuth(),
         locationAnalyzer,
         permissionsService,
-        disableRelationsCompatibility: true, // added
+        enableRelationsCompatibility: true, // added
       });
-      app = await wrapServer(express().use(router));
 
+      app = await wrapServer(express().use(router));
+      const entities: Entity[] = [
+        { apiVersion: 'a', kind: 'b', metadata: { name: 'n' } },
+      ];
+
+      entitiesCatalog.entities.mockResolvedValueOnce({
+        entities: { type: 'object', entities: [entities[0]] },
+        pageInfo: { hasNextPage: false },
+      });
+
+      const response = await request(app).get('/entities');
+
+      expect(response.status).toEqual(200);
+      expect(response.body).toEqual(entities);
+    });
+
+    it('parses single and multiple request parameters and passes them down', async () => {
       entitiesCatalog.queryEntities.mockResolvedValueOnce({
         items: { type: 'object', entities: [] },
         pageInfo: {},
@@ -252,6 +209,48 @@ describe('createRouter readonly disabled', () => {
         skipTotalItems: true,
       });
     });
+
+    it('parses single and multiple request parameters and passes them down with relations compat', async () => {
+      const router = await createRouter({
+        entitiesCatalog,
+        locationService,
+        orchestrator,
+        logger: mockServices.logger.mock(),
+        refreshService,
+        config: new ConfigReader(undefined),
+        permissionIntegrationRouter: express.Router(),
+        auth: mockServices.auth(),
+        httpAuth: mockServices.httpAuth(),
+        locationAnalyzer,
+        permissionsService,
+        enableRelationsCompatibility: true,
+      });
+      app = await wrapServer(express().use(router));
+      entitiesCatalog.entities.mockResolvedValueOnce({
+        entities: { type: 'object', entities: [] },
+        pageInfo: { hasNextPage: false },
+      });
+      const response = await request(app).get(
+        '/entities?filter=a=1,a=2,b=3&filter=c=4',
+      );
+
+      expect(response.status).toEqual(200);
+      expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1);
+      expect(entitiesCatalog.entities).toHaveBeenCalledWith({
+        filter: {
+          anyOf: [
+            {
+              allOf: [
+                { key: 'a', values: ['1', '2'] },
+                { key: 'b', values: ['3'] },
+              ],
+            },
+            { key: 'c', values: ['4'] },
+          ],
+        },
+        credentials: mockCredentials.user(),
+      });
+    });
   });
 
   describe('GET /entities/by-query', () => {
@@ -941,7 +940,6 @@ describe('createRouter readonly and raw json enabled', () => {
       getLocationByEntity: jest.fn(),
     };
     const router = await createRouter({
-      disableRelationsCompatibility: true,
       entitiesCatalog,
       locationService,
       logger: mockServices.logger.mock(),

plugins/catalog-backend/src/service/createRouter.ts

@@ -81,7 +81,7 @@ export interface RouterOptions {
   permissionsService: PermissionsService;
   // TODO: Require AuditorService once `backend-legacy` is removed
   auditor?: AuditorService;
-  disableRelationsCompatibility?: boolean;
+  enableRelationsCompatibility?: boolean;
 }
 
 /**
@@ -110,7 +110,7 @@ export async function createRouter(
     auth,
     httpAuth,
     auditor,
-    disableRelationsCompatibility = false,
+    enableRelationsCompatibility = false,
   } = options;
 
   const readonlyEnabled =
@@ -179,7 +179,7 @@ export async function createRouter(
           // When pagination parameters are passed in, use the legacy slow path
           // that loads all entities into memory
 
-          if (pagination || disableRelationsCompatibility !== true) {
+          if (pagination || enableRelationsCompatibility === true) {
             const { entities, pageInfo } = await entitiesCatalog.entities({
               filter,
               fields,
@@ -204,7 +204,7 @@ export async function createRouter(
             await writeEntitiesResponse({
               res,
               items: entities,
-              alwaysUseObjectMode: !disableRelationsCompatibility,
+              alwaysUseObjectMode: enableRelationsCompatibility,
             });
             return;
           }
@@ -295,7 +295,7 @@ export async function createRouter(
           await writeEntitiesResponse({
             res,
             items,
-            alwaysUseObjectMode: !disableRelationsCompatibility,
+            alwaysUseObjectMode: enableRelationsCompatibility,
             responseWrapper: entities => ({
               items: entities,
               ...meta,
@@ -482,7 +482,7 @@ export async function createRouter(
           await writeEntitiesResponse({
             res,
             items,
-            alwaysUseObjectMode: !disableRelationsCompatibility,
+            alwaysUseObjectMode: enableRelationsCompatibility,
             responseWrapper: entities => ({
               items: entities,
             }),

plugins/catalog-backend/src/stitching/types.ts

@@ -61,11 +61,11 @@ export function stitchingStrategyFromConfig(config: Config): StitchingStrategy {
     'catalog.stitchingStrategy.mode',
   );
 
-  if (strategyMode === undefined || strategyMode === 'immediate') {
+  if (strategyMode === 'immediate') {
     return {
       mode: 'immediate',
     };
-  } else if (strategyMode === 'deferred') {
+  } else if (strategyMode === undefined || strategyMode === 'deferred') {
     const pollingIntervalKey = 'catalog.stitchingStrategy.pollingInterval';
     const stitchTimeoutKey = 'catalog.stitchingStrategy.stitchTimeout';
 

plugins/catalog-backend/src/tests/integration.test.ts

@@ -205,7 +205,7 @@ class TestHarness {
   readonly #db: Knex;
 
   static async create(options: {
-    disableRelationsCompatibility?: boolean;
+    enableRelationsCompatibility?: boolean;
     logger?: LoggerService;
     db: Knex;
     permissions?: PermissionEvaluator;
@@ -283,7 +283,7 @@ class TestHarness {
       database: options.db,
       logger,
       stitcher,
-      disableRelationsCompatibility: options?.disableRelationsCompatibility,
+      enableRelationsCompatibility: options?.enableRelationsCompatibility,
     });
     const proxyProgressTracker = new ProxyProgressTracker(
       new NoopProgressTracker(),
@@ -863,7 +863,6 @@ describe('Catalog Backend Integration', () => {
   it('should return valid responses in raw JSON mode', async () => {
     const harness = await TestHarness.create({
       db: await databases.init('SQLITE_3'),
-      disableRelationsCompatibility: true,
     });
 
     const entityA = {