fix(dangling-servicemonitor): an empty namespaceSelector means the monitor's own namespace

[Eljees]

Fixes #1256.

The defect

Prometheus Operator reads an empty NamespaceSelector as "the namespace this ServiceMonitor is in". dangling-servicemonitor had one branch that never looked at namespaces at all:

if labelSelector.Matches(labels.Set(services.Labels)) && labelSelectorSet && !nsSelectorSet {
    // Found!
    return nil
}

A Service with matching labels in any namespace satisfied a monitor that set no namespace selector, so the dangling monitor in the report went unreported.

The change

The namespace is compared on that branch. A manifest that leaves the namespace out on either side carries nothing to compare - a Helm template rendered without one, or a file meant to be applied with -n - and keeps matching as it did. That shape is not hypothetical: the dont-fire pair in tests/checks/dangling-servicemonitor.yml is exactly it, a Service in dontfire with a ServiceMonitor that has no namespace, and it must keep passing.

Red, then green

The new unit case fails on untouched main:

--- FAIL: TestDanglingServiceMonitor/TestEmptyNamespaceSelectorDoesNotReachAnotherNamespace
    expected diagnostics and actual diagnostics do not match
    extra elements in list A:
     "no services found matching the service monitor's label selector (app=service1-test) and namespace selector ([])"

With the change:

$ go test ./pkg/templates/danglingservicemonitor/...
--- PASS: TestDanglingServiceMonitor/TestDanglingNamespaceSelector
--- PASS: TestDanglingServiceMonitor/TestEmptyNamespaceSelectorDoesNotReachAnotherNamespace
--- PASS: TestDanglingServiceMonitor/TestEmptyNamespaceSelectorMatchesItsOwnNamespace
--- PASS: TestDanglingServiceMonitor/TestInvalidSelector
--- PASS: TestDanglingServiceMonitor/TestNamespaceSelector
--- PASS: TestDanglingServiceMonitor/TestNamespacelessManifestStillMatches
--- PASS: TestDanglingServiceMonitor/TestNoDanglingServiceMonitors
--- PASS: TestDanglingServiceMonitor/TestOneDanglingServiceMonitorIsDangling
--- PASS: TestDanglingServiceMonitor/TestServiceMonitorEmpty

go test ./pkg/templates/... is green throughout.

The reproduction from the issue, through the built binary

$ kube-linter lint repro.yaml --do-not-auto-add-defaults --include dangling-servicemonitor
(object: monitoring/application-metrics ...Kind=ServiceMonitor) no services found matching the
service monitor's label selector (app=application) and namespace selector ([])
Error: found 1 lint errors

and the control the issue names, the Service moved into monitoring:

No lint errors found!

e2e

tests/checks/dangling-servicemonitor.yml gains that pair, so the fixture now produces five reports and the bats case asserts the fifth message and a count of five. Run against the built binary:

reports: 5
   ServiceMonitor: ... label selector (app.kubernetes.io/name=app) and namespace selector ([])
   ServiceMonitor: ... label selector (app.kubernetes.io/name=app) and namespace selector ([])
   ServiceMonitor: ... label selector () and namespace selector ([test2])
   ServiceMonitor: ... label selector (app.kubernetes.io/name=app1) and namespace selector ([test2])
   ServiceMonitor: ... label selector (app.kubernetes.io/name=monitoring-vs-application) and namespace selector ([])

AI-assisted (LLM used for drafting); every change verified and tested by me.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Advanced

Run ID: f6a3cd8e-adb7-4f28-8ef3-7515dc2f685e

📥 Commits

Reviewing files that changed from the base of the PR and between 3b485c4 and 204895e.

📒 Files selected for processing (4)

• e2etests/bats-tests.sh
• pkg/templates/danglingservicemonitor/template.go
• pkg/templates/danglingservicemonitor/template_test.go
• tests/checks/dangling-servicemonitor.yml

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Summary

Summary by CodeRabbit

• Bug Fixes
  • Improved dangling ServiceMonitor detection to respect namespace boundaries when no namespace selector is specified.
  • ServiceMonitors now match services in the same namespace, while still supporting resources with unspecified namespaces.
• Tests
  • Added coverage for same-namespace, cross-namespace, and unspecified-namespace scenarios.
  • Updated diagnostics to report all detected dangling ServiceMonitor cases.

Walkthrough

The dangling ServiceMonitor check now treats an empty namespace selector as the monitor namespace. Tests cover cross-namespace, same-namespace, and namespace-less resources. The end-to-end fixture and assertions include the additional diagnostic.

Changes

Dangling ServiceMonitor namespace matching

Layer / File(s)	Summary
Namespace-aware matching pkg/templates/danglingservicemonitor/template.go	The empty namespace-selector path now requires matching namespaces. Empty monitor and service namespaces still match.
Namespace regression coverage pkg/templates/danglingservicemonitor/template_test.go	Helpers and tests cover cross-namespace rejection, same-namespace matching, and namespace-less manifests.
End-to-end validation tests/checks/dangling-servicemonitor.yml, e2etests/bats-tests.sh	The fixture adds a cross-namespace ServiceMonitor case. The end-to-end test expects five reports and validates the new diagnostic.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the dangling-servicemonitor fix and accurately states the empty namespaceSelector behavior.
Description check	✅ Passed	The description directly explains the defect, implementation, preserved behavior, tests, and end-to-end validation for the changeset.
Linked Issues check	✅ Passed	Issue #1256 requires empty namespaceSelector to limit Service discovery to the ServiceMonitor namespace. The implementation adds namespace comparison for this case and keeps manifests with missing n…
Out of Scope Changes check	✅ Passed	All changed files support Issue #1256. The source change implements namespace-aware matching. The unit tests, fixture, and end-to-end assertions provide regression coverage for the required behavior. …

Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 31.72%. Comparing base (dbd7529) to head (204895e).
⚠️ Report is 360 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1262       +/-   ##
===========================================
- Coverage   62.36%   31.72%   -30.64%     
===========================================
  Files         197      239       +42     
  Lines        4854     6594     +1740     
===========================================
- Hits         3027     2092      -935     
- Misses       1439     4326     +2887     
+ Partials      388      176      -212     

Flag	Coverage Δ
unit	31.72% <100.00%> (-30.64%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Eljees]

The one red check here is codecov/project, and it belongs to the repository rather than to this branch: codecov has had no report for main since 13 June 2024, so every pull request is compared against a base hundreds of commits behind. codecov/patch on this branch is green, and so are build-and-test, kube-linter, test-sarif and the Windows sanity test.

The measurement is in #1242, and #1263 carries the two parts of it that do not need repository settings access.