From 0b92a1e7401bcf996e285774538cb72054b59663 Mon Sep 17 00:00:00 2001 From: "@pawelmitka" Date: Mon, 30 Aug 2021 11:10:25 +0200 Subject: [PATCH 1/5] 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({ From 2b69f8a4fd4d10c21d50e88d5f611e61251d06bd Mon Sep 17 00:00:00 2001 From: "@pawelmitka" Date: Tue, 31 Aug 2021 11:43:58 +0200 Subject: [PATCH 2/5] refactor: replace `getOctokit` with `OctokitProvider.getOctokit` to allow credentails caching Signed-off-by: @pawelmitka --- .../builtin/github/githubActionsDispatch.ts | 8 +- .../actions/builtin/github/githubWebhook.ts | 8 +- .../actions/builtin/github/helpers.test.ts | 30 ++---- .../actions/builtin/github/helpers.ts | 100 ++++++++++-------- .../actions/builtin/publish/github.ts | 8 +- 5 files changed, 72 insertions(+), 82 deletions(-) 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 c2d750051e..c7d2919efb 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubActionsDispatch.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubActionsDispatch.ts @@ -15,12 +15,13 @@ */ import { ScmIntegrationRegistry } from '@backstage/integration'; import { createTemplateAction } from '../../createTemplateAction'; -import { getOctokit } from './helpers'; +import { OctokitProvider } from './helpers'; export function createGithubActionsDispatchAction(options: { integrations: ScmIntegrationRegistry; }) { const { integrations } = options; + const octokitProvider = new OctokitProvider(integrations); return createTemplateAction<{ repoUrl: string; @@ -61,10 +62,7 @@ export function createGithubActionsDispatchAction(options: { `Dispatching workflow ${workflowId} for repo ${repoUrl} on ${branchOrTagName}`, ); - const { client, owner, repo } = await getOctokit({ - integrations, - repoUrl, - }); + const { client, owner, repo } = await octokitProvider.getOctokit(repoUrl); await client.rest.actions.createWorkflowDispatch({ owner, 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 5b83eca3fd..eabfebeb28 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.ts @@ -15,7 +15,7 @@ */ import { ScmIntegrationRegistry } from '@backstage/integration'; import { createTemplateAction } from '../../createTemplateAction'; -import { getOctokit } from './helpers'; +import { OctokitProvider } from './helpers'; type ContentType = 'form' | 'json'; @@ -23,6 +23,7 @@ export function createGithubWebhookAction(options: { integrations: ScmIntegrationRegistry; }) { const { integrations } = options; + const octokitProvider = new OctokitProvider(integrations); return createTemplateAction<{ repoUrl: string; @@ -96,10 +97,7 @@ export function createGithubWebhookAction(options: { ctx.logger.info(`Creating webhook ${webhookUrl} for repo ${repoUrl}`); - const { client, owner, repo } = await getOctokit({ - integrations, - repoUrl, - }); + const { client, owner, repo } = await octokitProvider.getOctokit(repoUrl); try { const insecure_ssl = insecureSsl ? '1' : '0'; 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 index e16f76d89f..a72a775701 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.test.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { getOctokit } from './helpers'; +import { OctokitProvider } from './helpers'; import { ScmIntegrations } from '@backstage/integration'; import { ConfigReader } from '@backstage/config'; @@ -29,6 +29,7 @@ describe('getOctokit', () => { }); const integrations = ScmIntegrations.fromConfig(config); + const octokitProvider = new OctokitProvider(integrations); beforeEach(() => { jest.resetAllMocks(); @@ -36,43 +37,30 @@ describe('getOctokit', () => { it('should throw an error when the repoUrl is not well formed', async () => { await expect( - getOctokit({ - integrations, - repoUrl: 'github.com?repo=bob', - }), + octokitProvider.getOctokit('github.com?repo=bob'), ).rejects.toThrow(/missing owner/); await expect( - getOctokit({ - integrations, - repoUrl: 'github.com?owner=owner', - }), + octokitProvider.getOctokit('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', - }), + octokitProvider.getOctokit('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', - }), + octokitProvider.getOctokit('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', - }); + const { client, token, owner, repo } = await octokitProvider.getOctokit( + 'github.com?repo=bob&owner=owner', + ); expect(client).toBeDefined(); expect(token).toBe('tokenlols'); expect(owner).toBe('owner'); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts index 89c7b43a1a..4ed10b9d64 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts @@ -22,62 +22,68 @@ import { import { Octokit } from '@octokit/rest'; import { parseRepoUrl } from '../publish/util'; -type OctokitOptions = { - integrations: ScmIntegrationRegistry; - repoUrl: string; -}; - -type OctokitIntegration = { +export 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); +export class OctokitProvider { + private readonly integrations: ScmIntegrationRegistry; + private readonly credentialsProviders: Map; - 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`, + constructor(integrations: ScmIntegrationRegistry) { + this.integrations = integrations; + this.credentialsProviders = new Map( + integrations.github.list().map(integration => { + const provider = GithubCredentialsProvider.create(integration.config); + return [integration.config.host, provider]; + }), ); } - // 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, - )}`, - }); + async getOctokit(repoUrl: string): Promise { + const { owner, repo, host } = parseRepoUrl(repoUrl, this.integrations); - if (!token) { - throw new InputError( - `No token available for host: ${host}, with owner ${owner}, and repo ${repo}`, - ); + if (!owner) { + throw new InputError(`No owner provided for repo ${repoUrl}`); + } + + const integrationConfig = this.integrations.github.byHost(host)?.config; + + if (!integrationConfig) { + throw new InputError(`No integration for host ${host}`); + } + + const credentialsProvider = this.credentialsProviders.get(host); + + 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 }; } - - 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.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts index 857847825a..eaedbf275f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts @@ -21,7 +21,7 @@ import { import { getRepoSourceDirectory } from './util'; import { createTemplateAction } from '../../createTemplateAction'; import { Config } from '@backstage/config'; -import { getOctokit } from '../github/helpers'; +import { OctokitProvider } from '../github/helpers'; type Permission = 'pull' | 'push' | 'admin' | 'maintain' | 'triage'; type Collaborator = { access: Permission; username: string }; @@ -31,6 +31,7 @@ export function createPublishGithubAction(options: { config: Config; }) { const { integrations, config } = options; + const octokitProvider = new OctokitProvider(integrations); return createTemplateAction<{ repoUrl: string; @@ -140,10 +141,9 @@ export function createPublishGithubAction(options: { topics, } = ctx.input; - const { client, token, owner, repo } = await getOctokit({ - integrations, + const { client, token, owner, repo } = await octokitProvider.getOctokit( repoUrl, - }); + ); const user = await client.users.getByUsername({ username: owner, From a239a4a5e554d50bbf74a5e67e9e05aa0b441cfa Mon Sep 17 00:00:00 2001 From: "@pawelmitka" Date: Wed, 1 Sep 2021 09:30:50 +0200 Subject: [PATCH 3/5] refactor: rename helpers.ts to OctokitProvider.ts and add docstrings Signed-off-by: @pawelmitka --- .../{helpers.test.ts => OctokitProvider.test.ts} | 2 +- .../builtin/github/{helpers.ts => OctokitProvider.ts} | 10 +++++++++- .../actions/builtin/github/githubActionsDispatch.ts | 2 +- .../scaffolder/actions/builtin/github/githubWebhook.ts | 2 +- .../src/scaffolder/actions/builtin/publish/github.ts | 2 +- 5 files changed, 13 insertions(+), 5 deletions(-) rename plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/{helpers.test.ts => OctokitProvider.test.ts} (97%) rename plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/{helpers.ts => OctokitProvider.ts} (90%) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.test.ts similarity index 97% rename from plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.test.ts rename to plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.test.ts index a72a775701..91b23f40dd 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.test.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { OctokitProvider } from './helpers'; +import { OctokitProvider } from './OctokitProvider'; import { ScmIntegrations } from '@backstage/integration'; import { ConfigReader } from '@backstage/config'; diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts similarity index 90% rename from plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts rename to plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts index 4ed10b9d64..bea027a91b 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts @@ -28,7 +28,10 @@ export type OctokitIntegration = { owner: string; repo: string; }; - +/** + * OctokitProvider provides Octokit client based on ScmIntegrationsRegistry configuration. + * OctokitProvider supports GitHub credentials caching out of the box. + */ export class OctokitProvider { private readonly integrations: ScmIntegrationRegistry; private readonly credentialsProviders: Map; @@ -43,6 +46,11 @@ export class OctokitProvider { ); } + /** + * gets standard Octokit client based on repository URL. + * + * @param repoUrl Repository URL + */ async getOctokit(repoUrl: string): Promise { const { owner, repo, host } = parseRepoUrl(repoUrl, this.integrations); 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 c7d2919efb..661101d184 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubActionsDispatch.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubActionsDispatch.ts @@ -15,7 +15,7 @@ */ import { ScmIntegrationRegistry } from '@backstage/integration'; import { createTemplateAction } from '../../createTemplateAction'; -import { OctokitProvider } from './helpers'; +import { OctokitProvider } from './OctokitProvider'; export function createGithubActionsDispatchAction(options: { integrations: ScmIntegrationRegistry; 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 eabfebeb28..33ed415062 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.ts @@ -15,7 +15,7 @@ */ import { ScmIntegrationRegistry } from '@backstage/integration'; import { createTemplateAction } from '../../createTemplateAction'; -import { OctokitProvider } from './helpers'; +import { OctokitProvider } from './OctokitProvider'; type ContentType = 'form' | 'json'; 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 eaedbf275f..b32f9b5b3f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts @@ -21,7 +21,7 @@ import { import { getRepoSourceDirectory } from './util'; import { createTemplateAction } from '../../createTemplateAction'; import { Config } from '@backstage/config'; -import { OctokitProvider } from '../github/helpers'; +import { OctokitProvider } from '../github/OctokitProvider'; type Permission = 'pull' | 'push' | 'admin' | 'maintain' | 'triage'; type Collaborator = { access: Permission; username: string }; From 0b93caefb675032d1cb2767a9cf5fc2a0df7388d Mon Sep 17 00:00:00 2001 From: "@pawelmitka" Date: Wed, 1 Sep 2021 12:41:52 +0200 Subject: [PATCH 4/5] refactor: export `OctokitProvider` Signed-off-by: @pawelmitka --- .../src/scaffolder/actions/builtin/github/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/index.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/index.ts index 07614e8a03..865c433c16 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/index.ts @@ -16,3 +16,4 @@ export { createGithubActionsDispatchAction } from './githubActionsDispatch'; export { createGithubWebhookAction } from './githubWebhook'; +export { OctokitProvider } from './OctokitProvider'; From d02960f76492028a6c5e10a17d9104480558927c Mon Sep 17 00:00:00 2001 From: "@pawelmitka" Date: Wed, 1 Sep 2021 12:52:06 +0200 Subject: [PATCH 5/5] fix: update plugins/scaffolder-backend/api-report.md Signed-off-by: @pawelmitka --- plugins/scaffolder-backend/api-report.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index dfda1ff2fc..5d848e42d2 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -14,6 +14,7 @@ import express from 'express'; import { JsonObject } from '@backstage/config'; import { JsonValue } from '@backstage/config'; import { Logger as Logger_2 } from 'winston'; +import { Octokit } from '@octokit/rest'; import { PluginDatabaseManager } from '@backstage/backend-common'; import { Schema } from 'jsonschema'; import { ScmIntegrationRegistry } from '@backstage/integration'; @@ -200,6 +201,16 @@ export function fetchContents({ outputPath: string; }): Promise; +// Warning: (ae-missing-release-tag) "OctokitProvider" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public +export class OctokitProvider { + constructor(integrations: ScmIntegrationRegistry); + // Warning: (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen + // Warning: (ae-forgotten-export) The symbol "OctokitIntegration" needs to be exported by the entry point index.d.ts + getOctokit(repoUrl: string): Promise; +} + // Warning: (ae-missing-release-tag) "RouterOptions" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public (undocumented)