From 0b92a1e7401bcf996e285774538cb72054b59663 Mon Sep 17 00:00:00 2001 From: "@pawelmitka" Date: Mon, 30 Aug 2021 11:10:25 +0200 Subject: [PATCH] refactor(plugin-scaffolder-backend): GitHub integration Created `github/helpers.ts` in order to extract common functions related to token and Octokit client. Duplicated tests have been moved appropriately. Signed-off-by: @pawelmitka --- .changeset/yellow-windows-fix.md | 5 ++ .../github/githubActionsDispatch.test.ts | 36 -------- .../builtin/github/githubActionsDispatch.ts | 52 ++---------- .../builtin/github/githubWebhook.test.ts | 36 -------- .../actions/builtin/github/githubWebhook.ts | 50 ++--------- .../actions/builtin/github/helpers.test.ts | 81 ++++++++++++++++++ .../actions/builtin/github/helpers.ts | 83 +++++++++++++++++++ .../actions/builtin/publish/github.test.ts | 36 -------- .../actions/builtin/publish/github.ts | 55 ++---------- 9 files changed, 185 insertions(+), 249 deletions(-) create mode 100644 .changeset/yellow-windows-fix.md create mode 100644 plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.test.ts create mode 100644 plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts diff --git a/.changeset/yellow-windows-fix.md b/.changeset/yellow-windows-fix.md new file mode 100644 index 0000000000..7572fdce88 --- /dev/null +++ b/.changeset/yellow-windows-fix.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +refactor: extract common Octokit related code and use it in actions: `publish:github`, `github:actions:dispatch`, `github:webhook`. diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubActionsDispatch.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubActionsDispatch.test.ts index 29e4338a39..ab1bbfc488 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubActionsDispatch.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubActionsDispatch.test.ts @@ -54,42 +54,6 @@ describe('github:actions:dispatch', () => { jest.resetAllMocks(); }); - it('should throw an error when the repoUrl is not well formed', async () => { - await expect( - action.handler({ - ...mockContext, - input: { repoUrl: 'github.com?repo=bob' }, - }), - ).rejects.toThrow(/missing owner/); - - await expect( - action.handler({ - ...mockContext, - input: { repoUrl: 'github.com?owner=owner' }, - }), - ).rejects.toThrow(/missing repo/); - }); - - it('should throw if there is no integration config provided', async () => { - await expect( - action.handler({ - ...mockContext, - input: { repoUrl: 'missing.com?repo=bob&owner=owner' }, - }), - ).rejects.toThrow(/No matching integration configuration/); - }); - - it('should throw if there is no token in the integration config that is returned', async () => { - await expect( - action.handler({ - ...mockContext, - input: { - repoUrl: 'ghe.github.com?repo=bob&owner=owner', - }, - }), - ).rejects.toThrow(/No token available for host/); - }); - it('should call the githubApis for creating WorkflowDispatch', async () => { mockGithubClient.rest.actions.createWorkflowDispatch.mockResolvedValue({ data: { diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubActionsDispatch.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubActionsDispatch.ts index 5dfba7776e..c2d750051e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubActionsDispatch.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubActionsDispatch.ts @@ -13,27 +13,15 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { InputError } from '@backstage/errors'; -import { - GithubCredentialsProvider, - ScmIntegrationRegistry, -} from '@backstage/integration'; -import { Octokit } from '@octokit/rest'; -import { parseRepoUrl } from '../publish/util'; +import { ScmIntegrationRegistry } from '@backstage/integration'; import { createTemplateAction } from '../../createTemplateAction'; +import { getOctokit } from './helpers'; export function createGithubActionsDispatchAction(options: { integrations: ScmIntegrationRegistry; }) { const { integrations } = options; - const credentialsProviders = new Map( - integrations.github.list().map(integration => { - const provider = GithubCredentialsProvider.create(integration.config); - return [integration.config.host, provider]; - }), - ); - return createTemplateAction<{ repoUrl: string; workflowId: string; @@ -69,43 +57,13 @@ export function createGithubActionsDispatchAction(options: { async handler(ctx) { const { repoUrl, workflowId, branchOrTagName } = ctx.input; - const { owner, repo, host } = parseRepoUrl(repoUrl, integrations); - - if (!owner) { - throw new InputError( - `No owner provided for host: ${host}, and repo ${repo}`, - ); - } - ctx.logger.info( `Dispatching workflow ${workflowId} for repo ${repoUrl} on ${branchOrTagName}`, ); - const credentialsProvider = credentialsProviders.get(host); - const integrationConfig = integrations.github.byHost(host); - - if (!credentialsProvider || !integrationConfig) { - throw new InputError( - `No matching integration configuration for host ${host}, please check your integrations config`, - ); - } - - const { token } = await credentialsProvider.getCredentials({ - url: `https://${host}/${encodeURIComponent(owner)}/${encodeURIComponent( - repo, - )}`, - }); - - if (!token) { - throw new InputError( - `No token available for host: ${host}, with owner ${owner}, and repo ${repo}`, - ); - } - - const client = new Octokit({ - auth: token, - baseUrl: integrationConfig.config.apiBaseUrl, - previews: ['nebula-preview'], + const { client, owner, repo } = await getOctokit({ + integrations, + repoUrl, }); await client.rest.actions.createWorkflowDispatch({ diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.test.ts index 8f11eefce1..3ea3037b90 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.test.ts @@ -54,42 +54,6 @@ describe('github:repository:webhook:create', () => { jest.resetAllMocks(); }); - it('should throw an error when the repoUrl is not well formed', async () => { - await expect( - action.handler({ - ...mockContext, - input: { repoUrl: 'github.com?repo=bob' }, - }), - ).rejects.toThrow(/missing owner/); - - await expect( - action.handler({ - ...mockContext, - input: { repoUrl: 'github.com?owner=owner' }, - }), - ).rejects.toThrow(/missing repo/); - }); - - it('should throw if there is no integration config provided', async () => { - await expect( - action.handler({ - ...mockContext, - input: { repoUrl: 'missing.com?repo=bob&owner=owner' }, - }), - ).rejects.toThrow(/No matching integration configuration/); - }); - - it('should throw if there is no token in the integration config that is returned', async () => { - await expect( - action.handler({ - ...mockContext, - input: { - repoUrl: 'ghe.github.com?repo=bob&owner=owner', - }, - }), - ).rejects.toThrow(/No token available for host/); - }); - it('should call the githubApi for creating repository Webhook', async () => { const repoUrl = 'github.com?repo=repo&owner=owner'; const webhookUrl = 'https://example.com/payload'; diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.ts index 50877d3bca..5b83eca3fd 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.ts @@ -13,14 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { InputError } from '@backstage/errors'; -import { - GithubCredentialsProvider, - ScmIntegrationRegistry, -} from '@backstage/integration'; -import { Octokit } from '@octokit/rest'; -import { parseRepoUrl } from '../publish/util'; +import { ScmIntegrationRegistry } from '@backstage/integration'; import { createTemplateAction } from '../../createTemplateAction'; +import { getOctokit } from './helpers'; type ContentType = 'form' | 'json'; @@ -29,13 +24,6 @@ export function createGithubWebhookAction(options: { }) { const { integrations } = options; - const credentialsProviders = new Map( - integrations.github.list().map(integration => { - const provider = GithubCredentialsProvider.create(integration.config); - return [integration.config.host, provider]; - }), - ); - return createTemplateAction<{ repoUrl: string; webhookUrl: string; @@ -106,39 +94,11 @@ export function createGithubWebhookAction(options: { insecureSsl = false, } = ctx.input; - const { owner, repo, host } = parseRepoUrl(repoUrl, integrations); - - if (!owner) { - throw new InputError(`No owner provided for repo ${repoUrl}`); - } - ctx.logger.info(`Creating webhook ${webhookUrl} for repo ${repoUrl}`); - const credentialsProvider = credentialsProviders.get(host); - const integrationConfig = integrations.github.byHost(host); - - if (!credentialsProvider || !integrationConfig) { - throw new InputError( - `No matching integration configuration for host ${host}, please check your integrations config`, - ); - } - - const { token } = await credentialsProvider.getCredentials({ - url: `https://${host}/${encodeURIComponent(owner)}/${encodeURIComponent( - repo, - )}`, - }); - - if (!token) { - throw new InputError( - `No token available for host: ${host}, with owner ${owner}, and repo ${repo}`, - ); - } - - const client = new Octokit({ - auth: token, - baseUrl: integrationConfig.config.apiBaseUrl, - previews: ['nebula-preview'], + const { client, owner, repo } = await getOctokit({ + integrations, + repoUrl, }); try { diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.test.ts new file mode 100644 index 0000000000..e16f76d89f --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.test.ts @@ -0,0 +1,81 @@ +/* + * Copyright 2021 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 { getOctokit } from './helpers'; +import { ScmIntegrations } from '@backstage/integration'; +import { ConfigReader } from '@backstage/config'; + +describe('getOctokit', () => { + const config = new ConfigReader({ + integrations: { + github: [ + { host: 'github.com', token: 'tokenlols' }, + { host: 'ghe.github.com' }, + ], + }, + }); + + const integrations = ScmIntegrations.fromConfig(config); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + it('should throw an error when the repoUrl is not well formed', async () => { + await expect( + getOctokit({ + integrations, + repoUrl: 'github.com?repo=bob', + }), + ).rejects.toThrow(/missing owner/); + + await expect( + getOctokit({ + integrations, + repoUrl: 'github.com?owner=owner', + }), + ).rejects.toThrow(/missing repo/); + }); + + it('should throw if there is no integration config provided', async () => { + await expect( + getOctokit({ + integrations, + repoUrl: 'missing.com?repo=bob&owner=owner', + }), + ).rejects.toThrow(/No matching integration configuration/); + }); + + it('should throw if there is no token in the integration config that is returned', async () => { + await expect( + getOctokit({ + integrations, + repoUrl: 'ghe.github.com?repo=bob&owner=owner', + }), + ).rejects.toThrow(/No token available for host/); + }); + + it('should return proper Octokit', async () => { + const { client, token, owner, repo } = await getOctokit({ + integrations, + repoUrl: 'github.com?repo=bob&owner=owner', + }); + expect(client).toBeDefined(); + expect(token).toBe('tokenlols'); + expect(owner).toBe('owner'); + expect(repo).toBe('bob'); + }); +}); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts new file mode 100644 index 0000000000..89c7b43a1a --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts @@ -0,0 +1,83 @@ +/* + * Copyright 2021 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 { InputError } from '@backstage/errors'; +import { + GithubCredentialsProvider, + ScmIntegrationRegistry, +} from '@backstage/integration'; +import { Octokit } from '@octokit/rest'; +import { parseRepoUrl } from '../publish/util'; + +type OctokitOptions = { + integrations: ScmIntegrationRegistry; + repoUrl: string; +}; + +type OctokitIntegration = { + client: Octokit; + token: string; + owner: string; + repo: string; +}; + +export const getOctokit = async ({ + integrations, + repoUrl, +}: OctokitOptions): Promise => { + const { owner, repo, host } = parseRepoUrl(repoUrl, integrations); + + if (!owner) { + throw new InputError(`No owner provided for repo ${repoUrl}`); + } + + const integrationConfig = integrations.github.byHost(host)?.config; + + if (!integrationConfig) { + throw new InputError(`No integration for host ${host}`); + } + + const credentialsProvider = + GithubCredentialsProvider.create(integrationConfig); + + if (!credentialsProvider) { + throw new InputError( + `No matching credentials for host ${host}, please check your integrations config`, + ); + } + + // TODO(blam): Consider changing this API to have owner, repo interface instead of URL as the it's + // needless to create URL and then parse again the other side. + const { token } = await credentialsProvider.getCredentials({ + url: `https://${host}/${encodeURIComponent(owner)}/${encodeURIComponent( + repo, + )}`, + }); + + if (!token) { + throw new InputError( + `No token available for host: ${host}, with owner ${owner}, and repo ${repo}`, + ); + } + + const client = new Octokit({ + auth: token, + baseUrl: integrationConfig.apiBaseUrl, + previews: ['nebula-preview'], + }); + + return { client, token, owner, repo }; +}; diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.test.ts index 4461d227e8..6a3ccf5e38 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.test.ts @@ -60,42 +60,6 @@ describe('publish:github', () => { jest.resetAllMocks(); }); - it('should throw an error when the repoUrl is not well formed', async () => { - await expect( - action.handler({ - ...mockContext, - input: { repoUrl: 'github.com?repo=bob' }, - }), - ).rejects.toThrow(/missing owner/); - - await expect( - action.handler({ - ...mockContext, - input: { repoUrl: 'github.com?owner=owner' }, - }), - ).rejects.toThrow(/missing repo/); - }); - - it('should throw if there is no integration config provided', async () => { - await expect( - action.handler({ - ...mockContext, - input: { repoUrl: 'missing.com?repo=bob&owner=owner' }, - }), - ).rejects.toThrow(/No matching integration configuration/); - }); - - it('should throw if there is no token in the integration config that is returned', async () => { - await expect( - action.handler({ - ...mockContext, - input: { - repoUrl: 'ghe.github.com?repo=bob&owner=owner', - }, - }), - ).rejects.toThrow(/No token available for host/); - }); - it('should call the githubApis with the correct values for createInOrg', async () => { mockGithubClient.users.getByUsername.mockResolvedValue({ data: { type: 'Organization' }, diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts index 5a358822af..857847825a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts @@ -13,19 +13,15 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { InputError } from '@backstage/errors'; -import { - GithubCredentialsProvider, - ScmIntegrationRegistry, -} from '@backstage/integration'; -import { Octokit } from '@octokit/rest'; +import { ScmIntegrationRegistry } from '@backstage/integration'; import { enableBranchProtectionOnDefaultRepoBranch, initRepoAndPush, } from '../helpers'; -import { getRepoSourceDirectory, parseRepoUrl } from './util'; +import { getRepoSourceDirectory } from './util'; import { createTemplateAction } from '../../createTemplateAction'; import { Config } from '@backstage/config'; +import { getOctokit } from '../github/helpers'; type Permission = 'pull' | 'push' | 'admin' | 'maintain' | 'triage'; type Collaborator = { access: Permission; username: string }; @@ -36,13 +32,6 @@ export function createPublishGithubAction(options: { }) { const { integrations, config } = options; - const credentialsProviders = new Map( - integrations.github.list().map(integration => { - const provider = GithubCredentialsProvider.create(integration.config); - return [integration.config.host, provider]; - }), - ); - return createTemplateAction<{ repoUrl: string; description?: string; @@ -151,41 +140,9 @@ export function createPublishGithubAction(options: { topics, } = ctx.input; - const { owner, repo, host } = parseRepoUrl(repoUrl, integrations); - - if (!owner) { - throw new InputError( - `No owner provided for host: ${host}, and repo ${repo}`, - ); - } - - const credentialsProvider = credentialsProviders.get(host); - const integrationConfig = integrations.github.byHost(host); - - if (!credentialsProvider || !integrationConfig) { - throw new InputError( - `No matching integration configuration for host ${host}, please check your integrations config`, - ); - } - - // TODO(blam): Consider changing this API to have owner, repo interface instead of URL as the it's - // needless to create URL and then parse again the other side. - const { token } = await credentialsProvider.getCredentials({ - url: `https://${host}/${encodeURIComponent(owner)}/${encodeURIComponent( - repo, - )}`, - }); - - if (!token) { - throw new InputError( - `No token available for host: ${host}, with owner ${owner}, and repo ${repo}`, - ); - } - - const client = new Octokit({ - auth: token, - baseUrl: integrationConfig.config.apiBaseUrl, - previews: ['nebula-preview'], + const { client, token, owner, repo } = await getOctokit({ + integrations, + repoUrl, }); const user = await client.users.getByUsername({