lower permissions when cni enabled, add file watcher and a lot of tests

control-plane/cni/config/config.go

@@ -12,7 +12,7 @@ type CNIConfig struct {
 	CNINetDir string `json:"cni_net_dir" mapstructure:"cni_net_dir"`
 	// DNSPrefix is used to determine the Consul Server DNS IP. The IP is set as an environment variable and the prefix allows us
 	// to search for it.
-	DNSPrefix string `json:"dns_prefix" mapstructure:"dns_prefix"`
+	DNSPrefix string `json:"dns_prefix"  mapstructure:"dns_prefix"`
 	// Kubeconfig file name. Can be set as a cli flag.
 	Kubeconfig string `json:"kubeconfig"  mapstructure:"kubeconfig"`
 	// LogLevl is the logging level. Can be set as a cli flag.

control-plane/cni/main.go

@@ -32,14 +32,17 @@ const (
 	// annotationInject is the key of the annotation that controls whether
 	// injection is explicitly enabled or disabled for a pod.
 	annotationInject = "consul.hashicorp.com/connect-inject"
-	// annotationCNIProxyConfig stores iptables.Config information so that the CNI plugin can use it to apply iptables rules.
+	// annotationCNIProxyConfig stores iptables.Config information so that the CNI plugin can use it to apply
+	// iptables rules.
 	annotationCNIProxyConfig = "consul.hashicorp.com/cni-proxy-config"
-	// retries is the number of backoff retries to attempt while waiting for an cni-proxy-config annotation to poplulate.
+	// retries is the number of backoff retries to attempt while waiting for an cni-proxy-config annotation to
+	// poplulate.
 	retries = 10
-	// dnsServiceHostEnvSuffix is the suffix that is used to get the DNS host IP. The DNS IP is saved as an environment variable
-	// with prefix + suffix. The prefix is passed in as cli flag to the endpoints controller which is then passed on in the
-	// cni-proxy-config annotation so that the CNI plugin can use it. The DNS environment variable usually looks like:
-	// CONSUL_CONSUL_DNS_SERVICE_HOST but the prefix can change depending on the helm install.
+	// dnsServiceHostEnvSuffix is the suffix that is used to get the DNS host IP. The DNS IP is saved as an
+	// environment variable with prefix + suffix. The prefix is passed in as cli flag to the endpoints controller
+	// which is then passed on in the cni-proxy-config annotation so that the CNI plugin can use it. The DNS
+	// environment variable usually looks like: CONSUL_CONSUL_DNS_SERVICE_HOST but the prefix can change
+	// depending on the helm install.
 	dnsServiceHostEnvSuffix = "DNS_SERVICE_HOST"
 )
 
@@ -75,7 +78,8 @@ type PluginConf struct {
 	CNIBinDir string `json:"cni_bin_dir"`
 	// CNINetDir is the locaion of the cni plugin on the node. Can be set as a cli flag.
 	CNINetDir string `json:"cni_net_dir"`
-	// DNSPrefix is used to determine the Consul Server DNS IP. The IP is set as an environment variable and the prefix allows us
+	// DNSPrefix is used to determine the Consul Server DNS IP. The IP is set as an environment variable and the
+	// prefix allows us
 	// to search for it. The DNS IP is determined using the prefix and the dnsServiceHostEnvSuffix constant.
 	DNSPrefix string `json:"dns_prefix"`
 	// Multus is if the plugin is a multus plugin. Can be set as a cli flag.
@@ -261,8 +265,8 @@ func parseAnnotation(pod corev1.Pod, annotation string) (iptables.Config, error)
 	return cfg, nil
 }
 
-// searchDNSIPFromEnvironment gets the consul server DNS IP from the pods environment variables. The prefix makes searching easier
-// return an empty string.
+// searchDNSIPFromEnvironment gets the consul server DNS IP from the pods environment variables. The prefix makes
+// searching easier return an empty string.
 func searchDNSIPFromEnvironment(pod corev1.Pod, prefix string) string {
 	var result string
 	upcaseResourcePrefix := strings.ToUpper(prefix)

control-plane/connect-inject/container_init.go

@@ -301,15 +301,25 @@ func (w *MeshWebhook) containerInit(namespace corev1.Namespace, pod corev1.Pod,
 	if tproxyEnabled {
 		// Running consul connect redirect-traffic with iptables
 		// requires both being a root user and having NET_ADMIN capability.
-		container.SecurityContext = &corev1.SecurityContext{
-			RunAsUser:  pointerToInt64(rootUserAndGroupID),
-			RunAsGroup: pointerToInt64(rootUserAndGroupID),
-			// RunAsNonRoot overrides any setting in the Pod so that we can still run as root here as required.
-			RunAsNonRoot: pointerToBool(false),
-			Privileged:   pointerToBool(true),
-			Capabilities: &corev1.Capabilities{
-				Add: []corev1.Capability{netAdminCapability},
-			},
+		if !w.EnableCNI {
+			container.SecurityContext = &corev1.SecurityContext{
+				RunAsUser:  pointerToInt64(rootUserAndGroupID),
+				RunAsGroup: pointerToInt64(rootUserAndGroupID),
+				// RunAsNonRoot overrides any setting in the Pod so that we can still run as root here as required.
+				RunAsNonRoot: pointerToBool(false),
+				Privileged:   pointerToBool(true),
+				Capabilities: &corev1.Capabilities{
+					Add: []corev1.Capability{netAdminCapability},
+				},
+			}
+		} else {
+			container.SecurityContext = &corev1.SecurityContext{
+				RunAsNonRoot: pointerToBool(true),
+				Privileged:   pointerToBool(false),
+				Capabilities: &corev1.Capabilities{
+					Drop: []corev1.Capability{"ALL"},
+				},
+			}
 		}
 	}
 

control-plane/connect-inject/container_init_test.go

@@ -227,39 +227,44 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD
 func TestHandlerContainerInit_transparentProxy(t *testing.T) {
 	cases := map[string]struct {
 		globalEnabled          bool
+		cniEnabled             bool
 		annotations            map[string]string
 		expectedContainsCmd    string
 		expectedNotContainsCmd string
 		namespaceLabel         map[string]string
 	}{
-		"enabled globally, ns not set, annotation not provided": {
+		"enabled globally, ns not set, annotation not provided, cni disabled": {
 			true,
+			false,
 			nil,
 			`/consul/connect-inject/consul connect redirect-traffic \
   -proxy-id="$(cat /consul/connect-inject/proxyid)" \
   -proxy-uid=5995`,
 			"",
 			nil,
 		},
-		"enabled globally, ns not set, annotation is false": {
+		"enabled globally, ns not set, annotation is false, cni disabled": {
 			true,
+			false,
 			map[string]string{keyTransparentProxy: "false"},
 			"",
 			`/consul/connect-inject/consul connect redirect-traffic \
   -proxy-id="$(cat /consul/connect-inject/proxyid)" \
   -proxy-uid=5995`,
 			nil,
 		},
-		"enabled globally, ns not set, annotation is true": {
+		"enabled globally, ns not set, annotation is true, cni disabled": {
 			true,
+			false,
 			map[string]string{keyTransparentProxy: "true"},
 			`/consul/connect-inject/consul connect redirect-traffic \
   -proxy-id="$(cat /consul/connect-inject/proxyid)" \
   -proxy-uid=5995`,
 			"",
 			nil,
 		},
-		"disabled globally, ns not set, annotation not provided": {
+		"disabled globally, ns not set, annotation not provided, cni disabled": {
+			false,
 			false,
 			nil,
 			"",
@@ -268,7 +273,8 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
   -proxy-uid=5995`,
 			nil,
 		},
-		"disabled globally, ns not set, annotation is false": {
+		"disabled globally, ns not set, annotation is false, cni disabled": {
+			false,
 			false,
 			map[string]string{keyTransparentProxy: "false"},
 			"",
@@ -277,7 +283,8 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
   -proxy-uid=5995`,
 			nil,
 		},
-		"disabled globally, ns not set, annotation is true": {
+		"disabled globally, ns not set, annotation is true, cni disabled": {
+			false,
 			false,
 			map[string]string{keyTransparentProxy: "true"},
 			`/consul/connect-inject/consul connect redirect-traffic \
@@ -286,8 +293,9 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
 			"",
 			nil,
 		},
-		"exclude-inbound-ports, ns is not set, annotation is provided": {
+		"exclude-inbound-ports, ns is not set, annotation is provided, cni disabled": {
 			true,
+			false,
 			map[string]string{
 				keyTransparentProxy:                 "true",
 				annotationTProxyExcludeInboundPorts: "9090,9091",
@@ -300,8 +308,9 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
 			"",
 			nil,
 		},
-		"exclude-outbound-ports, ns is not set, annotation is provided": {
+		"exclude-outbound-ports, ns is not set, annotation is provided, cni disabled": {
 			true,
+			false,
 			map[string]string{
 				keyTransparentProxy:                  "true",
 				annotationTProxyExcludeOutboundPorts: "9090,9091",
@@ -314,8 +323,9 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
 			"",
 			nil,
 		},
-		"exclude-outbound-cidrs annotation is provided": {
+		"exclude-outbound-cidrs annotation is provided, cni disabled": {
 			true,
+			false,
 			map[string]string{
 				keyTransparentProxy:                  "true",
 				annotationTProxyExcludeOutboundCIDRs: "1.1.1.1,2.2.2.2/24",
@@ -328,8 +338,9 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
 			"",
 			nil,
 		},
-		"exclude-uids annotation is provided, ns is not set": {
+		"exclude-uids annotation is provided, ns is not set, cni disabled": {
 			true,
+			false,
 			map[string]string{
 				keyTransparentProxy:         "true",
 				annotationTProxyExcludeUIDs: "6000,7000",
@@ -342,7 +353,8 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
 			"",
 			nil,
 		},
-		"disabled globally, ns enabled, annotation not set": {
+		"disabled globally, ns enabled, annotation not set, cni disabled": {
+			false,
 			false,
 			nil,
 			`/consul/connect-inject/consul connect redirect-traffic \
@@ -351,33 +363,63 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
 			"",
 			map[string]string{keyTransparentProxy: "true"},
 		},
-		"enabled globally, ns disabled, annotation not set": {
+		"enabled globally, ns disabled, annotation not set, cni disabled": {
 			true,
+			false,
 			nil,
 			"",
 			`/consul/connect-inject/consul connect redirect-traffic \
   -proxy-id="$(cat /consul/connect-inject/proxyid)" \
   -proxy-uid=5995`,
 			map[string]string{keyTransparentProxy: "false"},
 		},
+		"disabled globally, ns enabled, annotation not set, cni enabled": {
+			false,
+			true,
+			nil,
+			"",
+			`/consul/connect-inject/consul connect redirect-traffic \
+  -proxy-id="$(cat /consul/connect-inject/proxyid)" \
+  -proxy-uid=5995`,
+			map[string]string{keyTransparentProxy: "true"},
+		},
+
+		"enabled globally, ns not set, annotation not set, cni enabled": {
+			true,
+			true,
+			nil,
+			"",
+			`/consul/connect-inject/consul connect redirect-traffic \
+  -proxy-id="$(cat /consul/connect-inject/proxyid)" \
+  -proxy-uid=5995`,
+			nil,
+		},
 	}
 	for name, c := range cases {
 		t.Run(name, func(t *testing.T) {
 			w := MeshWebhook{
 				EnableTransparentProxy: c.globalEnabled,
 				ConsulAPITimeout:       5 * time.Second,
+				EnableCNI:              c.cniEnabled,
 			}
 			pod := minimal()
 			pod.Annotations = c.annotations
 
-			expectedSecurityContext := &corev1.SecurityContext{
-				RunAsUser:  pointerToInt64(0),
-				RunAsGroup: pointerToInt64(0),
-				Privileged: pointerToBool(true),
-				Capabilities: &corev1.Capabilities{
+			expectedSecurityContext := &corev1.SecurityContext{}
+			if !c.cniEnabled {
+				expectedSecurityContext.RunAsUser = pointerToInt64(0)
+				expectedSecurityContext.RunAsGroup = pointerToInt64(0)
+				expectedSecurityContext.RunAsNonRoot = pointerToBool(false)
+				expectedSecurityContext.Privileged = pointerToBool(true)
+				expectedSecurityContext.Capabilities = &corev1.Capabilities{
 					Add: []corev1.Capability{netAdminCapability},
-				},
-				RunAsNonRoot: pointerToBool(false),
+				}
+			} else {
+				expectedSecurityContext.RunAsNonRoot = pointerToBool(true)
+				expectedSecurityContext.Privileged = pointerToBool(false)
+				expectedSecurityContext.Capabilities = &corev1.Capabilities{
+					Drop: []corev1.Capability{"ALL"},
+				}
 			}
 			ns := testNS
 			ns.Labels = c.namespaceLabel
@@ -389,7 +431,11 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
 				require.Equal(t, expectedSecurityContext, container.SecurityContext)
 				require.Contains(t, actualCmd, c.expectedContainsCmd)
 			} else {
-				require.Nil(t, container.SecurityContext)
+				if !c.cniEnabled {
+					require.Nil(t, container.SecurityContext)
+				} else {
+					require.Equal(t, expectedSecurityContext, container.SecurityContext)
+				}
 				require.NotContains(t, actualCmd, c.expectedNotContainsCmd)
 			}
 		})
@@ -916,7 +962,8 @@ func TestHandlerContainerInit_Multiport(t *testing.T) {
 					serviceName:  "web-admin",
 				},
 			},
-			[]string{`/bin/sh -ec 
+			[]string{
+				`/bin/sh -ec 
 export CONSUL_HTTP_ADDR="${HOST_IP}:8500"
 export CONSUL_GRPC_ADDR="${HOST_IP}:8502"
 consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD_NAMESPACE} \
@@ -967,7 +1014,8 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD
 					serviceName:  "web-admin",
 				},
 			},
-			[]string{`/bin/sh -ec 
+			[]string{
+				`/bin/sh -ec 
 export CONSUL_HTTP_ADDR="${HOST_IP}:8500"
 export CONSUL_GRPC_ADDR="${HOST_IP}:8502"
 consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD_NAMESPACE} \