fix(x2a): job secrets to be owned by the job (#2710)

Currently the secret is not ownwed by the job, so it was not deleted
when the Job was GC.

This align secrets with the jobs.

FIX FLPATH-3555

Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

Author: eloycoto

workspaces/x2a/plugins/x2a-backend/src/services/JobResourceBuilder.test.ts

@@ -405,13 +405,21 @@ describe('JobResourceBuilder', () => {
         branch: 'main',
       },
     };
+    const ownerReference = {
+      apiVersion: 'batch/v1',
+      kind: 'Job',
+      name: 'job-x2a-init-abc123',
+      uid: 'test-uid-123',
+      blockOwnerDeletion: true,
+    };
 
     it('should include source repository credentials', () => {
       const secret = JobResourceBuilder.buildJobSecret(
         jobId,
         projectId,
         phase,
         gitCredentials,
+        ownerReference,
       );
 
       expect(secret.stringData).toMatchObject({
@@ -427,6 +435,7 @@ describe('JobResourceBuilder', () => {
         projectId,
         phase,
         gitCredentials,
+        ownerReference,
       );
 
       expect(secret.stringData).toMatchObject({
@@ -442,6 +451,7 @@ describe('JobResourceBuilder', () => {
         projectId,
         phase,
         gitCredentials,
+        ownerReference,
       );
 
       // Job secret should only have Git credentials
@@ -457,6 +467,7 @@ describe('JobResourceBuilder', () => {
         projectId,
         phase,
         gitCredentials,
+        ownerReference,
       );
 
       expect(secret.metadata?.labels).toMatchObject({
@@ -475,6 +486,7 @@ describe('JobResourceBuilder', () => {
         projectId,
         phase,
         gitCredentials,
+        ownerReference,
       );
 
       expect(secret.metadata?.name).toBe(`x2a-job-secret-${phase}-${jobId}`);
@@ -486,6 +498,7 @@ describe('JobResourceBuilder', () => {
         projectId,
         phase,
         gitCredentials,
+        ownerReference,
       );
 
       expect(secret.metadata?.annotations).toMatchObject({
@@ -495,12 +508,25 @@ describe('JobResourceBuilder', () => {
       });
     });
 
+    it('should include ownerReferences to the parent Job', () => {
+      const secret = JobResourceBuilder.buildJobSecret(
+        jobId,
+        projectId,
+        phase,
+        gitCredentials,
+        ownerReference,
+      );
+
+      expect(secret.metadata?.ownerReferences).toEqual([ownerReference]);
+    });
+
     it('should be Opaque secret type', () => {
       const secret = JobResourceBuilder.buildJobSecret(
         jobId,
         projectId,
         phase,
         gitCredentials,
+        ownerReference,
       );
 
       expect(secret.type).toBe('Opaque');

workspaces/x2a/plugins/x2a-backend/src/services/JobResourceBuilder.ts

@@ -15,7 +15,7 @@
  */
 
 import { resolvePackagePath } from '@backstage/backend-plugin-api';
-import { V1Job, V1Secret } from '@kubernetes/client-node';
+import { V1Job, V1OwnerReference, V1Secret } from '@kubernetes/client-node';
 import crypto from 'node:crypto';
 import fs from 'node:fs';
 import { X2AConfig } from '../../config';
@@ -137,13 +137,14 @@ export class JobResourceBuilder {
   /**
    * Builds a Kubernetes Secret for a specific job containing ephemeral Git credentials
    *
-   * Note: ownerReferences are NOT set here because the job doesn't exist yet at secret
-   * creation time. After job creation, KubeService.createJob() sets the ownerReference
-   * on this secret so it is automatically garbage-collected when the Job is deleted.
+   * The ownerReference links this secret to the Job so it is automatically
+   * garbage-collected when the Job is deleted by the TTL controller.
    *
    * @param jobId - The job UUID
    * @param projectId - The project UUID
+   * @param phase - The migration phase
    * @param gitCredentials - Git repository credentials from the user
+   * @param ownerReference - Owner reference to the parent Job for garbage collection
    * @returns V1Secret resource ready to be created in Kubernetes
    */
   static buildJobSecret(
@@ -154,6 +155,7 @@ export class JobResourceBuilder {
       sourceRepo: GitRepo;
       targetRepo: GitRepo;
     },
+    ownerReference: V1OwnerReference,
   ): V1Secret {
     const secretName = `x2a-job-secret-${phase}-${jobId}`;
 
@@ -175,6 +177,7 @@ export class JobResourceBuilder {
           'x2a.redhat.com/description':
             'Ephemeral Git credentials for X2A job (auto-deleted with job)',
         },
+        ownerReferences: [ownerReference],
       },
       type: 'Opaque',
       stringData: {

workspaces/x2a/plugins/x2a-backend/src/services/KubeService.test.ts

@@ -22,7 +22,6 @@ import { KubeService } from './KubeService';
 const mockCoreV1Api = {
   createNamespacedSecret: jest.fn(),
   readNamespacedSecret: jest.fn(),
-  replaceNamespacedSecret: jest.fn(),
   deleteNamespacedSecret: jest.fn(),
   listNamespacedPod: jest.fn(),
   readNamespacedPodLog: jest.fn(),
@@ -252,19 +251,9 @@ describe('KubeService', () => {
     beforeEach(() => {
       // Mock createProjectSecret and createJobSecret to succeed
       mockCoreV1Api.createNamespacedSecret.mockResolvedValue({});
-      // Mock readNamespacedSecret and replaceNamespacedSecret for ownerReference patching
-      mockCoreV1Api.readNamespacedSecret.mockResolvedValue({
-        apiVersion: 'v1',
-        kind: 'Secret',
-        metadata: {
-          name: 'x2a-job-secret-init-job-123',
-          namespace: 'test-namespace',
-        },
-      });
-      mockCoreV1Api.replaceNamespacedSecret.mockResolvedValue({});
     });
 
-    it('should create both project and job secrets before creating job', async () => {
+    it('should create project secret before job, and job secret after job with ownerReference', async () => {
       mockBatchV1Api.createNamespacedJob.mockResolvedValue({
         metadata: { name: 'job-x2a-init-abc123', uid: 'uid-123' },
       });
@@ -283,13 +272,22 @@ describe('KubeService', () => {
         }),
       );
 
-      // Should create job secret (Git credentials)
+      // Should create job secret (Git credentials) with ownerReference to the job
       expect(mockCoreV1Api.createNamespacedSecret).toHaveBeenCalledWith(
         expect.objectContaining({
           namespace: 'test-namespace',
           body: expect.objectContaining({
             metadata: expect.objectContaining({
               name: 'x2a-job-secret-init-job-123',
+              ownerReferences: [
+                expect.objectContaining({
+                  apiVersion: 'batch/v1',
+                  kind: 'Job',
+                  name: expect.stringMatching(/^job-x2a-init-/),
+                  uid: 'uid-123',
+                  blockOwnerDeletion: true,
+                }),
+              ],
             }),
             stringData: expect.objectContaining({
               SOURCE_REPO_URL: 'https://github.com/org/source',
@@ -342,51 +340,65 @@ describe('KubeService', () => {
       expect(result.k8sJobName).toBeDefined();
     });
 
-    it('should set ownerReference on job secret after job creation', async () => {
+    it('should create job secret with ownerReference containing job uid', async () => {
       mockBatchV1Api.createNamespacedJob.mockResolvedValue({
         metadata: { name: 'job-x2a-init-abc123', uid: 'uid-456' },
       });
 
       await kubeService.createJob(params);
 
-      // Should read the job secret
-      expect(mockCoreV1Api.readNamespacedSecret).toHaveBeenCalledWith({
-        name: 'x2a-job-secret-init-job-123',
-        namespace: 'test-namespace',
-      });
+      // Job secret should be the second createNamespacedSecret call (after project secret)
+      const calls = mockCoreV1Api.createNamespacedSecret.mock.calls;
+      const jobSecretCall = calls.find(
+        (c: any) => c[0].body.metadata?.name === 'x2a-job-secret-init-job-123',
+      );
+      expect(jobSecretCall).toBeDefined();
 
-      // Should replace the secret with ownerReference set
-      expect(mockCoreV1Api.replaceNamespacedSecret).toHaveBeenCalled();
-      const replaceCall =
-        mockCoreV1Api.replaceNamespacedSecret.mock.calls[0][0];
-      expect(replaceCall.name).toBe('x2a-job-secret-init-job-123');
-      expect(replaceCall.namespace).toBe('test-namespace');
-      const ownerRefs = replaceCall.body.metadata.ownerReferences;
+      const ownerRefs = jobSecretCall![0].body.metadata.ownerReferences;
       expect(ownerRefs).toHaveLength(1);
-      expect(ownerRefs[0]).toEqual(
+      expect(ownerRefs[0]).toEqual({
+        apiVersion: 'batch/v1',
+        kind: 'Job',
+        name: expect.stringMatching(/^job-x2a-init-/),
+        uid: 'uid-456',
+        blockOwnerDeletion: true,
+      });
+    });
+
+    it('should delete job and throw if job secret creation fails', async () => {
+      mockBatchV1Api.createNamespacedJob.mockResolvedValue({
+        metadata: { name: 'job-x2a-init-abc123', uid: 'uid-456' },
+      });
+      mockBatchV1Api.deleteNamespacedJob.mockResolvedValue({});
+      // First call succeeds (project secret), second fails (job secret)
+      mockCoreV1Api.createNamespacedSecret
+        .mockResolvedValueOnce({})
+        .mockRejectedValueOnce(new Error('Secret creation failed'));
+
+      await expect(kubeService.createJob(params)).rejects.toThrow(
+        'Secret creation failed',
+      );
+
+      // Should clean up the orphaned job
+      expect(mockBatchV1Api.deleteNamespacedJob).toHaveBeenCalledWith(
         expect.objectContaining({
-          apiVersion: 'batch/v1',
-          kind: 'Job',
-          uid: 'uid-456',
-          blockOwnerDeletion: true,
+          namespace: 'test-namespace',
         }),
       );
-      expect(ownerRefs[0].name).toMatch(/^job-x2a-init-/);
     });
 
-    it('should succeed even if ownerReference patching fails', async () => {
+    it('should delete job and throw if created job has no UID', async () => {
       mockBatchV1Api.createNamespacedJob.mockResolvedValue({
-        metadata: { name: 'job-x2a-init-abc123', uid: 'uid-456' },
+        metadata: { name: 'job-x2a-init-abc123' },
       });
-      mockCoreV1Api.readNamespacedSecret.mockRejectedValue(
-        new Error('Secret read failed'),
-      );
+      mockBatchV1Api.deleteNamespacedJob.mockResolvedValue({});
 
-      const result = await kubeService.createJob(params);
+      await expect(kubeService.createJob(params)).rejects.toThrow(
+        /created without UID/,
+      );
 
-      // Job creation should still succeed
-      expect(result.k8sJobName).toBeDefined();
-      expect(mockBatchV1Api.createNamespacedJob).toHaveBeenCalled();
+      // Should clean up the orphaned job
+      expect(mockBatchV1Api.deleteNamespacedJob).toHaveBeenCalled();
     });
   });