From afee525a363355bf8f2010baedb72ec43b80ded3 Mon Sep 17 00:00:00 2001 From: blam Date: Mon, 21 Feb 2022 15:40:38 +0100 Subject: [PATCH 1/8] chore: deprecating the OctokitProvider Signed-off-by: blam --- .../actions/builtin/github/OctokitProvider.ts | 4 ++ .../builtin/github/githubActionsDispatch.ts | 27 +++++++----- .../actions/builtin/github/githubWebhook.ts | 31 ++++++++----- .../actions/builtin/publish/github.ts | 31 +++++++------ .../builtin/publish/githubPullRequest.ts | 43 ++++--------------- 5 files changed, 68 insertions(+), 68 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts index 3ac66a0023..d481e42c9f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts @@ -32,6 +32,8 @@ export type OctokitIntegration = { /** * OctokitProvider provides Octokit client based on ScmIntegrationsRegistry configuration. * OctokitProvider supports GitHub credentials caching out of the box. + * + * @deprecated use the internal {@link getOctokitOptions} function instead */ export class OctokitProvider { private readonly integrations: ScmIntegrationRegistry; @@ -51,6 +53,8 @@ export class OctokitProvider { * gets standard Octokit client based on repository URL. * * @param repoUrl - Repository URL + * + * @deprecated use the internal {@link getOctokitOptions} function instead */ async getOctokit( repoUrl: string, 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 0c8b4d4aab..f2329c9ca1 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubActionsDispatch.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubActionsDispatch.ts @@ -13,24 +13,21 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import { InputError } from '@backstage/errors'; import { - DefaultGithubCredentialsProvider, GithubCredentialsProvider, ScmIntegrations, } from '@backstage/integration'; +import { Octokit } from 'octokit'; import { createTemplateAction } from '../../createTemplateAction'; -import { OctokitProvider } from './OctokitProvider'; +import { parseRepoUrl } from '../publish/util'; +import { getOctokitOptions } from './helpers'; export function createGithubActionsDispatchAction(options: { integrations: ScmIntegrations; githubCredentialsProvider?: GithubCredentialsProvider; }) { const { integrations, githubCredentialsProvider } = options; - const octokitProvider = new OctokitProvider( - integrations, - githubCredentialsProvider || - DefaultGithubCredentialsProvider.fromIntegrations(integrations), - ); return createTemplateAction<{ repoUrl: string; @@ -90,9 +87,19 @@ export function createGithubActionsDispatchAction(options: { `Dispatching workflow ${workflowId} for repo ${repoUrl} on ${branchOrTagName}`, ); - const { client, owner, repo } = await octokitProvider.getOctokit( - repoUrl, - { token: providedToken }, + const { owner, repo } = parseRepoUrl(repoUrl, integrations); + + if (!owner) { + throw new InputError('Invalid repository owner provided in repoUrl'); + } + + const client = new Octokit( + await getOctokitOptions({ + integrations, + repoUrl, + credentialsProvider: githubCredentialsProvider, + token: providedToken, + }), ); await client.rest.actions.createWorkflowDispatch({ 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 0c93ae1dd5..dc52bd5fb4 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.ts @@ -14,27 +14,27 @@ * limitations under the License. */ import { - DefaultGithubCredentialsProvider, GithubCredentialsProvider, ScmIntegrationRegistry, } from '@backstage/integration'; import { createTemplateAction } from '../../createTemplateAction'; -import { OctokitProvider } from './OctokitProvider'; import { emitterEventNames } from '@octokit/webhooks'; import { assertError } from '@backstage/errors'; +import { Octokit } from 'octokit'; +import { getOctokitOptions } from './helpers'; +import { parseRepoUrl } from '../publish/util'; export function createGithubWebhookAction(options: { integrations: ScmIntegrationRegistry; defaultWebhookSecret?: string; githubCredentialsProvider?: GithubCredentialsProvider; }) { - const { integrations, defaultWebhookSecret, githubCredentialsProvider } = - options; - const octokitProvider = new OctokitProvider( + const { integrations, - githubCredentialsProvider ?? - DefaultGithubCredentialsProvider.fromIntegrations(integrations), - ); + defaultWebhookSecret, + githubCredentialsProvider, + } = options; + const eventNames = emitterEventNames.filter(event => !event.includes('.')); return createTemplateAction<{ @@ -127,12 +127,21 @@ export function createGithubWebhookAction(options: { } = ctx.input; ctx.logger.info(`Creating webhook ${webhookUrl} for repo ${repoUrl}`); + const { owner, repo } = parseRepoUrl(repoUrl, integrations); - const { client, owner, repo } = await octokitProvider.getOctokit( - repoUrl, - { token: providedToken }, + const client = new Octokit( + await getOctokitOptions({ + integrations, + credentialsProvider: githubCredentialsProvider, + repoUrl: repoUrl, + token: providedToken, + }), ); + if (!owner) { + throw new InputError('Invalid repository owner'); + } + try { const insecure_ssl = insecureSsl ? '1' : '0'; await client.rest.repos.createWebhook({ 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 cabe40986f..418cce5043 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts @@ -14,7 +14,6 @@ * limitations under the License. */ import { - DefaultGithubCredentialsProvider, GithubCredentialsProvider, ScmIntegrationRegistry, } from '@backstage/integration'; @@ -22,11 +21,12 @@ import { enableBranchProtectionOnDefaultRepoBranch, initRepoAndPush, } from '../helpers'; -import { getRepoSourceDirectory } from './util'; +import { getRepoSourceDirectory, parseRepoUrl } from './util'; import { createTemplateAction } from '../../createTemplateAction'; import { Config } from '@backstage/config'; -import { OctokitProvider } from '../github/OctokitProvider'; -import { assertError } from '@backstage/errors'; +import { assertError, InputError } from '@backstage/errors'; +import { getOctokitOptions } from '../github/helpers'; +import { Octokit } from 'octokit'; export function createPublishGithubAction(options: { integrations: ScmIntegrationRegistry; @@ -34,11 +34,6 @@ export function createPublishGithubAction(options: { githubCredentialsProvider?: GithubCredentialsProvider; }) { const { integrations, config, githubCredentialsProvider } = options; - const octokitProvider = new OctokitProvider( - integrations, - githubCredentialsProvider || - DefaultGithubCredentialsProvider.fromIntegrations(integrations), - ); return createTemplateAction<{ repoUrl: string; @@ -160,10 +155,20 @@ export function createPublishGithubAction(options: { token: providedToken, } = ctx.input; - const { client, token, owner, repo } = await octokitProvider.getOctokit( + const { owner, repo } = parseRepoUrl(repoUrl, integrations); + + const octokitOptions = await getOctokitOptions({ + integrations, + credentialsProvider: githubCredentialsProvider, + token: providedToken, repoUrl, - { token: providedToken }, - ); + }); + + const client = new Octokit(octokitOptions); + + if (!owner) { + throw new InputError('Invalid repository owner provided in repoUrl'); + } const user = await client.rest.users.getByUsername({ username: owner, @@ -253,7 +258,7 @@ export function createPublishGithubAction(options: { defaultBranch, auth: { username: 'x-access-token', - password: token, + password: octokitOptions.token, }, logger: ctx.logger, commitMessage: config.getOptionalString( diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts index c329b6b6f3..ca70084ef5 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts @@ -20,7 +20,6 @@ import { parseRepoUrl, isExecutable } from './util'; import { GithubCredentialsProvider, ScmIntegrationRegistry, - SingleInstanceGithubCredentialsProvider, } from '@backstage/integration'; import { zipObject } from 'lodash'; import { createTemplateAction } from '../../createTemplateAction'; @@ -29,6 +28,7 @@ import { InputError, CustomErrorBase } from '@backstage/errors'; import { createPullRequest } from 'octokit-plugin-create-pull-request'; import globby from 'globby'; import { resolveSafeChildPath } from '@backstage/backend-common'; +import { getOctokitOptions } from '../github/helpers'; export type Encoding = 'utf-8' | 'base64'; @@ -65,40 +65,15 @@ export const defaultClientFactory = async ({ host = 'github.com', token: providedToken, }: ClientFactoryInput): Promise => { - const integrationConfig = integrations.github.byHost(host)?.config; + const octokitOptions = await getOctokitOptions({ + integrations, + credentialsProvider: githubCredentialsProvider, + repoUrl: `https://${host}/${owner}/${repo}`, + token: providedToken, + }); + const OctokitPR = Octokit.plugin(createPullRequest); - - if (!integrationConfig) { - throw new InputError(`No integration for host ${host}`); - } - - if (providedToken) { - return new OctokitPR({ - auth: providedToken, - baseUrl: integrationConfig.apiBaseUrl, - }); - } - - const credentialsProvider = - githubCredentialsProvider || - SingleInstanceGithubCredentialsProvider.create(integrationConfig); - - 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}`, - ); - } - - return new OctokitPR({ - auth: token, - baseUrl: integrationConfig.apiBaseUrl, - }); + return new OctokitPR(octokitOptions); }; interface CreateGithubPullRequestActionOptions { From 9553964f2858332c6b991226e4f621f8c4d8224c Mon Sep 17 00:00:00 2001 From: blam Date: Mon, 21 Feb 2022 15:41:30 +0100 Subject: [PATCH 2/8] chore: providing a new helper for generating octokit credentialsProvider Signed-off-by: blam --- .../actions/builtin/github/helpers.ts | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts 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..4f369b7263 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts @@ -0,0 +1,84 @@ +/* + * Copyright 2022 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 { + DefaultGithubCredentialsProvider, + GithubCredentialsProvider, + ScmIntegrationRegistry, +} from '@backstage/integration'; +import { OctokitOptions } from '@octokit/core/dist-types/types'; +import { parseRepoUrl } from '../publish/util'; + +interface GetOctokitOptionsInput { + integrations: ScmIntegrationRegistry; + credentialsProvider?: GithubCredentialsProvider; + token?: string; + repoUrl: string; +} +export const getOctokitOptions = async ({ + integrations, + credentialsProvider, + repoUrl, + token, +}: GetOctokitOptionsInput): 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}`); + } + + // Short circuit the internal Github Token provider the token provided + // by the action or the caller. + if (token) { + return { + auth: token, + baseUrl: integrationConfig.apiBaseUrl, + previews: ['nebula-preview'], + }; + } + + const githubCredentialsProvider = + credentialsProvider ?? + DefaultGithubCredentialsProvider.fromIntegrations(integrations); + + // 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: credentialProviderToken, + } = await githubCredentialsProvider.getCredentials({ + url: `https://${host}/${encodeURIComponent(owner)}/${encodeURIComponent( + repo, + )}`, + }); + + if (!credentialProviderToken) { + throw new InputError( + `No token available for host: ${host}, with owner ${owner}, and repo ${repo}`, + ); + } + + return { + auth: credentialProviderToken, + baseUrl: integrationConfig.apiBaseUrl, + previews: ['nebula-preview'], + }; +}; From 1a864a3b687b51a25d67d69d99826094b6ba1e85 Mon Sep 17 00:00:00 2001 From: blam Date: Mon, 21 Feb 2022 15:57:18 +0100 Subject: [PATCH 3/8] chore: updating the api report Signed-off-by: blam --- plugins/scaffolder-backend/api-report.md | 4 +++- .../actions/builtin/github/OctokitProvider.ts | 6 ++++-- .../actions/builtin/github/githubWebhook.ts | 11 ++++------- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index aea6689183..3947d64049 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -382,13 +382,15 @@ export function fetchContents({ // Warning: (ae-missing-release-tag) "OctokitProvider" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // -// @public +// @public @deprecated export class OctokitProvider { constructor( integrations: ScmIntegrationRegistry, githubCredentialsProvider?: GithubCredentialsProvider, ); // Warning: (ae-forgotten-export) The symbol "OctokitIntegration" needs to be exported by the entry point index.d.ts + // + // @deprecated getOctokit( repoUrl: string, options?: { diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts index d481e42c9f..92b8554e37 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts @@ -33,7 +33,8 @@ export type OctokitIntegration = { * OctokitProvider provides Octokit client based on ScmIntegrationsRegistry configuration. * OctokitProvider supports GitHub credentials caching out of the box. * - * @deprecated use the internal {@link getOctokitOptions} function instead + * @deprecated we are no longer providing a way from the scaffolder to generate octokit instances. + * Implement your own if you're using this method from an external package, or suse the internal `getOctokitOptions` function instead */ export class OctokitProvider { private readonly integrations: ScmIntegrationRegistry; @@ -54,7 +55,8 @@ export class OctokitProvider { * * @param repoUrl - Repository URL * - * @deprecated use the internal {@link getOctokitOptions} function instead + * @deprecated we are no longer providing a way from the scaffolder to generate octokit instances. + * Implement your own if you're using this method from an external package, or suse the internal `getOctokitOptions` function instead */ async getOctokit( repoUrl: string, 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 dc52bd5fb4..ac75dd9d82 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.ts @@ -19,7 +19,7 @@ import { } from '@backstage/integration'; import { createTemplateAction } from '../../createTemplateAction'; import { emitterEventNames } from '@octokit/webhooks'; -import { assertError } from '@backstage/errors'; +import { assertError, InputError } from '@backstage/errors'; import { Octokit } from 'octokit'; import { getOctokitOptions } from './helpers'; import { parseRepoUrl } from '../publish/util'; @@ -29,11 +29,8 @@ export function createGithubWebhookAction(options: { defaultWebhookSecret?: string; githubCredentialsProvider?: GithubCredentialsProvider; }) { - const { - integrations, - defaultWebhookSecret, - githubCredentialsProvider, - } = options; + const { integrations, defaultWebhookSecret, githubCredentialsProvider } = + options; const eventNames = emitterEventNames.filter(event => !event.includes('.')); @@ -139,7 +136,7 @@ export function createGithubWebhookAction(options: { ); if (!owner) { - throw new InputError('Invalid repository owner'); + throw new InputError('Invalid repository owner provided in repoUrl'); } try { From 5042a822f6ae510d9c3268f352d966c6dd74acf3 Mon Sep 17 00:00:00 2001 From: blam Date: Mon, 21 Feb 2022 16:08:16 +0100 Subject: [PATCH 4/8] chore: fix auth provided Signed-off-by: blam --- .../src/scaffolder/actions/builtin/publish/github.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 418cce5043..7ae9f317cf 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts @@ -258,7 +258,7 @@ export function createPublishGithubAction(options: { defaultBranch, auth: { username: 'x-access-token', - password: octokitOptions.token, + password: octokitOptions.auth, }, logger: ctx.logger, commitMessage: config.getOptionalString( From b1744f1153b33dd78dac5084f135f182ee221720 Mon Sep 17 00:00:00 2001 From: blam Date: Mon, 21 Feb 2022 16:13:07 +0100 Subject: [PATCH 5/8] chore: added changeset Signed-off-by: blam --- .changeset/poor-pens-complain.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/poor-pens-complain.md diff --git a/.changeset/poor-pens-complain.md b/.changeset/poor-pens-complain.md new file mode 100644 index 0000000000..333ec208ee --- /dev/null +++ b/.changeset/poor-pens-complain.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +- **DEPRECATED** - `OctokitProvider` has been deprecated and will be removed in upcoming versions + This helper doesn't make sense to be export from the `plugin-scaffolder-backend` and possibly will be moved into the `integrations` package at a later date. + All implementations have been moved over to a private implementation called `getOctokitOptions` which is then passed to the `Octokit` constructor. If you're using this API you should consider duplicating the logic that lives in `getOctokitOptions` and move away from the deprecated export. From 5b7079a6965768137fbe801f91bca9bb9e240436 Mon Sep 17 00:00:00 2001 From: blam Date: Mon, 21 Feb 2022 20:48:09 +0100 Subject: [PATCH 6/8] chore review comments Signed-off-by: blam Signed-off-by: blam --- .../actions/builtin/github/OctokitProvider.ts | 4 ++-- .../actions/builtin/github/githubWebhook.ts | 8 ++++---- .../actions/builtin/github/helpers.ts | 20 +++++++------------ .../actions/builtin/publish/github.ts | 8 ++++---- 4 files changed, 17 insertions(+), 23 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts index 92b8554e37..2f3c43a6b8 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts @@ -34,7 +34,7 @@ export type OctokitIntegration = { * OctokitProvider supports GitHub credentials caching out of the box. * * @deprecated we are no longer providing a way from the scaffolder to generate octokit instances. - * Implement your own if you're using this method from an external package, or suse the internal `getOctokitOptions` function instead + * Implement your own if you're using this method from an external package, or use the internal `getOctokitOptions` function instead */ export class OctokitProvider { private readonly integrations: ScmIntegrationRegistry; @@ -56,7 +56,7 @@ export class OctokitProvider { * @param repoUrl - Repository URL * * @deprecated we are no longer providing a way from the scaffolder to generate octokit instances. - * Implement your own if you're using this method from an external package, or suse the internal `getOctokitOptions` function instead + * Implement your own if you're using this method from an external package, or use the internal `getOctokitOptions` function instead */ async getOctokit( repoUrl: string, 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 ac75dd9d82..e2cf744845 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubWebhook.ts @@ -126,6 +126,10 @@ export function createGithubWebhookAction(options: { ctx.logger.info(`Creating webhook ${webhookUrl} for repo ${repoUrl}`); const { owner, repo } = parseRepoUrl(repoUrl, integrations); + if (!owner) { + throw new InputError('Invalid repository owner provided in repoUrl'); + } + const client = new Octokit( await getOctokitOptions({ integrations, @@ -135,10 +139,6 @@ export function createGithubWebhookAction(options: { }), ); - if (!owner) { - throw new InputError('Invalid repository owner provided in repoUrl'); - } - try { const insecure_ssl = insecureSsl ? '1' : '0'; await client.rest.repos.createWebhook({ 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 4f369b7263..542de013b4 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts @@ -22,18 +22,13 @@ import { import { OctokitOptions } from '@octokit/core/dist-types/types'; import { parseRepoUrl } from '../publish/util'; -interface GetOctokitOptionsInput { +export async function getOctokitOptions(options: { integrations: ScmIntegrationRegistry; credentialsProvider?: GithubCredentialsProvider; token?: string; repoUrl: string; -} -export const getOctokitOptions = async ({ - integrations, - credentialsProvider, - repoUrl, - token, -}: GetOctokitOptionsInput): Promise => { +}): Promise { + const { integrations, credentialsProvider, repoUrl, token } = options; const { owner, repo, host } = parseRepoUrl(repoUrl, integrations); if (!owner) { @@ -46,8 +41,7 @@ export const getOctokitOptions = async ({ throw new InputError(`No integration for host ${host}`); } - // Short circuit the internal Github Token provider the token provided - // by the action or the caller. + // short circuit the `githubCredentialsProvider` if there is a token provided by the caller already if (token) { return { auth: token, @@ -60,8 +54,8 @@ export const getOctokitOptions = async ({ credentialsProvider ?? DefaultGithubCredentialsProvider.fromIntegrations(integrations); - // 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. + // TODO(blam): Consider changing this API to take host and repo instead of repoUrl, as we end up parsing in this function + // and then parsing in the `getCredentials` function too the other side const { token: credentialProviderToken, } = await githubCredentialsProvider.getCredentials({ @@ -81,4 +75,4 @@ export const getOctokitOptions = async ({ baseUrl: integrationConfig.apiBaseUrl, previews: ['nebula-preview'], }; -}; +} 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 7ae9f317cf..37e2ffb7d3 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts @@ -157,6 +157,10 @@ export function createPublishGithubAction(options: { const { owner, repo } = parseRepoUrl(repoUrl, integrations); + if (!owner) { + throw new InputError('Invalid repository owner provided in repoUrl'); + } + const octokitOptions = await getOctokitOptions({ integrations, credentialsProvider: githubCredentialsProvider, @@ -166,10 +170,6 @@ export function createPublishGithubAction(options: { const client = new Octokit(octokitOptions); - if (!owner) { - throw new InputError('Invalid repository owner provided in repoUrl'); - } - const user = await client.rest.users.getByUsername({ username: owner, }); From 8142c09b3f08f27f571d0ece8c10992d2d9efcea Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 22 Feb 2022 11:29:35 +0100 Subject: [PATCH 7/8] chore: wth prettier Signed-off-by: blam --- .../scaffolder/actions/builtin/github/helpers.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) 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 542de013b4..c310389488 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts @@ -56,13 +56,12 @@ export async function getOctokitOptions(options: { // TODO(blam): Consider changing this API to take host and repo instead of repoUrl, as we end up parsing in this function // and then parsing in the `getCredentials` function too the other side - const { - token: credentialProviderToken, - } = await githubCredentialsProvider.getCredentials({ - url: `https://${host}/${encodeURIComponent(owner)}/${encodeURIComponent( - repo, - )}`, - }); + const { token: credentialProviderToken } = + await githubCredentialsProvider.getCredentials({ + url: `https://${host}/${encodeURIComponent(owner)}/${encodeURIComponent( + repo, + )}`, + }); if (!credentialProviderToken) { throw new InputError( From dbe239808d18aa1adb877ff7d8dda3c34e403ab6 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 22 Feb 2022 13:10:39 +0100 Subject: [PATCH 8/8] chore: encodeURIComponent Signed-off-by: blam diff --git c/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts i/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts index ca70084ef5..9efc11210a 100644 --- c/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts +++ i/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts @@ -65,10 +65,14 @@ export const defaultClientFactory = async ({ host = 'github.com', token: providedToken, }: ClientFactoryInput): Promise => { + const [encodedHost, encodedOwner, encodedRepo] = [host, owner, repo].map( + encodeURIComponent, + ); + const octokitOptions = await getOctokitOptions({ integrations, credentialsProvider: githubCredentialsProvider, - repoUrl: `https://${host}/${owner}/${repo}`, + repoUrl: `https://${encodedHost}/${encodedOwner}/${encodedRepo}`, token: providedToken, }); --- .../scaffolder/actions/builtin/publish/githubPullRequest.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts index ca70084ef5..9efc11210a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts @@ -65,10 +65,14 @@ export const defaultClientFactory = async ({ host = 'github.com', token: providedToken, }: ClientFactoryInput): Promise => { + const [encodedHost, encodedOwner, encodedRepo] = [host, owner, repo].map( + encodeURIComponent, + ); + const octokitOptions = await getOctokitOptions({ integrations, credentialsProvider: githubCredentialsProvider, - repoUrl: `https://${host}/${owner}/${repo}`, + repoUrl: `https://${encodedHost}/${encodedOwner}/${encodedRepo}`, token: providedToken, });