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

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) {

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)
+  })
+})

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,

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',

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) {

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(