diff --git a/daemonset/generic-kuberouter-cni-conf-template.yaml b/daemonset/generic-kuberouter-cni-conf-template.yaml new file mode 100644 index 000000000..0619b16b3 --- /dev/null +++ b/daemonset/generic-kuberouter-cni-conf-template.yaml @@ -0,0 +1,217 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: kube-router-cfg + namespace: kube-system + labels: + tier: node + k8s-app: kube-router +data: + cni-conf.json: | + { + "cniVersion": "0.3.0", + "name": "mynet", + "plugins": [ + { + "name": "kubernetes", + "type": "bridge", + "bridge": "kube-bridge", + "isDefaultGateway": true, + "ipam": { + "type": "host-local" + } + } + ] + } + +--- +apiVersion: apps/v1 +kind: DaemonSet +metadata: + labels: + k8s-app: kube-router + tier: node + name: kube-router + namespace: kube-system +spec: + selector: + matchLabels: + k8s-app: kube-router + tier: node + template: + metadata: + labels: + k8s-app: kube-router + tier: node + spec: + priorityClassName: system-node-critical + serviceAccountName: kube-router + initContainers: + - name: install-cni + image: docker.io/cloudnativelabs/kube-router + imagePullPolicy: Always + command: + - /bin/sh + - -ec + - | + [ -f /etc/cni/net.d/10-kuberouter.conflist ] || { + rm -f /etc/cni/net.d/*.conf + rm -f /etc/cni/net.d/*.conflist + } + /usr/local/bin/cni-install + volumeMounts: + - name: cni-conf-dir + mountPath: /etc/cni/net.d + - name: host-opt + mountPath: /opt + containers: + - name: kube-router + image: docker.io/cloudnativelabs/kube-router + imagePullPolicy: Always + args: + - --run-router=true + - --run-firewall=true + - --run-service-proxy=false + - --bgp-graceful-restart=true + env: + - name: NODE_NAME + valueFrom: + fieldRef: + fieldPath: spec.nodeName + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name + - name: KUBE_ROUTER_CNI_CONF_FILE + value: /etc/cni/net.d/10-kuberouter.conflist + - name: KUBE_ROUTER_CNI_CONF_TEMPLATE_FILE + value: /etc/kube-router/cni-conf.json + livenessProbe: + httpGet: + path: /healthz + port: 20244 + initialDelaySeconds: 10 + periodSeconds: 3 + resources: + requests: + cpu: 250m + memory: 250Mi + securityContext: + privileged: true + volumeMounts: + - name: lib-modules + mountPath: /lib/modules + readOnly: true + - name: cni-conf-dir + mountPath: /etc/cni/net.d + - name: kube-router-cfg + mountPath: /etc/kube-router + readOnly: true + - name: xtables-lock + mountPath: /run/xtables.lock + readOnly: false + hostNetwork: true + hostPID: true + tolerations: + - effect: NoSchedule + operator: Exists + - key: CriticalAddonsOnly + operator: Exists + - effect: NoExecute + operator: Exists + volumes: + - name: lib-modules + hostPath: + path: /lib/modules + - name: cni-conf-dir + hostPath: + path: /etc/cni/net.d + - name: kube-router-cfg + configMap: + name: kube-router-cfg + - name: xtables-lock + hostPath: + path: /run/xtables.lock + type: FileOrCreate + - name: host-opt + hostPath: + path: /opt + +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: kube-router + namespace: kube-system + +--- +kind: ClusterRole +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: kube-router + namespace: kube-system +rules: +- apiGroups: + - "" + resources: + - namespaces + - pods + - services + - nodes + - endpoints + verbs: + - list + - get + - watch +- apiGroups: + - networking.k8s.io + resources: + - networkpolicies + verbs: + - list + - get + - watch +- apiGroups: + - extensions + resources: + - networkpolicies + verbs: + - get + - list + - watch +- apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - get + - create + - update +- apiGroups: + - "" + resources: + - services/status + verbs: + - update +- apiGroups: + - discovery.k8s.io + resources: + - endpointslices + verbs: + - get + - list + - watch + +--- +kind: ClusterRoleBinding +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: kube-router +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: kube-router +subjects: +- kind: ServiceAccount + name: kube-router + namespace: kube-system diff --git a/docs/user-guide.md b/docs/user-guide.md index df1b9573b..028b4cb18 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -35,7 +35,9 @@ - [Hairpin Mode Example](#hairpin-mode-example) - [SNATing Service Traffic](#snating-service-traffic) - [Load balancing Scheduling Algorithms](#load-balancing-scheduling-algorithms) -- [HostPort support](#hostport-support) +- [CNI Configuration](#cni-configuration) + - [CNI Configuration Templates](#cni-configuration-templates) + - [HostPort support](#hostport-support) - [IPVS Graceful termination support](#ipvs-graceful-termination-support) - [MTU](#mtu) - [BGP configuration](#bgp-configuration) @@ -578,46 +580,92 @@ $ kubectl annotate service my-service "kube-router.io/service.schedflags=flag-2" $ kubectl annotate service my-service "kube-router.io/service.schedflags=flag-1,flag-2" ``` -## HostPort support - -If you would like to use `HostPort` functionality below changes are required in the manifest. - -- By default kube-router assumes CNI conf file to be `/etc/cni/net.d/10-kuberouter.conf`. Add an environment variable -`KUBE_ROUTER_CNI_CONF_FILE` to kube-router manifest and set it to `/etc/cni/net.d/10-kuberouter.conflist` - -- Modify `kube-router-cfg` ConfigMap with CNI config that supports `portmap` as additional plug-in - -```json - { - "cniVersion":"0.3.0", - "name":"mynet", - "plugins":[ - { - "name":"kubernetes", - "type":"bridge", - "bridge":"kube-bridge", - "isDefaultGateway":true, - "ipam":{ - "type":"host-local" - } - }, - { - "type":"portmap", - "capabilities":{ - "snat":true, - "portMappings":true - } - } - ] - } -``` - -- Update init container command to create `/etc/cni/net.d/10-kuberouter.conflist` file +## CNI Configuration + +kube-router provides pod networking by means of the standard [`bridge`][cni-bridge] and [`host-local`][cni-host-local] +[containernetworking plugins], optionally accompanied by others like [`portmap`][cni-portmap]. The kube-router Docker +image ships these plugins, and the [example daemonsets](../daemonset/) install them onto the node in an init container. +kube-router's own contribution is the routing of pod traffic between nodes and the management of the plugins' +configuration file, filling in the parts that are node-specific and only known at runtime. The latter is what +`--enable-cni` controls, which is enabled by default. Disable it to use kube-router's features alongside another CNI +provider. On startup, kube-router + +- inserts the node's pod CIDRs into the `host-local` IPAM ranges of the `bridge` plugin, as allocated to the node by + the kube-controller-manager, or as set via the `kube-router.io/pod-cidrs` annotation +- and sets the `bridge` plugin's MTU, if `--auto-mtu` is enabled (see [MTU](#mtu)). + +The completed configuration is written back once the `kube-bridge` interface has been set up. The configuration is read +from and written to `/etc/cni/net.d/10-kuberouter.conf` by default; the location can be changed via the +`KUBE_ROUTER_CNI_CONF_FILE` environment variable. A file name ending in `.conflist` is treated as a [configuration +list][cni-config-format], i.e. a chain of multiple plugins, any other name as a single plugin configuration. + +The configuration file must exist when kube-router starts. The [example daemonsets](../daemonset/) ship it in a +`kube-router-cfg` ConfigMap and use an init container to copy it into the CNI configuration directory. + +### CNI Configuration Templates + +A configuration file that's copied into the CNI configuration directory by an init container is already visible to the +container runtime before kube-router has completed it, so on a freshly booted node, pod sandbox creation may transiently +fail with errors like "no IP ranges specified" until kube-router has filled in the missing parts. + +To avoid this, the `KUBE_ROUTER_CNI_CONF_TEMPLATE_FILE` environment variable can point kube-router to a potentially +incomplete CNI configuration outside of the CNI configuration directory, e.g. a ConfigMap mounted at +`/etc/kube-router/cni-conf.json`. kube-router will then read the configuration from the template instead, and write the +completed configuration to `KUBE_ROUTER_CNI_CONF_FILE`, which doesn't need to exist beforehand. The template is +interpreted according to the file name given in `KUBE_ROUTER_CNI_CONF_FILE`, i.e. as a configuration list if that name +ends in `.conflist`. The template's own file name has no significance. As a result, the container runtime won't pick up +an incomplete configuration and try to set up pod networking too early. Since the configuration is re-derived from the +pristine template on every start, the written configuration is wholly owned by kube-router. + +For an example manifest, please look at the [CNI configuration template manifest]. + +### HostPort support + +If you would like to use `HostPort` functionality, the following changes are required in the manifest: + +- Since the `portmap` plugin needs to be chained after the `bridge` plugin, the CNI configuration has to be a config + list: set `KUBE_ROUTER_CNI_CONF_FILE` to `/etc/cni/net.d/10-kuberouter.conflist` (see above) +- Modify the `kube-router-cfg` ConfigMap with a CNI configuration that supports `portmap` as an additional plug-in: + + ```json + { + "cniVersion":"0.3.0", + "name":"mynet", + "plugins":[ + { + "name":"kubernetes", + "type":"bridge", + "bridge":"kube-bridge", + "isDefaultGateway":true, + "ipam":{ + "type":"host-local" + } + }, + { + "type":"portmap", + "capabilities":{ + "snat":true, + "portMappings":true + } + } + ] + } + ``` + +- Update the init container command to create `/etc/cni/net.d/10-kuberouter.conflist`, or point + `KUBE_ROUTER_CNI_CONF_TEMPLATE_FILE` at the ConfigMap mount instead - Restart the container runtime For an e.g manifest please look at [manifest](../daemonset/kubeadm-kuberouter-all-features-hostport.yaml) with necessary changes required for `HostPort` functionality. +[cni-bridge]: https://www.cni.dev/plugins/v1.1/main/bridge/ +[cni-host-local]: https://www.cni.dev/plugins/v1.1/ipam/host-local/ +[containernetworking plugins]: https://github.com/containernetworking/plugins +[cni-portmap]: https://www.cni.dev/plugins/v1.1/meta/portmap/ +[cni-config-format]: https://www.cni.dev/docs/spec/#configuration-format +[CNI configuration template manifest]: ../daemonset/generic-kuberouter-cni-conf-template.yaml + ## IPVS Graceful termination support We support experimental graceful termination of IPVS destinations. When possible, the pod's diff --git a/pkg/controllers/routing/network_routes_controller.go b/pkg/controllers/routing/network_routes_controller.go index 28da08fbf..5f4462ca0 100644 --- a/pkg/controllers/routing/network_routes_controller.go +++ b/pkg/controllers/routing/network_routes_controller.go @@ -146,6 +146,7 @@ type NetworkRoutingController struct { bgpRRServer bool bgpClusterID string cniConfFile string + cniConfTemplateFile string disableSrcDstCheck bool initSrcDstCheckDone atomic.Bool ec2IamAuthorized atomic.Bool @@ -407,10 +408,10 @@ func (nrc *NetworkRoutingController) initCNIConfig() (mtu int, _ *utils.CNINetwo return mtu, nil } - // Parse the existing IPAM CIDRs from the CNI conf file - cniNetConf, err := utils.NewCNINetworkConfig(nrc.cniConfFile) + // Parse the existing IPAM CIDRs from the CNI conf (template) file + cniNetConf, err := utils.NewCNINetworkConfigFromTemplate(nrc.cniConfFile, nrc.cniConfTemplateFile) if err != nil { - klog.Fatalf("failed to parse CNI Config: %v", err) + klog.Fatalf("failed to load CNI configuration: %v", err) } // Insert any IPv4 CIDRs that are missing from the IPAM configuration in the CNI @@ -1301,9 +1302,18 @@ func NewNetworkRoutingController(clientset kubernetes.Interface, if nrc.cniConfFile == "" { nrc.cniConfFile = "/etc/cni/net.d/10-kuberouter.conf" } - // #nosec G703: cniConfFile is not untrusted user input - if _, err := os.Stat(nrc.cniConfFile); os.IsNotExist(err) { - return nil, errors.New("CNI conf file " + nrc.cniConfFile + " does not exist.") + nrc.cniConfTemplateFile = os.Getenv("KUBE_ROUTER_CNI_CONF_TEMPLATE_FILE") + if nrc.cniConfTemplateFile != "" { + // The CNI conf file will be written from the template, so only the template needs to exist. + // #nosec G703: cniConfTemplateFile is not untrusted user input + if _, err := os.Stat(nrc.cniConfTemplateFile); os.IsNotExist(err) { + return nil, errors.New("CNI conf template file " + nrc.cniConfTemplateFile + " does not exist.") + } + } else { + // #nosec G703: cniConfFile is not untrusted user input + if _, err := os.Stat(nrc.cniConfFile); os.IsNotExist(err) { + return nil, errors.New("CNI conf file " + nrc.cniConfFile + " does not exist.") + } } } diff --git a/pkg/utils/cni.go b/pkg/utils/cni.go index a93f9e724..dd4d94631 100644 --- a/pkg/utils/cni.go +++ b/pkg/utils/cni.go @@ -16,24 +16,39 @@ type CNINetworkConfig struct { } func NewCNINetworkConfig(cniConfFilePath string) (*CNINetworkConfig, error) { + return NewCNINetworkConfigFromTemplate(cniConfFilePath, "") +} + +func NewCNINetworkConfigFromTemplate(cniConfFilePath, cniConfTemplateFilePath string) (*CNINetworkConfig, error) { cniNetConf := CNINetworkConfig{ filePath: cniConfFilePath, } - cniFileBytes, err := os.ReadFile(cniConfFilePath) + fileDesc, srcFilePath := "configuration file", cniConfFilePath + if cniConfTemplateFilePath != "" { + fileDesc, srcFilePath = "configuration template file", cniConfTemplateFilePath + } + + cniFileBytes, err := os.ReadFile(srcFilePath) if err != nil { - return nil, fmt.Errorf("error reading %s: %w", cniConfFilePath, err) + return nil, fmt.Errorf("failed to read %s: %w", fileDesc, err) } // If we're working with a conflist setup - if cniNetConf.IsConfList() { + if strings.HasSuffix(strings.ToLower(cniConfFilePath), ".conflist") { confList := new(ConfList) err = json.Unmarshal(cniFileBytes, confList) if err != nil { - return nil, fmt.Errorf("failed to load CNI conflist file: %w", err) + return nil, fmt.Errorf("failed to parse %s %s as configuration list: %w", fileDesc, srcFilePath, err) } - if len(confList.Plugins) == 0 { - return nil, fmt.Errorf("CNI config list %s has no plugins", cniConfFilePath) + for _, conf := range confList.Plugins { + if conf != nil && conf.Type == "bridge" { + cniNetConf.conf = conf + break + } + } + if cniNetConf.conf == nil { + return nil, fmt.Errorf("configuration list in %s %s has no bridge plugin", fileDesc, srcFilePath) } cniNetConf.confList = confList } else { @@ -41,10 +56,10 @@ func NewCNINetworkConfig(cniConfFilePath string) (*CNINetworkConfig, error) { conf := new(Conf) err = json.Unmarshal(cniFileBytes, conf) if err != nil { - return nil, fmt.Errorf("failed to load CNI conf file: %w", err) + return nil, fmt.Errorf("failed to parse %s %s: %w", fileDesc, srcFilePath, err) } if conf.Type == "" { - return nil, fmt.Errorf("error load CNI config, file appears to have no type: %s", cniConfFilePath) + return nil, fmt.Errorf("configuration in %s %s has no type", fileDesc, srcFilePath) } cniNetConf.conf = conf } @@ -60,30 +75,30 @@ func NewCNINetworkConfig(cniConfFilePath string) (*CNINetworkConfig, error) { // newer ranges variation. To account for this and make parsing simpler, we do the same thing that the official IPAM // config loader does and collapse them into ranges. func (c *CNINetworkConfig) consolidateSubnets() error { - brPlug := c.getBridgePlugin() - if brPlug.IPAM.Subnet != "" { - err := c.InsertPodCIDRIntoIPAM(brPlug.IPAM.Subnet) + if c.conf.IPAM.Subnet != "" { + err := c.InsertPodCIDRIntoIPAM(c.conf.IPAM.Subnet) if err != nil { return err } - brPlug.IPAM.Subnet = "" - delete(brPlug.IPAM.raw, "subnet") + c.conf.IPAM.Subnet = "" + delete(c.conf.IPAM.raw, "subnet") } return nil } -// IsConfList checks to see if this CNI configuration is a *.conflist file or if it is a *.conf file. Returns true for -// *.conflist, returns false for anything else. +// IsConfList checks to see if this CNI configuration is a conflist or a conf. +// +// Deprecated: This is internal state. func (c *CNINetworkConfig) IsConfList() bool { - return strings.HasSuffix(strings.ToLower(c.filePath), ".conflist") + return c.confList != nil } // getPodCIDRsMapFromCNISpec gets pod CIDR allocated to the node as a map from CNI spec file and returns it func (c *CNINetworkConfig) getPodCIDRsMapFromCNISpec() (map[string]*net.IPNet, error) { podCIDRs := make(map[string]*net.IPNet) - ipamConfig := c.getBridgePlugin().IPAM + ipamConfig := c.conf.IPAM // Parse ranges from ipamConfig if ipamConfig != nil && len(ipamConfig.Ranges) > 0 { @@ -117,23 +132,11 @@ func (c *CNINetworkConfig) GetPodCIDRsFromCNISpec() ([]*net.IPNet, error) { return podCIDRs, nil } -// getBridgePlugin get the bridge plugin configuration out of the cniNetworkConfig in a consistent manner -func (c *CNINetworkConfig) getBridgePlugin() *Conf { - if c.confList != nil { - for _, conf := range c.confList.Plugins { - if conf.Type == "bridge" { - return conf - } - } - } - return c.conf -} - // InsertPodCIDRIntoIPAM insert a new cidr into the CNI file. If the CIDR already exists in the CNI ranges, then // operation is a noop. Throws an error if either the passed cidr cannot be parsed or if there is a problem with the // CIDRs already in the CNI config. func (c *CNINetworkConfig) InsertPodCIDRIntoIPAM(cidr string) error { - ipamConfig := c.getBridgePlugin().IPAM + ipamConfig := c.conf.IPAM // This should have already been sanitized by the GetPodCIDR* functions before it comes to us, but you can never be // too safe... @@ -159,14 +162,13 @@ func (c *CNINetworkConfig) InsertPodCIDRIntoIPAM(cidr string) error { } func (c *CNINetworkConfig) SetMTU(mtu int) { - brPlugin := c.getBridgePlugin() - brPlugin.MTU = float64(mtu) + c.conf.MTU = float64(mtu) } func (c *CNINetworkConfig) WriteCNIConfig() error { var cniBytes []byte var err error - if c.IsConfList() { + if c.confList != nil { cniBytes, err = json.Marshal(c.confList) if err != nil { return fmt.Errorf("unable to marshal CNI ConfList: %w", err) diff --git a/pkg/utils/cni_test.go b/pkg/utils/cni_test.go index c65d1c5bf..557a71d32 100644 --- a/pkg/utils/cni_test.go +++ b/pkg/utils/cni_test.go @@ -2,19 +2,41 @@ package utils import ( "encoding/json" - "errors" "fmt" + "maps" "os" - "path" - "strings" + "path/filepath" + "slices" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -func getConfList() []byte { - return []byte(` -{ +const ( + cniConfTestFileName = "10-kuberouter.conf" + cniConfListTestFileName = "10-kuberouter.conflist" +) + +type cniConfContent struct { + name string + isConfList bool + bytes []byte +} + +func (c *cniConfContent) fileName() string { + if c.isConfList { + return cniConfListTestFileName + } + return cniConfTestFileName +} + +func getNullConfList() cniConfContent { + return cniConfContent{"nullConfList", true, []byte(`null`)} +} + +func getConfList() cniConfContent { + return cniConfContent{"confList", true, []byte(`{ "cniVersion":"0.3.0", "name":"mynet", "plugins":[ @@ -31,12 +53,11 @@ func getConfList() []byte { } ] } -`) +`)} } -func getConfListWithRanges() []byte { - return []byte(` -{ +func getConfListWithRanges() cniConfContent { + return cniConfContent{"confListWithRanges", true, []byte(`{ "cniVersion":"0.3.0", "name":"mynet", "plugins":[ @@ -45,22 +66,22 @@ func getConfListWithRanges() []byte { "ipam":{ "ranges": [ [ - { + { "subnet":"10.242.0.0/24" - }, - { + }, + { "subnet":"10.242.1.0/24" - } + } ], [ - { + { "subnet":"10.242.2.0/24" - }, - { + }, + { "subnet":"10.242.3.0/24" - } + } ] - ], + ], "subnet": "10.242.4.0/24", "type":"host-local" }, @@ -71,12 +92,11 @@ func getConfListWithRanges() []byte { } ] } -`) +`)} } -func getConfListWithDuplicateRanges() []byte { - return []byte(` -{ +func getConfListWithDuplicateRanges() cniConfContent { + return cniConfContent{"confListWithDuplicateRanges", true, []byte(`{ "cniVersion":"0.3.0", "name":"mynet", "plugins":[ @@ -85,22 +105,22 @@ func getConfListWithDuplicateRanges() []byte { "ipam":{ "ranges": [ [ - { + { "subnet":"10.242.0.0/24" - }, - { + }, + { "subnet":"10.242.1.0/24" - } + } ], [ - { + { "subnet":"10.242.2.0/24" - }, - { + }, + { "subnet":"10.242.3.0/24" - } + } ] - ], + ], "subnet": "10.242.0.0/24", "type":"host-local" }, @@ -111,12 +131,11 @@ func getConfListWithDuplicateRanges() []byte { } ] } -`) +`)} } -func getConfListWithIPv6DuplicateRanges() []byte { - return []byte(` -{ +func getConfListWithIPv6DuplicateRanges() cniConfContent { + return cniConfContent{"confListWithIPv6DuplicateRanges", true, []byte(`{ "cniVersion":"0.3.0", "name":"mynet", "plugins":[ @@ -125,22 +144,22 @@ func getConfListWithIPv6DuplicateRanges() []byte { "ipam":{ "ranges": [ [ - { + { "subnet":"10.242.0.0/24" - }, - { + }, + { "subnet":"10.242.1.0/24" - } + } ], [ - { + { "subnet":"10.242.2.0/24" - }, - { + }, + { "subnet":"2001:db8:42:2::/64" - } + } ] - ], + ], "subnet": "2001:db8:42:2::/64", "type":"host-local" }, @@ -151,12 +170,11 @@ func getConfListWithIPv6DuplicateRanges() []byte { } ] } -`) +`)} } -func getConfListWithNoSubnet() []byte { - return []byte(` -{ +func getConfListWithNoSubnet() cniConfContent { + return cniConfContent{"confListWithNoSubnet", true, []byte(`{ "cniVersion":"0.3.0", "name":"mynet", "plugins":[ @@ -171,21 +189,39 @@ func getConfListWithNoSubnet() []byte { } ] } -`) +`)} } -func getConfListWithNoPlugins() []byte { - return []byte(` -{ +func getConfListWithNoPlugins() cniConfContent { + return cniConfContent{"confListWithNoPlugins", true, []byte(`{ "cniVersion":"0.3.0", "name":"mynet" +}`)} +} + +func getConfListWithNullPlugin() cniConfContent { + return cniConfContent{"getConfListWithNullPlugin", true, []byte(`{ + "cniVersion": "1.1.0", + "name":"mynet", + "plugins":[null] +}`)} } -`) + +func getConfListWithoutBridgePlugin() cniConfContent { + return cniConfContent{"confListWithoutBridgePlugin", true, []byte(`{ + "cniVersion": "1.1.0", + "name":"mynet", + "plugins":[ + { + "type": "portmap", + "capabilities": {"portMappings": true} + } + ] +}`)} } -func getConf() []byte { - return []byte(` -{ +func getConf() cniConfContent { + return cniConfContent{"conf", false, []byte(`{ "cniVersion":"0.3.0", "name":"mynet", "bridge":"kube-bridge", @@ -197,313 +233,216 @@ func getConf() []byte { "name":"kubernetes", "type":"bridge" } -`) +`)} } -func getConfWithNoSubnet() []byte { - return []byte(` -{ +func getConfWithNoSubnet() cniConfContent { + return cniConfContent{"confWithNoSubnet", false, []byte(`{ "cniVersion":"0.3.0", "name":"mynet", "bridge":"kube-bridge", "ipam":{ - "type":"host-local" + "type":"host-local" }, "isDefaultGateway":true, "name":"kubernetes", "type":"bridge" } -`) +`)} } -func getConfWithNoType() []byte { - return []byte(` -{ +func getConfWithNoType() cniConfContent { + return cniConfContent{"confWithNoType", false, []byte(`{ "cniVersion":"0.3.0", "name":"mynet", "bridge":"kube-bridge", "ipam":{ - "type":"host-local" + "type":"host-local" }, "isDefaultGateway":true, "name":"kubernetes" } -`) +`)} } -func TestMarshalUnmarshalRestoration(t *testing.T) { - t.Run("Ensure ConfList is parsed and unparsed properly", func(t *testing.T) { - before := getConfList() - cl := new(ConfList) - - err := json.Unmarshal(before, cl) - if err != nil { - t.Fatalf("wasn't able to unmarshal JSON in test: %s", before) - } - - after, err := json.MarshalIndent(cl, "", " ") - if err != nil { - t.Fatalf("wasn't able to marshal JSON in test: %s", before) - } - - assert.JSONEqf(t, string(before), string(after), - "JSON is not equal!\nBefore:\n%s\nAfter:\n%s\n", before, after) - }) - t.Run("Ensure ConfListWithRange is parsed and unparsed properly", func(t *testing.T) { - before := getConfListWithRanges() - cl := new(ConfList) - - err := json.Unmarshal(before, cl) - if err != nil { - t.Fatalf("wasn't able to unmarshal JSON in test: %s", before) - } - - after, err := json.MarshalIndent(cl, "", " ") - if err != nil { - t.Fatalf("wasn't able to marshal JSON in test: %s", before) - } - - assert.JSONEqf(t, string(before), string(after), - "JSON is not equal!\nBefore:\n%s\nAfter:\n%s\n", before, after) - }) - t.Run("Ensure ConfListWithNoSubnet is parsed and unparsed properly", func(t *testing.T) { - before := getConfListWithNoSubnet() - cl := new(ConfList) - - err := json.Unmarshal(before, cl) - if err != nil { - t.Fatalf("wasn't able to unmarshal JSON in test: %s", before) - } - - after, err := json.MarshalIndent(cl, "", " ") - if err != nil { - t.Fatalf("wasn't able to marshal JSON in test: %s", before) - } +func allCNIConfContents() []cniConfContent { + return []cniConfContent{ + getNullConfList(), + getConfList(), + getConfListWithRanges(), + getConfListWithDuplicateRanges(), + getConfListWithIPv6DuplicateRanges(), + getConfListWithNoSubnet(), + getConfListWithNoPlugins(), + getConfListWithNullPlugin(), + getConfListWithoutBridgePlugin(), + getConf(), + getConfWithNoSubnet(), + getConfWithNoType(), + } +} - assert.JSONEqf(t, string(before), string(after), - "JSON is not equal!\nBefore:\n%s\nAfter:\n%s\n", before, after) - }) - t.Run("Ensure ConfWithNoSubnet is parsed and unparsed properly", func(t *testing.T) { - before := getConfWithNoSubnet() - c := new(Conf) +func TestMarshalUnmarshalRestoration(t *testing.T) { + for _, content := range allCNIConfContents() { + t.Run(content.name, func(t *testing.T) { + t.Parallel() - err := json.Unmarshal(before, c) - if err != nil { - t.Fatalf("wasn't able to unmarshal JSON in test: %s", before) - } + var obj any + if content.isConfList { + obj = new(ConfList) + } else { + obj = new(Conf) + } - after, err := json.MarshalIndent(c, "", " ") - if err != nil { - t.Fatalf("wasn't able to marshal JSON in test: %s", before) - } + require.NoError(t, json.Unmarshal(content.bytes, obj)) + after, err := json.Marshal(obj) + require.NoError(t, err) - assert.JSONEqf(t, string(before), string(after), - "JSON is not equal!\nBefore:\n%s\nAfter:\n%s\n", before, after) - }) + assert.JSONEq(t, string(content.bytes), string(after)) + }) + } } func TestNewCNINetworkConfig(t *testing.T) { testcases := []struct { - name string - filename string - isConfList bool - content []byte - err error - ranges []string + name string + content cniConfContent + err string + ranges []string }{ { - name: "Attempt reading from conf", - filename: "10-kuberouter.conf", - isConfList: false, - content: getConf(), - err: nil, + name: "Attempt reading from conf", + content: getConf(), + ranges: []string{"10.242.0.0/24"}, + }, + { + name: "Rejects null conflists", + content: getNullConfList(), + err: "10-kuberouter.conflist has no bridge plugin", + }, + { + name: "Attempt reading from conflist", + content: getConfList(), + ranges: []string{"10.242.0.0/24"}, + }, + { + name: "Ensure error upon reading from conf with no type", + content: getConfWithNoType(), + err: "10-kuberouter.conf has no type", }, { - name: "Attempt reading from conflist", - filename: "10-kuberouter.conflist", - isConfList: true, - content: getConfList(), - err: nil, + name: "Ensure error upon reading from conflist with no plugins", + content: getConfListWithNoPlugins(), + err: "10-kuberouter.conflist has no bridge plugin", }, { - name: "Ensure error upon reading from conf with no type", - filename: "10-kuberouter.conf", - isConfList: false, - content: getConfWithNoType(), - err: errors.New("error load CNI config, file appears to have no type: "), + name: "Ensure conf subnet get consolidated into ranges when only subnet exists", + content: getConf(), + ranges: []string{"10.242.0.0/24"}, }, { - name: "Ensure error upon reading from conflist with no plugins", - filename: "10-kuberouter.conflist", - isConfList: true, - content: getConfListWithNoPlugins(), - err: errors.New("CNI config list "), + name: "Ensure conflist subnet get consolidated into ranges when only subnet exists", + content: getConfList(), + ranges: []string{"10.242.0.0/24"}, }, { - name: "Ensure conf subnet get consolidated into ranges when only subnet exists", - filename: "10-kuberouter.conf", - isConfList: false, - content: getConf(), - err: nil, - ranges: []string{"10.242.0.0/24"}, + name: "Ensure conflist subnets get consolidated with ranges when both exist", + content: getConfListWithRanges(), + ranges: []string{"10.242.0.0/24", "10.242.1.0/24", "10.242.2.0/24", "10.242.3.0/24", "10.242.4.0/24"}, }, { - name: "Ensure conflist subnet get consolidated into ranges when only subnet exists", - filename: "10-kuberouter.conflist", - isConfList: true, - content: getConfList(), - err: nil, - ranges: []string{"10.242.0.0/24"}, + name: "Ensure conflist subnets get de-deduplicated with ranges when repeats exist", + content: getConfListWithDuplicateRanges(), + ranges: []string{"10.242.0.0/24", "10.242.1.0/24", "10.242.2.0/24", "10.242.3.0/24"}, }, { - name: "Ensure conflist subnets get consolidated with ranges when both exist", - filename: "10-kuberouter.conflist", - isConfList: true, - content: getConfListWithRanges(), - err: nil, - ranges: []string{"10.242.0.0/24", "10.242.1.0/24", "10.242.2.0/24", "10.242.3.0/24", "10.242.4.0/24"}, + name: "Doesn't panic on conflists with null plugins", + content: getConfListWithNullPlugin(), + err: "10-kuberouter.conflist has no bridge plugin", }, { - name: "Ensure conflist subnets get de-deduplicated with ranges when repeats exist", - filename: "10-kuberouter.conflist", - isConfList: true, - content: getConfListWithDuplicateRanges(), - err: nil, - ranges: []string{"10.242.0.0/24", "10.242.1.0/24", "10.242.2.0/24", "10.242.3.0/24"}, + name: "Rejects conflists without bridge plugin", + content: getConfListWithoutBridgePlugin(), + err: "10-kuberouter.conflist has no bridge plugin", }, } for _, testcase := range testcases { t.Run(testcase.name, func(t *testing.T) { - file, tmpDir, err := createFile(testcase.content, testcase.filename) - if err != nil { - t.Fatalf("Failed to create temporary CNI config file: %v", err) - } - defer os.RemoveAll(tmpDir) + confFilePath := filepath.Join(t.TempDir(), testcase.content.fileName()) + require.NoError(t, os.WriteFile(confFilePath, testcase.content.bytes, 0600)) - cni, err := NewCNINetworkConfig(file.Name()) - if err != nil { - if testcase.err == nil { - assert.Fail(t, "if error from NewCNINetworkConfig is not nil, the testcase shouldn't be "+ - "nil either") - } - assert.True(t, strings.HasPrefix(err.Error(), testcase.err.Error())) + cni, err := NewCNINetworkConfig(confFilePath) + if testcase.err != "" { + assert.Nil(t, cni) + assert.ErrorContains(t, err, testcase.err) return } + require.NoError(t, err) + require.NotNil(t, cni) - assert.Equal(t, testcase.isConfList, cni.IsConfList()) - - if testcase.ranges != nil { - assert.Emptyf(t, cni.getBridgePlugin().IPAM.Subnet, - "subnet of cniNetworkConfig should always be empty because it should be consolidated with "+ - "ranges upon creation") - - foundSubnets := make(map[string]any, 0) - for _, rangeSet := range cni.getBridgePlugin().IPAM.Ranges { - for _, rangeSubnet := range rangeSet { - foundSubnets[rangeSubnet.Subnet] = struct{}{} - } - } - - assert.Len(t, foundSubnets, len(testcase.ranges)) - - for _, subnet := range testcase.ranges { - _, found := foundSubnets[subnet] - assert.Truef(t, found, "subnet %s from testcase should have been found in the ranges inside "+ - "cniNetworkConfig", subnet) - } + if testcase.content.isConfList { + assert.NotNilf(t, cni.confList, "Expected a conflist for %s", testcase.content.name) + } else { + assert.Nilf(t, cni.confList, "Didn't expect a conflist for %s", testcase.content.name) } + + podCIDRs, err := cni.getPodCIDRsMapFromCNISpec() + require.NoError(t, err) + assert.ElementsMatch(t, testcase.ranges, slices.Collect(maps.Keys(podCIDRs))) }) } } func TestCniNetworkConfig_GetPodCIDRsFromCNISpec(t *testing.T) { testcases := []struct { - name string - filename string - isConfList bool - content []byte - err error - ranges []string + name string + content cniConfContent + ranges []string }{ { - name: "Ensure conf subnet get consolidated into ranges when only subnet exists", - filename: "10-kuberouter.conf", - isConfList: false, - content: getConf(), - err: nil, - ranges: []string{"10.242.0.0/24"}, + name: "Ensure conf subnet get consolidated into ranges when only subnet exists", + content: getConf(), + ranges: []string{"10.242.0.0/24"}, }, { - name: "Ensure conflist subnet get consolidated into ranges when only subnet exists", - filename: "10-kuberouter.conflist", - isConfList: true, - content: getConfList(), - err: nil, - ranges: []string{"10.242.0.0/24"}, + name: "Ensure conflist subnet get consolidated into ranges when only subnet exists", + content: getConfList(), + ranges: []string{"10.242.0.0/24"}, }, { - name: "Ensure conflist subnets get consolidated with ranges when both exist", - filename: "10-kuberouter.conflist", - isConfList: true, - content: getConfListWithRanges(), - err: nil, - ranges: []string{"10.242.0.0/24", "10.242.1.0/24", "10.242.2.0/24", "10.242.3.0/24", "10.242.4.0/24"}, + name: "Ensure conflist subnets get consolidated with ranges when both exist", + content: getConfListWithRanges(), + ranges: []string{"10.242.0.0/24", "10.242.1.0/24", "10.242.2.0/24", "10.242.3.0/24", "10.242.4.0/24"}, }, { - name: "Ensure conflist subnets get de-deduplicated with ranges when repeats exist", - filename: "10-kuberouter.conflist", - isConfList: true, - content: getConfListWithDuplicateRanges(), - err: nil, - ranges: []string{"10.242.0.0/24", "10.242.1.0/24", "10.242.2.0/24", "10.242.3.0/24"}, + name: "Ensure conflist subnets get de-deduplicated with ranges when repeats exist", + content: getConfListWithDuplicateRanges(), + ranges: []string{"10.242.0.0/24", "10.242.1.0/24", "10.242.2.0/24", "10.242.3.0/24"}, }, { - name: "Ensure conflist subnets get de-deduplicated with ranges when repeats exist", - filename: "10-kuberouter.conflist", - isConfList: true, - content: getConfListWithIPv6DuplicateRanges(), - err: nil, - ranges: []string{"10.242.0.0/24", "10.242.1.0/24", "10.242.2.0/24", "2001:db8:42:2::/64"}, + name: "Ensure conflist subnets get de-deduplicated with ranges when repeats exist", + content: getConfListWithIPv6DuplicateRanges(), + ranges: []string{"10.242.0.0/24", "10.242.1.0/24", "10.242.2.0/24", "2001:db8:42:2::/64"}, }, } for _, testcase := range testcases { t.Run(testcase.name, func(t *testing.T) { - file, tmpDir, err := createFile(testcase.content, testcase.filename) - if err != nil { - t.Fatalf("Failed to create temporary CNI config file: %v", err) - } - defer os.RemoveAll(tmpDir) - - cni, err := NewCNINetworkConfig(file.Name()) - assert.Equal(t, testcase.err, err) - if err != nil { - return - } - - assert.Equal(t, testcase.isConfList, cni.IsConfList()) - - if testcase.ranges != nil { - assert.Emptyf(t, cni.getBridgePlugin().IPAM.Subnet, - "subnet of cniNetworkConfig should always be empty because it should be consolidated with "+ - "ranges upon creation") - - foundSubnets, err := cni.GetPodCIDRsFromCNISpec() + confFilePath := filepath.Join(t.TempDir(), testcase.content.fileName()) + require.NoError(t, os.WriteFile(confFilePath, testcase.content.bytes, 0600)) - assert.Nil(t, err, "err should be nil at this point") + cni, err := NewCNINetworkConfig(confFilePath) + require.NoError(t, err) + require.NotNil(t, cni) - assert.Len(t, foundSubnets, len(testcase.ranges)) - - for _, subnet := range testcase.ranges { - found := false - for _, foundSubnet := range foundSubnets { - if subnet == foundSubnet.String() { - found = true - } - } - assert.Truef(t, found, "subnet %s from testcase should have been found in the ranges inside "+ - "cniNetworkConfig", subnet) - } + if testcase.content.isConfList { + assert.NotNilf(t, cni.confList, "Expected a conflist for %s", testcase.content.name) + } else { + assert.Nilf(t, cni.confList, "Didn't expect a conflist for %s", testcase.content.name) } + + podCIDRs, err := cni.getPodCIDRsMapFromCNISpec() + require.NoError(t, err) + assert.ElementsMatch(t, testcase.ranges, slices.Collect(maps.Keys(podCIDRs))) }) } } @@ -511,66 +450,47 @@ func TestCniNetworkConfig_GetPodCIDRsFromCNISpec(t *testing.T) { func TestCniNetworkConfig_InsertPodCIDRIntoIPAM(t *testing.T) { testcases := []struct { name string - filename string - isConfList bool - content []byte - err error + content cniConfContent + err string ranges []string insertRanges []string }{ { name: "Ensure passed CIDR is properly inserted into a CNI conf with no subnets defined", - filename: "10-kuberouter.conf", - isConfList: false, content: getConfWithNoSubnet(), - err: nil, ranges: []string{"10.242.0.0/24"}, insertRanges: []string{"10.242.0.0/24"}, }, { name: "Ensure multiple CIDRs are properly inserted into a CNI conf with no subnets defined", - filename: "10-kuberouter.conflist", - isConfList: true, content: getConfListWithNoSubnet(), - err: nil, ranges: []string{"10.242.0.0/24", "10.242.1.0/24"}, insertRanges: []string{"10.242.0.0/24", "10.242.1.0/24"}, }, { name: "Ensure multiple IPv4 & IPv6 CIDRs are properly inserted into a CNI conf with no subnets" + "defined", - filename: "10-kuberouter.conflist", - isConfList: true, content: getConfListWithNoSubnet(), - err: nil, ranges: []string{"10.242.0.0/24", "2001:db8:42:2::/64"}, insertRanges: []string{"10.242.0.0/24", "2001:db8:42:2::/64"}, }, { - name: "Ensure that new subnets are inserted into a conflist with existing ranges", - filename: "10-kuberouter.conflist", - isConfList: true, - content: getConfListWithRanges(), - err: nil, + name: "Ensure that new subnets are inserted into a conflist with existing ranges", + content: getConfListWithRanges(), ranges: []string{"10.242.0.0/24", "10.242.1.0/24", "10.242.2.0/24", "10.242.3.0/24", "10.242.4.0/24", "10.242.5.0/24", "10.242.6.0/24"}, insertRanges: []string{"10.242.5.0/24", "10.242.6.0/24"}, }, { name: "Ensure duplicates are not inserted without error", - filename: "10-kuberouter.conflist", - isConfList: true, content: getConfListWithDuplicateRanges(), - err: nil, ranges: []string{"10.242.0.0/24", "10.242.1.0/24", "10.242.2.0/24", "10.242.3.0/24", "10.242.4.0/24"}, insertRanges: []string{"10.242.4.0/24"}, }, { - name: "Ensure error is thrown for bad cidr", - filename: "10-kuberouter.conflist", - isConfList: true, - content: getConfListWithDuplicateRanges(), - err: fmt.Errorf("unable to parse input cidr: %s - %s", "10.242.4.0", + name: "Ensure error is thrown for bad cidr", + content: getConfListWithDuplicateRanges(), + err: fmt.Sprintf("unable to parse input cidr: %s - %s", "10.242.4.0", "invalid CIDR address: 10.242.4.0"), ranges: []string{"10.242.0.0/24", "10.242.1.0/24", "10.242.2.0/24", "10.242.3.0/24"}, insertRanges: []string{"10.242.4.0"}, @@ -578,23 +498,20 @@ func TestCniNetworkConfig_InsertPodCIDRIntoIPAM(t *testing.T) { } for _, testcase := range testcases { t.Run(testcase.name, func(t *testing.T) { - file, tmpDir, err := createFile(testcase.content, testcase.filename) - if err != nil { - t.Fatalf("Failed to create temporary CNI config file: %v", err) - } - defer os.RemoveAll(tmpDir) + confFilePath := filepath.Join(t.TempDir(), testcase.content.fileName()) + require.NoError(t, os.WriteFile(confFilePath, testcase.content.bytes, 0600)) - cni, err := NewCNINetworkConfig(file.Name()) + cni, err := NewCNINetworkConfig(confFilePath) if err != nil { assert.Fail(t, "err should always be nil when calling NewCNINetworkConfig for this suite") } for _, cidr := range testcase.insertRanges { err = cni.InsertPodCIDRIntoIPAM(cidr) - if testcase.err != nil { - assert.EqualError(t, err, testcase.err.Error()) + if testcase.err != "" { + assert.EqualError(t, err, testcase.err) } else { - assert.NoError(t, err) + assert.NoErrorf(t, err, "While inserting %s", cidr) } } @@ -611,53 +528,35 @@ func TestCniNetworkConfig_InsertPodCIDRIntoIPAM(t *testing.T) { func TestCniNetworkConfig_WriteCNIConfig(t *testing.T) { testcases := []struct { name string - filename string - isConfList bool - content []byte - err error + content cniConfContent ranges []string insertRanges []string }{ { - name: "Ensure written file is the same as read file when no ranges were inserted", - filename: "10-kuberouter.conf", - isConfList: false, - content: getConfWithNoSubnet(), - err: nil, + name: "Ensure written file is the same as read file when no ranges were inserted", + content: getConfWithNoSubnet(), }, { name: "Ensure written conf file contains single subnet", - filename: "10-kuberouter.conf", - isConfList: false, content: getConf(), - err: nil, ranges: []string{"10.242.0.0/24"}, insertRanges: []string{"10.242.0.0/24"}, }, { name: "Ensure written conflist file contains multiple subnets", - filename: "10-kuberouter.conflist", - isConfList: true, content: getConfListWithNoSubnet(), - err: nil, ranges: []string{"10.242.0.0/24", "10.242.1.0/24"}, insertRanges: []string{"10.242.0.0/24", "10.242.1.0/24"}, }, { name: "Ensure written conflist file has IPv4 & IPv6 CIDRs properly inserted", - filename: "10-kuberouter.conflist", - isConfList: true, content: getConfListWithNoSubnet(), - err: nil, ranges: []string{"10.242.0.0/24", "2001:db8:42:2::/64"}, insertRanges: []string{"10.242.0.0/24", "2001:db8:42:2::/64"}, }, { - name: "Ensure that conflist file has multiple subnets written when ranges already exist", - filename: "10-kuberouter.conflist", - isConfList: true, - content: getConfListWithRanges(), - err: nil, + name: "Ensure that conflist file has multiple subnets written when ranges already exist", + content: getConfListWithRanges(), ranges: []string{"10.242.0.0/24", "10.242.1.0/24", "10.242.2.0/24", "10.242.3.0/24", "10.242.4.0/24", "10.242.5.0/24", "10.242.6.0/24"}, insertRanges: []string{"10.242.5.0/24", "10.242.6.0/24"}, @@ -665,21 +564,18 @@ func TestCniNetworkConfig_WriteCNIConfig(t *testing.T) { } for _, testcase := range testcases { t.Run(testcase.name, func(t *testing.T) { - file, tmpDir, err := createFile(testcase.content, testcase.filename) - if err != nil { - t.Fatalf("Failed to create temporary CNI config file: %v", err) - } - defer os.RemoveAll(tmpDir) + confFilePath := filepath.Join(t.TempDir(), testcase.content.fileName()) + require.NoError(t, os.WriteFile(confFilePath, testcase.content.bytes, 0600)) - cni, err := NewCNINetworkConfig(file.Name()) + cni, err := NewCNINetworkConfig(confFilePath) if err != nil { assert.Fail(t, "err should always be nil when calling NewCNINetworkConfig for this suite") } if testcase.insertRanges != nil { for _, cidr := range testcase.insertRanges { - err = cni.InsertPodCIDRIntoIPAM(cidr) - assert.Equal(t, testcase.err, err) + err := cni.InsertPodCIDRIntoIPAM(cidr) + require.NoError(t, err) } } @@ -690,12 +586,12 @@ func TestCniNetworkConfig_WriteCNIConfig(t *testing.T) { // Read the CNI directly to ensure that subnet is really removed (which wouldn't be detected upon // re-initialization of NewCNINetworkConfig below because of how it treats subnets - cniFileBytes, err := os.ReadFile(file.Name()) + cniFileBytes, err := os.ReadFile(confFilePath) if err != nil { t.Fatalf("we should be able to read the CNI file we just wrote to") } var brPlug *Conf - if cni.IsConfList() { + if testcase.content.isConfList { cl := new(ConfList) err = json.Unmarshal(cniFileBytes, cl) if err != nil { @@ -721,54 +617,70 @@ func TestCniNetworkConfig_WriteCNIConfig(t *testing.T) { assert.Emptyf(t, brPlug.IPAM.Subnet, "upon calling WriteCNIConfig() subnet should ALWAYS be blank "+ "because it should have been consolidated with ranges") - cni, err = NewCNINetworkConfig(file.Name()) - if err != nil { - assert.Fail(t, "err should always be nil when calling NewCNINetworkConfig for this suite") - } - - if testcase.ranges != nil { - assert.Emptyf(t, cni.getBridgePlugin().IPAM.Subnet, - "subnet of cniNetworkConfig should always be empty because it should be consolidated with "+ - "ranges upon creation") - - foundSubnets := make(map[string]any, 0) - for _, rangeSet := range cni.getBridgePlugin().IPAM.Ranges { - for _, rangeSubnet := range rangeSet { - foundSubnets[rangeSubnet.Subnet] = struct{}{} - } - } - - assert.Len(t, foundSubnets, len(testcase.ranges)) + cni, err = NewCNINetworkConfig(confFilePath) + require.NoError(t, err) + require.NotNil(t, cni) - for _, subnet := range testcase.ranges { - _, found := foundSubnets[subnet] - assert.Truef(t, found, "subnet %s from testcase should have been found in the ranges inside "+ - "cniNetworkConfig", subnet) - } - } else { - assert.Emptyf(t, cni.getBridgePlugin().IPAM.Ranges, - "testcase ranges was nil, the subnets re-read from the CNI file after writing should have "+ - "been empty also") - } + podCIDRs, err := cni.getPodCIDRsMapFromCNISpec() + require.NoError(t, err) + assert.ElementsMatch(t, testcase.ranges, slices.Collect(maps.Keys(podCIDRs))) }) } } -func createFile(content []byte, filename string) (*os.File, string, error) { - dir, err := os.MkdirTemp("", "kube-router-cni-test") - if err != nil { - return nil, "", fmt.Errorf("cannot create tmpdir: %v", err) - } - fullPath := path.Join(dir, filename) - file, err := os.Create(fullPath) - if err != nil { - return nil, "", fmt.Errorf("cannot create file: %v", err) - } +func TestNewCNINetworkConfig_Template(t *testing.T) { + t.Run("Ensure template is parsed according to the CNI conf file name", func(t *testing.T) { + tmpDir := t.TempDir() + templateFilePath := filepath.Join(tmpDir, "cni-conf.json") + require.NoError(t, os.WriteFile(templateFilePath, getConfList().bytes, 0600)) - if _, err = file.Write(content); err != nil { - return nil, "", fmt.Errorf("cannot write to file: %v", err) - } + cni, err := NewCNINetworkConfigFromTemplate(filepath.Join(tmpDir, cniConfListTestFileName), templateFilePath) + require.NoError(t, err) + require.NotNil(t, cni) - fmt.Println("File is ", file.Name()) - return file, dir, nil + assert.NotNil(t, cni.confList, "The CNI conf file name, not the template file name, should determine "+ + "if the config is a conflist") + }) + + t.Run("Ensure CNI conf file is written from the template", func(t *testing.T) { + tmpDir := t.TempDir() + templateFilePath := filepath.Join(tmpDir, "cni-conf.json") + require.NoError(t, os.WriteFile(templateFilePath, getConfListWithNoSubnet().bytes, 0600)) + templateFileInfo, err := os.Stat(templateFilePath) + require.NoError(t, err) + + cniConfFilePath := filepath.Join(tmpDir, cniConfListTestFileName) + cni, err := NewCNINetworkConfigFromTemplate(cniConfFilePath, templateFilePath) + require.NoError(t, err) + require.NotNil(t, cni) + + require.NoError(t, cni.InsertPodCIDRIntoIPAM("10.242.0.0/24")) + require.NoError(t, cni.WriteCNIConfig()) + + writtenCNI, err := NewCNINetworkConfig(cniConfFilePath) + require.NoError(t, err) + require.NotNil(t, writtenCNI) + + podCIDRs, err := writtenCNI.getPodCIDRsMapFromCNISpec() + require.NoError(t, err) + assert.ElementsMatch(t, []string{"10.242.0.0/24"}, slices.Collect(maps.Keys(podCIDRs))) + + if newTemplateFileInfo, err := os.Stat(templateFilePath); assert.NoError(t, err) { + assert.True(t, templateFileInfo.Size() == newTemplateFileInfo.Size() && + templateFileInfo.ModTime().Equal(newTemplateFileInfo.ModTime()), + "The template file should remain untouched") + } + + entries, err := os.ReadDir(tmpDir) + require.NoError(t, err) + assert.Lenf(t, entries, 2, "No other files besides the template and the CNI conf file should be left "+ + "behind in the CNI conf dir") + }) + + t.Run("Ensure a missing template file is an error", func(t *testing.T) { + tmpDir := t.TempDir() + _, err := NewCNINetworkConfigFromTemplate( + filepath.Join(tmpDir, cniConfListTestFileName), filepath.Join(tmpDir, "cni-conf.json")) + assert.ErrorIs(t, err, os.ErrNotExist) + }) }