From 9903cb3ad33bd0ce21256a6c5b6275b1ba557cc5 Mon Sep 17 00:00:00 2001 From: Bruce Ma Date: Tue, 23 Jul 2019 21:15:17 +0800 Subject: [PATCH] add validation for CNI config before loading and fix some typo 1. add validation for CNI config before loading 2. make some CNI capabilities constants 3. add Capabilities field to cniNetwork struct Signed-off-by: Bruce Ma --- pkg/kubelet/dockershim/network/cni/BUILD | 1 + pkg/kubelet/dockershim/network/cni/cni.go | 55 ++++++++++++++--------- 2 files changed, 35 insertions(+), 21 deletions(-) diff --git a/pkg/kubelet/dockershim/network/cni/BUILD b/pkg/kubelet/dockershim/network/cni/BUILD index 60449de4436..782f856eb72 100644 --- a/pkg/kubelet/dockershim/network/cni/BUILD +++ b/pkg/kubelet/dockershim/network/cni/BUILD @@ -19,6 +19,7 @@ go_library( "//pkg/kubelet/container:go_default_library", "//pkg/kubelet/dockershim/network:go_default_library", "//pkg/util/bandwidth:go_default_library", + "//pkg/util/slice:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//staging/src/k8s.io/cri-api/pkg/apis/runtime/v1alpha2:go_default_library", "//vendor/github.com/containernetworking/cni/libcni:go_default_library", diff --git a/pkg/kubelet/dockershim/network/cni/cni.go b/pkg/kubelet/dockershim/network/cni/cni.go index 81351dcaa5b..8255d67232d 100644 --- a/pkg/kubelet/dockershim/network/cni/cni.go +++ b/pkg/kubelet/dockershim/network/cni/cni.go @@ -19,7 +19,6 @@ package cni import ( "context" "encoding/json" - "errors" "fmt" "math" "sort" @@ -36,6 +35,7 @@ import ( kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" "k8s.io/kubernetes/pkg/kubelet/dockershim/network" "k8s.io/kubernetes/pkg/util/bandwidth" + utilslice "k8s.io/kubernetes/pkg/util/slice" utilexec "k8s.io/utils/exec" ) @@ -46,6 +46,13 @@ const ( // defaultSyncConfigPeriod is the default period to sync CNI config // TODO: consider making this value configurable or to be a more appropriate value. defaultSyncConfigPeriod = time.Second * 5 + + // supported capabilities + // https://github.com/containernetworking/cni/blob/master/CONVENTIONS.md + portMappingsCapability = "portMappings" + ipRangesCapability = "ipRanges" + bandwidthCapability = "bandwidth" + dnsCapability = "dns" ) type cniNetworkPlugin struct { @@ -69,6 +76,7 @@ type cniNetwork struct { name string NetworkConfig *libcni.NetworkConfigList CNIConfig libcni.CNI + Capabilities []string } // cniPortMapping maps to the standard CNI portmapping Capability @@ -149,9 +157,11 @@ func getDefaultCNINetwork(confDir string, binDirs []string) (*cniNetwork, error) case err != nil: return nil, err case len(files) == 0: - return nil, fmt.Errorf("No networks found in %s", confDir) + return nil, fmt.Errorf("no networks found in %s", confDir) } + cniConfig := &libcni.CNIConfig{Path: binDirs} + sort.Strings(files) for _, confFile := range files { var confList *libcni.NetworkConfigList @@ -185,16 +195,24 @@ func getDefaultCNINetwork(confDir string, binDirs []string) (*cniNetwork, error) continue } + // Before using this CNI config, we have to validate it to make sure that + // all plugins of this config exist on disk + caps, err := cniConfig.ValidateNetworkList(context.TODO(), confList) + if err != nil { + klog.Warningf("Error validating CNI config %v: %v", confList, err) + continue + } + klog.V(4).Infof("Using CNI configuration file %s", confFile) - network := &cniNetwork{ + return &cniNetwork{ name: confList.Name, NetworkConfig: confList, - CNIConfig: &libcni.CNIConfig{Path: binDirs}, - } - return network, nil + CNIConfig: cniConfig, + Capabilities: caps, + }, nil } - return nil, fmt.Errorf("No valid networks found in %s", confDir) + return nil, fmt.Errorf("no valid networks found in %s", confDir) } func (plugin *cniNetworkPlugin) Init(host network.Host, hairpinMode kubeletconfig.HairpinMode, nonMasqueradeCIDR string, mtu int) error { @@ -236,18 +254,13 @@ func (plugin *cniNetworkPlugin) setDefaultNetwork(n *cniNetwork) { func (plugin *cniNetworkPlugin) checkInitialized() error { if plugin.getDefaultNetwork() == nil { - return errors.New("cni config uninitialized") + return fmt.Errorf("cni config uninitialized") } - // If the CNI configuration has the ipRanges capability, we need a PodCIDR assigned - for _, p := range plugin.getDefaultNetwork().NetworkConfig.Plugins { - if p.Network.Capabilities["ipRanges"] { - if plugin.podCidr == "" { - return errors.New("no PodCIDR set") - } - break - } + if utilslice.ContainsString(plugin.getDefaultNetwork().Capabilities, ipRangesCapability, nil) && plugin.podCidr == "" { + return fmt.Errorf("cni config needs ipRanges but no PodCIDR set") } + return nil } @@ -395,12 +408,12 @@ func (plugin *cniNetworkPlugin) buildCNIRuntimeConf(podName string, podNs string }) } rt.CapabilityArgs = map[string]interface{}{ - "portMappings": portMappingsParam, + portMappingsCapability: portMappingsParam, } ingress, egress, err := bandwidth.ExtractPodBandwidthResources(annotations) if err != nil { - return nil, fmt.Errorf("Error reading pod bandwidth annotations: %v", err) + return nil, fmt.Errorf("failed to get pod bandwidth from annotations: %v", err) } if ingress != nil || egress != nil { bandwidthParam := cniBandwidthEntry{} @@ -415,11 +428,11 @@ func (plugin *cniNetworkPlugin) buildCNIRuntimeConf(podName string, podNs string bandwidthParam.EgressRate = int(egress.Value()) bandwidthParam.EgressBurst = math.MaxInt32 // no limit } - rt.CapabilityArgs["bandwidth"] = bandwidthParam + rt.CapabilityArgs[bandwidthCapability] = bandwidthParam } // Set the PodCIDR - rt.CapabilityArgs["ipRanges"] = [][]cniIPRange{{{Subnet: plugin.podCidr}}} + rt.CapabilityArgs[ipRangesCapability] = [][]cniIPRange{{{Subnet: plugin.podCidr}}} // Set dns capability args. if dnsOptions, ok := options["dns"]; ok { @@ -429,7 +442,7 @@ func (plugin *cniNetworkPlugin) buildCNIRuntimeConf(podName string, podNs string return nil, fmt.Errorf("failed to unmarshal dns config %q: %v", dnsOptions, err) } if dnsParam := buildDNSCapabilities(&dnsConfig); dnsParam != nil { - rt.CapabilityArgs["dns"] = *dnsParam + rt.CapabilityArgs[dnsCapability] = *dnsParam } }