[SPARK-59118][SQL] Error when doAs is used and not supported - #58414

The guard's comment calls out that authType is null when the auth is explicitly set empty and that the constant-on-left comparison must not NPE. That branch isn't exercised — the closest case here uses a non-null unknown string. A case like check(hiveConf("", doAs = true)) asserting it fails closed without an NPE would lock in the null-safety you flagged.

Defaulting this to false means the Thrift Server refuses to start for any existing hive.server2.enable.doAs=true + verifying-auth (Kerberos/LDAP) deployment once it upgrades to 5.0. The SPARK-5159 rationale is sound, and the escape hatch + migration-guide entry + the 4.x warn-first #58438 are the right scaffolding. The open question is whether refuse-to-start is the right default for 5.0: it turns a long-latent, silently-"working" config into an immediate startup outage on upgrade. If the hard-fail default is intended, it's worth a prominent release-note callout (beyond the migration guide) given how many secured HS2 deployments run doAs=true; if not, a loud startup warning that only escalates to a hard error under an explicit opt-in would be less disruptive while still closing the security gap.

This is a user-facing error, so it should go through the error-class framework rather than a raw IllegalArgumentException with a prose message (common/utils/src/main/resources/error/README.md: "developers should specify a standardized SQLSTATE, an error condition, and message parameters rather than an arbitrary error message"). The sibling startup check in SparkSQLCLIService.init already does this via QueryExecutionErrors.invalidKerberosConfigForHiveServer2Error. Could you add an error condition to error-conditions.json and throw it via QueryExecutionErrors / HiveThriftServerErrors, so the exception is a SparkThrowable with a SQLSTATE and can be asserted with checkError?

"queries and the storage access they perform still run as the server's own service identity" is only true on the executor side. HiveSessionProxy wraps executeStatementAsync in the session UGI's doAs, so SparkExecuteStatementOperation.runInternal calls Utils.getUGI() while already impersonating the connecting user, and the background action runs under that proxy UGI on the driver. Driver-side FileSystem calls (e.g. InMemoryFileIndex listing) are therefore impersonated; only executor task I/O runs as the service principal. Since operators will read this text (and the conf doc / migration note, which repeat it) to decide whether to set allowIneffectiveDoAs=true, it would be better to say executor-side data access is not impersonated rather than "queries" as a whole.

The comment above says NONE/NOSASL "have nothing to impersonate", but that is not what Hive does. ThriftCLIService.getSessionHandle opens the session with impersonation whenever doAs is true and a username exists, and under NONE getUserName falls back to the client-supplied req.getUsername(). So with NONE + doAs=true (Hive's out-of-the-box config) metastore calls are impersonated as an unverified, client-chosen name while storage still runs as the service principal, which is the same SPARK-5159 gap this guard describes. Exempting NONE may still be the right call (there is no security expectation without a verified identity), but the rationale should say that rather than "nothing to impersonate". Alternatively, keying on doAs alone or on UserGroupInformation.isSecurityEnabled would avoid a hand-maintained AuthTypes list.

I don't think authType can be null here. HiveConf.getVar returns "" when the value is explicitly set empty and the default NONE for an empty <value/> in hive-site.xml (Hadoop Configuration.get(name, default) semantics), and Configuration.set(name, null) is rejected. So the constant-on-the-left is harmless but the comment is misleading. Separately, an empty or misspelled auth type is classified as "verifies user" and reported as a doAs problem; following the advice to set allowIneffectiveDoAs=true then does not help, because HiveAuthFactory.getAuthTransFactory fails with Unsupported authentication type. Either correct the comment, or leave invalid auth types to Hive's own error.

ThriftServerWithSparkContextInBinarySuite / InHttpSuite already construct a HiveThriftServer2 in-process via SharedThriftServer.startWithSparkSession, and they run in the default test group without a dedicated JVM. If the static loadHiveServer2Config flip were a real hazard, those suites would need this too. Unless there is an observed failure, I'd drop this entry so we don't fork an extra JVM for a suite of eight short tests.

Only the last test (init refuses to start the server) uses spark; the other seven call failIfIneffectiveDoAs directly on a HiveConf, and the branch-4.x sibling in #58438 uses SparkFunSuite for the same cases. Could we keep this suite on SparkFunSuite and move the single init() wiring test into an existing session-backed thrift-server suite? That avoids paying SharedSparkSession setup for pure-function tests and keeps the 4.x and master suites aligned.