main_test: remove config specfic tests and check if rules is not empt…

…y when a valid annotation is used

Add redirect traffic config and apply it in the plugin

control-plane/build-support/functions/20-build.sh

@@ -255,14 +255,14 @@ function build_consul_local {
          return 1
       fi
    else
-      status "Building sequentially with go install"
+      status "Building sequentially with go build"
       for os in ${build_os}
       do
          for arch in ${build_arch}
          do
             outdir="pkg.bin.new/${extra_dir}${os}_${arch}"
             osarch="${os}/${arch}"
-            if test "${osarch}" == "darwin/arm" -o "${osarch}" == "darwin/arm64" -o "${osarch}" == "freebsd/arm64" -o "${osarch}" == "windows/arm" -o "${osarch}" == "windows/arm64"
+            if test "${osarch}" == "darwin/arm" -o "${osarch}" == "freebsd/arm64" -o "${osarch}" == "windows/arm" -o "${osarch}" == "windows/arm64"
             then
                continue
             fi

control-plane/cni/go.sum

@@ -219,6 +219,8 @@ github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81P
 github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
 github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.7.1 h1:5TQK59W5E3v0r2duFAb7P95B6hEeOyEnHRa8MjYSMTY=
+github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU=

control-plane/cni/main.go

@@ -6,9 +6,7 @@ import (
 	"fmt"
 	"net"
 	"path/filepath"
-	"time"
 
-	"github.com/cenkalti/backoff"
 	"github.com/containernetworking/cni/pkg/skel"
 	"github.com/containernetworking/cni/pkg/types"
 	current "github.com/containernetworking/cni/pkg/types/100"
@@ -44,13 +42,16 @@ const (
 	// indicate the status of the CNI plugin.
 	complete = "complete"
 
-	// annotationTrafficRedirection stores iptables.Config information so that the CNI plugin can use it to apply
+	// annotationRedirectTraffic stores iptables.Config information so that the CNI plugin can use it to apply
 	// iptables rules.
-	annotationTrafficRedirection = "consul.hashicorp.com/traffic-redirection-config"
+	annotationRedirectTraffic = "consul.hashicorp.com/redirect-traffic-config"
 )
 
 type Command struct {
+	// client is a kubernetes client
 	client kubernetes.Interface
+	// iptablesProvider is the Provider that will apply iptables rules. Used for testing.
+	iptablesProvider iptables.Provider
 }
 
 type CNIArgs struct {
@@ -157,7 +158,6 @@ func (c *Command) cmdAdd(args *skel.CmdArgs) error {
 	result := prevResult
 	logger.Debug("consul-cni previous result", "result", result)
 
-	// Connect to kubernetes.
 	ctx := context.Background()
 	if c.client == nil {
 
@@ -180,34 +180,57 @@ func (c *Command) cmdAdd(args *skel.CmdArgs) error {
 
 	// Skip traffic redirection if the correct annotations are not on the pod.
 	if skipTrafficRedirection(*pod) {
-		logger.Debug("skipping traffic redirect on un-injected pod: %s", pod.Name)
+		logger.Debug("skipping traffic redirection because the pod is either not injected or transparent proxy is disabled: %s", pod.Name)
 		return types.PrintResult(result, cfg.CNIVersion)
 	}
 
-	err = c.updateTransparentProxyStatusAnnotation(pod, podNamespace, waiting)
+	// 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)
+	if !ok {
+		logger.Info("unable to update %s pod annotation to waiting", keyTransparentProxyStatus)
+	}
+
+	// Parse the cni-proxy-config annotation into an iptables.Config object.
+	iptablesCfg, err := parseAnnotation(*pod, annotationRedirectTraffic)
 	if err != nil {
-		return fmt.Errorf("error adding waiting annotation: %s", err)
+		return err
 	}
 
-	// TODO: Insert redirect here
+	// Set NetNS passed through the CNI.
+	iptablesCfg.NetNS = args.Netns
+
+	// Set the provider to a fake provider in testing, otherwise use the default iptables.Provider
+	if c.iptablesProvider != nil {
+		iptablesCfg.IptablesProvider = c.iptablesProvider
+	}
 
-	err = c.updateTransparentProxyStatusAnnotation(pod, podNamespace, complete)
+	// Apply the iptables rules.
+	err = iptables.Setup(iptablesCfg)
 	if err != nil {
-		return fmt.Errorf("error adding complete annotation: %s", err)
+		return fmt.Errorf("could not apply iptables setup: %v", err)
 	}
 
+	// 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)
+	if !ok {
+		logger.Info("unable to update %s pod annotation to complete", keyTransparentProxyStatus)
+	}
+
+	logger.Debug("traffic redirect rules applied to pod: %s", pod.Name)
 	// Pass through the result for the next plugin even though we are the final plugin in the chain.
 	return types.PrintResult(result, cfg.CNIVersion)
 }
 
 // cmdDel is called for DELETE requests.
-func cmdDel(args *skel.CmdArgs) error {
+func cmdDel(_ *skel.CmdArgs) error {
 	// Nothing to do but this function will still be called as part of the CNI specification.
 	return nil
 }
 
 // cmdCheck is called for CHECK requests.
-func cmdCheck(args *skel.CmdArgs) error {
+func cmdCheck(_ *skel.CmdArgs) error {
 	// Nothing to do but this function will still be called as part of the CNI specification.
 	return nil
 }
@@ -231,20 +254,6 @@ func skipTrafficRedirection(pod corev1.Pod) bool {
 	return false
 }
 
-// waitForAnnotation waits for an annotation to be available. Returns immediately if the annotation exists.
-func waitForAnnotation(pod corev1.Pod, annotation string, retries uint64) bool {
-	var err error
-	err = backoff.Retry(func() error {
-		var ok bool
-		_, ok = pod.Annotations[annotation]
-		if !ok {
-			return fmt.Errorf("annotation %s does not exist yet", annotation)
-		}
-		return err
-	}, backoff.WithMaxRetries(backoff.NewConstantBackOff(1*time.Second), retries))
-	return err == nil
-}
-
 // parseAnnotation parses the cni-proxy-config annotation into an iptables.Config object.
 func parseAnnotation(pod corev1.Pod, annotation string) (iptables.Config, error) {
 	anno, ok := pod.Annotations[annotation]
@@ -260,13 +269,9 @@ 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.
-func (c *Command) updateTransparentProxyStatusAnnotation(pod *corev1.Pod, namespace, status string) error {
+// CNI status on the pod.  Failing is not fatal.
+func (c *Command) updateTransparentProxyStatusAnnotation(pod *corev1.Pod, namespace, status string) bool {
 	pod.Annotations[keyTransparentProxyStatus] = status
 	_, err := c.client.CoreV1().Pods(namespace).Update(context.Background(), pod, metav1.UpdateOptions{})
-	if err != nil {
-		return fmt.Errorf("error adding annotation to pod: %s", err)
-	}
-
-	return nil
+	return err == nil
 }

control-plane/cni/main_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/containernetworking/cni/pkg/skel"
@@ -19,76 +20,137 @@ const (
 	defaultNamespace = "default"
 )
 
-func TestRun_cmdAdd(t *testing.T) {
-	t.Parallel()
+type fakeIptablesProvider struct {
+	rules []string
+}
 
-	cmd := &Command{
-		client: fake.NewSimpleClientset(),
-	}
+func (f *fakeIptablesProvider) AddRule(name string, args ...string) {
+	var rule []string
+	rule = append(rule, name)
+	rule = append(rule, args...)
+
+	f.rules = append(f.rules, strings.Join(rule, " "))
+}
+
+func (f *fakeIptablesProvider) ApplyRules() error {
+	return nil
+}
+
+func (f *fakeIptablesProvider) Rules() []string {
+	return f.rules
+}
+
+func Test_cmdAdd(t *testing.T) {
+	t.Parallel()
 
 	cases := []struct {
 		name          string
+		cmd           *Command
 		podName       string
 		stdInData     string
-		configuredPod func(*corev1.Pod) *corev1.Pod
+		configuredPod func(*corev1.Pod, *Command) *corev1.Pod
+		expectedRules bool
 		expectedErr   error
 	}{
 		{
 			name:      "K8S_POD_NAME missing from CNI args, should throw error",
+			cmd:       &Command{},
 			podName:   "",
 			stdInData: goodStdinData,
-			configuredPod: func(pod *corev1.Pod) *corev1.Pod {
+			configuredPod: func(pod *corev1.Pod, cmd *Command) *corev1.Pod {
 				return pod
 			},
-			expectedErr: fmt.Errorf("not running in a pod, namespace and pod should have values"),
+			expectedErr:   fmt.Errorf("not running in a pod, namespace and pod should have values"),
+			expectedRules: false, // Rules won't be applied because the command will throw an error first
 		},
 		{
-			name:      "Missing prevResult in stdin data, should throw error",
+			name: "Missing prevResult in stdin data, should throw error",
+			cmd: &Command{
+				client: fake.NewSimpleClientset(),
+			},
 			podName:   "missing-prev-result",
 			stdInData: missingPrevResultStdinData,
-			configuredPod: func(pod *corev1.Pod) *corev1.Pod {
-				_, err := cmd.client.CoreV1().Pods(defaultNamespace).Create(context.TODO(), pod, metav1.CreateOptions{})
+			configuredPod: func(pod *corev1.Pod, cmd *Command) *corev1.Pod {
+				_, err := cmd.client.CoreV1().Pods(defaultNamespace).Create(context.Background(), pod, metav1.CreateOptions{})
 				require.NoError(t, err)
 
 				return pod
 			},
-			expectedErr: fmt.Errorf("must be called as final chained plugin"),
+			expectedErr:   fmt.Errorf("must be called as final chained plugin"),
+			expectedRules: false, // Rules won't be applied because the command will throw an error first
 		},
 		{
-			name:      "Missing IPs in prevResult in stdin data, should throw error",
+			name: "Missing IPs in prevResult in stdin data, should throw error",
+			cmd: &Command{
+				client: fake.NewSimpleClientset(),
+			},
 			podName:   "corrupt-prev-result",
 			stdInData: missingIPsStdinData,
-			configuredPod: func(pod *corev1.Pod) *corev1.Pod {
-				_, err := cmd.client.CoreV1().Pods(defaultNamespace).Create(context.TODO(), pod, metav1.CreateOptions{})
+			configuredPod: func(pod *corev1.Pod, cmd *Command) *corev1.Pod {
+				_, err := cmd.client.CoreV1().Pods(defaultNamespace).Create(context.Background(), pod, metav1.CreateOptions{})
 				require.NoError(t, err)
 
 				return pod
 			},
-			expectedErr: fmt.Errorf("got no container IPs"),
+			expectedErr:   fmt.Errorf("got no container IPs"),
+			expectedRules: false, // Rules won't be applied because the command will throw an error first
 		},
-
 		{
-			name:      "Pod with traffic redirection annotation, should apply redirect",
-			podName:   "pod-with-annotation",
+			name: "Pod with incorrect traffic redirection annotation, should throw error",
+			cmd: &Command{
+				client: fake.NewSimpleClientset(),
+			},
+			podName:   "pod-with-incorrect-annotation",
 			stdInData: goodStdinData,
-			configuredPod: func(pod *corev1.Pod) *corev1.Pod {
-				_, err := cmd.client.CoreV1().Pods(defaultNamespace).Create(context.TODO(), pod, metav1.CreateOptions{})
+			configuredPod: func(pod *corev1.Pod, cmd *Command) *corev1.Pod {
+				pod.Annotations[keyInjectStatus] = "true"
+				pod.Annotations[keyTransparentProxyStatus] = "enabled"
+				pod.Annotations[annotationRedirectTraffic] = "{foo}"
+				_, err := cmd.client.CoreV1().Pods(defaultNamespace).Create(context.Background(), pod, metav1.CreateOptions{})
 				require.NoError(t, err)
 
-				pod.Annotations[annotationTrafficRedirection] = "{foo}"
-				_, err = cmd.client.CoreV1().Pods(defaultNamespace).Update(context.Background(), pod, metav1.UpdateOptions{})
+				return pod
+			},
+			expectedErr:   fmt.Errorf("could not unmarshal %s annotation for %s pod", annotationRedirectTraffic, "pod-with-incorrect-annotation"),
+			expectedRules: false, // Rules won't be applied because the command will throw an error first
+		},
+		{
+			name: "Pod with correct annotations, should create redirect traffic rules",
+			cmd: &Command{
+				client:           fake.NewSimpleClientset(),
+				iptablesProvider: &fakeIptablesProvider{},
+			},
+			podName:   "pod-no-proxy-outbound-port",
+			stdInData: goodStdinData,
+			configuredPod: func(pod *corev1.Pod, cmd *Command) *corev1.Pod {
+				pod.Annotations[keyInjectStatus] = "true"
+				pod.Annotations[keyTransparentProxyStatus] = "enabled"
+				cfg := iptables.Config{
+					ProxyUserID:      "123",
+					ProxyInboundPort: 20000,
+				}
+				iptablesConfigJson, err := json.Marshal(&cfg)
+				require.NoError(t, err)
+				pod.Annotations[annotationRedirectTraffic] = string(iptablesConfigJson)
+				_, err = cmd.client.CoreV1().Pods(defaultNamespace).Create(context.Background(), pod, metav1.CreateOptions{})
 				require.NoError(t, err)
 
 				return pod
 			},
-			expectedErr: nil,
+			expectedErr:   nil,
+			expectedRules: true, // Rules will be applied
 		},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
-			_ = c.configuredPod(minimalPod(c.podName))
-			actual := cmd.cmdAdd(minimalSkelArgs(c.podName, defaultNamespace, c.stdInData))
-			require.Equal(t, c.expectedErr, actual)
+			_ = c.configuredPod(minimalPod(c.podName), c.cmd)
+			err := c.cmd.cmdAdd(minimalSkelArgs(c.podName, defaultNamespace, c.stdInData))
+			require.Equal(t, c.expectedErr, err)
+
+			// Check to see that rules have been generated
+			if c.expectedErr == nil && c.expectedRules {
+				require.NotEmpty(t, c.cmd.iptablesProvider.Rules())
+			}
 		})
 	}
 }
@@ -152,15 +214,15 @@ func TestParseAnnotation(t *testing.T) {
 	}{
 		{
 			name:       "Pod with iptables.Config annotation",
-			annotation: annotationTrafficRedirection,
+			annotation: annotationRedirectTraffic,
 			configurePod: func(pod *corev1.Pod) *corev1.Pod {
 				// Use iptables.Config so that if the Config struct ever changes that the test is still valid
 				cfg := iptables.Config{ProxyUserID: "1234"}
 				j, err := json.Marshal(&cfg)
 				if err != nil {
 					t.Fatalf("could not marshal iptables config: %v", err)
 				}
-				pod.Annotations[annotationTrafficRedirection] = string(j)
+				pod.Annotations[annotationRedirectTraffic] = string(j)
 				return pod
 			},
 			expected: iptables.Config{
@@ -170,12 +232,12 @@ func TestParseAnnotation(t *testing.T) {
 		},
 		{
 			name:       "Pod without iptables.Config annotation",
-			annotation: annotationTrafficRedirection,
+			annotation: annotationRedirectTraffic,
 			configurePod: func(pod *corev1.Pod) *corev1.Pod {
 				return pod
 			},
 			expected: iptables.Config{},
-			err:      fmt.Errorf("could not find %s annotation for %s pod", annotationTrafficRedirection, defaultPodName),
+			err:      fmt.Errorf("could not find %s annotation for %s pod", annotationRedirectTraffic, defaultPodName),
 		},
 	}
 	for _, c := range cases {

control-plane/connect-inject/annotations.go

@@ -152,6 +152,10 @@ const (
 	// to point to the Envoy proxy when running in Transparent Proxy mode.
 	annotationTransparentProxyOverwriteProbes = "consul.hashicorp.com/transparent-proxy-overwrite-probes"
 
+	// annotationRedirectTraffic stores iptables.Config information so that the CNI plugin can use it to apply
+	// iptables rules.
+	annotationRedirectTraffic = "consul.hashicorp.com/redirect-traffic-config"
+
 	// annotationOriginalPod is the value of the pod before being overwritten by the consul
 	// webhook/meshWebhook.
 	annotationOriginalPod = "consul.hashicorp.com/original-pod"