NO-JIRA: fix default namespace pull secret patch when imagePullSecrets is missing

[kchawlani19]

Problem

Creating a default pull secret in a newly created namespace can fail with an invalid ServiceAccount patch request.

Root cause

The submit flow always attempted to append using JSON patch path /imagePullSecrets/- on the default ServiceAccount.
For new namespaces, imagePullSecrets may not exist yet, so append-to-array fails.

Solution

• Fetch the latest default ServiceAccount during submit.
• Choose patch strategy based on live field presence:
  • If imagePullSecrets exists: append to /imagePullSecrets/-
  • If missing: initialize /imagePullSecrets with the new secret entry
• Add focused tests for:
  • patch selection helper behavior
  • submit-flow call sequencing (k8sCreate -> k8sGet -> k8sPatchByName)
  • expected k8sGet call arguments
  • async close assertion stability

Testing

• node .yarn/releases/yarn-4.14.1.cjs test public/components/modals/__tests__/configure-ns-pull-secret-modal.spec.tsx public/components/__tests__/namespace.spec.tsx --runInBand
• node .yarn/releases/yarn-4.14.1.cjs eslint public/components/modals/configure-ns-pull-secret-modal.tsx public/components/modals/__tests__/configure-ns-pull-secret-modal.spec.tsx

Notes

• Existing non-blocking console warnings appear during Jest startup in this environment, but test suites pass.

Summary by CodeRabbit

• Bug Fixes

  • Fixed namespace pull secret configuration when the default service account does not already define image pull secrets.
  • Pull secrets are now correctly added whether the service account has an existing image pull secret list or not.
  • Improved successful submission handling by ensuring the configuration modal closes after the service account is updated.

• Tests

  • Added coverage for pull secret patch generation and submission behavior.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

[openshift-ci-robot]

@kchawlani19: This pull request explicitly references no jira issue.

Details

In response to this:

Problem

Creating a default pull secret in a newly created namespace can fail with an invalid ServiceAccount patch request.

Root cause

The submit flow always attempted to append using JSON patch path /imagePullSecrets/- on the default ServiceAccount.
For new namespaces, imagePullSecrets may not exist yet, so append-to-array fails.

Solution

• Fetch the latest default ServiceAccount during submit.
• Choose patch strategy based on live field presence:
• If imagePullSecrets exists: append to /imagePullSecrets/-
• If missing: initialize /imagePullSecrets with the new secret entry
• Add focused tests for:
• patch selection helper behavior
• submit-flow call sequencing (k8sCreate -> k8sGet -> k8sPatchByName)
• expected k8sGet call arguments
• async close assertion stability

Testing

• node .yarn/releases/yarn-4.14.1.cjs test public/components/modals/__tests__/configure-ns-pull-secret-modal.spec.tsx public/components/__tests__/namespace.spec.tsx --runInBand
• node .yarn/releases/yarn-4.14.1.cjs eslint public/components/modals/configure-ns-pull-secret-modal.tsx public/components/modals/__tests__/configure-ns-pull-secret-modal.spec.tsx

Notes

• Existing non-blocking console warnings appear during Jest startup in this environment, but test suites pass.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: kchawlani19
Once this PR has been reviewed and has the lgtm label, please assign rawagner for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• frontend/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Walkthrough

The modal now creates a namespace pull secret, fetches the default service account, and applies a conditional JSON patch depending on whether imagePullSecrets exists. Tests cover patch generation, Kubernetes call ordering, payloads, and overlay closure.

Changes

Namespace pull secret configuration

Layer / File(s)	Summary
Conditional service account patch frontend/public/components/modals/configure-ns-pull-secret-modal.tsx, frontend/public/components/modals/__tests__/configure-ns-pull-secret-modal.spec.tsx	Adds getDefaultServiceAccountPatch and tests patches for missing and existing imagePullSecrets.
Submit workflow integration frontend/public/components/modals/configure-ns-pull-secret-modal.tsx, frontend/public/components/modals/__tests__/configure-ns-pull-secret-modal.spec.tsx	Fetches the default service account after secret creation, applies the computed patch, verifies call ordering, and closes the overlay after success.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Modal
  participant KubernetesAPI
  participant Overlay
  Modal->>KubernetesAPI: Create pull secret
  Modal->>KubernetesAPI: Fetch default ServiceAccount
  Modal->>KubernetesAPI: Apply conditional JSON patch
  Modal->>Overlay: Close overlay

Loading

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately summarizes the main fix for missing imagePullSecrets on the default namespace ServiceAccount.
Description check	✅ Passed	The description covers the problem, root cause, solution, and testing, with only some template sections like screenshots and browser info omitted.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	All new test titles are static, descriptive strings; none include dynamic or generated values.
Test Structure And Quality	✅ Passed	PASS: These are Jest/RTL unit tests, not Ginkgo cluster tests; they use beforeEach for shared setup and have no cluster resources or Eventually waits to police.
Microshift Test Compatibility	✅ Passed	No new Ginkgo/e2e tests were added; the PR only changes frontend modal code and a Jest spec, with no MicroShift-relevant OpenShift APIs.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	PASS: The PR adds only a Jest/RTL frontend unit test and modal code; no Ginkgo e2e tests or SNO-sensitive multi-node assumptions are present.
Topology-Aware Scheduling Compatibility	✅ Passed	Only frontend modal/test code changed; no manifests, controllers, or scheduling/topology constraints were introduced.
Ote Binary Stdout Contract	✅ Passed	No stdout writes were added in process-level code; the changes are limited to a React modal and Jest tests, with no console/klog/fmt stdout calls.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	Only frontend TSX unit tests changed; no Ginkgo e2e tests, IPv4 assumptions, or external connectivity requirements were introduced.
No-Weak-Crypto	✅ Passed	No weak ciphers, custom crypto, or secret/token comparisons were added; the change only uses Base64 encoding and Kubernetes patch logic.
Container-Privileges	✅ Passed	PR only changes frontend TSX/tests; no container/K8s manifests or privileged settings were added.
No-Sensitive-Data-In-Logs	✅ Passed	No new console/logger calls or other sensitive-data output were added in the touched modal or test files.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

frontend/public/components/modals/configure-ns-pull-secret-modal.tsx (1)

157-160: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Prefer native checks over Lodash for improved robustness.

Using Array.isArray provides a stronger guarantee that imagePullSecrets is actually an array before computing a patch that appends to it (/-). This avoids potential 422 Unprocessable Entity API errors in the rare event that the field is explicitly set to null, and it reduces reliance on Lodash.

♻️ Proposed refactor

           const defaultServiceAccountPatch = getDefaultServiceAccountPatch(
-            _.has(serviceAccount, 'imagePullSecrets'),
+            Array.isArray(serviceAccount?.imagePullSecrets),
             pullSecretName,
           );

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/public/components/modals/configure-ns-pull-secret-modal.tsx` around
lines 157 - 160, Replace the Lodash _.has check in the
defaultServiceAccountPatch computation with an Array.isArray check on
serviceAccount.imagePullSecrets, so the append patch is generated only when the
field is an actual array and not when it is null or another non-array value.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@frontend/public/components/modals/configure-ns-pull-secret-modal.tsx`:
- Around line 157-160: Replace the Lodash _.has check in the
defaultServiceAccountPatch computation with an Array.isArray check on
serviceAccount.imagePullSecrets, so the append patch is generated only when the
field is an actual array and not when it is null or another non-array value.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 90304646-0cd6-4970-afe6-b75674fd8b35

📥 Commits

Reviewing files that changed from the base of the PR and between 6d0dcf1 and 3a5ad0f.

📒 Files selected for processing (2)

• frontend/public/components/modals/__tests__/configure-ns-pull-secret-modal.spec.tsx
• frontend/public/components/modals/configure-ns-pull-secret-modal.tsx

[openshift-ci]

@kchawlani19: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/backend	3a5ad0f	link	true	/test backend

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.