forgejo_admin/pal-e-platform
1
0
Fork 1
Code Issues 33 Pull requests 1 Projects Releases Packages Wiki Activity Actions

fix: add monitoring ingress to Keycloak NetworkPolicy + use internal probe URL #116

Closed
forgejo_admin wants to merge 1 commit from 111-fix-keycloak-probe into main
pull from: 111-fix-keycloak-probe
merge into: forgejo_admin:main
Conversation 4 Commits 1 Files changed 2 +2 -1
forgejo_admin commented 2026-03-18 18:36:58 +00:00
Owner
Copy link

Summary

The Keycloak blackbox probe was failing because (1) the Keycloak NetworkPolicy blocked traffic from the monitoring namespace where the blackbox exporter runs, and (2) the probe target used the external Tailscale URL instead of the internal cluster service URL.

Changes

  • terraform/network-policies.tf: Added monitoring namespace ingress rule to netpol_keycloak, matching the pattern used by every other platform service (forgejo, woodpecker, harbor, minio, etc.)
  • terraform/main.tf: Changed Keycloak blackbox probe target from https://keycloak.tail5b443a.ts.net to http://keycloak.keycloak.svc.cluster.local:80, consistent with all other platform service probes

Test Plan

  • tofu fmt passes (verified)
  • tofu validate passes (verified)
  • tofu plan shows NetworkPolicy update adding monitoring ingress (verified)
  • After apply, Keycloak blackbox probe target shows UP in Prometheus/Grafana
  • Confirm NetworkPolicy allows monitoring namespace ingress: kubectl get networkpolicy -n keycloak -o yaml

Review Checklist

  • Passed automated review-fix loop
  • No secrets committed
  • No unnecessary file changes
  • Commit messages are descriptive

Related

  • Closes #111
  • Plan: plan-pal-e-platform

tofu plan Output

# kubernetes_manifest.netpol_keycloak will be updated in-place
~ resource "kubernetes_manifest" "netpol_keycloak" {
    ~ manifest = {
        ~ spec = {
            ~ ingress = [
                  {
                      from = [
                          { namespaceSelector = { matchLabels = { "kubernetes.io/metadata.name" = "tailscale" } } },
                      ]
                  },
                + {
                    + from = [
                        + { namespaceSelector = { matchLabels = { "kubernetes.io/metadata.name" = "monitoring" } } },
                      ]
                  },
              ]
          }
      }
  }

Plan: 0 to add, 1 to change, 0 to destroy.

Note: The probe URL change is inside Helm values for kube_prometheus_stack and will show as a Helm release update during full apply.

## Summary The Keycloak blackbox probe was failing because (1) the Keycloak NetworkPolicy blocked traffic from the monitoring namespace where the blackbox exporter runs, and (2) the probe target used the external Tailscale URL instead of the internal cluster service URL. ## Changes - `terraform/network-policies.tf`: Added `monitoring` namespace ingress rule to `netpol_keycloak`, matching the pattern used by every other platform service (forgejo, woodpecker, harbor, minio, etc.) - `terraform/main.tf`: Changed Keycloak blackbox probe target from `https://keycloak.tail5b443a.ts.net` to `http://keycloak.keycloak.svc.cluster.local:80`, consistent with all other platform service probes ## Test Plan - [ ] `tofu fmt` passes (verified) - [ ] `tofu validate` passes (verified) - [ ] `tofu plan` shows NetworkPolicy update adding monitoring ingress (verified) - [ ] After apply, Keycloak blackbox probe target shows UP in Prometheus/Grafana - [ ] Confirm NetworkPolicy allows monitoring namespace ingress: `kubectl get networkpolicy -n keycloak -o yaml` ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related - Closes #111 - Plan: `plan-pal-e-platform` ### tofu plan Output ``` # kubernetes_manifest.netpol_keycloak will be updated in-place ~ resource "kubernetes_manifest" "netpol_keycloak" { ~ manifest = { ~ spec = { ~ ingress = [ { from = [ { namespaceSelector = { matchLabels = { "kubernetes.io/metadata.name" = "tailscale" } } }, ] }, + { + from = [ + { namespaceSelector = { matchLabels = { "kubernetes.io/metadata.name" = "monitoring" } } }, ] }, ] } } } Plan: 0 to add, 1 to change, 0 to destroy. ``` Note: The probe URL change is inside Helm values for kube_prometheus_stack and will show as a Helm release update during full apply.
fix: add monitoring ingress to Keycloak NetworkPolicy + use internal probe URL
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
Details
ci/woodpecker/pull_request_closed/woodpecker Pipeline failed
Details
12093c1050 
The Keycloak blackbox probe was failing for two reasons:

1. The Keycloak NetworkPolicy only allowed tailscale ingress, blocking the
   blackbox exporter (monitoring namespace) from reaching Keycloak.
2. The probe target used the external Tailscale URL instead of the internal
   cluster service URL, unlike every other platform service probe.

Closes #111

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
forgejo_admin commented 2026-03-18 18:37:31 +00:00
Author
Owner
Copy link

QA Review -- PR #116

Diff Summary

2 files changed, +2 / -1. Minimal, surgical fix.

Review Findings

terraform/network-policies.tf

  • Added monitoring namespace ingress rule to netpol_keycloak. This matches the exact pattern used by every other platform service NetworkPolicy (forgejo, woodpecker, harbor, minio, postgres, cnpg-system). Keycloak was the only one missing it. Correct.

terraform/main.tf

  • Changed Keycloak blackbox probe target from https://keycloak.tail5b443a.ts.net (external Tailscale URL) to http://keycloak.keycloak.svc.cluster.local:80 (internal cluster URL). This is consistent with all other platform probes (forgejo, harbor, argocd, minio). The service name keycloak in namespace keycloak on port 80 matches the kubernetes_service_v1.keycloak resource definition. Correct.

Checks

  • tofu fmt -- clean
  • tofu validate -- passes
  • tofu plan -- shows expected NetworkPolicy update (1 change)
  • No secrets committed
  • No unnecessary file changes
  • Follows established codebase patterns exactly
  • PR body has all required sections and Closes #111

Nits

None.

VERDICT: APPROVE

## QA Review -- PR #116 ### Diff Summary 2 files changed, +2 / -1. Minimal, surgical fix. ### Review Findings **terraform/network-policies.tf** - Added `monitoring` namespace ingress rule to `netpol_keycloak`. This matches the exact pattern used by every other platform service NetworkPolicy (forgejo, woodpecker, harbor, minio, postgres, cnpg-system). Keycloak was the only one missing it. Correct. **terraform/main.tf** - Changed Keycloak blackbox probe target from `https://keycloak.tail5b443a.ts.net` (external Tailscale URL) to `http://keycloak.keycloak.svc.cluster.local:80` (internal cluster URL). This is consistent with all other platform probes (forgejo, harbor, argocd, minio). The service name `keycloak` in namespace `keycloak` on port 80 matches the `kubernetes_service_v1.keycloak` resource definition. Correct. ### Checks - [x] `tofu fmt` -- clean - [x] `tofu validate` -- passes - [x] `tofu plan` -- shows expected NetworkPolicy update (1 change) - [x] No secrets committed - [x] No unnecessary file changes - [x] Follows established codebase patterns exactly - [x] PR body has all required sections and `Closes #111` ### Nits None. --- **VERDICT: APPROVE**
forgejo_admin commented 2026-03-18 18:42:53 +00:00
Author
Owner
Copy link

PR #116 Review

DOMAIN REVIEW

Tech stack: Terraform (OpenTofu) / Kubernetes NetworkPolicies / Helm values

NetworkPolicy change (network-policies.tf):

  • The added monitoring ingress rule on line 132 follows the exact pattern used by all other platform services: forgejo (line 45), woodpecker (line 66), harbor (line 88), minio (line 111), postgres (line 153), cnpg-system (line 192). Structurally identical -- single namespaceSelector with kubernetes.io/metadata.name = "monitoring".
  • The placement after the tailscale rule is consistent with other policies that have monitoring ingress.
  • depends_on reference to kubernetes_namespace_v1.keycloak is unchanged and correct.

Probe URL change (main.tf):

  • Changed from https://keycloak.tail5b443a.ts.net (external Tailscale funnel) to http://keycloak.keycloak.svc.cluster.local:80 (cluster-internal).
  • Verified against the Keycloak service definition at line 1981-2001 of main.tf: service name is keycloak, namespace is keycloak, port is 80. The URL http://keycloak.keycloak.svc.cluster.local:80 is correct.
  • This matches the pattern used by all other platform-tier probes (grafana, alertmanager, harbor, argocd, minio) which all use internal svc.cluster.local URLs. The external URLs are reserved for app-tier probes (pal-e-docs, pal-e-app) which intentionally validate the full funnel path.
  • Protocol change from https to http is correct: internal cluster traffic does not go through Tailscale TLS termination.

Terraform hygiene:

  • tofu plan output in the PR body confirms: 0 to add, 1 to change, 0 to destroy. Clean in-place update.
  • tofu fmt and tofu validate reported as passing.
  • No state-breaking changes. The kubernetes_manifest resource is updated in-place.

BLOCKERS

None.

This is a 2-line infrastructure fix with no new functionality requiring tests. The changes are pure configuration -- NetworkPolicy ingress rules and a probe URL. The test plan appropriately includes post-apply verification steps (Prometheus target status, kubectl NetworkPolicy inspection).

NITS

None.

SOP COMPLIANCE

  • Branch named after issue: 111-fix-keycloak-probe references issue #111
  • PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related sections all present
  • Related references plan slug: plan-pal-e-platform referenced, closes #111
  • No secrets committed: no credentials, tokens, or sensitive values in the diff
  • No unnecessary file changes: exactly 2 files changed, both directly related to the fix
  • Commit messages are descriptive: PR title follows conventional commits (fix: ...)
  • tofu plan output included in PR body per repo conventions

PROCESS OBSERVATIONS

  • MTTR impact: This fix directly improves Mean Time To Recovery by restoring Keycloak monitoring visibility. A down Keycloak probe means auth issues go undetected -- fixing this closes a blind spot.
  • Pattern consistency: The fact that Keycloak was the only platform service missing the monitoring ingress rule suggests it was likely added to the platform after the initial NetworkPolicy pass. Worth noting for future service onboarding -- monitoring ingress should be part of the namespace+netpol checklist.
  • Change failure risk: Very low. The NetworkPolicy change is additive (opening a path, not closing one). The probe URL change moves from an external path that was failing to an internal path consistent with all other platform probes.

VERDICT: APPROVED

## PR #116 Review ### DOMAIN REVIEW **Tech stack**: Terraform (OpenTofu) / Kubernetes NetworkPolicies / Helm values **NetworkPolicy change (`network-policies.tf`)**: - The added monitoring ingress rule on line 132 follows the exact pattern used by all other platform services: forgejo (line 45), woodpecker (line 66), harbor (line 88), minio (line 111), postgres (line 153), cnpg-system (line 192). Structurally identical -- single `namespaceSelector` with `kubernetes.io/metadata.name = "monitoring"`. - The placement after the `tailscale` rule is consistent with other policies that have monitoring ingress. - `depends_on` reference to `kubernetes_namespace_v1.keycloak` is unchanged and correct. **Probe URL change (`main.tf`)**: - Changed from `https://keycloak.tail5b443a.ts.net` (external Tailscale funnel) to `http://keycloak.keycloak.svc.cluster.local:80` (cluster-internal). - Verified against the Keycloak service definition at line 1981-2001 of `main.tf`: service name is `keycloak`, namespace is `keycloak`, port is 80. The URL `http://keycloak.keycloak.svc.cluster.local:80` is correct. - This matches the pattern used by all other platform-tier probes (grafana, alertmanager, harbor, argocd, minio) which all use internal `svc.cluster.local` URLs. The external URLs are reserved for app-tier probes (pal-e-docs, pal-e-app) which intentionally validate the full funnel path. - Protocol change from `https` to `http` is correct: internal cluster traffic does not go through Tailscale TLS termination. **Terraform hygiene**: - `tofu plan` output in the PR body confirms: 0 to add, 1 to change, 0 to destroy. Clean in-place update. - `tofu fmt` and `tofu validate` reported as passing. - No state-breaking changes. The `kubernetes_manifest` resource is updated in-place. ### BLOCKERS None. This is a 2-line infrastructure fix with no new functionality requiring tests. The changes are pure configuration -- NetworkPolicy ingress rules and a probe URL. The test plan appropriately includes post-apply verification steps (Prometheus target status, kubectl NetworkPolicy inspection). ### NITS None. ### SOP COMPLIANCE - [x] Branch named after issue: `111-fix-keycloak-probe` references issue #111 - [x] PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related sections all present - [x] Related references plan slug: `plan-pal-e-platform` referenced, closes #111 - [x] No secrets committed: no credentials, tokens, or sensitive values in the diff - [x] No unnecessary file changes: exactly 2 files changed, both directly related to the fix - [x] Commit messages are descriptive: PR title follows conventional commits (`fix: ...`) - [x] `tofu plan` output included in PR body per repo conventions ### PROCESS OBSERVATIONS - **MTTR impact**: This fix directly improves Mean Time To Recovery by restoring Keycloak monitoring visibility. A down Keycloak probe means auth issues go undetected -- fixing this closes a blind spot. - **Pattern consistency**: The fact that Keycloak was the only platform service missing the monitoring ingress rule suggests it was likely added to the platform after the initial NetworkPolicy pass. Worth noting for future service onboarding -- monitoring ingress should be part of the namespace+netpol checklist. - **Change failure risk**: Very low. The NetworkPolicy change is additive (opening a path, not closing one). The probe URL change moves from an external path that was failing to an internal path consistent with all other platform probes. ### VERDICT: APPROVED
forgejo_admin commented 2026-03-24 20:21:32 +00:00
Author
Owner
Copy link

PR #116 Stale PR Audit

FINDINGS

Both changes proposed by this PR are already present in main via other paths:

1. Keycloak NetworkPolicy monitoring ingress -- Already on main at terraform/network-policies.tf line 135:

{ from = [{ namespaceSelector = { matchLabels = { "kubernetes.io/metadata.name" = "monitoring" } } }] },

Main also has a basketball-api ingress rule (line 134) that this branch lacks, confirming the branch is based on a stale version of main.

2. Keycloak blackbox probe URL -- The PR changes from https://keycloak.tail5b443a.ts.net to http://keycloak.keycloak.svc.cluster.local:80. Main already uses an even more specific internal URL: http://keycloak.keycloak.svc.cluster.local:80/realms/master (line 440).

The PR is marked mergeable: false (conflicts with main), which corroborates that these changes landed through a different path.

RECOMMENDATION

CLOSE this PR. All proposed changes are already in main. Issue #111 is correctly marked done on the board. This PR is stale and would only introduce merge conflicts.

VERDICT: NOT APPROVED

Reason: PR is stale -- all changes already landed in main via other work. Recommend closing without merge.

## PR #116 Stale PR Audit ### FINDINGS Both changes proposed by this PR are **already present in main** via other paths: **1. Keycloak NetworkPolicy monitoring ingress** -- Already on main at `terraform/network-policies.tf` line 135: ``` { from = [{ namespaceSelector = { matchLabels = { "kubernetes.io/metadata.name" = "monitoring" } } }] }, ``` Main also has a `basketball-api` ingress rule (line 134) that this branch lacks, confirming the branch is based on a stale version of main. **2. Keycloak blackbox probe URL** -- The PR changes from `https://keycloak.tail5b443a.ts.net` to `http://keycloak.keycloak.svc.cluster.local:80`. Main already uses an even more specific internal URL: `http://keycloak.keycloak.svc.cluster.local:80/realms/master` (line 440). The PR is marked `mergeable: false` (conflicts with main), which corroborates that these changes landed through a different path. ### RECOMMENDATION **CLOSE this PR.** All proposed changes are already in main. Issue #111 is correctly marked done on the board. This PR is stale and would only introduce merge conflicts. ### VERDICT: NOT APPROVED Reason: PR is stale -- all changes already landed in main via other work. Recommend closing without merge.
forgejo_admin commented 2026-03-24 20:42:02 +00:00
Author
Owner
Copy link

Closing — both changes in this PR already landed in main via other PRs:

  • Monitoring ingress to Keycloak NetworkPolicy: present at network-policies.tf:135
  • Internal Keycloak probe URL: present at main.tf:440 (uses /realms/master path)

PR has merge conflicts confirming the work was done another way. Related issue #111 is done on the board.

Closing — both changes in this PR already landed in main via other PRs: - Monitoring ingress to Keycloak NetworkPolicy: present at `network-policies.tf:135` - Internal Keycloak probe URL: present at `main.tf:440` (uses `/realms/master` path) PR has merge conflicts confirming the work was done another way. Related issue #111 is done on the board.
forgejo_admin closed this pull request 2026-03-24 20:42:36 +00:00
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
Details
ci/woodpecker/pull_request_closed/woodpecker Pipeline failed
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
domain:backend

   
domain:devops

   
domain:frontend

   
status:approved

QA passed, awaiting merge approval

  
status:in-progress

Dev agent is actively working

  
status:needs-fix

QA found issues, back to dev

  
status:qa

PR submitted, awaiting QA review

  
type:bug

Bug fix

  
type:devops

Infrastructure/CI/config work

  
type:feature

New functionality

No labels
domain:backend
domain:devops
domain:frontend
status:approved
status:in-progress
status:needs-fix
status:qa
type:bug
type:devops
type:feature
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
forgejo_admin/pal-e-platform!116
No description provided.