NO-JIRA: Remove apply client workaround for controller-runtime < 0.22

[smrtrfszm]

What this PR does / why we need it:

The temporary applyClient wrapper and Apply interface existed only to work around a controller-runtime bug that prevented server-side apply with the fake client. That bug was fixed in controller-runtime 0.22, which the repository now uses.

Drop the Apply interface and applyClient/fakeApplyClient wrappers in favor of the native client.Apply API. The field owner is now set once per client via client.WithFieldOwner, and manifests are applied through client.ApplyConfigurationFromUnstructured with client.ForceOwnership.

Tests construct the error case with interceptor.NewClient instead of the custom errorApplyClient, and the fake client is wrapped with client.WithFieldOwner to mirror production behavior.

Which issue(s) this PR fixes:

Fixes

Special notes for your reviewer:

Checklist:

• Subject and description added to both, commit and PR.
• Relevant issues have been referenced.
• This change includes docs.
• This change includes unit tests.

Summary by CodeRabbit

• Refactor
  • Manifest application now automatically derives field owner from the binary name, eliminating explicit options at each apply point
  • Simplified internal manifest application workflow by removing temporary interface abstractions
  • Updated function signatures to improve consistency in manifest application handling across the bootstrap process
  • Test suite refactored to validate updated manifest application patterns and client configuration

[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]

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

Details

In response to this:

What this PR does / why we need it:

The temporary applyClient wrapper and Apply interface existed only to work around a controller-runtime bug that prevented server-side apply with the fake client. That bug was fixed in controller-runtime 0.22, which the repository now uses.

Drop the Apply interface and applyClient/fakeApplyClient wrappers in favor of the native client.Apply API. The field owner is now set once per client via client.WithFieldOwner, and manifests are applied through client.ApplyConfigurationFromUnstructured with client.ForceOwnership.

Tests construct the error case with interceptor.NewClient instead of the custom errorApplyClient, and the fake client is wrapped with client.WithFieldOwner to mirror production behavior.

Which issue(s) this PR fixes:

Fixes

Special notes for your reviewer:

Checklist:

• Subject and description added to both, commit and PR.
• Relevant issues have been referenced.
• This change includes docs.
• This change includes unit tests.

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]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 34b897ef-e3e7-467d-8ff7-c766be257758

📥 Commits

Reviewing files that changed from the base of the PR and between a101e66 and da47673.

📒 Files selected for processing (2)

• kas-bootstrap/kas_boostrap.go
• kas-bootstrap/kas_boostrap_test.go

---

📝 Walkthrough

Walkthrough

This PR refactors the manifest apply plumbing in kas-bootstrap to centralize field ownership configuration. Rather than passing explicit client.FieldOwner(...) options at each apply call site, the code now derives a fieldOwner from the binary name once and wraps both the admin and bootstrap clients using client.WithFieldOwner. The temporary Apply interface and applyClient wrapper are removed, and applyManifest and applyBootstrapManifests now accept plain client.Client instances that already carry the configured owner. Tests are similarly updated to use new fake client helpers that replace the prior custom Apply implementations.

🚥 Pre-merge checks | ✅ 11

✅ Passed checks (11 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly references the main change: removing the apply client workaround that is no longer needed due to controller-runtime 0.22 support.
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	No Ginkgo tests found in the PR. Tests use standard Go testing.T with t.Run(). All test names are static strings with no dynamic content like pods, timestamps, UUIDs, nodes, namespaces, or IPs.
Test Structure And Quality	✅ Passed	The custom check is scoped to "Ginkgo test code" but the modified test file uses traditional Go testing with Gomega, not Ginkgo. The check is not applicable to this PR.
Topology-Aware Scheduling Compatibility	✅ Passed	PR removes internal apply workaround; applies no workloads or scheduling constraints. Manifests are CRDs and ClusterRoleBindings only—no pod specs, affinity, or topology constraints introduced.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No Ginkgo e2e tests are added in this PR. It only refactors unit tests using Go's standard testing package in kas-bootstrap, removing an applyClient workaround.
No-Weak-Crypto	✅ Passed	No weak cryptographic algorithms, custom crypto implementations, or non-constant-time secret comparisons found. Changes only refactor Kubernetes client manifest application wrappers.
Container-Privileges	✅ Passed	PR modifies Go code and tests only; no container manifests with privileged configs, hostPID/Network/IPC, SYS_ADMIN, or allowPrivilegeEscalation are introduced or modified.
No-Sensitive-Data-In-Logs	✅ Passed	No sensitive data logging found. Logging only includes manifest file paths, status messages, and error messages without exposing passwords, tokens, credentials, PII, or internal hostnames.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 and usage tips.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: smrtrfszm
Once this PR has been reviewed and has the lgtm label, please assign enxebre 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:

• OWNERS

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

[codecov]

Codecov Report

❌ Patch coverage is 30.00000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.43%. Comparing base (a101e66) to head (da47673).

Files with missing lines	Patch %	Lines
kas-bootstrap/kas_boostrap.go	30.00%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8693      +/-   ##
==========================================
- Coverage   41.43%   41.43%   -0.01%     
==========================================
  Files         756      756              
  Lines       93648    93648              
==========================================
- Hits        38803    38802       -1     
- Misses      52124    52125       +1     
  Partials     2721     2721              

Files with missing lines	Coverage Δ
kas-bootstrap/kas_boostrap.go	45.60% <30.00%> (-0.55%)	⬇️

Flag	Coverage Δ
cmd-support	34.87% <ø> (ø)
cpo-hostedcontrolplane	43.50% <ø> (ø)
cpo-other	42.75% <ø> (ø)
hypershift-operator	51.57% <ø> (ø)
other	31.63% <30.00%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[hypershift-jira-solve-ci]

I now have the complete picture. Here's the analysis:

Test Failure Analysis Complete

Job Information

• Prow Job: codecov/patch (GitHub Check Run, not a Prow CI job)
• Build ID: 79895710032 (GitHub Check Run ID)
• PR: #8693 — NO-JIRA: Remove apply client workaround for controller-runtime < 0.22
• Repository: openshift/hypershift
• Target: codecov patch coverage gate

Test Failure Analysis

Error

codecov/patch: 30.00% of diff hit (target 41.43%)
7 lines in your changes missing coverage in kas-bootstrap/kas_boostrap.go

Summary

The codecov/patch check failed because only 3 out of 10 changed lines in kas-bootstrap/kas_boostrap.go are executed during unit tests, yielding 30% patch coverage against the auto-derived target of 41.43% (the project-wide coverage). All 7 uncovered lines reside inside the run() function — the program's main entry point that constructs real Kubernetes clients and orchestrates polling loops. These lines were never covered before and cannot be meaningfully unit-tested. The actual logic change (replacing the applyClient.Apply wrapper with the native client.Client.Apply method using ApplyConfigurationFromUnstructured) is fully covered by the updated tests in TestApplyManifest and TestApplyBootstrapManifests.

Root Cause

This is a false-positive coverage gate failure, not a real test deficiency. The root cause is structural:

1. The run() function is an integration entry point. It calls clientcmd.BuildConfigFromFlags, ctrl.GetConfig(), and client.New() to create real Kubernetes API clients, then uses wait.PollUntilContextTimeout to orchestrate retries. None of these operations can execute without a live cluster, so run() has never had unit test coverage.

2. The PR moved 7 lines within run(). Specifically:

   • fieldOwner := filepath.Base(os.Args[0]) (new)
   • adminClient = client.WithFieldOwner(adminClient, fieldOwner) (new)
   • bootstrapClient, err := client.New(cfg, client.Options{Scheme: configScheme}) (variable rename from client to bootstrapClient)
   • bootstrapClient = client.WithFieldOwner(bootstrapClient, fieldOwner) (new, replaces old applyClient wrapper)
   • Two call-site changes passing adminClient/bootstrapClient directly instead of the removed adminApplyClient/applyClient wrappers

3. Codecov counts all + lines in the diff regardless of whether those lines are in testable code. Since 7 of the 10 changed lines are in the untestable run() function, the patch coverage drops to 30%.

4. No explicit threshold is configured. The codecov.yml for this repo has no coverage.patch.default.target setting. Codecov defaults to using the project-wide coverage (41.43%) as the patch target, which is why 30% < 41.43% triggers the failure.

5. The testable code IS fully covered. The 3 lines that ARE covered are in applyManifest() and applyBootstrapManifests() — the functions where the actual behavioral change occurs. The test file was properly updated: fakeApplyClient was replaced with newFakeClient() using client.WithFieldOwner, and errorApplyClient was replaced with newErrorApplyClient() using interceptor.NewClient.

Recommendations

1. This failure is safe to override/ignore. The uncovered lines are in the run() integration entry point and were never tested before. The actual logic change is fully covered.

2. To prevent future false-positives, consider adding a coverage.patch.default.target to codecov.yml:

   coverage:
     patch:
       default:
         target: auto
         threshold: 15%

   This allows patch coverage to dip up to 15% below the project average before failing, which accommodates changes in integration-level code.

3. Alternatively, the kas-bootstrap/kas_boostrap.go file's run() function could be excluded from coverage via a codecov.yml ignore entry (though this is heavy-handed since the testable functions in the same file should remain covered).

4. No code changes needed. The PR correctly removes the Apply interface workaround, replaces it with the native client.Client.Apply method available in controller-runtime ≥ 0.22, and updates all tests accordingly.

Evidence

Evidence	Detail
Check Run Conclusion	failure — 30.00% patch coverage vs 41.43% target
File with missing coverage	kas-bootstrap/kas_boostrap.go — 7 of 10 changed lines uncovered
Uncovered lines location	All 7 lines are inside the run() function (lines ~51-97), the program main entry point
Covered lines location	3 lines in applyManifest() and applyBootstrapManifests() — the refactored logic
Test coverage of logic change	TestApplyManifest and TestApplyBootstrapManifests both updated and passing
Test file changes	Replaced fakeApplyClient/errorApplyClient (old Apply interface) with newFakeClient()/newErrorApplyClient() (native client.Client)
Codecov config	codecov.yml has no coverage.patch threshold — defaults to project coverage (41.43%)
Project coverage impact	-0.01% (41.43% → 41.43%), negligible — 1 additional miss from variable rename in run()
Pre-existing coverage of run()	0% — never had unit test coverage; requires a live Kubernetes cluster

---