ivandika3 commented on code in PR #8733:
URL: https://github.com/apache/ozone/pull/8733#discussion_r2202684980


##########
hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/ProxyServerIntegrationTest.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.s3;
+
+import static 
org.apache.ozone.test.GenericTestUtils.PortAllocator.localhostWithFreePort;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Scanner;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.servlet.ServletHolder;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Integration test class to verify the routing and direct access functionality
+ * of the ProxyServer.
+ */
+public class ProxyServerIntegrationTest {
+  private static final int NUM_SERVERS = 3;
+  private static final String SERVICE_PATH = "/service-name";
+
+  private static final List<Server> SERVERS = new ArrayList<>();
+  private static final List<String> ENDPOINTS = new ArrayList<>();
+  private static ProxyServer proxy;
+  private static String proxyUrl;
+
+  @BeforeAll
+  public static void setup() throws Exception {
+    for (int i = 0; i < NUM_SERVERS; i++) {
+      startMockServer("server-" + i);
+    }
+    Thread.sleep(1000);

Review Comment:
   Nit: Try to avoid sleep if possible, since this might cause the tests to be 
flaky. 
   
   I'm OK to leave it be for now.



##########
hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/ProxyServer.java:
##########
@@ -0,0 +1,210 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.s3;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.List;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletRequestWrapper;
+import javax.servlet.http.HttpServletResponse;
+import org.eclipse.jetty.proxy.ProxyServlet;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.servlet.ServletHolder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * ProxyServer acts as a reverse proxy for S3G endpoints.
+ * It forwards requests to the selected S3G endpoint based on the load 
balancing strategy.
+ */
+public class ProxyServer {
+  private static final Logger LOG = LoggerFactory.getLogger(ProxyServer.class);
+  private final List<String> s3gEndpoints;
+  private final LoadBalanceStrategy loadBalanceStrategy;
+  private final Server server;
+  private final String host;
+  private final int port;
+
+  public ProxyServer(List<String> s3gEndpoints, String host, int proxyPort) 
throws Exception {
+    this(s3gEndpoints, host, proxyPort, new RoundRobinStrategy());
+  }
+
+  public ProxyServer(List<String> s3gEndpoints, String host, int proxyPort,
+                     LoadBalanceStrategy loadBalanceStrategy) throws Exception 
{
+    this.s3gEndpoints = s3gEndpoints;
+    this.loadBalanceStrategy = loadBalanceStrategy;
+    this.host = host;
+    this.port = proxyPort;
+
+    server = new Server(proxyPort);
+    ServletContextHandler context = new ServletContextHandler();
+    context.setContextPath("/");
+
+    ProxyHandler proxyHandler = new ProxyHandler();
+    ServletHolder proxyHolder = new ServletHolder(proxyHandler);
+
+    proxyHolder.setInitParameter("proxyTo", "");
+
+
+    context.addServlet(proxyHolder, "/*");
+    server.setHandler(context);
+
+    LOG.info("ProxyServer initialized with endpoints: {}", s3gEndpoints);
+    LOG.info("Load balance strategy: {}", 
loadBalanceStrategy.getClass().getSimpleName());
+  }
+
+  public void start() throws Exception {
+    server.start();
+    LOG.info("Proxy started on http://{}:{}";, host, port);
+  }
+
+  public void stop() throws Exception {
+    server.stop();
+    LOG.info("Proxy stopped on http://{}:{}";, host, port);
+  }
+
+  /**
+   * ProxyHandler is a subclass of Jetty's ProxyServlet.Transparent.
+   * It implements logic for request rewriting, service handling,
+   * proxy response failure, and rewrite failure handling.
+   * This handler is mainly used to forward requests to different S3G endpoints
+   * based on the load balancing strategy, handle special HTTP headers (such 
as Expect),
+   * and manage exceptions during the proxy process.
+   */
+  public class ProxyHandler extends ProxyServlet.Transparent {
+
+    @Override
+    public void init() throws ServletException {
+      super.init();
+
+      LOG.info("MyProxyHandler initialized with {} endpoints",
+          s3gEndpoints != null ? s3gEndpoints.size() : 0);
+      if (s3gEndpoints != null) {
+        LOG.info("Endpoints: {}", s3gEndpoints);
+      }
+      LOG.info("Load balance strategy: {}", 
loadBalanceStrategy.getClass().getSimpleName());
+    }
+
+    @Override
+    protected String rewriteTarget(HttpServletRequest request) {
+
+      String baseUrl = loadBalanceStrategy.selectEndpoint(s3gEndpoints);
+
+      String requestUri = request.getRequestURI();
+      String queryString = request.getQueryString();
+
+      StringBuilder targetUrl = new StringBuilder(baseUrl);
+      if (requestUri != null) {
+        targetUrl.append(requestUri);
+      }
+      if (queryString != null) {
+        targetUrl.append('?').append(queryString);
+      }
+
+      String finalUrl = targetUrl.toString();
+      LOG.info("Rewriting target URL: [{}] {}", request.getMethod(), finalUrl);
+      return finalUrl;
+    }
+
+    @Override
+    protected void service(HttpServletRequest request, HttpServletResponse 
response)
+        throws ServletException, IOException {
+
+      // Remove the Expect header to avoid Jetty's 100-continue handling issue:
+      // In some scenarios (e.g. testPutObjectEmpty), when content-length != 
0, Jetty triggers the 100-continue process,
+      // causing the server to wait for a 100 response code. However, Jetty 
only sends the 100 response
+      // when the InputStream is read, which can lead to request failures.
+      if (request.getHeader("Expect") != null) {

Review Comment:
   Nit: Replace all the `Expect` with a constant (the http dependencies should 
have one).



-- 
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: issues-unsubscr...@ozone.apache.org

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


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

Reply via email to