-
Notifications
You must be signed in to change notification settings - Fork 717
fix: allow localhost DNS servers when using host network #4653
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This commit addresses the issue where nerdctl was unconditionally stripping localhost DNS servers from /etc/resolv.conf when containers used --network=host. Changes made: 1. Modified FilterResolvDNS() to support preserving localhost DNS servers via a new FilterResolvDNSWithLocalhostOption() function 2. Added allowLocalhostDNS parameter to fetchDNSResolverConfig() 3. Updated hostNetworkManager to use allowLocalhostDNS=true to preserve host DNS 4. Added comprehensive test coverage for the new functionality The fix ensures: - Host network mode respects host's /etc/resolv.conf including localhost resolvers - Isolated networks (bridge, CNI) continue to have fallback Google DNS - Backward compatible - all existing behavior unchanged - Docker-compatible behavior Fixes: containerd#4651 Signed-off-by: Youfu Zhang <[email protected]>
| } | ||
|
|
||
| func TestFilterResolvDnsWithLocalhostOption(t *testing.T) { | ||
| // Test 1: allowLocalhostDNS=false should strip localhost (original behavior) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should be subtests? (t.Run)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have some integration tests too?
nerdctl/cmd/nerdctl/container/container_run_network_linux_test.go
Lines 935 to 978 in 20a3eeb
| func TestHostNetworkHostName(t *testing.T) { | |
| nerdtest.Setup() | |
| testCase := &test.Case{ | |
| Require: require.Not(require.Windows), | |
| Setup: func(data test.Data, helpers test.Helpers) { | |
| helpers.Custom("cat", "/etc/hostname").Run(&test.Expected{ | |
| Output: func(stdout string, t tig.T) { | |
| data.Labels().Set("hostHostname", stdout) | |
| }, | |
| }) | |
| }, | |
| Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { | |
| return helpers.Command("run", "--rm", | |
| "--network", "host", | |
| testutil.AlpineImage, "cat", "/etc/hostname") | |
| }, | |
| Expected: func(data test.Data, helpers test.Helpers) *test.Expected { | |
| return &test.Expected{ | |
| Output: expect.Equals(data.Labels().Get("hostHostname")), | |
| } | |
| }, | |
| } | |
| testCase.Run(t) | |
| } | |
| func TestNoneNetworkDnsConfigs(t *testing.T) { | |
| nerdtest.Setup() | |
| testCase := &test.Case{ | |
| Require: require.Not(require.Windows), | |
| Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { | |
| return helpers.Command("run", "--rm", | |
| "--network", "none", | |
| "--dns", "0.1.2.3", "--dns-search", "example.com", "--dns-option", "timeout:3", "--dns-option", "attempts:5", | |
| testutil.CommonImage, "cat", "/etc/resolv.conf") | |
| }, | |
| Expected: test.Expects(0, nil, expect.Contains( | |
| "0.1.2.3", | |
| "example.com", | |
| "attempts:5", | |
| "timeout:3", | |
| )), | |
| } | |
| testCase.Run(t) | |
| } |
(This change is vibed by Claude Haiku 4.5, reviewed by human.)
This commit addresses the issue where nerdctl was unconditionally stripping localhost DNS servers from /etc/resolv.conf when containers used --network=host.
Changes made:
The fix ensures:
Fixes: #4651