From 1323cc552ac78a5354333bed97d466ca519602d8 Mon Sep 17 00:00:00 2001 From: Renan de Andrade Date: Wed, 18 Oct 2023 15:23:52 -0300 Subject: [PATCH 1/3] ref: use request body instead of query string at /functions/pipeline --- lib/domain/schemas.js | 35 +++++++++++++++++++ lib/http/routers/FunctionsRouter.js | 24 ++++++------- .../unit/http/routers/FunctionsRouter.test.js | 29 ++++++++++++--- 3 files changed, 72 insertions(+), 16 deletions(-) diff --git a/lib/domain/schemas.js b/lib/domain/schemas.js index 220a797..e4a69e9 100644 --- a/lib/domain/schemas.js +++ b/lib/domain/schemas.js @@ -173,6 +173,40 @@ const functionItem = { links: functionsItemLinks.concat(functionLinks), }; +const functionPipeline = { + $schema: 'http://json-schema.org/draft-04/hyper-schema#', + type: 'object', + title: 'Pipeline', + properties: { + steps: { + type: 'array', + title: 'Steps', + readOnly: true, + items:{ + type: 'object', + properties: { + namespace: { + type: 'string', + title: 'Namespace', + readOnly: true, + }, + id: { + type: 'string', + title: 'ID', + readOnly: true, + }, + } + } + }, + payload: { + type: 'object', + title: 'Payload', + readOnly: true, + }, + }, + required: ['steps'], +}; + const functionEnv = { $schema: 'http://json-schema.org/draft-04/hyper-schema#', type: 'string', @@ -202,4 +236,5 @@ exports.root = root; exports['functions/item'] = functionItem; exports['functions/env'] = functionEnv; exports['functions/list'] = functions; +exports['functions/pipeline'] = functionPipeline; exports['namespaces/item'] = namespaceItem; diff --git a/lib/http/routers/FunctionsRouter.js b/lib/http/routers/FunctionsRouter.js index b3680cb..499875d 100644 --- a/lib/http/routers/FunctionsRouter.js +++ b/lib/http/routers/FunctionsRouter.js @@ -291,26 +291,26 @@ router.all('/:namespace/:id/run', bodyParser.json({ limit: bodyParserLimit }), a router.put('/pipeline', bodyParser.json({ limit: bodyParserLimit }), async (req, res, next) => { const memoryStorage = req.app.get('memoryStorage'); const sandbox = req.app.get('sandbox'); + const validationResult = new Validator().validate(req.body, schemas['functions/pipeline']); - let { steps } = req.query; + if (!validationResult.valid) { + const error = 'Invalid pipeline configuration'; + const details = validationResult.errors.map(e => e.toString()); + + res.status(400).json({ error, details }); + return; + } + + let steps = req.body.steps; + req.body = req.body.payload; const span = req.span.tracer().startSpan('run pipeline', { childOf: req.span, tags: { - 'function.steps': Array.isArray(steps) ? steps.join(', ') : '', + 'function.steps': Array.isArray(steps) ? JSON.stringify(steps) : '', }, }); - if (!steps) { - const err = new Error('Pass step by querystring is required'); - reportError(span, err); - res.status(400).json({ error: err.message }); - return; - } - steps = steps.map((step) => { - const [namespace, id] = step.split('/', 2); - return { namespace, id }; - }); const metric = new Metric('pipeline-run'); diff --git a/test/unit/http/routers/FunctionsRouter.test.js b/test/unit/http/routers/FunctionsRouter.test.js index a7b4335..eb03df4 100644 --- a/test/unit/http/routers/FunctionsRouter.test.js +++ b/test/unit/http/routers/FunctionsRouter.test.js @@ -313,7 +313,8 @@ describe('PUT /functions/pipeline', () => { request(routes) .put('/functions/pipeline') .expect(400, { - error: 'Pass step by querystring is required', + error: 'Invalid pipeline configuration', + details: [ 'instance requires property "steps"' ], }, done); }); }); @@ -321,7 +322,13 @@ describe('PUT /functions/pipeline', () => { describe('when step does not exists', () => { it('should return a not found request', (done) => { request(routes) - .put('/functions/pipeline?steps[0]=backstage/not-found') + .put('/functions/pipeline') + .send({ steps: [ + { + namespace: 'backstage', + id: 'not-found', + }, + ] }) .expect(404, { error: 'Code \'backstage/not-found\' is not found', }, done); @@ -331,8 +338,22 @@ describe('PUT /functions/pipeline', () => { describe('when step use two steps', () => { it('should return a result', (done) => { request(routes) - .put('/functions/pipeline?steps[0]=backstage/step1&steps[1]=backstage/step2') - .send({ x: 1 }) + .put('/functions/pipeline') + .send({ + steps: [ + { + namespace: 'backstage', + id: 'step1', + }, + { + namespace: 'backstage', + id: 'step2', + }, + ], + payload: { + x: 1, + }, + }) .expect(200, { x: 200 }, done); }); }); From 722ce8abd66dbfa5f78a43dc54afcb1454ae03ed Mon Sep 17 00:00:00 2001 From: Renan de Andrade Date: Wed, 18 Oct 2023 15:30:17 -0300 Subject: [PATCH 2/3] doc: update pipeline request example in README.md --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 417778e..9c4f5c1 100644 --- a/README.md +++ b/README.md @@ -140,10 +140,10 @@ function main(req, res) { } ``` -``` -curl -g -i -X PUT 'http://localhost:8100/functions/pipeline?steps[0]=namespace/function0&steps[1]=namespace/function1' \ - -H 'content-type: application/json' - -d '{"x": 1}' +``` bash +$ curl -g -i -X PUT 'http://localhost:8100/functions/pipeline' \ + -H 'content-type: application/json' \ + -d '{"steps": [{"namespace":"namespace", "id":"function0"}, {"namespace":"namespace", "id": "function1"}], "payload":{"x":1}}' ``` Considering the curl above, the pipeline result would be like this: From 6ce64b9aacc31f74ed7f151030a7df74b972d83b Mon Sep 17 00:00:00 2001 From: Renan de Andrade Date: Wed, 18 Oct 2023 15:35:40 -0300 Subject: [PATCH 3/3] chore: fix some lint warnings --- lib/domain/schemas.js | 6 +++--- lib/http/routers/FunctionsRouter.js | 4 ++-- test/unit/http/routers/FunctionsRouter.test.js | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/domain/schemas.js b/lib/domain/schemas.js index e4a69e9..08b1f04 100644 --- a/lib/domain/schemas.js +++ b/lib/domain/schemas.js @@ -182,7 +182,7 @@ const functionPipeline = { type: 'array', title: 'Steps', readOnly: true, - items:{ + items: { type: 'object', properties: { namespace: { @@ -195,8 +195,8 @@ const functionPipeline = { title: 'ID', readOnly: true, }, - } - } + }, + }, }, payload: { type: 'object', diff --git a/lib/http/routers/FunctionsRouter.js b/lib/http/routers/FunctionsRouter.js index 499875d..8970e7f 100644 --- a/lib/http/routers/FunctionsRouter.js +++ b/lib/http/routers/FunctionsRouter.js @@ -301,7 +301,7 @@ router.put('/pipeline', bodyParser.json({ limit: bodyParserLimit }), async (req, return; } - let steps = req.body.steps; + const steps = req.body.steps; req.body = req.body.payload; const span = req.span.tracer().startSpan('run pipeline', { @@ -342,7 +342,7 @@ router.put('/pipeline', bodyParser.json({ limit: bodyParserLimit }), async (req, res.json(result.body); } catch (err) { reportError(span, err); - RecordOtelError(err) + RecordOtelError(err); const status = err.statusCode || 500; metric.observePipelineRun(status); res.status(status).json({ error: err.message }); diff --git a/test/unit/http/routers/FunctionsRouter.test.js b/test/unit/http/routers/FunctionsRouter.test.js index eb03df4..ca05604 100644 --- a/test/unit/http/routers/FunctionsRouter.test.js +++ b/test/unit/http/routers/FunctionsRouter.test.js @@ -314,7 +314,7 @@ describe('PUT /functions/pipeline', () => { .put('/functions/pipeline') .expect(400, { error: 'Invalid pipeline configuration', - details: [ 'instance requires property "steps"' ], + details: ['instance requires property "steps"'], }, done); }); });