From 13231e101b5b07a484e05bea71f4d5585e2c7f86 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 20 Jan 2021 12:47:17 +0100 Subject: [PATCH] chore: fixing some PR comments --- .../src/github/GithubCredentialsProvider.ts | 4 +-- .../integration/src/gitlab/config.test.ts | 7 +++- packages/integration/src/gitlab/config.ts | 17 +++++++-- .../scaffolder/stages/prepare/gitlab.test.ts | 2 +- .../src/scaffolder/stages/publish/azure.ts | 4 +-- .../scaffolder/stages/publish/bitbucket.ts | 4 +-- .../src/scaffolder/stages/publish/github.ts | 4 +-- .../src/scaffolder/stages/publish/gitlab.ts | 6 ++-- .../src/service/router.test.ts | 2 +- .../components/TemplatePage/TemplatePage.tsx | 36 +++++++++++-------- 10 files changed, 55 insertions(+), 31 deletions(-) diff --git a/packages/integration/src/github/GithubCredentialsProvider.ts b/packages/integration/src/github/GithubCredentialsProvider.ts index 843ef629ab..dfb809ee5f 100644 --- a/packages/integration/src/github/GithubCredentialsProvider.ts +++ b/packages/integration/src/github/GithubCredentialsProvider.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import gitUrlParse from 'git-url-parse'; +import parseGitUrl from 'git-url-parse'; import { GithubAppConfig, GitHubIntegrationConfig } from './config'; import { createAppAuth } from '@octokit/auth-app'; import { Octokit, RestEndpointMethodTypes } from '@octokit/rest'; @@ -221,7 +221,7 @@ export class GithubCredentialsProvider { * const { token, headers } = await getCredentials({url: 'github.com/backstage/foobar'}) */ async getCredentials(opts: { url: string }): Promise { - const parsed = gitUrlParse(opts.url); + const parsed = parseGitUrl(opts.url); const owner = parsed.owner || parsed.name; const repo = parsed.owner ? parsed.name : undefined; diff --git a/packages/integration/src/gitlab/config.test.ts b/packages/integration/src/gitlab/config.test.ts index bd67202ac7..7e298e6043 100644 --- a/packages/integration/src/gitlab/config.test.ts +++ b/packages/integration/src/gitlab/config.test.ts @@ -31,11 +31,14 @@ describe('readGitLabIntegrationConfig', () => { buildConfig({ host: 'a.com', token: 't', + baseUrl: 'https://baseurl.for.me/gitlab', }), ); + expect(output).toEqual({ host: 'a.com', token: 't', + baseUrl: 'https://baseurl.for.me/gitlab', }); }); @@ -43,7 +46,8 @@ describe('readGitLabIntegrationConfig', () => { const output = readGitLabIntegrationConfig(buildConfig({})); expect(output).toEqual({ host: 'gitlab.com', - apiBaseUrl: undefined, + apiBaseUrl: 'gitlab.com/api/v4', + baseUrl: 'https://gitlab.com', }); }); @@ -78,6 +82,7 @@ describe('readGitLabIntegrationConfigs', () => { expect(output).toContainEqual({ host: 'a.com', token: 't', + baseUrl: 'https://a.com', }); }); diff --git a/packages/integration/src/gitlab/config.ts b/packages/integration/src/gitlab/config.ts index 1666ece803..87a402176f 100644 --- a/packages/integration/src/gitlab/config.ts +++ b/packages/integration/src/gitlab/config.ts @@ -18,7 +18,7 @@ import { Config } from '@backstage/config'; import { isValidHost } from '../helpers'; const GITLAB_HOST = 'gitlab.com'; - +const GITLAB_API_BASE_URL = 'gitlab.com/api/v4'; /** * The configuration parameters for a single GitLab integration. */ @@ -29,6 +29,7 @@ export type GitLabIntegrationConfig = { host: string; /** + * @deprecated * The base URL of the API of this provider, e.g. "https://gitlab.com/api/v4", * with no trailing slash. * @@ -45,6 +46,14 @@ export type GitLabIntegrationConfig = { * If no token is specified, anonymous access is used. */ token?: string; + + /** + * The baseUrl of this provider, e.g "https://gitlab.com", + * which is passed into the gitlab client. + * + * If no baseUrl is provided, it will default to https://${host} + */ + baseUrl?: string; }; /** @@ -58,6 +67,7 @@ export function readGitLabIntegrationConfig( const host = config.getOptionalString('host') ?? GITLAB_HOST; let apiBaseUrl = config.getOptionalString('apiBaseUrl'); const token = config.getOptionalString('token'); + const baseUrl = config.getOptionalString('baseUrl') ?? `https://${host}`; if (!isValidHost(host)) { throw new Error( @@ -67,8 +77,11 @@ export function readGitLabIntegrationConfig( if (apiBaseUrl) { apiBaseUrl = apiBaseUrl.replace(/\/+$/, ''); + } else if (host === GITLAB_HOST) { + apiBaseUrl = GITLAB_API_BASE_URL; } - return { host, token, apiBaseUrl }; + + return { host, token, apiBaseUrl, baseUrl }; } /** diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/gitlab.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/gitlab.test.ts index a223f72bd5..19cbf793e4 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/gitlab.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/gitlab.test.ts @@ -32,7 +32,7 @@ const mockTemplate = (): TemplateEntityV1alpha1 => ({ metadata: { annotations: { [LOCATION_ANNOTATION]: - 'gitlab:https://gitlab.com/benjdlambert/backstage-graphql-template/-/blob/master/template.yaml', + 'url:https://gitlab.com/benjdlambert/backstage-graphql-template/-/blob/master/template.yaml', }, name: 'graphql-starter', title: 'GraphQL Service', diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/azure.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/azure.ts index 6d2aeea305..858809556f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/azure.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/azure.ts @@ -19,7 +19,7 @@ import { IGitApi } from 'azure-devops-node-api/GitApi'; import { GitRepositoryCreateOptions } from 'azure-devops-node-api/interfaces/GitInterfaces'; import { initRepoAndPush } from './helpers'; import { AzureIntegrationConfig } from '@backstage/integration'; -import gitUrlParse from 'git-url-parse'; +import parseGitUrl from 'git-url-parse'; import { getPersonalAccessTokenHandler, WebApi } from 'azure-devops-node-api'; export class AzurePublisher implements PublisherBase { @@ -43,7 +43,7 @@ export class AzurePublisher implements PublisherBase { directory, logger, }: PublisherOptions): Promise { - const { owner, name } = gitUrlParse(values.storePath); + const { owner, name } = parseGitUrl(values.storePath); const remoteUrl = await this.createRemote({ project: owner, diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/bitbucket.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/bitbucket.ts index d000eb7ddb..ddfc59a6f7 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/bitbucket.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/bitbucket.ts @@ -18,7 +18,7 @@ import { PublisherBase, PublisherOptions, PublisherResult } from './types'; import { initRepoAndPush } from './helpers'; import fetch from 'cross-fetch'; import { BitbucketIntegrationConfig } from '@backstage/integration'; -import gitUrlParse from 'git-url-parse'; +import parseGitUrl from 'git-url-parse'; // TODO(blam): We should probably start to use a bitbucket client here that we can change // the baseURL to point at on-prem or public bitbucket versions like we do for @@ -46,7 +46,7 @@ export class BitbucketPublisher implements PublisherBase { directory, logger, }: PublisherOptions): Promise { - const { owner: project, name } = gitUrlParse(values.storePath); + const { owner: project, name } = parseGitUrl(values.storePath); const description = values.description as string; const result = await this.createRemote({ diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts index cc8921ce40..01ec7b630a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts @@ -17,7 +17,7 @@ import { PublisherBase, PublisherOptions, PublisherResult } from './types'; import { initRepoAndPush } from './helpers'; import { GitHubIntegrationConfig } from '@backstage/integration'; -import gitUrlParse from 'git-url-parse'; +import parseGitUrl from 'git-url-parse'; import { Octokit } from '@octokit/rest'; export type RepoVisibilityOptions = 'private' | 'internal' | 'public'; @@ -48,7 +48,7 @@ export class GithubPublisher implements PublisherBase { directory, logger, }: PublisherOptions): Promise { - const { owner, name } = gitUrlParse(values.storePath); + const { owner, name } = parseGitUrl(values.storePath); const description = values.description as string; const access = values.access as string; diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/gitlab.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/gitlab.ts index b8139be49e..120ca751cd 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/gitlab.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/gitlab.ts @@ -18,7 +18,7 @@ import { PublisherBase, PublisherOptions, PublisherResult } from './types'; import { Gitlab } from '@gitbeaker/node'; import { Gitlab as GitlabClient } from '@gitbeaker/core'; import { initRepoAndPush } from './helpers'; -import gitUrlParse from 'git-url-parse'; +import parseGitUrl from 'git-url-parse'; import { GitLabIntegrationConfig } from '@backstage/integration'; @@ -28,7 +28,7 @@ export class GitlabPublisher implements PublisherBase { return undefined; } - const client = new Gitlab({ host: config.apiBaseUrl, token: config.token }); + const client = new Gitlab({ host: config.baseUrl, token: config.token }); return new GitlabPublisher(config.token, client); } @@ -42,7 +42,7 @@ export class GitlabPublisher implements PublisherBase { directory, logger, }: PublisherOptions): Promise { - const { owner, name } = gitUrlParse(values.storePath); + const { owner, name } = parseGitUrl(values.storePath); const remoteUrl = await this.createRemote({ owner, diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index 3bfd81ae69..785e099c3f 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -65,7 +65,7 @@ describe('createRouter - working directory', () => { kind: 'Template', metadata: { annotations: { - 'backstage.io/managed-by-location': 'azure:https://dev.azure.com', + 'backstage.io/managed-by-location': 'url:https://dev.azure.com', }, }, spec: { diff --git a/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx b/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx index 11c1029c40..60f5ad8dec 100644 --- a/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx +++ b/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx @@ -39,7 +39,7 @@ import { rootRoute } from '../../routes'; import { JobStatusModal } from '../JobStatusModal'; import { MultistepJsonForm } from '../MultistepJsonForm'; import { useJobPolling } from '../hooks/useJobPolling'; -import gitParse from 'git-url-parse'; +import parseGitUrl from 'git-url-parse'; const useTemplate = ( templateName: string, @@ -180,22 +180,28 @@ export const TemplatePage = () => { schema: OWNER_REPO_SCHEMA, validate: (formData, errors) => { const { storePath } = formData; - const parsedUrl = gitParse(storePath); + try { + const parsedUrl = parseGitUrl(storePath); - if ( - !parsedUrl.resource || - !parsedUrl.owner || - !parsedUrl.name - ) { - if (parsedUrl.resource === 'dev.azure.com') { - errors.storePath.addError( - "The store path should be formatted like https://dev.azure.com/{org}/{project}/_git/{repo} for Azure URL's", - ); - } else { - errors.storePath.addError( - 'The store path should be a complete Git URL to the new repository location. For example: https://github.com/{owner}/{repo}', - ); + if ( + !parsedUrl.resource || + !parsedUrl.owner || + !parsedUrl.name + ) { + if (parsedUrl.resource === 'dev.azure.com') { + errors.storePath.addError( + "The store path should be formatted like https://dev.azure.com/{org}/{project}/_git/{repo} for Azure URL's", + ); + } else { + errors.storePath.addError( + 'The store path should be a complete Git URL to the new repository location. For example: https://github.com/{owner}/{repo}', + ); + } } + } catch (ex) { + errors.storePath.addError( + `Failed validation of the store pathn with message ${ex.message}`, + ); } return errors;