Avoid duplicate runner names

5 files changed, 55 insertions, 1 deletions

diff --git a/server/initializers/constants.ts b/server/initializers/constants.ts
index 03ae94d35..e09f0e3c6 100644
--- a/server/initializers/constants.ts
+++ b/server/initializers/constants.ts
@@ -27,7 +27,7 @@ import { CONFIG, registerConfigChangedHandler } from './config'
 
 // ---------------------------------------------------------------------------
 
-const LAST_MIGRATION_VERSION = 790
+const LAST_MIGRATION_VERSION = 795
 
 // ---------------------------------------------------------------------------
 
diff --git a/server/initializers/migrations/0795-duplicate-runner-name.ts b/server/initializers/migrations/0795-duplicate-runner-name.ts
new file mode 100644
index 000000000..dd15a8157
--- /dev/null
+++ b/server/initializers/migrations/0795-duplicate-runner-name.ts
@@ -0,0 +1,24 @@
+import * as Sequelize from 'sequelize'
+
+async function up (utils: {
+  transaction: Sequelize.Transaction
+  queryInterface: Sequelize.QueryInterface
+  sequelize: Sequelize.Sequelize
+}): Promise<void> {
+  const { transaction } = utils
+
+  const query = 'DELETE FROM "runner" r1 ' +
+    'USING (SELECT MIN(id) as id, "name" FROM "runner" GROUP BY "name" HAVING COUNT(*) > 1) r2 ' +
+    'WHERE r1."name" = r2."name" AND r1.id <> r2.id'
+
+  await utils.sequelize.query(query, { transaction })
+}
+
+function down (options) {
+  throw new Error('Not implemented.')
+}
+
+export {
+  up,
+  down
+}
diff --git a/server/middlewares/validators/runners/runners.ts b/server/middlewares/validators/runners/runners.ts
index 71a1275d2..4d4d79b4c 100644
--- a/server/middlewares/validators/runners/runners.ts
+++ b/server/middlewares/validators/runners/runners.ts
@@ -35,6 +35,15 @@ const registerRunnerValidator = [
       })
     }
 
+    const existing = await RunnerModel.loadByName(body.name)
+    if (existing) {
+      return res.fail({
+        status: HttpStatusCode.BAD_REQUEST_400,
+        message: 'This runner name already exists on this instance',
+        tags
+      })
+    }
+
     res.locals.runnerRegistrationToken = runnerRegistrationToken
 
     return next()
diff --git a/server/models/runner/runner.ts b/server/models/runner/runner.ts
index 1ef0018b4..4d07707d8 100644
--- a/server/models/runner/runner.ts
+++ b/server/models/runner/runner.ts
@@ -16,6 +16,10 @@ import { CONSTRAINTS_FIELDS } from '@server/initializers/constants'
     },
     {
       fields: [ 'runnerRegistrationTokenId' ]
+    },
+    {
+      fields: [ 'name' ],
+      unique: true
     }
   ]
 })
@@ -74,6 +78,14 @@ export class RunnerModel extends Model<Partial<AttributesOnly<RunnerModel>>> {
     return RunnerModel.findOne(query)
   }
 
+  static loadByName (name: string) {
+    const query = {
+      where: { name }
+    }
+
+    return RunnerModel.findOne(query)
+  }
+
   static listForApi (options: {
     start: number
     count: number
diff --git a/server/tests/api/check-params/runners.ts b/server/tests/api/check-params/runners.ts
index 4ba90802f..7d70c412e 100644
--- a/server/tests/api/check-params/runners.ts
+++ b/server/tests/api/check-params/runners.ts
@@ -177,6 +177,15 @@ describe('Test managing runners', function () {
 
         toDeleteId = id
       })
+
+      it('Should fail with the same runner name', async function () {
+        await server.runners.register({
+          name,
+          description: 'super description',
+          registrationToken,
+          expectedStatus: HttpStatusCode.BAD_REQUEST_400
+        })
+      })
     })
 
     describe('Delete', function () {