OCPBUGS-98258: Fix upstreams for CoreDNS pods on Cloud platforms - #6383
OCPBUGS-98258: Fix upstreams for CoreDNS pods on Cloud platforms#6383sadasu wants to merge 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
jira/severity-moderate
|
@sadasu: This pull request references Jira Issue OCPBUGS-98258, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
WalkthroughChangesCoreDNS resolver integration
Estimated code review effort: 1 (Trivial) | ~2 minutes Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 passed)
✨ 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 |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@sadasu: This pull request references Jira Issue OCPBUGS-98258, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
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. |
|
@sadasu: This pull request references Jira Issue OCPBUGS-98258, which is valid. 3 validation(s) were run on this bug
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@templates/common/cloud-platform-alt-dns/files/coredns.yaml`:
- Line 50: Add the existing nm-resolv volume mount to the render-config-coredns
init container, matching the mount configuration already used by
coredns-monitor, so /var/run/NetworkManager/resolv.conf resolves to the host
file during runtimecfg render.
🪄 Autofix
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: 161884e4-64cb-4bd4-bcfa-40ccb1a59feb
📒 Files selected for processing (1)
templates/common/cloud-platform-alt-dns/files/coredns.yaml
On cloud platforms, the CoreDNS Corefile's Upstreams were getting generated using the host's /etc/resolv.conf that is modified by network manager to include the local host. That resulted in the CoreDNS upstreams to include the IP of the node on which the CoreDNS static pod was running on. Fixed to use the NetworkManager's original upstream resolv.conf instead.
c8d3cd1 to
8f541d0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@templates/common/cloud-platform-alt-dns/files/coredns.yaml`:
- Around line 62-64: Add readOnly: true to the nm-resolv volumeMount alongside
mountPath and mountPropagation, ensuring the NetworkManager resolver path is
mounted read-only while preserving the existing mount configuration.
🪄 Autofix
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: 2d628202-4f14-480a-82dd-543c043d0343
📒 Files selected for processing (1)
templates/common/cloud-platform-alt-dns/files/coredns.yaml
| - name: nm-resolv | ||
| mountPath: "/var/run/NetworkManager" | ||
| mountPropagation: HostToContainer |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Make the NetworkManager mount read-only.
runtimecfg render reads this path as resolver input. Without readOnly: true, the init container can modify the node's NetworkManager resolver files if it is compromised or malfunctions. Add the read-only flag.
Proposed fix
- name: nm-resolv
mountPath: "/var/run/NetworkManager"
mountPropagation: HostToContainer
+ readOnly: true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: nm-resolv | |
| mountPath: "/var/run/NetworkManager" | |
| mountPropagation: HostToContainer | |
| - name: nm-resolv | |
| mountPath: "/var/run/NetworkManager" | |
| mountPropagation: HostToContainer | |
| readOnly: true |
🤖 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 62 -
64, Add readOnly: true to the nm-resolv volumeMount alongside mountPath and
mountPropagation, ensuring the NetworkManager resolver path is mounted read-only
while preserving the existing mount configuration.
|
@sadasu: The following tests failed, say
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
On cloud platforms, the CoreDNS Corefile's Upstreams were getting generated using the host's /etc/resolv.conf that is modified by network manager to include the local host.
That resulted in the CoreDNS upstreams to include the IP of the node on which the CoreDNS static pod was running on. Fixed to use the NetworkManager's original upstream resolv.conf instead.
- What I did
- How to verify it
- Description for the changelog
Summary by CodeRabbit