More unit tests

pkg/webhook/k8s_secrets.go

@@ -48,26 +48,8 @@ func (i K8sSecretInjector) Inject(ctx context.Context, secret *core.Secret, p *c
 					},
 				},
 			})
-		} else if len(secret.Key) > 0 {
-			volumeName = secret.Key
-			p.Spec.Volumes = append(p.Spec.Volumes, corev1.Volume{
-				Name: volumeName,
-				VolumeSource: corev1.VolumeSource{
-					Secret: &corev1.SecretVolumeSource{
-						SecretName: secret.Key,
-					},
-				},
-			})
-		} else if len(secret.Group) > 0 {
-			volumeName = secret.Group
-			p.Spec.Volumes = append(p.Spec.Volumes, corev1.Volume{
-				Name: volumeName,
-				VolumeSource: corev1.VolumeSource{
-					Secret: &corev1.SecretVolumeSource{
-						SecretName: secret.Group,
-					},
-				},
-			})
+		} else {
+			return nil, false, fmt.Errorf("k8s Secrets Webhook require both key and group to be set")
 		}
 
 		// Mount the secret to all containers in the given pod.

pkg/webhook/k8s_secrets_test.go

@@ -150,13 +150,7 @@ func TestK8sSecretInjector_Inject(t *testing.T) {
 			want: &successPodFile, wantErr: false},
 		{name: "require file all keys", args: args{secret: &coreIdl.Secret{Key: "hello", MountRequirement: coreIdl.Secret_FILE},
 			p: inputPod.DeepCopy()},
-			want: &successPodFileAllKeys, wantErr: false},
-		{name: "require file all keys", args: args{secret: &coreIdl.Secret{Group: "hello", MountRequirement: coreIdl.Secret_FILE},
-			p: inputPod.DeepCopy()},
-			want: &successPodFileAllKeys, wantErr: false},
-		{name: "require file all keys", args: args{secret: &coreIdl.Secret{Group: "hello"},
-			p: inputPod.DeepCopy()},
-			want: &successPodFileAllKeys, wantErr: false},
+			want: &successPodFileAllKeys, wantErr: true},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

pkg/webhook/pod.go

@@ -26,6 +26,7 @@ import (
 
 const webhookName = "flyte-pod-webhook.flyte.org"
 
+// PodMutator implements controller-runtime WebHook interface.
 type PodMutator struct {
 	decoder  *admission.Decoder
 	cfg      *Config

pkg/webhook/pod_test.go

@@ -0,0 +1,158 @@
+package webhook
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"k8s.io/client-go/tools/clientcmd/api/latest"
+
+	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+
+	"github.com/flyteorg/flytestdlib/promutils"
+
+	"github.com/flyteorg/flytepropeller/pkg/webhook/mocks"
+	"github.com/stretchr/testify/mock"
+
+	"github.com/stretchr/testify/assert"
+	admissionv1 "k8s.io/api/admission/v1"
+	corev1 "k8s.io/api/core/v1"
+)
+
+func TestPodMutator_Mutate(t *testing.T) {
+	inputPod := &corev1.Pod{
+		Spec: corev1.PodSpec{
+			Containers: []corev1.Container{
+				{
+					Name: "container1",
+				},
+			},
+		},
+	}
+
+	successMutator := &mocks.Mutator{}
+	successMutator.OnID().Return("SucceedingMutator")
+	successMutator.OnMutateMatch(mock.Anything, mock.Anything).Return(nil, false, nil)
+
+	failedMutator := &mocks.Mutator{}
+	failedMutator.OnID().Return("FailingMutator")
+	failedMutator.OnMutateMatch(mock.Anything, mock.Anything).Return(nil, false, fmt.Errorf("failing mock"))
+
+	t.Run("Required Mutator Succeeded", func(t *testing.T) {
+		pm := &PodMutator{
+			Mutators: []MutatorConfig{
+				{
+					Mutator:  successMutator,
+					Required: true,
+				},
+			},
+		}
+		ctx := context.Background()
+		_, changed, err := pm.Mutate(ctx, inputPod.DeepCopy())
+		assert.NoError(t, err)
+		assert.False(t, changed)
+	})
+
+	t.Run("Required Mutator Failed", func(t *testing.T) {
+		pm := &PodMutator{
+			Mutators: []MutatorConfig{
+				{
+					Mutator:  failedMutator,
+					Required: true,
+				},
+			},
+		}
+		ctx := context.Background()
+		_, _, err := pm.Mutate(ctx, inputPod.DeepCopy())
+		assert.Error(t, err)
+	})
+
+	t.Run("Non-required Mutator Failed", func(t *testing.T) {
+		pm := &PodMutator{
+			Mutators: []MutatorConfig{
+				{
+					Mutator:  failedMutator,
+					Required: false,
+				},
+			},
+		}
+		ctx := context.Background()
+		_, _, err := pm.Mutate(ctx, inputPod.DeepCopy())
+		assert.NoError(t, err)
+	})
+}
+
+func Test_CreateMutationWebhookConfiguration(t *testing.T) {
+	pm := NewPodMutator(&Config{
+		CertDir:     "testdata",
+		ServiceName: "my-service",
+	}, promutils.NewTestScope())
+
+	t.Run("Empty namespace", func(t *testing.T) {
+		c, err := pm.CreateMutationWebhookConfiguration("")
+		assert.NoError(t, err)
+		assert.NotNil(t, c)
+	})
+
+	t.Run("With namespace", func(t *testing.T) {
+		c, err := pm.CreateMutationWebhookConfiguration("my-namespace")
+		assert.NoError(t, err)
+		assert.NotNil(t, c)
+	})
+}
+
+func Test_Handle(t *testing.T) {
+	pm := NewPodMutator(&Config{
+		CertDir:     "testdata",
+		ServiceName: "my-service",
+	}, promutils.NewTestScope())
+
+	decoder, err := admission.NewDecoder(latest.Scheme)
+	assert.NoError(t, err)
+	assert.NoError(t, pm.InjectDecoder(decoder))
+
+	req := admission.Request{
+		AdmissionRequest: admissionv1.AdmissionRequest{
+			Object: runtime.RawExtension{
+				Raw: []byte(`{
+    "apiVersion": "v1",
+    "kind": "Pod",
+    "metadata": {
+        "name": "foo",
+        "namespace": "default"
+    },
+    "spec": {
+        "containers": [
+            {
+                "image": "bar:v2",
+                "name": "bar"
+            }
+        ]
+    }
+}`),
+			},
+			OldObject: runtime.RawExtension{
+				Raw: []byte(`{
+    "apiVersion": "v1",
+    "kind": "Pod",
+    "metadata": {
+        "name": "foo",
+        "namespace": "default"
+    },
+    "spec": {
+        "containers": [
+            {
+                "image": "bar:v1",
+                "name": "bar"
+            }
+        ]
+    }
+}`),
+			},
+		},
+	}
+
+	resp := pm.Handle(context.Background(), req)
+	assert.True(t, resp.Allowed)
+}

pkg/webhook/secrets_test.go

@@ -41,7 +41,7 @@ func TestSecretsWebhook_Mutate(t *testing.T) {
 		}
 
 		_, changed, err := m.Mutate(context.Background(), podWithAnnotations.DeepCopy())
-		assert.NoError(t, err)
+		assert.Error(t, err)
 		assert.False(t, changed)
 	})
 

pkg/webhook/testdata/ca.crt

@@ -0,0 +1 @@
+SGVsbG8gV29ybGQK

pkg/webhook/utils_test.go

@@ -4,6 +4,8 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+
 	corev1 "k8s.io/api/core/v1"
 )
 
@@ -173,5 +175,14 @@ func TestUpdateEnvVars(t *testing.T) {
 }
 
 func TestReadFile(t *testing.T) {
+	t.Run("Found", func(t *testing.T) {
+		raw, err := ReadFile("utils.go")
+		assert.NoError(t, err)
+		assert.NotEmpty(t, raw)
+	})
 
-}
+	t.Run("Not Found", func(t *testing.T) {
+		_, err := ReadFile("utils-not-found.go")
+		assert.Error(t, err)
+	})
+}