refs platform/board#4082: remove module dependencies

[FabrizioCafolla]

PR Type

Enhancement

---

Description

• Made monitoring module variables optional with defaults

• Changed cluster_name from required to optional for cert-manager and kyverno

• Disabled monitoring alerts by default for kyverno, cert-manager, and konnectivity_agent

• Added conditional log filter generation based on cluster_name presence

---

Diagram Walkthrough

flowchart LR
  A["Module Variables"] -- "add defaults" --> B["Optional Configuration"]
  B -- "cluster_name nullable" --> C["Conditional Log Filters"]
  C -- "enabled=false" --> D["Disabled by Default"]

Loading

File Walkthrough

	Relevant files
Configuration changes	variables.tf Make module variables optional with disabled-by-default alerts variables.tf Added default = {} to cloud_sql, kyverno, cert_manager, and konnectivity_agent variables Changed enabled default from true to false for kyverno, cert-manager, and konnectivity_agent Made cluster_name optional (nullable) for kyverno and cert-manager Added enabled field to cloud_sql configuration +10/-5
Enhancement	cert_manager.tf Add conditional log filter generation for cert-manager      cert_manager.tf Added conditional logic to cert_manager_log_filter based on cluster_name presence Returns empty string when cluster_name is null +2/-1      kyverno.tf Add conditional log filter generation for kyverno                kyverno.tf Added conditional logic to kyverno_log_filter based on cluster_name presence Returns empty string when cluster_name is null +2/-1

---

[sparkfabrik-ai-bot]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Empty Filter Risk When cluster_name is null, cert_manager_log_filter becomes an empty string. This could cause the alert policy to match all logs or fail validation, potentially triggering false alerts or errors. Consider adding validation or a fallback filter to ensure the alert policy remains functional. cert_manager_log_filter = var.cert_manager.cluster_name != null ? (<<-EOT ( ( resource.type="k8s_container" AND resource.labels.project_id="${local.cert_manager_project_id}" AND resource.labels.cluster_name="${var.cert_manager.cluster_name}" AND resource.labels.namespace_name="${var.cert_manager.namespace}" ) OR ( log_id("events") AND resource.labels.project_id="${local.cert_manager_project_id}" AND resource.labels.cluster_name="${var.cert_manager.cluster_name}" AND ( jsonPayload.involvedObject.namespace="${var.cert_manager.namespace}" OR jsonPayload.metadata.namespace="${var.cert_manager.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.cert_manager.filter_extra)} EOT ) : "" Empty Filter Risk Similar to cert-manager, when cluster_name is null, kyverno_log_filter becomes an empty string. This could result in overly broad log matching or resource creation failures. Verify that downstream resources handle empty filters gracefully or add validation. kyverno_log_filter = var.kyverno.cluster_name != null ? (<<-EOT resource.type="k8s_container" AND resource.labels.project_id="${local.kyverno_project_id}" AND resource.labels.cluster_name="${var.kyverno.cluster_name}" AND resource.labels.namespace_name="${var.kyverno.namespace}" AND ( labels."k8s-pod/app_kubernetes_io/component"=~"(admission-controller|background-controller|cleanup-controller|reports-controller)" OR resource.labels.pod_name=~"kyverno-(admission|background|cleanup|reports)-controller-.*" ) AND ( jsonPayload.error=~"(?i)internal error" OR jsonPayload.error=~"(?i)failed calling webhook" OR jsonPayload.error=~"(?i)timeout" OR jsonPayload.error=~"(?i)client-side throttling" OR jsonPayload.error=~"(?i)failed to run warmup" OR jsonPayload.error=~"(?i)schema not found" OR jsonPayload.error=~"(?i)list resources failed" OR jsonPayload.error=~"(?i)failed to watch resource" OR jsonPayload.error=~"(?i)context deadline exceeded" OR jsonPayload.error=~"(?i)i/o timeout" OR jsonPayload.error=~"(?i)is forbidden" OR jsonPayload.error=~"(?i)cannot list resource" OR jsonPayload.error=~"(?i)cannot watch resource" OR jsonPayload.error=~"(?i)RBAC.*denied" OR jsonPayload.error=~"(?i)failed to start watcher" OR jsonPayload.error=~"(?i)failed to acquire lease" OR jsonPayload.error=~"(?i)leader election lost" OR jsonPayload.error=~"(?i)unable to update .*WebhookConfiguration" OR jsonPayload.error=~"(?i)failed to sync" OR jsonPayload.error=~"(?i)dropping request" OR jsonPayload.error=~"(?i)failed to load certificate" OR jsonPayload.error=~"(?i)failed to update lock" OR jsonPayload.error=~"(?i)the object has been modified" OR jsonPayload.error=~"(?i)no matches for kind" OR jsonPayload.error=~"(?i)the server could not find the requested resource" OR jsonPayload.error=~"(?i)Too Many Requests" OR jsonPayload.error=~"(?i)x509" OR jsonPayload.error=~"(?i)is invalid:" OR jsonPayload.error=~"(?i)connection refused" OR jsonPayload.error=~"(?i)no agent available" OR jsonPayload.error=~"(?i)fatal error" OR jsonPayload.error=~"(?i)panic" ) ${trimspace(var.kyverno.filter_extra)} EOT ) : "" Breaking Change Changing enabled default from true to false for kyverno, cert_manager, and konnectivity_agent is a breaking change that will disable existing monitoring alerts for users who upgrade without updating their configuration. Consider documenting this in migration notes or using a different approach to maintain backward compatibility. enabled = optional(bool, false) cluster_name = optional(string, null) project_id = optional(string, null) notification_enabled = optional(bool, true) notification_channels = optional(list(string), []) # Rate limit for notifications, e.g. "300s" for 5 minutes, used only for log match alerts logmatch_notification_rate_limit = optional(string, "300s") alert_documentation = optional(string, null) auto_close_seconds = optional(number, 3600) filter_extra = optional(string, "") namespace = optional(string, "kyverno") }) } variable "cert_manager" { description = "Configuration for cert-manager missing issuer log alert. Allows customization of project, cluster, namespace, notification channels, alert documentation, enablement, extra filters, auto-close timing, and notification rate limiting." default = {} type = object({ enabled = optional(bool, false) cluster_name = optional(string, null) project_id = optional(string, null) namespace = optional(string, "cert-manager") notification_enabled = optional(bool, true) notification_channels = optional(list(string), []) logmatch_notification_rate_limit = optional(string, "300s") alert_documentation = optional(string, null) auto_close_seconds = optional(number, 3600) filter_extra = optional(string, "") }) } variable "konnectivity_agent" { description = "Configuration for Konnectivity agent deployment replica alert in GKE. Triggers when there are no available replicas." default = {} type = object({ enabled = optional(bool, false)

[Copilot]

Pull request overview

Updates this Terraform module to reduce/avoid implicit module dependencies by making several alert configurations optional-by-default and safer when unset.

Changes:

• Add {} defaults to several object-typed variables to avoid forcing callers to provide full module inputs.
• Make kyverno / cert_manager cluster_name optional and guard log filters when it’s unset.
• Change defaults for some alert modules (e.g., enabled defaults) to reduce automatic enablement.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
variables.tf	Makes multiple module configuration inputs optional via {} defaults; adjusts enabled and cluster_name defaults.
kyverno.tf	Makes the Kyverno log filter conditional on cluster_name being set.
cert_manager.tf	Makes the cert-manager log filter conditional on cluster_name being set.

Comments suppressed due to low confidence (2)

kyverno.tf:55

• The count expression for the alert policy uses trimspace(var.kyverno.cluster_name) before checking var.kyverno.cluster_name != null. If enabled = true and cluster_name = null, Terraform will error evaluating trimspace(null) even though the resource should be skipped. Reorder the checks (null check first) or use a null-safe expression like try(trimspace(var.kyverno.cluster_name), "") != "".

  ) : ""
}

resource "google_monitoring_alert_policy" "kyverno_logmatch_alert" {
  count = (

cert_manager.tf:41

• The count expression for the alert policy uses trimspace(var.cert_manager.cluster_name) before checking var.cert_manager.cluster_name != null. If enabled = true and cluster_name = null, Terraform will error evaluating trimspace(null) even though the resource should be skipped. Reorder the checks (null check first) or use a null-safe expression like try(trimspace(var.cert_manager.cluster_name), "") != "".

  ) : ""
}

resource "google_monitoring_alert_policy" "cert_manager_logmatch_alert" {
  count = (

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

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

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (2)

kyverno.tf:12

• local.kyverno_cluster_name is computed (trimmed) but the log filter condition/interpolation still uses var.kyverno.cluster_name directly. If cluster_name is whitespace-only, local.kyverno_cluster_name becomes empty while kyverno_log_filter still renders with a blank/whitespace cluster name. Use the trimmed local both for the conditional and in the filter interpolation so the behavior is consistent.

  kyverno_cluster_name = var.kyverno.cluster_name != null ? trimspace(var.kyverno.cluster_name) : ""
  
  kyverno_log_filter = var.kyverno.cluster_name != null ? (<<-EOT
    resource.type="k8s_container"
    AND resource.labels.project_id="${local.kyverno_project_id}"
    AND resource.labels.cluster_name="${var.kyverno.cluster_name}"
    AND resource.labels.namespace_name="${var.kyverno.namespace}"

cert_manager.tf:19

• local.cert_manager_cluster_name is computed (trimmed) but cert_manager_log_filter still gates on var.cert_manager.cluster_name != null and interpolates var.cert_manager.cluster_name directly. This can yield filters with blank/whitespace cluster names. Use the trimmed local for the condition and interpolation so whitespace-only values behave like “unset”.

  cert_manager_cluster_name = var.cert_manager.cluster_name != null ? trimspace(var.cert_manager.cluster_name) : ""

  cert_manager_log_filter = var.cert_manager.cluster_name != null ? (<<-EOT
    (
      (
        resource.type="k8s_container"
        AND resource.labels.project_id="${local.cert_manager_project_id}"
        AND resource.labels.cluster_name="${var.cert_manager.cluster_name}"
        AND resource.labels.namespace_name="${var.cert_manager.namespace}"

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (2)

cert_manager.tf:24

• cert_manager_cluster_name is trimmed into local.cert_manager_cluster_name, but the events branch of the log filter still interpolates var.cert_manager.cluster_name (line 24). If callers pass whitespace (e.g., " my-cluster "), the count condition will pass (trimmed value non-empty) but the filter will use the untrimmed value and likely never match. Use the local trimmed cluster name consistently in the filter.

        AND resource.labels.cluster_name="${local.cert_manager_cluster_name}"
        AND resource.labels.namespace_name="${var.cert_manager.namespace}"
      )
      OR (
        log_id("events")
        AND resource.labels.project_id="${local.cert_manager_project_id}"
        AND resource.labels.cluster_name="${var.cert_manager.cluster_name}"

variables.tf:128

• Same issue as kyverno: konnectivity_agent has default = {} but enabled defaults to true and validation requires cluster_name when enabled, so omitting konnectivity_agent will fail validation. Default enabled to false (or default = { enabled = false }) to make this input truly optional.

  default     = {}
  type = object({
    enabled               = optional(bool, true)
    cluster_name          = optional(string, null)
    project_id            = optional(string, null)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.