Add trust-manager plugin for TLS certificate infrastructure#359
Open
eurijon wants to merge 1 commit into
Open
Conversation
WalkthroughThis PR introduces a new trust-manager plugin that bootstraps a Certificate Authority within cert-manager. The plugin registers as an addon, deploys via Helm, provisions a self-signed CA and ClusterIssuer for certificate signing, validates cert-manager prerequisites, and orchestrates the complete deployment and readiness flow. ChangesTrust-Manager Plugin Initialization
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
eurijon
force-pushed
the
trust-manager-plugin
branch
from
bc62f77 to
51151e2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/trust-manager/files/trust-manager.yaml`:
- Around line 603-610: The Role that grants secrets access is currently scoped
to the cert-manager namespace; change it to a dedicated trust namespace (e.g.,
"trust-manager") and scope both the Role (the resource listing secrets) and its
RoleBinding/ServiceAccount references to that dedicated namespace; also update
any references/flags such as --trust-namespace that point to "cert-manager" to
use the new namespace so the controller only reads secrets from the dedicated
trust namespace (apply same change for the other occurrence noted around lines
799-800).
In `@plugins/trust-manager/tasks/deploy.yaml`:
- Around line 12-24: The task currently named "Wait for trust-manager
deployment" only ensures the Deployment is available but not that the Bundle CRD
exists; add a follow-up check that polls the CRD "bundles.trust.cert-manager.io"
(use kubernetes.core.k8s_info with api_version: apiextensions.k8s.io/v1 and
kind: CustomResourceDefinition and name: bundles.trust.cert-manager.io) and
register it (e.g., __r_trust_manager_crd) with retries/delay until
__r_trust_manager_crd.resources | length > 0 (or until an appropriate condition
on the CRD resource is truthy) before declaring the role successful so
downstream tasks that create Bundle resources will not race the API server.
In `@plugins/trust-manager/tasks/pre-validate.yaml`:
- Around line 2-12: The current preflight uses the "Check cert-manager namespace
exists" task and __r_cert_manager_ns variable to detect install; instead check
cert-manager APIs/controllers instead: replace or add a task that queries the
CRD "certificates.cert-manager.io" via kubernetes.core.k8s_info (api_version:
apiextensions.k8s.io/v1, kind: CustomResourceDefinition, name:
certificates.cert-manager.io) and register its result (e.g.,
__r_cert_manager_crd), and/or query the cert-manager controller Deployment
(api_version: apps/v1, kind: Deployment, name: cert-manager, namespace:
cert-manager) and register (e.g., __r_cert_manager_deploy); then change the fail
task (currently "Fail if cert-manager is not installed") to check those
registers (fail when __r_cert_manager_crd.resources | length == 0 and
__r_cert_manager_deploy.resources | length == 0 or when the deployment has zero
available replicas) so validation only passes when the CRD or controller is
present and running.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1ef65d83-c592-4360-a459-51a06964c63b
📒 Files selected for processing (5)
plugins/trust-manager/files/ca-issuer.yamlplugins/trust-manager/files/trust-manager.yamlplugins/trust-manager/plugin.yamlplugins/trust-manager/tasks/deploy.yamlplugins/trust-manager/tasks/pre-validate.yaml
maorfr
reviewed
May 17, 2026
eurijon
force-pushed
the
trust-manager-plugin
branch
from
51151e2 to
d7f4b65
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/trust-manager/tasks/deploy.yaml`:
- Around line 19-22: The debug task reports the trust-manager as ready but only
checks CRDs; add an explicit readiness gate that waits for the trust-manager
Deployment in the cert-manager namespace to have at least one available replica
(or the Deployment condition "Available" true) before emitting the debug
message; locate the task named "Debug trust-manager status" and precede it with
an Ansible task that queries the Deployment "trust-manager" (e.g., using
kubernetes.core.k8s_info or kubernetes.core.k8s with wait/condition) with a
reasonable timeout/retries and only proceed to the existing debug message once
.status.availableReplicas > 0 (or condition type Available == True).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b5ae7f07-47cb-478c-8d58-160b186bfe38
📒 Files selected for processing (4)
plugins/trust-manager/files/ca-issuer.yamlplugins/trust-manager/plugin.yamlplugins/trust-manager/tasks/deploy.yamlplugins/trust-manager/tasks/pre-validate.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/trust-manager/plugin.yaml
eurijon
force-pushed
the
trust-manager-plugin
branch
from
d7f4b65 to
0dbde58
Compare
Contributor
Author
|
The plugin is not added to the default list of plugins, so it's being tested in a separate CI job in #360 |
eurijon
force-pushed
the
trust-manager-plugin
branch
from
0dbde58 to
2f74c15
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugins/trust-manager/tasks/deploy.yaml (1)
46-56: ⚡ Quick winImprove readability of the condition check.
Line 56 is difficult to read due to its length and complex filter chain. Consider splitting it across multiple lines for better maintainability.
♻️ Proposed refactor for readability
- name: Wait for ClusterIssuer to be ready kubernetes.core.k8s_info: api_version: cert-manager.io/v1 kind: ClusterIssuer name: default-ca register: __r_cluster_issuer retries: 30 delay: 10 until: - __r_cluster_issuer.resources | length > 0 - - __r_cluster_issuer.resources[0].status.conditions | default([]) | selectattr('type', 'equalto', 'Ready') | selectattr('status', 'equalto', 'True') | list | length > 0 + - __r_cluster_issuer.resources[0].status.conditions + | default([]) + | selectattr('type', 'equalto', 'Ready') + | selectattr('status', 'equalto', 'True') + | list + | length > 0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/trust-manager/tasks/deploy.yaml` around lines 46 - 56, The long until condition on the ClusterIssuer readiness is hard to read; split it into two clearer checks by first asserting __r_cluster_issuer.resources | length > 0 and then computing a separate, named expression for the ready-condition (inspect __r_cluster_issuer.resources[0].status.conditions) instead of chaining many filters on one line — e.g., compute a short Jinja variable or set_fact for ready_conditions (filtering status.conditions for type == 'Ready' and status == 'True') and use that variable in the until block to make the readiness check concise and maintainable (refer to ClusterIssuer and __r_cluster_issuer in your changes).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@plugins/trust-manager/tasks/deploy.yaml`:
- Around line 46-56: The long until condition on the ClusterIssuer readiness is
hard to read; split it into two clearer checks by first asserting
__r_cluster_issuer.resources | length > 0 and then computing a separate, named
expression for the ready-condition (inspect
__r_cluster_issuer.resources[0].status.conditions) instead of chaining many
filters on one line — e.g., compute a short Jinja variable or set_fact for
ready_conditions (filtering status.conditions for type == 'Ready' and status ==
'True') and use that variable in the until block to make the readiness check
concise and maintainable (refer to ClusterIssuer and __r_cluster_issuer in your
changes).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ae803e20-45a8-425e-8291-640faa257fdd
📒 Files selected for processing (4)
plugins/trust-manager/files/ca-issuer.yamlplugins/trust-manager/plugin.yamlplugins/trust-manager/tasks/deploy.yamlplugins/trust-manager/tasks/pre-validate.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- plugins/trust-manager/tasks/pre-validate.yaml
- plugins/trust-manager/files/ca-issuer.yaml
- plugins/trust-manager/plugin.yaml
maorfr
approved these changes
May 19, 2026
Contributor
Author
|
Plugin correctly deployed in e2e-OSAC job https://github.com/rh-ecosystem-edge/enclave/actions/runs/26109023827/job/76780823612?pr=360 in #360 |
rporres
approved these changes
May 20, 2026
Contributor
rporres
left a comment
rporres
left a comment
There was a problem hiding this comment.
looks great, the comments are purely nit-picking, address them at your will
Deploys trust-manager v0.20.0 via Helm chart from charts.jetstack.io and a self-signed CA ClusterIssuer into the cert-manager namespace, providing reusable TLS certificate infrastructure for plugins that require it. Pre-validation checks cert-manager CRD and controller are present and running before deployment. Requires cert-manager to be installed. OSAC-218
eurijon
force-pushed
the
trust-manager-plugin
branch
from
2f74c15 to
06cf356
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/trust-manager/tasks/deploy.yaml (1)
36-44: 💤 Low valueConsider multiline formatting for the template lookup expression.
Line 40 is quite long and could benefit from multiline formatting for improved readability, consistent with feedback from past reviews.
♻️ Optional formatting improvement
-- name: Apply CA issuer manifests kubernetes.core.k8s: state: present definition: "{{ item }}" - loop: "{{ lookup('template', plugin_dir ~ '/templates/ca-issuer.yaml.j2') | from_yaml_all | list }}" + loop: >- + {{ + lookup('template', plugin_dir ~ '/templates/ca-issuer.yaml.j2') + | from_yaml_all + | list + }} register: __r_ca_issuer retries: "{{ k8s_retries }}" delay: "{{ k8s_delay }}" until: __r_ca_issuer is success🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/trust-manager/tasks/deploy.yaml` around lines 36 - 44, The long lookup expression in the "Apply CA issuer manifests" task (kubernetes.core.k8s with loop using lookup('template' ...) | from_yaml_all | list) should be broken into multiline form for readability—extract the template lookup and parsing into a separate var or render the lookup across multiple lines inside the loop so it reads like: loop: "{{ lookup('template', plugin_dir ~ '/templates/ca-issuer.yaml.j2') | from_yaml_all | list }}" split across lines or assign that expression to a task/var (e.g., ca_issuer_manifests: "{{ lookup('template', plugin_dir ~ '/templates/ca-issuer.yaml.j2') | from_yaml_all | list }}") and then use loop: "{{ ca_issuer_manifests }}"; update references (__r_ca_issuer, k8s_retries, k8s_delay, until) to use the new var without changing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/trust-manager/tasks/deploy.yaml`:
- Around line 46-61: The task "Wait for ClusterIssuer to be ready" currently
hardcodes retries: 30; change that to use the shared variable retries: "{{
k8s_retries }}" so it follows the same pattern as other wait tasks (the
kubernetes.core.k8s_info task registering __r_cluster_issuer should use the
k8s_retries variable instead of 30). Ensure only the retries value is replaced
and surrounding logic/selectors for __r_cluster_issuer remain untouched.
---
Nitpick comments:
In `@plugins/trust-manager/tasks/deploy.yaml`:
- Around line 36-44: The long lookup expression in the "Apply CA issuer
manifests" task (kubernetes.core.k8s with loop using lookup('template' ...) |
from_yaml_all | list) should be broken into multiline form for
readability—extract the template lookup and parsing into a separate var or
render the lookup across multiple lines inside the loop so it reads like: loop:
"{{ lookup('template', plugin_dir ~ '/templates/ca-issuer.yaml.j2') |
from_yaml_all | list }}" split across lines or assign that expression to a
task/var (e.g., ca_issuer_manifests: "{{ lookup('template', plugin_dir ~
'/templates/ca-issuer.yaml.j2') | from_yaml_all | list }}") and then use loop:
"{{ ca_issuer_manifests }}"; update references (__r_ca_issuer, k8s_retries,
k8s_delay, until) to use the new var without changing behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a15e462b-14d9-4abc-b5b0-1035f9332141
📒 Files selected for processing (4)
plugins/trust-manager/plugin.yamlplugins/trust-manager/tasks/deploy.yamlplugins/trust-manager/tasks/pre-validate.yamlplugins/trust-manager/templates/ca-issuer.yaml.j2
✅ Files skipped from review due to trivial changes (1)
- plugins/trust-manager/templates/ca-issuer.yaml.j2
Contributor
Author
Thanks Rafa, all are addressed now |
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
3 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.
Deploys trust-manager v0.20.0 via Helm chart from charts.jetstack.io and a self-signed CA ClusterIssuer into the cert-manager namespace, providing reusable TLS certificate infrastructure for plugins that require it.
Pre-validation checks cert-manager CRD and controller are present and running before deployment.
Requires cert-manager to be installed.
OSAC-218
Summary by CodeRabbit
Release Notes