Merge pull request #10887 from backstage/rugvip/not-allowed

backend-common: nicer error message for all UrlReader methods

.changeset/twenty-eagles-turn.md

@@ -0,0 +1,5 @@
+---
+'@backstage/backend-common': patch
+---
+
+Tweaked the `UrlReader` multiplexer so that it uses the more helpful `NotAllowedError` messaging for all methods.

packages/backend-common/src/reading/UrlReaderPredicateMux.test.ts

@@ -0,0 +1,87 @@
+/*
+ * Copyright 2020 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { getVoidLogger } from '../logging';
+import { UrlReaderPredicateMux } from './UrlReaderPredicateMux';
+
+describe('UrlReaderPredicateMux', () => {
+  it('forwards methods based on predicate', async () => {
+    const fooReader = {
+      read: jest.fn(),
+      readUrl: jest.fn(),
+      readTree: jest.fn(),
+      search: jest.fn(),
+    };
+    const barReader = {
+      read: jest.fn(),
+      readUrl: jest.fn(),
+      readTree: jest.fn(),
+      search: jest.fn(),
+    };
+
+    const mux = new UrlReaderPredicateMux(getVoidLogger());
+    mux.register({
+      predicate: url => url.hostname === 'foo',
+      reader: fooReader,
+    });
+    mux.register({
+      predicate: url => url.hostname === 'bar',
+      reader: barReader,
+    });
+
+    await mux.read('http://foo/1');
+    expect(fooReader.read).toHaveBeenCalledWith('http://foo/1');
+    await mux.readUrl('http://foo/2');
+    expect(fooReader.readUrl).toHaveBeenCalledWith('http://foo/2', undefined);
+    await mux.readTree('http://foo/3');
+    expect(fooReader.readTree).toHaveBeenCalledWith('http://foo/3', undefined);
+    await mux.search('http://foo/4');
+    expect(fooReader.search).toHaveBeenCalledWith('http://foo/4', undefined);
+
+    await mux.read('http://bar/1');
+    expect(barReader.read).toHaveBeenCalledWith('http://bar/1');
+    await mux.readUrl('http://bar/2');
+    expect(barReader.readUrl).toHaveBeenCalledWith('http://bar/2', undefined);
+    await mux.readTree('http://bar/3');
+    expect(barReader.readTree).toHaveBeenCalledWith('http://bar/3', undefined);
+    await mux.search('http://bar/4');
+    expect(barReader.search).toHaveBeenCalledWith('http://bar/4', undefined);
+  });
+
+  it('throws an error if no predicate matches', async () => {
+    const mux = new UrlReaderPredicateMux(getVoidLogger());
+
+    await expect(mux.readUrl('http://foo/1')).rejects.toThrowError(
+      /^Reading from 'http:\/\/foo\/1' is not allowed. You may/,
+    );
+
+    mux.register({
+      predicate: url => url.hostname === 'foo',
+      reader: {
+        read: jest.fn(),
+        readUrl: jest.fn(),
+        readTree: jest.fn(),
+        search: jest.fn(),
+      },
+    });
+
+    await expect(mux.readUrl('http://foo/1')).resolves.toBeUndefined();
+
+    await expect(mux.readUrl('http://bar/1')).rejects.toThrowError(
+      /^Reading from 'http:\/\/bar\/1' is not allowed. You may/,
+    );
+  });
+});

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

@@ -29,6 +29,14 @@ import {
 
 const MIN_WARNING_INTERVAL_MS = 1000 * 60 * 15;
 
+function notAllowedMessage(url: string) {
+  return (
+    `Reading from '${url}' is not allowed. ` +
+    `You may need to configure an integration for the target host, or add it ` +
+    `to the configured list of allowed hosts at 'backend.reading.allow'`
+  );
+}
+
 /**
  * A UrlReader implementation that selects from a set of UrlReaders
  * based on a predicate tied to each reader.
@@ -52,11 +60,7 @@ export class UrlReaderPredicateMux implements UrlReader {
       }
     }
 
-    throw new NotAllowedError(
-      `Reading from '${url}' is not allowed. ` +
-        `You may need to configure an integration for the target host, or add it ` +
-        `to the configured list of allowed hosts at 'backend.reading.allow'`,
-    );
+    throw new NotAllowedError(notAllowedMessage(url));
   }
 
   async readUrl(
@@ -87,7 +91,7 @@ export class UrlReaderPredicateMux implements UrlReader {
       }
     }
 
-    throw new NotAllowedError(`Reading from '${url}' is not allowed`);
+    throw new NotAllowedError(notAllowedMessage(url));
   }
 
   async readTree(
@@ -102,7 +106,7 @@ export class UrlReaderPredicateMux implements UrlReader {
       }
     }
 
-    throw new NotAllowedError(`Reading from '${url}' is not allowed`);
+    throw new NotAllowedError(notAllowedMessage(url));
   }
 
   async search(url: string, options?: SearchOptions): Promise<SearchResponse> {
@@ -114,7 +118,7 @@ export class UrlReaderPredicateMux implements UrlReader {
       }
     }
 
-    throw new NotAllowedError(`Reading from '${url}' is not allowed`);
+    throw new NotAllowedError(notAllowedMessage(url));
   }
 
   toString() {