CM-1038: Replace istiocsr type-specific decoders with generic DecodeObjBytes

[sebrandon1]

Summary

• Remove 8 type-specific decode*ObjBytes functions from the istiocsr package (~95 lines) and replace all 22 call sites with the existing generic common.DecodeObjBytes[T] helper
• Cache the ServiceAccount name at init time to avoid redundant YAML deserialization on every reconcile cycle
• Aligns the istiocsr package with the pattern already used by the trustmanager package

Test plan

• Unit tests pass (go test ./pkg/controller/istiocsr/...)
• Build succeeds (go build ./pkg/controller/istiocsr/)
• No new lint issues introduced
• CI checks pass

Jira: https://redhat.atlassian.net/browse/CM-1038

Summary by CodeRabbit

• Refactor
  • Unified how embedded Kubernetes manifests and cert templates are deserialized across the IstioCSR controller, using a shared decoder for consistent resource handling.
  • Updated RBAC, Deployment, Service, and ServiceAccount reconciliation to rely on the shared decoding path.
  • Added caching for the service account name to reduce repeated parsing during reconciliation.
• Tests
  • Updated test resource builders to use the same shared decoding approach for embedded assets.

[openshift-ci-robot]

@sebrandon1: This pull request references CM-1038 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

• Remove 8 type-specific decode*ObjBytes functions from the istiocsr package (~95 lines) and replace all 22 call sites with the existing generic common.DecodeObjBytes[T] helper
• Cache the ServiceAccount name at init time to avoid redundant YAML deserialization on every reconcile cycle
• Aligns the istiocsr package with the pattern already used by the trustmanager package

Test plan

• Unit tests pass (go test ./pkg/controller/istiocsr/...)
• Build succeeds (go build ./pkg/controller/istiocsr/)
• No new lint issues introduced
• CI checks pass

Jira: https://redhat.atlassian.net/browse/CM-1038

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]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 902fc4c2-65a3-4f79-9726-cbc51941616d

📥 Commits

Reviewing files that changed from the base of the PR and between b251d62 and 0132362.

📒 Files selected for processing (7)

• pkg/controller/istiocsr/certificates.go
• pkg/controller/istiocsr/deployments.go
• pkg/controller/istiocsr/rbacs.go
• pkg/controller/istiocsr/serviceaccounts.go
• pkg/controller/istiocsr/services.go
• pkg/controller/istiocsr/test_utils.go
• pkg/controller/istiocsr/utils.go

✅ Files skipped from review due to trivial changes (1)

• pkg/controller/istiocsr/serviceaccounts.go

🚧 Files skipped from review as they are similar to previous changes (5)

• pkg/controller/istiocsr/deployments.go
• pkg/controller/istiocsr/rbacs.go
• pkg/controller/istiocsr/services.go
• pkg/controller/istiocsr/test_utils.go
• pkg/controller/istiocsr/utils.go

---

Walkthrough

Replaces many bespoke decode helpers with a single generic common.DecodeObjBytes[T](codecs, SchemeGroupVersion, asset) across the IstioCSR controller; removes the old decode helpers, adds a package-level cached ServiceAccount name initialized from the ServiceAccount asset, and updates test utilities accordingly.

Changes

Decoding Consolidation

Layer / File(s)	Summary
Caching foundation and helper removal pkg/controller/istiocsr/utils.go	Adds assets import and cachedServiceAccountName variable; initializes it in init() by decoding the embedded ServiceAccount asset via common.DecodeObjBytes[*corev1.ServiceAccount]; removes all previous unexported decode*ObjBytes helper functions.
RBAC object decoders and cached SA usage pkg/controller/istiocsr/rbacs.go	Updates createOrApplyRBACResource to use cachedServiceAccountName rather than decoding the ServiceAccount inline; all RBAC getters (ClusterRole, ClusterRoleBinding, Role, RoleBinding, and lease variants) now decode via common.DecodeObjBytes[...].
Individual resource type decoders pkg/controller/istiocsr/serviceaccounts.go, pkg/controller/istiocsr/certificates.go, pkg/controller/istiocsr/deployments.go, pkg/controller/istiocsr/services.go	ServiceAccount, Certificate, Deployment, and Service getters switch from bespoke decode helpers to common.DecodeObjBytes[T] with the appropriate type and SchemeGroupVersion.
Test utilities decoder updates pkg/controller/istiocsr/test_utils.go	Test helpers for Certificate, RBAC objects, Deployment, Service, and ServiceAccount are updated to decode embedded assets via common.DecodeObjBytes[T]; adds common import.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (14 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: replacing type-specific decoder functions with a generic DecodeObjBytes helper in the istiocsr package.
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 istiocsr package uses Go's standard testing framework, not Ginkgo. All test names are static, descriptive strings with no dynamic content. The PR changes do not introduce any test definitions.
Test Structure And Quality	✅ Passed	The custom check requires reviewing Ginkgo test code quality, but the istiocsr package uses standard Go testing (testing.T), not Ginkgo. The PR contains no Ginkgo tests or modifications to Ginkgo t...
Microshift Test Compatibility	✅ Passed	This PR is a refactoring-only change that removes type-specific decoders and uses a generic helper instead. No new Ginkgo e2e tests are added, making the MicroShift compatibility check not applicable.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	This PR contains no new Ginkgo e2e tests. It is a refactoring that replaces decoder functions with generic helpers and caches values. The check applies only to new Ginkgo tests, making it inapplica...
Topology-Aware Scheduling Compatibility	✅ Passed	PR is a code refactoring that replaces type-specific decoders with a generic DecodeObjBytes helper. No deployment manifests, scheduling constraints, affinity rules, replica counts, or topology-rela...
Ote Binary Stdout Contract	✅ Passed	PR introduces no stdout writes in process-level code. The init() function only deserializes YAML and caches the ServiceAccount name without logging or stdout emissions.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	PR does not add any Ginkgo e2e tests (It/Describe/Context/When). Changes are refactoring of existing controller code and test utility functions, not new e2e tests.
No-Weak-Crypto	✅ Passed	PR contains no weak cryptography patterns. Changes replace type-specific YAML decoders with generic DecodeObjBytes helper—a standard Kubernetes deserialization function. No crypto operations involved.
Container-Privileges	✅ Passed	PR contains no privileged: true, hostPID/hostNetwork/hostIPC, SYS_ADMIN capabilities, or allowPrivilegeEscalation: true settings; all containers follow secure defaults.
No-Sensitive-Data-In-Logs	✅ Passed	PR replaces type-specific decoders with generic DecodeObjBytes and caches ServiceAccount names. All logging statements log only resource names, namespaces, and error messages—no sensitive data (pas...

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{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: sebrandon1
Once this PR has been reviewed and has the lgtm label, please assign bharath-b-rh 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

[sebrandon1]

/retest

[sebrandon1]

/retest

[openshift-ci]

@sebrandon1: 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.