From f7f20ba5fb436964137fb6a1ba3b82817c304b6e Mon Sep 17 00:00:00 2001 From: Aramis Sennyey <159921952+aramissennyeydd@users.noreply.github.com> Date: Thu, 4 Apr 2024 18:09:08 -0400 Subject: [PATCH 1/6] fix: allow reserved characters in all parameters Signed-off-by: Aramis Sennyey <159921952+aramissennyeydd@users.noreply.github.com> --- .../src/commands/repo/schema/openapi/lint.ts | 15 +++++++++++++-- packages/repo-tools/src/lib/runner.ts | 1 + .../src/schema/openapi.generated.ts | 7 ++++++- plugins/search-backend/src/schema/openapi.yaml | 5 +++++ plugins/search-backend/src/service/router.test.ts | 3 ++- plugins/todo-backend/src/schema/openapi.yaml | 3 +++ 6 files changed, 30 insertions(+), 4 deletions(-) diff --git a/packages/repo-tools/src/commands/repo/schema/openapi/lint.ts b/packages/repo-tools/src/commands/repo/schema/openapi/lint.ts index eacb3ac50d..d19a76af0f 100644 --- a/packages/repo-tools/src/commands/repo/schema/openapi/lint.ts +++ b/packages/repo-tools/src/commands/repo/schema/openapi/lint.ts @@ -26,6 +26,7 @@ import fs from 'fs-extra'; import chalk from 'chalk'; import { runner } from '../../../../lib/runner'; import { oas } from '@stoplight/spectral-rulesets'; +import { truthy } from '@stoplight/spectral-functions'; import { DiagnosticSeverity } from '@stoplight/types'; import { pretty } from '@stoplight/spectral-formatters'; import { getPathToOpenApiSpec } from '../../../../lib/openapi/helpers'; @@ -35,7 +36,7 @@ async function lint(directoryPath: string, config?: { strict: boolean }) { let openapiPath = ''; try { openapiPath = await getPathToOpenApiSpec(directoryPath); - } catch { + } catch (err) { return; } const openapiFileContent = await fs.readFile(openapiPath, 'utf8'); @@ -47,6 +48,16 @@ async function lint(directoryPath: string, config?: { strict: boolean }) { const backstageRuleset = new Ruleset( { extends: [oas, ruleset], + rules: { + 'allow-reserved-in-params': { + given: '$.paths..parameters[*]', + then: { + field: 'allowReserved', + function: truthy, + }, + severity: 'error', + }, + }, overrides: [ { files: ['*'], @@ -63,8 +74,8 @@ async function lint(directoryPath: string, config?: { strict: boolean }) { } as RulesetDefinition, { source: openapiPath }, ); - spectral.setRuleset(backstageRuleset); + // we lint our document using the ruleset we passed to the Spectral object const result = await spectral.run(document); const errors = result.filter(e => e.severity === DiagnosticSeverity.Error); diff --git a/packages/repo-tools/src/lib/runner.ts b/packages/repo-tools/src/lib/runner.ts index b9700cd09b..1e7867df3d 100644 --- a/packages/repo-tools/src/lib/runner.ts +++ b/packages/repo-tools/src/lib/runner.ts @@ -37,6 +37,7 @@ export async function runner( port: options.startingPort, stopPort: options.startingPort + 1_000, })); + console.log(packages); const resultsList = await Promise.all( packages.map(pkg => limit(async () => { diff --git a/plugins/search-backend/src/schema/openapi.generated.ts b/plugins/search-backend/src/schema/openapi.generated.ts index 06b576ba70..8ad27546d6 100644 --- a/plugins/search-backend/src/schema/openapi.generated.ts +++ b/plugins/search-backend/src/schema/openapi.generated.ts @@ -1,5 +1,5 @@ /* - * Copyright 2023 The Backstage Authors + * Copyright 2024 The Backstage Authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -197,6 +197,7 @@ export const spec = { name: 'term', in: 'query', required: false, + allowReserved: true, schema: { type: 'string', default: '', @@ -208,6 +209,7 @@ export const spec = { required: false, style: 'deepObject', explode: true, + allowReserved: true, schema: { $ref: '#/components/schemas/JsonObject', }, @@ -216,6 +218,7 @@ export const spec = { name: 'types', in: 'query', required: false, + allowReserved: true, schema: { type: 'array', items: { @@ -227,6 +230,7 @@ export const spec = { name: 'pageCursor', in: 'query', required: false, + allowReserved: true, schema: { type: 'string', }, @@ -235,6 +239,7 @@ export const spec = { name: 'pageLimit', in: 'query', required: false, + allowReserved: true, schema: { type: 'integer', }, diff --git a/plugins/search-backend/src/schema/openapi.yaml b/plugins/search-backend/src/schema/openapi.yaml index 47f7e0a1e5..358f94fcf6 100644 --- a/plugins/search-backend/src/schema/openapi.yaml +++ b/plugins/search-backend/src/schema/openapi.yaml @@ -132,6 +132,7 @@ paths: - name: term in: query required: false + allowReserved: true schema: type: string default: '' @@ -140,6 +141,7 @@ paths: required: false style: deepObject explode: true + allowReserved: true schema: # JsonObject is used here instead of the full ZOD schema definition as # resolution of recursive schemas isn't possible with the library we're using @@ -148,6 +150,7 @@ paths: - name: types in: query required: false + allowReserved: true schema: type: array items: @@ -155,10 +158,12 @@ paths: - name: pageCursor in: query required: false + allowReserved: true schema: type: string - name: pageLimit in: query required: false + allowReserved: true schema: type: integer diff --git a/plugins/search-backend/src/service/router.test.ts b/plugins/search-backend/src/service/router.test.ts index 7c89a8c750..59f2f95afe 100644 --- a/plugins/search-backend/src/service/router.test.ts +++ b/plugins/search-backend/src/service/router.test.ts @@ -99,7 +99,6 @@ describe('createRouter', () => { mockSearchEngine.query.mockRejectedValueOnce(error); const response = await request(app).get('/query'); - console.log((response as any).text); expect(response.status).toEqual(500); expect(response.body).toMatchObject( @@ -127,6 +126,8 @@ describe('createRouter', () => { 'types[0]=first-type&types[1]=second-type', 'filters[prop]=value', 'pageCursor=foo', + // https://github.com/backstage/backstage/issues/23973 + 'term=foo+bar', ])('accepts valid query string "%s"', async queryString => { const response = await request(app).get(`/query?${queryString}`); diff --git a/plugins/todo-backend/src/schema/openapi.yaml b/plugins/todo-backend/src/schema/openapi.yaml index 7408927b99..98f92103fb 100644 --- a/plugins/todo-backend/src/schema/openapi.yaml +++ b/plugins/todo-backend/src/schema/openapi.yaml @@ -104,6 +104,7 @@ paths: - name: orderBy in: query required: false + allowReserved: true schema: type: string pattern: '^(text|tag|author|viewUrl|repoFilePath)=(asc|desc)$' @@ -121,6 +122,7 @@ paths: - name: offset in: query required: false + allowReserved: true schema: type: integer minimum: 0 @@ -128,6 +130,7 @@ paths: - name: limit in: query required: false + allowReserved: true schema: type: integer minimum: 1 From 2bd291e605315db865b5b04496e7c08a24565542 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey <159921952+aramissennyeydd@users.noreply.github.com> Date: Thu, 4 Apr 2024 18:11:27 -0400 Subject: [PATCH 2/6] add changesets Signed-off-by: Aramis Sennyey <159921952+aramissennyeydd@users.noreply.github.com> --- .changeset/chatty-crabs-tap.md | 5 +++++ .changeset/lucky-comics-hang.md | 6 ++++++ 2 files changed, 11 insertions(+) create mode 100644 .changeset/chatty-crabs-tap.md create mode 100644 .changeset/lucky-comics-hang.md diff --git a/.changeset/chatty-crabs-tap.md b/.changeset/chatty-crabs-tap.md new file mode 100644 index 0000000000..88871f817a --- /dev/null +++ b/.changeset/chatty-crabs-tap.md @@ -0,0 +1,5 @@ +--- +'@backstage/repo-tools': minor +--- + +Adds a lint rule to `repo schema openapi lint` to enforce `allowReserved: true`. diff --git a/.changeset/lucky-comics-hang.md b/.changeset/lucky-comics-hang.md new file mode 100644 index 0000000000..0174676d14 --- /dev/null +++ b/.changeset/lucky-comics-hang.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-search-backend': patch +'@backstage/plugin-todo-backend': patch +--- + +Allow reserved characters in requests. From f432d472d8252e7807cd1d433c05da9bbb0dfcfc Mon Sep 17 00:00:00 2001 From: Aramis Sennyey <159921952+aramissennyeydd@users.noreply.github.com> Date: Thu, 4 Apr 2024 18:13:41 -0400 Subject: [PATCH 3/6] unnecessary err var Signed-off-by: Aramis Sennyey <159921952+aramissennyeydd@users.noreply.github.com> --- packages/repo-tools/src/commands/repo/schema/openapi/lint.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/repo-tools/src/commands/repo/schema/openapi/lint.ts b/packages/repo-tools/src/commands/repo/schema/openapi/lint.ts index d19a76af0f..515d1d68ed 100644 --- a/packages/repo-tools/src/commands/repo/schema/openapi/lint.ts +++ b/packages/repo-tools/src/commands/repo/schema/openapi/lint.ts @@ -36,7 +36,7 @@ async function lint(directoryPath: string, config?: { strict: boolean }) { let openapiPath = ''; try { openapiPath = await getPathToOpenApiSpec(directoryPath); - } catch (err) { + } catch { return; } const openapiFileContent = await fs.readFile(openapiPath, 'utf8'); From 5bcc798eafd6d703a7dd90e9889d25be12ba7a3d Mon Sep 17 00:00:00 2001 From: web-next-automation Date: Mon, 8 Apr 2024 20:34:38 -0400 Subject: [PATCH 4/6] remove debug statement Signed-off-by: web-next-automation --- packages/repo-tools/src/lib/runner.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/repo-tools/src/lib/runner.ts b/packages/repo-tools/src/lib/runner.ts index 1e7867df3d..ea7b511938 100644 --- a/packages/repo-tools/src/lib/runner.ts +++ b/packages/repo-tools/src/lib/runner.ts @@ -37,7 +37,7 @@ export async function runner( port: options.startingPort, stopPort: options.startingPort + 1_000, })); - console.log(packages); + const resultsList = await Promise.all( packages.map(pkg => limit(async () => { From ec2b541b7d5270634a695725799dd2936a039ab9 Mon Sep 17 00:00:00 2001 From: web-next-automation Date: Mon, 8 Apr 2024 20:39:38 -0400 Subject: [PATCH 5/6] fix mismatch Signed-off-by: web-next-automation --- plugins/todo-backend/src/schema/openapi.generated.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/todo-backend/src/schema/openapi.generated.ts b/plugins/todo-backend/src/schema/openapi.generated.ts index 238be2fe5b..bbd9f1efb0 100644 --- a/plugins/todo-backend/src/schema/openapi.generated.ts +++ b/plugins/todo-backend/src/schema/openapi.generated.ts @@ -1,5 +1,5 @@ /* - * Copyright 2023 The Backstage Authors + * Copyright 2024 The Backstage Authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -162,6 +162,7 @@ export const spec = { name: 'orderBy', in: 'query', required: false, + allowReserved: true, schema: { type: 'string', pattern: '^(text|tag|author|viewUrl|repoFilePath)=(asc|desc)$', @@ -188,6 +189,7 @@ export const spec = { name: 'offset', in: 'query', required: false, + allowReserved: true, schema: { type: 'integer', minimum: 0, @@ -198,6 +200,7 @@ export const spec = { name: 'limit', in: 'query', required: false, + allowReserved: true, schema: { type: 'integer', minimum: 1, From 3956f3d7a6df3d2cb463b1c3646fe398786a0f2c Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Thu, 11 Apr 2024 09:21:54 -0400 Subject: [PATCH 6/6] fix changeset message Signed-off-by: aramissennyeydd --- .changeset/chatty-crabs-tap.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/.changeset/chatty-crabs-tap.md b/.changeset/chatty-crabs-tap.md index 88871f817a..97ec05fb61 100644 --- a/.changeset/chatty-crabs-tap.md +++ b/.changeset/chatty-crabs-tap.md @@ -2,4 +2,17 @@ '@backstage/repo-tools': minor --- -Adds a lint rule to `repo schema openapi lint` to enforce `allowReserved: true`. +Adds a lint rule to `repo schema openapi lint` to enforce `allowReserved` for all parameters. To fix this, simply add `allowReserved: true` to your parameters, like so + +```diff +/v1/todos: + get: + operationId: ListTodos + # ... + parameters: + - name: entity + in: query ++ allowReserved: true + schema: + type: string +```