github-actions[bot] commented on code in PR #67851:
URL: https://github.com/apache/doris/pull/67851#discussion_r3990174501
##########
fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/StreamingJobAction.java:
##########
@@ -18,26 +18,56 @@
package org.apache.doris.httpv2.rest;
import org.apache.doris.catalog.Env;
+import org.apache.doris.datasource.InternalCatalog;
import org.apache.doris.httpv2.entity.ResponseEntityBuilder;
+import org.apache.doris.httpv2.exception.BadRequestException;
import org.apache.doris.httpv2.exception.UnauthorizedException;
import org.apache.doris.job.base.AbstractJob;
import org.apache.doris.job.cdc.request.CommitOffsetRequest;
import org.apache.doris.job.cdc.request.TaskFailureRequest;
import org.apache.doris.job.extensions.insert.streaming.StreamingInsertJob;
+import org.apache.doris.mysql.privilege.PrivPredicate;
+import org.apache.doris.qe.AutoCloseConnectContext;
+import org.apache.doris.qe.ConnectContext;
+import org.apache.doris.qe.QueryState;
+import org.apache.doris.qe.StmtExecutor;
import com.google.common.base.Strings;
import jakarta.servlet.http.HttpServletRequest;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
+import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod;
import org.springframework.web.bind.annotation.RestController;
+import java.util.Map;
+
@RestController
public class StreamingJobAction extends RestBaseController {
private static final Logger LOG =
LogManager.getLogger(StreamingJobAction.class);
+ private final TableSchemaAction tableSchemaAction;
+
+ public StreamingJobAction(TableSchemaAction tableSchemaAction) {
+ this.tableSchemaAction = tableSchemaAction;
+ }
+
+ @RequestMapping(path = "/api/streaming/schema/{" + DB_KEY + "}/{" +
TABLE_KEY + "}",
Review Comment:
[P2] Keep the cluster-token endpoint inventory exhaustive
Both new routes authenticate with the cluster token and intentionally accept
no user password, but the canonical `threat-model.md` contract still enumerates
only `commit_offset` and `report_task_failure`. Section 4.8(11) says that list
is exhaustive and that an omitted qualifying path is a documentation defect.
Please add both schema routes to the per-endpoint trust table, the
cluster-token exclusion, and the corresponding false-positive example so future
reviews classify absent/invalid tokens versus valid-token calls correctly.
##########
fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/StreamingJobAction.java:
##########
@@ -50,14 +80,69 @@ public Object reportTaskFailure(@RequestBody
TaskFailureRequest failureRequest,
return failTask(failureRequest);
}
+ @RequestMapping(path = "/api/streaming/schema_change", method =
RequestMethod.POST)
+ public Object executeSchemaChange(@RequestBody Map<String, String> body,
HttpServletRequest request) {
+ checkAuth(request);
+ if (!Env.getCurrentEnv().isMaster()) {
+ return ResponseEntityBuilder.okWithCommonError("Schema change must
be executed on the master FE");
+ }
+ String stmt = body.get("stmt");
+ if (Strings.isNullOrEmpty(stmt)) {
+ return ResponseEntityBuilder.badRequest("Missing statement request
body");
+ }
+
+ ConnectContext ctx = createJobContext(request);
+ try (AutoCloseConnectContext ignored = new
AutoCloseConnectContext(ctx)) {
+ StmtExecutor executor = new StmtExecutor(ctx, stmt);
+ executor.execute();
+ if (ctx.getState().getStateType() ==
QueryState.MysqlStateType.ERR) {
+ return
ResponseEntityBuilder.okWithCommonError(ctx.getState().getErrorMessage());
+ }
+ return ResponseEntityBuilder.ok();
+ } catch (Exception e) {
+ LOG.warn("Failed to execute schema change", e);
+ return ResponseEntityBuilder.okWithCommonError(e.getMessage());
+ }
+ }
+
private void checkAuth(HttpServletRequest request) {
String authToken = request.getHeader("token");
if (Strings.isNullOrEmpty(authToken)) {
throw new UnauthorizedException("Miss token");
}
if (!checkClusterToken(authToken)) {
- throw new UnauthorizedException("Invalid token: " + authToken);
+ throw new UnauthorizedException("Invalid token");
+ }
+ }
+
+ // Call only after validating the internal token. The caller owns the
context's scope.
+ private static ConnectContext createJobContext(HttpServletRequest request)
{
+ String jobIdHeader = request.getHeader("jobId");
+ if (Strings.isNullOrEmpty(jobIdHeader)) {
+ throw new BadRequestException("Missing jobId header; CDC client
must send the streaming job ID");
+ }
+ long jobId;
+ try {
+ jobId = Long.parseLong(jobIdHeader);
+ } catch (NumberFormatException e) {
+ throw new BadRequestException("Invalid jobId header: " +
jobIdHeader);
+ }
+ AbstractJob job = Env.getCurrentEnv().getJobManager().getJob(jobId);
+ if (!(job instanceof StreamingInsertJob)) {
+ throw new BadRequestException("Job " + jobId + " is not a
streaming job or does not exist");
+ }
+ if (job.getCreateUser() == null) {
+ throw new BadRequestException("Streaming job " + jobId + " has no
creator identity");
+ }
+ ConnectContext ctx = new ConnectContext();
+ ctx.setEnv(Env.getCurrentEnv());
+ ctx.setRemoteIP(request.getRemoteAddr());
Review Comment:
[P1] Do not turn the CDC peer into the creator's client IP
This context runs a background job, but setting `remoteIP` to
`request.getRemoteAddr()` makes both the SHOW check and `AlterTableCommand`
pass the BE-hosted CDC client's address into `ConnectionAccessContext`. Ranger
explicitly treats background jobs as having no client IP and falls back to the
persisted subject host; with an IP-conditioned policy, a job authorized when
created can therefore fail its first schema event (or be evaluated against
policy for the unrelated BE address). Leave the background context's remote IP
unset, or restore a deliberately persisted creator-origin context, and cover
SHOW plus ALTER under an IP-conditioned Ranger policy.
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/insert/streaming/StreamingMultiTblTask.java:
##########
@@ -223,6 +223,7 @@ private WriteRecordRequest buildRequestParams() throws
JobException {
request.setDataSource(dataSourceType.name());
request.setTaskId(getTaskId() + "");
request.setToken(getToken());
+ request.setDorisUser(getUserIdentity().getQualifiedUser());
Review Comment:
[P1] Preserve the creator's host-qualified identity for stream load
`getQualifiedUser()` drops the host/domain part of the persisted creator
identity. After BE forwards this Basic username, `StreamLoadHandler`
reconstructs `user@%`; with workload groups enabled, the group is selected by
username but its USAGE privilege is checked against that reconstructed
identity. A valid owner such as `alice@10.%` therefore loses the roles keyed to
the full `UserIdentity` and every CDC batch can fail planning. Bind the token
request to the job/task and restore the persisted identity on FE (which also
enables the current-LOAD check), or carry a lossless authenticated identity;
add a host-scoped custom-workload-group case.
--
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]