Inject env vars for secret at the beginning of the existing env vars …

…for the containers (#333)

## Overview
Union secrets injected env vars should appear at the beggining of the env list.

This requirement came from the issue faced during NIMs poc where the sidecar container which required secret to be passed in with specific env var name

The NGC sidecar container requires a secret to passed in ENV var `NGC_API_KEY`
Since union injected secrets use _UNION_ prefix, we couldn't define the secret to be NGC_API_KEY directly as it would be injected as _UNION_NGC_API_KEY

Adding of _UNION_ prefix is to be able to distinguish the secret env vars injected by the webhook,
Unchanging that functionality , the proposal is to use https://kubernetes.io/docs/tasks/inject-data-application/define-interdependent-environment-variables/

which allow you to define NGC_API_KEY as following

`NGC_API_KEY= $(_UNION_NGC_API_KEY)`


Also the change removes duplicates if the user is trying to define the same Env var which union is injecting 




## Test Plan
Before the change
```
k describe pods -n development  agd92xq6rbhsvn25g7qb
    Environment:
      FLYTE_INTERNAL_EXECUTION_WORKFLOW:  flytesnacks:development:using_secrets.main
      FLYTE_INTERNAL_EXECUTION_ID:        agd92xq6rbhsvn25g7qb
      FLYTE_INTERNAL_EXECUTION_PROJECT:   flytesnacks
      FLYTE_INTERNAL_EXECUTION_DOMAIN:    development
      FLYTE_ATTEMPT_NUMBER:               0
      FLYTE_INTERNAL_TASK_PROJECT:        flytesnacks
      FLYTE_INTERNAL_TASK_DOMAIN:         development
      FLYTE_INTERNAL_TASK_NAME:           using_secrets.fn
      FLYTE_INTERNAL_TASK_VERSION:        zEKw37ArzIKUrfgKOlUHUg
      FLYTE_INTERNAL_PROJECT:             flytesnacks
      FLYTE_INTERNAL_DOMAIN:              development
      FLYTE_INTERNAL_NAME:                using_secrets.fn
      FLYTE_INTERNAL_VERSION:             zEKw37ArzIKUrfgKOlUHUg
      FLYTE_SECRETS_ENV_PREFIX:           _UNION_
      _UNION_MY-CUSTOM-SECRET:            Thisisasecret\r

```

After the change on dogfood-gcp
```
k describe pods -n development  av8hbdjlmf5lzc8gbp5k
    Environment:
      _UNION_MY-CUSTOM-SECRET:            Thisisasecret\r
                                          
      FLYTE_SECRETS_ENV_PREFIX:           _UNION_
      FLYTE_INTERNAL_EXECUTION_WORKFLOW:  flytesnacks:development:using_secrets.main
      FLYTE_INTERNAL_EXECUTION_ID:        av8hbdjlmf5lzc8gbp5k
      FLYTE_INTERNAL_EXECUTION_PROJECT:   flytesnacks
      FLYTE_INTERNAL_EXECUTION_DOMAIN:    development
      FLYTE_ATTEMPT_NUMBER:               0
      FLYTE_INTERNAL_TASK_PROJECT:        flytesnacks
      FLYTE_INTERNAL_TASK_DOMAIN:         development
      FLYTE_INTERNAL_TASK_NAME:           using_secrets.fn
      FLYTE_INTERNAL_TASK_VERSION:        zEKw37ArzIKUrfgKOlUHUg
      FLYTE_INTERNAL_PROJECT:             flytesnacks
      FLYTE_INTERNAL_DOMAIN:              development
      FLYTE_INTERNAL_NAME:                using_secrets.fn
      FLYTE_INTERNAL_VERSION:             zEKw37ArzIKUrfgKOlUHUg
```

Notice the position of _UNION_MY-CUSTOM-SECRET. Any union secrets would show up at the beginning of the list of ENV vars

## Rollout Plan (if applicable)
Rollout to staging and then demo tenant for NIMS feature


## Upstream Changes
Should this change be upstreamed to OSS (flyteorg/flyte)? If not, please uncheck this box, which is used for auditing. Note, it is the responsibility of each developer to actually upstream their changes. See [this guide](https://unionai.atlassian.net/wiki/spaces/ENG/pages/447610883/Flyte+-+Union+Cloud+Development+Runbook/#When-are-versions-updated%3F).

- [] To be upstreamed to OSS

## Issue
*TODO: Link Linear issue(s) using [magic words](https://linear.app/docs/github#magic-words). `fixes` will move to merged status, while `ref` will only link the PR.*

## Checklist
* [ ] Added tests
* [ ] Ran a deploy dry run and shared the terraform plan
* [ ] Added logging and metrics
* [ ] Updated [dashboards](https://unionai.grafana.net/dashboards) and [alerts](https://unionai.grafana.net/alerting/list)
* [ ] Updated documentation

flytepropeller/pkg/secret/aws_secret_manager_test.go

@@ -43,20 +43,20 @@ func TestAWSSecretManagerInjector_Inject(t *testing.T) {
 					Image: "docker.io/amazon/aws-secrets-manager-secret-sidecar:v0.1.4",
 					Env: []corev1.EnvVar{
 						{
-							Name:  "SECRET_ARN",
-							Value: inputSecret.Group + ":" + inputSecret.Key,
-						},
-						{
-							Name:  "SECRET_FILENAME",
-							Value: "/" + inputSecret.Group + "/" + inputSecret.Key,
+							Name:  "FLYTE_SECRETS_FILE_PREFIX",
+							Value: "",
 						},
 						{
 							Name:  "FLYTE_SECRETS_DEFAULT_DIR",
 							Value: "/etc/flyte/secrets",
 						},
 						{
-							Name:  "FLYTE_SECRETS_FILE_PREFIX",
-							Value: "",
+							Name:  "SECRET_ARN",
+							Value: inputSecret.Group + ":" + inputSecret.Key,
+						},
+						{
+							Name:  "SECRET_FILENAME",
+							Value: "/" + inputSecret.Group + "/" + inputSecret.Key,
 						},
 					},
 					VolumeMounts: []corev1.VolumeMount{

flytepropeller/pkg/secret/embedded_secret_manager_test.go

@@ -147,14 +147,14 @@ func TestEmbeddedSecretManagerInjector_Inject(t *testing.T) {
 					Containers: []corev1.Container{
 						{
 							Env: []corev1.EnvVar{
-								{
-									Name:  SecretEnvVarPrefix,
-									Value: UnionSecretEnvVarPrefix,
-								},
 								{
 									Name:  "_UNION_SECRETID",
 									Value: secretValue,
 								},
+								{
+									Name:  SecretEnvVarPrefix,
+									Value: UnionSecretEnvVarPrefix,
+								},
 							},
 						},
 					},

flytepropeller/pkg/secret/gcp_secret_manager_test.go

@@ -42,14 +42,14 @@ func TestGCPSecretManagerInjector_Inject(t *testing.T) {
 						"gcloud secrets versions access TestSecret/versions/2 --out-file=/etc/flyte/secrets/testsecret/2 || gcloud secrets versions access 2 --secret=TestSecret --out-file=/etc/flyte/secrets/testsecret/2; chmod +rX /etc/flyte/secrets/testsecret /etc/flyte/secrets/testsecret/2",
 					},
 					Env: []corev1.EnvVar{
-						{
-							Name:  "FLYTE_SECRETS_DEFAULT_DIR",
-							Value: "/etc/flyte/secrets",
-						},
 						{
 							Name:  "FLYTE_SECRETS_FILE_PREFIX",
 							Value: "",
 						},
+						{
+							Name:  "FLYTE_SECRETS_DEFAULT_DIR",
+							Value: "/etc/flyte/secrets",
+						},
 					},
 					VolumeMounts: []corev1.VolumeMount{
 						{

flytepropeller/pkg/secret/global_secrets_test.go

@@ -39,14 +39,14 @@ func TestGlobalSecrets_Inject(t *testing.T) {
 				{
 					Name: "container1",
 					Env: []corev1.EnvVar{
-						{
-							Name:  "FLYTE_SECRETS_ENV_PREFIX",
-							Value: "_FSEC_",
-						},
 						{
 							Name:  "_FSEC_GROUP_HELLO",
 							Value: "my_password",
 						},
+						{
+							Name:  "FLYTE_SECRETS_ENV_PREFIX",
+							Value: "_FSEC_",
+						},
 					},
 				},
 			},

flytepropeller/pkg/secret/k8s_secrets_test.go

@@ -30,6 +30,10 @@ func TestK8sSecretInjector_Inject(t *testing.T) {
 				{
 					Name: "container1",
 					Env: []corev1.EnvVar{
+						{
+							Name:  "FLYTE_SECRETS_ENV_PREFIX",
+							Value: "_FSEC_",
+						},
 						{
 							Name: "_FSEC_GROUP_HELLO",
 							ValueFrom: &corev1.EnvVarSource{
@@ -42,10 +46,6 @@ func TestK8sSecretInjector_Inject(t *testing.T) {
 								},
 							},
 						},
-						{
-							Name:  "FLYTE_SECRETS_ENV_PREFIX",
-							Value: "_FSEC_",
-						},
 					},
 				},
 			},
@@ -84,11 +84,11 @@ func TestK8sSecretInjector_Inject(t *testing.T) {
 					},
 					Env: []corev1.EnvVar{
 						{
-							Name:  "FLYTE_SECRETS_DEFAULT_DIR",
-							Value: "/etc/flyte/secrets",
+							Name: "FLYTE_SECRETS_FILE_PREFIX",
 						},
 						{
-							Name: "FLYTE_SECRETS_FILE_PREFIX",
+							Name:  "FLYTE_SECRETS_DEFAULT_DIR",
+							Value: "/etc/flyte/secrets",
 						},
 					},
 				},
@@ -132,11 +132,11 @@ func TestK8sSecretInjector_Inject(t *testing.T) {
 					},
 					Env: []corev1.EnvVar{
 						{
-							Name:  "FLYTE_SECRETS_DEFAULT_DIR",
-							Value: "/etc/flyte/secrets",
+							Name: "FLYTE_SECRETS_FILE_PREFIX",
 						},
 						{
-							Name: "FLYTE_SECRETS_FILE_PREFIX",
+							Name:  "FLYTE_SECRETS_DEFAULT_DIR",
+							Value: "/etc/flyte/secrets",
 						},
 					},
 				},
@@ -170,11 +170,11 @@ func TestK8sSecretInjector_Inject(t *testing.T) {
 					},
 					Env: []corev1.EnvVar{
 						{
-							Name:  "FLYTE_SECRETS_DEFAULT_DIR",
-							Value: "/etc/flyte/secrets",
+							Name: "FLYTE_SECRETS_FILE_PREFIX",
 						},
 						{
-							Name: "FLYTE_SECRETS_FILE_PREFIX",
+							Name:  "FLYTE_SECRETS_DEFAULT_DIR",
+							Value: "/etc/flyte/secrets",
 						},
 					},
 				},

flytepropeller/pkg/secret/utils.go

@@ -13,14 +13,15 @@ import (
 	"github.com/flyteorg/flyte/flytepropeller/pkg/secret/config"
 )
 
-func hasEnvVar(envVars []corev1.EnvVar, envVarKey string) bool {
-	for _, e := range envVars {
+// If env var exists in the existing list of envVars then return the index for it or else return -1
+func hasEnvVar(envVars []corev1.EnvVar, envVarKey string) int {
+	for index, e := range envVars {
 		if e.Name == envVarKey {
-			return true
+			return index
 		}
 	}
 
-	return false
+	return -1
 }
 
 func CreateEnvVarForSecret(secret *core.Secret) corev1.EnvVar {
@@ -80,10 +81,13 @@ func AppendVolumeMounts(containers []corev1.Container, mount corev1.VolumeMount)
 func AppendEnvVars(containers []corev1.Container, envVar corev1.EnvVar) []corev1.Container {
 	res := make([]corev1.Container, 0, len(containers))
 	for _, c := range containers {
-		if !hasEnvVar(c.Env, envVar.Name) {
-			c.Env = append(c.Env, envVar)
+		if foundIndex := hasEnvVar(c.Env, envVar.Name); foundIndex >= 0 {
+			// This would be someone adding a duplicate key to what the webhook is trying to add.We should delete the existing one and then add the new at the beginning
+			c.Env = append(c.Env[:foundIndex], c.Env[foundIndex+1:]...)
 		}
-
+		// Append the passed in environment variable to the start of the list.
+		// With multiple calls to this function too, eg : in case of injecting multiple secrets, the same premise holds.
+		c.Env = append([]corev1.EnvVar{envVar}, c.Env...)
 		res = append(res, c)
 	}
 

flytepropeller/pkg/secret/utils_test.go

@@ -17,7 +17,7 @@ func Test_hasEnvVar(t *testing.T) {
 	tests := []struct {
 		name string
 		args args
-		want bool
+		want int
 	}{
 		{
 			name: "exists",
@@ -29,7 +29,7 @@ func Test_hasEnvVar(t *testing.T) {
 				},
 				envVarKey: "ENV_VAR_1",
 			},
-			want: true,
+			want: 0,
 		},
 
 		{
@@ -42,7 +42,7 @@ func Test_hasEnvVar(t *testing.T) {
 				},
 				envVarKey: "ENV_VAR",
 			},
-			want: false,
+			want: -1,
 		},
 	}
 
@@ -137,7 +137,7 @@ func TestUpdateEnvVars(t *testing.T) {
 					Env: []corev1.EnvVar{
 						{
 							Name:  "my_secret",
-							Value: "my_val_already",
+							Value: "my_val",
 						},
 					},
 				},

flytepropeller/pkg/secret/vault_secret_manager_test.go

@@ -22,11 +22,11 @@ var (
 				Name: "container1",
 				Env: []corev1.EnvVar{
 					{
-						Name:  "FLYTE_SECRETS_DEFAULT_DIR",
-						Value: "/etc/flyte/secrets",
+						Name: "FLYTE_SECRETS_FILE_PREFIX",
 					},
 					{
-						Name: "FLYTE_SECRETS_FILE_PREFIX",
+						Name:  "FLYTE_SECRETS_DEFAULT_DIR",
+						Value: "/etc/flyte/secrets",
 					},
 				},
 			},