Feat: Install cert-manager for SSL certs #122
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes introduce a comprehensive configuration for cert-manager in a Kubernetes environment through a YAML file. This configuration includes global settings, controller settings, webhook configurations, and components like CA Injector and ACME Solver. Additionally, it adds a new entry in the external dependencies configuration, specifying cert-manager's repository, version, and namespace. These modifications enhance the flexibility and control over cert-manager deployment. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Kubernetes
participant CertManager
User->>Kubernetes: Deploy cert-manager
Kubernetes->>CertManager: Configure with values.yaml
CertManager->>Kubernetes: Set up global settings
CertManager->>Kubernetes: Configure controller settings
CertManager->>Kubernetes: Set up webhook configuration
CertManager->>Kubernetes: Initialize CA Injector and ACME Solver
CertManager->>Kubernetes: Run startup API check
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: 7
🧹 Outside diff range and nitpick comments (2)
molecules/cluster-resources/mrsauravsahu.tfvars (1)
47-52: Consider dependency ordering for reliable installation.Since cert-manager often works in conjunction with ingress-nginx, consider explicitly ordering these dependencies. While both are present in the externals list, their installation order matters for proper SSL certificate handling.
Consider:
- Adding a comment documenting the dependency relationship
- Ensuring the installation workflow (likely in another file) respects this ordering
molecules/cluster-resources/config/externals/cert-manager/values.yaml (1)
212-213: Fix YAML formatting issuesThere are several formatting issues in the file:
- Trailing spaces on lines 212-213, 620-621, and 973-974
- Wrong indentation in the webhook configuration
- Missing newline at end of file
Run the file through a YAML formatter to fix these issues.
Also applies to: 620-621, 973-974, 1384-1384
🧰 Tools
🪛 yamllint
[error] 212-212: trailing spaces
(trailing-spaces)
[error] 213-213: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
molecules/cluster-resources/config/externals/cert-manager/values.yaml(1 hunks)molecules/cluster-resources/mrsauravsahu.tfvars(1 hunks)
🧰 Additional context used
🪛 yamllint
molecules/cluster-resources/config/externals/cert-manager/values.yaml
[error] 212-212: trailing spaces
(trailing-spaces)
[error] 213-213: trailing spaces
(trailing-spaces)
[error] 620-620: trailing spaces
(trailing-spaces)
[error] 621-621: trailing spaces
(trailing-spaces)
[warning] 712-712: wrong indentation: expected 6 but found 8
(indentation)
[warning] 715-715: wrong indentation: expected 10 but found 12
(indentation)
[error] 973-973: trailing spaces
(trailing-spaces)
[error] 974-974: trailing spaces
(trailing-spaces)
[error] 1384-1384: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (1)
molecules/cluster-resources/mrsauravsahu.tfvars (1)
47-52: Verify cert-manager values configuration.
The AI summary mentions a values.yaml file at molecules/cluster-resources/config/externals/cert-manager/values.yaml, but it's not included in this review. Ensure that file includes:
- Proper resource requests/limits
- High availability configuration if needed
- Webhook configurations
- RBAC settings
Let's verify the values file exists:
✅ Verification successful
Based on the values.yaml content, I can provide the final response since I have all the necessary information to verify the configuration:
Cert-manager values configuration looks good with some recommendations
The values.yaml file includes essential configurations, but consider the following recommendations for production use:
- Set
replicaCount: 2or3(currently 1) and enablepodDisruptionBudgetfor high availability - Configure resource requests/limits (currently empty
resources: {}) - Review webhook timeoutSeconds (currently set to maximum 30s) if you don't need such a long timeout
- Consider enabling prometheus monitoring (currently enabled but ServiceMonitor/PodMonitor disabled)
The current configuration includes:
- Proper RBAC settings (rbac.create: true)
- Webhook configurations with proper security context
- Node selector configured to run on "pi" node
- Secure defaults for container security context
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if cert-manager values.yaml exists and show its content
if [ -f "molecules/cluster-resources/config/externals/cert-manager/values.yaml" ]; then
cat "molecules/cluster-resources/config/externals/cert-manager/values.yaml"
else
echo "Warning: values.yaml not found at expected location"
fi
Length of output: 50401
| { | ||
| name = "cert-manager" | ||
| repo = "https://charts.jetstack.io" | ||
| version = "1.14.5" | ||
| namespace = "cert-manager" | ||
| }, |
There was a problem hiding this comment.
💡 Codebase verification
Update cert-manager version to v1.16.1 for latest security patches and features
The current configuration uses cert-manager v1.14.5, while the latest stable version is v1.16.1 (released October 2024). Updating to the latest version is recommended to ensure you have the latest security patches and features.
Update the version field:
- version = "1.14.5"
+ version = "1.16.1"🔗 Analysis chain
Verify cert-manager version and consider version constraints.
The cert-manager configuration looks good, but version 1.14.5 might not be the latest stable release. Consider:
- Adding a comment indicating the minimum Kubernetes version requirement
- Using a version constraint (e.g.,
>=1.14.5) for better maintainability
Let's verify the latest stable version:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check latest cert-manager version from their GitHub releases
gh release list --repo jetstack/cert-manager --limit 1
Length of output: 100
| networkPolicy: | ||
| # Create network policies for the webhooks. | ||
| enabled: false |
There was a problem hiding this comment.
Enable network policies for webhook security
Network policies are currently disabled, which could expose the webhook to unnecessary traffic.
Enable network policies to restrict traffic to only necessary sources:
- enabled: false
+ enabled: true📝 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.
| networkPolicy: | |
| # Create network policies for the webhooks. | |
| enabled: false | |
| networkPolicy: | |
| # Create network policies for the webhooks. | |
| enabled: true |
| nodeSelector: | ||
| kubernetes.io/hostname: pi |
There was a problem hiding this comment.
Avoid hardcoding node selector to specific hostname
The current node selector configuration restricts pods to a specific node, which limits flexibility and scalability.
Consider using more flexible node selection criteria:
- nodeSelector:
- kubernetes.io/hostname: pi
+ nodeSelector:
+ kubernetes.io/os: linux📝 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.
| nodeSelector: | |
| kubernetes.io/hostname: pi | |
| nodeSelector: | |
| kubernetes.io/os: linux |
| # This option is equivalent to setting crds.enabled=true and crds.keep=true. | ||
| # Deprecated: use crds.enabled and crds.keep instead. | ||
| installCRDs: true | ||
|
|
||
| crds: | ||
| # This option decides if the CRDs should be installed | ||
| # as part of the Helm installation. | ||
| enabled: false | ||
|
|
||
| # This option makes it so that the "helm.sh/resource-policy": keep | ||
| # annotation is added to the CRD. This will prevent Helm from uninstalling | ||
| # the CRD when the Helm release is uninstalled. | ||
| # WARNING: when the CRDs are removed, all cert-manager custom resources | ||
| # (Certificates, Issuers, ...) will be removed too by the garbage collector. | ||
| keep: true |
There was a problem hiding this comment.
Resolve conflicting CRD installation settings
There's a conflict in the CRD installation configuration:
installCRDs: true(deprecated)crds.enabled: false
This could lead to unexpected behavior during installation.
Apply this diff to use the recommended approach:
-# This option is equivalent to setting crds.enabled=true and crds.keep=true.
-# Deprecated: use crds.enabled and crds.keep instead.
-installCRDs: true
crds:
- # This option decides if the CRDs should be installed
- # as part of the Helm installation.
- enabled: false
+ enabled: true📝 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.
| # This option is equivalent to setting crds.enabled=true and crds.keep=true. | |
| # Deprecated: use crds.enabled and crds.keep instead. | |
| installCRDs: true | |
| crds: | |
| # This option decides if the CRDs should be installed | |
| # as part of the Helm installation. | |
| enabled: false | |
| # This option makes it so that the "helm.sh/resource-policy": keep | |
| # annotation is added to the CRD. This will prevent Helm from uninstalling | |
| # the CRD when the Helm release is uninstalled. | |
| # WARNING: when the CRDs are removed, all cert-manager custom resources | |
| # (Certificates, Issuers, ...) will be removed too by the garbage collector. | |
| keep: true | |
| crds: | |
| enabled: true | |
| # This option makes it so that the "helm.sh/resource-policy": keep | |
| # annotation is added to the CRD. This will prevent Helm from uninstalling | |
| # the CRD when the Helm release is uninstalled. | |
| # WARNING: when the CRDs are removed, all cert-manager custom resources | |
| # (Certificates, Issuers, ...) will be removed too by the garbage collector. | |
| keep: true |
| # | ||
| # For example: | ||
| # requests: | ||
| # cpu: 10m | ||
| # memory: 32Mi | ||
| # | ||
| # For more information, see [Resource Management for Pods and Containers](https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/). | ||
| resources: {} |
There was a problem hiding this comment.
Define resource limits for the controller
Missing resource limits could lead to resource contention and affect cluster stability.
Add resource limits and requests:
-resources: {}
+resources:
+ requests:
+ cpu: 100m
+ memory: 128Mi
+ limits:
+ cpu: 500m
+ memory: 512Mi📝 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.
| # | |
| # For example: | |
| # requests: | |
| # cpu: 10m | |
| # memory: 32Mi | |
| # | |
| # For more information, see [Resource Management for Pods and Containers](https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/). | |
| resources: {} | |
| # | |
| # For example: | |
| # requests: | |
| # cpu: 10m | |
| # memory: 32Mi | |
| # | |
| # For more information, see [Resource Management for Pods and Containers](https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/). | |
| resources: | |
| requests: | |
| cpu: 100m | |
| memory: 128Mi | |
| limits: | |
| cpu: 500m | |
| memory: 512Mi |
| # | ||
| # The default is 1, but in production set this to 2 or 3 to provide high | ||
| # availability. | ||
| # | ||
| # If `replicas > 1`, consider setting `podDisruptionBudget.enabled=true`. | ||
| # | ||
| # Note that cert-manager uses leader election to ensure that there can | ||
| # only be a single instance active at a time. | ||
| replicaCount: 1 |
There was a problem hiding this comment.
Increase controller replicas for high availability
Running a single replica of cert-manager controller in production is not recommended for high availability.
Apply this diff to increase reliability:
-replicaCount: 1
+replicaCount: 2Also, enable PodDisruptionBudget by setting podDisruptionBudget.enabled: true to ensure availability during node maintenance.
Committable suggestion skipped: line range outside the PR's diff.
| # Create PodSecurityPolicy for cert-manager. | ||
| # | ||
| # Note that PodSecurityPolicy was deprecated in Kubernetes 1.21 and removed in Kubernetes 1.25. | ||
| enabled: false |
There was a problem hiding this comment.
Remove deprecated PodSecurityPolicy configuration
The PodSecurityPolicy (PSP) configuration is deprecated since Kubernetes 1.21 and removed in 1.25. Consider using Pod Security Standards (PSS) instead.
Apply this diff to remove the deprecated configuration:
- podSecurityPolicy:
- # Create PodSecurityPolicy for cert-manager.
- #
- # Note that PodSecurityPolicy was deprecated in Kubernetes 1.21 and removed in Kubernetes 1.25.
- enabled: false
- # Configure the PodSecurityPolicy to use AppArmor.
- useAppArmor: trueAnd add Pod Security Standards admission configuration to your cluster.
Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
These changes improve user control and management of cert-manager deployments.