CM-1040: Extract shared ApplyResource helper and migrate istiocsr to SSA

AGENTS.md

@@ -17,17 +17,17 @@ Deploys/configures upstream **cert-manager** (and optionally **istio-csr** / **t
 | CR | Scope / name | Stack | Gate |
 |----|--------------|-------|------|
 | [CertManager](harness-evals/harness-docs/domain/certmanager.md) | Cluster / `cluster` | library-go `resourceapply` | Always on |
-| [IstioCSR](harness-evals/harness-docs/domain/istiocsr.md) | Namespaced / `default` | Create+Update | `IstioCSR` GA default true |
+| [IstioCSR](harness-evals/harness-docs/domain/istiocsr.md) | Namespaced / `default` | SSA + Create/Update (mixed) | `IstioCSR` GA default true |
 | [TrustManager](harness-evals/harness-docs/domain/trustmanager.md) | Cluster / `cluster` | **SSA** | `TrustManager` TP default false |
 
 **Operands**: Certificate/Issuer/… → cert-manager; Bundle → trust-manager.  
 **Quick Start**: `oc get certmanager cluster -o yaml` | `make local-run`
 
 ## Critical Patterns
 
-1. **Never assume uniform SSA** — CertManager library-go; IstioCSR Create+`UpdateWithRetry`; TrustManager only SSA + field owner `trust-manager-controller`. [components.md](harness-evals/harness-docs/architecture/components.md)
+1. **Never assume uniform SSA** — CertManager library-go; IstioCSR mixed (SSA via `common.ApplyResource` for Service/SA/Certificate/Role/RoleBinding/NetworkPolicy; Create+`UpdateWithRetry` for Deployment, CA ConfigMap, ClusterRole/Binding); TrustManager all SSA + field owner `trust-manager-controller`. [components.md](harness-evals/harness-docs/architecture/components.md)
 2. **Never hand-edit generated assets** — bindata.go, clients, regenerated `bindata/` / `bundle/` (`make generate`, `update-manifests`, `bundle`, `verify-bindata`).
-3. **Greenfield = TrustManager** — copy SSA + `HandleReconcileResult`/`FromClientError` (30s requeue); not IstioCSR. [ADR-0002](harness-evals/harness-docs/decisions/adr-0002-apply-strategies.md)
+3. **Greenfield = TrustManager** — copy SSA (`common.ApplyResource`) + `HandleReconcileResult`/`FromClientError` (30s requeue); not IstioCSR. [ADR-0002](harness-evals/harness-docs/decisions/adr-0002-apply-strategies.md)
 4. **Feature gates** — `--unsupported-addon-features`; five touchpoints; no cluster FeatureSet for TrustManager (CM-1141). [ADR-0003](harness-evals/harness-docs/decisions/adr-0003-feature-gates.md)
 5. **Ignore** `certmanager_controller.go` placeholder — RBAC markers only, never started.
 6. **RELATED_IMAGE triple-sync** — manager.yaml env ↔ controller constants ↔ CSV `relatedImages` + `make bundle`.

harness-evals/harness-docs/CERT_MANAGER_OPERATOR_DEVELOPMENT.md

@@ -25,7 +25,7 @@ FIPS: `make build` sources `hack/go-fips.sh`. WARN (non-FIPS) builds are **local
 
 ### New controller-runtime operand (greenfield)
 
-1. Copy **TrustManager** (SSA `Patch` + unique `FieldOwner`), not IstioCSR Create+Update
+1. Copy **TrustManager** (SSA via `common.ApplyResource` + unique `FieldOwner`), not IstioCSR Create+Update
 2. Feature gate — all five touchpoints ([ADR-0003](./decisions/adr-0003-feature-gates.md))
 3. Image triple-sync (below) + bindata via `hack/update-*-manifests.sh` + `make update-manifests` / `verify-bindata`
 4. RBAC via `+kubebuilder:rbac` → `make manifests` → `make bundle` (never hand-edit CSV RBAC)
@@ -62,7 +62,7 @@ the packaged operand-version env value in `manager.yaml` omits `v`.
 ## Common Mistakes
 
 1. Hand-edit bindata / generated clients / generated `bundle/` CSV  
-2. Assume SSA for IstioCSR or CertManager  
+2. Assume SSA for CertManager or for every IstioCSR resource  
 3. Logic in `certmanager_controller.go` placeholder  
 4. Label-filtered cache for ConfigMaps or Issuer/ClusterIssuer  
 5. Expect IstioCSR/TrustManager delete to GC operands (warn-only TODO)  

harness-evals/harness-docs/CERT_MANAGER_OPERATOR_TESTING.md

@@ -17,10 +17,10 @@
 | Controller | Assert | Fake |
 |------------|--------|------|
 | CertManager | library-go `resourceapply` / expected generation / rollout | library-go fake clientset — **no** `FakeCtrlClient` |
-| IstioCSR | `CreateCallCount` / `UpdateWithRetryCallCount` | `fakes.FakeCtrlClient` — **not** `PatchCallCount` |
-| TrustManager | `PatchCallCount` with `client.Apply` + FieldOwner `trust-manager-controller` | `fakes.FakeCtrlClient` — **not** Update/Create |
+| IstioCSR | `ApplyCallCount` for SSA resources (Service, SA, Certificate, Role/RoleBinding, NetworkPolicy); `CreateCallCount` / `UpdateWithRetryCallCount` for Deployment, CA ConfigMap, ClusterRole/Binding | `fakes.FakeCtrlClient` |
+| TrustManager | `ApplyCallCount` (SSA via `CtrlClient.Apply`, FieldOwner `trust-manager-controller`) | `fakes.FakeCtrlClient` — **not** Update/Create |
 
-A TrustManager test asserting `UpdateCallCount` (or IstioCSR asserting `PatchCallCount`) is the wrong path even if green. Regenerate fakes with `make generate-fakes` after changing `CtrlClient` — never hand-edit `pkg/controller/common/fakes/`.
+A test asserting `UpdateCallCount` for an SSA resource, or `ApplyCallCount` for a Create/Update resource, is the wrong path even if green. `PatchCallCount` only counts the JSON merge-patch fallback `ApplyResource` uses when a drift-correcting apply fails. Regenerate fakes with `make generate-fakes` after changing `CtrlClient` — never hand-edit `pkg/controller/common/fakes/`.
 
 ```bash
 make test-unit

harness-evals/harness-docs/api-contracts-guidelines.md

@@ -175,8 +175,9 @@ types and run `make manifests` for operator CRDs; bump upstream versions and run
   checking its markers (`+kubebuilder:validation:*`, `+kubebuilder:default`,
   `XValidation`) directly.
 - Don't assume all three CRs share apply/reconcile semantics — CertManager uses
-  library-go `resourceapply` (not SSA), IstioCSR uses Create+`UpdateWithRetry`, only
-  TrustManager uses controller-runtime SSA with field owner `trust-manager-controller`.
+  library-go `resourceapply` (not SSA), IstioCSR mixes SSA (field owner `istio-csr-controller`)
+  with Create+`UpdateWithRetry`, and TrustManager uses controller-runtime SSA throughout with
+  field owner `trust-manager-controller`.
   Field-ownership/conflict behavior described in docs must match the actual controller.
 - Don't document `UnsupportedConfigOverrides.{Controller,Webhook,CAInjector}` as
   first-class `Spec` fields — they're helper types for parsing

harness-evals/harness-docs/architecture/components.md

@@ -51,13 +51,13 @@ Note: Informers start **before** library-go controllers run — do not assume fi
 |-----------|-------------|----------|--------------|
 | Framework | library-go | controller-runtime | controller-runtime |
 | CR | Cluster `cluster` | Namespaced `default` | Cluster `cluster` |
-| Apply | `resourceapply` / DeploymentController | Create + `UpdateWithRetry` | **SSA** `client.Apply` + `ForceOwnership` |
-| Field owner | n/a (library-go) | n/a | `trust-manager-controller` |
+| Apply | `resourceapply` / DeploymentController | SSA (`common.ApplyResource`) for most resources; Create + `UpdateWithRetry` for Deployment, CA ConfigMap, ClusterRole/Binding | **SSA** `common.ApplyResource` → `CtrlClient.Apply` + `ForceOwnership` |
+| Field owner | n/a (library-go) | `istio-csr-controller` | `trust-manager-controller` |
 | Status | `operatorv1.OperatorStatus` | `ConditionalStatus` | `ConditionalStatus` |
 | Gate | Always on | `IstioCSR` GA default true | `TrustManager` TP default false |
 | Finalizer cleanup | library-go | TODO warn-only | TODO warn-only |
 
-Evidence: TrustManager `services.go` / `deployments.go` (`Patch` + `client.Apply`); IstioCSR `services.go` / `deployments.go` (`UpdateWithRetry`); CertManager `cert_manager_networkpolicy.go` (`resourceapply.ApplyNetworkPolicy`).
+Evidence: `common/applier.go` (`ApplyResource`); TrustManager `services.go` / `deployments.go` (`ApplyResource`); IstioCSR `services.go` (`ApplyResource`) / `deployments.go` (`UpdateWithRetry`); CertManager `cert_manager_networkpolicy.go` (`resourceapply.ApplyNetworkPolicy`).
 
 **Greenfield rule**: New controller-runtime operands → copy **TrustManager**, not IstioCSR.
 
@@ -174,7 +174,7 @@ TrustManager/cluster ──ctrl-runtime SSA──► trust-manager Deployment (+
 ## SME Review Recommended
 
 - Exact uninstall / finalizer cleanup design intent for IstioCSR & TrustManager.  
-- Whether IstioCSR will be migrated to SSA.  
+- Whether IstioCSR Deployment, CA ConfigMap and ClusterRole/Binding will move to SSA.  
 - Operational gotchas not visible in code (Slack/Jira) — Phase 5.5 chai-bot skipped.  
 - Recipe nuances for adding a fourth operand controller (CSV env, RBAC, relatedImages checklist).
 

harness-evals/harness-docs/decisions/adr-0002-apply-strategies.md

@@ -2,6 +2,7 @@
 
 **Status**: Accepted  
 **Date**: 2026-03-11 (TrustManager SSA; IstioCSR remains Create+Update)  
+**Amended**: CM-1040 — shared `common.ApplyResource` helper; IstioCSR namespaced resources migrated to SSA  
 **Deciders**: cert-manager-operator maintainers
 
 ## Context
@@ -13,8 +14,8 @@ Documenting a single “we use SSA everywhere” claim is a known hallucination
 ## Decision
 
 1. CertManager continues with library-go apply helpers.  
-2. IstioCSR keeps Create+Update until explicitly migrated.  
-3. **New controller-runtime operand reconcilers must follow TrustManager SSA** (`Patch` + `client.Apply` + `FieldOwner` + `ForceOwnership`).  
+2. IstioCSR uses SSA via `common.ApplyResource` for Service, ServiceAccount, Certificate, Role/RoleBinding and NetworkPolicy. Deployment, CA ConfigMap and ClusterRole/ClusterRoleBinding (generated names tracked in status) keep Create+Update until explicitly migrated.  
+3. **New controller-runtime operand reconcilers must follow TrustManager SSA** (`common.ApplyResource` → `CtrlClient.Apply` + `FieldOwner` + `ForceOwnership`, with a JSON merge-patch fallback when a drift-correcting apply fails).  
 4. Status updates for IstioCSR/TrustManager may still use `UpdateWithRetry` on the CR object.
 
 ## Consequences
@@ -34,10 +35,11 @@ Documenting a single “we use SSA everywhere” claim is a known hallucination
 
 ## References
 
+- Shared helper: `pkg/controller/common/applier.go` (`ApplyResource`), `client.go` (`CtrlClient.Apply`)
 - TrustManager: `pkg/controller/trustmanager/services.go`, `deployments.go`, `constants.go` (`fieldOwner`)
-- IstioCSR: `pkg/controller/istiocsr/services.go`, `deployments.go`
+- IstioCSR: `pkg/controller/istiocsr/services.go` (SSA), `deployments.go` (Create+Update), `constants.go` (`fieldOwner`)
 - CertManager: `pkg/controller/certmanager/cert_manager_networkpolicy.go` (`resourceapply.ApplyNetworkPolicy`)
 
 ## SME Review Recommended
 
-Whether/when IstioCSR migrates to SSA; field-owner naming convention for future operands.
+Whether/when the remaining IstioCSR Create+Update resources migrate to SSA; field-owner naming convention for future operands.

harness-evals/harness-docs/domain/istiocsr.md

@@ -10,7 +10,7 @@
 
 Deploys and configures the **cert-manager-istio-csr** operand so Istio mesh workloads can obtain certificates via cert-manager issuers.
 
-**Key Principle**: Enabled by operator feature gate `IstioCSR` (GA, default **true**) via `--unsupported-addon-features`. Controller uses **Create + UpdateWithRetry**, not SSA.
+**Key Principle**: Enabled by operator feature gate `IstioCSR` (GA, default **true**) via `--unsupported-addon-features`. Controller is **mixed**: SSA via `common.ApplyResource` for most resources, Create + `UpdateWithRetry` for the rest (see Apply below).
 
 ## Spec Structure
 
@@ -66,7 +66,7 @@ Condition types (`conditions.go`): `Ready`, `Degraded`. Reasons: `Failed`, `Read
 | Concern | Behavior | Code |
 |---------|----------|------|
 | Image | `RELATED_IMAGE_CERT_MANAGER_ISTIOCSR` | `istiocsr/constants.go` |
-| Apply | Exists? update via `UpdateWithRetry`; else Create | e.g. `services.go`, `deployments.go` |
+| Apply | SSA via `common.ApplyResource` (field owner `istio-csr-controller`) for Service, ServiceAccount, Certificate, Role/RoleBinding, NetworkPolicy; Create + `UpdateWithRetry` for Deployment, CA ConfigMap, ClusterRole/ClusterRoleBinding | SSA: `services.go`, `serviceaccounts.go`, `certificates.go`, `rbacs.go`, `networkpolicies.go`; Create+Update: `deployments.go`, `rbacs.go` |
 | Singleton | Disallows multiple IstioCSR instances | `disallowMultipleIstioCSRInstances` |
 | Watches | Certificate, Deployment, RBAC, Service, SA, ConfigMap, Secret (metadata), NetworkPolicy, Issuer, ClusterIssuer | `controller.go` |
 | Labels | Managed label value `cert-manager-istio-csr`; watch label `istiocsr.openshift.operator.io/watched-by` | `constants.go` |

harness-evals/harness-docs/domain/trustmanager.md

@@ -64,7 +64,7 @@ Same `Ready` / `Degraded` condition types as IstioCSR (`conditions.go`).
 | Concern | Behavior | Code |
 |---------|----------|------|
 | Image | `RELATED_IMAGE_CERT_MANAGER_TRUST_MANAGER` | `trustmanager/constants.go` |
-| Apply | `Patch(..., client.Apply, FieldOwner("trust-manager-controller"), ForceOwnership)` | `services.go:41`, `deployments.go:40`, … |
+| Apply | `common.ApplyResource` → `CtrlClient.Apply(..., FieldOwner("trust-manager-controller"), ForceOwnership)` | `services.go`, `deployments.go`, `common/applier.go` |
 | Skip when unchanged | Managed-field-aware `*Modified` helpers before apply | `utils.go` / resource files |
 | Trusted CA | Watches CNO-injected ConfigMap `cert-manager-operator-trusted-ca-bundle` (separate from CertManager `--trusted-ca-configmap`) | `controller.go`, `configmaps.go` |
 | SecretTargets | `Disabled` (default) or `Custom`. Custom requires non-empty `authorizedSecrets`; write RBAC scoped via `ResourceNames` (cluster-wide read is intentional). Sort secrets before ClusterRole apply for deterministic diffs. | `rbacs.go` `appendSecretTargetRules` |
@@ -75,7 +75,7 @@ Same `Ready` / `Degraded` condition types as IstioCSR (`conditions.go`).
 
 ## Common Mistakes
 
-1. Documenting “all controllers use SSA” — false; only TrustManager among the three.
+1. Documenting “all controllers use SSA” — false; CertManager uses library-go, and IstioCSR still uses Create+Update for some resources.
 2. Expecting Bundle reconciliation in the operator — operator deploys trust-manager; Bundle is operand API.
 3. Hand-editing `bindata/trust-manager` YAML without `make update-manifests`.
 4. Granting wildcard Secret **write** for SecretTargets — keep write limited to `authorizedSecrets` ResourceNames.

harness-evals/harness-docs/operator-controllers-guidelines.md

@@ -28,12 +28,12 @@ package you're editing; do not "fix" one to look like another without an ADR upd
 | Package | Apply method | Evidence |
 |---|---|---|
 | `certmanager` | library-go `resourceapply.*` / `staticresourcecontroller` / `DeploymentController` | `cert_manager_networkpolicy.go` (`resourceapply.ApplyNetworkPolicy`) |
-| `istiocsr` | Imperative **Create, then `UpdateWithRetry`** on diff | `services.go`, `deployments.go`: `r.Create(...)` / `r.UpdateWithRetry(...)` |
-| `trustmanager` | **Server-Side Apply (SSA)**: `client.Patch(obj, client.Apply, client.FieldOwner(fieldOwner), client.ForceOwnership)` | `services.go`, `deployments.go`; field owner `trust-manager-controller` in `constants.go` |
+| `istiocsr` | **Mixed**: SSA via `common.ApplyResource` (field owner `istio-csr-controller`) for Service, ServiceAccount, Certificate, Role/RoleBinding, NetworkPolicy; imperative **Create, then `UpdateWithRetry`** for Deployment, CA ConfigMap, ClusterRole/ClusterRoleBinding | `services.go` (`ApplyResource`); `deployments.go`: `r.Create(...)` / `r.UpdateWithRetry(...)` |
+| `trustmanager` | **Server-Side Apply (SSA)**: `common.ApplyResource` → `CtrlClient.Apply(obj, client.FieldOwner(fieldOwner), client.ForceOwnership)` | `services.go`, `deployments.go`; field owner `trust-manager-controller` in `constants.go` |
 
 Do **not** claim "we use SSA everywhere" — this is a documented hallucination failure
-mode (ADR-0002). IstioCSR remains Create+Update until explicitly migrated; there is no
-committed timeline for that migration.
+mode (ADR-0002). IstioCSR's Deployment, CA ConfigMap and ClusterRole/ClusterRoleBinding
+remain Create+Update until explicitly migrated; there is no committed timeline for that.
 
 ## 3. Greenfield Rule: New ctrl-runtime Operand → Copy TrustManager
 
@@ -42,7 +42,7 @@ When adding a **new** feature-gated operand controller under `pkg/controller/`:
 1. **Copy the TrustManager package structure**, not IstioCSR:
    `controller.go` (Reconcile/SetupWithManager), `deployments.go`, `services.go`,
    `serviceaccounts.go`, `rbacs.go`, `webhooks.go`, `constants.go`, `install_*.go`, `utils.go`.
-2. Apply resources via `client.Patch(desired, client.Apply, client.FieldOwner(<name>-controller), client.ForceOwnership)`.
+2. Apply resources via `common.ApplyResource(ctx, r.CtrlClient, ..., fieldOwner, <kind>Modified)`, which server-side applies with `client.FieldOwner(<name>-controller)` + `client.ForceOwnership`.
    Pick a unique `fieldOwner` constant per controller.
 3. Diff only fields you manage via SSA (see `serviceModified` pattern in
    `trustmanager/services.go`) — do not `reflect.DeepEqual` the whole object.

harness-evals/harness-docs/security-guidelines.md

@@ -61,9 +61,9 @@ if effective.MinTLSVersion == configv1.VersionTLS13 {
 - library-go (CertManager) RBAC is not marker-driven — its ClusterRole comes from
   `bindata/cert-manager-deployment/**`/`config/rbac/*`. Don't add `+kubebuilder:rbac` markers expecting
   them to affect the CertManager controller set.
-- Apply methods differ per controller (library-go `resourceapply`, IstioCSR `Create+UpdateWithRetry`,
-  TrustManager SSA `client.Apply` + field owner `trust-manager-controller`) — when adding a new RBAC
-  object for TrustManager, follow the `*Modified` diff-then-`Patch(client.Apply, ForceOwnership)`
+- Apply methods differ per controller (library-go `resourceapply`, IstioCSR mixed SSA +
+  `Create+UpdateWithRetry`, TrustManager SSA + field owner `trust-manager-controller`) — when adding a new RBAC
+  object for TrustManager, follow the `*Modified` diff-then-`common.ApplyResource` (SSA, `ForceOwnership`)
   pattern in `rbacs.go`, not a blind re-apply.
 
 ## Secrets Handling (TrustManager `secretTargets`)