feat: create SharedTrust controller for cert-manager cacerts reconciliation#383
Merged
Conversation
odh-devops-app Bot
pushed a commit
to opendatahub-io/agents-operator
that referenced
this pull request
May 29, 2026
…n synthesizePipeline ## Why When a namespace's `authbridge-runtime-config` ConfigMap has no `pipeline:` of its own, the webhook synthesizes one from the namespace's `authbridge-config` env-var contract (NamespaceConfig). The synthesis passed `keycloak_url` + `keycloak_realm` only to the outbound token-exchange plugin. jwt-validation got just `issuer`. kagenti-extensions#383 extends the jwt-validation plugin to accept these two fields and derive jwks_url from the INTERNAL Keycloak URL — the sidecar actually reaches the JWKS endpoint from inside the cluster, and `issuer` is the PUBLIC hostname (required for `iss`-claim matching but typically unreachable from inside the pod). Without the keycloak_* hints, jwt-validation falls back to issuer-derivation and every inbound request fails with "connection refused" fetching the JWKS → 401. ## Scope Pure addition to `synthesizePipeline`: the same two fields that already feed token-exchange now also feed jwt-validation. Both plugins end up with the same "where is Keycloak internally" hint, mirroring the pre-PR-rossoctl#378 binary behavior where jwks_url was derived from outbound.token_url via a cross-plugin pass. Test: extend `TestEnsurePerAgentConfigMap_EmptyBaseYAML_Fallback FromNsConfig` to assert jwt-validation now receives keycloak_url and keycloak_realm. ## Compatibility Requires kagenti-extensions ≥ the tag that includes rossoctl#383 — older authbridge binaries reject the new fields at DisallowUnknownFields decode. The chart in rossoctl/rossoctl#1507 bumps both pins together. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
cwiklik
self-requested a review
May 29, 2026 14:12
cwiklik
approved these changes
May 29, 2026
cwiklik
left a comment
cwiklik
left a comment
Collaborator
There was a problem hiding this comment.
Well-structured PR with clean commit history, good test coverage (unit + e2e), and proper conditional registration pattern. The fingerprint verification logic is correct — it compares the root CA cert against the ca.crt field (issuing CA) stored in the intermediate's secret, validating the trust chain. All CI checks pass.
Areas reviewed: Go (controller logic, helpers, tests), YAML (RBAC)
Commits: 6 commits, all signed-off: yes
CI status: all 15 checks passing
Add github.com/cert-manager/cert-manager for typed access to Certificate and ClusterIssuer resources. Ref: RHAIENG-4901 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Ian Miller <milleryan2003@gmail.com>
…embly Pure functions with no Kubernetes dependency: - verifyCAFingerprint: SHA256 comparison of root vs intermediate CA - buildCacertsData: assembles Istio cacerts secret key mapping Includes unit tests for matching/mismatching fingerprints, invalid PEM/DER, correct key mapping, and cert-chain concatenation. Ref: RHAIENG-4901 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Ian Miller <milleryan2003@gmail.com>
Event-driven controller watching cert-manager Certificate objects to reconcile cacerts secrets for Istio mTLS. Creates cacerts in istio-system and openshift-ingress with the key format istiod expects. Triggers rollout restarts of istiod and ztunnel when cert data changes. Verifies root CA fingerprints match intermediate issuers; deletes stale intermediates to force re-issuance on mismatch. Registers conditionally via CertManagerCRDExists, following the TektonConfig pattern. Ref: RHAIENG-4901 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Ian Miller <milleryan2003@gmail.com>
Tests with fake client covering certificates missing, not ready, fingerprint mismatch, happy path cacerts creation, data change update, and no-restart when unchanged. Ref: RHAIENG-4901 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Ian Miller <milleryan2003@gmail.com>
End-to-end tests on Kind cluster with cert-manager covering cacerts creation, rebuild after intermediate deletion, restart annotations on istiod and ztunnel, and graceful handling of missing workloads. Ref: RHAIENG-4901 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Ian Miller <milleryan2003@gmail.com>
- Fix gofmt alignment in const block - Replace deprecated result.Requeue with RequeueAfter - Suppress deprecated GetEventRecorderFor (repo-wide, not just ours) - Break long lines in e2e test to stay under 120 chars Ref: RHAIENG-4901 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Ian Miller <milleryan2003@gmail.com>
r3v5
force-pushed
the
shared-trust-controller
branch
from
June 2, 2026 11:17
fd0670b to
64252a2
Compare
- Add Secret watch on cacerts to self-heal from manual drift - Replace time.Sleep in CertManagerCRDExists with wait.PollUntilContextTimeout for cleaner cancellation semantics during operator shutdown - Document certFingerprint first-PEM-block assumption - Document hardcoded config values match RHOAI/OSSM3 layout Ref: RHAIENG-4901 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Ian Miller <milleryan2003@gmail.com>
pdettori
approved these changes
Jun 2, 2026
pdettori
left a comment
pdettori
left a comment
Member
There was a problem hiding this comment.
CI failure analysis: The E2E failure is in the pre-existing "Combined AgentRuntime + AgentCard + Auth Bridge E2E" test, NOT in the SharedTrust code (all 4 SharedTrust tests pass).
Root cause: The controller-manager pod never materializes on re-deploy (second cycle). The test harness runs make undeploy → make uninstall → re-creates namespace → make deploy, but with cert-manager now installed in the cluster (for SharedTrust E2E), namespace termination is slower. The re-create races against the still-terminating namespace, and the pod is never scheduled. Evidence: 120s of polling returns empty ("No resources found in kagenti-operator-system namespace").
This is a test-infra race exposed by cert-manager's presence, not a code defect.
Areas reviewed: Go (controller, helpers, scheme registration, RBAC), CI logs (full E2E timeline analysis)
Commits: 6 commits, all signed-off: yes
CI status: 14/15 passing; E2E failure is unrelated test-infra race
Assisted-By: Claude Code
Replace polling loop with kubectl wait --for=delete when a namespace is still Terminating during re-deploy. With cert-manager installed, namespace finalizers take longer during teardown between test suites, causing a race where namespace creation silently fails. Also increase ensureNamespaceReady timeout from 60s to 120s. Ref: RHAIENG-4901 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Ian Miller <milleryan2003@gmail.com>
r3v5
force-pushed
the
shared-trust-controller
branch
from
June 2, 2026 14:10
4f6a276 to
7d51a33
Compare
github-project-automation
Bot
moved this from New /:ToDo
to Done
in Kagenti Issue Prioritization
Contributor
Author
|
I have submitted follow up PR refactor(ocp-installer): remove operator-owned shared-trust steps for ocp script. |
4 tasks
odh-devops-app Bot
pushed a commit
to opendatahub-io/agents-operator
that referenced
this pull request
Jun 6, 2026
…terRole The Helm chart's hand-maintained ClusterRole (charts/kagenti-operator/ templates/rbac/role.yaml) drifted from the kubebuilder-generated config/rbac/role.yaml: it was missing the cert-manager.io/certificates + clusterissuers get/list/watch rules. PR rossoctl#383 (SharedTrust controller) added a Watches(&cmv1.Certificate{}) informer and the corresponding +kubebuilder:rbac marker, and regenerated config/rbac/role.yaml — but did not update the chart's separate ClusterRole. Since installs render RBAC from the chart, the deployed operator's ServiceAccount cannot list/watch cert-manager Certificates. The Certificate informer never syncs, the manager misses its cache-sync deadline and exits 1 (~2.5 min crashloop), so NO controllers run — including ClientRegistrationReconciler. Agents' per-agent Keycloak client-credentials secrets are therefore never created, and agent pods hang on FailedMount (observed on weather-agent; matches a user report on a fresh main install). Add the cert-manager.io rules to the chart role to match the generated role. Follow-up (not in this PR): the chart ClusterRole is maintained separately from config/rbac/role.yaml with no sync, so any future RBAC marker can silently drift again. Worth generating the chart role from the kubebuilder role or adding a CI drift check. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
varshaprasad96
added a commit
to varshaprasad96/kagenti-operator
that referenced
this pull request
Jun 10, 2026
Update mTLS config delivery from ConfigMap injection to annotation + env var approach per PR rossoctl#405 team review: - Controller sets kagenti.io/mtls-mode annotation on pod template (triggers rolling restart on change, independent of config hash) - Webhook reads mTLSMode from AgentRuntime CR at pod CREATE time, sets MTLS_MODE env var on authbridge container - Acknowledge Istio mTLS coexistence (PR rossoctl#383, rossoctl#399, RHAIENG-5467) - Mark T005 as SUPERSEDED with new annotation-based approach - Update contracts, data-model, research, REVIEWERS.md Jira: RHAIENG-4944 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
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
4 participants
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
cacertssecrets for Istio mTLS in
istio-systemandopenshift-ingressthe new CA chain
re-issuance on mismatch
Jira
RHAIENG-4901
Next steps
When current PR is merged, I will submit this change
kagenti/scripts/ocp/setup-kagenti.sh(remove operator-owned steps from _ensure_rhoai_shared_trust; retain cluster-admin/static install steps)Test plan
graceful missing workload handling
CA chain, ztunnel mTLS with SPIFFE identities, cert rotation triggers rebuild
go test ./internal/controller/ -run "TestReconcile_|TestVerifyCA|TestBuildCacerts" -vexport KIND_EXPERIMENTAL_PROVIDER=podman && CONTAINER_TOOL=podman KIND_CLUSTER=kagenti go test ./test/e2e/ -v -count=1 -timeout 20m --ginkgo.focus="SharedTrust"