From 60df9ae66c75432484e5839a54adbff7d7ddb081 Mon Sep 17 00:00:00 2001
From: Chris Turner <23338096+christopherjturner@users.noreply.github.com>
Date: Tue, 27 Aug 2024 14:54:15 +0100
Subject: [PATCH] Fixes for tf-svc-infra workflow

- updateWorkflowStatus/updateCreationStatus no longer overwrites trigger
  data
- bulk-update-tf-svc-infra uses the new tenant json files
- refactoring dont-overwrite-status to be a bit more
  readable/understandable
- updated lookupTenantService to include ref (for future improvements),
  refacotring a bit (unsure if the joi validating actually makes it
better?
- removed PR related status/handlers
---
 .../deploy/helpers/lookup-tenant-service.js   |  69 ++++---
 .../helpers/lookup-tenant-service.test.js     | 193 ++++++++++++++++++
 src/config/config.js                          |  10 +
 src/constants/statuses.js                     |   2 -
 src/helpers/create/init-creation-status.js    |  21 +-
 .../create-resource-from-workflow.js          |  22 +-
 .../github/handlers/pull-request-handler.js   |  57 ------
 .../handlers/workflow-run-handler-v2.js       |  13 +-
 .../helpers/bulk-update-tf-svc-infra.js       |  45 ++--
 .../helpers/bulk-update-tf-svc-infra.test.js  |  88 ++++++++
 .../github/helpers/dont-overwrite-status.js   |  48 ++---
 .../helpers/dont-overwrite-status.test.js     |  66 ++++--
 .../lookup-tenant-service-for-commit.js       |  22 --
 src/listeners/github/message-handler.js       |   7 +-
 src/listeners/github/status-repo.js           |  69 ++-----
 15 files changed, 474 insertions(+), 258 deletions(-)
 create mode 100644 src/api/deploy/helpers/lookup-tenant-service.test.js
 delete mode 100644 src/listeners/github/handlers/pull-request-handler.js
 create mode 100644 src/listeners/github/helpers/bulk-update-tf-svc-infra.test.js
 delete mode 100644 src/listeners/github/helpers/lookup-tenant-service-for-commit.js

diff --git a/src/api/deploy/helpers/lookup-tenant-service.js b/src/api/deploy/helpers/lookup-tenant-service.js
index 1932288..89ab38a 100644
--- a/src/api/deploy/helpers/lookup-tenant-service.js
+++ b/src/api/deploy/helpers/lookup-tenant-service.js
@@ -1,42 +1,63 @@
 import { getContent } from '~/src/helpers/github/get-content'
 import { config } from '~/src/config'
+import { createLogger } from '~/src/helpers/logging/logger'
+import Joi from 'joi'
 
-async function lookupTenantService(service, environment, logger) {
-  const filePath = `environments/${environment}/tenants/${service}.json`
-  const owner = config.get('github.org')
-  const repo = config.get('github.repos.cdpTfSvcInfra')
+const logger = createLogger()
+
+const org = config.get('github.org')
+const repo = config.get('github.repos.cdpTfSvcInfra')
+
+const schema = Joi.object({
+  zone: Joi.string().valid('public', 'protected').required(),
+  service_code: Joi.string().min(3).required(),
+  mongo: Joi.boolean(),
+  redis: Joi.boolean(),
+  testSuite: Joi.string()
+}).unknown(true)
 
+/**
+ * Get service from the multi-file version of tenants.json
+ * @param {string} service
+ * @param {string} environment
+ * @param {string} ref
+ * @returns {Promise<undefined|*>}
+ */
+async function lookupTenantService(service, environment, ref = 'main') {
+  const filePath = `environments/${environment}/tenants/${service}.json`
   try {
-    const data = await getContent(owner, repo, filePath)
+    const data = await getContent(org, repo, filePath, ref)
     const service = JSON.parse(data)
-
-    if (service?.zone && service?.service_code) {
-      return service
-    } else {
-      logger.warn(
-        `${service}.json did not contain zone and service_code - Falling back to tenant_services.json`
-      )
-      return await lookupTenantServices(
-        service,
-        environment,
-        owner,
-        repo,
-        logger
-      )
-    }
+    Joi.assert(service, schema) // Check file in correct format
+    return service
   } catch (error) {
     logger.error(
-      error,
       `Error attempting to retrieve ${filePath} from GitHub - Falling back to tenant_services.json`
     )
-    return await lookupTenantServices(service, environment, owner, repo, logger)
+    return await lookupLegacyTenantService(service, environment, org, repo, ref)
   }
 }
 
-async function lookupTenantServices(service, environment, owner, repo, logger) {
+/**
+ * Get service from the legacy single-file version of tenants.json
+ * @param {string} service
+ * @param {string} environment
+ * @param {string} owner
+ * @param {string} repo
+ * @param {string} ref
+ * @returns {Promise<undefined|*>}
+ */
+async function lookupLegacyTenantService(
+  service,
+  environment,
+  owner,
+  repo,
+  ref
+) {
   const filePath = `environments/${environment}/resources/tenant_services.json`
+  logger.info(`Getting legacy tenant file ${filePath}`)
   try {
-    const data = await getContent(owner, repo, filePath)
+    const data = await getContent(owner, repo, filePath, ref)
     const services = JSON.parse(data)
     return services[0][service]
   } catch (error) {
diff --git a/src/api/deploy/helpers/lookup-tenant-service.test.js b/src/api/deploy/helpers/lookup-tenant-service.test.js
new file mode 100644
index 0000000..2bde0b0
--- /dev/null
+++ b/src/api/deploy/helpers/lookup-tenant-service.test.js
@@ -0,0 +1,193 @@
+import { getContent } from '~/src/helpers/github/get-content'
+import { config } from '~/src/config/config'
+import { lookupTenantService } from '~/src/api/deploy/helpers/lookup-tenant-service'
+
+jest.mock('~/src/helpers/github/get-content', () => ({
+  getContent: jest.fn()
+}))
+const mockGetContent = getContent
+
+describe('lookupTenantService', () => {
+  const org = config.get('github.org')
+  const repo = config.get('github.repos.cdpTfSvcInfra')
+
+  afterEach(() => {
+    jest.clearAllMocks()
+  })
+
+  test('should get the tenant service from the single file version when present', async () => {
+    const tenant = {
+      zone: 'public',
+      mongo: false,
+      redis: true,
+      service_code: 'ABC'
+    }
+
+    getContent.mockResolvedValue(JSON.stringify(tenant))
+
+    const result = await lookupTenantService('someService', 'dev')
+
+    expect(mockGetContent).toHaveBeenCalledWith(
+      org,
+      repo,
+      'environments/dev/tenants/someService.json',
+      'main'
+    )
+    expect(result).toEqual(tenant)
+  })
+
+  test('should fallback to legacy tenant json when not found', async () => {
+    const tenants = [
+      {
+        someService: {
+          zone: 'public',
+          mongo: false,
+          redis: true,
+          service_code: 'ABC'
+        }
+      }
+    ]
+
+    getContent
+      .mockReturnValueOnce(null)
+      .mockResolvedValue(JSON.stringify(tenants))
+
+    const result = await lookupTenantService('someService', 'dev')
+
+    expect(mockGetContent).toHaveBeenNthCalledWith(
+      1,
+      org,
+      repo,
+      'environments/dev/tenants/someService.json',
+      'main'
+    )
+
+    expect(mockGetContent).toHaveBeenNthCalledWith(
+      2,
+      org,
+      repo,
+      'environments/dev/resources/tenant_services.json',
+      'main'
+    )
+
+    expect(result).toEqual({
+      zone: 'public',
+      mongo: false,
+      redis: true,
+      service_code: 'ABC'
+    })
+  })
+
+  test('should fallback to legacy tenant json when service_code is missing', async () => {
+    const missingServiceCode = {
+      zone: 'public',
+      mongo: false,
+      redis: true
+    }
+    const tenants = [
+      {
+        someService: {
+          zone: 'public',
+          mongo: false,
+          redis: true,
+          service_code: 'ABC'
+        }
+      }
+    ]
+
+    getContent
+      .mockReturnValueOnce(missingServiceCode)
+      .mockResolvedValue(JSON.stringify(tenants))
+
+    const result = await lookupTenantService('someService', 'dev')
+
+    expect(mockGetContent).toHaveBeenNthCalledWith(
+      1,
+      org,
+      repo,
+      'environments/dev/tenants/someService.json',
+      'main'
+    )
+
+    expect(mockGetContent).toHaveBeenNthCalledWith(
+      2,
+      org,
+      repo,
+      'environments/dev/resources/tenant_services.json',
+      'main'
+    )
+
+    expect(result).toEqual({
+      zone: 'public',
+      mongo: false,
+      redis: true,
+      service_code: 'ABC'
+    })
+  })
+
+  test('should return nothing when the service doesnt exist', async () => {
+    getContent.mockResolvedValue(null)
+
+    const result = await lookupTenantService('someService', 'dev')
+
+    expect(mockGetContent).toHaveBeenCalledTimes(2)
+    expect(result).toBeUndefined()
+  })
+
+  test('should use a different ref when specified', async () => {
+    const tenants = [
+      {
+        someService: {
+          zone: 'public',
+          mongo: false,
+          redis: true,
+          service_code: 'ABC'
+        }
+      }
+    ]
+
+    getContent
+      .mockReturnValueOnce(null)
+      .mockResolvedValue(JSON.stringify(tenants))
+
+    await lookupTenantService('someService', 'dev', '87428fc5')
+
+    expect(mockGetContent).toHaveBeenNthCalledWith(
+      1,
+      org,
+      repo,
+      'environments/dev/tenants/someService.json',
+      '87428fc5'
+    )
+
+    expect(mockGetContent).toHaveBeenNthCalledWith(
+      2,
+      org,
+      repo,
+      'environments/dev/resources/tenant_services.json',
+      '87428fc5'
+    )
+  })
+
+  test('should get the tenant service from the single file version when present even if it has extra fields', async () => {
+    const tenant = {
+      zone: 'public',
+      mongo: false,
+      redis: true,
+      service_code: 'ABC',
+      postgres: true
+    }
+
+    getContent.mockResolvedValue(JSON.stringify(tenant))
+
+    const result = await lookupTenantService('someService', 'dev')
+
+    expect(mockGetContent).toHaveBeenCalledWith(
+      org,
+      repo,
+      'environments/dev/tenants/someService.json',
+      'main'
+    )
+    expect(result).toEqual(tenant)
+  })
+})
diff --git a/src/config/config.js b/src/config/config.js
index ded65a0..573668a 100644
--- a/src/config/config.js
+++ b/src/config/config.js
@@ -321,6 +321,16 @@ const config = convict({
       default: 'create-service.yml',
       env: 'WORKFLOWS_CREATE_TENANT_SERVICE'
     },
+    applyTenantService: {
+      doc: 'Github workflow triggered by merges to cdp-tf-svc-infra. Used for recovering failed create runs.',
+      format: String,
+      default: 'apply.yml'
+    },
+    manualApplyTenantService: {
+      doc: 'Github workflow for manually applying cdp-tf-svc-infra. Used for recovering failed create runs.',
+      format: String,
+      default: 'manual.yml'
+    },
     createDashboard: {
       doc: 'Github workflow to trigger when creating dashboard',
       format: String,
diff --git a/src/constants/statuses.js b/src/constants/statuses.js
index 2da46ed..2de0f59 100644
--- a/src/constants/statuses.js
+++ b/src/constants/statuses.js
@@ -4,8 +4,6 @@ const statuses = {
   completed: 'completed',
   raised: 'raised',
   queued: 'queued',
-  open: 'pr_open',
-  closed: 'pr_closed',
   merged: 'merged',
   inProgress: 'in-progress',
   success: 'success',
diff --git a/src/helpers/create/init-creation-status.js b/src/helpers/create/init-creation-status.js
index 47c2ca1..73855e4 100644
--- a/src/helpers/create/init-creation-status.js
+++ b/src/helpers/create/init-creation-status.js
@@ -107,10 +107,25 @@ async function initCreationStatus(
   return status
 }
 
+/**
+ *
+ * @param db
+ * @param {string} repo
+ * @param {string} field - The name of the field/workflow being updated
+ * @param {{status: string, trigger: {org: string, repo: string, workflow: string, inputs: object}, result: Object|undefined }} status
+ * @returns {Promise<*>}
+ */
 async function updateCreationStatus(db, repo, field, status) {
-  return await db
-    .collection('status')
-    .updateOne({ repositoryName: repo }, { $set: { [field]: status } })
+  return await db.collection('status').updateOne(
+    { repositoryName: repo },
+    {
+      $set: {
+        [`${field}.status`]: status.status,
+        [`${field}.trigger`]: status.trigger,
+        [`${field}.result`]: status.result
+      }
+    }
+  )
 }
 
 async function updateOverallStatus(db, repositoryName) {
diff --git a/src/helpers/create/workflows/create-resource-from-workflow.js b/src/helpers/create/workflows/create-resource-from-workflow.js
index ecc0dee..c091c24 100644
--- a/src/helpers/create/workflows/create-resource-from-workflow.js
+++ b/src/helpers/create/workflows/create-resource-from-workflow.js
@@ -20,24 +20,28 @@ const createResourceFromWorkflow = async (
   workflow,
   inputs
 ) => {
+  request.logger.info(
+    `Workflow ${repo}/${workflow} triggered for ${service} with inputs ${JSON.stringify(inputs)}`
+  )
+  const trigger = {
+    org,
+    repo,
+    workflow,
+    inputs
+  }
+
   try {
-    request.logger.info(
-      `Workflow ${repo}/${workflow} triggered for ${service} with inputs ${JSON.stringify(inputs)}`
-    )
     await triggerWorkflow(org, repo, workflow, inputs)
 
     await updateCreationStatus(request.db, service, repo, {
       status: statuses.requested,
-      trigger: {
-        org,
-        repo,
-        workflow,
-        inputs
-      }
+      trigger,
+      result: 'ok'
     })
   } catch (e) {
     await updateCreationStatus(request.db, service, repo, {
       status: statuses.failure,
+      trigger,
       result: e?.response ?? 'see cdp-self-service-ops logs'
     })
     request.logger.error(
diff --git a/src/listeners/github/handlers/pull-request-handler.js b/src/listeners/github/handlers/pull-request-handler.js
deleted file mode 100644
index 43a2280..0000000
--- a/src/listeners/github/handlers/pull-request-handler.js
+++ /dev/null
@@ -1,57 +0,0 @@
-import {
-  findByPrNumber,
-  updatePrStatus
-} from '~/src/listeners/github/status-repo'
-import { createLogger } from '~/src/helpers/logging/logger'
-
-const pullRequestHandler = async (db, message) => {
-  const logger = createLogger()
-  try {
-    const repo = message.repository?.name
-    const prNumber = message.number
-
-    logger.info(
-      `Received pr webhook ${repo}:${prNumber}, ${message.action} ${message.pull_request.state} ${message.pull_request.merged}`
-    )
-
-    // Ignore PR events that are not opened or closed.
-    if (message.action !== 'opened' && message.action !== 'closed') {
-      logger.info(
-        `Ignoring pull request action ${message.action} for ${repo} ${prNumber}`
-      )
-      return
-    }
-
-    const status = await findByPrNumber(db, repo, prNumber)
-    if (status === null) {
-      logger.info(
-        `Skipping pull request message, not a tracked repo ${repo} ${prNumber}`
-      )
-      return
-    }
-
-    // Note: we append `pr_` to the front of the status (i.e. pr_closed).
-    let prState = `pr_${message.pull_request.state}`
-
-    // Merged pull requests have a state of closed and a merged flag
-    if (message.pull_request.merged) {
-      prState = 'merged'
-    }
-
-    logger.info(
-      `Updating ${repo}/${prNumber} status to ${prState} head commit sha to ${message.pull_request.merge_commit_sha}`
-    )
-
-    await updatePrStatus(
-      db,
-      status.repositoryName,
-      repo,
-      prState,
-      message.pull_request.merge_commit_sha
-    )
-  } catch (e) {
-    logger.error(e)
-  }
-}
-
-export { pullRequestHandler }
diff --git a/src/listeners/github/handlers/workflow-run-handler-v2.js b/src/listeners/github/handlers/workflow-run-handler-v2.js
index 8bb2e62..6308194 100644
--- a/src/listeners/github/handlers/workflow-run-handler-v2.js
+++ b/src/listeners/github/handlers/workflow-run-handler-v2.js
@@ -20,8 +20,8 @@ const shouldWorkflowBeProcessed = (repo, workflow) => {
     case cfg.github.repos.cdpTfSvcInfra:
       return [
         cfg.workflows.createTenantService,
-        'manual.yml', // we also want to trigger the handler off apply and manual
-        'apply.yml'
+        cfg.workflows.applyTenantService, // we also want to trigger the handler off apply and manual
+        cfg.workflows.manualApplyTenantService
       ].includes(workflow)
     case cfg.github.repos.createWorkflows:
       return [
@@ -42,25 +42,24 @@ const workflowRunHandlerV2 = async (server, message) => {
   const workflowRepo = message.repository?.name
   const headBranch = message.workflow_run?.head_branch
   const headSHA = message.workflow_run?.head_sha
-  const workflowPath = message.workflow_run?.path
+  const workflowFile = message.workflow_run?.path
     ? path.basename(message.workflow_run?.path)
     : undefined
 
   if (headBranch !== 'main') {
     logger.info(
-      `Not processing workflow run ${workflowRepo}/${workflowPath}, not running on main branch`
+      `Not processing workflow run ${workflowRepo}/${workflowFile}, not running on main branch`
     )
     return
   }
 
   // Are we interested in handling the event?
-  if (shouldWorkflowBeProcessed(workflowRepo, workflowPath)) {
+  if (shouldWorkflowBeProcessed(workflowRepo, workflowFile)) {
     logger.info(
       `Processing workflow_run message for ${workflowRepo}, ${headBranch}/${headSHA}, action: ${message.action}`
     )
     switch (workflowRepo) {
       case config.get('github.repos.cdpTfSvcInfra'):
-        // cdp-tf-svc-infra has its own handler
         await handleTfSvcInfraWorkflow(db, logger, message)
         break
       default:
@@ -68,7 +67,7 @@ const workflowRunHandlerV2 = async (server, message) => {
         break
     }
   } else {
-    logger.info(`Not processing ${workflowRepo}/${workflowPath}`)
+    logger.info(`Not processing ${workflowRepo}/${workflowFile}`)
   }
 }
 
diff --git a/src/listeners/github/helpers/bulk-update-tf-svc-infra.js b/src/listeners/github/helpers/bulk-update-tf-svc-infra.js
index a5f3c92..e7fdde8 100644
--- a/src/listeners/github/helpers/bulk-update-tf-svc-infra.js
+++ b/src/listeners/github/helpers/bulk-update-tf-svc-infra.js
@@ -1,5 +1,4 @@
-import { lookupTenantServicesForCommit } from '~/src/listeners/github/helpers/lookup-tenant-service-for-commit'
-import { config, environments } from '~/src/config'
+import { config } from '~/src/config'
 import {
   findAllInProgressOrFailed,
   updateWorkflowStatus
@@ -7,35 +6,37 @@ import {
 import { updateOverallStatus } from '~/src/helpers/create/init-creation-status'
 import { createPlaceholderArtifact } from '~/src/listeners/github/helpers/create-placeholder-artifact'
 import { createLogger } from '~/src/helpers/logging/logger'
-import { ackEvent } from '~/src/helpers/queued-events/queued-events'
+import { lookupTenantService } from '~/src/api/deploy/helpers/lookup-tenant-service'
 
-// given a list of services, update the tf-svc-infra status for all of them to success
-// and create the placeholder artifact
+/**
+ * given a list of services, update the tf-svc-infra status for all of them to success
+ * and create the placeholder artifact
+ * @param db
+ * @param trimmedWorkflow
+ * @param status
+ * @returns {Promise<void>}
+ */
 const bulkUpdateTfSvcInfra = async (db, trimmedWorkflow, status) => {
   const logger = createLogger()
   const org = config.get('github.org')
   const tfSvcInfra = config.get('github.repos.cdpTfSvcInfra')
-  const eventType = config.get('serviceInfraCreateEvent')
 
-  const tenants = await lookupTenantServicesForCommit(environments.management)
-
-  logger.info(tenants)
-
-  if (tenants === undefined) {
-    // TODO: handle error
-    logger.error('Failed to lookup tenant services')
-    throw new Error('Failed to lookup tenant services')
-  }
-  const tenantNames = new Set(Object.keys(tenants))
   const inProgressOrFailed = await findAllInProgressOrFailed(db)
 
   // anything that's in-progress/failed and now exists in tenant-services
   // exists, so should be updated
-  const servicesToUpdate = inProgressOrFailed
-    .filter((s) => tenantNames.has(s.repositoryName))
-    .map((s) => {
-      return { name: s.repositoryName, tenantConfig: tenants[s.repositoryName] }
-    })
+
+  const servicesToUpdate = []
+
+  for (const service of inProgressOrFailed) {
+    const name = service.repositoryName
+
+    // TODO: maybe use exact commit ref of workflow
+    const tenantConfig = await lookupTenantService(name, 'management')
+    if (tenantConfig) {
+      servicesToUpdate.push({ name, tenantConfig })
+    }
+  }
 
   logger.info(
     `Updating ${servicesToUpdate.length} ${tfSvcInfra} statuses to success`
@@ -74,8 +75,6 @@ const bulkUpdateTfSvcInfra = async (db, trimmedWorkflow, status) => {
       gitHubUrl: `https://github.com/${org}/${serviceName}`,
       runMode
     })
-
-    await ackEvent(db, serviceName, eventType)
   }
 }
 
diff --git a/src/listeners/github/helpers/bulk-update-tf-svc-infra.test.js b/src/listeners/github/helpers/bulk-update-tf-svc-infra.test.js
new file mode 100644
index 0000000..701d72f
--- /dev/null
+++ b/src/listeners/github/helpers/bulk-update-tf-svc-infra.test.js
@@ -0,0 +1,88 @@
+import { bulkUpdateTfSvcInfra } from '~/src/listeners/github/helpers/bulk-update-tf-svc-infra'
+import {
+  findAllInProgressOrFailed,
+  updateWorkflowStatus
+} from '~/src/listeners/github/status-repo'
+
+import { createPlaceholderArtifact } from '~/src/listeners/github/helpers/create-placeholder-artifact'
+import { lookupTenantService } from '~/src/api/deploy/helpers/lookup-tenant-service'
+import { updateOverallStatus } from '~/src/helpers/create/init-creation-status'
+import { statuses } from '~/src/constants/statuses'
+
+jest.mock('~/src/helpers/create/init-creation-status', () => ({
+  updateOverallStatus: jest.fn()
+}))
+
+jest.mock('~/src/listeners/github/helpers/create-placeholder-artifact', () => ({
+  createPlaceholderArtifact: jest.fn()
+}))
+
+jest.mock('~/src/api/deploy/helpers/lookup-tenant-service', () => ({
+  lookupTenantService: jest.fn()
+}))
+
+jest.mock('~/src/listeners/github/status-repo', () => ({
+  findAllInProgressOrFailed: jest.fn(),
+  updateWorkflowStatus: jest.fn()
+}))
+
+describe('bulkUpdateTtfSvcInfra', () => {
+  test('it should update a service thats pending and has a tenant json entry', async () => {
+    findAllInProgressOrFailed.mockResolvedValue([
+      {
+        repositoryName: 'test1'
+      }
+    ])
+    lookupTenantService.mockResolvedValue({})
+
+    const db = {}
+    const workflow = {
+      name: 'test1',
+      id: 1,
+      html_url: `http://localhost:8080/test1`,
+      created_at: new Date(),
+      updated_at: new Date(),
+      path: '.github/workflows/create-service.yml'
+    }
+    await bulkUpdateTfSvcInfra(db, workflow, statuses.success)
+
+    expect(updateWorkflowStatus).toHaveBeenCalledWith(
+      db,
+      'test1',
+      'cdp-tf-svc-infra',
+      'main',
+      statuses.success,
+      workflow
+    )
+    expect(updateOverallStatus).toHaveBeenCalledWith(db, 'test1')
+    expect(createPlaceholderArtifact).toHaveBeenCalledWith({
+      service: 'test1',
+      gitHubUrl: `https://github.com/DEFRA/test1`,
+      runMode: 'Service'
+    })
+  })
+
+  test('it should not update a service that doesnt have a tenant json', async () => {
+    findAllInProgressOrFailed.mockResolvedValue([
+      {
+        repositoryName: 'test1'
+      }
+    ])
+    lookupTenantService.mockResolvedValue(null)
+
+    const db = {}
+    const workflow = {
+      name: 'test1',
+      id: 1,
+      html_url: `http://localhost:8080/test1`,
+      created_at: new Date(),
+      updated_at: new Date(),
+      path: '.github/workflows/create-service.yml'
+    }
+    await bulkUpdateTfSvcInfra(db, workflow, statuses.success)
+
+    expect(updateWorkflowStatus).not.toHaveBeenCalled()
+    expect(updateOverallStatus).not.toHaveBeenCalled()
+    expect(createPlaceholderArtifact).not.toHaveBeenCalled()
+  })
+})
diff --git a/src/listeners/github/helpers/dont-overwrite-status.js b/src/listeners/github/helpers/dont-overwrite-status.js
index 2c73bea..7099ee1 100644
--- a/src/listeners/github/helpers/dont-overwrite-status.js
+++ b/src/listeners/github/helpers/dont-overwrite-status.js
@@ -1,38 +1,20 @@
 import { statuses } from '~/src/constants/statuses'
 
-const dontOverwriteStatus = (workflowStatus) => {
-  switch (workflowStatus) {
-    case statuses.notRequested:
-      return Object.values(statuses)
-    case statuses.raised:
-    case statuses.open:
-    case statuses.closed:
-    case statuses.requested:
-      return [
-        statuses.queued,
-        statuses.success,
-        statuses.failure,
-        statuses.inProgress,
-        statuses.closed,
-        statuses.merged
-      ]
-    case statuses.queued:
-      return [
-        statuses.success,
-        statuses.failure,
-        statuses.inProgress,
-        statuses.closed,
-        statuses.merged
-      ]
-    case statuses.merged:
-      return [statuses.success, statuses.failure, statuses.inProgress]
-    case statuses.inProgress:
-      return [statuses.success, statuses.failure]
-    case statuses.success:
-    case statuses.failure:
-    default:
-      return []
-  }
+// Status precedence from lowest (0) to highest.
+// Lower statuses should not override higher ones.
+const statusPrecedence = [
+  [statuses.notRequested],
+  [statuses.raised, statuses.requested],
+  [statuses.queued],
+  [statuses.merged],
+  [statuses.inProgress],
+  [statuses.success, statuses.failure]
+]
+
+function dontOverwriteStatus(s) {
+  const idx = statusPrecedence.findIndex((t) => t.includes(s))
+  if (idx === -1) return []
+  return statusPrecedence.slice(idx + 1).flat()
 }
 
 export { dontOverwriteStatus }
diff --git a/src/listeners/github/helpers/dont-overwrite-status.test.js b/src/listeners/github/helpers/dont-overwrite-status.test.js
index 00520b8..ffae076 100644
--- a/src/listeners/github/helpers/dont-overwrite-status.test.js
+++ b/src/listeners/github/helpers/dont-overwrite-status.test.js
@@ -2,29 +2,59 @@ import { dontOverwriteStatus } from '~/src/listeners/github/helpers/dont-overwri
 import { statuses } from '~/src/constants/statuses'
 
 describe('#dont-overwrite-status', () => {
-  test('in progress cant overwrite complete and finished', () => {
-    expect(dontOverwriteStatus(statuses.inProgress)).toContain(statuses.success)
-    expect(dontOverwriteStatus(statuses.inProgress)).toContain(statuses.failure)
+  test('returns empty list when unknown status is given', () => {
+    expect(dontOverwriteStatus('an-unknown-status-cdoe')).toEqual([])
   })
 
-  test('success and failure can overwrite everything', () => {
-    expect(dontOverwriteStatus(statuses.success)).toEqual([])
-    expect(dontOverwriteStatus(statuses.failure)).toEqual([])
-  })
+  test('returns a list of statuses that cant be overwritten for a given status', () => {
+    expect(dontOverwriteStatus(statuses.success).sort()).toEqual([])
+    expect(dontOverwriteStatus(statuses.failure).sort()).toEqual([])
+    expect(dontOverwriteStatus(statuses.inProgress).sort()).toEqual(
+      [statuses.success, statuses.failure].sort()
+    )
+    expect(dontOverwriteStatus(statuses.merged).sort()).toEqual(
+      [statuses.inProgress, statuses.success, statuses.failure].sort()
+    )
 
-  test('requested cant overwrite in-progress, success or failure', () => {
-    expect(dontOverwriteStatus(statuses.requested)).toContain(statuses.failure)
-    expect(dontOverwriteStatus(statuses.requested)).toContain(statuses.success)
-    expect(dontOverwriteStatus(statuses.requested)).toContain(
-      statuses.inProgress
+    expect(dontOverwriteStatus(statuses.queued).sort()).toEqual(
+      [
+        statuses.merged,
+        statuses.inProgress,
+        statuses.success,
+        statuses.failure
+      ].sort()
     )
-  })
 
-  test('merge cannot be overwritten by pr raised', () => {
-    expect(dontOverwriteStatus(statuses.raised)).toContain(statuses.merged)
-  })
+    expect(dontOverwriteStatus(statuses.raised).sort()).toEqual(
+      [
+        statuses.queued,
+        statuses.merged,
+        statuses.inProgress,
+        statuses.success,
+        statuses.failure
+      ].sort()
+    )
+
+    expect(dontOverwriteStatus(statuses.requested).sort()).toEqual(
+      [
+        statuses.queued,
+        statuses.merged,
+        statuses.inProgress,
+        statuses.success,
+        statuses.failure
+      ].sort()
+    )
 
-  test('merge cannot be overwritten by pr closed', () => {
-    expect(dontOverwriteStatus(statuses.closed)).toContain(statuses.merged)
+    expect(dontOverwriteStatus(statuses.notRequested).sort()).toEqual(
+      [
+        statuses.raised,
+        statuses.requested,
+        statuses.queued,
+        statuses.merged,
+        statuses.inProgress,
+        statuses.success,
+        statuses.failure
+      ].sort()
+    )
   })
 })
diff --git a/src/listeners/github/helpers/lookup-tenant-service-for-commit.js b/src/listeners/github/helpers/lookup-tenant-service-for-commit.js
deleted file mode 100644
index c8570ff..0000000
--- a/src/listeners/github/helpers/lookup-tenant-service-for-commit.js
+++ /dev/null
@@ -1,22 +0,0 @@
-import { createLogger } from '~/src/helpers/logging/logger'
-import { getContent } from '~/src/helpers/github/get-content'
-import { config } from '~/src/config'
-
-async function lookupTenantServicesForCommit(environment, sha) {
-  const logger = createLogger()
-  const filePath = `environments/${environment}/resources/tenant_services.json`
-  const owner = config.get('github.org')
-  const repo = config.get('github.repos.cdpTfSvcInfra')
-
-  try {
-    const data = await getContent(owner, repo, filePath, sha)
-    const services = JSON.parse(data)
-
-    return services[0]
-  } catch (error) {
-    logger.error(error)
-    return undefined
-  }
-}
-
-export { lookupTenantServicesForCommit }
diff --git a/src/listeners/github/message-handler.js b/src/listeners/github/message-handler.js
index 6ca2e21..cfae792 100644
--- a/src/listeners/github/message-handler.js
+++ b/src/listeners/github/message-handler.js
@@ -1,4 +1,3 @@
-import { pullRequestHandler } from '~/src/listeners/github/handlers/pull-request-handler'
 import { config } from '~/src/config'
 import { workflowRunHandlerV2 } from '~/src/listeners/github/handlers/workflow-run-handler-v2'
 
@@ -11,7 +10,7 @@ const githubWebhooks = new Set([
   config.get('github.repos.cdpSquidProxy'),
   config.get('github.repos.cdpGrafanaSvc')
 ])
-const validActions = new Set(['workflow_run', 'pull_request'])
+const validActions = new Set(['workflow_run'])
 
 const shouldProcess = (message) => {
   const eventType = message.github_event
@@ -24,10 +23,6 @@ const handle = async (server, message) => {
     return
   }
 
-  if (message.github_event === 'pull_request') {
-    return await pullRequestHandler(server.db, message)
-  }
-
   if (message.github_event === 'workflow_run') {
     return await workflowRunHandlerV2(server, message)
   }
diff --git a/src/listeners/github/status-repo.js b/src/listeners/github/status-repo.js
index c1976c8..88449ca 100644
--- a/src/listeners/github/status-repo.js
+++ b/src/listeners/github/status-repo.js
@@ -1,44 +1,16 @@
 import { statuses } from '~/src/constants/statuses'
 import { dontOverwriteStatus } from '~/src/listeners/github/helpers/dont-overwrite-status'
 
-async function findByPrNumber(db, repo, prNumber) {
-  const searchOn = `${repo}.pr.number`
-  return db.collection('status').findOne({ [searchOn]: prNumber })
-}
-
-async function findByCommitHash(db, repo, sha) {
-  const searchOn = `${repo}.merged_sha`
-  return db.collection('status').findOne({ [searchOn]: sha })
-}
-
 async function findByRepoName(db, repoName) {
   return db.collection('status').findOne({ repositoryName: repoName })
 }
 
-async function updatePrStatus(db, repo, field, status, mergedSha) {
-  const statusField = `${field}.status`
-  const mergedShaField = `${field}.merged_sha`
-
-  const setFields = { [statusField]: status }
-  if (mergedSha) {
-    setFields[mergedShaField] = mergedSha
-  }
-
-  return db.collection('status').updateOne(
-    {
-      repositoryName: repo,
-      [statusField]: { $nin: dontOverwriteStatus(status) }
-    },
-    { $set: setFields }
-  )
-}
-
 /**
  * @param {*} db        - mongodb database
  * @param {string} repo - status record to update
- * @param {*} workflow  - workflow step to update (e.g. cdp-tf-svc-infra)
- * @param {*} branch    - is this update related to the PR or the main branch
- * @param {*} status    - status of this step
+ * @param {string} workflow  - workflow step to update (e.g. cdp-tf-svc-infra)
+ * @param {'main'|'pr'} branch    - is this update related to the PR or the main branch
+ * @param {string} status    - status of this step
  * @param {{path, updated_at, html_url, name, created_at, id}} workflowPayload - extra data to store against this step
  */
 async function updateWorkflowStatus(
@@ -49,21 +21,18 @@ async function updateWorkflowStatus(
   status,
   workflowPayload
 ) {
-  const statusField = `${workflow}.status`
-  const workflowField = `${workflow}.${branch}.workflow` // branch is either 'main' or 'pr'
-  return db.collection('status').updateOne(
-    {
-      repositoryName: repo,
-      [statusField]: { $nin: dontOverwriteStatus(status) }
-    },
-    { $set: { [statusField]: status, [workflowField]: workflowPayload } }
-  )
-}
+  const filter = {
+    repositoryName: repo,
+    [`${workflow}.status`]: { $nin: dontOverwriteStatus(status) }
+  }
+  const update = {
+    $set: {
+      [`${workflow}.status`]: status,
+      [`${workflow}.${branch}.workflow`]: workflowPayload
+    }
+  }
 
-async function updateStatus(db, repo, field, status) {
-  return await db
-    .collection('status')
-    .updateOne({ repositoryName: repo }, { $set: { [field]: status } })
+  return db.collection('status').updateOne(filter, update)
 }
 
 async function findAllInProgressOrFailed(db) {
@@ -80,12 +49,4 @@ async function findAllInProgressOrFailed(db) {
     .toArray()
 }
 
-export {
-  findAllInProgressOrFailed,
-  findByRepoName,
-  updatePrStatus,
-  updateWorkflowStatus,
-  findByCommitHash,
-  findByPrNumber,
-  updateStatus
-}
+export { findAllInProgressOrFailed, findByRepoName, updateWorkflowStatus }