refactor: remove err return and add unit tests for handle errors (#1091)

Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>
Azure · Aug 25, 2023 · 37dc12f · 37dc12f
1 parent 41f2e5e
commit 37dc12f
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 13 deletions.
diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go
@@ -159,10 +159,7 @@ func (m *podMutator) Handle(ctx context.Context, req admission.Request) (respons
 	pod.Spec.Containers = m.mutateContainers(pod.Spec.Containers, clientID, tenantID, skipContainers)
 
 	// add the projected service account token volume to the pod if not exists
-	if err = addProjectedServiceAccountTokenVolume(pod, serviceAccountTokenExpiration, m.audience); err != nil {
-		logger.Error("failed to add projected service account volume", err)
-		return admission.Errored(http.StatusBadRequest, err)
-	}
+	addProjectedServiceAccountTokenVolume(pod, serviceAccountTokenExpiration, m.audience)
 
 	marshaledPod, err := json.Marshal(pod)
 	if err != nil {
@@ -401,7 +398,7 @@ func addProjectedTokenVolumeMount(container corev1.Container) corev1.Container {
 	return container
 }
 
-func addProjectedServiceAccountTokenVolume(pod *corev1.Pod, serviceAccountTokenExpiration int64, audience string) error {
+func addProjectedServiceAccountTokenVolume(pod *corev1.Pod, serviceAccountTokenExpiration int64, audience string) {
 	// add the projected service account token volume to the pod if not exists
 	for _, volume := range pod.Spec.Volumes {
 		if volume.Projected == nil {
@@ -412,7 +409,7 @@ func addProjectedServiceAccountTokenVolume(pod *corev1.Pod, serviceAccountTokenE
 				continue
 			}
 			if pvs.ServiceAccountToken.Path == TokenFilePathName {
-				return nil
+				return
 			}
 		}
 	}
@@ -436,9 +433,8 @@ func addProjectedServiceAccountTokenVolume(pod *corev1.Pod, serviceAccountTokenE
 					},
 				},
 			},
-		})
-
-	return nil
+		},
+	)
 }
 
 // getAzureAuthorityHost returns the active directory endpoint to use for requesting

diff --git a/pkg/webhook/webhook_test.go b/pkg/webhook/webhook_test.go
@@ -481,10 +481,8 @@ func TestAddProjectedServiceAccountTokenVolume(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			err := addProjectedServiceAccountTokenVolume(test.pod, serviceAccountTokenExpiry, DefaultAudience)
-			if err != nil {
-				t.Fatalf("expected err to be nil, got: %v", err)
-			}
+			addProjectedServiceAccountTokenVolume(test.pod, serviceAccountTokenExpiry, DefaultAudience)
+
 			if !reflect.DeepEqual(test.pod.Spec.Volumes, test.expectedVolume) {
 				t.Fatalf("expected: %v, got: %v", test.pod.Spec.Volumes, test.expectedVolume)
 			}
@@ -1295,13 +1293,38 @@ func TestHandleError(t *testing.T) {
 		clientObjects []client.Object
 		expectedErr   string
 	}{
+		{
+			name:          "failed to decode pod",
+			object:        runtime.RawExtension{Raw: []byte("invalid")},
+			clientObjects: serviceAccounts,
+			expectedErr:   `couldn't get version/kind`,
+		},
+		{
+			name:        "service account not found",
+			object:      runtime.RawExtension{Raw: newPodRaw("pod", "ns1", "sa", map[string]string{UseWorkloadIdentityLabel: "true"}, nil, true)},
+			expectedErr: `serviceaccounts "sa" not found`,
+		},
 		{
 			name: "pod has host network",
 			object: runtime.RawExtension{Raw: newPodRaw("pod", "ns1", "sa",
 				map[string]string{UseWorkloadIdentityLabel: "true"}, map[string]string{InjectProxySidecarAnnotation: "true"}, true)},
 			clientObjects: serviceAccounts,
 			expectedErr:   "hostNetwork is set to true, cannot inject proxy sidecar",
 		},
+		{
+			name: "invalid proxy port",
+			object: runtime.RawExtension{Raw: newPodRaw("pod", "ns1", "sa", map[string]string{UseWorkloadIdentityLabel: "true"},
+				map[string]string{InjectProxySidecarAnnotation: "true", ProxySidecarPortAnnotation: "invalid"}, false)},
+			clientObjects: serviceAccounts,
+			expectedErr:   `failed to parse proxy sidecar port: strconv.ParseInt: parsing "invalid": invalid syntax`,
+		},
+		{
+			name: "invalid sa token expiry",
+			object: runtime.RawExtension{Raw: newPodRaw("pod", "ns1", "sa", map[string]string{UseWorkloadIdentityLabel: "true"},
+				map[string]string{ServiceAccountTokenExpiryAnnotation: "invalid"}, false)},
+			clientObjects: serviceAccounts,
+			expectedErr:   `strconv.ParseInt: parsing "invalid": invalid syntax`,
+		},
 	}
 
 	for _, test := range tests {