Skip to content

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

Merged
mareklibra merged 1 commit intoredhat-developer:mainfrom
eloycoto:SecretsNotowned
Apr 7, 2026
Merged

fix(x2a): job secrets to be owned by the job#2710
mareklibra merged 1 commit intoredhat-developer:mainfrom
eloycoto:SecretsNotowned

Conversation

@eloycoto
Copy link
Copy Markdown
Contributor

@eloycoto eloycoto commented Apr 7, 2026

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.

$ -> kubectl get secrets x2a-job-secret-init-97a86ab7-9d5d-4249-9c46-9c611f29c963  -o json | jq .metadata
{
  "annotations": {
    "x2a.redhat.com/created-by": "x2a-backend-plugin",
    "x2a.redhat.com/description": "Ephemeral Git credentials for X2A job (auto-deleted with job)"
  },
  "creationTimestamp": "2026-04-07T11:35:30Z",
  "labels": {
    "app.kubernetes.io/component": "credentials",
    "app.kubernetes.io/managed-by": "x2a-backend-plugin",
    "app.kubernetes.io/name": "x2a-job-secret",
    "x2a.redhat.com/job-id": "97a86ab7-9d5d-4249-9c46-9c611f29c963",
    "x2a.redhat.com/project-id": "8be1d104-1698-4717-9d1c-51ad3bf740ae",
    "x2a.redhat.com/secret-type": "job"
  },
  "name": "x2a-job-secret-init-97a86ab7-9d5d-4249-9c46-9c611f29c963",
  "namespace": "default",
  "ownerReferences": [
    {
      "apiVersion": "batch/v1",
      "blockOwnerDeletion": true,
      "kind": "Job",
      "name": "job-x2a-init-7905abd6",
      "uid": "90c013e8-25f9-458c-9410-d35f55879926"
    }
  ],
  "resourceVersion": "3500",
  "uid": "a47b27e8-c52d-4354-85d6-b840df45833b"
}

FIX FLPATH-3555

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 7, 2026

This pull request adds a new top-level directory under workspaces/. Please follow Submitting a Pull Request for a New Workspace in CONTRIBUTING.md.

@rhdh-gh-app
Copy link
Copy Markdown

rhdh-gh-app Bot commented Apr 7, 2026

Missing Changesets

The following package(s) are changed by this PR but do not have a changeset:

  • @red-hat-developer-hub/backstage-plugin-x2a-backend

See CONTRIBUTING.md for more information about how to add changesets.

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-x2a-backend workspaces/x2a/plugins/x2a-backend none v1.3.0

@rhdh-qodo-merge
Copy link
Copy Markdown

Review Summary by Qodo

Set job secret ownerReference at creation time for proper garbage collection

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Set ownerReference on job secrets at creation time
• Eliminates post-creation patching of secrets via read-replace
• Ensures secrets are garbage-collected with their parent Job
• Simplifies KubeService by removing setJobSecretOwnerReference method
Diagram
flowchart LR
  A["Job Creation Flow"] --> B["Create Project Secret"]
  B --> C["Create Job"]
  C --> D["Build OwnerReference"]
  D --> E["Create Job Secret with OwnerReference"]
  E --> F["Secret Auto-Deleted with Job"]
Loading

Grey Divider

File Changes

1. workspaces/x2a/plugins/x2a-backend/src/services/JobResourceBuilder.ts ✨ Enhancement +7/-4

Add ownerReference parameter to buildJobSecret method

• Added V1OwnerReference import from Kubernetes client
• Updated buildJobSecret method signature to accept ownerReference parameter
• Set ownerReferences array in secret metadata during creation
• Updated JSDoc to reflect that ownerReference is now set at creation time

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


2. workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts 🐞 Bug fix +27/-66

Refactor job creation to set ownerReference at secret creation

• Added V1OwnerReference import from Kubernetes client types
• Updated createJobSecret method to accept ownerReference parameter
• Reordered job creation flow: create project secret, then job, then job secret
• Removed setJobSecretOwnerReference private method that patched secrets post-creation
• Updated JSDoc comments to reflect new ownership model

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


3. workspaces/x2a/plugins/x2a-backend/src/services/KubeService.test.ts 🧪 Tests +34/-44

Update tests to verify ownerReference at creation time

• Removed mock for replaceNamespacedSecret API call
• Updated test to verify ownerReference is set during secret creation
• Removed mocks for readNamespacedSecret that were used for patching
• Changed test expectations to validate ownerReference in createNamespacedSecret call
• Replaced test for post-creation patching with test for creation-time ownership
• Updated test to verify job secret creation fails if secret creation fails

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


Grey Divider

Qodo Logo

@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge Bot commented Apr 7, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX Issues (0)

Grey Divider


Action required

1. Empty ownerReference UID 🐞 Bug ≡ Correctness
Description
KubeService.createJob builds an ownerReference with uid: createdJob.metadata?.uid || '', so a
missing UID becomes an empty string, which can make the Secret invalid and prevent the Secret from
being linked to the Job for GC.
Code

workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[R277-283]

+      const ownerReference: V1OwnerReference = {
+        apiVersion: 'batch/v1',
+        kind: 'Job',
+        name: k8sJobName,
+        uid: createdJob.metadata?.uid || '',
+        blockOwnerDeletion: true,
+      };
Evidence
createJob constructs an ownerReference that can contain an empty UID and passes it into
createJobSecret, and buildJobSecret unconditionally attaches that ownerReference to the Secret.
If the UID is empty/undefined, the created Secret is not a valid ownership link (and may be
rejected).

workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[268-295]
workspaces/x2a/plugins/x2a-backend/src/services/JobResourceBuilder.ts[150-194]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`createJob` builds a Secret ownerReference with `uid` defaulting to an empty string. Owner references must use the real Job UID; an empty UID can cause Secret creation to fail or break garbage-collection linkage.

### Issue Context
The ownerReference is now set at Secret creation time and is always applied by `JobResourceBuilder.buildJobSecret`, so any invalid ownerReference immediately affects Job creation flow.

### Fix Focus Areas
- workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[268-295]
- workspaces/x2a/plugins/x2a-backend/src/services/JobResourceBuilder.ts[150-194]

### What to change
- Remove the `|| ''` fallback.
- Validate `createdJob.metadata?.uid` is present and non-empty before constructing `ownerReference`.
- If UID is missing, throw a clear error (and consider deleting the just-created Job before throwing; see next finding).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Orphan job on secret failure 🐞 Bug ☼ Reliability
Description
createJob creates the Kubernetes Job before creating the job Secret; if job-secret creation fails,
createJob throws without deleting the already-created K8s Job, and the DB job stays pending
without k8sJobName so reconciliation never runs.
Code

workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[R272-297]

      });
      this.#logger.info(`Created job: ${k8sJobName}`);

-      // Set ownerReference on the job secret so it is garbage-collected when the Job is deleted
-      const jobUid = createdJob.metadata?.uid;
-      if (jobUid) {
-        await this.setJobSecretOwnerReference(
-          params.jobId,
-          params.phase,
-          k8sJobName,
-          jobUid,
-        );
-      }
+      // Step 3: Create ephemeral job secret owned by the job
+      // The pod will wait for the secret to appear before starting
+      const ownerReference: V1OwnerReference = {
+        apiVersion: 'batch/v1',
+        kind: 'Job',
+        name: k8sJobName,
+        uid: createdJob.metadata?.uid || '',
+        blockOwnerDeletion: true,
+      };
+
+      await this.createJobSecret(
+        params.jobId,
+        params.projectId,
+        params.phase,
+        {
+          sourceRepo: params.sourceRepo,
+          targetRepo: params.targetRepo,
+        },
+        ownerReference,
+      );

      return { k8sJobName };
    } catch (error: any) {
Evidence
createJob creates the K8s Job and then calls createJobSecret; failures in createJobSecret are
rethrown by the outer catch with no cleanup. The Job pod template imports env from the job secret,
so if the secret is missing the pod cannot start. Separately, routers only persist k8sJobName
after kubeService.createJob returns successfully, and reconcileJobStatus exits early when
k8sJobName is absent—so after this failure, the DB job remains active forever and blocks future
job submissions.

workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[256-301]
workspaces/x2a/plugins/x2a-backend/src/services/JobResourceBuilder.ts[204-288]
workspaces/x2a/plugins/x2a-backend/src/router/projects.ts[332-376]
workspaces/x2a/plugins/x2a-backend/src/router/modules.ts[311-377]
workspaces/x2a/plugins/x2a-backend/src/router/common.ts[296-305]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
After creating the K8s Job, `createJob` creates the job Secret. If Secret creation fails, `createJob` throws without deleting the K8s Job. This can leave an orphaned Job whose pod references the missing job Secret, and it also leaves a DB job record stuck in `pending` without `k8sJobName` (so reconciliation never runs).

### Issue Context
- The job pod template uses `envFrom.secretRef` for the job secret name.
- Routers update DB with `k8sJobName` only after `kubeService.createJob` succeeds.
- `reconcileJobStatus` no-ops when `k8sJobName` is missing.

### Fix Focus Areas
- workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[256-301]
- workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[411-429]
- workspaces/x2a/plugins/x2a-backend/src/router/projects.ts[332-376]
- workspaces/x2a/plugins/x2a-backend/src/router/modules.ts[311-377]
- workspaces/x2a/plugins/x2a-backend/src/router/common.ts[296-305]

### What to change
1) In `KubeService.createJob`, wrap Step 3 (job secret creation) in its own try/catch; on failure:
  - Attempt `await this.deleteJob(k8sJobName)` (best-effort; log warn on cleanup failure).
  - Then rethrow the original error.
2) In router handlers calling `kubeService.createJob`, catch errors and update the DB job status to `error` (or delete the DB record) so the system doesn’t permanently block new jobs when K8s creation fails.
3) (Optional hardening) Consider persisting `k8sJobName` immediately after Job creation (before secret creation) so reconciliation/cancellation paths can still find and clean up the Job.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment thread workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts Outdated
Comment thread workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts Outdated
@eloycoto eloycoto marked this pull request as draft April 7, 2026 11:48
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>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Apr 7, 2026

@eloycoto eloycoto marked this pull request as ready for review April 7, 2026 13:00
@rhdh-qodo-merge
Copy link
Copy Markdown

Review Summary by Qodo

Set ownerReferences on job secrets at creation time

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Job secrets now owned by parent Job for proper garbage collection
• Moved ownerReference assignment from post-creation patching to secret creation
• Reordered job creation workflow to create job before secret
• Improved error handling with job cleanup on secret creation failure
Diagram
flowchart LR
  A["Create Project Secret"] --> B["Create Job"]
  B --> C["Extract Job UID"]
  C --> D["Create Job Secret with ownerReference"]
  D --> E["Job owns Secret for GC"]
  F["Secret Creation Fails"] --> G["Delete Orphaned Job"]
Loading

Grey Divider

File Changes

1. workspaces/x2a/plugins/x2a-backend/src/services/JobResourceBuilder.ts ✨ Enhancement +7/-4

Add ownerReference parameter to buildJobSecret

• Added V1OwnerReference import from Kubernetes client
• Updated buildJobSecret() method signature to accept ownerReference parameter
• Set ownerReferences array in secret metadata during creation
• Updated JSDoc to reflect that ownerReference is now set at creation time

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


2. workspaces/x2a/plugins/x2a-backend/src/services/JobResourceBuilder.test.ts 🧪 Tests +26/-0

Add ownerReference tests to buildJobSecret

• Added ownerReference test fixture with Job reference details
• Updated all buildJobSecret() test calls to include ownerReference parameter
• Added new test case verifying ownerReferences are set in secret metadata

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


3. workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts 🐞 Bug fix +41/-68

Refactor job creation to set ownerReference upfront

• Added V1OwnerReference import from Kubernetes client
• Updated createJobSecret() method signature to accept ownerReference parameter
• Reordered job creation workflow: create job first, then secret with ownerReference
• Replaced post-creation patching logic with upfront ownerReference assignment
• Added error handling to delete orphaned job if secret creation fails
• Added validation to ensure created job has a UID before proceeding
• Removed setJobSecretOwnerReference() private method (no longer needed)

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


View more (1)
4. workspaces/x2a/plugins/x2a-backend/src/services/KubeService.test.ts 🧪 Tests +53/-41

Update tests for upfront ownerReference assignment

• Removed replaceNamespacedSecret mock (no longer used)
• Removed readNamespacedSecret mock setup (no longer needed)
• Updated test expectations to verify ownerReferences in job secret creation call
• Replaced post-creation patching tests with upfront ownerReference verification
• Added test for job cleanup when secret creation fails
• Added test for job cleanup when created job lacks UID

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


Grey Divider

Qodo Logo

@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge Bot commented Apr 7, 2026

Code Review by Qodo

🐞 Bugs (5) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX Issues (0)

Grey Divider


Action required

1. Empty ownerReference UID 🐞 Bug ≡ Correctness
Description
KubeService.createJob builds an ownerReference with uid: createdJob.metadata?.uid || '', so a
missing UID becomes an empty string, which can make the Secret invalid and prevent the Secret from
being linked to the Job for GC.
Code

workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[R277-283]

+      const ownerReference: V1OwnerReference = {
+        apiVersion: 'batch/v1',
+        kind: 'Job',
+        name: k8sJobName,
+        uid: createdJob.metadata?.uid || '',
+        blockOwnerDeletion: true,
+      };
Evidence
createJob constructs an ownerReference that can contain an empty UID and passes it into
createJobSecret, and buildJobSecret unconditionally attaches that ownerReference to the Secret.
If the UID is empty/undefined, the created Secret is not a valid ownership link (and may be
rejected).

workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[268-295]
workspaces/x2a/plugins/x2a-backend/src/services/JobResourceBuilder.ts[150-194]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`createJob` builds a Secret ownerReference with `uid` defaulting to an empty string. Owner references must use the real Job UID; an empty UID can cause Secret creation to fail or break garbage-collection linkage.

### Issue Context
The ownerReference is now set at Secret creation time and is always applied by `JobResourceBuilder.buildJobSecret`, so any invalid ownerReference immediately affects Job creation flow.

### Fix Focus Areas
- workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[268-295]
- workspaces/x2a/plugins/x2a-backend/src/services/JobResourceBuilder.ts[150-194]

### What to change
- Remove the `|| ''` fallback.
- Validate `createdJob.metadata?.uid` is present and non-empty before constructing `ownerReference`.
- If UID is missing, throw a clear error (and consider deleting the just-created Job before throwing; see next finding).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Orphan job on secret failure 🐞 Bug ☼ Reliability
Description
createJob creates the Kubernetes Job before creating the job Secret; if job-secret creation fails,
createJob throws without deleting the already-created K8s Job, and the DB job stays pending
without k8sJobName so reconciliation never runs.
Code

workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[R272-297]

      });
      this.#logger.info(`Created job: ${k8sJobName}`);

-      // Set ownerReference on the job secret so it is garbage-collected when the Job is deleted
-      const jobUid = createdJob.metadata?.uid;
-      if (jobUid) {
-        await this.setJobSecretOwnerReference(
-          params.jobId,
-          params.phase,
-          k8sJobName,
-          jobUid,
-        );
-      }
+      // Step 3: Create ephemeral job secret owned by the job
+      // The pod will wait for the secret to appear before starting
+      const ownerReference: V1OwnerReference = {
+        apiVersion: 'batch/v1',
+        kind: 'Job',
+        name: k8sJobName,
+        uid: createdJob.metadata?.uid || '',
+        blockOwnerDeletion: true,
+      };
+
+      await this.createJobSecret(
+        params.jobId,
+        params.projectId,
+        params.phase,
+        {
+          sourceRepo: params.sourceRepo,
+          targetRepo: params.targetRepo,
+        },
+        ownerReference,
+      );

      return { k8sJobName };
    } catch (error: any) {
Evidence
createJob creates the K8s Job and then calls createJobSecret; failures in createJobSecret are
rethrown by the outer catch with no cleanup. The Job pod template imports env from the job secret,
so if the secret is missing the pod cannot start. Separately, routers only persist k8sJobName
after kubeService.createJob returns successfully, and reconcileJobStatus exits early when
k8sJobName is absent—so after this failure, the DB job remains active forever and blocks future
job submissions.

workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[256-301]
workspaces/x2a/plugins/x2a-backend/src/services/JobResourceBuilder.ts[204-288]
workspaces/x2a/plugins/x2a-backend/src/router/projects.ts[332-376]
workspaces/x2a/plugins/x2a-backend/src/router/modules.ts[311-377]
workspaces/x2a/plugins/x2a-backend/src/router/common.ts[296-305]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
After creating the K8s Job, `createJob` creates the job Secret. If Secret creation fails, `createJob` throws without deleting the K8s Job. This can leave an orphaned Job whose pod references the missing job Secret, and it also leaves a DB job record stuck in `pending` without `k8sJobName` (so reconciliation never runs).

### Issue Context
- The job pod template uses `envFrom.secretRef` for the job secret name.
- Routers update DB with `k8sJobName` only after `kubeService.createJob` succeeds.
- `reconcileJobStatus` no-ops when `k8sJobName` is missing.

### Fix Focus Areas
- workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[256-301]
- workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[411-429]
- workspaces/x2a/plugins/x2a-backend/src/router/projects.ts[332-376]
- workspaces/x2a/plugins/x2a-backend/src/router/modules.ts[311-377]
- workspaces/x2a/plugins/x2a-backend/src/router/common.ts[296-305]

### What to change
1) In `KubeService.createJob`, wrap Step 3 (job secret creation) in its own try/catch; on failure:
  - Attempt `await this.deleteJob(k8sJobName)` (best-effort; log warn on cleanup failure).
  - Then rethrow the original error.
2) In router handlers calling `kubeService.createJob`, catch errors and update the DB job status to `error` (or delete the DB record) so the system doesn’t permanently block new jobs when K8s creation fails.
3) (Optional hardening) Consider persisting `k8sJobName` immediately after Job creation (before secret creation) so reconciliation/cancellation paths can still find and clean up the Job.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Job create failures unlogged 🐞 Bug ✧ Quality ⭐ New
Description
KubeService.createJob() calls createNamespacedJob() without a local try/catch, so job-creation
failures can propagate without any KubeService error log at the failure point. This reduces
debuggability when RBAC/quota/validation errors prevent job creation.
Code

workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[R268-272]

+    const createdJob = await this.#batchV1Api.createNamespacedJob({
+      namespace: this.#namespace,
+      body: job,
+    });
+    this.#logger.info(`Created job: ${k8sJobName}`);
Evidence
The createJob() flow directly awaits createNamespacedJob() and only logs success afterward; there is
no error log in this function for job-creation failures (contrast with the secret-creation try/catch
which is later in the function).

workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[256-273]
workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[293-310]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`KubeService.createJob()` does not log errors if `createNamespacedJob()` fails. The function logs the start and logs success, but failures at the job-creation API call site can escape without an error log from this service.

### Issue Context
The PR refactored the flow to create the Job before creating the job secret (so the secret can include an ownerReference). In this refactor, the previous job-creation try/catch (and its error logging) was removed.

### Fix Focus Areas
- workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[264-273]

### Suggested change
Wrap `createNamespacedJob()` in a `try/catch` and log an error that clearly indicates job creation failed (include namespace and intended job name), then rethrow the original error.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Cleanup masks root error 🐞 Bug ☼ Reliability ⭐ New
Description
If job secret creation fails, createJob() awaits deleteJob(); if deleteJob() throws, that cleanup
error can replace the original secret-creation error and obscure the root cause. The same masking
risk exists in the "job created without UID" cleanup path.
Code

workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[R304-309]

    } catch (error: any) {
-      this.#logger.warn(
-        `Failed to set ownerReference on job secret ${jobSecretName}: ${error.message}. ` +
-          `The job will still run but the secret may not be auto-cleaned.`,
+      this.#logger.error(
+        `Failed to create job secret, cleaning up job ${k8sJobName}: ${error.message}`,
      );
+      await this.deleteJob(k8sJobName);
+      throw error;
Evidence
In the secret-creation catch block, await this.deleteJob(...) is unguarded; deleteJob() rethrows
on non-404 failures, which would prevent the code from reaching throw error and thus mask the
original failure. The missing-UID branch also awaits deleteJob() before throwing its own error,
with the same masking behavior if cleanup fails.

workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[274-281]
workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[293-310]
workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[423-440]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`createJob()` performs best-effort cleanup (`deleteJob`) when secret creation fails or when the created Job has no UID. If cleanup fails, the cleanup exception can replace the original failure, obscuring the root cause.

### Issue Context
This can make operational debugging significantly harder (operators see a delete failure rather than the secret-creation failure that caused cleanup). It can also lead to inconsistent error semantics across failure modes.

### Fix Focus Areas
- workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[274-281]
- workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[304-310]
- workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[423-440]

### Suggested change
Wrap `deleteJob(k8sJobName)` in its own try/catch in both cleanup paths:
- Log cleanup failure (include original error + cleanup error).
- Re-throw the *original* error (or throw an `AggregateError` / attach `cause`) so the primary failure is preserved.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

5. Weak job deletion assertion 🐞 Bug ⚙ Maintainability ⭐ New
Description
The new unit test only asserts deleteNamespacedJob() was called with the namespace, so regressions
in the job name or propagationPolicy arguments would not be caught. This reduces confidence that
cleanup behavior matches deleteJob().
Code

workspaces/x2a/plugins/x2a-backend/src/services/KubeService.test.ts[R382-387]

+      // 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-/);
Evidence
The test uses expect.objectContaining({ namespace: ... }) only, while production deleteJob()
always passes name and propagationPolicy: 'Background' as well; those fields are currently
unvalidated by the test.

workspaces/x2a/plugins/x2a-backend/src/services/KubeService.test.ts[368-388]
workspaces/x2a/plugins/x2a-backend/src/services/KubeService.ts[423-431]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`KubeService.test.ts` cleanup assertion is too loose; it doesn't verify the job name or propagation policy passed to `deleteNamespacedJob()`.

### Issue Context
`deleteJob()` always calls `deleteNamespacedJob({ name, namespace, propagationPolicy: 'Background' })`. The test should validate these important arguments to prevent silent regressions.

### Fix Focus Areas
- workspaces/x2a/plugins/x2a-backend/src/services/KubeService.test.ts[382-387]

### Suggested change
Update the expectation to assert at least:
- `name` equals the created job name (or matches the expected pattern)
- `namespace` equals `test-namespace`
- `propagationPolicy` equals `'Background'`

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@mareklibra mareklibra merged commit b6c1049 into redhat-developer:main Apr 7, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants