morningman commented on code in PR #26278:
URL: https://github.com/apache/doris/pull/26278#discussion_r1380212205


##########
fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/LoadAction.java:
##########
@@ -257,4 +269,97 @@ private TNetworkAddress selectRedirectBackend(String 
clusterName) throws LoadExc
         }
         return new TNetworkAddress(backend.getHost(), backend.getHttpPort());
     }
+
+    // NOTE: This function can only be used for AuditlogPlugin stream load for 
now.
+    // AuditlogPlugin should be re-disigned carefully, and blow method focuses 
on
+    // temporarily addressing the users' needs for audit logs.
+    // So this function is not widely tested under general scenario
+    private boolean checkClusterToken(HttpServletRequest request) {
+        LOG.info("Checking cluser token, request {}", request.toString());

Review Comment:
   ```suggestion
           LOG.debug("Checking cluser token, request {}", request.toString());
   ```



##########
fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/LoadAction.java:
##########
@@ -257,4 +269,97 @@ private TNetworkAddress selectRedirectBackend(String 
clusterName) throws LoadExc
         }
         return new TNetworkAddress(backend.getHost(), backend.getHttpPort());
     }
+
+    // NOTE: This function can only be used for AuditlogPlugin stream load for 
now.
+    // AuditlogPlugin should be re-disigned carefully, and blow method focuses 
on
+    // temporarily addressing the users' needs for audit logs.
+    // So this function is not widely tested under general scenario
+    private boolean checkClusterToken(HttpServletRequest request) {
+        LOG.info("Checking cluser token, request {}", request.toString());
+        String authToken = request.getHeader("token");
+
+        if (Strings.isNullOrEmpty(authToken)) {
+            LOG.warn("Failed to check cluster token.");

Review Comment:
   I think this is not a warn.
   In normal case, if user submit load with wrong username/password, the 
`authToken` is always empty.
   So I think we can remove this log



##########
fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/LoadAction.java:
##########
@@ -257,4 +269,97 @@ private TNetworkAddress selectRedirectBackend(String 
clusterName) throws LoadExc
         }
         return new TNetworkAddress(backend.getHost(), backend.getHttpPort());
     }
+
+    // NOTE: This function can only be used for AuditlogPlugin stream load for 
now.
+    // AuditlogPlugin should be re-disigned carefully, and blow method focuses 
on
+    // temporarily addressing the users' needs for audit logs.
+    // So this function is not widely tested under general scenario
+    private boolean checkClusterToken(HttpServletRequest request) {
+        LOG.info("Checking cluser token, request {}", request.toString());
+        String authToken = request.getHeader("token");
+
+        if (Strings.isNullOrEmpty(authToken)) {
+            LOG.warn("Failed to check cluster token.");
+            return false;
+        }
+
+        return 
Env.getCurrentEnv().getLoadManager().getTokenManager().checkAuthToken(authToken);
+    }
+
+    // NOTE: This function can only be used for AuditlogPlugin stream load for 
now.
+    // AuditlogPlugin should be re-disigned carefully, and blow method focuses 
on
+    // temporarily addressing the users' needs for audit logs.
+    // So this function is not widely tested under general scenario
+    private Object executeWithClusterToken(HttpServletRequest request, String 
db,
+                                          String table, boolean isStreamLoad) {
+        try {
+            ConnectContext ctx = new ConnectContext();
+            ctx.setEnv(Env.getCurrentEnv());
+            ctx.setThreadLocalInfo();
+            ctx.setCluster(SystemInfoService.DEFAULT_CLUSTER);
+            ctx.setRemoteIP(request.getRemoteAddr());
+
+            String dbName = db;
+            String tableName = table;
+            // A 'Load' request must have 100-continue header
+            if (request.getHeader(HttpHeaderNames.EXPECT.toString()) == null) {
+                return new RestBaseResult("There is no 100-continue header");
+            }
+
+            final String clusterName = ConnectContext.get().getClusterName();
+            if (Strings.isNullOrEmpty(clusterName)) {
+                return new RestBaseResult("No cluster selected.");
+            }
+
+            if (Strings.isNullOrEmpty(dbName)) {
+                return new RestBaseResult("No database selected.");
+            }
+
+            if (Strings.isNullOrEmpty(tableName)) {
+                return new RestBaseResult("No table selected.");
+            }
+
+            String label = request.getParameter(LABEL_KEY);
+            if (isStreamLoad) {
+                label = request.getHeader(LABEL_KEY);
+            }
+
+            if (!isStreamLoad && Strings.isNullOrEmpty(label)) {
+                // for stream load, the label can be generated by system 
automatically
+                return new RestBaseResult("No label selected.");
+            }
+
+            TNetworkAddress redirectAddr = selectRedirectBackend(clusterName);
+
+            LOG.info("Redirect load action with auth token to destination={},"
+                        + "stream: {}, db: {}, tbl: {}, label: {}",
+                    redirectAddr.toString(), isStreamLoad, dbName, tableName, 
label);
+
+            URI urlObj = null;
+            URI resultUriObj = null;
+            String urlStr = request.getRequestURI();
+            String userInfo = null;
+
+            try {
+                urlObj = new URI(urlStr);
+                resultUriObj = new URI("http", userInfo, 
redirectAddr.getHostname(),
+                        redirectAddr.getPort(), urlObj.getPath(), "", null);
+            } catch (Exception e) {
+                throw new RuntimeException(e);
+            }
+            String redirectUrl = resultUriObj.toASCIIString();
+            if (!Strings.isNullOrEmpty(request.getQueryString())) {
+                redirectUrl += request.getQueryString();
+            }
+            LOG.info("Redirect url: {}", redirectUrl);
+            RedirectView redirectView = new RedirectView(redirectUrl);
+            redirectView.setContentType("text/html;charset=utf-8");
+            
redirectView.setStatusCode(org.springframework.http.HttpStatus.TEMPORARY_REDIRECT);
+
+            return redirectView;
+        } catch (Exception e) {
+            LOG.error("Failed to execute stream load with cluster token, {}", 
e);

Review Comment:
   ```suggestion
               LOG.warn("Failed to execute stream load with cluster token, {}", 
e);
   ```



-- 
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: commits-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to