Add spiffe-helper sidecar injection webhook#97
Open
fabiendupont wants to merge 1 commit into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a MutatingAdmissionWebhook and supporting controller, service, RBAC, assets, constants, and unit tests to auto-inject spiffe-helper init/sidecar into pods annotated with spiffe.openshift.io/inject-helper; registers webhook handler and exposes webhook port and RELATED_IMAGE_SPIFFE_HELPER image env. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant APIServer as Kube API Server
participant WebhookSvc as spiffe-helper-webhook
participant Injector as SpiffeHelperInjector
participant Pod as Pod (Mutated)
User->>APIServer: Create Pod with inject-helper annotation
APIServer->>WebhookSvc: Forward MutatingAdmissionReview (CREATE Pod)
WebhookSvc->>Injector: admission.Request
Injector->>Injector: Check annotation & existing containers
alt Annotation absent or already injected
Injector-->>APIServer: Allow (no mutation)
else Annotation present
Injector->>Injector: Read RELATED_IMAGE_SPIFFE_HELPER & annotations
Injector->>Injector: Build init+sidecar+volumes JSON Patch
Injector-->>APIServer: Allow + JSON Patch
end
APIServer->>Pod: Persist mutated Pod
sequenceDiagram
participant SpiffeCSIDriver as SpiffeCSIDriver CR
participant Controller as SpiffeHelperReconciler
participant APIServer as Kube API Server
participant StatusMgr as Status Manager
SpiffeCSIDriver->>Controller: Reconcile triggered
Controller->>APIServer: Get/Create/Update MutatingWebhookConfiguration
APIServer-->>Controller: MutatingWebhookConfiguration result
Controller->>StatusMgr: Set MutatingWebhookAvailable condition
Controller->>APIServer: Get/Create/Update webhook Service
APIServer-->>Controller: Service result
Controller->>StatusMgr: Set WebhookServiceAvailable condition
Controller-->>SpiffeCSIDriver: Reconciliation complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fabiendupont The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
config/rbac/role.yaml (1)
90-100:⚠️ Potential issue | 🟡 MinorRemove unused
patchverb and restrict to specific webhook byresourceNames.The controller only uses
get,create,update, andwatchoperations on the namedspiffe-helper-sidecar-injectorwebhook. Thepatchverb is unnecessary. Tighten the rule by removingpatchand restrictingcreate,get,list, andwatchtoresourceNames: ["spiffe-helper-sidecar-injector"].🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/rbac/role.yaml` around lines 90 - 100, Remove the unnecessary "patch" verb and scope the webhook rule to the specific named webhook: in the rule that targets apiGroups: admissionregistration.k8s.io and resources: mutatingwebhookconfigurations (and optionally validatingwebhookconfigurations if used), delete "patch" from verbs, ensure "update" is present (since the controller uses update), and add resourceNames: ["spiffe-helper-sidecar-injector"] so that create, get, list, watch (and update) are restricted to that specific mutatingwebhookconfiguration entry; keep other rules unchanged.
🧹 Nitpick comments (1)
pkg/controller/zero-trust-workload-identity-manager/controller.go (1)
158-159: Scopepatchverb tospiffe-helper-sidecar-injectorresource name.The controller only reconciles a single webhook:
spiffe-helper-sidecar-injector(hardcoded at line 127). Thepatchverb on line 158 currently allows unrestricted access to any mutating webhook configuration, whileupdateanddeleteare already scoped to the specific resource name. For consistency and least privilege,patchshould be restricted to the same resource.Suggested change
-// +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=mutatingwebhookconfigurations,verbs=get;list;watch;create;patch -// +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=mutatingwebhookconfigurations,verbs=update;delete,resourceNames=spiffe-helper-sidecar-injector +// +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=mutatingwebhookconfigurations,verbs=get;list;watch;create +// +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=mutatingwebhookconfigurations,verbs=patch;update;delete,resourceNames=spiffe-helper-sidecar-injector🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controller/zero-trust-workload-identity-manager/controller.go` around lines 158 - 159, The RBAC marker for mutatingwebhookconfigurations grants `patch` broadly; tighten it to the specific webhook resource by adding resourceNames=spiffe-helper-sidecar-injector to the `patch` rule so it matches the existing `update` and `delete` scope—update the kubebuilder comment that mentions `mutatingwebhookconfigurations` to include `resourceNames=spiffe-helper-sidecar-injector` (the same hardcoded webhook name used in this controller) to enforce least privilege.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bindata/spiffe-helper/spiffe-helper-mutating-webhook.yaml`:
- Around line 12-17: Add the OpenShift serving-cert annotation to the
spiffe-helper webhook Service and make the MutatingWebhookConfiguration preserve
and reference the injected CA bundle: add
metadata.annotations.service.beta.openshift.io/serving-cert-secret-name:
spiffe-helper-webhook-certs to the spiffe-helper-webhook Service
(spiffe-helper-webhook-service.yaml), update the MutatingWebhookConfiguration
for spiffe-helper-webhook to include a caBundle field referencing that secret,
and modify the controller reconcile logic that manages the
MutatingWebhookConfiguration (e.g., the reconcileMutatingWebhook / Reconcile
function that creates/updates the spiffe-helper MutatingWebhookConfiguration) to
read and preserve any existing caBundle value instead of overwriting it so the
injected cert is kept during reconciliation.
In `@bindata/spiffe-helper/spiffe-helper-webhook-service.yaml`:
- Around line 14-17: Service and container ports are mismatched: update the
Service spec to route to the webhook container's actual listening port (9443)
and expose the containerPort 9443; specifically change the Service
port/targetPort/name currently using 443/https (and any reference to 8443) to
use 9443, and update the Pod/Deployment containerPort entry that defines the
webhook container (the container port for the webhook image) to 9443 so the
Service maps correctly to the handler.
In `@pkg/controller/spiffe-helper/webhook_test.go`:
- Around line 303-315: applyResponse currently bypasses the webhook response by
calling injectSpiffeHelper with a hardcoded image instead of applying the
admission.Response patches, which can hide incorrect patches; change
applyResponse to take the original Pod, marshal it to raw JSON, use the
admission/PatchResponseFromRaw (or the response's patched raw) to apply
resp.Patches (or use a JSON patch library to apply resp.Patches) and then
unmarshal the patched raw back into a Pod to return—remove the hardcoded
image/injectSpiffeHelper path so the test validates the actual resp.Patches
emitted by Handle.
In `@pkg/controller/spiffe-helper/webhook.go`:
- Around line 121-149: The injection currently unconditionally appends volumes
(pod.Spec.Volumes) for VolumeNameWorkloadAPI, VolumeNameCerts, and
VolumeNameHelperConfig and similarly appends the cert VolumeMount, which can
create duplicates; modify the mutation to first check for existing entries and
only append when missing (e.g., implement/reuse small helpers like
hasVolume(pod, name) and hasVolumeMount(container, mountName) or inline loops)
and use those checks before adding VolumeNameWorkloadAPI, VolumeNameCerts,
VolumeNameHelperConfig and before adding the certificate mount to containers to
avoid duplicate volumes/mounts.
In `@pkg/controller/utils/resource_comparison.go`:
- Around line 285-291: Replace strict equality checks with a derivative
comparison so server-side defaulted fields don’t cause reconcilation churn: in
MutatingWebhookConfigurationNeedsUpdate use
equality.Semantic.DeepDerivative(desired.Webhooks, existing.Webhooks) instead of
DeepEqual, and make the identical change in
ValidatingWebhookConfigurationNeedsUpdate (use DeepDerivative(desired.Webhooks,
existing.Webhooks)) so unset fields in the desired manifest are ignored when
comparing to the API-server-stored existing object.
---
Outside diff comments:
In `@config/rbac/role.yaml`:
- Around line 90-100: Remove the unnecessary "patch" verb and scope the webhook
rule to the specific named webhook: in the rule that targets apiGroups:
admissionregistration.k8s.io and resources: mutatingwebhookconfigurations (and
optionally validatingwebhookconfigurations if used), delete "patch" from verbs,
ensure "update" is present (since the controller uses update), and add
resourceNames: ["spiffe-helper-sidecar-injector"] so that create, get, list,
watch (and update) are restricted to that specific mutatingwebhookconfiguration
entry; keep other rules unchanged.
---
Nitpick comments:
In `@pkg/controller/zero-trust-workload-identity-manager/controller.go`:
- Around line 158-159: The RBAC marker for mutatingwebhookconfigurations grants
`patch` broadly; tighten it to the specific webhook resource by adding
resourceNames=spiffe-helper-sidecar-injector to the `patch` rule so it matches
the existing `update` and `delete` scope—update the kubebuilder comment that
mentions `mutatingwebhookconfigurations` to include
`resourceNames=spiffe-helper-sidecar-injector` (the same hardcoded webhook name
used in this controller) to enforce least privilege.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: af34325e-62b4-4565-b098-169544d7bd14
📒 Files selected for processing (16)
bindata/spiffe-helper/spiffe-helper-mutating-webhook.yamlbindata/spiffe-helper/spiffe-helper-webhook-service.yamlcmd/zero-trust-workload-identity-manager/main.goconfig/manager/manager.yamlconfig/rbac/role.yamlpkg/controller/spiffe-helper/controller.gopkg/controller/spiffe-helper/controller_test.gopkg/controller/spiffe-helper/webhook.gopkg/controller/spiffe-helper/webhook_test.gopkg/controller/utils/constants.gopkg/controller/utils/labels.gopkg/controller/utils/resource_comparison.gopkg/controller/utils/utils.gopkg/controller/zero-trust-workload-identity-manager/controller.gopkg/operator/assets/bindata.gopkg/version/version.go
Comment on lines
+12
to
+17
| clientConfig: | ||
| service: | ||
| name: spiffe-helper-webhook | ||
| namespace: zero-trust-workload-identity-manager | ||
| path: /mutate-pods-spiffe-helper | ||
| failurePolicy: Ignore |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for webhook CA wiring across manifests and controller code
rg -n 'caBundle|inject-cabundle|serving-cert-secret-name|cert-manager.io/inject-ca-from' \
bindata config pkg -g '*.yaml' -g '*.go'
# Check webhook server cert configuration in manager bootstrap
rg -n 'webhook\.NewServer|CertDir|CertName|KeyName' cmd/zero-trust-workload-identity-manager/main.go -C2Repository: openshift/zero-trust-workload-identity-manager
Length of output: 3064
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check full spiffe-helper-mutating-webhook.yaml
cat bindata/spiffe-helper/spiffe-helper-mutating-webhook.yaml
# Check the webhook.go file to understand caBundle handling
cat pkg/controller/spire-server/webhook.go | head -100
# Check if spiffe-helper has any CA-related annotations or setup
rg -n 'spiffe-helper' bindata config pkg -g '*.yaml' -g '*.go' | grep -i 'ca\|cert\|webhook'Repository: openshift/zero-trust-workload-identity-manager
Length of output: 12687
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check spiffe-helper webhook service for CA annotations
cat bindata/spiffe-helper/spiffe-helper-webhook-service.yaml
# Check if spiffe-helper controller handles caBundle like spire-server does
rg -n 'caBundle|Preserve|injected' pkg/controller/spiffe-helper/controller.go -C3Repository: openshift/zero-trust-workload-identity-manager
Length of output: 1057
Webhook trust chain is incomplete—caBundle must be configured for TLS.
The spiffe-helper-webhook service lacks the service.beta.openshift.io/serving-cert-secret-name annotation needed to trigger OpenShift's service-ca-operator to inject the serving certificate. The webhook configuration has no caBundle field and no code to preserve or inject it (unlike the spire-server controller, which explicitly preserves injected caBundle values). With failurePolicy: Ignore, any TLS verification failure silently skips mutation without logging, leaving workloads without the spiffe-helper sidecar.
Add the annotation to spiffe-helper-webhook-service.yaml:
metadata:
annotations:
service.beta.openshift.io/serving-cert-secret-name: spiffe-helper-webhook-certsThen update the webhook configuration to reference it, and ensure the controller preserves the injected caBundle during reconciliation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bindata/spiffe-helper/spiffe-helper-mutating-webhook.yaml` around lines 12 -
17, Add the OpenShift serving-cert annotation to the spiffe-helper webhook
Service and make the MutatingWebhookConfiguration preserve and reference the
injected CA bundle: add
metadata.annotations.service.beta.openshift.io/serving-cert-secret-name:
spiffe-helper-webhook-certs to the spiffe-helper-webhook Service
(spiffe-helper-webhook-service.yaml), update the MutatingWebhookConfiguration
for spiffe-helper-webhook to include a caBundle field referencing that secret,
and modify the controller reconcile logic that manages the
MutatingWebhookConfiguration (e.g., the reconcileMutatingWebhook / Reconcile
function that creates/updates the spiffe-helper MutatingWebhookConfiguration) to
read and preserve any existing caBundle value instead of overwriting it so the
injected cert is kept during reconciliation.
Comment on lines
+14
to
+17
| - name: https | ||
| port: 443 | ||
| targetPort: https | ||
| protocol: TCP |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify webhook server options (look for explicit webhook port override)
rg -n 'webhook\.NewServer|webhook\.Options|Port:' cmd/zero-trust-workload-identity-manager/main.go -C2
# Verify manager container exposed ports
rg -n 'containerPort:|name:\s*https|name:\s*webhook' config/manager/manager.yaml -C1
# Verify service routing config
rg -n 'port:\s*443|targetPort:' bindata/spiffe-helper/spiffe-helper-webhook-service.yaml -C1Repository: openshift/zero-trust-workload-identity-manager
Length of output: 457
🏁 Script executed:
# Additional search: Find all webhook-related configuration files
find . -type f -name "*webhook*" | head -20Repository: openshift/zero-trust-workload-identity-manager
Length of output: 1684
🏁 Script executed:
# Check the actual spiffe-helper-webhook-service.yaml file content
cat -n bindata/spiffe-helper/spiffe-helper-webhook-service.yamlRepository: openshift/zero-trust-workload-identity-manager
Length of output: 744
🏁 Script executed:
# Search for manager.yaml files that define container ports
find . -type f -name "manager.yaml" -o -name "*manager*.yaml" | xargs cat -n 2>/dev/null | head -100Repository: openshift/zero-trust-workload-identity-manager
Length of output: 4300
🏁 Script executed:
# Check the webhook implementation
cat -n cmd/zero-trust-workload-identity-manager/main.go | grep -A 10 -B 10 "webhook.NewServer"Repository: openshift/zero-trust-workload-identity-manager
Length of output: 1053
🏁 Script executed:
# Check if there's any TLS or port configuration in the webhook setup
rg -n 'ListenOn|Port|port.*8443|port.*9443|http2' cmd/zero-trust-workload-identity-manager/main.go -C3Repository: openshift/zero-trust-workload-identity-manager
Length of output: 954
🏁 Script executed:
# Look for webhook server configuration more broadly
rg -n 'SetupWebhookWithManager|webhook\.Server' --type go -C3Repository: openshift/zero-trust-workload-identity-manager
Length of output: 10662
🏁 Script executed:
# Check what the default webhook server port is in controller-runtime
# by looking at the actual webhook initialization
rg -n 'webhookServer' cmd/zero-trust-workload-identity-manager/main.go -A 5Repository: openshift/zero-trust-workload-identity-manager
Length of output: 666
🏁 Script executed:
# Check the webhook server implementation for default port
rg -n "Port.*int|port.*:.*int|9443|8443" vendor/sigs.k8s.io/controller-runtime/pkg/webhook/server.go | head -20Repository: openshift/zero-trust-workload-identity-manager
Length of output: 180
🏁 Script executed:
# Look for the Start method which would show how the server binds to a port
rg -n "func.*Start|ListenAndServe|:Port|DefaultPort" vendor/sigs.k8s.io/controller-runtime/pkg/webhook/ -C2Repository: openshift/zero-trust-workload-identity-manager
Length of output: 2062
🏁 Script executed:
# Check if there are any config files or deployment manifests that show the actual port binding
rg -n "9443|8443|webhookPort|webhook.*port" config/ -C1Repository: openshift/zero-trust-workload-identity-manager
Length of output: 3257
🏁 Script executed:
# Search for any webhook port configuration in the project
rg -n "webhook.*[Pp]ort|[Pp]ort.*webhook" cmd/zero-trust-workload-identity-manager/ -C2Repository: openshift/zero-trust-workload-identity-manager
Length of output: 72
Webhook Service routes to incorrect backend port.
The webhook server defaults to port 9443 (confirmed in controller-runtime), but the service only exposes the container's port 8443. This mismatch prevents admission webhook calls from reaching the handler.
Proposed manifest fix
- name: https
port: 443
- targetPort: https
+ targetPort: 9443
protocol: TCPAdditionally, the container port definition must be updated to expose 9443:
- containerPort: 8443
name: https
+ - containerPort: 9443
+ name: webhook
protocol: TCP📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: https | |
| port: 443 | |
| targetPort: https | |
| protocol: TCP | |
| - name: https | |
| port: 443 | |
| targetPort: 9443 | |
| protocol: TCP |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bindata/spiffe-helper/spiffe-helper-webhook-service.yaml` around lines 14 -
17, Service and container ports are mismatched: update the Service spec to route
to the webhook container's actual listening port (9443) and expose the
containerPort 9443; specifically change the Service port/targetPort/name
currently using 443/https (and any reference to 8443) to use 9443, and update
the Pod/Deployment containerPort entry that defines the webhook container (the
container port for the webhook image) to 9443 so the Service maps correctly to
the handler.
fabiendupont
force-pushed
the
feat/spiffe-helper-webhook
branch
from
bde329e to
28e32f1
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
bindata/spiffe-helper/spiffe-helper-mutating-webhook.yaml (1)
3-3:⚠️ Potential issue | 🔴 CriticalWebhook TLS trust wiring appears incomplete for reliable mutation.
Line 12-Line 16 defines service routing, but this manifest still does not show OpenShift CA-injection wiring on the webhook configuration. If
caBundleis not injected/preserved elsewhere, TLS calls fail andfailurePolicy: Ignore(Line 17) silently skips injection.Suggested fix (manifest-side)
apiVersion: admissionregistration.k8s.io/v1 kind: MutatingWebhookConfiguration metadata: name: spiffe-helper-sidecar-injector + annotations: + service.beta.openshift.io/inject-cabundle: "true" labels: app.kubernetes.io/name: spiffe-helperVerification target: ensure at least one complete CA path exists (webhook config CA injection and service serving-cert annotation), and that controller updates do not drop injected
caBundle.#!/bin/bash set -euo pipefail # 1) Webhook config CA injection signals rg -n 'inject-cabundle|caBundle' bindata/spiffe-helper/spiffe-helper-mutating-webhook.yaml -C2 || true # 2) Service serving-cert annotation used by OpenShift service-ca operator rg -n 'serving-cert-secret-name' bindata/spiffe-helper/spiffe-helper-webhook-service.yaml -C2 || true # 3) Reconciler logic preserving/setting caBundle during update rg -n 'caBundle|MutatingWebhookConfiguration' pkg/controller/spiffe-helper/controller.go -C3 || trueAlso applies to: 12-17
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bindata/spiffe-helper/spiffe-helper-mutating-webhook.yaml` at line 3, The MutatingWebhookConfiguration manifest lacks CA wiring: ensure the webhook config includes an injected caBundle (or inject-cabundle annotation) and the webhook Service has the OpenShift serving-cert annotation (serving-cert-secret-name) so the service-ca operator will provision serving certs; additionally, update the controller reconciler that manipulates MutatingWebhookConfiguration (search for MutatingWebhookConfiguration and caBundle in pkg/controller/spiffe-helper/controller.go) to preserve any existing caBundle on updates (do not overwrite or remove injected caBundle) and to copy the Service’s current serving cert into the webhook's caBundle when creating/updating the webhook config; also consider changing failurePolicy if you want errors surfaced.
🧹 Nitpick comments (1)
pkg/controller/spiffe-helper/controller_test.go (1)
37-85: Strengthen webhook configuration test with spec-level assertions.
TestGetSpiffeHelperMutatingWebhookConfigurationcurrently validates labels/name only. Please also assert service target, path, rules, andfailurePolicyso routing regressions are caught early.Suggested test additions
func TestGetSpiffeHelperMutatingWebhookConfiguration(t *testing.T) { @@ if webhook.Name != "spiffe-helper-sidecar-injector" { t.Errorf("Expected name 'spiffe-helper-sidecar-injector', got '%s'", webhook.Name) } + if len(webhook.Webhooks) != 1 { + t.Fatalf("Expected exactly 1 webhook, got %d", len(webhook.Webhooks)) + } + wh := webhook.Webhooks[0] + if wh.ClientConfig.Service == nil { + t.Fatal("Expected webhook clientConfig.service to be set") + } + if wh.ClientConfig.Service.Name != "spiffe-helper-webhook" { + t.Errorf("Expected service name 'spiffe-helper-webhook', got '%s'", wh.ClientConfig.Service.Name) + } + if wh.ClientConfig.Service.Namespace != "zero-trust-workload-identity-manager" { + t.Errorf("Expected service namespace 'zero-trust-workload-identity-manager', got '%s'", wh.ClientConfig.Service.Namespace) + } + if wh.ClientConfig.Service.Path == nil || *wh.ClientConfig.Service.Path != "/mutate-pods-spiffe-helper" { + t.Errorf("Expected webhook path '/mutate-pods-spiffe-helper'") + } + if wh.FailurePolicy == nil || *wh.FailurePolicy != admissionregistrationv1.Ignore { + t.Errorf("Expected failurePolicy Ignore") + } + if len(wh.Rules) != 1 || len(wh.Rules[0].Operations) != 1 || wh.Rules[0].Operations[0] != admissionregistrationv1.Create { + t.Errorf("Expected a single CREATE rule for pods") + } @@ }) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controller/spiffe-helper/controller_test.go` around lines 37 - 85, Extend TestGetSpiffeHelperMutatingWebhookConfiguration to assert the mutating webhook spec: after getting webhook := getSpiffeHelperMutatingWebhookConfiguration(...), check that webhook.Webhooks has at least one entry, then for webhook.Webhooks[0] assert ClientConfig.Service is non-nil and its Name, Namespace and Path match the values set in getSpiffeHelperMutatingWebhookConfiguration (assert Service.Name, Service.Namespace, Service.Path); assert the Rules slice is non-empty and that each rule contains the expected Operations/APIGroups/APIVersions/Resources used by getSpiffeHelperMutatingWebhookConfiguration; and assert FailurePolicy equals the expected policy used there (and any other critical fields like SideEffects if set). Ensure you reference getSpiffeHelperMutatingWebhookConfiguration and TestGetSpiffeHelperMutatingWebhookConfiguration when adding these checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/controller/spiffe-helper/controller.go`:
- Around line 161-190: The current webhook-preservation copies fields by list
index which can apply values to the wrong webhook and block updates; instead
build a map from existing.Webhooks by webhook.ClientConfig.Service.Name or
webhook.Name and for each desired.Webhooks entry locate the existing webhook by
that unique name, then only copy the externally-managed fields
(ClientConfig.CABundle, MatchPolicy, TimeoutSeconds, NamespaceSelector,
ObjectSelector, ClientConfig.Service.Port) from the matched existing webhook;
for rule scopes, match desired.Webhooks[i].Rules to existing.Webhooks[...]
.Rules by rule identity (APIGroups/APIVersions/Resources/Operations) rather than
by index and only copy Scope when a rule match is found. Ensure ResourceVersion
is still preserved on the top-level desired object.
- Around line 72-86: The status manager (statusMgr) is created but its changes
are never persisted because ApplyStatus is not called; add a defer immediately
after creating statusMgr to call statusMgr.ApplyStatus(ctx, &csiDriver, func()
*v1alpha1.ConditionalStatus { return &csiDriver.Status.ConditionalStatus }) and
log errors via r.log.Error so that conditions set in reconcileMutatingWebhook
and reconcileWebhookService are always applied even on error paths; place this
defer right after the status.NewManager(r.ctrlClient) line in the reconcile
function.
---
Duplicate comments:
In `@bindata/spiffe-helper/spiffe-helper-mutating-webhook.yaml`:
- Line 3: The MutatingWebhookConfiguration manifest lacks CA wiring: ensure the
webhook config includes an injected caBundle (or inject-cabundle annotation) and
the webhook Service has the OpenShift serving-cert annotation
(serving-cert-secret-name) so the service-ca operator will provision serving
certs; additionally, update the controller reconciler that manipulates
MutatingWebhookConfiguration (search for MutatingWebhookConfiguration and
caBundle in pkg/controller/spiffe-helper/controller.go) to preserve any existing
caBundle on updates (do not overwrite or remove injected caBundle) and to copy
the Service’s current serving cert into the webhook's caBundle when
creating/updating the webhook config; also consider changing failurePolicy if
you want errors surfaced.
---
Nitpick comments:
In `@pkg/controller/spiffe-helper/controller_test.go`:
- Around line 37-85: Extend TestGetSpiffeHelperMutatingWebhookConfiguration to
assert the mutating webhook spec: after getting webhook :=
getSpiffeHelperMutatingWebhookConfiguration(...), check that webhook.Webhooks
has at least one entry, then for webhook.Webhooks[0] assert ClientConfig.Service
is non-nil and its Name, Namespace and Path match the values set in
getSpiffeHelperMutatingWebhookConfiguration (assert Service.Name,
Service.Namespace, Service.Path); assert the Rules slice is non-empty and that
each rule contains the expected Operations/APIGroups/APIVersions/Resources used
by getSpiffeHelperMutatingWebhookConfiguration; and assert FailurePolicy equals
the expected policy used there (and any other critical fields like SideEffects
if set). Ensure you reference getSpiffeHelperMutatingWebhookConfiguration and
TestGetSpiffeHelperMutatingWebhookConfiguration when adding these checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 00a43e08-9b01-46d1-ab70-5f99c5838c95
📒 Files selected for processing (16)
bindata/spiffe-helper/spiffe-helper-mutating-webhook.yamlbindata/spiffe-helper/spiffe-helper-webhook-service.yamlcmd/zero-trust-workload-identity-manager/main.goconfig/manager/manager.yamlconfig/rbac/role.yamlpkg/controller/spiffe-helper/controller.gopkg/controller/spiffe-helper/controller_test.gopkg/controller/spiffe-helper/webhook.gopkg/controller/spiffe-helper/webhook_test.gopkg/controller/utils/constants.gopkg/controller/utils/labels.gopkg/controller/utils/resource_comparison.gopkg/controller/utils/utils.gopkg/controller/zero-trust-workload-identity-manager/controller.gopkg/operator/assets/bindata.gopkg/version/version.go
🚧 Files skipped from review as they are similar to previous changes (7)
- config/manager/manager.yaml
- pkg/controller/utils/utils.go
- pkg/controller/spiffe-helper/webhook.go
- bindata/spiffe-helper/spiffe-helper-webhook-service.yaml
- pkg/controller/spiffe-helper/webhook_test.go
- pkg/controller/zero-trust-workload-identity-manager/controller.go
- pkg/operator/assets/bindata.go
fabiendupont
force-pushed
the
feat/spiffe-helper-webhook
branch
from
28e32f1 to
acf0790
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/controller/spiffe-helper/webhook.go`:
- Around line 121-152: The current volume dedupe logic in the mutating webhook
only checks names (hasVolume) and can silently accept incompatible pre-existing
volumes or conflicting mounts for VolumeNameWorkloadAPI, VolumeNameCerts, and
VolumeNameHelperConfig; update the mutation to validate compatibility before
appending: implement a helper like ensureCompatibleVolume(pod *corev1.Pod,
expected corev1.Volume) error that iterates pod.Spec.Volumes, returns an error
if a volume with the same Name exists but its VolumeSource differs (use
reflect.DeepEqual on VolumeSource), and only appends the expected volume when
absent; call this helper in place of the current name-only checks and propagate
admission errors so incompatible pre-existing volumes are rejected during
admission rather than causing runtime failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0391e19d-b1ba-49e5-bf51-a6900ce9f6e5
📒 Files selected for processing (16)
bindata/spiffe-helper/spiffe-helper-mutating-webhook.yamlbindata/spiffe-helper/spiffe-helper-webhook-service.yamlcmd/zero-trust-workload-identity-manager/main.goconfig/manager/manager.yamlconfig/rbac/role.yamlpkg/controller/spiffe-helper/controller.gopkg/controller/spiffe-helper/controller_test.gopkg/controller/spiffe-helper/webhook.gopkg/controller/spiffe-helper/webhook_test.gopkg/controller/utils/constants.gopkg/controller/utils/labels.gopkg/controller/utils/resource_comparison.gopkg/controller/utils/utils.gopkg/controller/zero-trust-workload-identity-manager/controller.gopkg/operator/assets/bindata.gopkg/version/version.go
🚧 Files skipped from review as they are similar to previous changes (9)
- pkg/controller/utils/utils.go
- pkg/controller/spiffe-helper/webhook_test.go
- pkg/controller/spiffe-helper/controller_test.go
- pkg/version/version.go
- pkg/controller/utils/resource_comparison.go
- pkg/operator/assets/bindata.go
- config/rbac/role.yaml
- config/manager/manager.yaml
- bindata/spiffe-helper/spiffe-helper-webhook-service.yaml
fabiendupont
force-pushed
the
feat/spiffe-helper-webhook
branch
from
acf0790 to
2388280
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/controller/utils/resource_comparison.go (1)
294-301: Inconsistency:ValidatingWebhookConfigurationNeedsUpdatestill usesDeepEqual.The new
MutatingWebhookConfigurationNeedsUpdatecorrectly usesDeepDerivative, butValidatingWebhookConfigurationNeedsUpdatestill usesDeepEqual. This inconsistency means validating webhooks will experience the same reconcile churn issue that was addressed for mutating webhooks.♻️ Proposed fix for consistency
// ValidatingWebhookConfigurationNeedsUpdate checks if a ValidatingWebhookConfiguration needs updating +// Uses DeepDerivative so that server-side defaulted fields in existing don't cause unnecessary updates. func ValidatingWebhookConfigurationNeedsUpdate(existing, desired *admissionregistrationv1.ValidatingWebhookConfiguration) bool { - // Compare webhooks - the main content of the configuration - if !equality.Semantic.DeepEqual(existing.Webhooks, desired.Webhooks) { + if !equality.Semantic.DeepDerivative(desired.Webhooks, existing.Webhooks) { return true } return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controller/utils/resource_comparison.go` around lines 294 - 301, ValidatingWebhookConfigurationNeedsUpdate currently uses equality.Semantic.DeepEqual to compare existing.Webhooks and desired.Webhooks, causing reconcile churn; change the comparison to use equality.Semantic.DeepDerivative (same approach as MutatingWebhookConfigurationNeedsUpdate) so that only meaningful differences trigger updates, i.e., replace the DeepEqual check in ValidatingWebhookConfigurationNeedsUpdate with a DeepDerivative check for the Webhooks slice.pkg/controller/spiffe-helper/webhook.go (1)
178-191: Consider adding SecurityContext to injected containers for restricted namespace compatibility.The injected init and sidecar containers lack explicit
SecurityContext. In namespaces with restricted Pod Security Standards or SCCs, pods may be rejected without proper security settings.♻️ Proposed enhancement
+ // Minimal security context for compatibility with restricted namespaces + securityContext := &corev1.SecurityContext{ + AllowPrivilegeEscalation: boolPtr(false), + RunAsNonRoot: boolPtr(true), + Capabilities: &corev1.Capabilities{ + Drop: []corev1.Capability{"ALL"}, + }, + SeccompProfile: &corev1.SeccompProfile{ + Type: corev1.SeccompProfileTypeRuntimeDefault, + }, + } + // Add init container pod.Spec.InitContainers = append(pod.Spec.InitContainers, corev1.Container{ Name: InitContainerName, Image: image, Args: []string{"-config", "/etc/spiffe-helper/helper-init.conf"}, VolumeMounts: helperVolumeMounts, + SecurityContext: securityContext, }) // Add sidecar container pod.Spec.Containers = append(pod.Spec.Containers, corev1.Container{ Name: SidecarContainerName, Image: image, Args: []string{"-config", "/etc/spiffe-helper/helper.conf"}, VolumeMounts: helperVolumeMounts, + SecurityContext: securityContext, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controller/spiffe-helper/webhook.go` around lines 178 - 191, Injected init and sidecar containers lack SecurityContext which can cause pod admission failures in restricted namespaces; update the container specs created where InitContainerName and SidecarContainerName are appended to pod.Spec.InitContainers and pod.Spec.Containers to include a SecurityContext (e.g., set RunAsNonRoot=true, RunAsUser to a non-zero UID, AllowPrivilegeEscalation=false, ReadOnlyRootFilesystem=true, drop all capabilities and only add needed ones) so the injected containers meet PodSecurity/SCC requirements while preserving helperVolumeMounts and existing Args/Image settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/controller/spiffe-helper/webhook.go`:
- Around line 178-191: Injected init and sidecar containers lack SecurityContext
which can cause pod admission failures in restricted namespaces; update the
container specs created where InitContainerName and SidecarContainerName are
appended to pod.Spec.InitContainers and pod.Spec.Containers to include a
SecurityContext (e.g., set RunAsNonRoot=true, RunAsUser to a non-zero UID,
AllowPrivilegeEscalation=false, ReadOnlyRootFilesystem=true, drop all
capabilities and only add needed ones) so the injected containers meet
PodSecurity/SCC requirements while preserving helperVolumeMounts and existing
Args/Image settings.
In `@pkg/controller/utils/resource_comparison.go`:
- Around line 294-301: ValidatingWebhookConfigurationNeedsUpdate currently uses
equality.Semantic.DeepEqual to compare existing.Webhooks and desired.Webhooks,
causing reconcile churn; change the comparison to use
equality.Semantic.DeepDerivative (same approach as
MutatingWebhookConfigurationNeedsUpdate) so that only meaningful differences
trigger updates, i.e., replace the DeepEqual check in
ValidatingWebhookConfigurationNeedsUpdate with a DeepDerivative check for the
Webhooks slice.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6c37ff8b-de02-427a-ac92-c622c58bd656
📒 Files selected for processing (16)
bindata/spiffe-helper/spiffe-helper-mutating-webhook.yamlbindata/spiffe-helper/spiffe-helper-webhook-service.yamlcmd/zero-trust-workload-identity-manager/main.goconfig/manager/manager.yamlconfig/rbac/role.yamlpkg/controller/spiffe-helper/controller.gopkg/controller/spiffe-helper/controller_test.gopkg/controller/spiffe-helper/webhook.gopkg/controller/spiffe-helper/webhook_test.gopkg/controller/utils/constants.gopkg/controller/utils/labels.gopkg/controller/utils/resource_comparison.gopkg/controller/utils/utils.gopkg/controller/zero-trust-workload-identity-manager/controller.gopkg/operator/assets/bindata.gopkg/version/version.go
🚧 Files skipped from review as they are similar to previous changes (9)
- pkg/controller/utils/utils.go
- bindata/spiffe-helper/spiffe-helper-mutating-webhook.yaml
- pkg/controller/spiffe-helper/webhook_test.go
- pkg/controller/zero-trust-workload-identity-manager/controller.go
- bindata/spiffe-helper/spiffe-helper-webhook-service.yaml
- cmd/zero-trust-workload-identity-manager/main.go
- pkg/controller/spiffe-helper/controller.go
- config/manager/manager.yaml
- pkg/controller/spiffe-helper/controller_test.go
fabiendupont
force-pushed
the
feat/spiffe-helper-webhook
branch
from
2388280 to
b614b66
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pkg/controller/spiffe-helper/controller_test.go (1)
255-257: Add missing negative assertions for unexpectedCreate()calls.Current checks only validate the positive path (
expectCreate == true). Add the negative check too, so accidental creates fail tests.Suggested test hardening
@@ if tt.expectCreate && fakeClient.CreateCallCount() != 1 { t.Errorf("Expected Create to be called once, called %d times", fakeClient.CreateCallCount()) } + if !tt.expectCreate && fakeClient.CreateCallCount() != 0 { + t.Errorf("Expected Create not to be called, called %d times", fakeClient.CreateCallCount()) + } @@ if tt.expectCreate && fakeClient.CreateCallCount() != 1 { t.Errorf("Expected Create to be called once, called %d times", fakeClient.CreateCallCount()) } + if !tt.expectCreate && fakeClient.CreateCallCount() != 0 { + t.Errorf("Expected Create not to be called, called %d times", fakeClient.CreateCallCount()) + }Also applies to: 327-329
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controller/spiffe-helper/controller_test.go` around lines 255 - 257, The test only asserts when tt.expectCreate is true; add the negative assertion for when tt.expectCreate is false by checking fakeClient.CreateCallCount() == 0 and failing the test if it is non-zero (i.e., replace the single conditional with an if/else that asserts Create was called exactly once when expectCreate is true, and asserts it was not called when expectCreate is false). Apply the same change to the second occurrence that mirrors the check (the block around the other fakeClient.CreateCallCount() assertion).pkg/controller/spiffe-helper/controller.go (1)
185-190: Don’t always overwrite desired webhook Service.Port.Copying
existingWH.ClientConfig.Service.Portunconditionally can block intentional port updates from desired manifests later.Safer preservation logic
- if existingWH.ClientConfig.Service != nil && - desired.Webhooks[i].ClientConfig.Service != nil && - existingWH.ClientConfig.Service.Port != nil { + if existingWH.ClientConfig.Service != nil && + desired.Webhooks[i].ClientConfig.Service != nil && + desired.Webhooks[i].ClientConfig.Service.Port == nil && + existingWH.ClientConfig.Service.Port != nil { desired.Webhooks[i].ClientConfig.Service.Port = existingWH.ClientConfig.Service.Port }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controller/spiffe-helper/controller.go` around lines 185 - 190, The current logic always copies existingWH.ClientConfig.Service.Port into desired.Webhooks[i].ClientConfig.Service.Port which prevents intentional port changes; change the condition to only preserve the existing port when the desired port is nil (i.e., check desired.Webhooks[i].ClientConfig.Service.Port == nil before assigning), keeping the rest of the nil checks for ClientConfig.Service and existingWH.ClientConfig.Service.Port intact so you only overwrite when the desired manifest did not specify a port.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/controller/spiffe-helper/webhook.go`:
- Around line 67-71: The current early-return uses
hasSpiffeHelperContainers(pod) which treats any pre-existing helper container as
full injection; change the logic to require both helper containers exist before
skipping mutation: update or replace hasSpiffeHelperContainers to explicitly
check for the presence of both "spiffe-helper-init" and "spiffe-helper" (or add
a new helper like hasAllSpiffeHelperContainers) and use that check where
currently called (including the other occurrence around lines 101-113) so
injection only skips when both init and main helper containers are present.
---
Nitpick comments:
In `@pkg/controller/spiffe-helper/controller_test.go`:
- Around line 255-257: The test only asserts when tt.expectCreate is true; add
the negative assertion for when tt.expectCreate is false by checking
fakeClient.CreateCallCount() == 0 and failing the test if it is non-zero (i.e.,
replace the single conditional with an if/else that asserts Create was called
exactly once when expectCreate is true, and asserts it was not called when
expectCreate is false). Apply the same change to the second occurrence that
mirrors the check (the block around the other fakeClient.CreateCallCount()
assertion).
In `@pkg/controller/spiffe-helper/controller.go`:
- Around line 185-190: The current logic always copies
existingWH.ClientConfig.Service.Port into
desired.Webhooks[i].ClientConfig.Service.Port which prevents intentional port
changes; change the condition to only preserve the existing port when the
desired port is nil (i.e., check desired.Webhooks[i].ClientConfig.Service.Port
== nil before assigning), keeping the rest of the nil checks for
ClientConfig.Service and existingWH.ClientConfig.Service.Port intact so you only
overwrite when the desired manifest did not specify a port.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f061fb3f-d015-4a10-9e0c-f1bc73b18370
📒 Files selected for processing (16)
bindata/spiffe-helper/spiffe-helper-mutating-webhook.yamlbindata/spiffe-helper/spiffe-helper-webhook-service.yamlcmd/zero-trust-workload-identity-manager/main.goconfig/manager/manager.yamlconfig/rbac/role.yamlpkg/controller/spiffe-helper/controller.gopkg/controller/spiffe-helper/controller_test.gopkg/controller/spiffe-helper/webhook.gopkg/controller/spiffe-helper/webhook_test.gopkg/controller/utils/constants.gopkg/controller/utils/labels.gopkg/controller/utils/resource_comparison.gopkg/controller/utils/utils.gopkg/controller/zero-trust-workload-identity-manager/controller.gopkg/operator/assets/bindata.gopkg/version/version.go
🚧 Files skipped from review as they are similar to previous changes (9)
- cmd/zero-trust-workload-identity-manager/main.go
- pkg/version/version.go
- pkg/controller/utils/utils.go
- pkg/controller/zero-trust-workload-identity-manager/controller.go
- bindata/spiffe-helper/spiffe-helper-mutating-webhook.yaml
- pkg/operator/assets/bindata.go
- pkg/controller/utils/labels.go
- pkg/controller/utils/constants.go
- pkg/controller/spiffe-helper/webhook_test.go
fabiendupont
force-pushed
the
feat/spiffe-helper-webhook
branch
3 times, most recently
from
ea1867e to
2857b8f
Compare
Add a MutatingAdmissionWebhook that automatically injects spiffe-helper init containers and sidecars into pods annotated with spiffe.openshift.io/inject-helper: "true", eliminating manual sidecar configuration for applications that need on-disk TLS certificates from SPIRE. The webhook injects: - Init container (spiffe-helper-init) for initial SVID fetch - Sidecar container (spiffe-helper) for continuous certificate rotation - CSI volume (csi.spiffe.io) for SPIRE workload API access - EmptyDir volume for certificate storage - ConfigMap volume for helper configuration Optional annotations allow customizing the cert directory (spiffe.openshift.io/cert-dir) and ConfigMap name (spiffe.openshift.io/helper-config). Closes openshift#96 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Fabien Dupont <fdupont@redhat.com>
fabiendupont
force-pushed
the
feat/spiffe-helper-webhook
branch
from
2857b8f to
9dd5d8f
Compare
|
@fabiendupont: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
spiffe-helperinit containers and sidecars into pods annotated withspiffe.openshift.io/inject-helper: "true"MutatingWebhookConfigurationand webhookService, tied toSpiffeCSIDriverCR lifecycleWhat gets injected
spiffe-helper-init)spiffe-helper)csi.spiffe.io)spiffe-certs)spiffe-helper-config)Files changed
New files (6):
bindata/spiffe-helper/— MutatingWebhookConfiguration and Service YAML assetspkg/controller/spiffe-helper/webhook.go— Admission webhook handlerpkg/controller/spiffe-helper/controller.go— Controller/reconcilerpkg/controller/spiffe-helper/webhook_test.go— 7 webhook unit testspkg/controller/spiffe-helper/controller_test.go— 10 controller unit testsModified files (10):
pkg/version/version.go—SpiffeHelperVersion = "0.11.0"pkg/controller/utils/— Constants, labels, decoder, resource comparison for MutatingWebhookConfigurationcmd/.../main.go— Controller and webhook handler registrationconfig/manager/manager.yaml—RELATED_IMAGE_SPIFFE_HELPERenv varconfig/rbac/role.yaml—mutatingwebhookconfigurationspermissions (auto-generated from kubebuilder markers)pkg/operator/assets/bindata.go— RegeneratedTest plan
make buildcompiles successfullymake test— all unit tests pass (17 new tests + existing tests unaffected)spiffe.openshift.io/inject-helper: "true"→ init + sidecar injected/var/run/secrets/tls/Closes #96
Summary by CodeRabbit
New Features
Infrastructure
Tests