Fix handling of optional property in catalog:register scaffolder action

Signed-off-by: Andreas Berger <andreas@berger-ecommerce.com>

.changeset/angry-jobs-ring.md

@@ -0,0 +1,5 @@
+---
+'@backstage/catalog-client': patch
+---
+
+Updated API docs

.changeset/few-oranges-complain.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-scaffolder-backend': patch
+---
+
+Fix handling of `optional` property in `catalog:register` scaffolder action

packages/catalog-client/src/types/api.ts

@@ -353,6 +353,10 @@ export type Location = {
 export type AddLocationRequest = {
   type?: string;
   target: string;
+  /**
+   * If set to true, the location will not be added, but the response will
+   * contain the entities that match the given location.
+   */
   dryRun?: boolean;
 };
 
@@ -363,8 +367,13 @@ export type AddLocationRequest = {
  */
 export type AddLocationResponse = {
   location: Location;
+  /**
+   * The entities matching this location. Will only be filled in dryRun mode
+   */
   entities: Entity[];
-  // Only set in dryRun mode.
+  /**
+   * True, if the location exists. Will only be filled in dryRun mode
+   */
   exists?: boolean;
 };
 

plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.test.ts

@@ -265,9 +265,7 @@ describe('catalog:register', () => {
 
   it('should ignore failures when dry running the location in the catalog if `optional` is set', async () => {
     addLocation
-      .mockResolvedValueOnce({
-        entities: [],
-      })
+      .mockRejectedValueOnce(new Error('Not found'))
       .mockRejectedValueOnce(new Error('Not found'));
     await action.handler({
       ...mockContext,
@@ -300,4 +298,54 @@ describe('catalog:register', () => {
       'http://foo/var',
     );
   });
+
+  it('should fetch entities when adding location in the catalog fails and `optional` is set', async () => {
+    addLocation
+      .mockRejectedValueOnce(new Error('Already registered'))
+      .mockResolvedValueOnce({
+        entities: [
+          {
+            metadata: {
+              namespace: 'default',
+              name: 'test',
+            },
+            kind: 'Component',
+          } as Entity,
+        ],
+      });
+    await action.handler({
+      ...mockContext,
+      input: {
+        catalogInfoUrl: 'http://foo/var',
+        optional: true,
+      },
+    });
+
+    expect(addLocation).toHaveBeenNthCalledWith(
+      1,
+      {
+        type: 'url',
+        target: 'http://foo/var',
+      },
+      {},
+    );
+    expect(addLocation).toHaveBeenNthCalledWith(
+      2,
+      {
+        dryRun: true,
+        type: 'url',
+        target: 'http://foo/var',
+      },
+      {},
+    );
+
+    expect(mockContext.output).toHaveBeenCalledWith(
+      'catalogInfoUrl',
+      'http://foo/var',
+    );
+    expect(mockContext.output).toHaveBeenCalledWith(
+      'entityRef',
+      'component:default/test',
+    );
+  });
 });

plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.ts

@@ -17,7 +17,7 @@
 import { InputError } from '@backstage/errors';
 import { ScmIntegrations } from '@backstage/integration';
 import { CatalogApi } from '@backstage/catalog-client';
-import { stringifyEntityRef } from '@backstage/catalog-model';
+import { stringifyEntityRef, Entity } from '@backstage/catalog-model';
 import { createTemplateAction } from '@backstage/plugin-scaffolder-node';
 import yaml from 'yaml';
 
@@ -144,17 +144,26 @@ export function createCatalogRegisterAction(options: {
 
       ctx.logger.info(`Registering ${catalogInfoUrl} in the catalog`);
 
-      await catalogClient.addLocation(
-        {
-          type: 'url',
-          target: catalogInfoUrl,
-        },
-        ctx.secrets?.backstageToken
-          ? { token: ctx.secrets.backstageToken }
-          : {},
-      );
+      try {
+        // 1st try to register the location, this will throw an error if the location already exists (see catch)
+        await catalogClient.addLocation(
+          {
+            type: 'url',
+            target: catalogInfoUrl,
+          },
+          ctx.secrets?.backstageToken
+            ? { token: ctx.secrets.backstageToken }
+            : {},
+        );
+      } catch (e) {
+        if (!input.optional) {
+          // if optional is false or unset, it is not allowed to register the same location twice, we rethrow the error
+          throw e;
+        }
+      }
 
       try {
+        // 2nd retry the registration as a dry run, this will not throw an error if the location already exists
         const result = await catalogClient.addLocation(
           {
             dryRun: true,
@@ -166,18 +175,18 @@ export function createCatalogRegisterAction(options: {
             : {},
         );
 
-        if (result.entities.length > 0) {
+        if (result.entities.length) {
           const { entities } = result;
-          let entity: any;
+          let entity: Entity | undefined;
           // prioritise 'Component' type as it is the most central kind of entity
           entity = entities.find(
-            (e: any) =>
+            e =>
               !e.metadata.name.startsWith('generated-') &&
               e.kind === 'Component',
           );
           if (!entity) {
             entity = entities.find(
-              (e: any) => !e.metadata.name.startsWith('generated-'),
+              e => !e.metadata.name.startsWith('generated-'),
             );
           }
           if (!entity) {