From f854f8cc46d2987e923a72eeed0d3dbde8aa1ad7 Mon Sep 17 00:00:00 2001
From: Fabian Meyer <3982806+meyfa@users.noreply.github.com>
Date: Sat, 13 Jul 2024 12:32:10 +0200
Subject: [PATCH] feat: Verify job ownership (#37)

Co-authored-by: Lukas Jost <lukas.jost@outlook.com>
---
 backend/src/api/jobs.ts                     |  2 +-
 backend/src/controllers/job.ts              | 18 ++++++++++++++++--
 backend/src/controllers/trigger.ts          |  7 ++++++-
 backend/src/kubernetes/api.ts               | 15 +++++++++++++++
 backend/src/{annotations.ts => metadata.ts} |  4 ++++
 5 files changed, 42 insertions(+), 4 deletions(-)
 rename backend/src/{annotations.ts => metadata.ts} (76%)

diff --git a/backend/src/api/jobs.ts b/backend/src/api/jobs.ts
index 9581966..5e3bf3f 100644
--- a/backend/src/api/jobs.ts
+++ b/backend/src/api/jobs.ts
@@ -5,7 +5,7 @@ import { authenticateSession } from '../auth/common.js'
 import { V1Job } from '@kubernetes/client-node'
 import assert from 'node:assert'
 import { DEFAULT_NAMESPACE } from '../kubernetes/api.js'
-import { ForemanAnnotations } from '../annotations.js'
+import { ForemanAnnotations } from '../metadata.js'
 
 interface JobsItem {
   namespace: string
diff --git a/backend/src/controllers/job.ts b/backend/src/controllers/job.ts
index 82e5160..4994efd 100644
--- a/backend/src/controllers/job.ts
+++ b/backend/src/controllers/job.ts
@@ -3,6 +3,7 @@ import { KubernetesApi } from '../kubernetes/api.js'
 import assert from 'node:assert'
 import { StrongCache } from '../util/cache.js'
 import { V1CronJob, V1Job } from '@kubernetes/client-node'
+import { ForemanLabels } from '../metadata.js'
 
 export class JobController {
   private readonly cache = new StrongCache<V1Job[]>(5000)
@@ -23,9 +24,22 @@ export class JobController {
     assert.ok(namespace != null)
 
     return await this.cache.lazyCompute([namespace], async () => {
-      return await this.k8s.getJobs({
+      const jobs = await this.k8s.getJobs({
         namespace
-        // TODO ensure owner
+      })
+      // Ensure the job is associated with the cronjob
+      return jobs?.filter((job) => {
+        // manually-triggered jobs
+        if (job.metadata?.labels?.[ForemanLabels.CronJob] === cronJob.metadata?.name) {
+          return true
+        }
+        // direct owner reference
+        return job.metadata?.ownerReferences?.some((ref) => {
+          return ref.controller === true &&
+            ref.apiVersion === cronJob.apiVersion &&
+            ref.kind === cronJob.kind &&
+            ref.name === cronJob.metadata?.name
+        }) === true
       })
     })
   }
diff --git a/backend/src/controllers/trigger.ts b/backend/src/controllers/trigger.ts
index 5a09bc4..dd0ab1e 100644
--- a/backend/src/controllers/trigger.ts
+++ b/backend/src/controllers/trigger.ts
@@ -3,7 +3,7 @@ import { CronJobController } from './cronjob.js'
 import assert from 'node:assert'
 import { randomBytes } from 'node:crypto'
 import { V1Job } from '@kubernetes/client-node'
-import { ForemanAnnotations } from '../annotations.js'
+import { ForemanAnnotations, ForemanLabels } from '../metadata.js'
 import { JobController } from './job.js'
 
 export class TriggerController {
@@ -40,10 +40,15 @@ export class TriggerController {
       annotations[ForemanAnnotations.DebugLogging] = 'true'
     }
 
+    // Ensure we can attribute the job to the cronjob later
+    const labels: Record<string, string> = {}
+    labels[ForemanLabels.CronJob] = cronJob.metadata.name
+
     const result = await this.k8s.triggerCronJob({
       cronJob,
       jobName: `${cronJob.metadata.name}-foreman-${randomId()}`,
       annotations,
+      labels,
       env
     })
 
diff --git a/backend/src/kubernetes/api.ts b/backend/src/kubernetes/api.ts
index 277d4e2..f86d96d 100644
--- a/backend/src/kubernetes/api.ts
+++ b/backend/src/kubernetes/api.ts
@@ -93,6 +93,7 @@ export class KubernetesApi {
     cronJob: V1CronJob
     jobName: string
     annotations?: Record<string, string>
+    labels?: Record<string, string>
     env: Record<string, string>
   }): Promise<V1Job> {
     assert.ok(options.cronJob.metadata?.namespace != null)
@@ -110,6 +111,7 @@ export class KubernetesApi {
     let jobBody = options.cronJob.spec.jobTemplate
     jobBody = applyMetadataName(jobBody, options.jobName)
     jobBody = applyMetadataAnnotations(jobBody, options.annotations ?? {})
+    jobBody = applyMetadataLabels(jobBody, options.labels ?? {})
     jobBody = applyEnvToJobContainers(jobBody, options.env)
     // Cleanup job after 1 hour
     jobBody = applyTtl(jobBody, 60 * 60)
@@ -133,6 +135,19 @@ function applyMetadataAnnotations (jobBody: k8s.V1Job, annotations: Record<strin
   }
 }
 
+function applyMetadataLabels (jobBody: k8s.V1Job, labels: Record<string, string>): k8s.V1Job {
+  return {
+    ...jobBody,
+    metadata: {
+      ...jobBody.metadata,
+      labels: {
+        ...jobBody.metadata?.labels,
+        ...labels
+      }
+    }
+  }
+}
+
 function applyMetadataName (jobBody: k8s.V1Job, name: string): k8s.V1Job {
   return {
     ...jobBody,
diff --git a/backend/src/annotations.ts b/backend/src/metadata.ts
similarity index 76%
rename from backend/src/annotations.ts
rename to backend/src/metadata.ts
index 0eed6d7..e4d0b82 100644
--- a/backend/src/annotations.ts
+++ b/backend/src/metadata.ts
@@ -4,3 +4,7 @@ export enum ForemanAnnotations {
   RepositoryScope = 'foreman.contane.net/repository-scope',
   DebugLogging = 'foreman.contane.net/debug-logging'
 }
+
+export enum ForemanLabels {
+  CronJob = 'foreman.contane.net/cronjob'
+}