Update cert-manager #595
Update cert-manager #595
Conversation
dosubot
bot
added
the
size:XXL
WalkthroughThe pull request introduces comprehensive updates to the cert-manager Helm chart, focusing on version upgrades, enhanced configuration options, and improved documentation. The changes span multiple files across the chart, including Changes
Sequence DiagramsequenceDiagram
participant User
participant HelmChart
participant Kubernetes
participant CertManager
participant Prometheus
User->>HelmChart: Configure values
HelmChart->>Kubernetes: Deploy CRDs
HelmChart->>Kubernetes: Create RBAC resources
HelmChart->>Kubernetes: Deploy Cert-Manager components
Kubernetes->>CertManager: Start components
CertManager->>Prometheus: Expose metrics (optional)
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (15)
packages/system/cert-manager/charts/cert-manager/templates/extras-objects.yaml (2)
1-4: Implementation looks good, consider adding template documentation.The template correctly implements dynamic object generation using Helm's templating features. This provides great flexibility for extending cert-manager's functionality.
Consider adding template documentation to explain:
- The purpose of this template
- The expected structure of
extraObjects- Example usage
+{{/* +@description Renders additional Kubernetes objects defined in values.extraObjects +@example +extraObjects: + - apiVersion: v1 + kind: ConfigMap + metadata: + name: extra-config + data: + key: value +*/}} {{ range .Values.extraObjects }} --- {{ tpl . $ }} {{ end }}🧰 Tools
🪛 yamllint (1.35.1)
[error] 4-4: syntax error: expected '', but found '{'
(syntax)
1-4: Consider adding validation for extraObjects.While the implementation is functional, it might benefit from some basic validation to prevent runtime errors.
Consider adding validation:
+{{- if .Values.extraObjects }} +{{- if not (kindIs "slice" .Values.extraObjects) }} +{{- fail "Expected .Values.extraObjects to be a list" }} +{{- end }} {{- range .Values.extraObjects }} +{{- if not (kindIs "map" .) }} +{{- fail "Each item in .Values.extraObjects must be a map" }} +{{- end }} --- {{ tpl . $ }} {{- end }} +{{- end }}🧰 Tools
🪛 yamllint (1.35.1)
[error] 4-4: syntax error: expected '', but found '{'
(syntax)
packages/system/cert-manager/charts/cert-manager/templates/cainjector-poddisruptionbudget.yaml (1)
20-27: Consider documenting PDB configuration in values.yaml.The PDB implementation is solid across all cert-manager components. To help users understand the available options and their implications, consider adding documentation in
values.yamlthat explains:
- The default behavior (
minAvailable: 1)- How to configure
minAvailableormaxUnavailable- Recommendations for different deployment scenarios (e.g., production vs development)
packages/system/cert-manager/charts/cert-manager/values.schema.json (2)
2070-2074: Consider a more conservative default timeout value.While the documentation explains the rationale for setting
timeoutSecondsto 30 (maximum allowed), this could potentially mask underlying connection issues. Consider:
- Setting a lower default value (e.g., 10 seconds)
- Providing guidance in the description for increasing the value when needed
- Adding monitoring recommendations to track webhook response times
986-1005: Add schema-level validation for mutually exclusive monitoring options.While the description warns that PodMonitor and ServiceMonitor are mutually exclusive, consider adding schema-level validation to prevent both from being enabled simultaneously. This could be achieved using JSON Schema's
oneOfordependencieskeywords.packages/system/cert-manager/charts/cert-manager/templates/cainjector-service.yaml (1)
20-29: Consider enhancing service configuration flexibility.While the implementation is correct, consider these improvements:
- Make the metrics port configurable through Values
- Use a more specific port name (e.g.,
cainjector-metrics) to avoid potential conflicts- Consider using a label selector template to reduce duplication with metadata labels
Example improvement:
ports: - protocol: TCP - port: 9402 - name: http-metrics + port: {{ .Values.cainjector.metrics.port | default 9402 }} + name: cainjector-metrics selector: - app.kubernetes.io/name: {{ include "cainjector.name" . }} - app.kubernetes.io/instance: {{ .Release.Name }} - app.kubernetes.io/component: "cainjector" + {{- include "cainjector.selectorLabels" . | nindent 4 }}packages/system/cert-manager/charts/cert-manager/templates/webhook-mutating-webhook.yaml (1)
Line range hint
18-48: Architectural change: Removal of UPDATE operation.The removal of the UPDATE operation from the webhook configuration suggests an architectural change in how cert-manager handles certificate request updates. This change aligns with the principle of immutable CertificateRequest resources.
This is a positive change as it enforces immutability of CertificateRequest resources, which is a recommended pattern for Kubernetes controllers. New requests should be created instead of updating existing ones, improving traceability and reducing potential race conditions.
packages/system/cert-manager/charts/cert-manager/templates/controller-config.yaml (1)
1-19: Well-structured controller configuration template!The implementation follows the same pattern as other components and includes all necessary metadata and labels.
Consider adding a comment block at the top of the file documenting the expected structure of
.Values.configfor better maintainability.🧰 Tools
🪛 yamllint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/cert-manager/charts/cert-manager/templates/cainjector-deployment.yaml (2)
47-54: Consider making metrics port configurable.The Prometheus metrics port (9402) is hardcoded. Consider making it configurable through values for better flexibility.
- prometheus.io/port: '9402' + prometheus.io/port: {{ .Values.cainjector.metricsPort | default "9402" | quote }}
98-106: Consider adding health checks for metrics endpoint.When Prometheus is enabled, consider adding readiness/liveness probes for the metrics endpoint to ensure proper monitoring of the metrics server health.
packages/system/cert-manager/charts/cert-manager/templates/webhook-deployment.yaml (2)
46-53: Consider making metrics port configurable.Similar to the cainjector, the Prometheus metrics port (9402) is hardcoded. Consider making it configurable through values.
- prometheus.io/port: '9402' + prometheus.io/port: {{ .Values.webhook.metricsPort | default "9402" | quote }}
126-130: Consider adding health checks for metrics endpoint.When Prometheus is enabled, consider adding readiness/liveness probes for the metrics endpoint to ensure proper monitoring of the metrics server health.
packages/system/cert-manager/charts/cert-manager/values.yaml (3)
305-321: Consider enhancing security context documentation.While the security contexts are well-configured following Pod Security Standards, consider adding documentation about:
- The security implications of these settings
- Why these specific restrictions are necessary
- The impact on workload compatibility
488-603: Consider adding validation for monitoring configuration.While the Prometheus monitoring configuration is comprehensive, consider adding validation to:
- Prevent simultaneous enabling of PodMonitor and ServiceMonitor
- Validate TLS configuration when enabled
- Ensure consistent port configurations
632-658: Consider enhancing webhook security validation.While the webhook configuration is comprehensive, consider adding:
- Validation for TLS configuration completeness
- Checks for secure port ranges in different environments
- Documentation about security implications of port choices
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
packages/system/cert-manager-crds/charts/cert-manager/Chart.yaml(2 hunks)packages/system/cert-manager-crds/charts/cert-manager/README.md(56 hunks)packages/system/cert-manager/charts/cert-manager/Chart.yaml(2 hunks)packages/system/cert-manager/charts/cert-manager/README.md(4 hunks)packages/system/cert-manager/charts/cert-manager/templates/NOTES.txt(1 hunks)packages/system/cert-manager/charts/cert-manager/templates/_helpers.tpl(2 hunks)packages/system/cert-manager/charts/cert-manager/templates/cainjector-config.yaml(1 hunks)packages/system/cert-manager/charts/cert-manager/templates/cainjector-deployment.yaml(6 hunks)packages/system/cert-manager/charts/cert-manager/templates/cainjector-poddisruptionbudget.yaml(1 hunks)packages/system/cert-manager/charts/cert-manager/templates/cainjector-rbac.yaml(1 hunks)packages/system/cert-manager/charts/cert-manager/templates/cainjector-service.yaml(1 hunks)packages/system/cert-manager/charts/cert-manager/templates/controller-config.yaml(1 hunks)packages/system/cert-manager/charts/cert-manager/templates/deployment.yaml(6 hunks)packages/system/cert-manager/charts/cert-manager/templates/extras-objects.yaml(1 hunks)packages/system/cert-manager/charts/cert-manager/templates/networkpolicy-egress.yaml(0 hunks)packages/system/cert-manager/charts/cert-manager/templates/networkpolicy-webhooks.yaml(1 hunks)packages/system/cert-manager/charts/cert-manager/templates/poddisruptionbudget.yaml(1 hunks)packages/system/cert-manager/charts/cert-manager/templates/podmonitor.yaml(1 hunks)packages/system/cert-manager/charts/cert-manager/templates/rbac.yaml(6 hunks)packages/system/cert-manager/charts/cert-manager/templates/service.yaml(2 hunks)packages/system/cert-manager/charts/cert-manager/templates/serviceaccount.yaml(1 hunks)packages/system/cert-manager/charts/cert-manager/templates/servicemonitor.yaml(3 hunks)packages/system/cert-manager/charts/cert-manager/templates/startupapicheck-job.yaml(3 hunks)packages/system/cert-manager/charts/cert-manager/templates/startupapicheck-rbac.yaml(1 hunks)packages/system/cert-manager/charts/cert-manager/templates/webhook-config.yaml(2 hunks)packages/system/cert-manager/charts/cert-manager/templates/webhook-deployment.yaml(9 hunks)packages/system/cert-manager/charts/cert-manager/templates/webhook-mutating-webhook.yaml(2 hunks)packages/system/cert-manager/charts/cert-manager/templates/webhook-poddisruptionbudget.yaml(1 hunks)packages/system/cert-manager/charts/cert-manager/templates/webhook-rbac.yaml(3 hunks)packages/system/cert-manager/charts/cert-manager/templates/webhook-service.yaml(2 hunks)packages/system/cert-manager/charts/cert-manager/templates/webhook-validating-webhook.yaml(1 hunks)packages/system/cert-manager/charts/cert-manager/values.schema.json(1 hunks)packages/system/cert-manager/charts/cert-manager/values.yaml(6 hunks)
💤 Files with no reviewable changes (1)
- packages/system/cert-manager/charts/cert-manager/templates/networkpolicy-egress.yaml
✅ Files skipped from review due to trivial changes (2)
- packages/system/cert-manager/charts/cert-manager/templates/serviceaccount.yaml
- packages/system/cert-manager-crds/charts/cert-manager/Chart.yaml
🧰 Additional context used
🪛 yamllint (1.35.1)
packages/system/cert-manager/charts/cert-manager/templates/podmonitor.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/cert-manager/charts/cert-manager/templates/servicemonitor.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/cert-manager/charts/cert-manager/templates/controller-config.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/cert-manager/charts/cert-manager/templates/service.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/cert-manager/charts/cert-manager/templates/deployment.yaml
[error] 71-71: trailing spaces
(trailing-spaces)
[error] 157-157: trailing spaces
(trailing-spaces)
[error] 231-231: no new line character at the end of file
(new-line-at-end-of-file)
packages/system/cert-manager/charts/cert-manager/templates/cainjector-config.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/cert-manager/charts/cert-manager/templates/extras-objects.yaml
[error] 4-4: syntax error: expected '', but found '{'
(syntax)
packages/system/cert-manager/charts/cert-manager/templates/cainjector-service.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🪛 Markdownlint (0.37.0)
packages/system/cert-manager/charts/cert-manager/README.md
22-22: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
59-59: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
68-68: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
407-407: null
Bare URL used
(MD034, no-bare-urls)
416-416: null
Bare URL used
(MD034, no-bare-urls)
627-627: null
Bare URL used
(MD034, no-bare-urls)
packages/system/cert-manager-crds/charts/cert-manager/README.md
22-22: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
68-68: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
627-627: null
Bare URL used
(MD034, no-bare-urls)
🪛 LanguageTool
packages/system/cert-manager/charts/cert-manager/README.md
[style] ~97-~97: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...v1.ACMEChallengeSolverHTTP01Ingress). For example, secretTemplate in CertificateS...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[typographical] ~147-~147: If you want to indicate numerical ranges or time ranges, consider using an en dash.
Context: ... verbosity of cert-manager. A range of 0 - 6, with 6 being the most verbose. #### *...
(DASH_RULE)
[grammar] ~157-~157: Did you mean “LED” (= diode, light)?
Context: ...l attempting to acquire leadership of a led but unrenewed leader slot. This is effe...
(LED)
[typographical] ~296-~296: It seems that a comma is missing.
Context: ...the services are installed into. If not set then they use the namespace of the rele...
(IF_NOT_COMMA)
[uncategorized] ~303-~303: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...o these annotations (some resources use eg. "cainjector.name" which resolves to the...
(E_G)
[formatting] ~415-~415: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...rt-manager. To disable the auto-approval, because eg. you are using approver-policy, you ...
(COMMA_BEFORE_BECAUSE)
[uncategorized] ~415-~415: The abbreviation “e.g.” (= for example) requires two periods.
Context: .... To disable the auto-approval, because eg. you are using approver-policy, you can ...
(E_G)
[style] ~531-~531: To form a complete sentence, be sure to include a subject or ‘there’.
Context: ...ch they should be applied to ClusterIP. Can be IPv4 and/or IPv6. #### **podDnsPoli...
(MISSING_IT_THERE)
[style] ~548-~548: To form a complete sentence, be sure to include a subject.
Context: ...iases for cert-manager-controller pods. May be useful when performing ACME DNS-01 s...
(MISSING_IT_THERE)
[misspelling] ~573-~573: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...e HTTP_PROXY environment variable where a HTTP proxy is required. #### **https_p...
(EN_A_VS_AN)
[misspelling] ~577-~577: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... HTTPS_PROXY environment variable where a HTTP proxy is required. #### **no_prox...
(EN_A_VS_AN)
[misspelling] ~581-~581: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...the NO_PROXY environment variable where a HTTP proxy is required, but certain dom...
(EN_A_VS_AN)
[style] ~654-~654: Consider a shorter alternative to avoid wordiness.
Context: ...r Pod. This is enabled by default, in order to enable the clock-skew liveness probe th...
(IN_ORDER_TO_PREMIUM)
[style] ~674-~674: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...ager-webhook Deployments. Note that you can not enable both PodMonitor and ServiceMonit...
(CAN_NOT_PREMIUM)
[style] ~959-~959: To form a complete sentence, be sure to include a subject.
Context: ...minimum available pods for disruptions. Can either be set to an integer (e.g. 1) or...
(MISSING_IT_THERE)
[style] ~965-~965: To form a complete sentence, be sure to include a subject.
Context: ...ximum unavailable pods for disruptions. Can either be set to an integer (e.g. 1) or...
(MISSING_IT_THERE)
[style] ~1175-~1175: To form a complete sentence, be sure to include a subject or ‘there’.
Context: ...ch they should be applied to ClusterIP. Can be IPv4 and/or IPv6. #### **webhook.ima...
(MISSING_IT_THERE)
[formatting] ~1249-~1249: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...S EKS) with custom. CNI (such as calico), because control-plane managed by AWS cannot com...
(COMMA_BEFORE_BECAUSE)
[style] ~1258-~1258: This phrase is redundant. Consider using “outside”.
Context: ...seful if you want to expose the webhook outside of the cluster. In some cases, the control...
(OUTSIDE_OF)
packages/system/cert-manager-crds/charts/cert-manager/README.md
[style] ~97-~97: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...v1.ACMEChallengeSolverHTTP01Ingress). For example, secretTemplate in CertificateS...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[formatting] ~415-~415: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...rt-manager. To disable the auto-approval, because eg. you are using approver-policy, you ...
(COMMA_BEFORE_BECAUSE)
[uncategorized] ~415-~415: The abbreviation “e.g.” (= for example) requires two periods.
Context: .... To disable the auto-approval, because eg. you are using approver-policy, you can ...
(E_G)
[style] ~654-~654: Consider a shorter alternative to avoid wordiness.
Context: ...r Pod. This is enabled by default, in order to enable the clock-skew liveness probe th...
(IN_ORDER_TO_PREMIUM)
[style] ~959-~959: To form a complete sentence, be sure to include a subject.
Context: ...minimum available pods for disruptions. Can either be set to an integer (e.g. 1) or...
(MISSING_IT_THERE)
[style] ~965-~965: To form a complete sentence, be sure to include a subject.
Context: ...ximum unavailable pods for disruptions. Can either be set to an integer (e.g. 1) or...
(MISSING_IT_THERE)
[formatting] ~1249-~1249: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...S EKS) with custom. CNI (such as calico), because control-plane managed by AWS cannot com...
(COMMA_BEFORE_BECAUSE)
🔇 Additional comments (44)
packages/system/cert-manager/charts/cert-manager/templates/poddisruptionbudget.yaml (1)
20-27: LGTM! The PDB configuration follows best practices.The implementation correctly handles the following scenarios:
- Sets a safe default of
minAvailable: 1when no values are provided- Allows explicit configuration of either
minAvailableormaxUnavailable- Maintains mutual exclusivity between
minAvailableandmaxUnavailablepackages/system/cert-manager/charts/cert-manager/templates/webhook-poddisruptionbudget.yaml (1)
20-27: LGTM! Consistent with controller PDB configuration.The implementation maintains consistency with the controller PDB while properly scoping to webhook-specific values.
packages/system/cert-manager/charts/cert-manager/templates/cainjector-poddisruptionbudget.yaml (1)
20-27: LGTM! Consistent PDB configuration across all cert-manager components.The implementation maintains consistency with other cert-manager components while properly scoping to cainjector-specific values.
packages/system/cert-manager/charts/cert-manager/values.schema.json (3)
1-5: Well-structured root schema definition!The schema follows JSON Schema best practices with proper Draft-07 declaration and root reference structure.
Also applies to: 2133-2135
585-597: Robust security defaults aligned with best practices!The security configurations across components demonstrate strong security posture:
- Non-root execution
- Minimal capabilities
- Read-only filesystem
- Runtime default seccomp profile
Also applies to: 1194-1203, 1725-1737, 1987-1996
1-2135: Excellent documentation quality throughout the schema!The schema provides:
- Comprehensive descriptions for properties
- Practical configuration examples
- Relevant links to Kubernetes documentation
- Clear guidance for security configurations
packages/system/cert-manager/charts/cert-manager/templates/cainjector-service.yaml (1)
1-19: Well-structured conditional rendering and metadata!The implementation follows Kubernetes and Helm best practices:
- Proper conditional creation based on monitoring configuration
- Consistent naming using helper templates
- Standard Kubernetes label schema
- Flexible customization through Values
🧰 Tools
🪛 yamllint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/cert-manager/charts/cert-manager/templates/networkpolicy-webhooks.yaml (1)
15-17: LGTM! Improved label structure following Kubernetes best practices.The podSelector now uses standardized
app.kubernetes.io/*labels, which aligns with Kubernetes recommended label practices.Let's verify if any existing deployments rely on the removed
.Values.webhook.podLabelsfunctionality:✅ Verification successful
Label changes are safe to proceed
The removal of
.Values.webhook.podLabelsand the 'app' label has no impact on existing configurations. No instances of custom pod labels or dependencies on the old label pattern were found in the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for custom webhook pod labels in existing values files # Look for any values files that might be setting custom webhook pod labels # Search for webhook.podLabels in values files rg -l "webhook:\s*\n.*podLabels:" --type yaml # Search for direct references to the removed 'app' label rg -l "app: cert-manager-webhook" --type yamlLength of output: 287
Script:
#!/bin/bash # Search with proper multiline patterns # Check for webhook.podLabels in values files rg -U "webhook:(\n|\r\n).*podLabels:" --type yaml # Check default values file fd -t f "values.yaml" -x cat {} \; | grep -A 5 "webhook:" # Look for any references to the old label pattern rg -U "app:\s*cert-manager-webhook" --type yaml # Check for any other potential custom label configurations rg -U "webhook:(\n|\r\n).*labels:" --type yamlLength of output: 2504
packages/system/cert-manager/charts/cert-manager/templates/webhook-service.yaml (2)
21-26: LGTM! Excellent addition of IP family configuration support.The implementation correctly adds support for dual-stack networking through
ipFamilyPolicyandipFamiliesfields, following Kubernetes best practices. This enhancement allows for better control over the IP stack configuration of the webhook service.
35-40: LGTM! Verify metrics endpoint configuration in your environment.The metrics port configuration is correctly implemented with the standard cert-manager webhook metrics port (9402). The conditional logic ensures the metrics endpoint is only exposed when Prometheus is enabled but podmonitor is not used.
Please verify the metrics endpoint accessibility after deployment:
✅ Verification successful
Metrics configuration is correctly implemented and consistent
The metrics port configuration in the webhook service is verified to be:
- Consistent with other cert-manager components using the standard port 9402
- Properly configured with the "http-metrics" targetPort
- Correctly conditional based on prometheus settings
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the metrics port configuration in the deployed service # Check if the service has the metrics port configured when prometheus is enabled kubectl get svc -n cert-manager {{ template "webhook.fullname" . }} -o jsonpath='{.spec.ports[?(@.name=="metrics")].port}'Length of output: 224
Script:
#!/bin/bash # Search for port 9402 references and http-metrics targetPort echo "=== Checking port 9402 usage ===" rg "9402" --type yaml echo -e "\n=== Checking http-metrics port definitions ===" rg "http-metrics" --type yaml echo -e "\n=== Checking prometheus configuration patterns ===" rg "prometheus\.enabled|prometheus\.podmonitor" --type yaml packages/system/cert-manager/charts/cert-manager/templates/Length of output: 7832
packages/system/cert-manager/charts/cert-manager/templates/NOTES.txt (1)
1-3: Enhance the deprecation warning visibility and guidance.While the warning is functional, consider these improvements for better user experience:
- Make the warning more visible using ASCII art or multiple warning symbols
- Include an example of the new
crds.enabledsyntax- Add a link to migration documentation if available
{{- if .Values.installCRDs }} -⚠️ WARNING: `installCRDs` is deprecated, use `crds.enabled` instead. +⚠️ =============== WARNING =============== ⚠️ +The `installCRDs` value is deprecated and will be removed in a future release. +Please use `crds.enabled: true` instead. +For migration steps, see: https://cert-manager.io/docs/installation/helm/ +======================================== {{- end }}Let's verify the values configuration:
packages/system/cert-manager/charts/cert-manager/templates/webhook-mutating-webhook.yaml (2)
18-21: Verify the impact of the new namespaceSelector.The addition of namespaceSelector provides more fine-grained control over which namespaces the webhook operates on. While this is a security enhancement, it could affect existing deployments if not properly configured.
Run this script to check existing webhook configurations and identify potential impacts:
✅ Verification successful
The namespaceSelector addition is safe and provides optional security enhancement.
The default configuration (
namespaceSelector: {}) allows the webhook to operate on all namespaces, maintaining backward compatibility. This change adds the ability to restrict webhook operations to specific namespaces if needed, following Kubernetes best practices for security hardening.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check existing webhook configurations and their namespace selectors # Get current webhook configuration echo "Current webhook configuration:" kubectl get mutatingwebhookconfiguration -l app.kubernetes.io/name=cert-manager -o yaml | grep -A 10 "namespaceSelector:" # List namespaces with cert-manager resources echo -e "\nNamespaces with cert-manager resources:" kubectl get certificaterequests --all-namespaces -o custom-columns=NAMESPACE:.metadata.namespace,NAME:.metadata.name --no-headers | sort -u -k1,1Length of output: 632
Script:
#!/bin/bash # Check values.yaml for default namespaceSelector configuration echo "=== Checking values.yaml for namespaceSelector configuration ===" fd values.yaml --exec grep -l "namespaceSelector" {} \; | while read -r file; do echo "=== Found in: $file ===" cat "$file" | grep -A 5 "namespaceSelector:" done echo -e "\n=== Checking for related documentation ===" fd . -e md -e txt | xargs grep -l "namespaceSelector" | while read -r file; do echo "=== Found in: $file ===" cat "$file" | grep -B 2 -A 2 "namespaceSelector" doneLength of output: 7113
30-30: Verify the impact of restricting resources to 'certificaterequests'.The resource scope has been narrowed from "/" to specifically "certificaterequests". This is a security improvement but requires verification of compatibility with existing cert-manager workflows.
Run this script to check for potential impacts:
✅ Verification successful
Restricting mutating webhook to 'certificaterequests' is correct and secure
The change from "/" to "certificaterequests" is a positive security improvement that:
- Correctly limits the webhook to only the resources it needs to mutate
- Maintains full functionality as the webhook only processes certificaterequests during creation
- Follows the principle of least privilege by removing unnecessary broad permissions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for cert-manager resources that might be affected by the webhook scope change # Check for any cert-manager resources being mutated echo "Checking audit logs for cert-manager webhook mutations:" kubectl get --raw "/apis/audit.k8s.io/v1/events" | jq '.items[] | select(.objectRef.resource != "certificaterequests" and .objectRef.apiGroup == "cert-manager.io" and .responseStatus.code == 200 and .annotations["mutation.webhook.admission.k8s.io/round_0_index_0"] != null)'Length of output: 493
Script:
#!/bin/bash # Search for webhook configurations and related files echo "=== Webhook Configurations ===" fd -e yaml -e yml . packages/system/cert-manager/charts --exec grep -l "MutatingWebhookConfiguration" {} \; echo -e "\n=== Webhook Implementation ===" rg -l "webhook" packages/system/cert-manager/ echo -e "\n=== Documentation ===" fd README.md . packages/system/cert-manager/ --exec cat {} \;Length of output: 127477
Script:
#!/bin/bash # Search for webhook configurations and related files echo "=== Webhook Configuration ===" rg -A 10 "MutatingWebhookConfiguration" packages/system/cert-manager/charts/cert-manager/templates/ echo -e "\n=== Webhook Implementation ===" rg -A 5 "mutating" packages/system/cert-manager/charts/cert-manager/templates/ echo -e "\n=== Related Documentation ===" rg -l "webhook" packages/system/cert-manager/charts/cert-manager/Length of output: 5721
Script:
#!/bin/bash # Get complete webhook configuration echo "=== Complete Webhook Configuration ===" cat packages/system/cert-manager/charts/cert-manager/templates/webhook-mutating-webhook.yaml echo -e "\n=== Webhook Values Configuration ===" cat packages/system/cert-manager/charts/cert-manager/values.yaml | grep -A 20 "webhook:" echo -e "\n=== README Content ===" cat packages/system/cert-manager/charts/cert-manager/README.md | grep -A 10 -B 10 "webhook"Length of output: 21487
packages/system/cert-manager/charts/cert-manager/templates/_helpers.tpl (3)
176-188: Well-structured image URL generation template!The implementation follows best practices by:
- Properly handling optional registry prefix
- Correctly prioritizing digest over tag when both are present
- Maintaining standardization across cert-manager organization
190-202: Excellent validation for CRD configuration options!The template properly validates and prevents misconfigurations between deprecated
.installCRDsand new.crds.enabled/.crds.keepoptions, with clear error messages to guide users.
155-155: Verify the impact of removing the default creator value.The removal of the default "helm" value in the creator comparison makes the configuration more explicit but might affect existing deployments.
Run this script to check for potential impact:
✅ Verification successful
Default creator value removal is safe
The change is safe as the creator value is explicitly set to "helm" in all relevant values.yaml files, maintaining the same behavior while promoting explicit configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for explicit creator value settings in values files rg -l 'creator:' | while read -r file; do echo "=== $file ===" rg -A 1 'creator:' "$file" doneLength of output: 564
packages/system/cert-manager/charts/cert-manager/templates/webhook-config.yaml (1)
2-4: Clean implementation of webhook configuration defaults!The changes properly set default values for
apiVersionandkindwhile maintaining the ability to override them through values.Also applies to: 18-18
packages/system/cert-manager/charts/cert-manager/templates/cainjector-config.yaml (1)
1-19: Well-structured cainjector configuration template!The implementation maintains consistency with other component configurations and includes all necessary metadata and labels.
Let's verify the configuration structure across all components for consistency:
✅ Verification successful
Configuration structure verified - perfectly consistent!
The CAInjector configuration template follows the same pattern as the controller and webhook components, with matching structure for conditional creation, apiVersion domains, and configuration kinds.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Compare configuration structure across components echo "=== Controller Config ===" rg -A 5 'Values.config' packages/system/cert-manager/charts/cert-manager/templates/controller-config.yaml echo -e "\n=== Webhook Config ===" rg -A 5 'Values.webhook.config' packages/system/cert-manager/charts/cert-manager/templates/webhook-config.yaml echo -e "\n=== CAInjector Config ===" rg -A 5 'Values.cainjector.config' packages/system/cert-manager/charts/cert-manager/templates/cainjector-config.yamlLength of output: 1442
🧰 Tools
🪛 yamllint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/cert-manager/charts/cert-manager/Chart.yaml (3)
2-3: LGTM: Enhanced metadata annotations.The new annotations improve discoverability on ArtifactHub by properly categorizing the chart and specifying the license.
11-12: LGTM: Updated URLs.The home and icon URLs have been updated to point to the official cert-manager website and community repository respectively.
8-9: Verify compatibility with the API version upgrade.The chart has been upgraded from v1.12.3 to v1.16.3 and the API version from v1 to v2. This is a significant update that spans multiple minor versions.
Let's verify the changelog and migration notes:
Also applies to: 26-26
✅ Verification successful
API version upgrade is compatible and safe
The upgrade from cert-manager v1.12.3 to v1.16.3 and the Chart API version change from v1 to v2 are both standard and safe changes:
- The Chart API version v2 is the current standard for Helm charts
- The cert-manager version upgrade includes security fixes, bug fixes, and feature additions with no breaking changes
- The changes are backward compatible and well-tested through multiple releases
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for breaking changes in cert-manager releases # between v1.12.3 and v1.16.3 # Fetch the changelog gh api repos/cert-manager/cert-manager/releases | \ jq -r '.[] | select(.tag_name | startswith("v1.") and (. >= "v1.12.3" and . <= "v1.16.3")) | "\(.tag_name)\n\(.body)"'Length of output: 52342
Script:
#!/bin/bash # Check for Helm chart documentation and version compatibility fd "README.md|UPGRADING.md|CHANGELOG.md" packages/system/cert-manager/charts/cert-manager/ # Check for any Chart.yaml history to understand API version changes git log -p packages/system/cert-manager/charts/cert-manager/Chart.yamlLength of output: 151
Script:
#!/bin/bash # Read the chart's README.md cat packages/system/cert-manager/charts/cert-manager/README.md # Look for any version-specific documentation rg -A 5 "apiVersion: v2" packages/system/cert-manager/charts/cert-manager/Length of output: 60431
packages/system/cert-manager/charts/cert-manager/templates/service.yaml (2)
1-1: LGTM: Enhanced monitoring configuration logic.The condition prevents duplicate metrics endpoints by ensuring the service is only created when Prometheus is enabled but podmonitor is not.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
22-27: LGTM: Added IP family configuration.The new IP family settings enhance network configuration flexibility by supporting:
ipFamilyPolicyfor controlling the IP family behavioripFamiliesfor specifying the supported IP families (e.g., IPv4, IPv6)packages/system/cert-manager/charts/cert-manager/templates/startupapicheck-rbac.yaml (1)
21-21: Verify the RBAC permission change impact.The resource type has been changed from "certificates" to "certificaterequests". This is a significant change in permissions that needs verification.
Let's check the startup API check implementation to confirm this change:
✅ Verification successful
RBAC change to certificaterequests is correct and improves security
The change from "certificates" to "certificaterequests" is appropriate for the startup API check job. This job only needs to verify the webhook endpoints are working by creating test certificate requests, and doesn't need permissions to manage full certificates. This change follows the principle of least privilege by granting only the necessary permissions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the startupapicheck implementation's resource requirements # Search for resource usage in the startupapicheck code rg -A 5 "cert-manager.io" $(fd -t f -e go startupapicheck)Length of output: 80833
Script:
#!/bin/bash # Search for startup API check implementation and test files echo "=== Searching for startup API check implementation files ===" fd -t f startupapicheck echo -e "\n=== Searching for startup API check content in Go files ===" rg -g "*.go" -l "startupapicheck" echo -e "\n=== Searching for startup API check test files ===" fd -t f -e test.go startupapicheck echo -e "\n=== Searching for startup API check documentation ===" rg -g "*.md" "startup.?api.?check"Length of output: 12674
Script:
#!/bin/bash # Get startupapicheck job implementation echo "=== Checking startupapicheck job template ===" cat packages/system/cert-manager/charts/cert-manager/templates/startupapicheck-job.yaml echo -e "\n=== Searching for certificaterequest usage in startupapicheck context ===" rg -A 5 -B 5 "certificaterequest.*startupapicheck"Length of output: 3895
packages/system/cert-manager/charts/cert-manager/templates/servicemonitor.yaml (3)
1-3: LGTM: Added monitoring configuration validation.The mutual exclusivity check prevents configuration conflicts by ensuring only one monitoring method (podmonitor or servicemonitor) is enabled at a time.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
32-48: LGTM: Enhanced label selector configuration.The switch to
matchExpressionsprovides more flexible and precise label matching, correctly targeting all cert-manager components (controller, cainjector, and webhook).
60-62: LGTM: Added support for additional endpoint properties.The new configuration allows for customization of endpoint properties through
.Values.prometheus.servicemonitor.endpointAdditionalProperties.packages/system/cert-manager/charts/cert-manager/templates/podmonitor.yaml (1)
1-63: LGTM: Well-structured PodMonitor implementation.The new PodMonitor implementation:
- Maintains consistency with ServiceMonitor configuration
- Uses the same robust label selector strategy
- Provides equivalent monitoring capabilities
🧰 Tools
🪛 yamllint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/cert-manager/charts/cert-manager/templates/webhook-rbac.yaml (1)
18-26: LGTM! Secure RBAC configuration for metrics CA Secret.The conditional block properly scopes access to the metrics CA Secret when dynamic TLS is enabled and the Secret is in the same namespace as cert-manager. The permissions follow the principle of least privilege.
packages/system/cert-manager/charts/cert-manager/templates/startupapicheck-job.yaml (1)
40-40: LGTM! Well-structured job configuration.The changes follow Kubernetes best practices:
- Proper use of enableServiceLinks
- Consistent image templating
- Standard environment variable configuration
Also applies to: 50-51, 63-70
packages/system/cert-manager/charts/cert-manager/templates/cainjector-rbac.yaml (1)
104-156: LGTM! Consistent RBAC pattern for metrics TLS.The Role and RoleBinding follow the same secure pattern as the webhook component, maintaining consistency across the cert-manager components.
packages/system/cert-manager/charts/cert-manager/templates/cainjector-deployment.yaml (1)
19-22: LGTM! Proper handling of revisionHistoryLimit.The conditional check ensures the field is only included when explicitly set, following Helm best practices.
packages/system/cert-manager/charts/cert-manager/templates/webhook-deployment.yaml (1)
70-72: Verify security implications of hostNetwork.When hostNetwork is enabled, ensure:
- The webhook's port doesn't conflict with host ports
- Network policies are properly configured
- Security implications are documented
✅ Verification successful
Security implications of hostNetwork are properly documented and mitigated
The cert-manager webhook's hostNetwork configuration is well-managed with proper security controls:
- Network policies are in place (networkpolicy-egress.yaml, networkpolicy-webhooks.yaml)
- Port configuration (10250) is documented with warnings about conflicts
- Documentation explains when hostNetwork is needed (e.g., EKS with custom CNI)
- Secure by default (hostNetwork: false)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if any documentation mentions hostNetwork security implications rg -l "hostNetwork" README.md docs/Length of output: 81
Script:
#!/bin/bash # Find relevant files and search for hostNetwork and port configurations fd -e md -e yaml -e yml . | xargs rg -l "hostNetwork|ports:|containerPort|NetworkPolicy" # Search specifically in values.yaml files fd "values.yaml" . | xargs rg "webhook.*port|hostNetwork" -A 5 -B 5 # Look for any security-related comments near hostNetwork rg "hostNetwork" -B 3 -A 3Length of output: 66548
packages/system/cert-manager/charts/cert-manager/templates/deployment.yaml (5)
18-21: LGTM: Proper implementation of revisionHistoryLimitThe implementation correctly handles the edge case where the value could be 0 by using a list comparison instead of a simple truthiness check.
46-48: LGTM: Improved Prometheus monitoring conditionThe condition has been enhanced to check both servicemonitor and podmonitor settings, preventing potential monitoring configuration conflicts.
67-76: LGTM: Well-structured config volume mountThe implementation properly handles conditional volume mounting with appropriate checks for both config and volumes.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 71-71: trailing spaces
(trailing-spaces)
140-142: LGTM: New auto-approval toggle featureThe addition of the
disableAutoApprovalflag provides a clean way to disable the built-in certificate request approver, which is useful when using external approvers like approver-policy.
229-231: LGTM: Standard hostAliases configurationThe hostAliases configuration follows Kubernetes best practices for DNS resolution customization.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 231-231: no new line character at the end of file
(new-line-at-end-of-file)
packages/system/cert-manager/charts/cert-manager/templates/rbac.yaml (4)
42-44: LGTM! Well-structured RBAC binding.The ServiceAccount reference in the RoleBinding is properly implemented using templating functions for both the name and namespace, following Kubernetes RBAC best practices.
48-88: Security-conscious implementation of ServiceAccount token request permissions.The new Role and RoleBinding are well-implemented with:
- Proper scope limiting token creation to the service account's own tokens
- Following the principle of least privilege
- Clear labeling and metadata
444-463: Well-structured implementation of aggregated cluster roles.The new cluster-view ClusterRole is properly implemented with:
- Correct aggregation labels for the cluster-reader role
- Properly scoped read-only permissions
- Conditional creation based on the aggregateClusterRoles flag
Line range hint
520-567: Flexible and secure implementation of certificate approval permissions.The auto-approval configuration is well-implemented with:
- Configurable signer names for fine-grained control
- Proper scoping of approval permissions
- Clear conditional logic for enabling/disabling the feature
packages/system/cert-manager/charts/cert-manager/README.md (1)
22-22: Version updates are consistent and well-documented.The version updates in the installation commands are consistent with the upgrade from v1.12.3 to v1.16.3.
Also applies to: 32-32
🧰 Tools
🪛 Markdownlint (0.37.0)
22-22: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
packages/system/cert-manager-crds/charts/cert-manager/README.md (1)
22-22: Version updates are consistent across documentation.The version updates in the CRDs documentation match the main cert-manager documentation.
Also applies to: 32-32
🧰 Tools
🪛 Markdownlint (0.37.0)
22-22: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
| {{- with .Values.webhook.validatingWebhookConfiguration.namespaceSelector }} | ||
| namespaceSelector: | ||
| matchExpressions: | ||
| - key: "cert-manager.io/disable-validation" | ||
| operator: "NotIn" | ||
| values: | ||
| - "true" | ||
| - key: "name" | ||
| operator: "NotIn" | ||
| values: | ||
| - {{ include "cert-manager.namespace" . }} | ||
| {{- toYaml . | nindent 6 }} | ||
| {{- end }} |
There was a problem hiding this comment.
💡 Codebase verification
Safety mechanism partially removed in namespaceSelector configuration
The default configuration maintains the cert-manager.io/disable-validation label check but removes a critical safety mechanism: the automatic exclusion of cert-manager's own namespace. This could potentially lead to deadlocks during cert-manager upgrades or restarts.
values.yaml: Add default matchExpression to exclude cert-manager's namespace inwebhook.validatingWebhookConfiguration.namespaceSelector- Consider adding template validation to ensure cert-manager's namespace is always excluded, regardless of user configuration
🔗 Analysis chain
Verify safety implications of flexible namespaceSelector configuration.
The change from hardcoded matchExpressions to a templated namespaceSelector configuration introduces flexibility but removes two important safety mechanisms:
- Prevention of webhook validation in namespaces explicitly marked for exclusion
- Prevention of potential deadlocks by excluding cert-manager's own namespace
Let's verify if these safety mechanisms are documented or handled elsewhere:
Consider:
- Documenting the recommended namespaceSelector configuration to prevent validation deadlocks
- Adding validation in the template to ensure cert-manager's namespace is always excluded
- Adding a warning in the values schema about the security implications of misconfiguration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the safety mechanisms are documented in values.yaml or README
echo "Checking for namespace selector documentation..."
rg -A 5 "webhook\.validatingWebhookConfiguration\.namespaceSelector" --type yaml
echo "Checking for potential deadlock prevention documentation..."
rg -i "cert-manager.*namespace.*exclude|disable.*validation" --type md
Length of output: 1679
Script:
#!/bin/bash
# Check values.yaml for default configuration
echo "Checking values.yaml for default configuration..."
rg -A 10 "webhook:" packages/system/cert-manager/charts/cert-manager/values.yaml
# Check for upgrade/migration documentation
echo -e "\nChecking for upgrade/migration documentation..."
fd -t f -e md . packages/system/cert-manager | xargs rg -i "upgrade|migration|breaking.?changes" -A 5
Length of output: 2864
Script:
#!/bin/bash
# Search for validatingWebhookConfiguration in values.yaml
echo "Checking for validatingWebhookConfiguration defaults..."
rg -B 5 -A 10 "validatingWebhookConfiguration:" packages/system/cert-manager/charts/cert-manager/values.yaml
# Also check the default values in Chart.yaml
echo -e "\nChecking Chart.yaml for any relevant configurations..."
cat packages/system/cert-manager/charts/cert-manager/Chart.yaml
Length of output: 1766
3 participants
Bumped the embedded cert-manager chart to the latest upstream version.
Summary by CodeRabbit
Based on the comprehensive changes across multiple files in the cert-manager Helm chart, here are the release notes:
New Features
Improvements
Bug Fixes
installCRDsoption in favor of more explicit settingsChores