Update PR to reflect change requests

Signed-off-by: Sean Tan <seant@splunk.com>

.changeset/shiny-emus-perform.md

@@ -1,7 +0,0 @@
----
-'@backstage/backend-common': patch
-'@backstage/integration': patch
-'@backstage/plugin-catalog-backend': patch
----
-
-Add readTree() to AwsS3UrlReader and aws-read-tree Catalog Processor

docs/features/software-templates/migrating-from-v1alpha1-to-v1beta2.md

@@ -325,7 +325,7 @@ spec:
   output:
     links:
       - url: '{{steps.publish.output.remoteUrl}}'
-        text: 'Go to Repo'
+        title: 'Go to Repo'
 ```
 
 ## Questions?

docs/integrations/aws-s3/discovery.md

@@ -0,0 +1,28 @@
+---
+id: discovery
+title: AWS S3 Discovery
+sidebar_label: Discovery
+# prettier-ignore
+description: Automatically discovering catalog entities from an AWS S3 Bucket
+---
+
+The AWS S3 integration has a special discovery processor for discovering catalog
+entities located in an S3 Bucket. If you have a bucket that contains multiple
+catalog-info files and want to automatically discover them, you can use this
+processor. The processor will crawl your S3 bucket and register entities
+matching the configured path. This can be useful as an alternative to static
+locations or manually adding things to the catalog.
+
+To use the discovery processor, you'll need an AWS S3 integration
+[set up](locations.md) with an `AWS_ACCESS_KEY`, `AWS_SECRET_ACCESS_KEY`, and
+optionally a `roleArn`. Then you can add a location target to the catalog
+configuration:
+
+```yaml
+catalog:
+  locations:
+    - type: awsS3-discovery
+      target: https://sample-bucket.s3.us-east-2.amazonaws.com/
+```
+
+Note the `awsS3-discovery` type, as this is not a regular `url` processor.

docs/integrations/aws-s3/readtree.md

@@ -1,28 +0,0 @@
----
-id: read-tree
-title: AWS S3 Read Tree
-sidebar_label: Read Tree
-# prettier-ignore
-description: Discovering multiple catalog entities from an AWS S3 Bucket
----
-
-The AWS S3 integration has a special processor for using AwsS3UrlReader's
-readTree() method. The method readTree() allows a user to retrieve more than one
-file hosted inside an S3 bucket. The processor passes a valid S3 URL and
-credentials to AwsS3UrlReader's readTree(). A user can supply a URL which points
-to root of a bucket or a subdirectory inside the bucket. This can be useful as
-an alternative to static locations or manually adding things to the catalog.
-
-To use the discovery processor, you'll need an AWS S3 integration
-[set up](locations.md) with an `AWS_ACCESS_KEY`, `AWS_SECRET_ACCESS_KEY`, and
-optionally a `roleArn`. Then you can add a location target to the catalog
-configuration:
-
-```yaml
-catalog:
-  locations:
-    - type: s3-bucket
-      target: https://sample-bucket.s3.us-east-2.amazonaws.com/
-```
-
-Note the `s3-bucket` type, as this is not a regular `url` processor.

microsite/sidebars.json

@@ -107,7 +107,7 @@
       {
         "type": "subcategory",
         "label": "AWS S3",
-        "ids": ["integrations/aws-s3/locations", "integrations/aws-s3/readtree"]
+        "ids": ["integrations/aws-s3/locations", "integrations/aws-s3/discovery"]
       },
       {
         "type": "subcategory",

mkdocs.yml

@@ -81,7 +81,7 @@ nav:
       - Overview: 'integrations/index.md'
       - AWS S3:
           - Locations: 'integrations/aws-s3/locations.md'
-          - Read Tree: 'integrations/aws-s3/readtree.md'
+          - Discovery: 'integrations/aws-s3/discovery.md'
       - Azure:
           - Locations: 'integrations/azure/locations.md'
           - Org Data: 'integrations/azure/org.md'

packages/backend-common/src/reading/AwsS3UrlReader.ts

@@ -27,7 +27,7 @@ import {
 } from './types';
 import getRawBody from 'raw-body';
 import { AwsS3Integration, ScmIntegrations } from '@backstage/integration';
-import { Readable } from 'stream';
+import { ListObjectsV2Output, ObjectList } from 'aws-sdk/clients/s3';
 
 const parseURL = (
   url: string,
@@ -169,54 +169,39 @@ export class AwsS3UrlReader implements UrlReader {
   async readTree(url: string): Promise<ReadTreeResponse> {
     try {
       const { path, bucket, region } = parseURL(url);
-
-      let moreKeys = true;
-      let awsS3Readables: Readable[] = [];
-      let continuationToken = '';
-
-      while (moreKeys) {
-        let params;
-        if (continuationToken === '') {
-          params = {
-            Bucket: bucket,
-            Prefix: path,
-          };
-        } else {
-          params = {
-            Bucket: bucket,
-            Prefix: path,
-            ContinuationToken: continuationToken,
-          };
-        }
+      const allObjects: ObjectList = [];
+      const responses = [];
+      let continuationToken: string | undefined;
+      let output: ListObjectsV2Output;
+      do {
         aws.config.update({ region: region });
-        const { Contents, IsTruncated, NextContinuationToken } =
-          await this.deps.s3.listObjectsV2(params).promise();
-        const responses = await Promise.all(
-          (Contents || []).map(({ Key }) => {
-            const s3Response = this.deps.s3
-              .getObject({ Bucket: bucket, Key: String(Key) })
-              .createReadStream();
-            Object.defineProperty(s3Response, 'path', {
-              value: String(Key),
-              writable: false,
-            });
-            return s3Response;
-          }),
-        );
-
-        if (IsTruncated) {
-          continuationToken = String(NextContinuationToken);
-        } else {
-          continuationToken = '';
-          moreKeys = false;
+        output = await this.deps.s3
+          .listObjectsV2({
+            Bucket: bucket,
+            ContinuationToken: continuationToken,
+            Prefix: path,
+          })
+          .promise();
+        if (output.Contents) {
+          output.Contents.forEach(function (contents) {
+            allObjects.push(contents);
+          });
         }
-        awsS3Readables = awsS3Readables.concat(responses);
+        continuationToken = output.NextContinuationToken;
+      } while (continuationToken);
+
+      for (let i = 0; i < allObjects.length; i++) {
+        const object = this.deps.s3.getObject({
+          Bucket: bucket,
+          Key: String(allObjects[i].Key),
+        });
+        responses.push({
+          data: object.createReadStream(),
+          path: String(allObjects[i].Key),
+        });
       }
 
-      return await this.deps.treeResponseFactory.fromReadableArray({
-        stream: awsS3Readables,
-        etag: '',
-      });
+      return await this.deps.treeResponseFactory.fromReadableArray(responses);
     } catch (e) {
       throw new Error(`Could not retrieve file tree from S3: ${e.message}`);
     }

packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts

@@ -59,10 +59,6 @@ export class DefaultReadTreeResponseFactory implements ReadTreeResponseFactory {
   async fromReadableArray(
     options: FromReadableArrayOptions,
   ): Promise<ReadTreeResponse> {
-    return new ReadableArrayResponse(
-      options.stream,
-      this.workDir,
-      options.etag,
-    );
+    return new ReadableArrayResponse(options, this.workDir, '');
   }
 }

packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts

@@ -15,20 +15,19 @@
  */
 import { ReadableArrayResponse } from './ReadableArrayResponse';
 import path, { resolve as resolvePath } from 'path';
-
-import { Readable } from 'stream';
 import fs from 'fs-extra';
+import { FromReadableArrayOptions } from '../types';
 
-const arr: Readable[] = [];
-const arr2: Readable[] = [];
-const file1 = path.resolve(
+const arr: FromReadableArrayOptions = [];
+const arr2: FromReadableArrayOptions = [];
+const path1 = path.resolve(
   'src',
   'reading',
   '__fixtures__',
   'awsS3',
   'awsS3-mock-object.yaml',
 );
-const file2 = path.resolve(
+const path2 = path.resolve(
   'src',
   'reading',
   '__fixtures__',
@@ -38,21 +37,21 @@ const file2 = path.resolve(
 
 describe('ReadableArrayResponse', () => {
   it('should read files', async () => {
-    const stream1 = fs.createReadStream(file1);
-    const stream2 = fs.createReadStream(file2);
-    arr.push(stream1);
-    arr.push(stream2);
+    const stream1 = fs.createReadStream(path1);
+    const stream2 = fs.createReadStream(path2);
+    arr.push({ data: stream1, path: path1 });
+    arr.push({ data: stream2, path: path2 });
 
     const res = new ReadableArrayResponse(arr, '/tmp', 'etag');
     const files = await res.files();
 
     expect(files).toEqual([
       {
-        path: file1,
+        path: path1,
         content: expect.any(Function),
       },
       {
-        path: file2,
+        path: path2,
         content: expect.any(Function),
       },
     ]);
@@ -64,11 +63,11 @@ describe('ReadableArrayResponse', () => {
   });
 
   it('should extract entire archive into directory', async () => {
-    const stream1 = fs.createReadStream(file1);
-    const stream2 = fs.createReadStream(file2);
+    const stream1 = fs.createReadStream(path1);
+    const stream2 = fs.createReadStream(path2);
 
-    arr2.push(stream1);
-    arr2.push(stream2);
+    arr2.push({ data: stream1, path: path1 });
+    arr2.push({ data: stream2, path: path2 });
 
     const res = new ReadableArrayResponse(arr2, '/tmp', 'etag');
     const dir = await res.dir();

packages/backend-common/src/reading/tree/ReadableArrayResponse.ts

@@ -26,6 +26,7 @@ import {
   ReadTreeResponse,
   ReadTreeResponseFile,
   ReadTreeResponseDirOptions,
+  FromReadableArrayOptions,
 } from '../types';
 
 const pipeline = promisify(pipelineCb);
@@ -37,7 +38,7 @@ export class ReadableArrayResponse implements ReadTreeResponse {
   private read = false;
 
   constructor(
-    private readonly stream: Readable[],
+    private readonly stream: FromReadableArrayOptions,
     private readonly workDir: string,
     public readonly etag: string,
   ) {
@@ -58,10 +59,10 @@ export class ReadableArrayResponse implements ReadTreeResponse {
     const files = Array<ReadTreeResponseFile>();
 
     for (let i = 0; i < this.stream.length; i++) {
-      if (!(this.stream[i] as any).path.endsWith('/')) {
+      if (!this.stream[i].path.endsWith('/')) {
         files.push({
-          path: (this.stream[i] as any).path,
-          content: () => getRawBody(this.stream[i]),
+          path: this.stream[i].path,
+          content: () => getRawBody(this.stream[i].data),
         });
       }
     }
@@ -93,11 +94,11 @@ export class ReadableArrayResponse implements ReadTreeResponse {
       (await fs.mkdtemp(platformPath.join(this.workDir, 'backstage-')));
 
     for (let i = 0; i < this.stream.length; i++) {
-      if (!(this.stream[i] as any).path.endsWith('/')) {
+      if (!this.stream[i].path.endsWith('/')) {
         await pipeline(
-          this.stream[i],
+          this.stream[i].data,
           fs.createWriteStream(
-            platformPath.join(dir, basename((this.stream[i] as any).path)),
+            platformPath.join(dir, basename(this.stream[i].path)),
           ),
         );
       }

packages/backend-common/src/reading/types.ts

@@ -167,12 +167,12 @@ export type FromArchiveOptions = {
   filter?: (path: string, info?: { size: number }) => boolean;
 };
 
-export type FromReadableArrayOptions = {
-  // An array of readable streams
-  stream: Readable[];
-  // etag of the file tree
-  etag: string;
-};
+export type FromReadableArrayOptions = Array<{
+  // Data in the form of a readable
+  data: Readable;
+  // A string containing the filepath of the data
+  path: string;
+}>;
 export interface ReadTreeResponseFactory {
   fromTarArchive(options: FromArchiveOptions): Promise<ReadTreeResponse>;
   fromZipArchive(options: FromArchiveOptions): Promise<ReadTreeResponse>;

plugins/catalog-backend/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@backstage/plugin-catalog-backend",
-  "version": "0.13.5",
+  "version": "0.13.2",
   "main": "src/index.ts",
   "types": "src/index.ts",
   "license": "Apache-2.0",

plugins/catalog-backend/src/ingestion/processors/AwsS3DiscoveryProcessor.test.ts

@@ -15,7 +15,7 @@
  */
 import { getVoidLogger, UrlReaders } from '@backstage/backend-common';
 import { ConfigReader } from '@backstage/config';
-import { AwsS3ReadTreeProcessor } from './AwsS3ReadTreeProcessor';
+import { AwsS3DiscoveryProcessor } from './AwsS3DiscoveryProcessor';
 import { CatalogProcessorEntityResult, CatalogProcessorResult } from './types';
 import { defaultEntityDataParser } from './util/parse';
 import AWSMock from 'aws-sdk-mock';
@@ -58,9 +58,9 @@ const reader = UrlReaders.default({
 });
 
 describe('readLocation', () => {
-  const processor = new AwsS3ReadTreeProcessor(reader);
+  const processor = new AwsS3DiscoveryProcessor(reader);
   const spec = {
-    type: 's3-bucket',
+    type: 'awsS3-discovery',
     target: 'https://testbucket.s3.us-east-2.amazonaws.com',
   };
 
@@ -71,7 +71,7 @@ describe('readLocation', () => {
     expect(generated.type).toBe('entity');
     expect(generated.location).toEqual({
       target: 'awsS3-mock-object.txt',
-      type: 's3-bucket',
+      type: 'awsS3-discovery',
     });
     expect(generated.entity).toEqual({ site_name: 'Test' });
   });

plugins/catalog-backend/src/ingestion/processors/AwsS3DiscoveryProcessor.ts

@@ -16,6 +16,7 @@
 
 import { UrlReader } from '@backstage/backend-common';
 import { LocationSpec } from '@backstage/catalog-model';
+import limiterFactory from 'p-limit';
 import * as result from './results';
 import {
   CatalogProcessor,
@@ -23,7 +24,7 @@ import {
   CatalogProcessorParser,
 } from './types';
 
-export class AwsS3ReadTreeProcessor implements CatalogProcessor {
+export class AwsS3DiscoveryProcessor implements CatalogProcessor {
   constructor(private readonly reader: UrlReader) {}
 
   async readLocation(
@@ -32,7 +33,7 @@ export class AwsS3ReadTreeProcessor implements CatalogProcessor {
     emit: CatalogProcessorEmit,
     parser: CatalogProcessorParser,
   ): Promise<boolean> {
-    if (location.type !== 's3-bucket') {
+    if (location.type !== 'awsS3-discovery') {
       return false;
     }
 
@@ -63,11 +64,12 @@ export class AwsS3ReadTreeProcessor implements CatalogProcessor {
   private async doRead(
     location: string,
   ): Promise<{ data: Buffer; url: string }[]> {
+    const limiter = limiterFactory(5);
     const response = await this.reader.readTree(location);
     const responseFiles = await response.files();
     const output = responseFiles.map(async file => ({
       url: file.path,
-      data: await file.content(),
+      data: await limiter(file.content),
     }));
     return Promise.all(output);
   }

plugins/catalog-backend/src/ingestion/processors/index.ts

@@ -19,7 +19,7 @@ import * as results from './results';
 export { AnnotateLocationEntityProcessor } from './AnnotateLocationEntityProcessor';
 export { AnnotateScmSlugEntityProcessor } from './AnnotateScmSlugEntityProcessor';
 export { AwsOrganizationCloudAccountProcessor } from './AwsOrganizationCloudAccountProcessor';
-export { AwsS3ReadTreeProcessor } from './AwsS3ReadTreeProcessor';
+export { AwsS3DiscoveryProcessor } from './AwsS3DiscoveryProcessor';
 export { BitbucketDiscoveryProcessor } from './BitbucketDiscoveryProcessor';
 export { BuiltinKindsEntityProcessor } from './BuiltinKindsEntityProcessor';
 export { CodeOwnersProcessor } from './CodeOwnersProcessor';