Added service account to dev branch

[NicholasYancey]

Why the changes were made

The CLI and VMDP download server pods on oadp-dev currently run with no ServiceAccount specified, defaulting to the namespace's default ServiceAccount. This fails Telco/CNF certification:

access-control-pod-service-account requires every pod to use a non-default ServiceAccount. This same fix was already backported directly to oadp-1.3 (#2310), oadp-1.4 (#2312), and oadp-1.5 (#2309), but was never to oadp-dev as that branch only picked up readiness/liveness probes via #2306. This PR closes that gap on oadp-dev, covering both the CLI and VMDP servers.

How to test the changes made

How to test the changes made

go test ./internal/controller/... -run 'CLIServer|VMDPServer' -v

1. Deploy the operator from this branch.
2. Check the ServiceAccounts exist:

oc get sa -n openshift-adp openshift-adp-cli-server openshift-adp-vmdp-server

3. Check the deployments use them (not default):

oc get deploy -n openshift-adp openshift-adp-oadp-cli-server openshift-adp-oadp-vmdp-server \
  -o jsonpath='{.spec.template.spec.serviceAccountName}{"\n"}'

Summary by CodeRabbit

• Security

  • CLI and VMDP server workloads now run with dedicated service accounts.
  • Service account token automount is explicitly disabled for improved security.

• Bug Fixes

  • Reconciliation now creates and enforces the dedicated service accounts (labels, owner reference, and automount setting).
  • Existing server deployments are backfilled with missing readiness/liveness probes and updated to use the correct service account.

• Tests

  • Added unit test coverage for building and reconciling both server service accounts and deployments, including drift correction and no-op behavior when already correct.

[openshift-ci-robot]

@NicholasYancey: This pull request references OADP-8448 which is a valid jira issue.

Details

In response to this:

Why the changes were made

The CLI and VMDP download server pods on oadp-dev currently run with no ServiceAccount specified, defaulting to the namespace's default ServiceAccount. This fails Telco/CNF certification:

access-control-pod-service-account requires every pod to use a non-default ServiceAccount. This same fix was already backported directly to oadp-1.3 (#2310), oadp-1.4 (#2312), and oadp-1.5 (#2309), but was never to oadp-dev as that branch only picked up readiness/liveness probes via #2306. This PR closes that gap on oadp-dev, covering both the CLI and VMDP servers.

How to test the changes made

How to test the changes made

go test ./internal/controller/... -run 'CLIServer|VMDPServer' -v

1. Deploy the operator from this branch.
2. Check the ServiceAccounts exist:

oc get sa -n openshift-adp openshift-adp-cli-server openshift-adp-vmdp-server

3. Check the deployments use them (not default):

oc get deploy -n openshift-adp openshift-adp-oadp-cli-server openshift-adp-oadp-vmdp-server \
 -o jsonpath='{.spec.template.spec.serviceAccountName}{"\n"}'

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.

[coderabbitai]

Warning

Review limit reached

@NicholasYancey, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 13 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 78cce77c-c1f7-432f-8315-f06ee5be1866

📥 Commits

Reviewing files that changed from the base of the PR and between deb931a and 741c6ca.

📒 Files selected for processing (2)

• internal/controller/cli_download_controller_test.go
• internal/controller/vmdp_download_controller_test.go

Walkthrough

The CLI and VMDP controllers now create dedicated ServiceAccounts with token automounting disabled, configure their server Deployments to use them, and update existing Deployments when fields are missing or outdated. Unit tests cover ServiceAccount and Deployment construction and reconciliation.

Changes

Download server ServiceAccounts

Layer / File(s)	Summary
CLI ServiceAccount and Deployment reconciliation internal/controller/cli_download_controller.go	Creates or repairs the dedicated CLI ServiceAccount, configures Deployment pod identity and token automounting, and backfills missing probes and settings on existing Deployments.
CLI reconciliation tests internal/controller/cli_download_controller_test.go	Tests resource construction, ServiceAccount creation, drift correction, owner-reference repair, and no-op reconciliation.
VMDP ServiceAccount and Deployment reconciliation internal/controller/vmdp_download_controller.go	Creates or repairs the dedicated VMDP ServiceAccount, configures Deployment pod identity and token automounting, and backfills missing probes and settings on existing Deployments.
VMDP reconciliation tests internal/controller/vmdp_download_controller_test.go	Tests resource construction, ServiceAccount creation, drift correction, owner-reference repair, and no-op reconciliation.

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

Sequence Diagram(s)

sequenceDiagram
  participant Controller
  participant KubernetesAPI
  participant ServerDeployment
  Controller->>KubernetesAPI: Get or create dedicated ServiceAccount
  Controller->>KubernetesAPI: Reconcile labels, owner reference, and token automount
  Controller->>KubernetesAPI: Get server Deployment
  Controller->>ServerDeployment: Apply probes and ServiceAccount settings
  Controller->>KubernetesAPI: Update Deployment when fields differ
  Controller->>KubernetesAPI: Reconcile Route and ConsoleCLIDownload

Loading

Possibly related PRs

• openshift/oadp-operator#2319: Overlaps with the probe backfill logic in the CLI and VMDP Deployment reconciliation paths.

Suggested reviewers: mpryc, shubham-pampattiwar

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is related to the main change, but it is broad and does not mention both CLI and VMDP service accounts.
Description check	✅ Passed	The description includes the required why and how-to-test sections with concrete validation steps.
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	The added tests use static Go test function names; no Ginkgo titles, t.Run names, or dynamic title construction were found.
Test Structure And Quality	✅ Passed	PASS: these are plain testing.T unit tests with fake clients, not Ginkgo; they’re single-purpose and have no cluster waits or cleanup concerns.
Microshift Test Compatibility	✅ Passed	PASS: the PR adds only testing.T unit tests in controller files; no new Ginkgo e2e specs or MicroShift-specific concerns are introduced.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	Only plain Go unit tests were added; no Ginkgo/e2e specs or SNO-multi-node assumptions were introduced.
Topology-Aware Scheduling Compatibility	✅ Passed	Changed controllers only add ServiceAccounts and set serviceAccountName/automount=false; no node selectors, affinity, spread constraints, PDBs, or topology-driven replica logic.
Ote Binary Stdout Contract	✅ Passed	No new process-level stdout writes were added: the new test files only contain ordinary Test funcs, and there’s no TestMain/init/RunSpecs or klog/stdout setup in the diff.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added; the changes are unit tests only and contain no IPv4-only or external connectivity assumptions.
No-Weak-Crypto	✅ Passed	No MD5/SHA1/DES/RC4/3DES/Blowfish/ECB usage, no custom crypto, and no secret/token comparisons in the changed code.
Container-Privileges	✅ Passed	Changed CLI/VMDP builders keep restrictive settings (RunAsNonRoot=true, allowPrivilegeEscalation=false, drop ALL) and add no privileged fields.
No-Sensitive-Data-In-Logs	✅ Passed	No new sensitive data is logged; added logs are generic service-account messages, and tests only assert non-sensitive fields.

✨ 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.}

[openshift-ci-robot]

@NicholasYancey: No Jira issue is referenced in the title of this pull request.
To reference a jira issue, add 'XYZ-NNN:' to the title of this pull request and request another refresh with /jira refresh.

Details

In response to this:

Why the changes were made

The CLI and VMDP download server pods on oadp-dev currently run with no ServiceAccount specified, defaulting to the namespace's default ServiceAccount. This fails Telco/CNF certification:

access-control-pod-service-account requires every pod to use a non-default ServiceAccount. This same fix was already backported directly to oadp-1.3 (#2310), oadp-1.4 (#2312), and oadp-1.5 (#2309), but was never to oadp-dev as that branch only picked up readiness/liveness probes via #2306. This PR closes that gap on oadp-dev, covering both the CLI and VMDP servers.

How to test the changes made

How to test the changes made

go test ./internal/controller/... -run 'CLIServer|VMDPServer' -v

1. Deploy the operator from this branch.
2. Check the ServiceAccounts exist:

oc get sa -n openshift-adp openshift-adp-cli-server openshift-adp-vmdp-server

3. Check the deployments use them (not default):

oc get deploy -n openshift-adp openshift-adp-oadp-cli-server openshift-adp-oadp-vmdp-server \
 -o jsonpath='{.spec.template.spec.serviceAccountName}{"\n"}'

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.

[coderabbitai]

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@internal/controller/cli_download_controller.go`:
- Around line 133-136: Reconcile AutomountServiceAccountToken independently from
the ServiceAccountName condition in the Deployment update logic. Update both
internal/controller/cli_download_controller.go lines 133-136 and
internal/controller/vmdp_download_controller.go lines 128-131 so each desired
field is compared and applied independently, ensuring token automounting is
reset to the desired value even when the ServiceAccount name already matches.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 662acb2e-f815-42d8-8a2a-def3c9bdc37e

📥 Commits

Reviewing files that changed from the base of the PR and between 1c39e7c and 52f79fc.

📒 Files selected for processing (4)

• internal/controller/cli_download_controller.go
• internal/controller/cli_download_controller_test.go
• internal/controller/vmdp_download_controller.go
• internal/controller/vmdp_download_controller_test.go

[kaovilai]

Consider reconciling the ServiceAccount object itself when it already exists.

Right now, the controllers create the dedicated ServiceAccounts when missing, but if the ServiceAccount already exists with:

AutomountServiceAccountToken unset or true
missing/incorrect labels
missing owner reference
…it looks like the code does not update it.

[coderabbitai]

Actionable comments posted: 2

🤖 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.

Inline comments:
In `@internal/controller/cli_download_controller_test.go`:
- Line 85: Update each of the four reconcileCLIResources calls in the affected
tests to capture and explicitly assert or log the returned error instead of
assigning it to blank. Preserve the tests’ expected failure flow and subsequent
fakeClient.Get assertions, while ensuring any reconcile error is surfaced with
useful test context.

In `@internal/controller/vmdp_download_controller_test.go`:
- Line 59: Update all four reconcileVMDPResources calls in the VMDP controller
tests to handle the returned error using the same documented pattern as the CLI
test file, rather than discarding it with “_”. Preserve the existing test
rationale for the unregistered Route/Console step while applying the shared fix
consistently.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: beefde83-92fa-472e-86b7-1e267bce8a15

📥 Commits

Reviewing files that changed from the base of the PR and between 52f79fc and deb931a.

📒 Files selected for processing (4)

• internal/controller/cli_download_controller.go
• internal/controller/cli_download_controller_test.go
• internal/controller/vmdp_download_controller.go
• internal/controller/vmdp_download_controller_test.go

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kaovilai, NicholasYancey, weshayutin

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [kaovilai]

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

[openshift-ci]

@NicholasYancey: all tests passed!

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.

[NicholasYancey]

/cherry-pick oadp-1.6

[openshift-cherrypick-robot]

@NicholasYancey: new pull request created: #2320

Details

In response to this:

/cherry-pick oadp-1.6

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.