no-jira: Add localhost resolution within the CoreDNS Corefile for cloud platforms#6228
no-jira: Add localhost resolution within the CoreDNS Corefile for cloud platforms#6228sadasu wants to merge 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
sadasu
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@sadasu: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe CoreDNS static Pod manifest (and its template) gains a ChangesCoreDNS /etc/hosts integration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sadasu The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
templates/common/cloud-platform-alt-dns/files/coredns.yaml (1)
31-33: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConstrain
hostPathtype for/etc/hoststo a file.Line 31-33 should set
hostPath.type: Fileto prevent accidental/non-file path binding and tighten hostPath behavior.Suggested patch
- name: hosts-dir hostPath: path: "/etc/hosts" + type: File🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/common/cloud-platform-alt-dns/files/coredns.yaml` around lines 31 - 33, The hosts-dir volume in the coredns.yaml file is using a hostPath without specifying a type constraint. Add a type field with the value File to the hostPath object for the hosts-dir volume (lines 31-33) to explicitly specify that the path must be a file, preventing accidental binding to non-file paths and improving security posture.manifests/cloud-platform-alt-dns/coredns.yaml (1)
27-29: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winHarden
hostPathwithtype: File.Line 27-29 should explicitly set
type: Filefor/etc/hoststo enforce expected path kind.Suggested patch
- name: hosts-dir hostPath: path: "/etc/hosts" + type: File🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@manifests/cloud-platform-alt-dns/coredns.yaml` around lines 27 - 29, The hostPath volume definition for the hosts-dir volume mounting /etc/hosts lacks an explicit type specification, which should be hardened for security and clarity. Add the type field with value File to the hostPath configuration to explicitly enforce that the path refers to a file rather than leaving it unspecified. This makes the expected path kind explicit and hardens the volume definition.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@manifests/cloud-platform-alt-dns/coredns.yaml`:
- Around line 61-63: The hosts-dir volume mount at mountPath "/etc/hosts" lacks
the readOnly: true flag, violating least-privilege principles for hostPath
access. Add readOnly: true to the volumeMounts entry for the hosts-dir volume to
make the mount read-only. This same issue appears in two locations (line 62 and
line 81), so apply the readOnly: true flag to both hosts-dir volumeMounts
entries to ensure all containers can only read (not modify) the node's
/etc/hosts file.
In `@templates/common/cloud-platform-alt-dns/files/coredns.yaml`:
- Around line 65-67: The volumeMount entries for the hosts-dir volume that
mounts /etc/hosts are missing the readOnly: true property, which violates
least-privilege security principles by allowing containers to mutate the host's
/etc/hosts file. Add readOnly: true to all volumeMount definitions where name is
hosts-dir and mountPath is /etc/hosts to enforce read-only access across all
occurrences in the manifest.
---
Nitpick comments:
In `@manifests/cloud-platform-alt-dns/coredns.yaml`:
- Around line 27-29: The hostPath volume definition for the hosts-dir volume
mounting /etc/hosts lacks an explicit type specification, which should be
hardened for security and clarity. Add the type field with value File to the
hostPath configuration to explicitly enforce that the path refers to a file
rather than leaving it unspecified. This makes the expected path kind explicit
and hardens the volume definition.
In `@templates/common/cloud-platform-alt-dns/files/coredns.yaml`:
- Around line 31-33: The hosts-dir volume in the coredns.yaml file is using a
hostPath without specifying a type constraint. Add a type field with the value
File to the hostPath object for the hosts-dir volume (lines 31-33) to explicitly
specify that the path must be a file, preventing accidental binding to non-file
paths and improving security posture.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0980aac9-50c4-4ec7-a84e-f42774daf93f
📒 Files selected for processing (4)
manifests/cloud-platform-alt-dns/coredns-corefile.tmplmanifests/cloud-platform-alt-dns/coredns.yamltemplates/common/cloud-platform-alt-dns/files/coredns-corefile.yamltemplates/common/cloud-platform-alt-dns/files/coredns.yaml
sadasu
force-pushed
the
custom-dns-localhost-resolution
branch
from
4774e1b to
9543674
Compare
|
@sadasu: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
- What I did
- How to verify it
- Description for the changelog
Summary by CodeRabbit
Release Notes
hostsplugin backed by the system/etc/hoststo resolvelocalhostandlocalhost.localdomainfor both IPv4 (127.0.0.1) and IPv6 (::1), withfallthroughenabled./etc/hostsas read-only to ensure consistent localhost resolution.