Skip to content

fix: ensure domain defined in acls is included in host rules#884

Merged
steveiliop56 merged 1 commit into
mainfrom
fix/kubernetes-ingress
May 23, 2026
Merged

fix: ensure domain defined in acls is included in host rules#884
steveiliop56 merged 1 commit into
mainfrom
fix/kubernetes-ingress

Conversation

@steveiliop56
Copy link
Copy Markdown
Member

@steveiliop56 steveiliop56 commented May 21, 2026

Fixes GHSA-pcj4-r7vg-3jvw

@sondt99 please review.

Summary by CodeRabbit

  • Bug Fixes
    • Improved Kubernetes Ingress parsing with enhanced hostname extraction and validation
    • Added domain matching validation to ensure apps only use correctly configured ingress entries
    • Enhanced warning messages for incomplete ingress rule configurations

Review Change Stack

@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label May 21, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 21, 2026

📝 Walkthrough

Walkthrough

This PR enhances Kubernetes ingress parsing by extracting ingress hostnames and HTTP paths with validation helpers. When processing ingress updates, it now validates that hosts can be extracted; failed extractions trigger ingress removal. During app-to-ingress mapping, apps whose configured domain does not match extracted ingress hosts are skipped with warnings.

Changes

Kubernetes Ingress Host Extraction and Validation

Layer / File(s) Summary
Ingress parsing helpers
internal/service/kubernetes_service.go
Adds slices package import and defines extractPathsFromRule (extracts HTTP paths from ingress rules, warns if catch-all / is missing) and extractHostsFromRule (extracts hostnames from ingress rules).
Ingress host validation in update flow
internal/service/kubernetes_service.go
Modifies updateFromItem to call the host-extraction helper and cache the result; if extraction fails, removes the ingress from the cache and returns early.
App-to-ingress domain filtering
internal/service/kubernetes_service.go
Filters app-to-ingress mapping to skip apps whose configured domain is not present among extracted ingress hosts, with a warning message.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A tiny rabbit hops through Kubernetes lanes,
Extracting ingress hosts with careful chains,
Apps now match domains, no more strays,
The service stays clean in its filtered ways! 🎯

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main fix: validating that app domains (from ACLs) are included in Kubernetes ingress host rules, which is the core change in the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/kubernetes-ingress

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2026

Codecov Report

❌ Patch coverage is 10.20408% with 44 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/service/kubernetes_service.go 10.20% 41 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown

@sondt99 sondt99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix LGTM from a security perspective.

Approved in CodeRabbit Change Stack

Copy link
Copy Markdown

@sondt99 sondt99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix LGTM from a security perspective.

Approved in CodeRabbit Change Stack

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label May 23, 2026
@steveiliop56 steveiliop56 merged commit 55bef72 into main May 23, 2026
5 checks passed
@steveiliop56 steveiliop56 deleted the fix/kubernetes-ingress branch May 23, 2026 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants