Refactor handler webhook to be of type admission.Webhook

- Replace pointer references with values refernces in methods used by connect-inject for Pods.
hashicorp · Mar 23, 2021 · c2c32fb · c2c32fb
1 parent 715854a
commit c2c32fb
Show file tree

Hide file tree

Showing 19 changed files with 666 additions and 1,079 deletions.
diff --git a/connect-inject/consul_sidecar.go b/connect-inject/consul_sidecar.go
@@ -7,7 +7,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 )
 
-func (h *Handler) consulSidecar(pod *corev1.Pod) (corev1.Container, error) {
+func (h *Handler) consulSidecar(pod corev1.Pod) (corev1.Container, error) {
 	command := []string{
 		"consul-k8s",
 		"consul-sidecar",

diff --git a/connect-inject/consul_sidecar_test.go b/connect-inject/consul_sidecar_test.go
@@ -34,7 +34,7 @@ func TestConsulSidecar_Default(t *testing.T) {
 		ImageConsulK8S:         "hashicorp/consul-k8s:9.9.9",
 		ConsulSidecarResources: consulSidecarResources,
 	}
-	container, err := handler.consulSidecar(&corev1.Pod{
+	container, err := handler.consulSidecar(corev1.Pod{
 		Spec: corev1.PodSpec{
 			Containers: []corev1.Container{
 				{
@@ -84,7 +84,7 @@ func TestConsulSidecar_AuthMethod(t *testing.T) {
 				AuthMethod:     authMethod,
 				ImageConsulK8S: "hashicorp/consul-k8s:9.9.9",
 			}
-			container, err := handler.consulSidecar(&corev1.Pod{
+			container, err := handler.consulSidecar(corev1.Pod{
 				Spec: corev1.PodSpec{
 					Containers: []corev1.Container{
 						{
@@ -113,7 +113,7 @@ func TestConsulSidecar_SyncPeriodAnnotation(t *testing.T) {
 		Log:            hclog.Default().Named("handler"),
 		ImageConsulK8S: "hashicorp/consul-k8s:9.9.9",
 	}
-	container, err := handler.consulSidecar(&corev1.Pod{
+	container, err := handler.consulSidecar(corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			Annotations: map[string]string{
 				"consul.hashicorp.com/connect-sync-period": "55s",
@@ -141,7 +141,7 @@ func TestConsulSidecar_TLS(t *testing.T) {
 		ConsulCACert:           "consul-ca-cert",
 		ConsulSidecarResources: consulSidecarResources,
 	}
-	container, err := handler.consulSidecar(&corev1.Pod{
+	container, err := handler.consulSidecar(corev1.Pod{
 		Spec: corev1.PodSpec{
 			Containers: []corev1.Container{
 				{
@@ -194,7 +194,7 @@ func TestConsulSidecar_MetricsFlags(t *testing.T) {
 		DefaultEnableMetrics:        true,
 		DefaultEnableMetricsMerging: true,
 	}
-	container, err := handler.consulSidecar(&corev1.Pod{
+	container, err := handler.consulSidecar(corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			Annotations: map[string]string{
 				annotationMergedMetricsPort:  "20100",

diff --git a/connect-inject/container_env.go b/connect-inject/container_env.go
@@ -8,7 +8,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 )
 
-func (h *Handler) containerEnvVars(pod *corev1.Pod) []corev1.EnvVar {
+func (h *Handler) containerEnvVars(pod corev1.Pod) []corev1.EnvVar {
 	raw, ok := pod.Annotations[annotationUpstreams]
 	if !ok || raw == "" {
 		return []corev1.EnvVar{}

diff --git a/connect-inject/container_env_test.go b/connect-inject/container_env_test.go
@@ -29,7 +29,7 @@ func TestContainerEnvVars(t *testing.T) {
 			require := require.New(t)
 
 			var h Handler
-			envVars := h.containerEnvVars(&corev1.Pod{
+			envVars := h.containerEnvVars(corev1.Pod{
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: map[string]string{
 						annotationService:   "foo",

diff --git a/connect-inject/container_init.go b/connect-inject/container_init.go
@@ -85,7 +85,7 @@ func (h *Handler) containerInitCopyContainer() corev1.Container {
 
 // containerInit returns the init container spec for registering the Consul
 // service, setting up the Envoy bootstrap, etc.
-func (h *Handler) containerInit(pod *corev1.Pod, k8sNamespace string) (corev1.Container, error) {
+func (h *Handler) containerInit(pod corev1.Pod, k8sNamespace string) (corev1.Container, error) {
 	data := initContainerCommandData{
 		ServiceName:               pod.Annotations[annotationService],
 		ProxyServiceName:          fmt.Sprintf("%s-sidecar-proxy", pod.Annotations[annotationService]),

diff --git a/connect-inject/container_init_test.go b/connect-inject/container_init_test.go
@@ -672,7 +672,7 @@ services {
 			h := Handler{
 				ConsulClient: consulClient,
 			}
-			container, err := h.containerInit(tt.Pod(minimal()), k8sNamespace)
+			container, err := h.containerInit(*tt.Pod(minimal()), k8sNamespace)
 			require.NoError(err)
 			actual := strings.Join(container.Command, " ")
 			require.Contains(actual, tt.Cmd)
@@ -1103,7 +1103,7 @@ EOF
 			require.True(written)
 			h.ConsulClient = consulClient
 
-			container, err := h.containerInit(tt.Pod(minimal()), k8sNamespace)
+			container, err := h.containerInit(*tt.Pod(minimal()), k8sNamespace)
 			require.NoError(err)
 			actual := strings.Join(container.Command, " ")
 			require.Contains(actual, tt.Cmd)
@@ -1142,7 +1142,7 @@ func TestHandlerContainerInit_authMethod(t *testing.T) {
 			ServiceAccountName: "foo",
 		},
 	}
-	container, err := h.containerInit(pod, k8sNamespace)
+	container, err := h.containerInit(*pod, k8sNamespace)
 	require.NoError(err)
 	actual := strings.Join(container.Command, " ")
 	require.Contains(actual, `
@@ -1183,7 +1183,7 @@ func TestHandlerContainerInit_WithTLS(t *testing.T) {
 			},
 		},
 	}
-	container, err := h.containerInit(pod, k8sNamespace)
+	container, err := h.containerInit(*pod, k8sNamespace)
 	require.NoError(err)
 	actual := strings.Join(container.Command, " ")
 	require.Contains(actual, `
@@ -1227,7 +1227,7 @@ func TestHandlerContainerInit_Resources(t *testing.T) {
 			},
 		},
 	}
-	container, err := h.containerInit(pod, k8sNamespace)
+	container, err := h.containerInit(*pod, k8sNamespace)
 	require.NoError(err)
 	require.Equal(corev1.ResourceRequirements{
 		Limits: corev1.ResourceList{
@@ -1262,7 +1262,7 @@ func TestHandlerContainerInit_MismatchedServiceNameServiceAccountNameWithACLsEna
 		},
 	}
 
-	_, err := h.containerInit(pod, k8sNamespace)
+	_, err := h.containerInit(*pod, k8sNamespace)
 	require.EqualError(err, `serviceAccountName "notServiceName" does not match service name "foo"`)
 }
 
@@ -1285,7 +1285,7 @@ func TestHandlerContainerInit_MismatchedServiceNameServiceAccountNameWithACLsDis
 		},
 	}
 
-	_, err := h.containerInit(pod, k8sNamespace)
+	_, err := h.containerInit(*pod, k8sNamespace)
 	require.NoError(err)
 }
 
@@ -1410,7 +1410,7 @@ func TestHandlerContainerInit_MeshGatewayModeErrors(t *testing.T) {
 					},
 				},
 			}
-			_, err = h.containerInit(pod, k8sNamespace)
+			_, err = h.containerInit(*pod, k8sNamespace)
 			if c.ExpError == "" {
 				require.NoError(err)
 			} else {

diff --git a/connect-inject/endpoints_controller.go b/connect-inject/endpoints_controller.go
@@ -146,7 +146,7 @@ func (r *EndpointsController) createServiceRegistrations(pod corev1.Pod, service
 	// and register that port for the host service.
 	var servicePort int
 	if raw, ok := pod.Annotations[annotationPort]; ok && raw != "" {
-		if port, _ := portValue(&pod, raw); port > 0 {
+		if port, _ := portValue(pod, raw); port > 0 {
 			servicePort = int(port)
 		}
 	}
@@ -208,7 +208,7 @@ func (r *EndpointsController) createServiceRegistrations(pod corev1.Pod, service
 		proxyConfig.LocalServicePort = servicePort
 	}
 
-	upstreams, err := r.processUpstreams(&pod)
+	upstreams, err := r.processUpstreams(pod)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -316,7 +316,7 @@ func (r *EndpointsController) deregisterServiceOnAllAgents(k8sSvcName, k8sSvcNam
 
 // processUpstreams reads the list of upstreams from the Pod annotation and converts them into a list of api.Upstream
 // objects.
-func (r *EndpointsController) processUpstreams(pod *corev1.Pod) ([]api.Upstream, error) {
+func (r *EndpointsController) processUpstreams(pod corev1.Pod) ([]api.Upstream, error) {
 	var upstreams []api.Upstream
 	if raw, ok := pod.Annotations[annotationUpstreams]; ok && raw != "" {
 		for _, raw := range strings.Split(raw, ",") {

diff --git a/connect-inject/endpoints_controller_test.go b/connect-inject/endpoints_controller_test.go
@@ -226,7 +226,7 @@ func TestProcessUpstreams(t *testing.T) {
 				DenyK8sNamespacesSet:  mapset.NewSetWith(),
 			}
 
-			upstreams, err := ep.processUpstreams(tt.pod())
+			upstreams, err := ep.processUpstreams(*tt.pod())
 			if tt.expErr != "" {
 				require.EqualError(t, err, tt.expErr)
 			} else {

diff --git a/connect-inject/envoy_sidecar.go b/connect-inject/envoy_sidecar.go
@@ -17,7 +17,7 @@ type sidecarContainerCommandData struct {
 	ConsulNamespace string
 }
 
-func (h *Handler) envoySidecar(pod *corev1.Pod, k8sNamespace string) (corev1.Container, error) {
+func (h *Handler) envoySidecar(pod corev1.Pod, k8sNamespace string) (corev1.Container, error) {
 	templateData := sidecarContainerCommandData{
 		AuthMethod:      h.AuthMethod,
 		ConsulNamespace: h.consulNamespace(k8sNamespace),
@@ -91,7 +91,7 @@ func (h *Handler) envoySidecar(pod *corev1.Pod, k8sNamespace string) (corev1.Con
 	}
 	return container, nil
 }
-func (h *Handler) getContainerSidecarCommand(pod *corev1.Pod) ([]string, error) {
+func (h *Handler) getContainerSidecarCommand(pod corev1.Pod) ([]string, error) {
 	cmd := []string{
 		"envoy",
 		"--config-path", "/consul/connect-inject/envoy-bootstrap.yaml",
@@ -124,7 +124,7 @@ func (h *Handler) getContainerSidecarCommand(pod *corev1.Pod) ([]string, error)
 	return cmd, nil
 }
 
-func (h *Handler) envoySidecarResources(pod *corev1.Pod) (corev1.ResourceRequirements, error) {
+func (h *Handler) envoySidecarResources(pod corev1.Pod) (corev1.ResourceRequirements, error) {
 	resources := corev1.ResourceRequirements{
 		Limits:   corev1.ResourceList{},
 		Requests: corev1.ResourceList{},

diff --git a/connect-inject/envoy_sidecar_test.go b/connect-inject/envoy_sidecar_test.go
@@ -13,7 +13,7 @@ import (
 func TestHandlerEnvoySidecar(t *testing.T) {
 	require := require.New(t)
 	h := Handler{}
-	pod := &corev1.Pod{
+	pod := corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			Annotations: map[string]string{
 				annotationService: "foo",
@@ -144,7 +144,7 @@ func TestHandlerEnvoySidecar_EnvoyExtraArgs(t *testing.T) {
 				EnvoyExtraArgs: tc.envoyExtraArgs,
 			}
 
-			c, err := h.envoySidecar(tc.pod, k8sNamespace)
+			c, err := h.envoySidecar(*tc.pod, k8sNamespace)
 			require.NoError(t, err)
 			require.Equal(t, tc.expectedContainerCommand, c.Command)
 		})
@@ -158,7 +158,7 @@ func TestHandlerEnvoySidecar_AuthMethod(t *testing.T) {
 	h := Handler{
 		AuthMethod: "test-auth-method",
 	}
-	pod := &corev1.Pod{
+	pod := corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			Annotations: map[string]string{
 				annotationService: "foo",
@@ -192,7 +192,7 @@ func TestHandlerEnvoySidecar_WithTLS(t *testing.T) {
 	h := Handler{
 		ConsulCACert: "consul-ca-cert",
 	}
-	pod := &corev1.Pod{
+	pod := corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			Annotations: map[string]string{
 				annotationService: "foo",
@@ -237,7 +237,7 @@ func TestHandlerEnvoySidecar_Namespaces(t *testing.T) {
 		EnableNamespaces:           true,
 		ConsulDestinationNamespace: k8sNamespace,
 	}
-	pod := &corev1.Pod{
+	pod := corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			Annotations: map[string]string{
 				annotationService: "foo",
@@ -268,7 +268,7 @@ func TestHandlerEnvoySidecar_NamespacesAndAuthMethod(t *testing.T) {
 		ConsulDestinationNamespace: k8sNamespace,
 		AuthMethod:                 "test-auth-method",
 	}
-	pod := &corev1.Pod{
+	pod := corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			Annotations: map[string]string{
 				annotationService: "foo",
@@ -449,7 +449,7 @@ func TestHandlerEnvoySidecar_Resources(t *testing.T) {
 	for name, c := range cases {
 		t.Run(name, func(tt *testing.T) {
 			require := require.New(tt)
-			pod := &corev1.Pod{
+			pod := corev1.Pod{
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: c.annotations,
 				},