From c1a7704ad1370dd1e9288be961bcbc61692c54a1 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 19 May 2021 20:08:47 +0200 Subject: [PATCH 1/8] chore: add deprecation warning when using v1 templates Signed-off-by: blam --- .../scaffolder-backend/src/service/router.ts | 133 +----------------- 1 file changed, 4 insertions(+), 129 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 5e0790526e..0a38517c4a 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -137,134 +137,6 @@ export async function createRouter( worker.start(); - router - .get('/v1/job/:jobId', ({ params }, res) => { - const job = jobProcessor.get(params.jobId); - - if (!job) { - res.status(404).json({ error: 'job not found' }); - return; - } - - res.json({ - id: job.id, - metadata: { - ...job.context, - logger: undefined, - logStream: undefined, - }, - status: job.status, - stages: job.stages.map(stage => ({ - ...stage, - handler: undefined, - })), - error: job.error, - }); - }) - .post('/v1/jobs', async (req, res) => { - const templateName: string = req.body.templateName; - const values: TemplaterValues = { - ...req.body.values, - destination: { - git: parseGitUrl(req.body.values.storePath), - }, - }; - - // Forward authorization from client - const template = await entityClient.findTemplate(templateName, { - token: getBearerToken(req.headers.authorization), - }); - if (!isAlpha1Template(template)) { - throw new InputError( - `This endpoint does not support templates with version ${template.apiVersion}`, - ); - } - - const validationResult: ValidatorResult = validate( - values, - template.spec.schema, - ); - - if (!validationResult.valid) { - res.status(400).json({ errors: validationResult.errors }); - return; - } - - const job = jobProcessor.create({ - entity: template, - values, - stages: [ - { - name: 'Prepare the skeleton', - async handler(ctx) { - const { - protocol, - location: templateEntityLocation, - } = parseLocationAnnotation(ctx.entity); - - if (protocol === 'file') { - const preparer = new FilePreparer(); - - const path = resolvePath( - dirname(templateEntityLocation), - template.spec.path || '.', - ); - - await preparer.prepare({ - url: `file://${path}`, - logger: ctx.logger, - workspacePath: ctx.workspacePath, - }); - return; - } - - const preparer = preparers.get(templateEntityLocation); - - const url = joinGitUrlPath( - templateEntityLocation, - template.spec.path, - ); - - await preparer.prepare({ - url, - logger: ctx.logger, - workspacePath: ctx.workspacePath, - }); - }, - }, - { - name: 'Run the templater', - async handler(ctx) { - const templater = templaters.get(ctx.entity.spec.templater); - await templater.run({ - workspacePath: ctx.workspacePath, - logStream: ctx.logStream, - values: ctx.values, - }); - }, - }, - { - name: 'Publish template', - 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, - workspacePath: ctx.workspacePath, - logger: ctx.logger, - }); - return result; - }, - }, - ], - }); - - jobProcessor.run(job); - - res.status(201).json({ id: job.id }); - }); - - // NOTE: The v2 API is unstable router .get( '/v2/templates/:namespace/:kind/:name/parameter-schema', @@ -359,8 +231,11 @@ export async function createRouter( let taskSpec; if (isAlpha1Template(template)) { - const result = validate(values, template.spec.schema); + logger.warn( + `[DEPRECATION] - Template: ${template.metadata.name} has version ${template.apiVersion} which is going to be deprecated. Please refer to https://backstage.io/docs/features/software-templates/migrating-from-v1alpha1-to-v1beta2 for help on migrating`, + ); + const result = validate(values, template.spec.schema); if (!result.valid) { res.status(400).json({ errors: result.errors }); return; From 5f53dd2c38fcf826ff16d695c0d68a0fb0a51a62 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 19 May 2021 20:37:16 +0200 Subject: [PATCH 2/8] chore: added a UI deprecation too for the templates Signed-off-by: blam --- .../ScaffolderPage/ScaffolderPage.tsx | 5 +- .../components/TemplateCard/TemplateCard.tsx | 64 ++++++++++++++++++- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder/src/components/ScaffolderPage/ScaffolderPage.tsx b/plugins/scaffolder/src/components/ScaffolderPage/ScaffolderPage.tsx index 4a482402f8..bc1bcefe1e 100644 --- a/plugins/scaffolder/src/components/ScaffolderPage/ScaffolderPage.tsx +++ b/plugins/scaffolder/src/components/ScaffolderPage/ScaffolderPage.tsx @@ -175,7 +175,10 @@ export const ScaffolderPageContents = () => { {matchingEntities && matchingEntities?.length > 0 && matchingEntities.map(template => ( - + ))} diff --git a/plugins/scaffolder/src/components/TemplateCard/TemplateCard.tsx b/plugins/scaffolder/src/components/TemplateCard/TemplateCard.tsx index 8c64d981ad..b529780842 100644 --- a/plugins/scaffolder/src/components/TemplateCard/TemplateCard.tsx +++ b/plugins/scaffolder/src/components/TemplateCard/TemplateCard.tsx @@ -22,10 +22,14 @@ import { CardContent, CardMedia, Chip, + Link, makeStyles, + Popover, + Typography, useTheme, } from '@material-ui/core'; import React from 'react'; +import WarningIcon from '@material-ui/icons/Warning'; import { generatePath } from 'react-router'; import { rootRouteRef } from '../../routes'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; @@ -47,8 +51,18 @@ const useStyles = makeStyles({ }, }); +const useDeprecationStyles = makeStyles(theme => ({ + deprecationIcon: { + position: 'absolute', + top: theme.spacing(0.5), + right: theme.spacing(3.5), + padding: '0.25rem', + }, +})); + export type TemplateCardProps = { template: TemplateEntityV1alpha1; + deprecated?: boolean; }; type TemplateProps = { @@ -72,7 +86,54 @@ const getTemplateCardProps = ( }; }; -export const TemplateCard = ({ template }: TemplateCardProps) => { +const DeprecationWarning = () => { + const [anchorEl, setAnchorEl] = React.useState(null); + + const handleClick = (event: React.MouseEvent) => { + setAnchorEl(event.currentTarget); + }; + + const styles = useDeprecationStyles(); + + const handleClose = () => { + setAnchorEl(null); + }; + + const open = Boolean(anchorEl); + + const Title = ( + + This template syntax is deprecated. Please follow the documentation{' '} + + here + {' '} + to migrate + + ); + + return ( +
+ + + {Title} + +
+ ); +}; + +export const TemplateCard = ({ template, deprecated }: TemplateCardProps) => { const backstageTheme = useTheme(); const rootLink = useRouteRef(rootRouteRef); const templateProps = getTemplateCardProps(template); @@ -88,6 +149,7 @@ export const TemplateCard = ({ template }: TemplateCardProps) => { + {deprecated && } Date: Wed, 19 May 2021 20:42:39 +0200 Subject: [PATCH 3/8] chore: added changeset Signed-off-by: blam --- .changeset/silent-needles-attack.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/silent-needles-attack.md diff --git a/.changeset/silent-needles-attack.md b/.changeset/silent-needles-attack.md new file mode 100644 index 0000000000..7ead2b674e --- /dev/null +++ b/.changeset/silent-needles-attack.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-scaffolder': patch +'@backstage/plugin-scaffolder-backend': patch +--- + +Added deprecation warnings for `v1alpha1` templates From 3c83ed9e2180689eba42f60a7fd574a7f9091684 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 19 May 2021 20:44:52 +0200 Subject: [PATCH 4/8] chore: fixing typescript Signed-off-by: blam --- plugins/scaffolder-backend/src/service/router.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 0a38517c4a..2cdfd2f76c 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -16,7 +16,6 @@ import { Config } from '@backstage/config'; import express from 'express'; -import { resolve as resolvePath, dirname } from 'path'; import Router from 'express-promise-router'; import { Logger } from 'winston'; import { @@ -25,13 +24,9 @@ import { TemplaterBuilder, TemplaterValues, PublisherBuilder, - parseLocationAnnotation, - joinGitUrlPath, - FilePreparer, } from '../scaffolder'; import { CatalogEntityClient } from '../lib/catalog'; -import { validate, ValidatorResult } from 'jsonschema'; -import parseGitUrl from 'git-url-parse'; +import { validate } from 'jsonschema'; import { DatabaseTaskStore, StorageTaskBroker, @@ -101,7 +96,6 @@ export async function createRouter( const logger = parentLogger.child({ plugin: 'scaffolder' }); const workingDirectory = await getWorkingDirectory(config, logger); - const jobProcessor = await JobProcessor.fromConfig({ config, logger }); const entityClient = new CatalogEntityClient(catalogClient); const integrations = ScmIntegrations.fromConfig(config); From b920de280e37581c88318bcdce0364365178cb27 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 19 May 2021 20:51:04 +0200 Subject: [PATCH 5/8] chore: fixing typescript for reals this time.. Signed-off-by: blam --- plugins/scaffolder-backend/src/service/router.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 2cdfd2f76c..dc8e69eb46 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -19,7 +19,6 @@ import express from 'express'; import Router from 'express-promise-router'; import { Logger } from 'winston'; import { - JobProcessor, PreparerBuilder, TemplaterBuilder, TemplaterValues, From 7cdad6e430d2b2a795eab432cef6bbf96bbc6c8b Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 19 May 2021 21:15:57 +0200 Subject: [PATCH 6/8] chore: removing scaffolder tests for v1 Signed-off-by: blam --- .../src/service/router.test.ts | 129 ------------------ 1 file changed, 129 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index e8aa76a63a..9ca2eef9e6 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -64,120 +64,6 @@ const mockUrlReader = UrlReaders.default({ config: new ConfigReader({}), }); -describe('createRouter - working directory', () => { - const mockPrepare = jest.fn(); - const mockPreparers = new Preparers(); - - beforeAll(() => { - const mockPreparer = { - prepare: mockPrepare, - }; - mockPreparers.register('dev.azure.com', mockPreparer); - }); - - beforeEach(() => { - jest.resetAllMocks(); - }); - - const workDirConfig = (path: string) => ({ - backend: { - workingDirectory: path, - }, - }); - - const template = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Template', - metadata: { - annotations: { - 'backstage.io/managed-by-location': 'url:https://dev.azure.com', - }, - }, - spec: { - owner: 'template@backstage.io', - path: '.', - schema: {}, - }, - }; - - it('should throw an error when working directory does not exist or is not writable', async () => { - mockAccess.mockImplementation(() => { - throw new Error('access error'); - }); - - await expect( - createRouter({ - logger: getVoidLogger(), - preparers: new Preparers(), - templaters: new Templaters(), - publishers: new Publishers(), - config: new ConfigReader(workDirConfig('/path')), - database: createDatabase(), - catalogClient: createCatalogClient([template]), - reader: mockUrlReader, - }), - ).rejects.toThrow('access error'); - }); - - it('should use the working directory when configured', async () => { - const router = await createRouter({ - logger: getVoidLogger(), - preparers: mockPreparers, - templaters: new Templaters(), - publishers: new Publishers(), - config: new ConfigReader(workDirConfig('/path')), - database: createDatabase(), - catalogClient: createCatalogClient([template]), - reader: mockUrlReader, - }); - - const app = express().use(router); - await request(app) - .post('/v1/jobs') - .send({ - templateName: '', - values: { - storePath: 'https://github.com/backstage/good', - }, - }); - - expect(mockPrepare).toBeCalledWith({ - logger: expect.anything(), - workspacePath: expect.stringContaining('path'), - url: expect.anything(), - }); - }); - - it('should not pass along anything when no working directory is configured', async () => { - const router = await createRouter({ - logger: getVoidLogger(), - preparers: mockPreparers, - templaters: new Templaters(), - publishers: new Publishers(), - config: new ConfigReader({}), - database: createDatabase(), - catalogClient: createCatalogClient([template]), - reader: mockUrlReader, - }); - - const app = express().use(router); - await request(app) - .post('/v1/jobs') - .send({ - templateName: '', - values: { - storePath: 'https://github.com/backstage/goodrepo', - }, - }); - - expect(mockPrepare).toBeCalledWith({ - logger: expect.anything(), - workspacePath: expect.anything(), - url: expect.anything(), - }); - }); -}); - describe('createRouter', () => { let app: express.Express; const template = { @@ -239,21 +125,6 @@ describe('createRouter', () => { jest.resetAllMocks(); }); - describe('POST /v1/jobs', () => { - it('rejects template values which do not match the template schema definition', async () => { - const response = await request(app) - .post('/v1/jobs') - .send({ - templateName: '', - values: { - storePath: 'https://github.com/backstage/backstage', - }, - }); - - expect(response.status).toEqual(400); - }); - }); - describe('GET /v2/actions', () => { it('lists available actions', async () => { const response = await request(app).get('/v2/actions').send(); From 9e313e56524debf0db9b3c03a762b39ec3a84542 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 20 May 2021 09:11:42 +0200 Subject: [PATCH 7/8] chore: fixing wording for deprecation notice Signed-off-by: blam --- .../src/components/TemplateCard/TemplateCard.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/scaffolder/src/components/TemplateCard/TemplateCard.tsx b/plugins/scaffolder/src/components/TemplateCard/TemplateCard.tsx index b529780842..18b7679000 100644 --- a/plugins/scaffolder/src/components/TemplateCard/TemplateCard.tsx +++ b/plugins/scaffolder/src/components/TemplateCard/TemplateCard.tsx @@ -103,11 +103,11 @@ const DeprecationWarning = () => { const Title = ( - This template syntax is deprecated. Please follow the documentation{' '} + This template syntax is deprecated. +
- here - {' '} - to migrate + Please follow the documentation to migrate + ); From 3117288b7bf6ad7594cc734a658903631459ce74 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 20 May 2021 10:33:22 +0200 Subject: [PATCH 8/8] chore: fix formatting and use tooltip instead Signed-off-by: blam --- .../components/TemplateCard/TemplateCard.tsx | 47 +++++-------------- 1 file changed, 13 insertions(+), 34 deletions(-) diff --git a/plugins/scaffolder/src/components/TemplateCard/TemplateCard.tsx b/plugins/scaffolder/src/components/TemplateCard/TemplateCard.tsx index 18b7679000..7bdaa0a671 100644 --- a/plugins/scaffolder/src/components/TemplateCard/TemplateCard.tsx +++ b/plugins/scaffolder/src/components/TemplateCard/TemplateCard.tsx @@ -24,7 +24,7 @@ import { Chip, Link, makeStyles, - Popover, + Tooltip, Typography, useTheme, } from '@material-ui/core'; @@ -58,6 +58,9 @@ const useDeprecationStyles = makeStyles(theme => ({ right: theme.spacing(3.5), padding: '0.25rem', }, + link: { + color: theme.palette.warning.light, + }, })); export type TemplateCardProps = { @@ -87,48 +90,24 @@ const getTemplateCardProps = ( }; const DeprecationWarning = () => { - const [anchorEl, setAnchorEl] = React.useState(null); - - const handleClick = (event: React.MouseEvent) => { - setAnchorEl(event.currentTarget); - }; - const styles = useDeprecationStyles(); - const handleClose = () => { - setAnchorEl(null); - }; - - const open = Boolean(anchorEl); - const Title = ( - This template syntax is deprecated. -
- - Please follow the documentation to migrate - + This template syntax is deprecated. Click for more info. ); return (
- - - {Title} - + + + + +
); };