[BUG] [Spark plugin] Environment variables in the pod_template that use ValueFrom don't work

[Tom-Newton]

Describe the bug

If we have a task like this

@task(
    task_config=Spark(spark_conf={"spark.driver.memory": "1000M"}),
    pod_template=PodTemplate(
        pod_spec=client.V1PodSpec(
            containers=[
                client.V1Container(
                    name="primary",
                    env=[
                        client.V1EnvVar(
                            name="SECRET_ENV_VAR",
                            value_from=client.V1EnvVarSource(
                                secret_key_ref=client.V1SecretKeySelector(
                                    name="name-in-k8s-key-vault", key="name-of-key-within-secret"
                                )
                            ),
                        )
                    ],
                ),
            ],
        ),
    ),
)
def spark_task() -> str:
    ...

This results in the resulting SparkApplication spec

    - name: `SECRET_ENV_VAR`

SECRET_ENV_VAR is set to empty string.

Expected behavior

The SparkApplication spec should look like this

    - name: `SECRET_ENV_VAR`
      valueFrom:
        secretKeyRef:
          key: name-of-key-within-secret
          name: name-in-k8s-key-vault

Additional context to reproduce

I have a change to the unittests which can catch this.

The bug seems to be in how the pod_template is converted into a spark application. Specifically I think the problem comes from createSparkPodSpec where we just take the Value of the corev1.EnvVar which is expected to be empty when using ValueFrom.

This is done because currently we try to funnel the []EnvVar from Container.Env into map[string]string at SparkPodSpec.EnvVars. I propose we instead put these values in SparkPodSpec.Env which has type []EnvVar meaning no type conversion is required and it will support all features of corev1.EnvVar.

I can make a PR with this change if my proposal seems sensible.

Screenshots

No response

Are you sure this issue hasn't been raised already?

• Yes

Have you read the Code of Conduct?

• Yes

[dosubot]

🤖

It's great to see that you've identified the issue and even proposed a solution along with a potential code change. Your contribution would definitely be valuable to the flyte community. I encourage you to go ahead and create a PR with your proposed changes. Thank you for your willingness to contribute!

---

This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

[hamersaw]

@tom as you suggested using the SparkPodSpec.Env rather than SparkPodSpec.EnvVars sounds like the right idea to me. Especially as the later is technically deprecated. Please feel free to open a PR with your suggested changes!