Skip to content

Commit

Permalink
CRI: An empty DNSConfig != unspecified
Browse files Browse the repository at this point in the history 
If we find that DNSConfig is provided and empty (not nil), we should not
replace it with the host's resolv.conf.

Also adds tests.

Signed-off-by: Tim Hockin <thockin@google.com>
  • Loading branch information
@thockin @dims
thockin authored and dims committed Jul 16, 2024
1 parent 923bb1f commit 209ee4f
Show file tree
Hide file tree
Showing 4 changed files with 166 additions and 26 deletions.
22 changes: 9 additions & 13 deletions pkg/cri/sbserver/podsandbox/sandbox_run_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,25 +253,21 @@ func (c *Controller) setupSandboxFiles(id string, config *runtime.PodSandboxConf
}

// Set DNS options. Maintain a resolv.conf for the sandbox.
var err error
resolvContent := ""
resolvPath := c.getResolvPath(id)

if dnsConfig := config.GetDnsConfig(); dnsConfig != nil {
resolvContent, err = parseDNSOptions(dnsConfig.Servers, dnsConfig.Searches, dnsConfig.Options)
resolvContent, err := parseDNSOptions(dnsConfig.Servers, dnsConfig.Searches, dnsConfig.Options)
if err != nil {
return fmt.Errorf("failed to parse sandbox DNSConfig %+v: %w", dnsConfig, err)
}
}
resolvPath := c.getResolvPath(id)
if resolvContent == "" {
// copy host's resolv.conf to resolvPath
err = c.os.CopyFile(resolvConfPath, resolvPath, 0644)
if err != nil {
return fmt.Errorf("failed to copy host's resolv.conf to %q: %w", resolvPath, err)
if err := c.os.WriteFile(resolvPath, []byte(resolvContent), 0644); err != nil {
return fmt.Errorf("failed to write resolv content to %q: %w", resolvPath, err)
}
} else {
err = c.os.WriteFile(resolvPath, []byte(resolvContent), 0644)
if err != nil {
return fmt.Errorf("failed to write resolv content to %q: %w", resolvPath, err)
// The DnsConfig was nil - we interpret that to mean "use the global
// default", which is dubious but backwards-compatible.
if err := c.os.CopyFile(resolvConfPath, resolvPath, 0644); err != nil {
return fmt.Errorf("failed to copy host's resolv.conf to %q: %w", resolvPath, err)
}
}

Expand Down
74 changes: 74 additions & 0 deletions pkg/cri/sbserver/podsandbox/sandbox_run_linux_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,80 @@ options timeout:1
},
},
},
"should create empty /etc/resolv.conf if DNSOptions is empty": {
dnsConfig: &runtime.DNSConfig{},
ipcMode: runtime.NamespaceMode_NODE,
expectedCalls: []ostesting.CalledDetail{
{
Name: "Hostname",
},
{
Name: "WriteFile",
Arguments: []interface{}{
filepath.Join(testRootDir, sandboxesDir, testID, "hostname"),
[]byte(realhostname + "\n"),
os.FileMode(0644),
},
},
{
Name: "CopyFile",
Arguments: []interface{}{
"/etc/hosts",
filepath.Join(testRootDir, sandboxesDir, testID, "hosts"),
os.FileMode(0644),
},
},
{
Name: "WriteFile",
Arguments: []interface{}{
filepath.Join(testRootDir, sandboxesDir, testID, "resolv.conf"),
[]byte{},
os.FileMode(0644),
},
},
{
Name: "Stat",
Arguments: []interface{}{"/dev/shm"},
},
},
},
"should copy host /etc/resolv.conf if DNSOptions is not set": {
dnsConfig: nil,
ipcMode: runtime.NamespaceMode_NODE,
expectedCalls: []ostesting.CalledDetail{
{
Name: "Hostname",
},
{
Name: "WriteFile",
Arguments: []interface{}{
filepath.Join(testRootDir, sandboxesDir, testID, "hostname"),
[]byte(realhostname + "\n"),
os.FileMode(0644),
},
},
{
Name: "CopyFile",
Arguments: []interface{}{
"/etc/hosts",
filepath.Join(testRootDir, sandboxesDir, testID, "hosts"),
os.FileMode(0644),
},
},
{
Name: "CopyFile",
Arguments: []interface{}{
filepath.Join("/etc/resolv.conf"),
filepath.Join(testRootDir, sandboxesDir, testID, "resolv.conf"),
os.FileMode(0644),
},
},
{
Name: "Stat",
Arguments: []interface{}{"/dev/shm"},
},
},
},
"should create sandbox shm when ipc namespace mode is not NODE": {
ipcMode: runtime.NamespaceMode_POD,
expectedCalls: []ostesting.CalledDetail{
Expand Down
22 changes: 9 additions & 13 deletions pkg/cri/server/sandbox_run_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,25 +270,21 @@ func (c *criService) setupSandboxFiles(id string, config *runtime.PodSandboxConf
}

// Set DNS options. Maintain a resolv.conf for the sandbox.
var err error
resolvContent := ""
resolvPath := c.getResolvPath(id)

if dnsConfig := config.GetDnsConfig(); dnsConfig != nil {
resolvContent, err = parseDNSOptions(dnsConfig.Servers, dnsConfig.Searches, dnsConfig.Options)
resolvContent, err := parseDNSOptions(dnsConfig.Servers, dnsConfig.Searches, dnsConfig.Options)
if err != nil {
return fmt.Errorf("failed to parse sandbox DNSConfig %+v: %w", dnsConfig, err)
}
}
resolvPath := c.getResolvPath(id)
if resolvContent == "" {
// copy host's resolv.conf to resolvPath
err = c.os.CopyFile(resolvConfPath, resolvPath, 0644)
if err != nil {
return fmt.Errorf("failed to copy host's resolv.conf to %q: %w", resolvPath, err)
if err := c.os.WriteFile(resolvPath, []byte(resolvContent), 0644); err != nil {
return fmt.Errorf("failed to write resolv content to %q: %w", resolvPath, err)
}
} else {
err = c.os.WriteFile(resolvPath, []byte(resolvContent), 0644)
if err != nil {
return fmt.Errorf("failed to write resolv content to %q: %w", resolvPath, err)
// The DnsConfig was nil - we interpret that to mean "use the global
// default", which is dubious but backwards-compatible.
if err := c.os.CopyFile(resolvConfPath, resolvPath, 0644); err != nil {
return fmt.Errorf("failed to copy host's resolv.conf to %q: %w", resolvPath, err)
}
}

Expand Down
74 changes: 74 additions & 0 deletions pkg/cri/server/sandbox_run_linux_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,80 @@ options timeout:1
},
},
},
"should create empty /etc/resolv.conf if DNSOptions is empty": {
dnsConfig: &runtime.DNSConfig{},
ipcMode: runtime.NamespaceMode_NODE,
expectedCalls: []ostesting.CalledDetail{
{
Name: "Hostname",
},
{
Name: "WriteFile",
Arguments: []interface{}{
filepath.Join(testRootDir, sandboxesDir, testID, "hostname"),
[]byte(realhostname + "\n"),
os.FileMode(0644),
},
},
{
Name: "CopyFile",
Arguments: []interface{}{
"/etc/hosts",
filepath.Join(testRootDir, sandboxesDir, testID, "hosts"),
os.FileMode(0644),
},
},
{
Name: "WriteFile",
Arguments: []interface{}{
filepath.Join(testRootDir, sandboxesDir, testID, "resolv.conf"),
[]byte{},
os.FileMode(0644),
},
},
{
Name: "Stat",
Arguments: []interface{}{"/dev/shm"},
},
},
},
"should copy host /etc/resolv.conf if DNSOptions is not set": {
dnsConfig: nil,
ipcMode: runtime.NamespaceMode_NODE,
expectedCalls: []ostesting.CalledDetail{
{
Name: "Hostname",
},
{
Name: "WriteFile",
Arguments: []interface{}{
filepath.Join(testRootDir, sandboxesDir, testID, "hostname"),
[]byte(realhostname + "\n"),
os.FileMode(0644),
},
},
{
Name: "CopyFile",
Arguments: []interface{}{
"/etc/hosts",
filepath.Join(testRootDir, sandboxesDir, testID, "hosts"),
os.FileMode(0644),
},
},
{
Name: "CopyFile",
Arguments: []interface{}{
filepath.Join("/etc/resolv.conf"),
filepath.Join(testRootDir, sandboxesDir, testID, "resolv.conf"),
os.FileMode(0644),
},
},
{
Name: "Stat",
Arguments: []interface{}{"/dev/shm"},
},
},
},
"should create sandbox shm when ipc namespace mode is not NODE": {
ipcMode: runtime.NamespaceMode_POD,
expectedCalls: []ostesting.CalledDetail{
Expand Down

0 comments on commit 209ee4f

Please sign in to comment.