Fix some comments

Signed-off-by: Martina Iglesias Fernandez <martina@roadie.io>
jeremyguarini · Apr 7, 2021 · 9c4cedf · 9c4cedf
1 parent b779b5f
commit 9c4cedf
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 77 deletions.
diff --git a/.changeset/sour-clocks-pay.md b/.changeset/sour-clocks-pay.md
@@ -1,5 +1,5 @@
 ---
-'@backstage/backend-common': minor
+'@backstage/backend-common': patch
 ---
 
 Add UrlReader for Google Cloud Storage
diff --git a/app-config.yaml b/app-config.yaml
@@ -122,9 +122,8 @@ kafka:
         - localhost:9092
 
 integrations:
-  gcs:
-    - host: 'storage.cloud.google.com'
-      clientEmail:
+  googleGcs:
+    - clientEmail:
         $env: GCS_CLIENT_EMAIL
       privateKey:
         $env: GCS_PRIVATE_KEY

diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json
@@ -57,6 +57,7 @@
     "minimatch": "^3.0.4",
     "minimist": "^1.2.5",
     "morgan": "^1.10.0",
+    "raw-body": "^2.4.1",
     "selfsigned": "^1.10.7",
     "stoppable": "^1.1.0",
     "tar": "^6.0.5",

diff --git a/packages/backend-common/src/reading/GcsUrlReader.test.ts b/packages/backend-common/src/reading/GcsUrlReader.test.ts
@@ -31,7 +31,7 @@ describe('GcsUrlReader', () => {
     });
   };
 
-  it('does not create a reader without the gcs field', () => {
+  it('does not create a reader without the googleGcs field', () => {
     const entries = createReader({
       integrations: {},
     });
@@ -41,7 +41,7 @@ describe('GcsUrlReader', () => {
   it('creates a reader with credentials correctly configured', () => {
     const entries = createReader({
       integrations: {
-        gcs: [
+        googleGcs: [
           {
             privateKey: '--- BEGIN KEY ---- fakekey --- END KEY ---',
             clientEmail: 'someone@example.com',
@@ -60,7 +60,7 @@ describe('GcsUrlReader', () => {
   it('does not create a reader if the privateKey is missing', () => {
     const entries = createReader({
       integrations: {
-        gcs: [
+        googleGcs: [
           {
             clientEmail: 'someone@example.com',
           },
@@ -73,7 +73,7 @@ describe('GcsUrlReader', () => {
   it('does not create a reader if the clientEmail is missing', () => {
     const entries = createReader({
       integrations: {
-        gcs: [
+        googleGcs: [
           {
             privateKey:
               '-----BEGIN PRIVATE KEY----- fakekey -----END PRIVATE KEY-----',
@@ -84,10 +84,10 @@ describe('GcsUrlReader', () => {
     expect(entries).toHaveLength(0);
   });
 
-  it('predicates', () => {
+  describe('predicates', () => {
     const readers = createReader({
       integrations: {
-        gcs: [
+        googleGcs: [
           {
             privateKey:
               '-----BEGIN PRIVATE KEY----- fakekey -----END PRIVATE KEY-----',
@@ -97,17 +97,29 @@ describe('GcsUrlReader', () => {
       },
     });
     const predicate = readers[0].predicate;
-    expect(predicate(new URL('https://storage.cloud.google.com'))).toBe(true);
-    expect(
-      predicate(
-        new URL(
-          'https://storage.cloud.google.com/team1/service1/catalog-info.yaml',
+
+    it('returns true for the correct google cloud storage host', () => {
+      expect(predicate(new URL('https://storage.cloud.google.com'))).toBe(true);
+    });
+    it('returns true for a url with the full path and the correct host', () => {
+      expect(
+        predicate(
+          new URL(
+            'https://storage.cloud.google.com/team1/service1/catalog-info.yaml',
+          ),
         ),
-      ),
-    ).toBe(true);
-    expect(predicate(new URL('https://storage2.cloud.google.com'))).toBe(false);
-    expect(predicate(new URL('https://cloud.google.com'))).toBe(false);
-    expect(predicate(new URL('https://google.com'))).toBe(false);
-    expect(predicate(new URL('https://a.example.com/test'))).toBe(false);
+      ).toBe(true);
+    });
+    it('returns false for the wrong hostname under cloud.google.com', () => {
+      expect(predicate(new URL('https://storage2.cloud.google.com'))).toBe(
+        false,
+      );
+    });
+    it('returns false for a partially correct host', () => {
+      expect(predicate(new URL('https://cloud.google.com'))).toBe(false);
+    });
+    it('returns false for a completely different host', () => {
+      expect(predicate(new URL('https://a.example.com/test'))).toBe(false);
+    });
   });
 });
diff --git a/packages/backend-common/src/reading/GcsUrlReader.ts b/packages/backend-common/src/reading/GcsUrlReader.ts
@@ -22,7 +22,6 @@ import {
   UrlReader,
 } from './types';
 import getRawBody from 'raw-body';
-import { Logger } from 'winston';
 
 const parseURL = (
   url: string,
@@ -43,11 +42,14 @@ const parseURL = (
 
 export class GcsUrlReader implements UrlReader {
   static factory: ReaderFactory = ({ config, logger }) => {
-    if (!config.has('integrations.gcs')) {
+    if (!config.has('integrations.googleGcs')) {
       return [];
     }
-    return config
-      .getConfigArray('integrations.gcs')
+    const configs = config.getOptionalConfigArray('integrations.googleGcs');
+    if (!configs) {
+      return [];
+    }
+    return configs
       .filter(integration => {
         if (!integration.has('clientEmail') || !integration.has('privateKey')) {
           logger.warn(
@@ -69,7 +71,7 @@ export class GcsUrlReader implements UrlReader {
             private_key: privKey,
           },
         });
-        const reader = new GcsUrlReader(storage, logger);
+        const reader = new GcsUrlReader(storage);
         const host =
           integration.getOptionalString('host') || 'storage.cloud.google.com';
 
@@ -79,21 +81,16 @@ export class GcsUrlReader implements UrlReader {
       });
   };
 
-  constructor(
-    private readonly storage: Storage,
-    private readonly logger: Logger,
-  ) {}
+  constructor(private readonly storage: Storage) {}
 
   async read(url: string): Promise<Buffer> {
     try {
       const { bucket, key } = parseURL(url);
 
-      this.logger.info('Reading GCS Location');
       return await getRawBody(
         this.storage.bucket(bucket).file(key).createReadStream(),
       );
     } catch (error) {
-      this.logger.warn(error.stack);
       throw new Error(`unable to read gcs file from ${url}`);
     }
   }
@@ -103,7 +100,7 @@ export class GcsUrlReader implements UrlReader {
   }
 
   async search(): Promise<SearchResponse> {
-    throw new Error('GcsUrlReader does not implement readTree');
+    throw new Error('GcsUrlReader does not implement search');
   }
 
   toString() {

diff --git a/yarn.lock b/yarn.lock
@@ -2471,21 +2471,6 @@
     retry-request "^4.1.1"
     teeny-request "^7.0.0"
 
-"@google-cloud/common@^3.6.0":
-  version "3.6.0"
-  resolved "https://registry.npmjs.org/@google-cloud/common/-/common-3.6.0.tgz#c2f6da5f79279a4a9ac7c71fc02d582beab98e8b"
-  integrity sha512-aHIFTqJZmeTNO9md8XxV+ywuvXF3xBm5WNmgWeeCK+XN5X+kGW0WEX94wGwj+/MdOnrVf4dL2RvSIt9J5yJG6Q==
-  dependencies:
-    "@google-cloud/projectify" "^2.0.0"
-    "@google-cloud/promisify" "^2.0.0"
-    arrify "^2.0.1"
-    duplexify "^4.1.1"
-    ent "^2.2.0"
-    extend "^3.0.2"
-    google-auth-library "^7.0.2"
-    retry-request "^4.1.1"
-    teeny-request "^7.0.0"
-
 "@google-cloud/container@^2.2.0":
   version "2.2.0"
   resolved "https://registry.npmjs.org/@google-cloud/container/-/container-2.2.0.tgz#e97ae1cee9040b6af09cc8199ed0aa2d4ae6238e"
@@ -11435,11 +11420,6 @@ date-and-time@^0.14.2:
   resolved "https://registry.npmjs.org/date-and-time/-/date-and-time-0.14.2.tgz#a4266c3dead460f6c231fe9674e585908dac354e"
   integrity sha512-EFTCh9zRSEpGPmJaexg7HTuzZHh6cnJj1ui7IGCFNXzd2QdpsNh05Db5TF3xzJm30YN+A8/6xHSuRcQqoc3kFA==
 
-date-and-time@^0.14.2:
-  version "0.14.2"
-  resolved "https://registry.npmjs.org/date-and-time/-/date-and-time-0.14.2.tgz#a4266c3dead460f6c231fe9674e585908dac354e"
-  integrity sha512-EFTCh9zRSEpGPmJaexg7HTuzZHh6cnJj1ui7IGCFNXzd2QdpsNh05Db5TF3xzJm30YN+A8/6xHSuRcQqoc3kFA==
-
 date-and-time@^0.6.3:
   version "0.6.3"
   resolved "https://registry.npmjs.org/date-and-time/-/date-and-time-0.6.3.tgz#2daee52df67c28bd93bce862756ac86b68cf4237"
@@ -14026,19 +14006,6 @@ gcs-resumable-upload@^3.1.3:
     pumpify "^2.0.0"
     stream-events "^1.0.4"
 
-gcs-resumable-upload@^3.1.3:
-  version "3.1.3"
-  resolved "https://registry.npmjs.org/gcs-resumable-upload/-/gcs-resumable-upload-3.1.3.tgz#1e38e1339600b85812e6430a5ab455453c64cce3"
-  integrity sha512-LjVrv6YVH0XqBr/iBW0JgRA1ndxhK6zfEFFJR4im51QVTj/4sInOXimY2evDZuSZ75D3bHxTaQAdXRukMc1y+w==
-  dependencies:
-    abort-controller "^3.0.0"
-    configstore "^5.0.0"
-    extend "^3.0.2"
-    gaxios "^4.0.0"
-    google-auth-library "^7.0.0"
-    pumpify "^2.0.0"
-    stream-events "^1.0.4"
-
 generic-names@^2.0.1:
   version "2.0.1"
   resolved "https://registry.npmjs.org/generic-names/-/generic-names-2.0.1.tgz#f8a378ead2ccaa7a34f0317b05554832ae41b872"
@@ -15159,6 +15126,17 @@ http-errors@1.7.2:
     statuses ">= 1.5.0 < 2"
     toidentifier "1.0.0"
 
+http-errors@1.7.3, http-errors@~1.7.2:
+  version "1.7.3"
+  resolved "https://registry.npmjs.org/http-errors/-/http-errors-1.7.3.tgz#6c619e4f9c60308c38519498c14fbb10aacebb06"
+  integrity sha512-ZTTX0MWrsQ2ZAhA1cejAwDLycFsd7I7nVtnkT3Ol0aqodaKW+0CTZDQ1uBv5whptCnc8e8HeRRJxRs0kmm/Qfw==
+  dependencies:
+    depd "~1.1.2"
+    inherits "2.0.4"
+    setprototypeof "1.1.1"
+    statuses ">= 1.5.0 < 2"
+    toidentifier "1.0.0"
+
 http-errors@^1.7.3:
   version "1.8.0"
   resolved "https://registry.npmjs.org/http-errors/-/http-errors-1.8.0.tgz#75d1bbe497e1044f51e4ee9e704a62f28d336507"
@@ -15180,17 +15158,6 @@ http-errors@~1.6.2:
     setprototypeof "1.1.0"
     statuses ">= 1.4.0 < 2"
 
-http-errors@~1.7.2:
-  version "1.7.3"
-  resolved "https://registry.npmjs.org/http-errors/-/http-errors-1.7.3.tgz#6c619e4f9c60308c38519498c14fbb10aacebb06"
-  integrity sha512-ZTTX0MWrsQ2ZAhA1cejAwDLycFsd7I7nVtnkT3Ol0aqodaKW+0CTZDQ1uBv5whptCnc8e8HeRRJxRs0kmm/Qfw==
-  dependencies:
-    depd "~1.1.2"
-    inherits "2.0.4"
-    setprototypeof "1.1.1"
-    statuses ">= 1.5.0 < 2"
-    toidentifier "1.0.0"
-
 "http-parser-js@>=0.4.0 <0.4.11":
   version "0.4.10"
   resolved "https://registry.npmjs.org/http-parser-js/-/http-parser-js-0.4.10.tgz#92c9c1374c35085f75db359ec56cc257cbb93fa4"
@@ -21976,6 +21943,16 @@ raw-body@2.4.0:
     iconv-lite "0.4.24"
     unpipe "1.0.0"
 
+raw-body@^2.4.1:
+  version "2.4.1"
+  resolved "https://registry.npmjs.org/raw-body/-/raw-body-2.4.1.tgz#30ac82f98bb5ae8c152e67149dac8d55153b168c"
+  integrity sha512-9WmIKF6mkvA0SLmA2Knm9+qj89e+j1zqgyn8aXGd7+nAduPoqgI9lO57SAZNn/Byzo5P7JhXTyg9PzaJbH73bA==
+  dependencies:
+    bytes "3.1.0"
+    http-errors "1.7.3"
+    iconv-lite "0.4.24"
+    unpipe "1.0.0"
+
 raw-loader@^4.0.1:
   version "4.0.2"
   resolved "https://registry.npmjs.org/raw-loader/-/raw-loader-4.0.2.tgz#1aac6b7d1ad1501e66efdac1522c73e59a584eb6"