fix: change endpoints for create and remove tags

plugins/templates/README.md

@@ -27,26 +27,26 @@ server.register({
 
 ### Routes
 
-#### Get all templates
-`page` and `count` optional
+#### Template
+##### Get all templates
 
-`GET /templates?page={pageNumber}&count={countNumber}`
+`GET /templates`
 
-#### Get a single template
+##### Get a single template
 
 You can get a single template by providing the template name and the specific version or the tag.
 
 `GET /templates/{name}/{tag}` or `GET /templates/{name}/{version}`
 
-**Arguments**
+###### Arguments
 
 'name', 'tag' or 'version'
 
 * `name` - Name of the template
 * `tag` - Tag of the template (e.g. `stable`, `latest`, etc)
 * `version` - Version of the template
 
-#### Create a template
+##### Create a template
 Creating a template will store the template data (`config`, `name`, `version`, `description`, `maintainer`, `labels`) into the datastore.
 
 If the exact template and version already exist, the only thing that can be changed is `labels`.
@@ -57,7 +57,7 @@ If the template already exists but not the version, the new version will be stor
 
 `POST /templates`
 
-**Arguments**
+###### Arguments
 
 'name', 'version', 'description', 'maintainer', labels
 
@@ -84,25 +84,23 @@ Example payload:
 }
 ```
 
-#### Create/Update a tag for a template version
+#### Template Tag
+Template tag allows fetching on template version by tag. For example, tag `[email protected]` as `stable`. 
 
-Tagging a template version allows fetching on template version by tag. For example, tag `[email protected]` as `stable`. 
+##### Create/Update a tag
+
+If the template tag already exists, it will update the tag with the new version. If the template tag doesn't exist yet, this endpoint will create the tag.
 
 *Note: This endpoint is only accessible in `build` scope and the permission is tied to the pipeline that creates the template.*
 
-`PUT /templates/tags` with the following payload
+`PUT /templates/{templateName}/tags/{tagName}` with the following payload
 
-* `name` - Name of the template (ex: `mytemplate`)
-* `version` - Version of the template (ex: `1.1.0`)
-* `tag` - Name of the tag (ex: `stable`)
+* `version` - Exact version of the template (ex: `1.1.0`)
 
-#### Delete a template tag
+##### Delete a tag
 
 Delete the template tag. This does not delete the template itself. 
 
 *Note: This endpoint is only accessible in `build` scope and the permission is tied to the pipeline that creates the template.*
 
-`DELETE /templates/tags` with the following payload
-
-* `name` - Name of the template (ex: `mytemplate`)
-* `tag` - Name of the tag (ex: `stable`)
+`DELETE /templates/{templateName}/tags/{tagName}`

plugins/templates/createTag.js

@@ -1,15 +1,17 @@
 'use strict';
 
 const boom = require('boom');
+const joi = require('joi');
 const schema = require('screwdriver-data-schema');
+const baseSchema = schema.models.templateTag.base;
 const urlLib = require('url');
 
 /* Currently, only build scope is allowed to tag template due to security reasons.
  * The same pipeline that publishes the template has the permission to tag it.
  */
 module.exports = () => ({
     method: 'PUT',
-    path: '/templates/tags',
+    path: '/templates/{templateName}/tags/{tagName}',
     config: {
         description: 'Add or update a template tag',
         notes: 'Add or update a specific template',
@@ -28,22 +30,18 @@ module.exports = () => ({
             const templateFactory = request.server.app.templateFactory;
             const templateTagFactory = request.server.app.templateTagFactory;
             const pipelineId = request.auth.credentials.pipelineId;
-            const config = request.payload;
+            const name = request.params.templateName;
+            const tag = request.params.tagName;
+            const version = request.payload.version;
 
             return Promise.all([
                 pipelineFactory.get(pipelineId),
-                templateFactory.get({
-                    name: config.name,
-                    version: config.version
-                }),
-                templateTagFactory.get({
-                    name: config.name,
-                    tag: config.tag
-                })
+                templateFactory.get({ name, version }),
+                templateTagFactory.get({ name, tag })
             ]).then(([pipeline, template, templateTag]) => {
                 // If template doesn't exist, throw error
                 if (!template) {
-                    throw boom.notFound(`Template ${config.name}@${config.version} not found`);
+                    throw boom.notFound(`Template ${name}@${version} not found`);
                 }
 
                 // If template exists, but this build's pipelineId is not the same as template's pipelineId
@@ -54,26 +52,33 @@ module.exports = () => ({
 
                 // If template tag exists, then the only thing it can update is the version
                 if (templateTag) {
-                    templateTag.version = config.version;
+                    templateTag.version = version;
 
-                    return templateTag.update().then(tag => reply(tag.toJson()).code(200));
+                    return templateTag.update().then(newTag => reply(newTag.toJson()).code(200));
                 }
 
                 // If template exists, then create the tag
-                return templateTagFactory.create(config).then((tag) => {
+                return templateTagFactory.create({ name, tag, version })
+                .then((newTag) => {
                     const location = urlLib.format({
                         host: request.headers.host,
                         port: request.headers.port,
                         protocol: request.server.info.protocol,
-                        pathname: `${request.path}/${tag.id}`
+                        pathname: `${request.path}/${newTag.id}`
                     });
 
-                    return reply(tag.toJson()).header('Location', location).code(201);
+                    return reply(newTag.toJson()).header('Location', location).code(201);
                 });
             }).catch(err => reply(boom.wrap(err)));
         },
         validate: {
-            payload: schema.models.templateTag.create
+            params: {
+                templateName: joi.reach(baseSchema, 'name'),
+                tagName: joi.reach(baseSchema, 'tag')
+            },
+            payload: {
+                version: joi.reach(baseSchema, 'version')
+            }
         }
     }
 });

plugins/templates/removeTag.js

@@ -1,14 +1,16 @@
 'use strict';
 
 const boom = require('boom');
+const joi = require('joi');
 const schema = require('screwdriver-data-schema');
+const baseSchema = schema.models.templateTag.base;
 
 /* Currently, only build scope is allowed to tag template due to security reasons.
  * The same pipeline that publishes the template has the permission to tag it.
  */
 module.exports = () => ({
     method: 'DELETE',
-    path: '/templates/tags',
+    path: '/templates/{templateName}/tags/{tagName}',
     config: {
         description: 'Delete a template tag',
         notes: 'Delete a specific template',
@@ -27,12 +29,10 @@ module.exports = () => ({
             const templateFactory = request.server.app.templateFactory;
             const templateTagFactory = request.server.app.templateTagFactory;
             const pipelineId = request.auth.credentials.pipelineId;
-            const config = request.payload;
+            const name = request.params.templateName;
+            const tag = request.params.tagName;
 
-            return templateTagFactory.get({
-                name: config.name,
-                tag: config.tag
-            })
+            return templateTagFactory.get({ name, tag })
             .then((templateTag) => {
                 if (!templateTag) {
                     throw boom.notFound('Template tag does not exist');
@@ -41,7 +41,7 @@ module.exports = () => ({
                 return Promise.all([
                     pipelineFactory.get(pipelineId),
                     templateFactory.get({
-                        name: config.name,
+                        name,
                         version: templateTag.version
                     })
                 ])
@@ -59,7 +59,10 @@ module.exports = () => ({
             .catch(err => reply(boom.wrap(err)));
         },
         validate: {
-            payload: schema.models.templateTag.remove
+            params: {
+                templateName: joi.reach(baseSchema, 'name'),
+                tagName: joi.reach(baseSchema, 'tag')
+            }
         }
     }
 });

test/plugins/templates.test.js

@@ -70,9 +70,6 @@ describe('template plugin test', () => {
             get: sinon.stub(),
             remove: sinon.stub()
         };
-        templateTagFactoryMock = {
-            get: sinon.stub()
-        };
         pipelineFactoryMock = {
             get: sinon.stub()
         };
@@ -404,20 +401,17 @@ describe('template plugin test', () => {
         let options;
         let templateMock;
         let pipelineMock;
-        const payload = {
-            name: 'testtemplate',
-            tag: 'stable'
-        };
-        const testTemplateTag = decorateObj(hoek.merge({
+        const testTemplateTag = decorateObj({
             id: 1,
+            name: 'testtemplate',
+            tag: 'stable',
             remove: sinon.stub().resolves(null)
-        }, payload));
+        });
 
         beforeEach(() => {
             options = {
                 method: 'DELETE',
-                url: '/templates/tags',
-                payload,
+                url: '/templates/testtemplate/tags/stable',
                 credentials: {
                     scope: ['build']
                 }
@@ -460,16 +454,14 @@ describe('template plugin test', () => {
         let templateMock;
         let pipelineMock;
         const payload = {
-            name: 'testtemplate',
-            tag: 'stable',
             version: '1.2.0'
         };
         const testTemplateTag = decorateObj(hoek.merge({ id: 1 }, payload));
 
         beforeEach(() => {
             options = {
                 method: 'PUT',
-                url: '/templates/tags',
+                url: '/templates/testtemplate/tags/stable',
                 payload,
                 credentials: {
                     scope: ['build']
@@ -522,7 +514,11 @@ describe('template plugin test', () => {
                     name: 'testtemplate',
                     tag: 'stable'
                 });
-                assert.calledWith(templateTagFactoryMock.create, payload);
+                assert.calledWith(templateTagFactoryMock.create, {
+                    name: 'testtemplate',
+                    tag: 'stable',
+                    version: '1.2.0'
+                });
                 assert.equal(reply.statusCode, 201);
             });
         });