[SPARK-58926] Driver pod SPARK_USER should reflect --proxy-user

[otterc]

What changes were proposed in this pull request?

When the operator builds the driver pod spec by invoking Spark's driver feature steps directly, BasicDriverFeatureStep sets SPARK_USER via Utils.getCurrentUserName(). In spark-submit, SparkSubmit wraps runMain in proxyUser.doAs(...), so that call returns the proxy user; the operator has no equivalent wrapper, so SPARK_USER ends up as the operator's identity rather than the effective one. This affects SparkContext.sparkUser, Spark UI ACLs, and external authorization integrations that key off SPARK_USER.

Override SPARK_USER on the driver container after the feature steps run when a proxy user is configured on the driver conf.

Why are the changes needed?

Explained above.

Does this PR introduce any user-facing change?

Yes.

How was this patch tested?

Unit tests

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Claude Opus 4.7)

[otterc]

@dongjoon-hyun Could you please help review?

[dongjoon-hyun]

Thank you for working on this, @otterc. The fix direction looks right — the operator indeed has no proxyUser.doAs(...) equivalent, so BasicDriverFeatureStep stamps the operator's identity. I verified the change against Spark's sources and found a few issues worth addressing before merging.

1. Re-appending SPARK_USER at the end of the env list breaks $(SPARK_USER) references (bug)

overrideSparkUserForProxyUser does removeMatchingFromEnv + addNewEnv, which moves SPARK_USER from the front of the driver container's env list (where BasicDriverFeatureStep puts it, before driverCustomEnvs) to the end (after custom envs and the SPARK_CONF_DIR entry). Kubernetes resolves $(VAR) references only against variables defined earlier in the list, so e.g.:

proxyUser: alice
sparkConf:
  spark.kubernetes.driverEnv.HADOOP_USER_NAME: $(SPARK_USER)

previously resolved to the user value, but now HADOOP_USER_NAME becomes the literal string $(SPARK_USER). Consider replacing the value in place (or inserting at index 0 via addToEnv(0, ...)) instead of remove+append, to preserve the position BasicDriverFeatureStep established.

2. Explicit spark.kubernetes.driverEnv.SPARK_USER is silently discarded (behavior divergence)

removeMatchingFromEnv deletes all SPARK_USER entries, including one the user explicitly set via spark.kubernetes.driverEnv.SPARK_USER. In spark-submit, the feature step's automatic SPARK_USER comes first and driverCustomEnvs are appended after, so an explicit override wins (kubelet last-entry-wins) even with --proxy-user. After this PR the proxy user always wins on the driver — while spark.executorEnv.SPARK_USER can still override executors, so driver and executors can end up with mismatched identities with no way to align them. If "proxy user wins" is the intended semantics, it deserves a note in the javadoc/docs.

3. Kerberos delegation tokens still use the operator's identity (known limitation worth documenting)

The env patch covers only SPARK_USER. KerberosConfDriverFeatureStep (part of the stock KubernetesDriverBuilder feature list invoked by SparkAppSubmissionWorker) still calls UserGroupInformation.getCurrentUser().getCredentials() and obtains delegation tokens in the operator JVM — i.e., as the operator principal, not the proxy user. For a kerberized app with proxyUser set, this diverges from spark-submit's doAs. Fine to keep out of scope here, but please document it as a limitation (or file a follow-up JIRA).

4. Javadoc: doAs alone would not have fixed this

The javadoc frames the missing doAs as the root cause, but Utils.getCurrentUserName() prefers the SPARK_USER env var over UGI, and the default Helm chart exports SPARK_USER=spark into the operator container (values.yaml). So wrapping buildFromFeatures in doAs would still yield spark in default deployments. One extra sentence in the javadoc would prevent a future "clean up into doAs" refactor from silently regressing this.

Minor (tests)

• The two new tests duplicate ~90% of their body, and the 3-line mockConf setup is now repeated in all three tests — a shared helper (matching the existing buildBasicContainer/buildBasicPod style) or a @ParameterizedTest would reduce this.
• getContainers().get(1) selects the driver container by position; filtering by container name would survive ordering changes.
• New test code uses .collect(Collectors.toList()) (plus a new import) where the repo's newer code uses Stream.toList() (Java 21 target).

[otterc]

@dongjoon-hyun Thank you for the thorough review. I have address the comments.

[dongjoon-hyun]

I reviewed this PR and compared it against the upstream one-liner, apache/spark#58190. I'd prefer that we land the fix there instead.

The root cause is that BasicDriverFeatureStep ignores conf.proxyUser and stamps Utils.getCurrentUserName() into ENV_SPARK_USER. Fixing that one line upstream covers the operator as a side effect, and it also fixes a case this PR cannot reach: Utils.getCurrentUserName() prefers the SPARK_USER environment variable over the UGI short name, so a plain bin/spark-submit --proxy-user alice launched from an environment that exports SPARK_USER produces the same wrong value, even though SparkSubmit wraps runMain in proxyUser.doAs(...). The doAs wrapper never gets a chance to matter there. So this is not operator-specific.

One data point in favor of the upstream fix being sufficient: BasicExecutorFeatureStep evaluates Utils.getCurrentUserName() inside the driver JVM, so executors inherit the driver container's SPARK_USER. Correcting the driver env alone is enough to make driver and executor identity agree.

On the operator side, this is ~60 lines of post-processing plus a 35-line Javadoc on a private method, and parts of it are not quite right:

• The !replaced fallback branch is unreachable. BasicDriverFeatureStep unconditionally adds SPARK_USER before the operator ever sees the pod, so that branch exists only for the mocked unit tests and is not covered by them either.
• The early return on environment().contains(ENV_SPARK_USER) is justified in the test comment as preventing driver/executor identity divergence, but that is not what happens. When a user sets spark.kubernetes.driverEnv.SPARK_USER=carol, the container ends up with two SPARK_USER entries (the feature step's, then the custom one), the later one wins at runtime, and executors inherit carol regardless of whether we patch the first entry. The sparkUserValue() helper asserts SPARK_USER appears exactly once, which the mock-built container satisfies but a real feature-step container would not.

All of this disappears once the upstream change is in.

@otterc Could we close this in favor of reopening apache/spark#58190?

[otterc]

@otterc Could we close this in favor of reopening apache/spark#58190?

Sure. You are right.