From 92d39a0c9fa9c32c9f02d65f1f343f2fb07fcf33 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Tue, 26 Jan 2021 16:42:41 +0100 Subject: [PATCH 01/19] Refactor scaffolder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: Patrik Oldsberg --- .../src/scaffolder/jobs/processor.ts | 51 ++++++++++- .../src/scaffolder/jobs/types.ts | 1 + .../src/scaffolder/stages/prepare/azure.ts | 41 ++++----- .../scaffolder/stages/prepare/bitbucket.ts | 51 ++++------- .../src/scaffolder/stages/prepare/file.ts | 33 ++----- .../src/scaffolder/stages/prepare/github.ts | 43 ++++------ .../src/scaffolder/stages/prepare/gitlab.ts | 41 ++++----- .../src/scaffolder/stages/prepare/types.ts | 19 ++-- .../src/scaffolder/stages/publish/github.ts | 8 +- .../src/scaffolder/stages/publish/types.ts | 2 +- .../stages/templater/cookiecutter.ts | 44 +++++----- .../scaffolder/stages/templater/templaters.ts | 20 ++--- .../src/scaffolder/stages/templater/types.ts | 7 +- .../scaffolder-backend/src/service/router.ts | 86 ++++++++++--------- 14 files changed, 218 insertions(+), 229 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index b932db7fe1..08423872f2 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -13,13 +13,19 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + +import os from 'os'; +import fs from 'fs-extra'; import { Processor, Job, StageContext, StageInput } from './types'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import * as uuid from 'uuid'; import Docker from 'dockerode'; +import path from 'path'; import { TemplaterValues, TemplaterBase } from '../stages/templater'; import { PreparerBuilder } from '../stages/prepare'; import { makeLogStream } from './logger'; +import { Logger } from 'winston'; +import { Config } from '@backstage/config'; export type JobProcessorArguments = { preparers: PreparerBuilder; @@ -33,7 +39,45 @@ export type JobAndDirectoryTuple = { }; export class JobProcessor implements Processor { - private jobs = new Map(); + private readonly workingDirectory: string; + private readonly jobs: Map; + + static async fromConfig({ + config, + logger, + }: { + config: Config; + logger: Logger; + }) { + let workingDirectory: string; + if (config.has('backend.workingDirectory')) { + workingDirectory = config.getString('backend.workingDirectory'); + try { + // Check if working directory exists and is writable + await fs.promises.access( + workingDirectory, + fs.constants.F_OK | fs.constants.W_OK, + ); + logger.info(`using working directory: ${workingDirectory}`); + } catch (err) { + logger.error( + `working directory ${workingDirectory} ${ + err.code === 'ENOENT' ? 'does not exist' : 'is not writable' + }`, + ); + throw err; + } + } else { + workingDirectory = os.tmpdir(); + } + + return new JobProcessor(workingDirectory); + } + + constructor(workingDirectory: string) { + this.workingDirectory = workingDirectory; + this.jobs = new Map(); + } create({ entity, @@ -52,6 +96,7 @@ export class JobProcessor implements Processor { values, logger, logStream: stream, + workspacePath: path.join(this.workingDirectory, id), }; const job: Job = { @@ -80,6 +125,8 @@ export class JobProcessor implements Processor { throw new Error("Job is not in a 'PENDING' state"); } + await fs.mkdir(job.context.workspacePath); + job.status = 'STARTED'; try { @@ -134,6 +181,8 @@ export class JobProcessor implements Processor { // If something went wrong, fail the job, and set the error property on the job. job.error = { name: error.name, message: error.message }; job.status = 'FAILED'; + } finally { + await fs.remove(job.context.workspacePath); } } } diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts index 4d0369708c..e45e6c8b96 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts @@ -26,6 +26,7 @@ export type StageContext = { entity: TemplateEntityV1alpha1; logger: Logger; logStream: Writable; + workspacePath: string; } & T; export type ProcessorStatus = 'PENDING' | 'STARTED' | 'COMPLETED' | 'FAILED'; diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/azure.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/azure.ts index 8b204fa799..cca6baa0b1 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/azure.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/azure.ts @@ -13,11 +13,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import os from 'os'; import fs from 'fs-extra'; import path from 'path'; -import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; -import { parseLocationAnnotation } from '../helpers'; import { Git } from '@backstage/backend-common'; import { PreparerBase, PreparerOptions } from './types'; import parseGitUrl from 'git-url-parse'; @@ -30,25 +27,13 @@ export class AzurePreparer implements PreparerBase { constructor(private readonly config: { token?: string }) {} - async prepare( - template: TemplateEntityV1alpha1, - opts: PreparerOptions, - ): Promise { - const { location } = parseLocationAnnotation(template); - const workingDirectory = opts.workingDirectory ?? os.tmpdir(); - const logger = opts.logger; - - const templateId = template.metadata.name; - - const parsedGitLocation = parseGitUrl(location); - const repositoryCheckoutUrl = parsedGitLocation.toString('https'); - const tempDir = await fs.promises.mkdtemp( - path.join(workingDirectory, templateId), - ); - - const templateDirectory = path.join( - `${path.dirname(parsedGitLocation.filepath)}`, - template.spec.path ?? '.', + async prepare({ url, workspacePath, logger }: PreparerOptions) { + const parsedGitUrl = parseGitUrl(url); + const checkoutPath = path.join(workspacePath, 'checkout'); + const targetPath = path.join(workspacePath, 'template'); + const fullPathToTemplate = path.resolve( + checkoutPath, + parsedGitUrl.filepath, ); // Username can be anything but the empty string according to: @@ -62,10 +47,16 @@ export class AzurePreparer implements PreparerBase { : Git.fromAuth({ logger }); await git.clone({ - url: repositoryCheckoutUrl, - dir: tempDir, + url: parsedGitUrl.toString('https'), + dir: checkoutPath, }); - return path.resolve(tempDir, templateDirectory); + await fs.move(fullPathToTemplate, targetPath); + + try { + await fs.rmdir(path.join(targetPath, '.git')); + } catch { + // Ignore intentionally + } } } diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/bitbucket.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/bitbucket.ts index 5020a5f658..ad8bbca0b8 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/bitbucket.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/bitbucket.ts @@ -13,11 +13,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import os from 'os'; import fs from 'fs-extra'; import path from 'path'; -import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; -import { parseLocationAnnotation } from '../helpers'; import { Git } from '@backstage/backend-common'; import { PreparerBase, PreparerOptions } from './types'; import { BitbucketIntegrationConfig } from '@backstage/integration'; @@ -40,43 +37,29 @@ export class BitbucketPreparer implements PreparerBase { }, ) {} - async prepare( - template: TemplateEntityV1alpha1, - opts: PreparerOptions, - ): Promise { - const { location } = parseLocationAnnotation(template); - const workingDirectory = opts.workingDirectory ?? os.tmpdir(); - const logger = opts.logger; - const templateId = template.metadata.name; - - const repo = parseGitUrl(location); - const repositoryCheckoutUrl = repo.toString('https'); - - const tempDir = await fs.promises.mkdtemp( - path.join(workingDirectory, templateId), + async prepare({ url, workspacePath, logger }: PreparerOptions) { + const parsedGitUrl = parseGitUrl(url); + const checkoutPath = path.join(workspacePath, 'checkout'); + const targetPath = path.join(workspacePath, 'template'); + const fullPathToTemplate = path.resolve( + checkoutPath, + parsedGitUrl.filepath, ); - const templateDirectory = path.join( - `${path.dirname(repo.filepath)}`, - template.spec.path ?? '.', - ); - - const checkoutLocation = path.resolve(tempDir, templateDirectory); - - const auth = this.getAuth(); - const git = auth - ? Git.fromAuth({ - ...auth, - logger, - }) - : Git.fromAuth({ logger }); + const git = Git.fromAuth({ logger, ...this.getAuth() }); await git.clone({ - url: repositoryCheckoutUrl, - dir: tempDir, + url: parsedGitUrl.toString('https'), + dir: checkoutPath, }); - return checkoutLocation; + await fs.move(fullPathToTemplate, targetPath); + + try { + await fs.rmdir(path.join(targetPath, '.git')); + } catch { + // Ignore intentionally + } } private getAuth(): { username: string; password: string } | undefined { diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.ts index 08129c7397..cbf81bdb06 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.ts @@ -13,43 +13,24 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import os from 'os'; import fs from 'fs-extra'; import path from 'path'; -import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; -import { parseLocationAnnotation } from '../helpers'; import { InputError } from '@backstage/backend-common'; import { PreparerBase, PreparerOptions } from './types'; export class FilePreparer implements PreparerBase { - async prepare( - template: TemplateEntityV1alpha1, - opts: PreparerOptions, - ): Promise { - const { protocol, location } = parseLocationAnnotation(template); - const workingDirectory = opts?.workingDirectory ?? os.tmpdir(); - - if (protocol !== 'file') { - throw new InputError( - `Wrong location protocol: ${protocol}, should be 'file'`, - ); + async prepare({ url, workspacePath }: PreparerOptions) { + if (!url.startsWith('file:///')) { + throw new InputError(`Wrong location protocol, should be 'file', ${url}`); } - const templateId = template.metadata.name; - const tempDir = await fs.promises.mkdtemp( - path.join(workingDirectory, templateId), - ); + const checkoutDir = path.join(workspacePath, 'checkout'); + await fs.ensureDir(checkoutDir); - const parentDirectory = path.resolve( - path.dirname(location), - template.spec.path ?? '.', - ); + const templatePath = url.slice('file://'.length); - await fs.copy(parentDirectory, tempDir, { - filter: src => src !== location, + await fs.copy(templatePath, checkoutDir, { recursive: true, }); - - return tempDir; } } diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts index cfeff454e7..4273336d56 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts @@ -13,11 +13,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import os from 'os'; import fs from 'fs-extra'; import path from 'path'; -import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; -import { parseLocationAnnotation } from '../helpers'; import { Git } from '@backstage/backend-common'; import { PreparerBase, PreparerOptions } from './types'; import parseGitUrl from 'git-url-parse'; @@ -30,29 +27,15 @@ export class GithubPreparer implements PreparerBase { constructor(private readonly config: { token?: string }) {} - async prepare( - template: TemplateEntityV1alpha1, - opts: PreparerOptions, - ): Promise { - const { location } = parseLocationAnnotation(template); - const workingDirectory = opts.workingDirectory ?? os.tmpdir(); - const logger = opts.logger; - - const templateId = template.metadata.name; - - const parsedGitLocation = parseGitUrl(location); - const repositoryCheckoutUrl = parsedGitLocation.toString('https'); - const tempDir = await fs.promises.mkdtemp( - path.join(workingDirectory, templateId), + async prepare({ url, workspacePath, logger }: PreparerOptions) { + const parsedGitUrl = parseGitUrl(url); + const checkoutPath = path.join(workspacePath, 'checkout'); + const targetPath = path.join(workspacePath, 'template'); + const fullPathToTemplate = path.resolve( + checkoutPath, + parsedGitUrl.filepath, ); - const templateDirectory = path.join( - `${path.dirname(parsedGitLocation.filepath)}`, - template.spec.path ?? '.', - ); - - const checkoutLocation = path.resolve(tempDir, templateDirectory); - const git = this.config.token ? Git.fromAuth({ username: this.config.token, @@ -62,10 +45,16 @@ export class GithubPreparer implements PreparerBase { : Git.fromAuth({ logger }); await git.clone({ - url: repositoryCheckoutUrl, - dir: tempDir, + url: parsedGitUrl.toString('https'), + dir: checkoutPath, }); - return checkoutLocation; + await fs.move(fullPathToTemplate, targetPath); + + try { + await fs.rmdir(path.join(targetPath, '.git')); + } catch { + // Ignore intentionally + } } } diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/gitlab.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/gitlab.ts index 1fca2ff0f9..98901577d5 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/gitlab.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/gitlab.ts @@ -14,13 +14,10 @@ * limitations under the License. */ import { Git } from '@backstage/backend-common'; -import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import { GitLabIntegrationConfig } from '@backstage/integration'; import fs from 'fs-extra'; import parseGitUrl from 'git-url-parse'; -import os from 'os'; import path from 'path'; -import { parseLocationAnnotation } from '../helpers'; import { PreparerBase, PreparerOptions } from './types'; export class GitlabPreparer implements PreparerBase { @@ -30,25 +27,13 @@ export class GitlabPreparer implements PreparerBase { constructor(private readonly config: { token?: string }) {} - async prepare( - template: TemplateEntityV1alpha1, - opts: PreparerOptions, - ): Promise { - const { location } = parseLocationAnnotation(template); - const logger = opts.logger; - const workingDirectory = opts.workingDirectory ?? os.tmpdir(); - - const templateId = template.metadata.name; - - const parsedGitLocation = parseGitUrl(location); - const repositoryCheckoutUrl = parsedGitLocation.toString('https'); - const tempDir = await fs.promises.mkdtemp( - path.join(workingDirectory, templateId), - ); - - const templateDirectory = path.join( - `${path.dirname(parsedGitLocation.filepath)}`, - template.spec.path ?? '.', + async prepare({ url, workspacePath, logger }: PreparerOptions) { + const parsedGitUrl = parseGitUrl(url); + const checkoutPath = path.join(workspacePath, 'checkout'); + const targetPath = path.join(workspacePath, 'template'); + const fullPathToTemplate = path.resolve( + checkoutPath, + parsedGitUrl.filepath, ); const git = this.config.token @@ -60,10 +45,16 @@ export class GitlabPreparer implements PreparerBase { : Git.fromAuth({ logger }); await git.clone({ - url: repositoryCheckoutUrl, - dir: tempDir, + url: parsedGitUrl.toString('https'), + dir: checkoutPath, }); - return path.resolve(tempDir, templateDirectory); + await fs.move(fullPathToTemplate, targetPath); + + try { + await fs.rmdir(path.join(targetPath, '.git')); + } catch { + // Ignore intentionally + } } } diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/types.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/types.ts index 17ffea5557..6a2b7808f1 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/types.ts @@ -13,24 +13,25 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import type { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import { Logger } from 'winston'; export type PreparerOptions = { - workingDirectory?: string; + /** + * Full URL to the directory containg template data + */ + url: string; + /** + * The workspace path that will eventually be the the root of the new repo + */ + workspacePath: string; logger: Logger; }; export interface PreparerBase { /** - * Given an Entity definition from the Service Catalog, go and prepare a directory - * with contents from the remote location in temporary storage and return the path - * @param template The template entity from the Service Catalog + * Prepare a directory with contents from the remote location */ - prepare( - template: TemplateEntityV1alpha1, - opts?: PreparerOptions, - ): Promise; + prepare(opts: PreparerOptions): Promise; } export type PreparerBuilder = { diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts index 31d94d5f53..13aab7ff34 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts @@ -19,8 +19,10 @@ import { initRepoAndPush } from './helpers'; import { GitHubIntegrationConfig } from '@backstage/integration'; import parseGitUrl from 'git-url-parse'; import { Octokit } from '@octokit/rest'; +import path from 'path'; export type RepoVisibilityOptions = 'private' | 'internal' | 'public'; + export class GithubPublisher implements PublisherBase { static async fromConfig( config: GitHubIntegrationConfig, @@ -41,6 +43,7 @@ export class GithubPublisher implements PublisherBase { repoVisibility, }); } + constructor( private readonly config: { token: string; @@ -51,7 +54,7 @@ export class GithubPublisher implements PublisherBase { async publish({ values, - directory, + workspacePath, logger, }: PublisherOptions): Promise { const { owner, name } = parseGitUrl(values.storePath); @@ -66,7 +69,7 @@ export class GithubPublisher implements PublisherBase { }); await initRepoAndPush({ - dir: directory, + dir: path.join(workspacePath, 'result'), remoteUrl, auth: { username: this.config.token, @@ -79,7 +82,6 @@ export class GithubPublisher implements PublisherBase { /\.git$/, '/blob/master/catalog-info.yaml', ); - return { remoteUrl, catalogInfoUrl }; } diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/types.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/types.ts index fbf656206d..3ae92b8b7d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/types.ts @@ -32,7 +32,7 @@ export type PublisherBase = { export type PublisherOptions = { values: TemplaterValues; - directory: string; + workspacePath: string; logger: Logger; }; diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts index 7f7fd3494f..d19112561a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts @@ -19,8 +19,6 @@ import { runDockerContainer, runCommand } from './helpers'; import { TemplaterBase, TemplaterRunOptions } from '.'; import path from 'path'; -import { TemplaterRunResult } from './types'; - const commandExists = require('command-exists-promise'); export class CookieCutter implements TemplaterBase { @@ -38,28 +36,32 @@ export class CookieCutter implements TemplaterBase { } } - public async run(options: TemplaterRunOptions): Promise { + public async run({ + workspacePath, + dockerClient, + values, + logStream, + }: TemplaterRunOptions): Promise { + const templateDir = path.join(workspacePath, 'template'); + const intermediateDir = path.join(workspacePath, 'intermediate'); + const resultDir = path.join(workspacePath, 'result'); + // First lets grab the default cookiecutter.json file - const cookieCutterJson = await this.fetchTemplateCookieCutter( - options.directory, - ); + const cookieCutterJson = await this.fetchTemplateCookieCutter(templateDir); const cookieInfo = { ...cookieCutterJson, - ...options.values, + ...values, }; - await fs.writeJSON(`${options.directory}/cookiecutter.json`, cookieInfo); - - const templateDir = options.directory; - const resultDir = await fs.promises.mkdtemp(`${options.directory}-result`); + await fs.writeJSON(`${templateDir}/cookiecutter.json`, cookieInfo); const cookieCutterInstalled = await commandExists('cookiecutter'); if (cookieCutterInstalled) { await runCommand({ command: 'cookiecutter', - args: ['--no-input', '-o', resultDir, templateDir, '--verbose'], - logStream: options.logStream, + args: ['--no-input', '-o', intermediateDir, templateDir, '--verbose'], + logStream, }); } else { await runDockerContainer({ @@ -73,22 +75,20 @@ export class CookieCutter implements TemplaterBase { '--verbose', ], templateDir, - resultDir, - logStream: options.logStream, - dockerClient: options.dockerClient, + resultDir: intermediateDir, + logStream, + dockerClient, }); } - // if cookiecutter was successful, resultDir will contain + // if cookiecutter was successful, intermediateDir will contain // exactly one directory. - const [generated] = await fs.readdir(resultDir); + const [generated] = await fs.readdir(intermediateDir); if (generated === undefined) { - throw new Error('Cookie Cutter did not generate anything'); + throw new Error('No data generated by cookiecutter'); } - return { - resultDir: path.resolve(resultDir, generated), - }; + await fs.move(path.join(intermediateDir, generated), resultDir); } } diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/templater/templaters.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/templaters.ts index 549b898af8..9535d28837 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/templaters.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/templater/templaters.ts @@ -20,26 +20,20 @@ import { TemplaterBuilder, } from './types'; -import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; -import { getTemplaterKey } from './helpers'; - export class Templaters implements TemplaterBuilder { - private preparerMap = new Map(); + private templaterMap = new Map(); register(templaterKey: SupportedTemplatingKey, templater: TemplaterBase) { - this.preparerMap.set(templaterKey, templater); + this.templaterMap.set(templaterKey, templater); } - get(template: TemplateEntityV1alpha1): TemplaterBase { - const templaterKey = getTemplaterKey(template); - const preparer = this.preparerMap.get(templaterKey); + get(templaterId: string): TemplaterBase { + const templater = this.templaterMap.get(templaterId); - if (!preparer) { - throw new Error( - `No templater registered for template: "${templaterKey}"`, - ); + if (!templater) { + throw new Error(`No templater registered for template: "${templaterId}"`); } - return preparer; + return templater; } } diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/templater/types.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/types.ts index 812fb972d5..fa14788d22 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/templater/types.ts @@ -15,7 +15,6 @@ */ import type { Writable } from 'stream'; import Docker from 'dockerode'; -import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import gitUrlParse from 'git-url-parse'; /** @@ -46,7 +45,7 @@ export type TemplaterRunResult = { * client to run any templater on top of your directory. */ export type TemplaterRunOptions = { - directory: string; + workspacePath: string; values: TemplaterValues; logStream?: Writable; dockerClient: Docker; @@ -54,7 +53,7 @@ export type TemplaterRunOptions = { export type TemplaterBase = { // runs the templating with the values and returns the directory to push the VCS - run(opts: TemplaterRunOptions): Promise; + run(opts: TemplaterRunOptions): Promise; }; export type TemplaterConfig = { @@ -71,5 +70,5 @@ export type SupportedTemplatingKey = 'cookiecutter' | string; */ export type TemplaterBuilder = { register(protocol: SupportedTemplatingKey, templater: TemplaterBase): void; - get(template: TemplateEntityV1alpha1): TemplaterBase; + get(templater: string): TemplaterBase; }; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 115efc2ec4..4fc9b9a911 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -15,7 +15,6 @@ */ import { Config } from '@backstage/config'; -import fs from 'fs-extra'; import Docker from 'dockerode'; import express from 'express'; import Router from 'express-promise-router'; @@ -33,6 +32,7 @@ import { import { CatalogEntityClient } from '../lib/catalog'; import { validate, ValidatorResult } from 'jsonschema'; import parseGitUrl from 'git-url-parse'; +import { PreparerBase } from '../scaffolder/stages/prepare'; export interface RouterOptions { preparers: PreparerBuilder; @@ -62,27 +62,7 @@ export async function createRouter( } = options; const logger = parentLogger.child({ plugin: 'scaffolder' }); - const jobProcessor = new JobProcessor(); - - let workingDirectory: string; - if (config.has('backend.workingDirectory')) { - workingDirectory = config.getString('backend.workingDirectory'); - try { - // Check if working directory exists and is writable - await fs.promises.access( - workingDirectory, - fs.constants.F_OK | fs.constants.W_OK, - ); - logger.info(`using working directory: ${workingDirectory}`); - } catch (err) { - logger.error( - `working directory ${workingDirectory} ${ - err.code === 'ENOENT' ? 'does not exist' : 'is not writable' - }`, - ); - throw err; - } - } + const jobProcessor = await JobProcessor.fromConfig({ config, logger }); router .get('/v1/job/:jobId', ({ params }, res) => { @@ -128,51 +108,79 @@ export async function createRouter( res.status(400).json({ errors: validationResult.errors }); return; } + const job = jobProcessor.create({ entity: template, values, stages: [ { name: 'Prepare the skeleton', - handler: async ctx => { - const { protocol, location: pullPath } = parseLocationAnnotation( - ctx.entity, - ); + async handler(ctx) { + const { + protocol, + location: templateEntityLocation, + } = parseLocationAnnotation(ctx.entity); - const preparer = + if (protocol === 'file') { + const preparer: PreparerBase = + protocol === 'file' + ? new FilePreparer() + : preparers.get(templateEntityLocation); + + const url = new URL( + template.spec.path || '.', + templateEntityLocation, + ) + .toString() + .replace(/\/$/, ''); + + await preparer.prepare({ + url, + logger: ctx.logger, + workspacePath: ctx.workspacePath, + }); + return; + } + + const preparer: PreparerBase = protocol === 'file' ? new FilePreparer() - : preparers.get(pullPath); + : preparers.get(templateEntityLocation); - const skeletonDir = await preparer.prepare(ctx.entity, { + const url = new URL( + template.spec.path || '.', + templateEntityLocation, + ) + .toString() + .replace(/\/$/, ''); + + await preparer.prepare({ + url, logger: ctx.logger, - workingDirectory, + workspacePath: ctx.workspacePath, }); - return { skeletonDir }; }, }, { name: 'Run the templater', - handler: async (ctx: StageContext<{ skeletonDir: string }>) => { - const templater = templaters.get(ctx.entity); - const { resultDir } = await templater.run({ - directory: ctx.skeletonDir, + async handler(ctx) { + const templater = templaters.get(ctx.entity.spec.templater); + await templater.run({ + workspacePath: ctx.workspacePath, dockerClient, logStream: ctx.logStream, values: ctx.values, }); - - return { resultDir }; }, }, { name: 'Publish template', - handler: async (ctx: StageContext<{ resultDir: string }>) => { + handler: async ctx => { const publisher = publishers.get(ctx.values.storePath); ctx.logger.info('Will now store the template'); const result = await publisher.publish({ values: ctx.values, - directory: ctx.resultDir, + workspacePath: ctx.workspacePath, logger: ctx.logger, }); return result; From add5ce1350946fe0c35c22ec6dcff175d9a0eb9c Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Wed, 27 Jan 2021 08:59:41 +0100 Subject: [PATCH 02/19] cleanup filepreparer usage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Co-authored-by: Patrik Oldsberg --- .../scaffolder-backend/src/service/router.ts | 23 ++++++------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 4fc9b9a911..a3e7202f95 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -17,12 +17,12 @@ import { Config } from '@backstage/config'; import Docker from 'dockerode'; import express from 'express'; +import { resolve as resolvePath } from 'path'; import Router from 'express-promise-router'; import { Logger } from 'winston'; import { JobProcessor, PreparerBuilder, - StageContext, TemplaterBuilder, TemplaterValues, PublisherBuilder, @@ -32,7 +32,6 @@ import { import { CatalogEntityClient } from '../lib/catalog'; import { validate, ValidatorResult } from 'jsonschema'; import parseGitUrl from 'git-url-parse'; -import { PreparerBase } from '../scaffolder/stages/prepare'; export interface RouterOptions { preparers: PreparerBuilder; @@ -122,30 +121,22 @@ export async function createRouter( } = parseLocationAnnotation(ctx.entity); if (protocol === 'file') { - const preparer: PreparerBase = - protocol === 'file' - ? new FilePreparer() - : preparers.get(templateEntityLocation); + const preparer = new FilePreparer(); - const url = new URL( - template.spec.path || '.', + const path = resolvePath( templateEntityLocation, - ) - .toString() - .replace(/\/$/, ''); + template.spec.path || '.', + ); await preparer.prepare({ - url, + url: `file://${path}`, logger: ctx.logger, workspacePath: ctx.workspacePath, }); return; } - const preparer: PreparerBase = - protocol === 'file' - ? new FilePreparer() - : preparers.get(templateEntityLocation); + const preparer = preparers.get(templateEntityLocation); const url = new URL( template.spec.path || '.', From 85365cc23de2a7a5e5f71a249913217370949cb7 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Wed, 27 Jan 2021 09:12:36 +0100 Subject: [PATCH 03/19] publish repo from result/ directory Update tests --- .../scaffolder/stages/publish/azure.test.ts | 4 ++-- .../src/scaffolder/stages/publish/azure.ts | 5 +++-- .../stages/publish/bitbucket.test.ts | 8 ++++---- .../scaffolder/stages/publish/bitbucket.ts | 5 +++-- .../scaffolder/stages/publish/github.test.ts | 20 +++++++++---------- .../scaffolder/stages/publish/gitlab.test.ts | 8 ++++---- .../src/scaffolder/stages/publish/gitlab.ts | 6 +++--- 7 files changed, 29 insertions(+), 27 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/azure.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/azure.test.ts index e4410e9838..81901809e4 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/azure.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/azure.test.ts @@ -53,7 +53,7 @@ describe('Azure Publisher', () => { storePath: 'https://dev.azure.com/organisation/project/_git/repo', owner: 'bob', }, - directory: '/tmp/test', + workspacePath: '/tmp/test', logger, }); @@ -74,7 +74,7 @@ describe('Azure Publisher', () => { 'project', ); expect(helpers.initRepoAndPush).toHaveBeenCalledWith({ - dir: '/tmp/test', + dir: '/tmp/test/result', remoteUrl: 'https://dev.azure.com/organization/project/_git/repo', auth: { username: 'notempty', password: 'fake-azure-token' }, logger, diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/azure.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/azure.ts index b2ac486d77..4190ba8abb 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/azure.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/azure.ts @@ -21,6 +21,7 @@ import { initRepoAndPush } from './helpers'; import { AzureIntegrationConfig } from '@backstage/integration'; import parseGitUrl from 'git-url-parse'; import { getPersonalAccessTokenHandler, WebApi } from 'azure-devops-node-api'; +import path from 'path'; export class AzurePublisher implements PublisherBase { static async fromConfig(config: AzureIntegrationConfig) { @@ -34,7 +35,7 @@ export class AzurePublisher implements PublisherBase { async publish({ values, - directory, + workspacePath, logger, }: PublisherOptions): Promise { const { owner, name, organization, resource } = parseGitUrl( @@ -56,7 +57,7 @@ export class AzurePublisher implements PublisherBase { const catalogInfoUrl = `${remoteUrl}?path=%2Fcatalog-info.yaml`; await initRepoAndPush({ - dir: directory, + dir: path.join(workspacePath, 'result'), remoteUrl, auth: { username: 'notempty', diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/bitbucket.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/bitbucket.test.ts index f75eea939a..3b6ec9da89 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/bitbucket.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/bitbucket.test.ts @@ -69,7 +69,7 @@ describe('Bitbucket Publisher', () => { storePath: 'https://bitbucket.org/project/repo', owner: 'bob', }, - directory: '/tmp/test', + workspacePath: '/tmp/test', logger: logger, }); @@ -80,7 +80,7 @@ describe('Bitbucket Publisher', () => { }); expect(initRepoAndPush).toHaveBeenCalledWith({ - dir: '/tmp/test', + dir: '/tmp/test/result', remoteUrl: 'https://bitbucket.org/project/repo', auth: { username: 'fake-user', password: 'fake-token' }, logger: logger, @@ -127,7 +127,7 @@ describe('Bitbucket Publisher', () => { storePath: 'https://bitbucket.mycompany.com/project/repo', owner: 'bob', }, - directory: '/tmp/test', + workspacePath: '/tmp/test', logger: logger, }); @@ -138,7 +138,7 @@ describe('Bitbucket Publisher', () => { }); expect(initRepoAndPush).toHaveBeenCalledWith({ - dir: '/tmp/test', + dir: '/tmp/test/result', remoteUrl: 'https://bitbucket.mycompany.com/scm/project/repo', auth: { username: 'x-token-auth', password: 'fake-token' }, logger: logger, diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/bitbucket.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/bitbucket.ts index a0b867507d..2a274144ff 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/bitbucket.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/bitbucket.ts @@ -19,6 +19,7 @@ import { initRepoAndPush } from './helpers'; import fetch from 'cross-fetch'; import { BitbucketIntegrationConfig } from '@backstage/integration'; import parseGitUrl from 'git-url-parse'; +import path from 'path'; // 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 @@ -45,7 +46,7 @@ export class BitbucketPublisher implements PublisherBase { async publish({ values, - directory, + workspacePath, logger, }: PublisherOptions): Promise { const { owner: project, name } = parseGitUrl(values.storePath); @@ -58,7 +59,7 @@ export class BitbucketPublisher implements PublisherBase { }); await initRepoAndPush({ - dir: directory, + dir: path.join(workspacePath, 'result'), remoteUrl: result.remoteUrl, auth: { username: this.config.username ? this.config.username : 'x-token-auth', diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.test.ts index 9a788b960d..faa8c49124 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.test.ts @@ -64,7 +64,7 @@ describe('GitHub Publisher', () => { owner: 'bob', access: 'blam/team', }, - directory: '/tmp/test', + workspacePath: '/tmp/test', logger, }); @@ -89,7 +89,7 @@ describe('GitHub Publisher', () => { permission: 'admin', }); expect(initRepoAndPush).toHaveBeenCalledWith({ - dir: '/tmp/test', + dir: '/tmp/test/result', remoteUrl: 'https://github.com/backstage/backstage.git', auth: { username: 'fake-token', password: 'x-oauth-basic' }, logger, @@ -122,7 +122,7 @@ describe('GitHub Publisher', () => { owner: 'bob', access: 'blam', }, - directory: '/tmp/test', + workspacePath: '/tmp/test', logger, }); @@ -140,7 +140,7 @@ describe('GitHub Publisher', () => { expect(mockGithubClient.repos.addCollaborator).not.toHaveBeenCalled(); expect(initRepoAndPush).toHaveBeenCalledWith({ - dir: '/tmp/test', + dir: '/tmp/test/result', remoteUrl: 'https://github.com/backstage/backstage.git', auth: { username: 'fake-token', password: 'x-oauth-basic' }, logger, @@ -175,7 +175,7 @@ describe('GitHub Publisher', () => { access: 'bob', description: 'description', }, - directory: '/tmp/test', + workspacePath: '/tmp/test', logger, }); @@ -198,7 +198,7 @@ describe('GitHub Publisher', () => { permission: 'admin', }); expect(initRepoAndPush).toHaveBeenCalledWith({ - dir: '/tmp/test', + dir: '/tmp/test/result', remoteUrl: 'https://github.com/backstage/backstage.git', auth: { username: 'fake-token', password: 'x-oauth-basic' }, logger, @@ -233,7 +233,7 @@ describe('GitHub Publisher', () => { storePath: 'https://github.com/blam/test', owner: 'bob', }, - directory: '/tmp/test', + workspacePath: '/tmp/test', logger, }); @@ -249,7 +249,7 @@ describe('GitHub Publisher', () => { visibility: 'internal', }); expect(initRepoAndPush).toHaveBeenCalledWith({ - dir: '/tmp/test', + dir: '/tmp/test/result', remoteUrl: 'https://github.com/backstage/backstage.git', auth: { username: 'fake-token', password: 'x-oauth-basic' }, logger, @@ -283,7 +283,7 @@ describe('GitHub Publisher', () => { storePath: 'https://github.com/blam/test', owner: 'bob', }, - directory: '/tmp/test', + workspacePath: '/tmp/test', logger, }); @@ -299,7 +299,7 @@ describe('GitHub Publisher', () => { private: true, }); expect(initRepoAndPush).toHaveBeenCalledWith({ - dir: '/tmp/test', + dir: '/tmp/test/result', remoteUrl: 'https://github.com/backstage/backstage.git', auth: { username: 'fake-token', password: 'x-oauth-basic' }, logger, diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/gitlab.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/gitlab.test.ts index 11b32a6de5..99ddbbb2b7 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/gitlab.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/gitlab.test.ts @@ -68,7 +68,7 @@ describe('GitLab Publisher', () => { storePath: 'https://gitlab.com/blam/test', owner: 'bob', }, - directory: '/tmp/test', + workspacePath: '/tmp/test', logger, }); @@ -85,7 +85,7 @@ describe('GitLab Publisher', () => { name: 'test', }); expect(initRepoAndPush).toHaveBeenCalledWith({ - dir: '/tmp/test', + dir: '/tmp/test/result', remoteUrl: 'mockclone', auth: { username: 'oauth2', password: 'fake-token' }, logger, @@ -111,7 +111,7 @@ describe('GitLab Publisher', () => { storePath: 'https://gitlab.com/blam/test', owner: 'bob', }, - directory: '/tmp/test', + workspacePath: '/tmp/test', logger, }); @@ -125,7 +125,7 @@ describe('GitLab Publisher', () => { name: 'test', }); expect(initRepoAndPush).toHaveBeenCalledWith({ - dir: '/tmp/test', + dir: '/tmp/test/result', remoteUrl: 'mockclone', auth: { username: 'oauth2', password: 'fake-token' }, logger, diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/gitlab.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/gitlab.ts index 24149ab2f3..ace5791b1f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/gitlab.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/gitlab.ts @@ -19,7 +19,7 @@ import { Gitlab } from '@gitbeaker/node'; import { Gitlab as GitlabClient } from '@gitbeaker/core'; import { initRepoAndPush } from './helpers'; import parseGitUrl from 'git-url-parse'; - +import path from 'path'; import { GitLabIntegrationConfig } from '@backstage/integration'; export class GitlabPublisher implements PublisherBase { @@ -38,7 +38,7 @@ export class GitlabPublisher implements PublisherBase { async publish({ values, - directory, + workspacePath, logger, }: PublisherOptions): Promise { const { owner, name } = parseGitUrl(values.storePath); @@ -49,7 +49,7 @@ export class GitlabPublisher implements PublisherBase { }); await initRepoAndPush({ - dir: directory, + dir: path.join(workspacePath, 'result'), remoteUrl, auth: { username: 'oauth2', From 7e7e24c61c7efbaca584ab1827d61d9296a24c5b Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Wed, 27 Jan 2021 11:25:24 +0100 Subject: [PATCH 04/19] Pass working directory --- .../src/scaffolder/jobs/processor.test.ts | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts index eaae937337..0492400db6 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts @@ -70,7 +70,7 @@ describe('JobProcessor', () => { describe('create', () => { it('creates should create a new job with a unique id', async () => { - const processor = new JobProcessor(); + const processor = new JobProcessor('/tmp'); const job = processor.create({ entity: mockEntity, @@ -84,7 +84,7 @@ describe('JobProcessor', () => { }); it('should setup the correct context for the job', async () => { - const processor = new JobProcessor(); + const processor = new JobProcessor('/tmp'); const job = processor.create({ entity: mockEntity, @@ -97,7 +97,7 @@ describe('JobProcessor', () => { }); it('should set the status as pending', async () => { - const processor = new JobProcessor(); + const processor = new JobProcessor('/tmp'); const job = processor.create({ entity: mockEntity, @@ -120,7 +120,7 @@ describe('JobProcessor', () => { }, ]; - const processor = new JobProcessor(); + const processor = new JobProcessor('/tmp'); const job = processor.create({ entity: mockEntity, @@ -139,12 +139,12 @@ describe('JobProcessor', () => { describe('get', () => { it('return undefined for when the job does not exist', () => { - const processor = new JobProcessor(); + const processor = new JobProcessor('/tmp'); expect(processor.get('123')).not.toBeDefined(); }); it('should return the exact same instance of the job when one is created', async () => { - const processor = new JobProcessor(); + const processor = new JobProcessor('/tmp'); const job = processor.create({ entity: mockEntity, values: mockValues, @@ -156,7 +156,7 @@ describe('JobProcessor', () => { }); describe('process', () => { it('throws an error when the status of the job is not in pending state', async () => { - const processor = new JobProcessor(); + const processor = new JobProcessor('/tmp'); const job = processor.create({ entity: mockEntity, values: mockValues, @@ -182,7 +182,7 @@ describe('JobProcessor', () => { }, ]; - const processor = new JobProcessor(); + const processor = new JobProcessor('/tmp'); const job = processor.create({ entity: mockEntity, values: mockValues, @@ -208,7 +208,7 @@ describe('JobProcessor', () => { }, ]; - const processor = new JobProcessor(); + const processor = new JobProcessor('/tmp'); const job = processor.create({ entity: mockEntity, values: mockValues, @@ -244,7 +244,7 @@ describe('JobProcessor', () => { }, ]; - const processor = new JobProcessor(); + const processor = new JobProcessor('/tmp'); const job = processor.create({ entity: mockEntity, values: mockValues, @@ -283,7 +283,7 @@ describe('JobProcessor', () => { }, ]; - const processor = new JobProcessor(); + const processor = new JobProcessor('/tmp'); const job = processor.create({ entity: mockEntity, values: mockValues, From 46211fa5bea80aa108d3cfc03dc9a945ea23e7a7 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Wed, 27 Jan 2021 11:26:59 +0100 Subject: [PATCH 05/19] Update prepare test --- .../stages/prepare/bitbucket.test.ts | 100 ++++------------ .../scaffolder/stages/prepare/file.test.ts | 71 ++++------- .../scaffolder/stages/prepare/github.test.ts | 108 +++++------------ .../scaffolder/stages/prepare/gitlab.test.ts | 112 ++++-------------- 4 files changed, 99 insertions(+), 292 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/bitbucket.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/bitbucket.test.ts index ff74fffdd7..84e72eafb8 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/bitbucket.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/bitbucket.test.ts @@ -14,21 +14,13 @@ * limitations under the License. */ -jest.doMock('fs-extra', () => ({ - promises: { - mkdtemp: jest.fn(dir => `${dir}-static`), - }, -})); - +import fs from 'fs-extra'; import { BitbucketPreparer } from './bitbucket'; -import { - TemplateEntityV1alpha1, - LOCATION_ANNOTATION, -} from '@backstage/catalog-model'; import { getVoidLogger, Git } from '@backstage/backend-common'; +jest.mock('fs-extra'); + describe('BitbucketPreparer', () => { - let mockEntity: TemplateEntityV1alpha1; const logger = getVoidLogger(); const mockGitClient = { clone: jest.fn(), @@ -38,44 +30,6 @@ describe('BitbucketPreparer', () => { beforeEach(() => { jest.clearAllMocks(); - mockEntity = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Template', - metadata: { - annotations: { - [LOCATION_ANNOTATION]: - 'bitbucket:https://bitbucket.org/backstage-project/backstage-repo', - }, - name: 'graphql-starter', - title: 'GraphQL Service', - description: - 'A GraphQL starter template for backstage to get you up and running\nthe best pracices with GraphQL\n', - uid: '9cf16bad-16e0-4213-b314-c4eec773c50b', - etag: 'ZTkxMjUxMjUtYWY3Yi00MjU2LWFkYWMtZTZjNjU5ZjJhOWM2', - generation: 1, - }, - spec: { - type: 'website', - templater: 'cookiecutter', - path: './template', - schema: { - $schema: 'http://json-schema.org/draft-07/schema#', - required: ['storePath', 'owner'], - properties: { - owner: { - type: 'string', - title: 'Owner', - description: 'Who is going to own this component', - }, - storePath: { - type: 'string', - title: 'Store path', - description: 'GitHub store path in org/repo format', - }, - }, - }, - }, - }; }); const preparer = BitbucketPreparer.fromConfig({ @@ -84,12 +38,20 @@ describe('BitbucketPreparer', () => { appPassword: 'fake-password', }); + const prepareOptions = { + url: 'https://bitbucket.org/backstage-project/backstage-repo', + logger, + workspacePath: '/tmp', + }; + it('calls the clone command with the correct arguments for a repository', async () => { - await preparer.prepare(mockEntity, { logger: getVoidLogger() }); + await preparer.prepare(prepareOptions); expect(mockGitClient.clone).toHaveBeenCalledWith({ url: 'https://bitbucket.org/backstage-project/backstage-repo', - dir: expect.any(String), + dir: '/tmp/checkout', }); + expect(fs.move).toHaveBeenCalledWith('/tmp/checkout', '/tmp/template'); + expect(fs.rmdir).toHaveBeenCalledWith('/tmp/template/.git'); }); it('calls the clone command with the correct arguments if an app password is provided for a repository', async () => { @@ -98,7 +60,7 @@ describe('BitbucketPreparer', () => { username: 'fake-user', appPassword: 'fake-password', }); - await preparer.prepare(mockEntity, { logger }); + await preparer.prepare(prepareOptions); expect(Git.fromAuth).toHaveBeenCalledWith({ logger, @@ -108,22 +70,22 @@ describe('BitbucketPreparer', () => { }); it('calls the clone command with the correct arguments for a repository when no path is provided', async () => { - delete mockEntity.spec.path; - await preparer.prepare(mockEntity, { logger: getVoidLogger() }); + await preparer.prepare(prepareOptions); expect(mockGitClient.clone).toHaveBeenCalledWith({ url: 'https://bitbucket.org/backstage-project/backstage-repo', - dir: expect.any(String), + dir: '/tmp/checkout', }); }); - it('return the temp directory with the path to the folder if it is specified', async () => { - mockEntity.spec.path = './template/test/1/2/3'; - const response = await preparer.prepare(mockEntity, { - logger: getVoidLogger(), + it('moves a template subdirectory to checkout if specified', async () => { + await preparer.prepare({ + url: 'https://bitbucket.org/foo/bar/src/master/1/2/3', + logger, + workspacePath: '/tmp', }); - - expect(response.split('\\').join('/')).toMatch( - /\/template\/test\/1\/2\/3$/, + expect(fs.move).toHaveBeenCalledWith( + '/tmp/checkout/1/2/3', + '/tmp/template', ); }); @@ -133,7 +95,7 @@ describe('BitbucketPreparer', () => { token: 'fake-token', }); - await preparer.prepare(mockEntity, { logger }); + await preparer.prepare(prepareOptions); expect(Git.fromAuth).toHaveBeenCalledWith({ logger, @@ -141,16 +103,4 @@ describe('BitbucketPreparer', () => { password: 'fake-token', }); }); - - it('return the working directory with the path to the folder if it is specified', async () => { - mockEntity.spec.path = './template/test/1/2/3'; - const response = await preparer.prepare(mockEntity, { - workingDirectory: '/workDir', - logger: getVoidLogger(), - }); - - expect(response.split('\\').join('/')).toMatch( - /\/workDir\/graphql-starter-static\/template\/test\/1\/2\/3$/, - ); - }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.test.ts index 2b328f420b..6e7f70ac3d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.test.ts @@ -14,57 +14,34 @@ * limitations under the License. */ -import os from 'os'; -import fs from 'fs-extra'; -import YAML from 'yaml'; -import { FilePreparer } from './file'; -import path from 'path'; -import { - TemplateEntityV1alpha1, - LOCATION_ANNOTATION, -} from '@backstage/catalog-model'; import { getVoidLogger } from '@backstage/backend-common'; +import fs from 'fs-extra'; +import { FilePreparer } from './file'; -const setupTest = async (fixturePath: string) => { - const locationForTemplateYaml = path.resolve( - path.dirname( - require.resolve('@backstage/plugin-scaffolder-backend/package'), - ), - 'fixtures', - fixturePath, - ); - - const [parsedDocument] = YAML.parseAllDocuments( - await fs.readFile(locationForTemplateYaml, 'utf-8'), - ); - - const template: TemplateEntityV1alpha1 = parsedDocument.toJSON(); - template.metadata.annotations = { - [LOCATION_ANNOTATION]: `file:${locationForTemplateYaml}`, - }; - - const filePreparer = new FilePreparer(); - const resultDir = await filePreparer.prepare(template, { - logger: getVoidLogger(), - workingDirectory: os.tmpdir(), - }); - - return { filePreparer, template, resultDir }; -}; +jest.mock('fs-extra'); describe('File preparer', () => { - it('excludes the yaml file from the temp folder', async () => { - const { resultDir } = await setupTest('test-simple-template/template.yaml'); - expect(fs.existsSync(`${resultDir}/template.yaml`)).toBe(false); - }); + const logger = getVoidLogger(); + const preparer = new FilePreparer(); + it('prepares templates from a file path', async () => { + await preparer.prepare({ + url: 'file:///path/to/template', + logger, + workspacePath: '/tmp', + }); + expect(fs.copy).toHaveBeenCalledWith('/path/to/template', '/tmp/checkout', { + recursive: true, + }); + expect(fs.ensureDir).toHaveBeenCalledWith('/tmp/checkout'); - it('resolves relative path from the template', async () => { - const { resultDir } = await setupTest('test-simple-template/template.yaml'); - expect(fs.existsSync(`${resultDir}/expected_file.ts`)).toBe(true); - }); - - it('resolves relative path from the nested template', async () => { - const { resultDir } = await setupTest('test-nested-template/template.yaml'); - expect(fs.existsSync(`${resultDir}/expected_file.ts`)).toBe(true); + await expect( + preparer.prepare({ + url: 'file://not/full/path', + logger, + workspacePath: '/tmp', + }), + ).rejects.toThrow( + "Wrong location protocol, should be 'file', file://not/full/path", + ); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts index b43545f5fc..ac3be609fa 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts @@ -14,12 +14,7 @@ * limitations under the License. */ -jest.doMock('fs-extra', () => ({ - promises: { - mkdtemp: jest.fn(dir => `${dir}-static`), - }, -})); - +import fs from 'fs-extra'; import { GithubPreparer } from './github'; import { TemplateEntityV1alpha1, @@ -27,6 +22,8 @@ import { } from '@backstage/catalog-model'; import { getVoidLogger, Git } from '@backstage/backend-common'; +jest.mock('fs-extra'); + describe('GitHubPreparer', () => { let mockEntity: TemplateEntityV1alpha1; const mockGitClient = { @@ -38,44 +35,6 @@ describe('GitHubPreparer', () => { beforeEach(() => { jest.clearAllMocks(); - mockEntity = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Template', - metadata: { - annotations: { - [LOCATION_ANNOTATION]: - 'github:https://github.com/benjdlambert/backstage-graphql-template/blob/master/template.yaml', - }, - name: 'graphql-starter', - title: 'GraphQL Service', - description: - 'A GraphQL starter template for backstage to get you up and running\nthe best pracices with GraphQL\n', - uid: '9cf16bad-16e0-4213-b314-c4eec773c50b', - etag: 'ZTkxMjUxMjUtYWY3Yi00MjU2LWFkYWMtZTZjNjU5ZjJhOWM2', - generation: 1, - }, - spec: { - type: 'website', - templater: 'cookiecutter', - path: './template', - schema: { - $schema: 'http://json-schema.org/draft-07/schema#', - required: ['storePath', 'owner'], - properties: { - owner: { - type: 'string', - title: 'Owner', - description: 'Who is going to own this component', - }, - storePath: { - type: 'string', - title: 'Store path', - description: 'GitHub store path in org/repo format', - }, - }, - }, - }, - }; }); const preparer = GithubPreparer.fromConfig({ @@ -84,58 +43,47 @@ describe('GitHubPreparer', () => { }); it('calls the clone command with the correct arguments for a repository', async () => { - await preparer.prepare(mockEntity, { logger: getVoidLogger() }); + await preparer.prepare({ + url: + 'https://github.com/benjdlambert/backstage-graphql-template/blob/master/templates/graphql-starter/template', + logger, + workspacePath: '/tmp', + }); expect(mockGitClient.clone).toHaveBeenCalledWith({ url: 'https://github.com/benjdlambert/backstage-graphql-template', dir: expect.any(String), }); + expect(fs.move).toHaveBeenCalledWith( + '/tmp/checkout/templates/graphql-starter/template', + '/tmp/template', + ); + expect(fs.rmdir).toHaveBeenCalledWith('/tmp/template/.git'); }); it('calls the clone command with the correct arguments for a repository when no path is provided', async () => { - delete mockEntity.spec.path; - - await preparer.prepare(mockEntity, { logger: getVoidLogger() }); + await preparer.prepare({ + url: + 'https://github.com/benjdlambert/backstage-graphql-template/blob/master', + logger, + workspacePath: '/tmp', + }); expect(mockGitClient.clone).toHaveBeenCalledWith({ url: 'https://github.com/benjdlambert/backstage-graphql-template', dir: expect.any(String), }); - }); - - it('return the temp directory with the path to the folder if it is specified', async () => { - const preparer = GithubPreparer.fromConfig({ - host: 'github.com', - token: 'fake-token', - }); - mockEntity.spec.path = './template/test/1/2/3'; - const response = await preparer.prepare(mockEntity, { - logger: getVoidLogger(), - }); - expect(response.split('\\').join('/')).toMatch( - /\/template\/test\/1\/2\/3$/, - ); - }); - - it('return the working directory with the path to the folder if it is specified', async () => { - const preparer = GithubPreparer.fromConfig({ - host: 'github.com', - token: 'fake-token', - }); - - mockEntity.spec.path = './template/test/1/2/3'; - const response = await preparer.prepare(mockEntity, { - workingDirectory: '/workDir', - logger: getVoidLogger(), - }); - - expect(response.split('\\').join('/')).toMatch( - /\/workDir\/graphql-starter-static\/template\/test\/1\/2\/3$/, - ); + expect(fs.move).toHaveBeenCalledWith('/tmp/checkout', '/tmp/template'); + expect(fs.rmdir).toHaveBeenCalledWith('/tmp/template/.git'); }); it('calls the clone command with token', async () => { - await preparer.prepare(mockEntity, { logger }); + await preparer.prepare({ + url: + 'https://github.com/benjdlambert/backstage-graphql-template/blob/master', + logger, + workspacePath: '/tmp', + }); expect(Git.fromAuth).toHaveBeenCalledWith({ logger, 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 19cbf793e4..8bb602fb34 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/gitlab.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/gitlab.test.ts @@ -13,60 +13,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -jest.doMock('fs-extra', () => ({ - promises: { - mkdtemp: jest.fn(dir => `${dir}-static`), - }, -})); - +import fs from 'fs-extra'; import { GitlabPreparer } from './gitlab'; -import { - TemplateEntityV1alpha1, - LOCATION_ANNOTATION, -} from '@backstage/catalog-model'; import { getVoidLogger, Git } from '@backstage/backend-common'; -const mockTemplate = (): TemplateEntityV1alpha1 => ({ - apiVersion: 'backstage.io/v1alpha1', - kind: 'Template', - metadata: { - annotations: { - [LOCATION_ANNOTATION]: - 'url:https://gitlab.com/benjdlambert/backstage-graphql-template/-/blob/master/template.yaml', - }, - name: 'graphql-starter', - title: 'GraphQL Service', - description: - 'A GraphQL starter template for backstage to get you up and running\nthe best practices with GraphQL\n', - uid: '9cf16bad-16e0-4213-b314-c4eec773c50b', - etag: 'ZTkxMjUxMjUtYWY3Yi00MjU2LWFkYWMtZTZjNjU5ZjJhOWM2', - generation: 1, - }, - spec: { - type: 'website', - templater: 'cookiecutter', - path: './template', - schema: { - $schema: 'http://json-schema.org/draft-07/schema#', - required: ['storePath', 'owner'], - properties: { - owner: { - type: 'string', - title: 'Owner', - description: 'Who is going to own this component', - }, - storePath: { - type: 'string', - title: 'Store path', - description: 'GitHub store path in org/repo format', - }, - }, - }, - }, -}); +jest.mock('fs-extra'); describe('GitLabPreparer', () => { - let mockEntity: TemplateEntityV1alpha1; const mockGitClient = { clone: jest.fn(), }; @@ -81,61 +34,40 @@ describe('GitLabPreparer', () => { host: 'gitlab.com', token: 'fake-token', }); - it(`calls the clone command with the correct arguments for a repository`, async () => { - mockEntity = mockTemplate(); - await preparer.prepare(mockEntity, { logger: getVoidLogger() }); + it(`calls the clone command with the correct arguments for a repository`, async () => { + await preparer.prepare({ + url: + 'https://gitlab.com/benjdlambert/backstage-graphql-template/-/blob/master', + logger, + workspacePath: '/tmp', + }); expect(mockGitClient.clone).toHaveBeenCalledWith({ url: 'https://gitlab.com/benjdlambert/backstage-graphql-template', - dir: expect.any(String), + dir: '/tmp/checkout', }); - }); - - it(`calls the clone command with the correct arguments if an access token is provided in integrations for a repository`, async () => { - mockEntity = mockTemplate(); - - await preparer.prepare(mockEntity, { logger }); expect(Git.fromAuth).toHaveBeenCalledWith({ logger, username: 'oauth2', password: 'fake-token', }); + + expect(fs.move).toHaveBeenCalledWith('/tmp/checkout', '/tmp/template'); + expect(fs.rmdir).toHaveBeenCalledWith('/tmp/template/.git'); }); - it(`calls the clone command with the correct arguments for a repository when no path is provided`, async () => { - mockEntity = mockTemplate(); - delete mockEntity.spec.path; - - await preparer.prepare(mockEntity, { logger: getVoidLogger() }); - - expect(mockGitClient.clone).toHaveBeenCalledWith({ - url: 'https://gitlab.com/benjdlambert/backstage-graphql-template', - dir: expect.any(String), + it(`clones the template from a sub directory if specified`, async () => { + await preparer.prepare({ + url: + 'https://gitlab.com/benjdlambert/backstage-graphql-template/-/blob/master/1/2/3', + logger, + workspacePath: '/tmp', }); - }); - - it(`return the temp directory with the path to the folder if it is specified`, async () => { - mockEntity = mockTemplate(); - mockEntity.spec.path = './template/test/1/2/3'; - const response = await preparer.prepare(mockEntity, { - logger: getVoidLogger(), - }); - expect(response.split('\\').join('/')).toMatch( - /\/template\/test\/1\/2\/3$/, - ); - }); - - it('return the working directory with the path to the folder if it is specified', async () => { - mockEntity.spec.path = './template/test/1/2/3'; - const response = await preparer.prepare(mockEntity, { - workingDirectory: '/workDir', - logger: getVoidLogger(), - }); - - expect(response.split('\\').join('/')).toMatch( - /\/workDir\/graphql-starter-static\/template\/test\/1\/2\/3$/, + expect(fs.move).toHaveBeenCalledWith( + '/tmp/checkout/1/2/3', + '/tmp/template', ); }); }); From 4944dcf0a7ee7e423c657d426edc7bcd6e67f9bb Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Wed, 27 Jan 2021 11:43:20 +0100 Subject: [PATCH 06/19] joinGitUrlPath Co-authored-by: Patrik Oldsberg --- .../src/scaffolder/stages/helpers.ts | 19 +++++++++++++++++++ .../scaffolder-backend/src/service/router.ts | 9 ++++----- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/stages/helpers.ts index f8d8229298..784e72ca87 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/helpers.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/helpers.ts @@ -13,6 +13,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + +import { posix as posixPath } from 'path'; import { TemplateEntityV1alpha1, LOCATION_ANNOTATION, @@ -53,3 +55,20 @@ export const parseLocationAnnotation = ( location, }; }; + +export function joinGitUrlPath(repoUrl: string, path?: string): string { + const parsed = new URL(repoUrl); + + if (parsed.hostname.endsWith('azure.com')) { + const templatePath = posixPath.normalize( + posixPath.join( + posixPath.dirname(parsed.searchParams.get('path') || '/'), + path || '.', + ), + ); + parsed.searchParams.set('path', templatePath); + return parsed.toString(); + } + + return new URL(path || '.', repoUrl).toString().replace(/\/$/, ''); +} diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index a3e7202f95..9098e5d05e 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -27,6 +27,7 @@ import { TemplaterValues, PublisherBuilder, parseLocationAnnotation, + joinGitUrlPath, FilePreparer, } from '../scaffolder'; import { CatalogEntityClient } from '../lib/catalog'; @@ -138,12 +139,10 @@ export async function createRouter( const preparer = preparers.get(templateEntityLocation); - const url = new URL( - template.spec.path || '.', + const url = joinGitUrlPath( templateEntityLocation, - ) - .toString() - .replace(/\/$/, ''); + template.spec.path, + ); await preparer.prepare({ url, From 766e6526ded114ca148dad96a0501356f83a1f71 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Wed, 27 Jan 2021 11:44:37 +0100 Subject: [PATCH 07/19] add joinGitUrlPath tests --- .../src/scaffolder/stages/helpers.test.ts | 76 ++++++++++++++++++- 1 file changed, 74 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/helpers.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/helpers.test.ts index be6d215e5b..5c809d229e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/helpers.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/helpers.test.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { parseLocationAnnotation } from './helpers'; +import { parseLocationAnnotation, joinGitUrlPath } from './helpers'; import { TemplateEntityV1alpha1, LOCATION_ANNOTATION, @@ -73,7 +73,7 @@ describe('Helpers', () => { metadata: { annotations: { [LOCATION_ANNOTATION]: - ':https://github.com/benjdlambert/backstage-graphql-template/blob/master/template.yaml', + ':https://github.com/o/r/blob/master/template.yaml', }, name: 'graphql-starter', title: 'GraphQL Service', @@ -250,4 +250,76 @@ describe('Helpers', () => { }); }); }); + + describe('joinGitUrlPath', () => { + it.each([ + [ + 'https://github.com/o/r/blob/master/template.yaml', + 'template', + 'https://github.com/o/r/blob/master/template', + ], + [ + 'https://dev.azure.com/o/p/_git/template-repo?path=%2Ftemplate.yaml', + undefined, + 'https://dev.azure.com/o/p/_git/template-repo?path=%2F', + ], + [ + 'https://dev.azure.com/o/p/_git/template-repo?path=%2Ftemplate.yaml', + 'a', + 'https://dev.azure.com/o/p/_git/template-repo?path=%2Fa', + ], + [ + 'https://dev.azure.com/o/p/_git/template-repo?path=%2Fa%2Ftemplate.yaml', + 'b', + 'https://dev.azure.com/o/p/_git/template-repo?path=%2Fa%2Fb', + ], + [ + 'https://github.com/o/r/blob/master/template.yaml', + undefined, + 'https://github.com/o/r/blob/master', + ], + [ + 'https://github.com/o/r/blob/master/template.yaml', + 'template', + 'https://github.com/o/r/blob/master/template', + ], + [ + 'https://github.com/o/r/blob/master/templates/graphql-starter/template.yaml', + 'template', + 'https://github.com/o/r/blob/master/templates/graphql-starter/template', + ], + [ + 'https://gitlab.com/o/r/-/blob/master/template.yaml', + undefined, + 'https://gitlab.com/o/r/-/blob/master', + ], + [ + 'https://gitlab.com/o/r/-/blob/master/template.yaml', + 'template', + 'https://gitlab.com/o/r/-/blob/master/template', + ], + [ + 'https://gitlab.com/o/r/-/blob/master/a/b/c/template.yaml', + '../../c', + 'https://gitlab.com/o/r/-/blob/master/a/c', + ], + [ + 'https://bitbucket.org/p/r/src/master/a/b/template.yaml', + undefined, + 'https://bitbucket.org/p/r/src/master/a/b', + ], + [ + 'https://bitbucket.org/p/r/src/master/a/b/template.yaml', + 'c', + 'https://bitbucket.org/p/r/src/master/a/b/c', + ], + [ + 'https://bitbucket.org/p/r/src/master/a/b/template.yaml', + '../c', + 'https://bitbucket.org/p/r/src/master/a/c', + ], + ])('should join git url %s with path %s', (url, path, result) => { + expect(joinGitUrlPath(url, path)).toBe(result); + }); + }); }); From 6b541f10f921be82f9d20f3a16e917ace2f98801 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Wed, 27 Jan 2021 11:45:51 +0100 Subject: [PATCH 08/19] Update azure tests --- .../scaffolder/stages/prepare/azure.test.ts | 123 +++++------------- .../src/scaffolder/stages/prepare/azure.ts | 2 +- 2 files changed, 34 insertions(+), 91 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/azure.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/azure.test.ts index 8dd72c35e9..1319c2b499 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/azure.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/azure.test.ts @@ -14,19 +14,12 @@ * limitations under the License. */ -jest.doMock('fs-extra', () => ({ - promises: { - mkdtemp: jest.fn(dir => `${dir}-static`), - }, -})); - +import fs from 'fs-extra'; import { AzurePreparer } from './azure'; -import { - TemplateEntityV1alpha1, - LOCATION_ANNOTATION, -} from '@backstage/catalog-model'; import { getVoidLogger, Git } from '@backstage/backend-common'; +jest.mock('fs-extra'); + describe('AzurePreparer', () => { const mockGitClient = { clone: jest.fn(), @@ -36,119 +29,69 @@ describe('AzurePreparer', () => { jest.spyOn(Git, 'fromAuth').mockReturnValue(mockGitClient as any); - let mockEntity: TemplateEntityV1alpha1; - beforeEach(() => { - jest.clearAllMocks(); - mockEntity = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Template', - metadata: { - annotations: { - [LOCATION_ANNOTATION]: - 'url:https://dev.azure.com/backstage-org/backstage-project/_git/template-repo?path=%2Ftemplate.yaml', - }, - name: 'graphql-starter', - title: 'GraphQL Service', - description: - 'A GraphQL starter template for backstage to get you up and running\nthe best pracices with GraphQL\n', - uid: '9cf16bad-16e0-4213-b314-c4eec773c50b', - etag: 'ZTkxMjUxMjUtYWY3Yi00MjU2LWFkYWMtZTZjNjU5ZjJhOWM2', - generation: 1, - }, - spec: { - type: 'website', - templater: 'cookiecutter', - path: './template', - schema: { - $schema: 'http://json-schema.org/draft-07/schema#', - required: ['storePath', 'owner'], - properties: { - owner: { - type: 'string', - title: 'Owner', - description: 'Who is going to own this component', - }, - storePath: { - type: 'string', - title: 'Store path', - description: 'GitHub store path in org/repo format', - }, - }, - }, - }, - }; - }); - const preparer = AzurePreparer.fromConfig({ host: 'dev.azure.com', token: 'fake-azure-token', }); - // TODO(blam): Here's a test that will fail when the deprecation is complete - it('calls the clone command with deprecated token', async () => { - await preparer.prepare(mockEntity, { logger }); - - expect(Git.fromAuth).toHaveBeenCalledWith({ - logger, - password: 'fake-azure-token', - username: 'notempty', - }); - }); + const prepareOptions = { + url: + 'https://dev.azure.com/backstage-org/backstage-project/_git/template-repo', + workspacePath: '/tmp', + logger, + }; it('calls the clone command with token from integrations config', async () => { - await preparer.prepare(mockEntity, { logger }); + await preparer.prepare(prepareOptions); expect(Git.fromAuth).toHaveBeenCalledWith({ logger, password: 'fake-azure-token', username: 'notempty', }); + expect(fs.move).toHaveBeenCalledWith('/tmp/checkout', '/tmp/template'); + expect(fs.rmdir).toHaveBeenCalledWith('/tmp/template/.git'); }); it('calls the clone command with the correct arguments for a repository', async () => { - await preparer.prepare(mockEntity, { logger: getVoidLogger() }); + await preparer.prepare(prepareOptions); expect(mockGitClient.clone).toHaveBeenCalledWith({ url: 'https://dev.azure.com/backstage-org/backstage-project/_git/template-repo', - dir: expect.any(String), + dir: '/tmp/checkout', }); }); it('calls the clone command with the correct arguments for a repository when no path is provided', async () => { - delete mockEntity.spec.path; - - await preparer.prepare(mockEntity, { logger: getVoidLogger() }); + await preparer.prepare({ + url: + 'https://dev.azure.com/backstage-org/backstage-project/_git/template-repo', + workspacePath: '/tmp', + logger, + }); expect(mockGitClient.clone).toHaveBeenCalledWith({ url: 'https://dev.azure.com/backstage-org/backstage-project/_git/template-repo', - dir: expect.any(String), + dir: '/tmp/checkout', }); + expect(fs.move).toHaveBeenCalledWith('/tmp/checkout', '/tmp/template'); }); - it('return the temp directory with the path to the folder if it is specified', async () => { - mockEntity.spec.path = './template/test/1/2/3'; - - const response = await preparer.prepare(mockEntity, { - logger: getVoidLogger(), + it('moves the template from path if it is specified', async () => { + const path = './template/test/1/2/3'; + await preparer.prepare({ + url: `https://dev.azure.com/backstage-org/backstage-project/_git/template-repo?path=${encodeURIComponent( + path, + )}`, + logger, + workspacePath: '/tmp', }); - expect(response.split('\\').join('/')).toMatch( - /\/template\/test\/1\/2\/3$/, - ); - }); - - it('return the working directory with the path to the folder if it is specified', async () => { - mockEntity.spec.path = './template/test/1/2/3'; - - const response = await preparer.prepare(mockEntity, { - workingDirectory: '/workDir', - logger: getVoidLogger(), - }); - - expect(response.split('\\').join('/')).toMatch( - /\/workDir\/graphql-starter-static\/template\/test\/1\/2\/3$/, + expect(fs.move).toHaveBeenCalledWith( + '/tmp/checkout/template/test/1/2/3', + '/tmp/template', ); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/azure.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/azure.ts index cca6baa0b1..e3fd5fe68b 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/azure.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/azure.ts @@ -33,7 +33,7 @@ export class AzurePreparer implements PreparerBase { const targetPath = path.join(workspacePath, 'template'); const fullPathToTemplate = path.resolve( checkoutPath, - parsedGitUrl.filepath, + parsedGitUrl.filepath ?? '', ); // Username can be anything but the empty string according to: From 06af096c49bf31d2a0b11d3eb898f5455367e3c7 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Wed, 27 Jan 2021 15:15:52 +0100 Subject: [PATCH 09/19] Set workspace path based on platform --- .../scaffolder/stages/prepare/azure.test.ts | 28 +++++++++++-------- .../stages/prepare/bitbucket.test.ts | 22 +++++++++------ 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/azure.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/azure.test.ts index 1319c2b499..08b3c325ae 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/azure.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/azure.test.ts @@ -15,6 +15,8 @@ */ import fs from 'fs-extra'; +import os from 'os'; +import { resolve } from 'path'; import { AzurePreparer } from './azure'; import { getVoidLogger, Git } from '@backstage/backend-common'; @@ -34,23 +36,25 @@ describe('AzurePreparer', () => { token: 'fake-azure-token', }); + const workspacePath = os.platform() === 'win32' ? 'C:\\tmp' : '/tmp'; + const checkoutPath = resolve(workspacePath, 'checkout'); + const templatePath = resolve(workspacePath, 'template'); const prepareOptions = { url: 'https://dev.azure.com/backstage-org/backstage-project/_git/template-repo', - workspacePath: '/tmp', + workspacePath, logger, }; it('calls the clone command with token from integrations config', async () => { await preparer.prepare(prepareOptions); - expect(Git.fromAuth).toHaveBeenCalledWith({ logger, password: 'fake-azure-token', username: 'notempty', }); - expect(fs.move).toHaveBeenCalledWith('/tmp/checkout', '/tmp/template'); - expect(fs.rmdir).toHaveBeenCalledWith('/tmp/template/.git'); + expect(fs.move).toHaveBeenCalledWith(checkoutPath, templatePath); + expect(fs.rmdir).toHaveBeenCalledWith(resolve(templatePath, '.git')); }); it('calls the clone command with the correct arguments for a repository', async () => { @@ -59,7 +63,7 @@ describe('AzurePreparer', () => { expect(mockGitClient.clone).toHaveBeenCalledWith({ url: 'https://dev.azure.com/backstage-org/backstage-project/_git/template-repo', - dir: '/tmp/checkout', + dir: checkoutPath, }); }); @@ -67,31 +71,31 @@ describe('AzurePreparer', () => { await preparer.prepare({ url: 'https://dev.azure.com/backstage-org/backstage-project/_git/template-repo', - workspacePath: '/tmp', + workspacePath, logger, }); expect(mockGitClient.clone).toHaveBeenCalledWith({ url: 'https://dev.azure.com/backstage-org/backstage-project/_git/template-repo', - dir: '/tmp/checkout', + dir: checkoutPath, }); - expect(fs.move).toHaveBeenCalledWith('/tmp/checkout', '/tmp/template'); + expect(fs.move).toHaveBeenCalledWith(checkoutPath, templatePath); }); it('moves the template from path if it is specified', async () => { - const path = './template/test/1/2/3'; + const path = './subdir'; await preparer.prepare({ url: `https://dev.azure.com/backstage-org/backstage-project/_git/template-repo?path=${encodeURIComponent( path, )}`, logger, - workspacePath: '/tmp', + workspacePath, }); expect(fs.move).toHaveBeenCalledWith( - '/tmp/checkout/template/test/1/2/3', - '/tmp/template', + resolve(checkoutPath, 'subdir'), + templatePath, ); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/bitbucket.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/bitbucket.test.ts index 84e72eafb8..2f93346470 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/bitbucket.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/bitbucket.test.ts @@ -17,6 +17,8 @@ import fs from 'fs-extra'; import { BitbucketPreparer } from './bitbucket'; import { getVoidLogger, Git } from '@backstage/backend-common'; +import { resolve } from 'path'; +import os from 'os'; jest.mock('fs-extra'); @@ -38,20 +40,24 @@ describe('BitbucketPreparer', () => { appPassword: 'fake-password', }); + const workspacePath = os.platform() === 'win32' ? 'C:\\tmp' : '/tmp'; + const checkoutPath = resolve(workspacePath, 'checkout'); + const templatePath = resolve(workspacePath, 'template'); + const prepareOptions = { url: 'https://bitbucket.org/backstage-project/backstage-repo', logger, - workspacePath: '/tmp', + workspacePath, }; it('calls the clone command with the correct arguments for a repository', async () => { await preparer.prepare(prepareOptions); expect(mockGitClient.clone).toHaveBeenCalledWith({ url: 'https://bitbucket.org/backstage-project/backstage-repo', - dir: '/tmp/checkout', + dir: checkoutPath, }); - expect(fs.move).toHaveBeenCalledWith('/tmp/checkout', '/tmp/template'); - expect(fs.rmdir).toHaveBeenCalledWith('/tmp/template/.git'); + expect(fs.move).toHaveBeenCalledWith(checkoutPath, templatePath); + expect(fs.rmdir).toHaveBeenCalledWith(resolve(templatePath, '.git')); }); it('calls the clone command with the correct arguments if an app password is provided for a repository', async () => { @@ -73,7 +79,7 @@ describe('BitbucketPreparer', () => { await preparer.prepare(prepareOptions); expect(mockGitClient.clone).toHaveBeenCalledWith({ url: 'https://bitbucket.org/backstage-project/backstage-repo', - dir: '/tmp/checkout', + dir: checkoutPath, }); }); @@ -81,11 +87,11 @@ describe('BitbucketPreparer', () => { await preparer.prepare({ url: 'https://bitbucket.org/foo/bar/src/master/1/2/3', logger, - workspacePath: '/tmp', + workspacePath, }); expect(fs.move).toHaveBeenCalledWith( - '/tmp/checkout/1/2/3', - '/tmp/template', + resolve(checkoutPath, '1', '2', '3'), + templatePath, ); }); From 80366b60ad3226544d9a305537d18881957f83dd Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Wed, 27 Jan 2021 16:48:19 +0100 Subject: [PATCH 10/19] Use windows friendly temp dir --- .../scaffolder/stages/prepare/github.test.ts | 29 ++++++++++--------- .../scaffolder/stages/prepare/gitlab.test.ts | 20 ++++++++----- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts index ac3be609fa..7b97975ebf 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts @@ -15,17 +15,18 @@ */ import fs from 'fs-extra'; +import os from 'os'; +import { resolve } from 'path'; import { GithubPreparer } from './github'; -import { - TemplateEntityV1alpha1, - LOCATION_ANNOTATION, -} from '@backstage/catalog-model'; import { getVoidLogger, Git } from '@backstage/backend-common'; jest.mock('fs-extra'); describe('GitHubPreparer', () => { - let mockEntity: TemplateEntityV1alpha1; + const workspacePath = os.platform() === 'win32' ? 'C:\\tmp' : '/tmp'; + const checkoutPath = resolve(workspacePath, 'checkout'); + const templatePath = resolve(workspacePath, 'template'); + const mockGitClient = { clone: jest.fn(), }; @@ -47,16 +48,16 @@ describe('GitHubPreparer', () => { url: 'https://github.com/benjdlambert/backstage-graphql-template/blob/master/templates/graphql-starter/template', logger, - workspacePath: '/tmp', + workspacePath, }); expect(mockGitClient.clone).toHaveBeenCalledWith({ url: 'https://github.com/benjdlambert/backstage-graphql-template', - dir: expect.any(String), + dir: checkoutPath, }); expect(fs.move).toHaveBeenCalledWith( - '/tmp/checkout/templates/graphql-starter/template', - '/tmp/template', + resolve(checkoutPath, 'templates', 'graphql-starter', 'template'), + templatePath, ); expect(fs.rmdir).toHaveBeenCalledWith('/tmp/template/.git'); }); @@ -66,15 +67,15 @@ describe('GitHubPreparer', () => { url: 'https://github.com/benjdlambert/backstage-graphql-template/blob/master', logger, - workspacePath: '/tmp', + workspacePath, }); expect(mockGitClient.clone).toHaveBeenCalledWith({ url: 'https://github.com/benjdlambert/backstage-graphql-template', - dir: expect.any(String), + dir: checkoutPath, }); - expect(fs.move).toHaveBeenCalledWith('/tmp/checkout', '/tmp/template'); - expect(fs.rmdir).toHaveBeenCalledWith('/tmp/template/.git'); + expect(fs.move).toHaveBeenCalledWith(checkoutPath, templatePath); + expect(fs.rmdir).toHaveBeenCalledWith(resolve(templatePath, '.git')); }); it('calls the clone command with token', async () => { @@ -82,7 +83,7 @@ describe('GitHubPreparer', () => { url: 'https://github.com/benjdlambert/backstage-graphql-template/blob/master', logger, - workspacePath: '/tmp', + workspacePath, }); expect(Git.fromAuth).toHaveBeenCalledWith({ 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 8bb602fb34..35da164c3c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/gitlab.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/gitlab.test.ts @@ -14,12 +14,18 @@ * limitations under the License. */ import fs from 'fs-extra'; +import os from 'os'; +import { resolve } from 'path'; import { GitlabPreparer } from './gitlab'; import { getVoidLogger, Git } from '@backstage/backend-common'; jest.mock('fs-extra'); describe('GitLabPreparer', () => { + const workspacePath = os.platform() === 'win32' ? 'C:\\tmp' : '/tmp'; + const checkoutPath = resolve(workspacePath, 'checkout'); + const templatePath = resolve(workspacePath, 'template'); + const mockGitClient = { clone: jest.fn(), }; @@ -40,12 +46,12 @@ describe('GitLabPreparer', () => { url: 'https://gitlab.com/benjdlambert/backstage-graphql-template/-/blob/master', logger, - workspacePath: '/tmp', + workspacePath, }); expect(mockGitClient.clone).toHaveBeenCalledWith({ url: 'https://gitlab.com/benjdlambert/backstage-graphql-template', - dir: '/tmp/checkout', + dir: checkoutPath, }); expect(Git.fromAuth).toHaveBeenCalledWith({ @@ -54,8 +60,8 @@ describe('GitLabPreparer', () => { password: 'fake-token', }); - expect(fs.move).toHaveBeenCalledWith('/tmp/checkout', '/tmp/template'); - expect(fs.rmdir).toHaveBeenCalledWith('/tmp/template/.git'); + expect(fs.move).toHaveBeenCalledWith(checkoutPath, templatePath); + expect(fs.rmdir).toHaveBeenCalledWith(resolve(templatePath, '.git')); }); it(`clones the template from a sub directory if specified`, async () => { @@ -63,11 +69,11 @@ describe('GitLabPreparer', () => { url: 'https://gitlab.com/benjdlambert/backstage-graphql-template/-/blob/master/1/2/3', logger, - workspacePath: '/tmp', + workspacePath, }); expect(fs.move).toHaveBeenCalledWith( - '/tmp/checkout/1/2/3', - '/tmp/template', + resolve(checkoutPath, '1', '2', '3'), + templatePath, ); }); }); From 459027920dcddc56b68e7b7354aa82b4a7b38a11 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Thu, 28 Jan 2021 09:06:42 +0100 Subject: [PATCH 11/19] Fix cookiecutter tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw --- .../stages/templater/cookiecutter.test.ts | 173 ++++++++---------- 1 file changed, 73 insertions(+), 100 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts index 656abd9613..f3c716e8a7 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts @@ -13,48 +13,29 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -jest.mock('./helpers', () => ({ - runDockerContainer: jest.fn(), - runCommand: jest.fn(), -})); -jest.mock('command-exists-promise', () => jest.fn()); + +const runDockerContainer = jest.fn(); +const runCommand = jest.fn(); +const commandExists = jest.fn(); + +jest.mock('./helpers', () => ({ runDockerContainer, runCommand })); +jest.mock('command-exists-promise', () => commandExists); +jest.mock('fs-extra'); import { CookieCutter } from './cookiecutter'; import fs from 'fs-extra'; -import os from 'os'; -import path from 'path'; -import { RunDockerContainerOptions, RunCommandOptions } from './helpers'; import { PassThrough } from 'stream'; import Docker from 'dockerode'; import parseGitUrl from 'git-url-parse'; -const commandExists = require('command-exists-promise'); - describe('CookieCutter Templater', () => { - const cookie = new CookieCutter(); const mockDocker = {} as Docker; - const { - runDockerContainer, - }: { - runDockerContainer: jest.Mock; - } = require('./helpers'); - jest - .spyOn(fs, 'readdir') - .mockImplementation(() => Promise.resolve(['newthing'])); - - beforeEach(async () => { + beforeEach(() => { jest.clearAllMocks(); }); - const mkTemp = async () => { - const tempDir = os.tmpdir(); - return await fs.promises.mkdtemp(path.join(tempDir, 'temp')); - }; - it('should write a cookiecutter.json file with the values from the entity', async () => { - const tempdir = await mkTemp(); - const values = { owner: 'blobby', storePath: 'https://github.com/org/repo', @@ -65,22 +46,30 @@ describe('CookieCutter Templater', () => { }, }; - await cookie.run({ directory: tempdir, values, dockerClient: mockDocker }); + jest.spyOn(fs, 'readdir').mockResolvedValueOnce(['newthing']); - const cookieCutterJson = await fs.readJSON(`${tempdir}/cookiecutter.json`); + const templater = new CookieCutter(); + await templater.run({ + workspacePath: 'tempdir', + values, + dockerClient: mockDocker, + }); - expect(cookieCutterJson).toEqual( - expect.objectContaining(JSON.parse(JSON.stringify(values))), + expect(fs.writeJson).toBeCalledWith( + 'tempdir/template/cookiecutter.json', + expect.objectContaining(values), ); }); it('should merge any value that is in the cookiecutter.json path already', async () => { - const tempdir = await mkTemp(); const existingJson = { _copy_without_render: ['./github/workflows/*'], }; - await fs.writeJSON(`${tempdir}/cookiecutter.json`, existingJson); + jest + .spyOn(fs, 'readJSON') + .mockImplementationOnce(() => Promise.resolve(existingJson)); + jest.spyOn(fs, 'readdir').mockResolvedValueOnce(['newthing']); const values = { owner: 'blobby', @@ -91,11 +80,14 @@ describe('CookieCutter Templater', () => { }, }; - await cookie.run({ directory: tempdir, values, dockerClient: mockDocker }); + const templater = new CookieCutter(); + await templater.run({ + workspacePath: 'tempdir', + values, + dockerClient: mockDocker, + }); - const cookieCutterJson = await fs.readJSON(`${tempdir}/cookiecutter.json`); - - expect(cookieCutterJson).toEqual({ + expect(fs.writeJSON).toBeCalledWith('tempdir/template/cookiecutter.json', { ...existingJson, ...values, destination: { @@ -105,9 +97,9 @@ describe('CookieCutter Templater', () => { }); it('should throw an error if the cookiecutter json is malformed and not missing', async () => { - const tempdir = await mkTemp(); - - await fs.writeFile(`${tempdir}/cookiecutter.json`, "{'"); + jest.spyOn(fs, 'readJSON').mockImplementationOnce(() => { + throw new Error('BAM'); + }); const values = { owner: 'blobby', @@ -117,14 +109,17 @@ describe('CookieCutter Templater', () => { }, }; + const templater = new CookieCutter(); await expect( - cookie.run({ directory: tempdir, values, dockerClient: mockDocker }), - ).rejects.toThrow(/Unexpected token ' in JSON at position 1/); + templater.run({ + workspacePath: 'tempdir', + values, + dockerClient: mockDocker, + }), + ).rejects.toThrow('BAM'); }); it('should run the correct docker container with the correct bindings for the volumes', async () => { - const tempdir = await mkTemp(); - const values = { owner: 'blobby', storePath: 'https://github.com/org/repo', @@ -134,7 +129,14 @@ describe('CookieCutter Templater', () => { }, }; - await cookie.run({ directory: tempdir, values, dockerClient: mockDocker }); + jest.spyOn(fs, 'readdir').mockResolvedValueOnce(['newthing']); + + const templater = new CookieCutter(); + await templater.run({ + workspacePath: 'tempdir', + values, + dockerClient: mockDocker, + }); expect(runDockerContainer).toHaveBeenCalledWith({ imageName: 'spotify/backstage-cookiecutter', @@ -146,39 +148,16 @@ describe('CookieCutter Templater', () => { '/template', '--verbose', ], - templateDir: tempdir, - resultDir: expect.stringContaining(`${tempdir}-result`), + templateDir: 'tempdir/template', + resultDir: 'tempdir/intermediate', logStream: undefined, dockerClient: mockDocker, }); }); - it('should return the result path to the end templated folder', async () => { - const tempdir = await mkTemp(); - - const values = { - owner: 'blobby', - storePath: 'https://github.com/org/repo', - component_id: 'newthing', - destination: { - git: parseGitUrl('https://github.com/org/repo'), - }, - }; - - const { resultDir } = await cookie.run({ - directory: tempdir, - values, - dockerClient: mockDocker, - }); - - expect(resultDir.startsWith(`${tempdir}-result`)).toBeTruthy(); - }); - it('should pass through the streamer to the run docker helper', async () => { const stream = new PassThrough(); - const tempdir = await mkTemp(); - const values = { owner: 'blobby', storePath: 'https://github.com/org/repo', @@ -188,8 +167,11 @@ describe('CookieCutter Templater', () => { }, }; - await cookie.run({ - directory: tempdir, + jest.spyOn(fs, 'readdir').mockResolvedValueOnce(['newthing']); + + const templater = new CookieCutter(); + await templater.run({ + workspacePath: 'tempdir', values, logStream: stream, dockerClient: mockDocker, @@ -205,29 +187,17 @@ describe('CookieCutter Templater', () => { '/template', '--verbose', ], - templateDir: tempdir, - resultDir: expect.stringContaining(`${tempdir}-result`), + templateDir: 'tempdir/template', + resultDir: 'tempdir/intermediate', logStream: stream, dockerClient: mockDocker, }); }); describe('when cookiecutter is available', () => { - beforeAll(() => { - commandExists.mockImplementation(() => () => true); - }); - it('use the binary', async () => { - const { - runCommand, - }: { - runCommand: jest.Mock; - } = require('./helpers'); - const stream = new PassThrough(); - const tempdir = await mkTemp(); - const values = { owner: 'blobby', storePath: 'https://github.com/org/repo', @@ -237,8 +207,12 @@ describe('CookieCutter Templater', () => { }, }; - await cookie.run({ - directory: tempdir, + jest.spyOn(fs, 'readdir').mockResolvedValueOnce(['newthing']); + commandExists.mockImplementationOnce(() => () => true); + + const templater = new CookieCutter(); + await templater.run({ + workspacePath: 'tempdir', values, logStream: stream, dockerClient: mockDocker, @@ -249,8 +223,8 @@ describe('CookieCutter Templater', () => { args: expect.arrayContaining([ '--no-input', '-o', - tempdir, - expect.stringContaining(`${tempdir}-result`), + 'tempdir/intermediate', + 'tempdir/template', '--verbose', ]), logStream: stream, @@ -259,18 +233,17 @@ describe('CookieCutter Templater', () => { }); describe('when nothing was generated', () => { - beforeEach(() => { - jest.spyOn(fs, 'readdir').mockImplementation(() => Promise.resolve([])); - }); - it('throws an error', async () => { const stream = new PassThrough(); - const tempdir = await mkTemp(); + jest + .spyOn(fs, 'readdir') + .mockImplementationOnce(() => Promise.resolve([])); - return expect( - cookie.run({ - directory: tempdir, + const templater = new CookieCutter(); + await expect( + templater.run({ + workspacePath: 'tempdir', values: { owner: 'blobby', storePath: 'https://github.com/org/repo', @@ -281,7 +254,7 @@ describe('CookieCutter Templater', () => { logStream: stream, dockerClient: mockDocker, }), - ).rejects.toThrow(/Cookie Cutter did not generate anything/); + ).rejects.toThrow(/No data generated by cookiecutter/); }); }); }); From 502849fe8ea5a801253bb6b11af92bacd71acf33 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Thu, 28 Jan 2021 09:14:43 +0100 Subject: [PATCH 12/19] Fix paths tests for windows --- .../scaffolder/stages/prepare/file.test.ts | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.test.ts index 6e7f70ac3d..408653aee0 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.test.ts @@ -17,28 +17,37 @@ import { getVoidLogger } from '@backstage/backend-common'; import fs from 'fs-extra'; import { FilePreparer } from './file'; +import os from 'os'; +import { resolve } from 'path'; jest.mock('fs-extra'); describe('File preparer', () => { - const logger = getVoidLogger(); - const preparer = new FilePreparer(); it('prepares templates from a file path', async () => { + const logger = getVoidLogger(); + const preparer = new FilePreparer(); + const workspacePath = os.platform() === 'win32' ? 'C:\\tmp' : '/tmp'; + const checkoutPath = resolve(workspacePath, 'checkout'); + await preparer.prepare({ url: 'file:///path/to/template', logger, - workspacePath: '/tmp', + workspacePath, }); - expect(fs.copy).toHaveBeenCalledWith('/path/to/template', '/tmp/checkout', { - recursive: true, - }); - expect(fs.ensureDir).toHaveBeenCalledWith('/tmp/checkout'); + expect(fs.copy).toHaveBeenCalledWith( + resolve('/path', 'to', 'template'), + checkoutPath, + { + recursive: true, + }, + ); + expect(fs.ensureDir).toHaveBeenCalledWith(checkoutPath); await expect( preparer.prepare({ url: 'file://not/full/path', logger, - workspacePath: '/tmp', + workspacePath, }), ).rejects.toThrow( "Wrong location protocol, should be 'file', file://not/full/path", From b77a9852059e80b019698e9636b331332048e2f8 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Thu, 28 Jan 2021 09:33:55 +0100 Subject: [PATCH 13/19] Update templater tests --- .../stages/templater/templaters.test.ts | 92 +------------------ 1 file changed, 2 insertions(+), 90 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/templater/templaters.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/templaters.test.ts index 9ef20b7740..209210c075 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/templaters.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/templater/templaters.test.ts @@ -14,52 +14,13 @@ * limitations under the License. */ import { Templaters } from '.'; -import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import { CookieCutter } from './cookiecutter'; describe('Templaters', () => { - const mockTemplate: TemplateEntityV1alpha1 = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Template', - metadata: { - annotations: { - 'backstage.io/managed-by-location': - 'file:/Users/bingo/backstage/plugins/scaffolder-backend/sample-templates/react-ssr-template/template.yaml', - }, - name: 'react-ssr-template', - title: 'React SSR Template', - description: - 'Next.js application skeleton for creating isomorphic web applications.', - uid: '7357f4c5-aa58-4a1e-9670-18931eef771f', - etag: 'YWUxZWQyY2EtZDkxMC00MDM0LWI0ODAtMDgwMWY0YzdlMWIw', - generation: 1, - }, - spec: { - templater: 'cookiecutter', - path: '.', - type: 'website', - schema: { - $schema: 'http://json-schema.org/draft-07/schema#', - required: ['storePath', 'owner'], - properties: { - owner: { - type: 'string', - title: 'Owner', - description: 'Who is going to own this component', - }, - storePath: { - type: 'string', - title: 'Store path', - description: 'GitHub store path in org/repo format', - }, - }, - }, - }, - }; it('should throw an error when the templater is not registered', () => { const templaters = new Templaters(); - expect(() => templaters.get(mockTemplate)).toThrow( + expect(() => templaters.get('cookiecutter')).toThrow( expect.objectContaining({ message: 'No templater registered for template: "cookiecutter"', }), @@ -71,55 +32,6 @@ describe('Templaters', () => { templaters.register('cookiecutter', templater); - expect(templaters.get(mockTemplate)).toBe(templater); - }); - - it('should throw an error if the templater does not exist in the entity', () => { - const brokenTemplate: TemplateEntityV1alpha1 = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Template', - metadata: { - annotations: {}, - name: 'react-ssr-template', - title: 'React SSR Template', - description: - 'Next.js application skeleton for creating isomorphic web applications.', - uid: '7357f4c5-aa58-4a1e-9670-18931eef771f', - etag: 'YWUxZWQyY2EtZDkxMC00MDM0LWI0ODAtMDgwMWY0YzdlMWIw', - generation: 1, - }, - spec: { - type: 'website', - path: '.', - templater: '', - schema: { - $schema: 'http://json-schema.org/draft-07/schema#', - required: ['storePath', 'owner'], - properties: { - owner: { - type: 'string', - title: 'Owner', - description: 'Who is going to own this component', - }, - storePath: { - type: 'string', - title: 'Store path', - description: 'GitHub store path in org/repo format', - }, - }, - }, - }, - }; - - const templaters = new Templaters(); - - expect(() => templaters.get(brokenTemplate)).toThrow( - expect.objectContaining({ - name: 'InputError', - message: expect.stringContaining( - 'Template does not have a required templating key', - ), - }), - ); + expect(templaters.get('cookiecutter')).toBe(templater); }); }); From cdea0baf1c142a696516f21f1d0c115e824dc593 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Thu, 28 Jan 2021 15:33:47 +0100 Subject: [PATCH 14/19] Add changeset --- .changeset/wicked-beds-buy.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 .changeset/wicked-beds-buy.md diff --git a/.changeset/wicked-beds-buy.md b/.changeset/wicked-beds-buy.md new file mode 100644 index 0000000000..2f0586bf93 --- /dev/null +++ b/.changeset/wicked-beds-buy.md @@ -0,0 +1,10 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +--- + +The scaffolder is updated to generate a unique workspace directory inside the temp folder which gets cleaned up afterwards. + +prepare/template/publish steps is refactored to operate on known directories(`checkout/`, `template/`, `result/`) inside the generated temp directory. +Updates preparers to take the template url instead of the entire template. This is all in preparation f + +Fix broken configuration GitHub actions in Create React App template. From 327c94da3920c55fc7b06840028ec10a75be960b Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Thu, 28 Jan 2021 15:34:35 +0100 Subject: [PATCH 15/19] Fix CRA templating The GitHub actions setup was never templated correctly. Use known directories for templating --- .../scaffolder/stages/templater/cra/index.ts | 102 +++++++++++++----- .../cra/templates/.github/workflows/main.yml | 41 ------- 2 files changed, 74 insertions(+), 69 deletions(-) delete mode 100644 plugins/scaffolder-backend/src/scaffolder/stages/templater/cra/templates/.github/workflows/main.yml diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cra/index.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cra/index.ts index f5863feb10..5e957e0504 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cra/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cra/index.ts @@ -17,25 +17,30 @@ import fs from 'fs-extra'; import { runDockerContainer } from '../helpers'; import { TemplaterBase, TemplaterRunOptions } from '..'; import path from 'path'; -import { TemplaterRunResult } from '../types'; import * as yaml from 'yaml'; -import { resolvePackagePath } from '@backstage/backend-common'; // TODO(blam): Replace with the universal import from github-actions after a release // As it will break the E2E without it const GITHUB_ACTIONS_ANNOTATION = 'github.com/project-slug'; export class CreateReactAppTemplater implements TemplaterBase { - public async run(options: TemplaterRunOptions): Promise { + public async run({ + workspacePath, + values, + logStream, + dockerClient, + }: TemplaterRunOptions): Promise { const { component_id: componentName, use_typescript: withTypescript, use_github_actions: withGithubActions, description, owner, - } = options.values; + } = values; + const intermediateDir = path.join(workspacePath, 'template'); + await fs.ensureDir(intermediateDir); - const resultDir = await fs.promises.mkdtemp(`${options.directory}-result`); + const resultDir = path.join(workspacePath, 'result'); await runDockerContainer({ imageName: 'node:lts-alpine', @@ -44,35 +49,80 @@ export class CreateReactAppTemplater implements TemplaterBase { componentName as string, withTypescript ? ' --template typescript' : '', ], - templateDir: options.directory, - resultDir, - logStream: options.logStream, - dockerClient: options.dockerClient, + templateDir: intermediateDir, + resultDir: intermediateDir, + logStream: logStream, + dockerClient: dockerClient, createOptions: { Entrypoint: ['npx'], WorkingDir: '/result', }, }); - const extraAnnotations: Record = {}; - const finalDir = path.resolve( - resultDir, - options.values.component_id as string, - ); + // if cookiecutter was successful, intermediateDir will contain + // exactly one directory. + const [generated] = await fs.readdir(intermediateDir); + if (generated === undefined) { + throw new Error('No data generated by cookiecutter'); + } + + await fs.move(path.join(intermediateDir, generated), resultDir); + + const extraAnnotations: Record = {}; if (withGithubActions) { - await fs.promises.mkdir(`${finalDir}/.github`); - await fs.promises.mkdir(`${finalDir}/.github/workflows`); - await fs.promises.copyFile( - `${resolvePackagePath( - '@backstage/plugin-scaffolder-backend', - )}/templates/.github/workflows/main.yml`, - `${finalDir}/.github/workflows/main.yml`, + await fs.mkdir(`${resultDir}/.github`); + await fs.mkdir(`${resultDir}/.github/workflows`); + const githubActionsYaml = ` +name: CRA Build + +on: + push: + branches: [ master ] + pull_request: + branches: [ master ] + +jobs: + build: + runs-on: ubuntu-latest + + strategy: + matrix: + node-version: [12.x] + + steps: + - name: checkout code + uses: actions/checkout@v1 + - name: get yarn cache + id: yarn-cache + run: echo "::set-output name=dir::$(yarn cache dir)" + - uses: actions/cache@v2 + with: + path: \${{ steps.yarn-cache.outputs.dir }} + key: \${{ runner.os }}-yarn-\${{ hashFiles('**/yarn.lock') }} + restore-keys: | + \${{ runner.os }}-yarn- + - name: use node.js \${{ matrix.node-version }} + uses: actions/setup-node@v1 + with: + node-version: \${{ matrix.node-version }} + - name: yarn install, build, and test + working-directory: . + run: | + yarn install + yarn build + yarn test + env: + CI: true + `; + await fs.writeFile( + `${resultDir}/.github/workflows/main.yml`, + githubActionsYaml, ); extraAnnotations[ GITHUB_ACTIONS_ANNOTATION - ] = `${options.values?.destination?.git?.owner}/${options.values?.destination?.git?.name}`; + ] = `${values?.destination?.git?.owner}/${values?.destination?.git?.name}`; } const componentInfo = { @@ -92,13 +142,9 @@ export class CreateReactAppTemplater implements TemplaterBase { }, }; - await fs.promises.writeFile( - `${finalDir}/catalog-info.yaml`, + await fs.writeFile( + `${resultDir}/catalog-info.yaml`, yaml.stringify(componentInfo), ); - - return { - resultDir: finalDir, - }; } } diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cra/templates/.github/workflows/main.yml b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cra/templates/.github/workflows/main.yml deleted file mode 100644 index 65cae9abbb..0000000000 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cra/templates/.github/workflows/main.yml +++ /dev/null @@ -1,41 +0,0 @@ -name: CRA Build - -on: - push: - branches: [ master ] - pull_request: - branches: [ master ] - - -jobs: - build: - runs-on: ubuntu-latest - - strategy: - matrix: - node-version: [12.x] - - steps: - - name: checkout code - uses: actions/checkout@v1 - - name: get yarn cache - id: yarn-cache - run: echo "::set-output name=dir::$(yarn cache dir)" - - uses: actions/cache@v2 - with: - path: ${{ steps.yarn-cache.outputs.dir }} - key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }} - restore-keys: | - ${{ runner.os }}-yarn- - - name: use node.js ${{ matrix.node-version }} - uses: actions/setup-node@v1 - with: - node-version: ${{ matrix.node-version }} - - name: yarn install, build, and test - working-directory: . - run: | - yarn install - yarn build - yarn test - env: - CI: true From d904fd5b3df939108f11e264489d2c1a160cf931 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Thu, 28 Jan 2021 15:44:36 +0100 Subject: [PATCH 16/19] Fix changeset wording --- .changeset/wicked-beds-buy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/wicked-beds-buy.md b/.changeset/wicked-beds-buy.md index 2f0586bf93..daa8fffeab 100644 --- a/.changeset/wicked-beds-buy.md +++ b/.changeset/wicked-beds-buy.md @@ -5,6 +5,6 @@ The scaffolder is updated to generate a unique workspace directory inside the temp folder which gets cleaned up afterwards. prepare/template/publish steps is refactored to operate on known directories(`checkout/`, `template/`, `result/`) inside the generated temp directory. -Updates preparers to take the template url instead of the entire template. This is all in preparation f +Updates preparers to take the template url instead of the entire template. This is done primarly to allow for backwards compatability between v1 and v2 scaffolder templates. Fix broken configuration GitHub actions in Create React App template. From 359ebd6814d47cfd2d7970925d211d03252e2613 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Thu, 28 Jan 2021 16:00:21 +0100 Subject: [PATCH 17/19] fix typos --- .changeset/wicked-beds-buy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/wicked-beds-buy.md b/.changeset/wicked-beds-buy.md index daa8fffeab..3218269ff0 100644 --- a/.changeset/wicked-beds-buy.md +++ b/.changeset/wicked-beds-buy.md @@ -5,6 +5,6 @@ The scaffolder is updated to generate a unique workspace directory inside the temp folder which gets cleaned up afterwards. prepare/template/publish steps is refactored to operate on known directories(`checkout/`, `template/`, `result/`) inside the generated temp directory. -Updates preparers to take the template url instead of the entire template. This is done primarly to allow for backwards compatability between v1 and v2 scaffolder templates. +Updates preparers to take the template url instead of the entire template. This is done primarily to allow for backwards compatibility between v1 and v2 scaffolder templates. Fix broken configuration GitHub actions in Create React App template. From ea890cde8dda386f0246521b109f1e34737cbe00 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Thu, 28 Jan 2021 16:01:23 +0100 Subject: [PATCH 18/19] Fix router tests --- plugins/scaffolder-backend/src/service/router.test.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index 59d5f4696e..31f62febd2 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -23,6 +23,8 @@ jest.doMock('fs-extra', () => ({ F_OK: 0, W_OK: 1, }, + mkdir: jest.fn(), + remove: jest.fn(), })); import { getVoidLogger } from '@backstage/backend-common'; @@ -116,9 +118,10 @@ describe('createRouter - working directory', () => { }, }); - expect(mockPrepare).toBeCalledWith(expect.anything(), { + expect(mockPrepare).toBeCalledWith({ logger: expect.anything(), - workingDirectory: '/path', + workspacePath: expect.stringContaining('path'), + url: expect.anything(), }); }); @@ -143,8 +146,10 @@ describe('createRouter - working directory', () => { }, }); - expect(mockPrepare).toBeCalledWith(expect.anything(), { + expect(mockPrepare).toBeCalledWith({ logger: expect.anything(), + workspacePath: expect.anything(), + url: expect.anything(), }); }); }); From 1236f229091ff88b09d391ede1f63b6f111e2501 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 29 Jan 2021 10:51:28 +0100 Subject: [PATCH 19/19] Update changeset --- .changeset/wicked-beds-buy.md | 59 ++++++++++++++++++++++++++++++++--- 1 file changed, 55 insertions(+), 4 deletions(-) diff --git a/.changeset/wicked-beds-buy.md b/.changeset/wicked-beds-buy.md index 3218269ff0..ddcf4f880e 100644 --- a/.changeset/wicked-beds-buy.md +++ b/.changeset/wicked-beds-buy.md @@ -2,9 +2,60 @@ '@backstage/plugin-scaffolder-backend': minor --- -The scaffolder is updated to generate a unique workspace directory inside the temp folder which gets cleaned up afterwards. +The scaffolder is updated to generate a unique workspace directory inside the temp folder. This directory is cleaned up by the job processor after each run. -prepare/template/publish steps is refactored to operate on known directories(`checkout/`, `template/`, `result/`) inside the generated temp directory. -Updates preparers to take the template url instead of the entire template. This is done primarily to allow for backwards compatibility between v1 and v2 scaffolder templates. +The prepare/template/publish steps have been refactored to operate on known directories, `template/` and `result/`, inside the temporary workspace path. -Fix broken configuration GitHub actions in Create React App template. +Updated preparers to accept the template url instead of the entire template. This is done primarily to allow for backwards compatibility between v1 and v2 scaffolder templates. + +Fixes broken GitHub actions templating in the Create React App template. + +#### For those with **custom** preparers, templates, or publishers + +The preparer interface has changed, the prepare method now only takes a single argument, and doesn't return anything. As part of this change the preparers were refactored to accept a URL pointing to the target directory, rather than computing that from the template entity. + +The `workingDirectory` option was also removed, and replaced with a `workspacePath` option. The difference between the two is that `workingDirectory` was a place for the preparer to create temporary directories, while the `workspacePath` is the specific folder were the entire templating process for a single template job takes place. Instead of returning a path to the folder were the prepared contents were placed, the contents are put at the `/template` path. + +```diff +type PreparerOptions = { +- workingDirectory?: string; ++ /** ++ * Full URL to the directory containg template data ++ */ ++ url: string; ++ /** ++ * The workspace path that will eventually be the the root of the new repo ++ */ ++ workspacePath: string; + logger: Logger; +}; + +-prepare(template: TemplateEntityV1alpha1, opts?: PreparerOptions): Promise ++prepare(opts: PreparerOptions): Promise; +``` + +Instead of returning a path to the folder were the templaters contents were placed, the contents are put at the `/result` path. All templaters now also expect the source template to be present in the `template` directory within the `workspacePath`. + +```diff +export type TemplaterRunOptions = { +- directory: string; ++ workspacePath: string; + values: TemplaterValues; + logStream?: Writable; + dockerClient: Docker; +}; + +-public async run(options: TemplaterRunOptions): Promise ++public async run(options: TemplaterRunOptions): Promise +``` + +Just like the preparer and templaters, the publishers have also switched to using `workspacePath`. The root of the new repo is expected to be located at `/result`. + +```diff +export type PublisherOptions = { + values: TemplaterValues; +- directory: string; ++ workspacePath: string; + logger: Logger; +}; +```