feat: add cert-manager missing issuer log alert configuration and mon…

[filippolmt]

User description

…itoring resource

---

PR Type

Enhancement

---

Description

• Add cert-manager missing issuer log alert configuration

• Implement CloudSQL monitoring alerts for CPU/memory/disk utilization

• Update Kyverno cluster_name parameter to optional

• Add comprehensive monitoring resource documentation

---

Changes walkthrough 📝

	Relevant files
Enhancement	cert_manager_issuer_log_alert.tf Add cert-manager missing issuer alert configuration            cert_manager_issuer_log_alert.tf Create new cert-manager issuer log alert resource Add log filter for missing Issuer/ClusterIssuer detection Configure notification channels and alert documentation Implement auto-close and rate limiting features +69/-0    cloud_sql.tf Implement CloudSQL monitoring alerts infrastructure            cloud_sql.tf Add comprehensive CloudSQL monitoring alerts Implement CPU, memory, and disk utilization thresholds Create dynamic alert policies for multiple instances Configure notification channels and auto-close settings [link]
Configuration changes	variables.tf Add cert-manager variables and update Kyverno config          variables.tf Add cert_manager_issuer variable configuration Change Kyverno cluster_name from required to optional Define alert documentation and notification settings Add filter and auto-close configuration options +17/-1
Documentation	README.md Update documentation for new monitoring features                  README.md Add cert_manager_issuer input documentation Update Kyverno cluster_name parameter description Add new monitoring alert policy resources Update supported services documentation +3/-1

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[sparkfabrik-ai-bot]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Regex Escaping The regex pattern uses double quotes within the filter which may need proper escaping. The pattern Referenced \\"(Issuer|ClusterIssuer)\\" not found should be validated to ensure it correctly matches the expected log messages from cert-manager. textPayload=~"Referenced \\"(Issuer|ClusterIssuer)\\" not found" OR jsonPayload.message=~"Referenced \\"(Issuer|ClusterIssuer)\\" not found" OR jsonPayload.note=~"Referenced \\"(Issuer|ClusterIssuer)\\" not found" ) Filter Logic The log filter combines multiple OR conditions for different log sources (k8s_container and events) but the namespace filtering logic differs between them. This asymmetry could lead to missed or incorrect matches depending on the log source structure. certificate_log_filter = <<-EOT ( ( resource.type="k8s_container" AND resource.labels.project_id="${local.certificate_project_id}" AND resource.labels.cluster_name="${var.certificate.cluster_name}" AND resource.labels.namespace_name="${var.certificate.namespace}" ) OR ( log_id("events") AND resource.labels.project_id="${local.certificate_project_id}" AND resource.labels.cluster_name="${var.certificate.cluster_name}" AND ( jsonPayload.involvedObject.namespace="${var.certificate.namespace}" OR jsonPayload.metadata.namespace="${var.certificate.namespace}" ) ) ) AND ( textPayload=~"Referenced \\"(Issuer|ClusterIssuer)\\" not found" OR jsonPayload.message=~"Referenced \\"(Issuer|ClusterIssuer)\\" not found" OR jsonPayload.note=~"Referenced \\"(Issuer|ClusterIssuer)\\" not found" ) ${trimspace(var.certificate.filter_extra)} EOT Required Field The cluster_name field is marked as required (no default value) but the resource creation logic checks for empty strings. This could lead to runtime errors if an empty string is provided instead of proper validation at the variable level. cluster_name = string

[filippolmt]

/describe

[filippolmt]

/improve

[sparkfabrik-ai-bot]

PR Description updated to latest commit (9541882)

[filippolmt]

/describe

[filippolmt]

/improve

[sparkfabrik-ai-bot]

PR Code Suggestions ✨

Latest suggestions up to 0854d1e

Category	Suggestion	Impact
Possible issue	Add null check before trimspace The resource creation condition only checks if cluster_name is not empty but doesn't validate if it's null. This could cause runtime errors when cluster_name is null and trimspace() is called on it. cert_manager_issuer_log_alert.tf [40-43] resource "google_monitoring_alert_policy" "cert_manager_issuer_logmatch_alert" { count = ( var.cert_manager_issuer.enabled + && var.cert_manager_issuer.cluster_name != null && trimspace(var.cert_manager_issuer.cluster_name) != "" ) ? 1 : 0 Suggestion importance[1-10]: 8 __ Why: This is a critical bug fix that prevents runtime errors when cluster_name is null. The trimspace() function would fail on null values, making this a high-impact improvement for code reliability.	Medium

---

Previous suggestions

Suggestions up to commit 9541882

Category	Suggestion	Impact
Possible issue	Add null check before trimspace The count condition should validate that cluster_name is not null before calling trimspace(). If cluster_name is null, trimspace() will fail with an error. certificate_log_alert.tf [40-43] count = ( var.certificate.enabled + && var.certificate.cluster_name != null && trimspace(var.certificate.cluster_name) != "" ) ? 1 : 0 Suggestion importance[1-10]: 8 __ Why: This is a critical bug fix. Calling trimspace() on a null value would cause a runtime error, and the suggestion correctly identifies this potential issue and provides the proper null check.	Medium

Suggestions up to commit 792e4ac

Category	Suggestion	Impact
Possible issue	Add null check before trimspace The count condition should validate that cluster_name is not null before calling trimspace(). If cluster_name is null, trimspace() will fail with an error. certificate_log_alert.tf [40-43] resource "google_monitoring_alert_policy" "certificate_logmatch_alert" { count = ( var.certificate.enabled + && var.certificate.cluster_name != null && trimspace(var.certificate.cluster_name) != "" ) ? 1 : 0 Suggestion importance[1-10]: 8 __ Why: This is a critical bug fix. The trimspace() function will fail if cluster_name is null, causing Terraform to error during plan/apply. Adding the null check prevents runtime errors.	Medium
	Handle null filter_extra safely The filter_extra variable could be null, which would cause trimspace() to fail. Add a null check or use coalesce() to provide a default empty string. certificate_log_alert.tf [35] -${trimspace(var.certificate.filter_extra)} +${trimspace(coalesce(var.certificate.filter_extra, ""))} Suggestion importance[1-10]: 7 __ Why: This prevents potential runtime errors if filter_extra is null. However, since filter_extra has a default value of "" in the variable definition, this is less critical than the first suggestion.	Medium
General	Handle null cluster_name in interpolation The cluster_name variable is used directly in string interpolation without null checking. If it's null, this could cause unexpected behavior in the log filter. certificate_log_alert.tf [17] -AND resource.labels.cluster_name="${var.certificate.cluster_name}" +AND resource.labels.cluster_name="${coalesce(var.certificate.cluster_name, "")}" Suggestion importance[1-10]: 6 __ Why: While this adds defensive programming, cluster_name is marked as required (no default value) in the variable definition, so it should not be null in practice. The suggestion is technically correct but addresses a less likely scenario.	Low

[sparkfabrik-ai-bot]

PR Description updated to latest commit (0854d1e)

[Copilot]

Pull Request Overview

This PR enhances the Terraform Google Services Monitoring module by adding cert-manager monitoring capabilities, improving CloudSQL monitoring alerts, and making the Kyverno cluster_name parameter optional.

• Added cert-manager missing issuer log alert configuration with comprehensive filtering and notification settings
• Enhanced CloudSQL monitoring alerts with CPU, memory, and disk utilization thresholds for multiple instances
• Updated Kyverno configuration to make cluster_name optional and added null check validation

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
cert_manager.tf	Implements new cert-manager missing issuer/cluster issuer log alert with configurable filtering and notification settings
variables.tf	Adds cert_manager variable configuration block with comprehensive alert customization options
kyverno.tf	Adds null check for cluster_name to prevent errors when parameter is not provided
examples/variables.tf	Updates example configuration to include cert_manager variables and removes deprecated kyverno metric options
examples/main.tf	Adds cert_manager configuration example and removes deprecated kyverno metric threshold settings
README.md	Updates documentation to reflect new cert_manager input variable and monitoring alert policy resources
CHANGELOG.md	Documents version 0.4.0 changes including file renames and new cert-manager functionality

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

The regex pattern is duplicated across three lines. Consider extracting it to a local variable to improve maintainability and reduce the chance of inconsistencies.