ITQC/containernetworking-plugins
24
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 34 Wiki Activity

ptp: only override DNS conf if DNS settings provided

Browse Source 
Previously, if an IPAM plugin provided DNS settings in the result to the PTP
plugin, those settings were always lost because the PTP plugin would always
provide its own DNS settings in the result even if the PTP plugin was not
configured with any DNS settings.

This was especially problematic when trying to use, for example, the host-local
IPAM plugin's support for retrieving DNS settings from a resolv.conf file on
the host. Before this change, those DNS settings were always lost when using the
PTP plugin and couldn't be specified as part of PTP instead because PTP does not
support parsing a resolv.conf file.

This change checks to see if any fields were actually set in the PTP plugin's
DNS settings and only overrides any previous DNS results from an IPAM plugin in
the case that settings actually were provided to PTP. In the case where no
DNS settings are provided to PTP, the DNS results of the IPAM plugin (if any)
are used instead.

Signed-off-by: Erik Sipsma <sipsma@amazon.com>
This commit is contained in:
Erik Sipsma 2019-09-17 21:23:21 +00:00
parent 23d5525ec3
commit a96c469e62
3 changed files with 199 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
pkg/testutils/dns.go Normal file
View File

@ -0,0 +1,60 @@
// Copyright 2019 CNI authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package testutils
import (
"fmt"
"io/ioutil"
"os"
"strings"
"github.com/containernetworking/cni/pkg/types"
)
// TmpResolvConf will create a temporary file and write the provided DNS settings to
// it in the resolv.conf format. It returns the path of the created temporary file or
// an error if any occurs while creating/writing the file. It is the caller's
// responsibility to remove the file.
func TmpResolvConf(dnsConf types.DNS) (string, error) {
f, err := ioutil.TempFile("", "cni_test_resolv.conf")
if err != nil {
return "", fmt.Errorf("failed to get temp file for CNI test resolv.conf: %v", err)
}
defer f.Close()
path := f.Name()
defer func() {
if err != nil {
os.RemoveAll(path)
}
}()
// see "man 5 resolv.conf" for the format of resolv.conf
var resolvConfLines []string
for _, nameserver := range dnsConf.Nameservers {
resolvConfLines = append(resolvConfLines, fmt.Sprintf("nameserver %s", nameserver))
}
resolvConfLines = append(resolvConfLines, fmt.Sprintf("domain %s", dnsConf.Domain))
resolvConfLines = append(resolvConfLines, fmt.Sprintf("search %s", strings.Join(dnsConf.Search, " ")))
resolvConfLines = append(resolvConfLines, fmt.Sprintf("options %s", strings.Join(dnsConf.Options, " ")))
resolvConf := strings.Join(resolvConfLines, "\n")
_, err = f.Write([]byte(resolvConf))
if err != nil {
return "", fmt.Errorf("failed to write temp resolv.conf for CNI test: %v", err)
}
return path, err
}

15
plugins/main/ptp/ptp.go
View File

@ -247,12 +247,25 @@ func cmdAdd(args *skel.CmdArgs) error {
}
}
result.DNS = conf.DNS
// Only override the DNS settings in the previous result if any DNS fields
// were provided to the ptp plugin. This allows, for example, IPAM plugins
// to specify the DNS settings instead of the ptp plugin.
if dnsConfSet(conf.DNS) {
result.DNS = conf.DNS
}
result.Interfaces = []*current.Interface{hostInterface, containerInterface}
return types.PrintResult(result, conf.CNIVersion)
}
func dnsConfSet(dnsConf types.DNS) bool {
return dnsConf.Nameservers != nil ||
dnsConf.Search != nil ||
dnsConf.Options != nil ||
dnsConf.Domain != ""
}
func cmdDel(args *skel.CmdArgs) error {
conf := NetConf{}
if err := json.Unmarshal(args.StdinData, &conf); err != nil {

131
plugins/main/ptp/ptp_test.go
View File

@ -17,6 +17,7 @@ package main
import (
"encoding/json"
"fmt"
"os"
"github.com/containernetworking/cni/pkg/skel"
"github.com/containernetworking/cni/pkg/types"
@ -101,7 +102,7 @@ var _ = Describe("ptp Operations", func() {
Expect(testutils.UnmountNS(originalNS)).To(Succeed())
})
doTest := func(conf string, numIPs int) {
doTest := func(conf string, numIPs int, expectedDNSConf types.DNS) {
const IFNAME = "ptp0"
targetNs, err := testutils.NewNS()
@ -176,6 +177,9 @@ var _ = Describe("ptp Operations", func() {
Expect(res.Interfaces[1].Mac).To(Equal(wantMac))
Expect(res.Interfaces[1].Sandbox).To(Equal(targetNs.Path()))
// make sure DNS is correct
Expect(res.DNS).To(Equal(expectedDNSConf))
// Call the plugins with the DEL command, deleting the veth endpoints
err = originalNS.Do(func(ns.NetNS) error {
defer GinkgoRecover()
@ -328,7 +332,16 @@ var _ = Describe("ptp Operations", func() {
}
It("configures and deconfigures a ptp link with ADD/DEL", func() {
conf := `{
dnsConf := types.DNS{
Nameservers: []string{"10.1.2.123"},
Domain: "some.domain.test",
Search: []string{"search.test"},
Options: []string{"option1:foo"},
}
dnsConfBytes, err := json.Marshal(dnsConf)
Expect(err).NotTo(HaveOccurred())
conf := fmt.Sprintf(`{
"cniVersion": "0.3.1",
"name": "mynet",
"type": "ptp",
@ -337,10 +350,11 @@ var _ = Describe("ptp Operations", func() {
"ipam": {
"type": "host-local",
"subnet": "10.1.2.0/24"
}
}`
},
"dns": %s
}`, string(dnsConfBytes))
doTest(conf, 1)
doTest(conf, 1, dnsConf)
})
It("configures and deconfigures a dual-stack ptp link with ADD/DEL", func() {
@ -359,7 +373,112 @@ var _ = Describe("ptp Operations", func() {
}
}`
doTest(conf, 2)
doTest(conf, 2, types.DNS{})
})
It("does not override IPAM DNS settings if no DNS settings provided", func() {
ipamDNSConf := types.DNS{
Nameservers: []string{"10.1.2.123"},
Domain: "some.domain.test",
Search: []string{"search.test"},
Options: []string{"option1:foo"},
}
resolvConfPath, err := testutils.TmpResolvConf(ipamDNSConf)
Expect(err).NotTo(HaveOccurred())
defer os.RemoveAll(resolvConfPath)
conf := fmt.Sprintf(`{
"cniVersion": "0.3.1",
"name": "mynet",
"type": "ptp",
"ipMasq": true,
"mtu": 5000,
"ipam": {
"type": "host-local",
"subnet": "10.1.2.0/24",
"resolvConf": "%s"
}
}`, resolvConfPath)
doTest(conf, 1, ipamDNSConf)
})
It("overrides IPAM DNS settings if any DNS settings provided", func() {
ipamDNSConf := types.DNS{
Nameservers: []string{"10.1.2.123"},
Domain: "some.domain.test",
Search: []string{"search.test"},
Options: []string{"option1:foo"},
}
resolvConfPath, err := testutils.TmpResolvConf(ipamDNSConf)
Expect(err).NotTo(HaveOccurred())
defer os.RemoveAll(resolvConfPath)
for _, ptpDNSConf := range []types.DNS{
{
Nameservers: []string{"10.1.2.234"},
},
{
Domain: "someother.domain.test",
},
{
Search: []string{"search.elsewhere.test"},
},
{
Options: []string{"option2:bar"},
},
} {
dnsConfBytes, err := json.Marshal(ptpDNSConf)
Expect(err).NotTo(HaveOccurred())
conf := fmt.Sprintf(`{
"cniVersion": "0.3.1",
"name": "mynet",
"type": "ptp",
"ipMasq": true,
"mtu": 5000,
"ipam": {
"type": "host-local",
"subnet": "10.1.2.0/24",
"resolvConf": "%s"
},
"dns": %s
}`, resolvConfPath, string(dnsConfBytes))
doTest(conf, 1, ptpDNSConf)
}
})
It("overrides IPAM DNS settings if any empty list DNS settings provided", func() {
ipamDNSConf := types.DNS{
Nameservers: []string{"10.1.2.123"},
Domain: "some.domain.test",
Search: []string{"search.test"},
Options: []string{"option1:foo"},
}
resolvConfPath, err := testutils.TmpResolvConf(ipamDNSConf)
Expect(err).NotTo(HaveOccurred())
defer os.RemoveAll(resolvConfPath)
conf := fmt.Sprintf(`{
"cniVersion": "0.3.1",
"name": "mynet",
"type": "ptp",
"ipMasq": true,
"mtu": 5000,
"ipam": {
"type": "host-local",
"subnet": "10.1.2.0/24",
"resolvConf": "%s"
},
"dns": {
"nameservers": [],
"search": [],
"options": []
}
}`, resolvConfPath)
doTest(conf, 1, types.DNS{})
})
It("deconfigures an unconfigured ptp link with DEL", func() {