From 4391bfa694ea71352f239152d2d74fe241be637f Mon Sep 17 00:00:00 2001
From: Rebecca Zanzig <rzazzl@hotmail.com>
Date: Thu, 10 Jan 2019 13:44:24 -0800
Subject: [PATCH] Create patches for the default annotations that are being
 added

Fixes 20.

Previously, the default annotations were added to the local Pod object, but
no patches were created to update the running pod. This meant we were bypassing
the check for empty annotations, causing errors when trying to add an annotation
to a nonexistent object.
---
 connect-inject/handler.go      | 29 ++++++++++++++++++++++++-----
 connect-inject/handler_test.go | 15 ++++++++++++++-
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/connect-inject/handler.go b/connect-inject/handler.go
index 2520184cc5..01454ffe3e 100644
--- a/connect-inject/handler.go
+++ b/connect-inject/handler.go
@@ -138,9 +138,12 @@ func (h *Handler) Mutate(req *v1beta1.AdmissionRequest) *v1beta1.AdmissionRespon
 		UID:     req.UID,
 	}
 
+	// Accumulate any patches here
+	var patches []jsonpatch.JsonPatchOperation
+
 	// Setup the default annotation values that are used for the container.
 	// This MUST be done before shouldInject is called since k.
-	if err := h.defaultAnnotations(&pod); err != nil {
+	if err := h.defaultAnnotations(&pod, &patches); err != nil {
 		return &v1beta1.AdmissionResponse{
 			Result: &metav1.Status{
 				Message: err.Error(),
@@ -160,9 +163,6 @@ func (h *Handler) Mutate(req *v1beta1.AdmissionRequest) *v1beta1.AdmissionRespon
 		return resp
 	}
 
-	// Accumulate any patches here
-	var patches []jsonpatch.JsonPatchOperation
-
 	// Add our volume that will be shared by the init container and
 	// the sidecar for passing data in the pod.
 	patches = append(patches, addVolume(
@@ -263,7 +263,7 @@ func (h *Handler) shouldInject(pod *corev1.Pod) (bool, error) {
 	return !h.RequireAnnotation, nil
 }
 
-func (h *Handler) defaultAnnotations(pod *corev1.Pod) error {
+func (h *Handler) defaultAnnotations(pod *corev1.Pod, patches *[]jsonpatch.JsonPatchOperation) error {
 	if pod.ObjectMeta.Annotations == nil {
 		pod.ObjectMeta.Annotations = make(map[string]string)
 	}
@@ -271,6 +271,13 @@ func (h *Handler) defaultAnnotations(pod *corev1.Pod) error {
 	// Default service name is the name of the first container.
 	if _, ok := pod.ObjectMeta.Annotations[annotationService]; !ok {
 		if cs := pod.Spec.Containers; len(cs) > 0 {
+			// Create the patch for this first, so that the Annotation
+			// object will be created if necessary
+			*patches = append(*patches, updateAnnotation(
+				pod.Annotations,
+				map[string]string{annotationService: cs[0].Name})...)
+
+			// Set the annotation for checking in shouldInject
 			pod.ObjectMeta.Annotations[annotationService] = cs[0].Name
 		}
 	}
@@ -280,8 +287,20 @@ func (h *Handler) defaultAnnotations(pod *corev1.Pod) error {
 		if cs := pod.Spec.Containers; len(cs) > 0 {
 			if ps := cs[0].Ports; len(ps) > 0 {
 				if ps[0].Name != "" {
+					// Create the patch for this first, so that the Annotation
+					// object will be created if necessary
+					*patches = append(*patches, updateAnnotation(
+						pod.Annotations,
+						map[string]string{annotationPort: ps[0].Name})...)
+
 					pod.ObjectMeta.Annotations[annotationPort] = ps[0].Name
 				} else {
+					// Create the patch for this first, so that the Annotation
+					// object will be created if necessary
+					*patches = append(*patches, updateAnnotation(
+						pod.Annotations,
+						map[string]string{annotationPort: strconv.Itoa(int(ps[0].ContainerPort))})...)
+
 					pod.ObjectMeta.Annotations[annotationPort] = strconv.Itoa(int(ps[0].ContainerPort))
 				}
 			}
diff --git a/connect-inject/handler_test.go b/connect-inject/handler_test.go
index fa5f51e12b..81b495e051 100644
--- a/connect-inject/handler_test.go
+++ b/connect-inject/handler_test.go
@@ -72,6 +72,10 @@ func TestHandlerHandle(t *testing.T) {
 			},
 			"",
 			[]jsonpatch.JsonPatchOperation{
+				{
+					Operation: "add",
+					Path:      "/metadata/annotations",
+				},
 				{
 					Operation: "add",
 					Path:      "/spec/volumes",
@@ -107,6 +111,10 @@ func TestHandlerHandle(t *testing.T) {
 			},
 			"",
 			[]jsonpatch.JsonPatchOperation{
+				{
+					Operation: "add",
+					Path:      "/metadata/annotations/" + escapeJSONPointer(annotationService),
+				},
 				{
 					Operation: "add",
 					Path:      "/spec/volumes",
@@ -176,6 +184,10 @@ func TestHandlerHandle(t *testing.T) {
 			},
 			"",
 			[]jsonpatch.JsonPatchOperation{
+				{
+					Operation: "add",
+					Path:      "/metadata/annotations/" + escapeJSONPointer(annotationService),
+				},
 				{
 					Operation: "add",
 					Path:      "/spec/volumes",
@@ -369,7 +381,8 @@ func TestHandlerDefaultAnnotations(t *testing.T) {
 			require := require.New(t)
 
 			var h Handler
-			err := h.defaultAnnotations(tt.Pod)
+			var patches []jsonpatch.JsonPatchOperation
+			err := h.defaultAnnotations(tt.Pod, &patches)
 			if (tt.Err != "") != (err != nil) {
 				t.Fatalf("actual: %v, expected err: %v", err, tt.Err)
 			}