Merge pull request #4420 from backstage/freben/gitlab-config

integration: update the gitlab config mandatory fields to match reality

packages/integration/src/gitlab/GitLabIntegration.test.ts

@@ -26,6 +26,8 @@ describe('GitLabIntegration', () => {
             {
               host: 'h.com',
               token: 't',
+              apiBaseUrl: 'https://h.com/api/v4',
+              baseUrl: 'https://h.com',
             },
           ],
         },

packages/integration/src/gitlab/config.test.ts

@@ -31,6 +31,7 @@ describe('readGitLabIntegrationConfig', () => {
       buildConfig({
         host: 'a.com',
         token: 't',
+        apiBaseUrl: 'https://a.com',
         baseUrl: 'https://baseurl.for.me/gitlab',
       }),
     );
@@ -38,12 +39,15 @@ describe('readGitLabIntegrationConfig', () => {
     expect(output).toEqual({
       host: 'a.com',
       token: 't',
+      apiBaseUrl: 'https://a.com',
       baseUrl: 'https://baseurl.for.me/gitlab',
     });
   });
 
   it('inserts the defaults if missing', () => {
-    const output = readGitLabIntegrationConfig(buildConfig({}));
+    const output = readGitLabIntegrationConfig(
+      buildConfig({ host: 'gitlab.com' }),
+    );
     expect(output).toEqual({
       host: 'gitlab.com',
       apiBaseUrl: 'https://gitlab.com/api/v4',
@@ -88,12 +92,15 @@ describe('readGitLabIntegrationConfigs', () => {
         {
           host: 'a.com',
           token: 't',
+          apiBaseUrl: 'https://a.com/api/v4',
+          baseUrl: 'https://a.com',
         },
       ]),
     );
     expect(output).toContainEqual({
       host: 'a.com',
       token: 't',
+      apiBaseUrl: 'https://a.com/api/v4',
       baseUrl: 'https://a.com',
     });
   });
@@ -104,6 +111,7 @@ describe('readGitLabIntegrationConfigs', () => {
       {
         host: 'gitlab.com',
         apiBaseUrl: 'https://gitlab.com/api/v4',
+        baseUrl: 'https://gitlab.com',
       },
     ]);
   });

packages/integration/src/gitlab/config.ts

@@ -15,7 +15,7 @@
  */
 
 import { Config } from '@backstage/config';
-import { isValidHost } from '../helpers';
+import { isValidHost, isValidUrl } from '../helpers';
 
 const GITLAB_HOST = 'gitlab.com';
 const GITLAB_API_BASE_URL = 'https://gitlab.com/api/v4';
@@ -38,7 +38,7 @@ export type GitLabIntegrationConfig = {
    * The API will always be preferred if both its base URL and a token are
    * present.
    */
-  apiBaseUrl?: string;
+  apiBaseUrl: string;
 
   /**
    * The authorization token to use for requests this provider.
@@ -53,7 +53,7 @@ export type GitLabIntegrationConfig = {
    *
    * If no baseUrl is provided, it will default to https://${host}
    */
-  baseUrl?: string;
+  baseUrl: string;
 };
 
 /**
@@ -64,16 +64,10 @@ export type GitLabIntegrationConfig = {
 export function readGitLabIntegrationConfig(
   config: Config,
 ): GitLabIntegrationConfig {
-  const host = config.getOptionalString('host') ?? GITLAB_HOST;
+  const host = config.getString('host');
   let apiBaseUrl = config.getOptionalString('apiBaseUrl');
   const token = config.getOptionalString('token');
-  const baseUrl = config.getOptionalString('baseUrl') ?? `https://${host}`;
-
-  if (!isValidHost(host)) {
-    throw new Error(
-      `Invalid GitLab integration config, '${host}' is not a valid host`,
-    );
-  }
+  let baseUrl = config.getOptionalString('baseUrl');
 
   if (apiBaseUrl) {
     apiBaseUrl = apiBaseUrl.replace(/\/+$/, '');
@@ -81,6 +75,30 @@ export function readGitLabIntegrationConfig(
     apiBaseUrl = GITLAB_API_BASE_URL;
   }
 
+  if (baseUrl) {
+    baseUrl = baseUrl.replace(/\/+$/, '');
+  } else {
+    baseUrl = `https://${host}`;
+  }
+
+  if (host.includes(':')) {
+    throw new Error(
+      `Invalid GitLab integration config, host '${host}' should just be the host name (e.g. "github.com"), not a URL`,
+    );
+  } else if (!isValidHost(host)) {
+    throw new Error(
+      `Invalid GitLab integration config, '${host}' is not a valid host`,
+    );
+  } else if (!apiBaseUrl || !isValidUrl(apiBaseUrl)) {
+    throw new Error(
+      `Invalid GitLab integration config, '${apiBaseUrl}' is not a valid apiBaseUrl`,
+    );
+  } else if (!isValidUrl(baseUrl)) {
+    throw new Error(
+      `Invalid GitLab integration config, '${baseUrl}' is not a valid baseUrl`,
+    );
+  }
+
   return { host, token, apiBaseUrl, baseUrl };
 }
 
@@ -99,7 +117,11 @@ export function readGitLabIntegrationConfigs(
   // As a convenience we always make sure there's at least an unauthenticated
   // reader for public gitlab repos.
   if (!result.some(c => c.host === GITLAB_HOST)) {
-    result.push({ host: GITLAB_HOST, apiBaseUrl: GITLAB_API_BASE_URL });
+    result.push({
+      host: GITLAB_HOST,
+      apiBaseUrl: GITLAB_API_BASE_URL,
+      baseUrl: `https://${GITLAB_HOST}`,
+    });
   }
 
   return result;

packages/integration/src/gitlab/core.test.ts

@@ -37,10 +37,14 @@ describe('gitlab core', () => {
   const configWithToken: GitLabIntegrationConfig = {
     host: 'g.com',
     token: '0123456789',
+    apiBaseUrl: '<ignored>',
+    baseUrl: '<ignored>',
   };
 
   const configWithNoToken: GitLabIntegrationConfig = {
     host: 'g.com',
+    apiBaseUrl: '<ignored>',
+    baseUrl: '<ignored>',
   };
 
   describe('getGitLabFileFetchUrl', () => {

packages/integration/src/helpers.ts

@@ -16,11 +16,22 @@
 
 import { ScmIntegration, ScmIntegrationsGroup } from './types';
 
-/** Checks whether the given url is a valid host */
-export function isValidHost(url: string): boolean {
+/** Checks whether the given argument is a valid URL hostname */
+export function isValidHost(host: string): boolean {
   const check = new URL('http://example.com');
-  check.host = url;
-  return check.host === url;
+  check.host = host;
+  return check.host === host;
+}
+
+/** Checks whether the given argument is a valid URL */
+export function isValidUrl(url: string): boolean {
+  try {
+    // eslint-disable-next-line no-new
+    new URL(url);
+    return true;
+  } catch {
+    return false;
+  }
 }
 
 export function basicIntegrations<T extends ScmIntegration>(