OCPBUGS-60568: lib/resourcemerge: Add support for automountServiceAccountToken #1264

tchap · 2025-11-26T11:22:55Z

Take automountServiceAccountToken flag into account when reconciling a pod spec.

Taking over #1225

This is needed for e.g. openshift/cluster-kube-controller-manager-operator#858

Take automountServiceAccountToken flag into account when reconciling a pod spec.

openshift-ci-robot · 2025-11-26T11:23:02Z

@tchap: This pull request references Jira Issue OCPBUGS-60579, which is invalid:

expected the bug to target the "4.21.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Take automountServiceAccountToken flag into account when reconciling a pod spec.

Taking over #1225

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 · 2025-11-26T11:23:04Z

Walkthrough

This change extends the resource merge functionality to support AutomountServiceAccountToken handling in PodSpec. The implementation adds a new field modification step using the existing setBoolPtr pattern, with four new test cases validating set, unset, change, and unchanged scenarios.

Changes

Cohort / File(s)	Summary
AutomountServiceAccountToken handling implementation `lib/resourcemerge/core.go`	Adds a new modification step in `ensurePodSpec` to set `AutomountServiceAccountToken` on the PodSpec via `setBoolPtr`, positioned after `RestartPolicy` handling and before other field updates.
AutomountServiceAccountToken test coverage `lib/resourcemerge/core_test.go`	Adds four test cases to `TestEnsurePodSpec`: validation for set, unset, change, and unchanged scenarios for `AutomountServiceAccountToken`, covering the modified flag behavior and resulting PodSpec state.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Verify that AutomountServiceAccountToken handling via setBoolPtr follows the established pattern used for other boolean PodSpec fields
Confirm the four test cases adequately cover set/unset/change/unchanged transitions and properly assert the modified flag
Ensure no edge cases or field ordering issues relative to surrounding PodSpec field updates

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between df1ec55 and 30036c8.

📒 Files selected for processing (2)

lib/resourcemerge/core.go (1 hunks)
lib/resourcemerge/core_test.go (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

lib/resourcemerge/core.go
lib/resourcemerge/core_test.go

🔇 Additional comments (2)

lib/resourcemerge/core.go (1)

48-48: LGTM! Correct implementation following established patterns.

The AutomountServiceAccountToken handling correctly uses setBoolPtr, consistent with the ServiceAccount implementation (line 25) and other boolean pointer fields in PodSpec. The placement after RestartPolicy and before ServiceAccountName is logical.

lib/resourcemerge/core_test.go (1)

59-108: LGTM! Comprehensive test coverage for AutomountServiceAccountToken.

The four test cases thoroughly validate the boolean pointer handling: setting the field, unsetting it, changing the value, and ensuring no modification when values match. The test structure is consistent with existing patterns and correctly verifies both the modified flag and the resulting PodSpec state.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

Provide your own instructions using the high_level_summary_instructions setting.
Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

📝 Description — Summarize the main change in 50–60 words, explaining what was done.

📓 References — List relevant issues, discussions, documentation, or related PRs.

📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.

📊 Contributor Summary — Include a Markdown table showing contributions:
| Contributor | Lines Added | Lines Removed | Files Changed |

✔️ Additional Notes — Add any extra reviewer context.
Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

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

tchap · 2025-11-26T11:23:45Z

/jira refresh

openshift-ci-robot · 2025-11-26T11:23:52Z

@tchap: This pull request references Jira Issue OCPBUGS-60579, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.21.0) matches configured target version for branch (4.21.0)
bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

In response to this:

/jira refresh

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-robot · 2025-11-26T11:27:11Z

@tchap: This pull request references Jira Issue OCPBUGS-60579, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.21.0) matches configured target version for branch (4.21.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

In response to this:

Take automountServiceAccountToken flag into account when reconciling a pod spec.

Taking over #1225

This is needed for e.g. openshift/cluster-kube-controller-manager-operator#858

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.

tchap · 2025-11-26T14:37:17Z

/retest

openshift-ci-robot · 2025-11-26T15:32:51Z

@tchap: This pull request references Jira Issue OCPBUGS-60568, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.21.0) matches configured target version for branch (4.21.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Take automountServiceAccountToken flag into account when reconciling a pod spec.

Taking over #1225

This is needed for e.g. openshift/cluster-kube-controller-manager-operator#858

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 · 2025-11-26T16:31:48Z

@tchap: all tests passed!

Full PR test history. Your PR dashboard.

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.

hongkailiu

The change looks good to me. Only a couple of questions.

Do we want to leave the comment like this?
Do we have a way of testing it before merging? Like, without this pull, a test with openshift/cluster-kube-controller-manager-operator#858 is failing vs successful with it. See https://docs.ci.openshift.org/docs/how-tos/multi-pr-presubmit-testing/ for reference. https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_cluster-kube-controller-manager-operator/858/pull-ci-openshift-cluster-kube-controller-manager-operator-main-e2e-aws-ovn-upgrade/1993335772639399936 seems like a good candicate. Let me try.

hongkailiu · 2025-11-26T21:24:29Z

/testwith openshift/cluster-kube-controller-manager-operator/main/e2e-aws-ovn-upgrade openshift/cluster-kube-controller-manager-operator#858

hongkailiu · 2025-11-27T00:09:48Z

Why didnt e2e-ote work there?

/testwith openshift/cluster-kube-controller-manager-operator/main/e2e-aws-ovn-upgrade openshift/cluster-kube-controller-manager-operator#858

hongkailiu · 2025-11-27T00:12:47Z

/testwith openshift/cluster-kube-controller-manager-operator/main/e2e-aws-ovn-upgrade openshift/cluster-kube-controller-manager-operator#858

lib/resourcemerge/core_test.go

tchap · 2025-11-27T09:22:48Z

Do we want to leave the comment like this?

I think that we don't, there is a Jira issue for CVO separately, so we won't miss this.

Do we have a way of testing it before merging?

Looks like it didn't work, an image failed to be built properly 😐

There is also /payload-job-with-prs and other similar commands, but I never really understood what the right syntax is TBH.

hongkailiu · 2025-11-27T17:33:20Z

I heard that /payload-job-with-prs does not work for presubmit.
Its doc is https://docs.ci.openshift.org/docs/release-oversight/pull-request-testing/#payload-commands
I used them quite often lately.

I am asking about /testwith here.
hold for their answers. Let us give them a day or two to reply.

/hold

/approve

openshift-ci · 2025-11-27T17:33:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hongkailiu, tchap

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [hongkailiu]

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

hongkailiu · 2025-11-27T19:47:43Z

@tchap @lance5890
Could you please do a rebase on openshift/cluster-kube-controller-manager-operator#858?

I am expect the rebase will fix the /testwith

lance5890 · 2025-11-28T01:11:35Z

@tchap @lance5890 Could you please do a rebase on openshift/cluster-kube-controller-manager-operator#858?

I am expect the rebase will fix the /testwith

rebased

hongkailiu · 2025-11-28T14:19:53Z

/testwith openshift/cluster-kube-controller-manager-operator/main/e2e-aws-ovn-upgrade openshift/cluster-kube-controller-manager-operator#858

lib/resourcemerge: Add support for automountServiceAccountToken

30036c8

Take automountServiceAccountToken flag into account when reconciling a pod spec.

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Nov 26, 2025

tchap mentioned this pull request Nov 26, 2025

OCPBUGS-60579: Suppuort automountServiceAccountToken modify #1225

Closed

This was referenced Nov 26, 2025

Use BoundServceAccountTokenVolume dy default openshift/library-go#2001

Open

OCPBUGS-60568: Use BoundServceAccountTokenVolume dy default openshift/cluster-kube-controller-manager-operator#858

Open

tchap changed the title ~~OCPBUGS-60579: lib/resourcemerge: Add support for automountServiceAccountToken~~ OCPBUGS-60568: lib/resourcemerge: Add support for automountServiceAccountToken Nov 26, 2025

hongkailiu reviewed Nov 26, 2025

View reviewed changes

lance5890 reviewed Nov 27, 2025

View reviewed changes

lib/resourcemerge/core_test.go Show resolved Hide resolved

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 27, 2025

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 27, 2025

OCPBUGS-60568: lib/resourcemerge: Add support for automountServiceAccountToken #1264

Are you sure you want to change the base?

OCPBUGS-60568: lib/resourcemerge: Add support for automountServiceAccountToken #1264

Conversation

tchap commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Nov 26, 2025

Uh oh!

coderabbitai bot commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Uh oh!

tchap commented Nov 26, 2025

Uh oh!

openshift-ci-robot commented Nov 26, 2025

Uh oh!

openshift-ci-robot commented Nov 26, 2025

Uh oh!

tchap commented Nov 26, 2025

Uh oh!

openshift-ci-robot commented Nov 26, 2025

Uh oh!

openshift-ci bot commented Nov 26, 2025

Uh oh!

hongkailiu left a comment

Choose a reason for hiding this comment

Uh oh!

hongkailiu commented Nov 26, 2025

Uh oh!

hongkailiu commented Nov 27, 2025

Uh oh!

hongkailiu commented Nov 27, 2025

Uh oh!

Uh oh!

tchap commented Nov 27, 2025

Uh oh!

hongkailiu commented Nov 27, 2025

Uh oh!

openshift-ci bot commented Nov 27, 2025

Uh oh!

hongkailiu commented Nov 27, 2025

Uh oh!

lance5890 commented Nov 28, 2025

Uh oh!

hongkailiu commented Nov 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tchap commented Nov 26, 2025 •

edited

Loading

coderabbitai bot commented Nov 26, 2025 •

edited

Loading