From d959b763f089ed4c1087aa4aeb824a8ef6743111 Mon Sep 17 00:00:00 2001
From: Chocobozzz <me@florianbigard.com>
Date: Wed, 12 Jul 2023 10:53:46 +0200
Subject: [PATCH] Avoid duplicate runner names

---
 server/initializers/constants.ts              |  2 +-
 .../migrations/0795-duplicate-runner-name.ts  | 24 +++++++++++++++++++
 .../middlewares/validators/runners/runners.ts |  9 +++++++
 server/models/runner/runner.ts                | 12 ++++++++++
 server/tests/api/check-params/runners.ts      |  9 +++++++
 .../runners/runners-command.ts                |  3 ++-
 6 files changed, 57 insertions(+), 2 deletions(-)
 create mode 100644 server/initializers/migrations/0795-duplicate-runner-name.ts

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 () {
diff --git a/shared/server-commands/runners/runners-command.ts b/shared/server-commands/runners/runners-command.ts
index ca9a1d7a3..b0083e841 100644
--- a/shared/server-commands/runners/runners-command.ts
+++ b/shared/server-commands/runners/runners-command.ts
@@ -1,4 +1,5 @@
 import { pick } from '@shared/core-utils'
+import { buildUUID } from '@shared/extra-utils'
 import { HttpStatusCode, RegisterRunnerBody, RegisterRunnerResult, ResultList, Runner, UnregisterRunnerBody } from '@shared/models'
 import { unwrapBody } from '../requests'
 import { AbstractCommand, OverrideCommandOptions } from '../shared'
@@ -68,7 +69,7 @@ export class RunnersCommand extends AbstractCommand {
     const { data } = await this.server.runnerRegistrationTokens.list({ sort: 'createdAt' })
 
     const { runnerToken } = await this.register({
-      name: 'runner',
+      name: 'runner ' + buildUUID(),
       registrationToken: data[0].registrationToken
     })