Remove metadata IP blocklist from DB check URL validation

[nickmisasi]

Summary

• Removes the cloud metadata IP blocklist (169.254.169.254, 100.100.100.200, fd00:ec2::254) and associated DNS resolution from validateDBCheckURL
• Removes the hostnameResolver / defaultResolveHostnameIPs functions and test stub
• Keeps scheme validation (blocking file://, gopher://, etc.) which provides effective SSRF mitigation

Why

The metadata IP blocklist was vulnerable to DNS rebinding (TOCTOU between validation at reconcile time and actual request at pod runtime), performed DNS resolution in the reconcile loop adding latency and fragility, and was inherently incomplete. Scheme validation is the effective layer here; network-level controls (e.g., NetworkPolicies) are the correct place to block metadata endpoint access.

Test plan

• All existing DB check URL tests pass with metadata cases removed
• Scheme validation tests still cover file://, gopher://, ftp://, javascript: blocking
• Per-db-type scheme validation still enforced (e.g. MySQL rejects mysql:// scheme)

🤖 Generated with Claude Code

Summary by CodeRabbit

• Refactor

  • Simplified database URL validation logic.

• Tests

  • Removed related test cases for database configuration validation.

[mm-cloud-bot]

@nickmisasi: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

I understand the commands that are listed here

[nickmisasi]

/release-note-none

[nickmisasi]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

📝 Walkthrough

Walkthrough

This change removes IP range filtering and DNS resolution logic from database external URL validation. The validateDBCheckURL function now performs only basic URL parsing, scheme validation, and hostname presence checks, eliminating metadata IP blocking behaviour and associated test coverage.

Changes

Cohort / File(s)	Summary
Core Implementation pkg/mattermost/database_external.go	Removed metadata IP initialization, DNS resolution helpers (hostnameResolver, defaultResolveHostnameIPs), and IP range blocking logic from validateDBCheckURL. Function now only parses URL, validates scheme, and ensures hostname exists. Updated imports by removing net, context, and time.
Test Coverage pkg/mattermost/database_external_test.go	Removed stubResolver mock helper, eliminated DNS lookup test scaffolding, and deleted test subtests validating metadata IP blocking and hostname resolution checks. Removed net and strings imports.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and accurately summarizes the main change: removing the metadata IP blocklist from DB check URL validation, which aligns with the core modifications across both files.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/remove-metadata-ip-blocklist

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/mattermost/database_external.go`:
- Around line 177-179: The current URL validation only checks parsed.Hostname(),
allowing literal IPs (e.g., 169.254.169.254) which is an SSRF risk; change the
validation in the same block that checks parsed.Hostname() to also call
net.ParseIP(parsed.Hostname()) and return an error if it returns non-nil (i.e.,
the host is a literal IP). Import the "net" package if needed and keep the
existing DNS/name checks intact while deterministically rejecting literal IP
hosts (refer to the parsed variable and the hostname check where you added `if
parsed.Hostname() == ""`).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 07054181-bc00-49e2-941b-3a54e29028b9

📥 Commits

Reviewing files that changed from the base of the PR and between b0d8e38 and 97bd6bc.

📒 Files selected for processing (2)

• pkg/mattermost/database_external.go
• pkg/mattermost/database_external_test.go

💤 Files with no reviewable changes (1)

• pkg/mattermost/database_external_test.go