CNI: Add support for OpenShift and Multus (#1527)

* Added SecurityContextConstraints CRD so that CNI has the permissions to run on OpenShift and gave the consul-cni service account permissions to reach OpenShifts SCC CRD.
* Added a --multus flag to the CNI installer so that the flag can be used by the CNI plugin.
* When --multus flag is set, create a default Network Attachment Definition CRD for Multus to use. Note, Pods require an annotation in order for Multus to run the consul-cni plugin. Please see Multus documentation for more information about how to use Multus.
* Fix: Fixed installer to rename .conf file .conflist file when consul-cni is added to a base plugin.
* Improvement: The plugin is now more consistent at updating annotations.

CHANGELOG.md

@@ -5,6 +5,8 @@
 FEATURES:
 * CLI:
   * Add support for tab autocompletion [[GH-1437](https://github.com/hashicorp/consul-k8s/pull/1501)]
+* Consul CNI Plugin
+  * Support for OpenShift and Multus CNI plugin [[GH-1527](https://github.com/hashicorp/consul-k8s/pull/1527)]
 
 BUG FIXES:
 * Control plane

charts/consul/templates/cni-clusterrole.yaml

@@ -27,4 +27,12 @@ rules:
   - {{ template "consul.fullname" . }}-cni
   verbs:
   - use
+{{- if .Values.global.openshift.enabled}}
+- apiGroups: ["security.openshift.io"]
+  resources: ["securitycontextconstraints"]
+  resourceNames:
+  - {{ template "consul.fullname" . }}-cni
+  verbs:
+  - use
+{{- end }}
 {{- end }}

charts/consul/templates/cni-daemonset.yaml

@@ -63,6 +63,7 @@ spec:
             - -log-level={{ default .Values.global.logLevel .Values.connectInject.cni.logLevel }}
             - -cni-bin-dir={{ .Values.connectInject.cni.cniBinDir }}
             - -cni-net-dir={{ .Values.connectInject.cni.cniNetDir }}
+            - -multus={{ .Values.connectInject.cni.multus }}
           {{- with .Values.connectInject.cni.resources }}
           resources:
             {{- toYaml . | nindent 12 }}

charts/consul/templates/cni-networkattachmentdefinition.yaml

@@ -0,0 +1,25 @@
+{{- if (and (.Values.connectInject.cni.enabled) (.Values.connectInject.cni.multus)) }}
+apiVersion: "k8s.cni.cncf.io/v1"
+kind: NetworkAttachmentDefinition
+metadata:
+  name: {{ template "consul.fullname" . }}-cni
+  namespace: {{ .Release.Namespace }}
+  labels:
+    app: {{ template "consul.name" . }}
+    chart: {{ template "consul.chart" . }}
+    heritage: {{ .Release.Service }}
+    release: {{ .Release.Name }}
+    component: cni 
+spec:
+  config: '{
+            "cniVersion": "0.3.1",
+            "type": "consul-cni",
+            "cni_bin_dir": "{{ .Values.connectInject.cni.cniBinDir }}",
+            "cni_net_dir": "{{ .Values.connectInject.cni.cniNetDir }}",
+            "kubeconfig": "ZZZ-consul-cni-kubeconfig",
+            "log_level": "{{ default .Values.global.logLevel .Values.connectInject.cni.logLevel }}",
+            "multus": true,
+            "name": "consul-cni",
+            "type": "consul-cni"
+        }'
+{{- end }}

charts/consul/templates/cni-securitycontextconstraints.yaml

@@ -0,0 +1,50 @@
+{{- if (and (.Values.connectInject.cni.enabled) (.Values.global.openshift.enabled)) }}
+apiVersion: security.openshift.io/v1
+kind: SecurityContextConstraints
+metadata:
+  name: {{ template "consul.fullname" . }}-cni
+  namespace: {{ .Release.Namespace }}
+  labels:
+    app: {{ template "consul.name" . }}
+    chart: {{ template "consul.chart" . }}
+    heritage: {{ .Release.Service }}
+    release: {{ .Release.Name }}
+    component: cni
+  annotations:
+    kubernetes.io/description: {{ template "consul.fullname" . }}-cni are the security context constraints required
+      to run consul-cni.
+allowHostDirVolumePlugin: true
+allowHostIPC: false
+allowHostNetwork: false
+allowHostPID: false
+allowHostPorts: false
+allowPrivilegeEscalation: true
+allowPrivilegedContainer: true
+allowedCapabilities: null
+defaultAddCapabilities: null
+fsGroup:
+  type: MustRunAs
+groups: []
+priority: null
+readOnlyRootFilesystem: false
+requiredDropCapabilities:
+- KILL
+- MKNOD
+- SETUID
+- SETGID
+runAsUser:
+  type: MustRunAsRange
+seLinuxContext:
+  type: MustRunAs
+supplementalGroups:
+  type: MustRunAs
+users: []
+volumes:
+- configMap
+- downwardAPI
+- emptyDir
+- persistentVolumeClaim
+- projected
+- secret
+- hostPath
+{{- end }}

charts/consul/test/unit/cni-daemonset.bats

@@ -63,6 +63,7 @@ load _helpers
       --set 'connectInject.cni.logLevel=bar' \
       --set 'connectInject.cni.cniBinDir=baz' \
       --set 'connectInject.cni.cniNetDir=foo' \
+      --set 'connectInject.cni.multus=false' \
       bar \
       . | tee /dev/stderr |
       yq '.spec.template.spec.containers[0].command' | tee /dev/stderr)
@@ -86,6 +87,10 @@ load _helpers
   local actual=$(echo "$cmd" |
     yq 'any(contains("cni-net-dir=foo"))' | tee /dev/stderr)
   [ "${actual}" = "true" ]
+
+  local actual=$(echo "$cmd" |
+    yq 'any(contains("multus=false"))' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
 }
 
 #--------------------------------------------------------------------

charts/consul/test/unit/cni-networkattachmentdefinition.bats

@@ -0,0 +1,61 @@
+#!/usr/bin/env bats
+
+load _helpers
+
+@test "cni/NetworkAttachmentDefinition: disabled by default" {
+  cd `chart_dir`
+  assert_empty helm template \
+      -s templates/cni-networkattachmentdefinition.yaml  \
+      .
+}
+
+@test "cni/NetworkAttachmentDefinition: disabled when cni enabled and multus disabled" {
+  cd `chart_dir`
+  assert_empty helm template \
+      -s templates/cni-securitycontextconstraints.yaml  \
+      --set 'connectInject.enabled=true' \
+      --set 'connectInject.cni.enabled=true' \
+      --set 'connectInject.cni.multus=false' \
+      .
+}
+
+@test "cni/NetworkAttachmentDefinition: enabled when cni enabled and multus enabled" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/cni-networkattachmentdefinition.yaml  \
+      --set 'connectInject.enabled=true' \
+      --set 'connectInject.cni.enabled=true' \
+      --set 'connectInject.cni.multus=true' \
+      . | tee /dev/stderr |
+      yq -s 'length > 0' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
+@test "cni/NetworkAttachmentDefinition: config is set" {
+  cd `chart_dir`
+  local cmd=$(helm template \
+      -s templates/cni-networkattachmentdefinition.yaml  \
+      --set 'connectInject.cni.enabled=true' \
+      --set 'connectInject.enabled=true' \
+      --set 'connectInject.cni.multus=true' \
+      --set 'connectInject.cni.logLevel=bar' \
+      --set 'connectInject.cni.cniBinDir=baz' \
+      --set 'connectInject.cni.cniNetDir=foo' \
+      bar \
+      . | tee /dev/stderr |
+      yq -rc '.spec.config' | tee /dev/stderr)
+
+  local actual=$(echo "$cmd" |
+    yq '.log_level' | tee /dev/stderr)
+  [ "${actual}" = '"bar"' ]
+
+  local actual=$(echo "$cmd" |
+    yq '.cni_bin_dir' | tee /dev/stderr)
+  [ "${actual}" = '"baz"' ]
+
+  local actual=$(echo "$cmd" |
+    yq '.cni_net_dir' | tee /dev/stderr)
+  [ "${actual}" = '"foo"' ]
+
+}
+

charts/consul/test/unit/cni-securitycontextcontstraints.bats

@@ -0,0 +1,33 @@
+#!/usr/bin/env bats
+
+load _helpers
+
+@test "cni/SecurityContextConstraints: disabled by default" {
+  cd `chart_dir`
+  assert_empty helm template \
+      -s templates/cni-securitycontextconstraints.yaml  \
+      .
+}
+
+@test "cni/SecurityContextConstraints: disabled when cni disabled and global.openshift.enabled=true" {
+  cd `chart_dir`
+  assert_empty helm template \
+      -s templates/cni-securitycontextconstraints.yaml  \
+      --set 'connectInject.enabled=true' \
+      --set 'connectInject.cni.enabled=false' \
+      --set 'global.openshift.enabled=true' \
+      .
+}
+
+@test "cni/SecurityContextConstraints: enabled when cni enabled and global.openshift.enabled=true" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/cni-securitycontextconstraints.yaml  \
+      --set 'connectInject.enabled=true' \
+      --set 'connectInject.cni.enabled=true' \
+      --set 'global.openshift.enabled=true' \
+      . | tee /dev/stderr |
+      yq -s 'length > 0' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+

charts/consul/values.yaml

@@ -1960,7 +1960,20 @@ connectInject:
     # Location on the kubernetes node of all CNI configuration. Should be the absolute path and start with a '/'
     # @type: string
     cniNetDir: "/etc/cni/net.d"
-
+
+    # If multus CNI plugin is enabled with consul-cni. When enabled, consul-cni will not be installed as a chained
+    # CNI plugin. Instead, a NetworkAttachementDefinition CustomResourceDefinition (CRD) will be created in the helm
+    # release namespace. Following multus plugin standards, an annotation is required in order for the consul-cni plugin
+    # to be executed and for your service to be added to the Consul Service Mesh. 
+    #
+    # Add the annotation `'k8s.v1.cni.cncf.io/networks': '[{ "name":"consul-cni","namespace": "consul" }]'` to your pod
+    # to use the default installed NetworkAttachementDefinition CRD.
+    #
+    # Please refer to the [Multus Quickstart Guide](https://github.com/k8snetworkplumbingwg/multus-cni/blob/master/docs/quickstart.md)
+    # for more information about using multus. 
+    # @type: string
+    multus: false 
+
     # The resource settings for CNI installer daemonset.
     # @recurse: false
     # @type: map

control-plane/cni/main.go

@@ -137,26 +137,28 @@ func (c *Command) cmdAdd(args *skel.CmdArgs) error {
 		Level: hclog.LevelFromString(cfg.LogLevel),
 	})
 
-	// Check to see if the plugin is a chained plugin.
-	if cfg.PrevResult == nil {
-		return fmt.Errorf("must be called as final chained plugin")
-	}
-
 	logger.Debug("consul-cni plugin config", "config", cfg)
-	// Convert the PrevResult to a concrete Result type that can be modified. The CNI standard says
-	// that the previous result needs to be passed onto the next plugin.
-	prevResult, err := current.GetResult(cfg.PrevResult)
-	if err != nil {
-		return fmt.Errorf("failed to convert prevResult: %w", err)
-	}
 
-	if len(prevResult.IPs) == 0 {
-		return fmt.Errorf("got no container IPs")
-	}
+	// Only chained plugins have a previous result.
+	var result *current.Result
 
-	// Pass the prevResult through this plugin to the next one.
-	result := prevResult
-	logger.Debug("consul-cni previous result", "result", result)
+	// Check to see if the plugin is a chained plugin.
+	if cfg.PrevResult == nil {
+		// The plugin is not chained (ie multus) so create a fake result
+		result = &current.Result{
+			CNIVersion: "0.3.1",
+		}
+	} else {
+		prevResult, err := current.GetResult(cfg.PrevResult)
+		if err != nil {
+			return fmt.Errorf("failed to convert prevResult: %w", err)
+		}
+		if len(prevResult.IPs) == 0 {
+			return fmt.Errorf("got no container IPs")
+		}
+		// Pass the prevResult through this plugin to the next one.
+		result = prevResult
+	}
 
 	ctx := context.Background()
 	if c.client == nil {
@@ -186,7 +188,7 @@ func (c *Command) cmdAdd(args *skel.CmdArgs) error {
 
 	// We do not throw an error here because kubernetes will often throw a benign error where the pod has been
 	// updated in between the get and update of the annotation. Eventually kubernetes will update the annotation
-	ok := c.updateTransparentProxyStatusAnnotation(pod, podNamespace, waiting)
+	ok := c.updateTransparentProxyStatusAnnotation(podName, podNamespace, waiting)
 	if !ok {
 		logger.Info("unable to update %s pod annotation to waiting", keyTransparentProxyStatus)
 	}
@@ -213,7 +215,7 @@ func (c *Command) cmdAdd(args *skel.CmdArgs) error {
 
 	// We do not throw an error here because kubernetes will often throw a benign error where the pod has been
 	// updated in between the get and update of the annotation. Eventually kubernetes will update the annotation
-	ok = c.updateTransparentProxyStatusAnnotation(pod, podNamespace, complete)
+	ok = c.updateTransparentProxyStatusAnnotation(podName, podNamespace, complete)
 	if !ok {
 		logger.Info("unable to update %s pod annotation to complete", keyTransparentProxyStatus)
 	}
@@ -270,8 +272,13 @@ func parseAnnotation(pod corev1.Pod, annotation string) (iptables.Config, error)
 
 // updateTransparentProxyStatusAnnotation updates the transparent-proxy-status annotation. We use it as a simple inicator of
 // CNI status on the pod.  Failing is not fatal.
-func (c *Command) updateTransparentProxyStatusAnnotation(pod *corev1.Pod, namespace, status string) bool {
+func (c *Command) updateTransparentProxyStatusAnnotation(podName, namespace, status string) bool {
+	// Refresh the pod so that we can update it without problems
+	pod, err := c.client.CoreV1().Pods(namespace).Get(context.Background(), podName, metav1.GetOptions{})
+	if err != nil {
+		return false
+	}
 	pod.Annotations[keyTransparentProxyStatus] = status
-	_, err := c.client.CoreV1().Pods(namespace).Update(context.Background(), pod, metav1.UpdateOptions{})
+	_, err = c.client.CoreV1().Pods(namespace).Update(context.Background(), pod, metav1.UpdateOptions{})
 	return err == nil
 }

control-plane/subcommand/install-cni/cniconfig.go

@@ -16,7 +16,7 @@ import (
 // defaultCNIConfigFile gets the the correct config file from the cni net dir.
 // Adapted from kubelet: https://github.com/kubernetes/kubernetes/blob/954996e231074dc7429f7be1256a579bedd8344c/pkg/kubelet/dockershim/network/cni/cni.go#L134.
 func defaultCNIConfigFile(dir string) (string, error) {
-	files, err := libcni.ConfFiles(dir, []string{".conf", ".conflist", ".json"})
+	files, err := libcni.ConfFiles(dir, []string{".conf", ".conflist"})
 	if err != nil {
 		return "", fmt.Errorf("error while trying to find files in %s: %w", dir, err)
 	}
@@ -58,13 +58,57 @@ func defaultCNIConfigFile(dir string) (string, error) {
 			// CNI config list has no networks, skipping".
 			continue
 		}
-
 		return confFile, nil
 	}
 	// There were files but none of them were valid
 	return "", fmt.Errorf("no valid config files found in %s", dir)
 }
 
+// confListFileFromConfFile converts a .conf file into a .conflist file. Chained plugins use .conflist files.
+func confListFileFromConfFile(cfgFile string) (string, error) {
+	if !strings.HasSuffix(cfgFile, ".conf") {
+		return "", fmt.Errorf("invalid conf file: %s", cfgFile)
+	}
+
+	// Convert the .conf file into a map so that we can remove pieces of it.
+	cfgMap, err := configFileToMap(cfgFile)
+	if err != nil {
+		return "", fmt.Errorf("could not convert .conf file to map: %w", err)
+	}
+
+	// Remove the cniVersion header from the conf map.
+	delete(cfgMap, "cniVersion")
+
+	// Create the new plugins: [] section and add the contents from cfgMap to it.
+	plugins := make([]map[string]interface{}, 1)
+	plugins[0] = cfgMap
+
+	listMap := map[string]interface{}{
+		"name":       "k8s-pod-network",
+		"cniVersion": "0.3.1",
+		"plugins":    plugins,
+	}
+
+	listFile := fmt.Sprintf("%s%s", cfgFile, "list")
+
+	// Marshal into a new json object.
+	listJSON, err := json.MarshalIndent(listMap, "", "  ")
+	if err != nil {
+		return "", fmt.Errorf("error marshalling conflist: %w", err)
+	}
+
+	// Libcni nuance/bug. If the newline is missing, the cni plugin will throw errors saying that it cannot get parse the config.
+	listJSON = append(listJSON, "\n"...)
+
+	// Write the .conflist file out.
+	err = os.WriteFile(listFile, listJSON, os.FileMode(0o644))
+	if err != nil {
+		return "", fmt.Errorf("error writing conflist file %s: %w", listFile, err)
+	}
+
+	return listFile, nil
+}
+
 // The format of the main cni config file is unstructured json consisting of a header and list of plugins
 //
 // {