tests(injector): increase coverage

This change adds additional tests to increase unit test coverage in pkg/injector. No functional changes. Fixes #3432 Signed-off-by: Jon Huhn <[email protected]>
openservicemesh · Aug 27, 2021 · 12bdb15 · 12bdb15
1 parent 423f36c
commit 12bdb15
Show file tree

Hide file tree

Showing 7 changed files with 514 additions and 13 deletions.
diff --git a/pkg/injector/envoy_config_health_probes_test.go b/pkg/injector/envoy_config_health_probes_test.go
@@ -1,11 +1,15 @@
 package injector
 
 import (
+	"testing"
+
+	xds_cluster "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
+	xds_listener "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
 	"github.com/onsi/ginkgo"
+	"github.com/stretchr/testify/assert"
+	"google.golang.org/protobuf/reflect/protoreflect"
 
 	"github.com/openservicemesh/osm/pkg/injector/test"
-
-	"google.golang.org/protobuf/reflect/protoreflect"
 )
 
 var _ = ginkgo.Describe("Test functions creating Envoy config and rewriting the Pod's health probes to pass through Envoy", func() {
@@ -55,3 +59,104 @@ var _ = ginkgo.Describe("Test functions creating Envoy config and rewriting the
 		test.ThisXdsListenerFunction(fnName, fn)
 	}
 })
+
+func TestGetProbeCluster(t *testing.T) {
+	type probeClusterTest struct {
+		name     string
+		probe    *healthProbe
+		expected *xds_cluster.Cluster
+	}
+
+	t.Run("liveness", func(t *testing.T) {
+		tests := []probeClusterTest{
+			{
+				name: "nil",
+			},
+		}
+		for _, test := range tests {
+			t.Run(test.name, func(t *testing.T) {
+				assert.Equal(t, test.expected, getLivenessCluster(test.probe))
+			})
+		}
+	})
+
+	t.Run("readiness", func(t *testing.T) {
+		tests := []probeClusterTest{
+			{
+				name: "nil",
+			},
+		}
+		for _, test := range tests {
+			t.Run(test.name, func(t *testing.T) {
+				assert.Equal(t, test.expected, getReadinessCluster(test.probe))
+			})
+		}
+	})
+
+	t.Run("startup", func(t *testing.T) {
+		tests := []probeClusterTest{
+			{
+				name: "nil",
+			},
+		}
+		for _, test := range tests {
+			t.Run(test.name, func(t *testing.T) {
+				assert.Equal(t, test.expected, getStartupCluster(test.probe))
+			})
+		}
+	})
+}
+
+func TestGetProbeListener(t *testing.T) {
+	type probeListenerTest struct {
+		name     string
+		probe    *healthProbe
+		expected *xds_listener.Listener
+		err      error
+	}
+
+	t.Run("liveness", func(t *testing.T) {
+		tests := []probeListenerTest{
+			{
+				name: "nil",
+			},
+		}
+		for _, test := range tests {
+			t.Run(test.name, func(t *testing.T) {
+				actual, err := getLivenessListener(test.probe)
+				assert.Equal(t, test.expected, actual)
+				assert.Equal(t, test.err, err)
+			})
+		}
+	})
+
+	t.Run("readiness", func(t *testing.T) {
+		tests := []probeListenerTest{
+			{
+				name: "nil",
+			},
+		}
+		for _, test := range tests {
+			t.Run(test.name, func(t *testing.T) {
+				actual, err := getReadinessListener(test.probe)
+				assert.Equal(t, test.expected, actual)
+				assert.Equal(t, test.err, err)
+			})
+		}
+	})
+
+	t.Run("startup", func(t *testing.T) {
+		tests := []probeListenerTest{
+			{
+				name: "nil",
+			},
+		}
+		for _, test := range tests {
+			t.Run(test.name, func(t *testing.T) {
+				actual, err := getStartupListener(test.probe)
+				assert.Equal(t, test.expected, actual)
+				assert.Equal(t, test.err, err)
+			})
+		}
+	})
+}
diff --git a/pkg/injector/envoy_config_test.go b/pkg/injector/envoy_config_test.go
@@ -172,6 +172,52 @@ var _ = Describe("Test functions creating Envoy bootstrap configuration", func()
 			// Now check the entire struct
 			Expect(*secret).To(Equal(expected))
 		})
+
+		It("Updates bootstrap config for the Envoy proxy if it already exists", func() {
+			name := uuid.New().String()
+			namespace := "a"
+			osmNamespace := "b"
+			meta := metav1.ObjectMeta{
+				Name:      name,
+				Namespace: namespace,
+				Labels: map[string]string{
+					constants.OSMAppNameLabelKey:     constants.OSMAppNameLabelValue,
+					constants.OSMAppInstanceLabelKey: "some-mesh",
+					constants.OSMAppVersionLabelKey:  version.Version,
+				},
+			}
+			existing := &corev1.Secret{
+				ObjectMeta: meta,
+				Data: map[string][]byte{
+					"old": []byte("data"),
+				},
+			}
+			wh := &mutatingWebhook{
+				kubeClient:          fake.NewSimpleClientset(existing),
+				kubeController:      k8s.NewMockController(gomock.NewController(GinkgoT())),
+				nonInjectNamespaces: mapset.NewSet(),
+				meshName:            "some-mesh",
+			}
+
+			secret, err := wh.createEnvoyBootstrapConfig(name, namespace, osmNamespace, cert, probes)
+			Expect(err).ToNot(HaveOccurred())
+
+			expected := corev1.Secret{
+				ObjectMeta: meta,
+				Data: map[string][]byte{
+					envoyBootstrapConfigFile: []byte(getExpectedEnvoyYAML(expectedEnvoyBootstrapConfigFileName)),
+				},
+			}
+
+			// Contains only the "bootstrap.yaml" key
+			Expect(len(secret.Data)).To(Equal(1))
+
+			Expect(secret.Data[envoyBootstrapConfigFile]).To(Equal(expected.Data[envoyBootstrapConfigFile]),
+				fmt.Sprintf("Expected YAML: %s;\nActual YAML: %s\n", expected.Data, secret.Data))
+
+			// Now check the entire struct
+			Expect(*secret).To(Equal(expected))
+		})
 	})
 
 	Context("Test getProbeResources()", func() {

diff --git a/pkg/injector/health_probes_test.go b/pkg/injector/health_probes_test.go
@@ -141,6 +141,25 @@ func TestRewriteProbe(t *testing.T) {
 			newPort  int32
 			expected *healthProbe
 		}{
+			{
+				name:     "nil",
+				probe:    nil,
+				expected: nil,
+			},
+			{
+				name:     "no http or tcp",
+				probe:    &v1.Probe{},
+				expected: nil,
+			},
+			{
+				name:  "getPort() error",
+				probe: makeHTTPProbe("/x/y/z", 0),
+				expected: &healthProbe{
+					path:   "/x/y/z",
+					port:   0,
+					isHTTP: true,
+				},
+			},
 			{
 				name:    "http",
 				probe:   makeHTTPProbe("/x/y/z", 3456),
@@ -185,12 +204,14 @@ func TestRewriteProbe(t *testing.T) {
 				assert.Equal(test.expected, actual)
 
 				// Verify the probe was modified correctly
-				if test.probe.Handler.HTTPGet != nil {
-					assert.Equal(test.probe.Handler.HTTPGet.Port, intstr.FromInt(int(test.newPort)))
-					assert.Equal(test.probe.Handler.HTTPGet.Path, test.newPath)
-				}
-				if test.probe.Handler.TCPSocket != nil {
-					assert.Equal(test.probe.Handler.TCPSocket.Port, intstr.FromInt(int(test.newPort)))
+				if test.probe != nil {
+					if test.probe.Handler.HTTPGet != nil {
+						assert.Equal(test.probe.Handler.HTTPGet.Port, intstr.FromInt(int(test.newPort)))
+						assert.Equal(test.probe.Handler.HTTPGet.Path, test.newPath)
+					}
+					if test.probe.Handler.TCPSocket != nil {
+						assert.Equal(test.probe.Handler.TCPSocket.Port, intstr.FromInt(int(test.newPort)))
+					}
 				}
 			})
 		}

diff --git a/pkg/injector/metrics_test.go b/pkg/injector/metrics_test.go
@@ -54,6 +54,8 @@ func TestIsMetricsEnabled(t *testing.T) {
 	ns, _ = fakeClient.CoreV1().Namespaces().Create(context.TODO(), nsWithInvalidAnnotation, metav1.CreateOptions{})
 	assert.NotNil(ns)
 
+	nsDoesntExist := newNamespace("ns-5", nil)
+
 	// Test different scenarios using table driven testing
 	testCases := []struct {
 		namespace                string
@@ -64,6 +66,7 @@ func TestIsMetricsEnabled(t *testing.T) {
 		{nsWithMetricsDisabled.Name, false, false},
 		{nsWithoutMetricsAnnotation.Name, false, false},
 		{nsWithInvalidAnnotation.Name, false, true},
+		{nsDoesntExist.Name, false, true},
 	}
 
 	mockController := k8s.NewMockController(gomock.NewController(t))
@@ -77,6 +80,7 @@ func TestIsMetricsEnabled(t *testing.T) {
 	mockController.EXPECT().GetNamespace("ns-2").Return(nsWithMetricsDisabled)
 	mockController.EXPECT().GetNamespace("ns-3").Return(nsWithoutMetricsAnnotation)
 	mockController.EXPECT().GetNamespace("ns-4").Return(nsWithInvalidAnnotation)
+	mockController.EXPECT().GetNamespace("ns-5").Return(nil)
 
 	for _, tc := range testCases {
 		t.Run(fmt.Sprintf("Namespace %s", tc.namespace), func(t *testing.T) {

diff --git a/pkg/injector/patch_test.go b/pkg/injector/patch_test.go
@@ -24,8 +24,6 @@ import (
 )
 
 func TestCreatePatch(t *testing.T) {
-	assert := tassert.New(t)
-
 	// Setup all variables and constants needed for the tests
 	proxyUUID := uuid.New()
 	const (
@@ -37,6 +35,7 @@ func TestCreatePatch(t *testing.T) {
 		name            string
 		os              string
 		namespace       *corev1.Namespace
+		dryRun          bool
 		expectedPatches []string
 	}{
 		{
@@ -109,10 +108,22 @@ func TestCreatePatch(t *testing.T) {
 				`"command":["envoy"]`,
 			},
 		},
+		{
+			name: "unix dry run",
+			os:   constants.OSLinux,
+			namespace: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: namespace,
+				},
+			},
+			dryRun: true,
+		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
+			assert := tassert.New(t)
+
 			client := fake.NewSimpleClientset()
 			mockCtrl := gomock.NewController(t)
 			mockConfigurator := configurator.NewMockConfigurator(mockCtrl)
@@ -146,7 +157,11 @@ func TestCreatePatch(t *testing.T) {
 			raw, err := json.Marshal(pod)
 			assert.NoError(err)
 
-			req := &admissionv1.AdmissionRequest{Namespace: namespace, Object: runtime.RawExtension{Raw: raw}}
+			req := &admissionv1.AdmissionRequest{
+				Namespace: namespace,
+				Object:    runtime.RawExtension{Raw: raw},
+				DryRun:    &tc.dryRun,
+			}
 			rawPatches, err := wh.createPatch(&pod, req, proxyUUID)
 
 			assert.NoError(err)
@@ -158,6 +173,47 @@ func TestCreatePatch(t *testing.T) {
 			}
 		})
 	}
+
+	t.Run("error checking if metrics is enabled", func(t *testing.T) {
+		assert := tassert.New(t)
+		client := fake.NewSimpleClientset()
+		mockCtrl := gomock.NewController(t)
+		mockConfigurator := configurator.NewMockConfigurator(mockCtrl)
+		mockNsController := k8s.NewMockController(mockCtrl)
+		mockNsController.EXPECT().GetNamespace("not-" + namespace).Return(nil)
+
+		wh := &mutatingWebhook{
+			kubeClient:          client,
+			kubeController:      mockNsController,
+			certManager:         tresor.NewFakeCertManager(mockConfigurator),
+			configurator:        mockConfigurator,
+			nonInjectNamespaces: mapset.NewSet(),
+		}
+
+		mockConfigurator.EXPECT().GetEnvoyWindowsImage().Return("")
+		mockConfigurator.EXPECT().GetEnvoyImage().Return("")
+
+		mockConfigurator.EXPECT().GetEnvoyLogLevel().Return("")
+		mockConfigurator.EXPECT().GetInitContainerImage().Return("")
+		mockConfigurator.EXPECT().IsPrivilegedInitContainer().Return(false)
+		mockConfigurator.EXPECT().GetOutboundIPRangeExclusionList().Return(nil)
+		mockConfigurator.EXPECT().GetOutboundPortExclusionList().Return(nil)
+		mockConfigurator.EXPECT().GetInboundPortExclusionList().Return(nil)
+		mockConfigurator.EXPECT().GetProxyResources().Return(corev1.ResourceRequirements{})
+		mockConfigurator.EXPECT().GetCertKeyBitSize().Return(2048)
+
+		pod := tests.NewOsSpecificPodFixture(namespace, podName, tests.BookstoreServiceAccountName, nil, constants.OSLinux)
+
+		raw, err := json.Marshal(pod)
+		assert.NoError(err)
+
+		req := &admissionv1.AdmissionRequest{
+			Namespace: "not-" + namespace,
+			Object:    runtime.RawExtension{Raw: raw},
+		}
+		_, err = wh.createPatch(&pod, req, proxyUUID)
+		assert.Error(err)
+	})
 }
 
 func TestMergePortExclusionLists(t *testing.T) {

diff --git a/pkg/injector/webhook.go b/pkg/injector/webhook.go
@@ -171,7 +171,11 @@ func (wh *mutatingWebhook) getAdmissionReqResp(proxyUUID uuid.UUID, admissionReq
 	admissionResp.TypeMeta = admissionReq.TypeMeta
 	admissionResp.Kind = admissionReq.Kind
 
-	return admissionReq.Request.Namespace, admissionResp
+	if admissionReq.Request != nil {
+		requestForNamespace = admissionReq.Request.Namespace
+	}
+
+	return
 }
 
 // podCreationHandler is a MutatingWebhookConfiguration handler exclusive to POD CREATE events.
@@ -301,7 +305,7 @@ func (wh *mutatingWebhook) mustInject(pod *corev1.Pod, namespace string) (bool,
 	ns := wh.kubeController.GetNamespace(namespace)
 	if ns == nil {
 		log.Error().Err(errNamespaceNotFound).Msgf("Error retrieving namespace %s", namespace)
-		return false, err
+		return false, errNamespaceNotFound
 	}
 	nsInjectAnnotationExists, nsInject, err := isAnnotatedForInjection(ns.Annotations, "Namespace", ns.Name)
 	if err != nil {