dongjoon-hyun commented on PR #803:
URL:
https://github.com/apache/spark-kubernetes-operator/pull/803#issuecomment-5443862665
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.:
```yaml
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).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
