Respect allow/deny lists even when namespaces aren't enabled

Fixes #296
hashicorp · Jul 21, 2020 · 34896cb · 34896cb
1 parent 9bbf6df
commit 34896cb
Show file tree

Hide file tree

Showing 3 changed files with 177 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 ## UNRELEASED
 
+BUG FIXES:
+
+* Connect: Respect allow/deny list flags when namespaces are disabled. [[GH-296](https://github.com/hashicorp/consul-k8s/issues/296)]
+
 ## 0.17.0 (July 09, 2020)
 
 BREAKING CHANGES:

diff --git a/connect-inject/handler.go b/connect-inject/handler.go
@@ -382,16 +382,14 @@ func (h *Handler) shouldInject(pod *corev1.Pod, namespace string) (bool, error)
 	}
 
 	// Namespace logic
-	if h.EnableNamespaces {
-		// If in deny list, don't inject
-		if h.DenyK8sNamespacesSet.Contains(namespace) {
-			return false, nil
-		}
+	// If in deny list, don't inject
+	if h.DenyK8sNamespacesSet.Contains(namespace) {
+		return false, nil
+	}
 
-		// If not in allow list or allow list is not *, don't inject
-		if !h.AllowK8sNamespacesSet.Contains("*") && !h.AllowK8sNamespacesSet.Contains(namespace) {
-			return false, nil
-		}
+	// If not in allow list or allow list is not *, don't inject
+	if !h.AllowK8sNamespacesSet.Contains("*") && !h.AllowK8sNamespacesSet.Contains(namespace) {
+		return false, nil
 	}
 
 	// If we already injected then don't inject again

diff --git a/connect-inject/handler_test.go b/connect-inject/handler_test.go
@@ -34,7 +34,11 @@ func TestHandlerHandle(t *testing.T) {
 	}{
 		{
 			"kube-system namespace",
-			Handler{Log: hclog.Default().Named("handler")},
+			Handler{
+				Log:                   hclog.Default().Named("handler"),
+				AllowK8sNamespacesSet: mapset.NewSetWith("*"),
+				DenyK8sNamespacesSet:  mapset.NewSet(),
+			},
 			v1beta1.AdmissionRequest{
 				Namespace: metav1.NamespaceSystem,
 				Object: encodeRaw(t, &corev1.Pod{
@@ -47,7 +51,11 @@ func TestHandlerHandle(t *testing.T) {
 
 		{
 			"already injected",
-			Handler{Log: hclog.Default().Named("handler")},
+			Handler{
+				Log:                   hclog.Default().Named("handler"),
+				AllowK8sNamespacesSet: mapset.NewSetWith("*"),
+				DenyK8sNamespacesSet:  mapset.NewSet(),
+			},
 			v1beta1.AdmissionRequest{
 				Object: encodeRaw(t, &corev1.Pod{
 					ObjectMeta: metav1.ObjectMeta{
@@ -65,7 +73,11 @@ func TestHandlerHandle(t *testing.T) {
 
 		{
 			"empty pod basic",
-			Handler{Log: hclog.Default().Named("handler")},
+			Handler{
+				Log:                   hclog.Default().Named("handler"),
+				AllowK8sNamespacesSet: mapset.NewSetWith("*"),
+				DenyK8sNamespacesSet:  mapset.NewSet(),
+			},
 			v1beta1.AdmissionRequest{
 				Object: encodeRaw(t, &corev1.Pod{
 					Spec: basicSpec,
@@ -102,7 +114,11 @@ func TestHandlerHandle(t *testing.T) {
 
 		{
 			"pod with upstreams specified",
-			Handler{Log: hclog.Default().Named("handler")},
+			Handler{
+				Log:                   hclog.Default().Named("handler"),
+				AllowK8sNamespacesSet: mapset.NewSetWith("*"),
+				DenyK8sNamespacesSet:  mapset.NewSet(),
+			},
 			v1beta1.AdmissionRequest{
 				Object: encodeRaw(t, &corev1.Pod{
 					ObjectMeta: metav1.ObjectMeta{
@@ -161,7 +177,11 @@ func TestHandlerHandle(t *testing.T) {
 
 		{
 			"empty pod with injection disabled",
-			Handler{Log: hclog.Default().Named("handler")},
+			Handler{
+				Log:                   hclog.Default().Named("handler"),
+				AllowK8sNamespacesSet: mapset.NewSetWith("*"),
+				DenyK8sNamespacesSet:  mapset.NewSet(),
+			},
 			v1beta1.AdmissionRequest{
 				Object: encodeRaw(t, &corev1.Pod{
 					ObjectMeta: metav1.ObjectMeta{
@@ -179,7 +199,11 @@ func TestHandlerHandle(t *testing.T) {
 
 		{
 			"empty pod with injection truthy",
-			Handler{Log: hclog.Default().Named("handler")},
+			Handler{
+				Log:                   hclog.Default().Named("handler"),
+				AllowK8sNamespacesSet: mapset.NewSetWith("*"),
+				DenyK8sNamespacesSet:  mapset.NewSet(),
+			},
 			v1beta1.AdmissionRequest{
 				Object: encodeRaw(t, &corev1.Pod{
 					ObjectMeta: metav1.ObjectMeta{
@@ -222,7 +246,13 @@ func TestHandlerHandle(t *testing.T) {
 
 		{
 			"empty pod basic, no default protocol",
-			Handler{WriteServiceDefaults: true, DefaultProtocol: "", Log: hclog.Default().Named("handler")},
+			Handler{
+				WriteServiceDefaults:  true,
+				DefaultProtocol:       "",
+				Log:                   hclog.Default().Named("handler"),
+				AllowK8sNamespacesSet: mapset.NewSetWith("*"),
+				DenyK8sNamespacesSet:  mapset.NewSet(),
+			},
 			v1beta1.AdmissionRequest{
 				Object: encodeRaw(t, &corev1.Pod{
 					Spec: basicSpec,
@@ -260,7 +290,12 @@ func TestHandlerHandle(t *testing.T) {
 
 		{
 			"empty pod basic, protocol in annotation",
-			Handler{WriteServiceDefaults: true, Log: hclog.Default().Named("handler")},
+			Handler{
+				WriteServiceDefaults:  true,
+				Log:                   hclog.Default().Named("handler"),
+				AllowK8sNamespacesSet: mapset.NewSetWith("*"),
+				DenyK8sNamespacesSet:  mapset.NewSet(),
+			},
 			v1beta1.AdmissionRequest{
 				Object: encodeRaw(t, &corev1.Pod{
 					Spec: basicSpec,
@@ -299,7 +334,13 @@ func TestHandlerHandle(t *testing.T) {
 
 		{
 			"empty pod basic, default protocol specified",
-			Handler{WriteServiceDefaults: true, DefaultProtocol: "http", Log: hclog.Default().Named("handler")},
+			Handler{
+				WriteServiceDefaults:  true,
+				DefaultProtocol:       "http",
+				Log:                   hclog.Default().Named("handler"),
+				AllowK8sNamespacesSet: mapset.NewSetWith("*"),
+				DenyK8sNamespacesSet:  mapset.NewSet(),
+			},
 			v1beta1.AdmissionRequest{
 				Object: encodeRaw(t, &corev1.Pod{
 					Spec: basicSpec,
@@ -369,7 +410,11 @@ func TestHandlerHandle_badContentType(t *testing.T) {
 	require.NoError(t, err)
 	req.Header.Set("Content-Type", "text/plain")
 
-	h := Handler{Log: hclog.Default().Named("handler")}
+	h := Handler{
+		Log:                   hclog.Default().Named("handler"),
+		AllowK8sNamespacesSet: mapset.NewSetWith("*"),
+		DenyK8sNamespacesSet:  mapset.NewSet(),
+	}
 	rec := httptest.NewRecorder()
 	h.Handle(rec, req)
 	require.Equal(t, http.StatusBadRequest, rec.Code)
@@ -382,7 +427,11 @@ func TestHandlerHandle_noBody(t *testing.T) {
 	require.NoError(t, err)
 	req.Header.Set("Content-Type", "application/json")
 
-	h := Handler{Log: hclog.Default().Named("handler")}
+	h := Handler{
+		Log:                   hclog.Default().Named("handler"),
+		AllowK8sNamespacesSet: mapset.NewSetWith("*"),
+		DenyK8sNamespacesSet:  mapset.NewSet(),
+	}
 	rec := httptest.NewRecorder()
 	h.Handle(rec, req)
 	require.Equal(t, http.StatusBadRequest, rec.Code)
@@ -789,7 +838,7 @@ func TestShouldInject(t *testing.T) {
 			false,
 		},
 		{
-			"namespaces disabled",
+			"namespaces disabled, empty allow/deny lists",
 			&corev1.Pod{
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: map[string]string{
@@ -801,8 +850,113 @@ func TestShouldInject(t *testing.T) {
 			false,
 			mapset.NewSet(),
 			mapset.NewSet(),
+			false,
+		},
+		{
+			"namespaces disabled, allow *",
+			&corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						annotationService: "testing",
+					},
+				},
+			},
+			"default",
+			false,
+			mapset.NewSetWith("*"),
+			mapset.NewSet(),
 			true,
 		},
+		{
+			"namespaces disabled, allow default",
+			&corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						annotationService: "testing",
+					},
+				},
+			},
+			"default",
+			false,
+			mapset.NewSetWith("default"),
+			mapset.NewSet(),
+			true,
+		},
+		{
+			"namespaces disabled, allow * and default",
+			&corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						annotationService: "testing",
+					},
+				},
+			},
+			"default",
+			false,
+			mapset.NewSetWith("*", "default"),
+			mapset.NewSet(),
+			true,
+		},
+		{
+			"namespaces disabled, allow only ns1 and ns2",
+			&corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						annotationService: "testing",
+					},
+				},
+			},
+			"default",
+			false,
+			mapset.NewSetWith("ns1", "ns2"),
+			mapset.NewSet(),
+			false,
+		},
+		{
+			"namespaces disabled, deny default ns",
+			&corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						annotationService: "testing",
+					},
+				},
+			},
+			"default",
+			false,
+			mapset.NewSet(),
+			mapset.NewSetWith("default"),
+			false,
+		},
+		{
+			"namespaces disabled, allow *, deny default ns",
+			&corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						annotationService: "testing",
+					},
+				},
+			},
+			"default",
+			false,
+			mapset.NewSetWith("*"),
+			mapset.NewSetWith("default"),
+			false,
+		},
+		{
+			"namespaces disabled, default ns in both allow and deny lists",
+			&corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						annotationService: "testing",
+					},
+				},
+			},
+			"default",
+			false,
+			mapset.NewSetWith("default"),
+			mapset.NewSetWith("default"),
+			false,
+		},
 		{
 			"namespaces enabled, empty allow/deny lists",
 			&corev1.Pod{