Merge a9691eb into d9586b0

flyteorg · Oct 3, 2023 · abff384 · abff384
2 parents d9586b0 + a9691eb
commit abff384
Show file tree

Hide file tree

Showing 12 changed files with 322 additions and 87 deletions.
diff --git a/flytepropeller/go.mod b/flytepropeller/go.mod
@@ -145,6 +145,7 @@ require (
 	sigs.k8s.io/yaml v1.3.0 // indirect
 )
 
+<<<<<<< HEAD
 replace (
 	github.com/aws/amazon-sagemaker-operator-for-k8s => github.com/aws/amazon-sagemaker-operator-for-k8s v1.0.1-0.20210303003444-0fb33b1fd49d
 	github.com/flyteorg/flyte/datacatalog => ../datacatalog
@@ -154,3 +155,8 @@ replace (
 	github.com/flyteorg/flyte/flytestdlib => ../flytestdlib
 	github.com/flyteorg/flyteidl => ../flyteidl
 )
+=======
+replace github.com/aws/amazon-sagemaker-operator-for-k8s => github.com/aws/amazon-sagemaker-operator-for-k8s v1.0.1-0.20210303003444-0fb33b1fd49d
+
+replace github.com/flyteorg/flyteidl => /home/geert/git/flyte/flyteidl
+>>>>>>> flytepropeller/feat-add-optional-environment-variable-name-to-secret
diff --git a/...r/test/testdata/snacks-core/012_core.containerization.use_secrets.secret_task_1_task.yaml b/...r/test/testdata/snacks-core/012_core.containerization.use_secrets.secret_task_1_task.yaml
@@ -43,6 +43,7 @@ template:
       version: 0.32.6
   security_context:
     secrets:
-    - group: user-info
+    - MountTarget: null
+      group: user-info
       key: user_secret
   type: python-task
diff --git a/...est/testdata/snacks-core/013_core.containerization.use_secrets.user_info_task_1_task.yaml b/...est/testdata/snacks-core/013_core.containerization.use_secrets.user_info_task_1_task.yaml
@@ -48,8 +48,10 @@ template:
       version: 0.32.6
   security_context:
     secrets:
-    - group: user-info
+    - MountTarget: null
+      group: user-info
       key: username
-    - group: user-info
+    - MountTarget: null
+      group: user-info
       key: password
   type: python-task
diff --git a/...t/testdata/snacks-core/014_core.containerization.use_secrets.secret_file_task_1_task.yaml b/...t/testdata/snacks-core/014_core.containerization.use_secrets.secret_file_task_1_task.yaml
@@ -48,7 +48,8 @@ template:
       version: 0.32.6
   security_context:
     secrets:
-    - group: user-info
+    - MountTarget: null
+      group: user-info
       key: user_secret
       mount_requirement: 1
   type: python-task
diff --git a/...stdata/snacks-core/compiled/012_core.containerization.use_secrets.secret_task_1_task.yaml b/...stdata/snacks-core/compiled/012_core.containerization.use_secrets.secret_task_1_task.yaml
@@ -50,6 +50,7 @@ template:
       version: 0.32.6
   security_context:
     secrets:
-    - group: user-info
+    - MountTarget: null
+      group: user-info
       key: user_secret
   type: python-task
diff --git a/...ata/snacks-core/compiled/013_core.containerization.use_secrets.user_info_task_1_task.yaml b/...ata/snacks-core/compiled/013_core.containerization.use_secrets.user_info_task_1_task.yaml
@@ -55,8 +55,10 @@ template:
       version: 0.32.6
   security_context:
     secrets:
-    - group: user-info
+    - MountTarget: null
+      group: user-info
       key: username
-    - group: user-info
+    - MountTarget: null
+      group: user-info
       key: password
   type: python-task
diff --git a/...a/snacks-core/compiled/014_core.containerization.use_secrets.secret_file_task_1_task.yaml b/...a/snacks-core/compiled/014_core.containerization.use_secrets.secret_file_task_1_task.yaml
@@ -55,7 +55,8 @@ template:
       version: 0.32.6
   security_context:
     secrets:
-    - group: user-info
+    - MountTarget: null
+      group: user-info
       key: user_secret
       mount_requirement: 1
   type: python-task
diff --git a/...a/snacks-core/compiled/015_core.containerization.use_secrets.my_secret_workflow_2_wf.yaml b/...a/snacks-core/compiled/015_core.containerization.use_secrets.my_secret_workflow_2_wf.yaml
@@ -226,7 +226,8 @@ tasks:
         version: 0.32.6
     security_context:
       secrets:
-      - group: user-info
+      - MountTarget: null
+        group: user-info
         key: user_secret
         mount_requirement: 1
     type: python-task
@@ -282,7 +283,8 @@ tasks:
         version: 0.32.6
     security_context:
       secrets:
-      - group: user-info
+      - MountTarget: null
+        group: user-info
         key: user_secret
     type: python-task
 - template:
@@ -342,8 +344,10 @@ tasks:
         version: 0.32.6
     security_context:
       secrets:
-      - group: user-info
+      - MountTarget: null
+        group: user-info
         key: username
-      - group: user-info
+      - MountTarget: null
+        group: user-info
         key: password
     type: python-task
diff --git a/flytepropeller/pkg/webhook/global_secrets.go b/flytepropeller/pkg/webhook/global_secrets.go
@@ -30,42 +30,46 @@ func (g GlobalSecrets) Type() config.SecretManagerType {
 }
 
 func (g GlobalSecrets) Inject(ctx context.Context, secret *coreIdl.Secret, p *corev1.Pod) (newP *corev1.Pod, injected bool, err error) {
+
 	v, err := g.envSecretManager.GetForSecret(ctx, secret)
 	if err != nil {
 		return p, false, err
 	}
 
-	switch secret.MountRequirement {
-	case coreIdl.Secret_FILE:
-		return nil, false, fmt.Errorf("global secrets can only be injected as environment "+
-			"variables [%v/%v]", secret.Group, secret.Key)
-	case coreIdl.Secret_ANY:
-		fallthrough
-	case coreIdl.Secret_ENV_VAR:
-		if len(secret.Group) == 0 {
-			return nil, false, fmt.Errorf("mounting a secret to env var requires selecting the "+
-				"secret and a single key within. Key [%v]", secret.Key)
-		}
-
-		envVar := corev1.EnvVar{
-			Name:  strings.ToUpper(K8sDefaultEnvVarPrefix + secret.Group + EnvVarGroupKeySeparator + secret.Key),
-			Value: v,
+	if secret.MountTarget != nil {
+		switch secret.MountTarget.(type) {
+		case *coreIdl.Secret_EnvVar:
+			target, ok := secret.GetMountTarget().(*coreIdl.Secret_EnvVar)
+			if ok {
+				InjectEnvVar(p, secret, &target.EnvVar.Name, v)
+			}
+		case *coreIdl.Secret_File:
+			return nil, false, fmt.Errorf("global secrets can only be injected as environment "+
+				"variables [%v/%v]", secret.Group, secret.Key)
+		default:
+			err := fmt.Errorf("unrecognized mount target [%v] for secret [%v]", secret.GetMountTarget(), secret.Key)
+			logger.Error(ctx, err)
+			return p, false, err
 		}
+	} else {
+		switch secret.MountRequirement {
+		case coreIdl.Secret_FILE:
+			return nil, false, fmt.Errorf("global secrets can only be injected as environment "+
+				"variables [%v/%v]", secret.Group, secret.Key)
+		case coreIdl.Secret_ANY:
+			fallthrough
+		case coreIdl.Secret_ENV_VAR:
+			if len(secret.Group) == 0 {
+				return nil, false, fmt.Errorf("mounting a secret to env var requires selecting the "+
+					"secret and a single key within. Key [%v]", secret.Key)
+			}
 
-		prefixEnvVar := corev1.EnvVar{
-			Name:  SecretEnvVarPrefix,
-			Value: K8sDefaultEnvVarPrefix,
+			InjectEnvVar(p, secret, nil, v)
+		default:
+			err := fmt.Errorf("unrecognized mount requirement [%v] for secret [%v]", secret.MountRequirement.String(), secret.Key)
+			logger.Error(ctx, err)
+			return p, false, err
 		}
-
-		p.Spec.InitContainers = AppendEnvVars(p.Spec.InitContainers, prefixEnvVar)
-		p.Spec.Containers = AppendEnvVars(p.Spec.Containers, prefixEnvVar)
-
-		p.Spec.InitContainers = AppendEnvVars(p.Spec.InitContainers, envVar)
-		p.Spec.Containers = AppendEnvVars(p.Spec.Containers, envVar)
-	default:
-		err := fmt.Errorf("unrecognized mount requirement [%v] for secret [%v]", secret.MountRequirement.String(), secret.Key)
-		logger.Error(ctx, err)
-		return p, false, err
 	}
 
 	return p, true, nil
@@ -76,3 +80,26 @@ func NewGlobalSecrets(provider GlobalSecretProvider) GlobalSecrets {
 		envSecretManager: provider,
 	}
 }
+
+func InjectEnvVar(p *corev1.Pod, secret *coreIdl.Secret, envVarName *string, value string) {
+	_envVarName := strings.ToUpper(K8sDefaultEnvVarPrefix + secret.Group + EnvVarGroupKeySeparator + secret.Key)
+	if envVarName != nil {
+		_envVarName = *envVarName
+	}
+
+	envVar := corev1.EnvVar{
+		Name:  _envVarName,
+		Value: value,
+	}
+
+	prefixEnvVar := corev1.EnvVar{
+		Name:  SecretEnvVarPrefix,
+		Value: K8sDefaultEnvVarPrefix,
+	}
+
+	p.Spec.InitContainers = AppendEnvVars(p.Spec.InitContainers, prefixEnvVar)
+	p.Spec.Containers = AppendEnvVars(p.Spec.Containers, prefixEnvVar)
+
+	p.Spec.InitContainers = AppendEnvVars(p.Spec.InitContainers, envVar)
+	p.Spec.Containers = AppendEnvVars(p.Spec.Containers, envVar)
+}
diff --git a/flytepropeller/pkg/webhook/k8s_secrets.go b/flytepropeller/pkg/webhook/k8s_secrets.go
@@ -38,64 +38,99 @@ func (i K8sSecretInjector) Type() config.SecretManagerType {
 }
 
 func (i K8sSecretInjector) Inject(ctx context.Context, secret *core.Secret, p *corev1.Pod) (newP *corev1.Pod, injected bool, err error) {
+
 	if len(secret.Group) == 0 || len(secret.Key) == 0 {
 		return nil, false, fmt.Errorf("k8s Secrets Webhook require both key and group to be set. "+
 			"Secret: [%v]", secret)
 	}
 
-	switch secret.MountRequirement {
-	case core.Secret_ANY:
-		fallthrough
-	case core.Secret_FILE:
-		// Inject a Volume that to the pod and all of its containers and init containers that mounts the secret into a
-		// file.
-
-		volume := CreateVolumeForSecret(secret)
-		p.Spec.Volumes = AppendVolume(p.Spec.Volumes, volume)
-
-		// Mount the secret to all containers in the given pod.
-		mount := CreateVolumeMountForSecret(volume.Name, secret)
-		p.Spec.InitContainers = AppendVolumeMounts(p.Spec.InitContainers, mount)
-		p.Spec.Containers = AppendVolumeMounts(p.Spec.Containers, mount)
-
-		// Set environment variable to let the container know where to find the mounted files.
-		defaultDirEnvVar := corev1.EnvVar{
-			Name:  SecretPathDefaultDirEnvVar,
-			Value: filepath.Join(K8sSecretPathPrefix...),
+	// Check for new MountTarget field. If MountTarget doesn't exist, fall back to old MountRequirement field.
+	if secret.MountTarget != nil {
+		switch secret.MountTarget.(type) {
+		case *core.Secret_EnvVar:
+			target, ok := secret.GetMountTarget().(*core.Secret_EnvVar)
+			if ok {
+				InjectSecretAsEnvVar(p, secret, &target.EnvVar.Name)
+			}
+		case *core.Secret_File:
+			target, ok := secret.GetMountTarget().(*core.Secret_File)
+			if ok {
+				InjectSecretAsFile(p, secret, &target.File.Path)
+			}
+		default:
+			err := fmt.Errorf("unrecognized mount target [%v] for secret [%v]", secret.GetMountTarget(), secret.Key)
+			logger.Error(ctx, err)
+			return p, false, err
+		}
+	} else {
+		switch secret.MountRequirement {
+		case core.Secret_ANY:
+			fallthrough
+		case core.Secret_FILE:
+			InjectSecretAsFile(p, secret, nil)
+		case core.Secret_ENV_VAR:
+			InjectSecretAsEnvVar(p, secret, nil)
+		default:
+			err := fmt.Errorf("unrecognized mount requirement [%v] for secret [%v]", secret.MountRequirement.String(), secret.Key)
+			logger.Error(ctx, err)
+			return p, false, err
 		}
+	}
 
-		p.Spec.InitContainers = AppendEnvVars(p.Spec.InitContainers, defaultDirEnvVar)
-		p.Spec.Containers = AppendEnvVars(p.Spec.Containers, defaultDirEnvVar)
+	return p, true, nil
+}
 
-		// Sets an empty prefix to let the containers know the file names will match the secret keys as-is.
-		prefixEnvVar := corev1.EnvVar{
-			Name:  SecretPathFilePrefixEnvVar,
-			Value: "",
-		}
+func NewK8sSecretsInjector() K8sSecretInjector {
+	return K8sSecretInjector{}
+}
 
-		p.Spec.InitContainers = AppendEnvVars(p.Spec.InitContainers, prefixEnvVar)
-		p.Spec.Containers = AppendEnvVars(p.Spec.Containers, prefixEnvVar)
-	case core.Secret_ENV_VAR:
-		envVar := CreateEnvVarForSecret(secret)
-		p.Spec.InitContainers = AppendEnvVars(p.Spec.InitContainers, envVar)
-		p.Spec.Containers = AppendEnvVars(p.Spec.Containers, envVar)
+func InjectSecretAsFile(p *corev1.Pod, secret *core.Secret, mountPath *string) {
+	// Inject a Volume that to the pod and all of its containers and init containers that mounts the secret into a
+	// file.
+	volume := CreateVolumeForSecret(secret)
+	p.Spec.Volumes = AppendVolume(p.Spec.Volumes, volume)
 
-		prefixEnvVar := corev1.EnvVar{
-			Name:  SecretEnvVarPrefix,
-			Value: K8sDefaultEnvVarPrefix,
-		}
+	// Mount the secret to all containers in the given pod.
+	mount := CreateVolumeMountForSecret(volume.Name, secret)
+	if mountPath != nil {
+		mount = CreateVolumeMountForSecretWithMountPath(volume.Name, secret, *mountPath)
+	}
+	p.Spec.InitContainers = AppendVolumeMounts(p.Spec.InitContainers, mount)
+	p.Spec.Containers = AppendVolumeMounts(p.Spec.Containers, mount)
 
-		p.Spec.InitContainers = AppendEnvVars(p.Spec.InitContainers, prefixEnvVar)
-		p.Spec.Containers = AppendEnvVars(p.Spec.Containers, prefixEnvVar)
-	default:
-		err := fmt.Errorf("unrecognized mount requirement [%v] for secret [%v]", secret.MountRequirement.String(), secret.Key)
-		logger.Error(ctx, err)
-		return p, false, err
+	// Set environment variable to let the container know where to find the mounted files.
+	defaultDirEnvVar := corev1.EnvVar{
+		Name:  SecretPathDefaultDirEnvVar,
+		Value: filepath.Join(K8sSecretPathPrefix...),
 	}
 
-	return p, true, nil
+	p.Spec.InitContainers = AppendEnvVars(p.Spec.InitContainers, defaultDirEnvVar)
+	p.Spec.Containers = AppendEnvVars(p.Spec.Containers, defaultDirEnvVar)
+
+	// Sets an empty prefix to let the containers know the file names will match the secret keys as-is.
+	prefixEnvVar := corev1.EnvVar{
+		Name:  SecretPathFilePrefixEnvVar,
+		Value: "",
+	}
+
+	p.Spec.InitContainers = AppendEnvVars(p.Spec.InitContainers, prefixEnvVar)
+	p.Spec.Containers = AppendEnvVars(p.Spec.Containers, prefixEnvVar)
 }
 
-func NewK8sSecretsInjector() K8sSecretInjector {
-	return K8sSecretInjector{}
+func InjectSecretAsEnvVar(p *corev1.Pod, secret *core.Secret, envVarName *string) {
+	envVar := CreateEnvVarForSecret(secret)
+	if envVarName != nil {
+		envVar = CreateNamedEnvVarForSecret(secret, *envVarName)
+	}
+
+	p.Spec.InitContainers = AppendEnvVars(p.Spec.InitContainers, envVar)
+	p.Spec.Containers = AppendEnvVars(p.Spec.Containers, envVar)
+
+	prefixEnvVar := corev1.EnvVar{
+		Name:  SecretEnvVarPrefix,
+		Value: K8sDefaultEnvVarPrefix,
+	}
+
+	p.Spec.InitContainers = AppendEnvVars(p.Spec.InitContainers, prefixEnvVar)
+	p.Spec.Containers = AppendEnvVars(p.Spec.Containers, prefixEnvVar)
 }