Fix pod creation issue when having multiple secret from same secret g…

…roup (flyteorg#306) * Fix pod creation issue when having multiple secret from same secret group Signed-off-by: Pradithya Aria <[email protected]> * Add new line Signed-off-by: Pradithya Aria <[email protected]> * Remove garbage Signed-off-by: Pradithya Aria <[email protected]>
eapolinario · Aug 20, 2021 · bab3c23 · bab3c23
1 parent fbb24fa
commit bab3c23
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 4 deletions.
diff --git a/flytepropeller/pkg/webhook/k8s_secrets.go b/flytepropeller/pkg/webhook/k8s_secrets.go
@@ -51,7 +51,7 @@ func (i K8sSecretInjector) Inject(ctx context.Context, secret *core.Secret, p *c
 		// file.
 
 		volume := CreateVolumeForSecret(secret)
-		p.Spec.Volumes = append(p.Spec.Volumes, volume)
+		p.Spec.Volumes = AppendVolume(p.Spec.Volumes, volume)
 
 		// Mount the secret to all containers in the given pod.
 		mount := CreateVolumeMountForSecret(volume.Name, secret)

diff --git a/flytepropeller/pkg/webhook/k8s_secrets_test.go b/flytepropeller/pkg/webhook/k8s_secrets_test.go
@@ -54,7 +54,7 @@ func TestK8sSecretInjector_Inject(t *testing.T) {
 		Spec: corev1.PodSpec{
 			Volumes: []corev1.Volume{
 				{
-					Name: "m4zg54lql4ugk2dmn4pq",
+					Name: "m4zg54lql3",
 					VolumeSource: corev1.VolumeSource{
 						Secret: &corev1.SecretVolumeSource{
 							SecretName: "group",
@@ -74,7 +74,54 @@ func TestK8sSecretInjector_Inject(t *testing.T) {
 					Name: "container1",
 					VolumeMounts: []corev1.VolumeMount{
 						{
-							Name:      "m4zg54lql4ugk2dmn4pq",
+							Name:      "m4zg54lql3",
+							MountPath: "/etc/flyte/secrets/group",
+							ReadOnly:  true,
+						},
+					},
+					Env: []corev1.EnvVar{
+						{
+							Name:  "FLYTE_SECRETS_DEFAULT_DIR",
+							Value: "/etc/flyte/secrets",
+						},
+						{
+							Name: "FLYTE_SECRETS_FILE_PREFIX",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	successPodMultiFiles := corev1.Pod{
+		Spec: corev1.PodSpec{
+			Volumes: []corev1.Volume{
+				{
+					Name: "m4zg54lql3",
+					VolumeSource: corev1.VolumeSource{
+						Secret: &corev1.SecretVolumeSource{
+							SecretName: "group",
+							Items: []corev1.KeyToPath{
+								{
+									Key:  "hello",
+									Path: "hello",
+								},
+								{
+									Key:  "world",
+									Path: "world",
+								},
+							},
+						},
+					},
+				},
+			},
+			InitContainers: []corev1.Container{},
+			Containers: []corev1.Container{
+				{
+					Name: "container1",
+					VolumeMounts: []corev1.VolumeMount{
+						{
+							Name:      "m4zg54lql3",
 							MountPath: "/etc/flyte/secrets/group",
 							ReadOnly:  true,
 						},
@@ -148,6 +195,9 @@ func TestK8sSecretInjector_Inject(t *testing.T) {
 		{name: "require file single", args: args{secret: &coreIdl.Secret{Group: "group", Key: "hello", MountRequirement: coreIdl.Secret_FILE},
 			p: inputPod.DeepCopy()},
 			want: &successPodFile, wantErr: false},
+		{name: "require file multiple from same secret group", args: args{secret: &coreIdl.Secret{Group: "group", Key: "world", MountRequirement: coreIdl.Secret_FILE},
+			p: successPodFile.DeepCopy()},
+			want: &successPodMultiFiles, wantErr: false},
 		{name: "require file all keys", args: args{secret: &coreIdl.Secret{Key: "hello", MountRequirement: coreIdl.Secret_FILE},
 			p: inputPod.DeepCopy()},
 			want: &successPodFileAllKeys, wantErr: true},

diff --git a/flytepropeller/pkg/webhook/utils.go b/flytepropeller/pkg/webhook/utils.go
@@ -37,7 +37,8 @@ func CreateEnvVarForSecret(secret *core.Secret) corev1.EnvVar {
 
 func CreateVolumeForSecret(secret *core.Secret) corev1.Volume {
 	return corev1.Volume{
-		Name: utils.Base32Encoder.EncodeToString([]byte(secret.Group + EnvVarGroupKeySeparator + secret.Key + EnvVarGroupKeySeparator + secret.GroupVersion)),
+		// we don't want to create different volume for the same secret group
+		Name: utils.Base32Encoder.EncodeToString([]byte(secret.Group + EnvVarGroupKeySeparator + secret.GroupVersion)),
 		VolumeSource: corev1.VolumeSource{
 			Secret: &corev1.SecretVolumeSource{
 				SecretName: secret.Group,
@@ -102,3 +103,15 @@ func appendVolumeMountIfNotExists(volumes []corev1.VolumeMount, vol corev1.Volum
 
 	return append(volumes, vol)
 }
+
+func AppendVolume(volumes []corev1.Volume, volume corev1.Volume) []corev1.Volume {
+	for _, v := range volumes {
+		// append secret items to existing volume for secret within same secret group
+		if v.Secret.SecretName == volume.Secret.SecretName {
+			v.Secret.Items = append(v.Secret.Items, volume.Secret.Items...)
+			return volumes
+		}
+	}
+
+	return append(volumes, volume)
+}