Skip to content

Commit 51fdfd3

Browse files
committed
fix but encountering 400 and 403 for running rex explainIT
Signed-off-by: Jialiang Liang <jiallian@amazon.com>
1 parent ca23856 commit 51fdfd3

3 files changed

Lines changed: 158 additions & 38 deletions

File tree

integ-test/src/test/java/org/opensearch/sql/AwsHttpRequestInterceptor.java

Lines changed: 117 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
import com.amazonaws.auth.AWS4Signer;
1111
import com.amazonaws.auth.BasicSessionCredentials;
1212
import com.amazonaws.http.HttpMethodName;
13-
import java.io.ByteArrayInputStream;
1413
import java.io.IOException;
1514
import java.net.URI;
1615
import java.nio.charset.StandardCharsets;
@@ -36,13 +35,17 @@ public class AwsHttpRequestInterceptor implements HttpRequestInterceptor {
3635

3736
// Hardcoded AWS credentials for AOSS (temporary approach)
3837
// TODO : Replace the following values
39-
private static final String ACCESS_KEY = "<access_key>";
40-
private static final String SECRET_KEY = "<secret_key>";
41-
private static final String SESSION_TOKEN = "<session_token>";
38+
private static final String ACCESS_KEY = "";
39+
private static final String SECRET_KEY = "";
40+
private static final String SESSION_TOKEN = "";
4241

4342
private static final String REGION = "us-east-1";
4443
private static final String SERVICE = "aoss";
45-
private static final String AOSS_ENDPOINT = "<aoss_endpoint>";
44+
private static final String AOSS_ENDPOINT = "";
45+
46+
// AOSS-specific headers
47+
private static final String AOSS_ACCOUNT_ID = "";
48+
private static final String AOSS_COLLECTION_ID = ""; // Extracted from endpoint
4649

4750
private final AWS4Signer signer;
4851
private final BasicSessionCredentials credentials;
@@ -60,25 +63,111 @@ public void process(HttpRequest request, EntityDetails entity, HttpContext conte
6063
throws HttpException, IOException {
6164

6265
try {
63-
// Create AWS request from Apache HttpClient request
64-
Request<Void> awsRequest = createAwsRequest(request, entity);
66+
// Create AWS request for signing without touching the original request entity
67+
Request<Void> awsRequest = createSigningRequest(request);
6568

6669
// Sign the request
6770
signer.sign(awsRequest, credentials);
6871

69-
// Copy signed headers back to the original request
70-
awsRequest
71-
.getHeaders()
72-
.forEach(
73-
(name, value) -> {
74-
request.setHeader(name, value);
75-
});
72+
// Copy only the AWS authentication headers - never touch request body
73+
String authorization = awsRequest.getHeaders().get("Authorization");
74+
String amzDate = awsRequest.getHeaders().get("x-amz-date");
75+
String amzSecurityToken = awsRequest.getHeaders().get("x-amz-security-token");
76+
77+
if (authorization != null) {
78+
request.setHeader("Authorization", authorization);
79+
}
80+
if (amzDate != null) {
81+
request.setHeader("x-amz-date", amzDate);
82+
}
83+
if (amzSecurityToken != null) {
84+
request.setHeader("x-amz-security-token", amzSecurityToken);
85+
}
86+
87+
// Add AOSS-specific headers required for AOSS APIs
88+
request.setHeader("X-Amzn-Aoss-Account-Id", AOSS_ACCOUNT_ID);
89+
request.setHeader("X-Amzn-Aoss-Collection-Id", AOSS_COLLECTION_ID);
7690

7791
} catch (Exception e) {
7892
throw new HttpException("Failed to sign AWS request", e);
7993
}
8094
}
8195

96+
/**
97+
* Create AWS request for signing that never touches the original request entity. This completely
98+
* avoids HTTP encoding conflicts by using empty body SHA256.
99+
*/
100+
private Request<Void> createSigningRequest(HttpRequest httpRequest) throws IOException {
101+
String requestPath = httpRequest.getRequestUri();
102+
URI fullEndpoint = URI.create("https://" + AOSS_ENDPOINT + ":443");
103+
104+
Request<Void> awsRequest = new DefaultRequest<>(SERVICE);
105+
awsRequest.setHttpMethod(HttpMethodName.fromValue(httpRequest.getMethod()));
106+
awsRequest.setEndpoint(fullEndpoint);
107+
awsRequest.setResourcePath(requestPath);
108+
109+
// Required headers for signing
110+
awsRequest.addHeader("host", fullEndpoint.getHost());
111+
112+
// Always use empty body SHA256 to avoid any entity interference
113+
awsRequest.addHeader("x-amz-content-sha256", sha256Hex("".getBytes(StandardCharsets.UTF_8)));
114+
115+
// Add timestamp
116+
String timestamp =
117+
DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmss'Z'")
118+
.withZone(ZoneOffset.UTC)
119+
.format(Instant.now());
120+
awsRequest.addHeader("x-amz-date", timestamp);
121+
122+
// Add session token
123+
awsRequest.addHeader("x-amz-security-token", SESSION_TOKEN);
124+
125+
// Add AOSS-specific headers before signing so they're included in the signature
126+
awsRequest.addHeader("X-Amzn-Aoss-Account-Id", AOSS_ACCOUNT_ID);
127+
awsRequest.addHeader("X-Amzn-Aoss-Collection-Id", AOSS_COLLECTION_ID);
128+
129+
return awsRequest;
130+
}
131+
132+
private Request<Void> createMinimalAwsRequest(HttpRequest httpRequest) throws IOException {
133+
// Create a minimal AWS request for signing without copying problematic headers
134+
String requestPath = httpRequest.getRequestUri();
135+
URI fullEndpoint = URI.create("https://" + AOSS_ENDPOINT + ":443");
136+
137+
Request<Void> awsRequest = new DefaultRequest<>(SERVICE);
138+
awsRequest.setHttpMethod(HttpMethodName.fromValue(httpRequest.getMethod()));
139+
awsRequest.setEndpoint(fullEndpoint);
140+
awsRequest.setResourcePath(requestPath);
141+
142+
// Add only essential headers for signing - avoid content headers that cause conflicts
143+
awsRequest.addHeader("host", fullEndpoint.getHost());
144+
145+
// Don't add x-amz-content-sha256 for GET requests - AOSS doesn't expect it
146+
// Only add it for POST/PUT requests that have actual content
147+
148+
// Add timestamp
149+
String timestamp =
150+
DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmss'Z'")
151+
.withZone(ZoneOffset.UTC)
152+
.format(Instant.now());
153+
awsRequest.addHeader("x-amz-date", timestamp);
154+
155+
// Add session token
156+
awsRequest.addHeader("x-amz-security-token", SESSION_TOKEN);
157+
158+
// Add AOSS-specific headers before signing so they're included in the signature
159+
awsRequest.addHeader("X-Amzn-Aoss-Account-Id", AOSS_ACCOUNT_ID);
160+
awsRequest.addHeader("X-Amzn-Aoss-Collection-Id", AOSS_COLLECTION_ID);
161+
162+
// Debug logging to see what's in the AWS request before signing
163+
System.out.println("=== AWS REQUEST BEFORE SIGNING ===");
164+
System.out.println("AWS Request Path: " + awsRequest.getResourcePath());
165+
System.out.println("AWS Request Headers: " + awsRequest.getHeaders());
166+
System.out.println("==================================");
167+
168+
return awsRequest;
169+
}
170+
82171
private Request<Void> createAwsRequest(HttpRequest httpRequest, EntityDetails entity)
83172
throws IOException {
84173
// The httpRequest.getRequestUri() only contains the path, we need to construct the full URL
@@ -94,23 +183,25 @@ private Request<Void> createAwsRequest(HttpRequest httpRequest, EntityDetails en
94183
awsRequest.setResourcePath(requestPath);
95184

96185
// Copy headers from HttpClient request to AWS request
186+
// Skip content-related headers that might interfere with encoding
97187
Arrays.stream(httpRequest.getHeaders())
188+
.filter(
189+
header ->
190+
!header.getName().equalsIgnoreCase("Content-Length")
191+
&& !header.getName().equalsIgnoreCase("Content-Type")
192+
&& !header.getName().equalsIgnoreCase("Content-Encoding"))
98193
.forEach(
99194
header -> {
100195
awsRequest.addHeader(header.getName(), header.getValue());
101196
});
102197

103198
// Handle request body - for POST requests, we need to provide content hash
104-
// For AWS signing, we need the content SHA256, but we don't want to interfere with the actual
105-
// request
199+
// We ONLY set the content hash header, not the actual content to avoid encoding conflicts
106200
if ("POST".equals(httpRequest.getMethod()) || "PUT".equals(httpRequest.getMethod())) {
107-
// For PPL queries, typical body is JSON. Since we can't safely read the actual body,
108-
// we'll use a common pattern for the content hash. This is a simplification.
109-
String commonJsonBody = "{\"query\": \"source=accounts | head 10\"}";
110-
byte[] bodyBytes = commonJsonBody.getBytes(StandardCharsets.UTF_8);
201+
// For now, use empty content hash to avoid reading entity which would interfere with encoding
202+
byte[] bodyBytes = getActualEntityContent(entity);
111203

112-
awsRequest.setContent(new ByteArrayInputStream(bodyBytes));
113-
awsRequest.addHeader("Content-Length", String.valueOf(bodyBytes.length));
204+
// Only set the SHA256 hash header - do NOT set content or Content-Length on AWS request
114205
awsRequest.addHeader("x-amz-content-sha256", sha256Hex(bodyBytes));
115206
} else {
116207
// Empty body SHA256 for GET requests
@@ -133,12 +224,10 @@ private Request<Void> createAwsRequest(HttpRequest httpRequest, EntityDetails en
133224
return awsRequest;
134225
}
135226

136-
private byte[] getEntityContent(EntityDetails entity) throws IOException {
137-
// For now, assume most content is relatively small (typical for API calls)
138-
// In a full implementation, we'd handle streaming properly
139-
if (entity != null) {
140-
return "{}".getBytes(StandardCharsets.UTF_8); // Default empty JSON
141-
}
227+
private byte[] getActualEntityContent(EntityDetails entity) throws IOException {
228+
// For AWS signing we need the actual content hash, but reading the entity here
229+
// would interfere with the request encoding. For now, use empty content
230+
// This is a limitation - ideally we'd compute hash from actual request body
142231
return new byte[0];
143232
}
144233

integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
import org.opensearch.client.Response;
4242
import org.opensearch.client.RestClient;
4343
import org.opensearch.client.RestClientBuilder;
44-
import org.opensearch.common.settings.Settings;
4544
import org.opensearch.common.unit.TimeValue;
4645
import org.opensearch.common.util.concurrent.ThreadContext;
4746
import org.opensearch.common.util.io.IOUtils;
@@ -87,10 +86,17 @@ public abstract class OpenSearchSQLRestTestCase extends OpenSearchRestTestCase {
8786
private static RestClient remoteAdminClient;
8887

8988
protected boolean isHttps() {
89+
String httpsProperty = System.getProperty("https");
9090
boolean isHttps =
91-
Optional.ofNullable(System.getProperty("https"))
92-
.map("true"::equalsIgnoreCase)
93-
.orElse(false);
91+
Optional.ofNullable(httpsProperty).map("true"::equalsIgnoreCase).orElse(false);
92+
93+
// Debug logging
94+
System.out.println("=== HTTPS DETECTION DEBUG ===");
95+
System.out.println("https property: " + httpsProperty);
96+
System.out.println("isHttps: " + isHttps);
97+
System.out.println("tests.rest.cluster: " + System.getProperty("tests.rest.cluster"));
98+
System.out.println("=============================");
99+
94100
if (isHttps) {
95101
// currently only external cluster is supported for security enabled testing
96102
if (!Optional.ofNullable(System.getProperty("tests.rest.cluster")).isPresent()) {
@@ -125,7 +131,13 @@ protected static RestClient remoteAdminClient() {
125131
return remoteAdminClient;
126132
}
127133

128-
protected RestClient buildClient(Settings settings, HttpHost[] hosts) throws IOException {
134+
@Override
135+
protected RestClient buildClient(
136+
org.opensearch.common.settings.Settings settings, HttpHost[] hosts) throws IOException {
137+
System.out.println("CUSTOM buildClient() called for AOSS integration!");
138+
System.out.println("Hosts: " + java.util.Arrays.toString(hosts));
139+
140+
// Build the client directly with AOSS/HTTPS detection
129141
RestClientBuilder builder = RestClient.builder(hosts);
130142
if (isHttps()) {
131143
configureHttpsClient(builder, settings, hosts[0]);
@@ -155,7 +167,8 @@ public RestClient initClient(String clusterName) throws IOException {
155167
int port = Integer.parseInt(stringUrl.substring(portSeparator + 1));
156168
hosts.add(buildHttpHost(host, port));
157169
}
158-
Settings.Builder builder = Settings.builder();
170+
org.opensearch.common.settings.Settings.Builder builder =
171+
org.opensearch.common.settings.Settings.builder();
159172
if (System.getProperty("tests.rest.client_path_prefix") != null) {
160173
builder.put(CLIENT_PATH_PREFIX, System.getProperty("tests.rest.client_path_prefix"));
161174
}
@@ -241,7 +254,8 @@ protected static void wipeAllOpenSearchIndices(RestClient client) throws IOExcep
241254
* Configure authentication and pass <b>builder</b> to superclass to configure other stuff.<br>
242255
* By default, auth is configure when <b>https</b> is set only.
243256
*/
244-
protected static void configureClient(RestClientBuilder builder, Settings settings)
257+
protected static void configureClient(
258+
RestClientBuilder builder, org.opensearch.common.settings.Settings settings)
245259
throws IOException {
246260
String userName = System.getProperty("user");
247261
String password = System.getProperty("password");
@@ -259,7 +273,10 @@ protected static void configureClient(RestClientBuilder builder, Settings settin
259273
}
260274

261275
protected static void configureHttpsClient(
262-
RestClientBuilder builder, Settings settings, HttpHost httpHost) throws IOException {
276+
RestClientBuilder builder,
277+
org.opensearch.common.settings.Settings settings,
278+
HttpHost httpHost)
279+
throws IOException {
263280
Map<String, String> headers = ThreadContext.buildDefaultHeaders(settings);
264281
Header[] defaultHeaders = new Header[headers.size()];
265282
int i = 0;
@@ -291,7 +308,10 @@ protected static void configureHttpsClient(
291308
* authentication.
292309
*/
293310
private static void configureAwsClient(
294-
RestClientBuilder builder, Settings settings, HttpHost httpHost) throws IOException {
311+
RestClientBuilder builder,
312+
org.opensearch.common.settings.Settings settings,
313+
HttpHost httpHost)
314+
throws IOException {
295315
builder.setHttpClientConfigCallback(
296316
httpClientBuilder -> {
297317
try {
@@ -331,7 +351,10 @@ private static void configureAwsClient(
331351

332352
/** Configure HTTP client for regular OpenSearch using basic authentication. */
333353
private static void configureBasicAuthClient(
334-
RestClientBuilder builder, Settings settings, HttpHost httpHost) throws IOException {
354+
RestClientBuilder builder,
355+
org.opensearch.common.settings.Settings settings,
356+
HttpHost httpHost)
357+
throws IOException {
335358
builder.setHttpClientConfigCallback(
336359
httpClientBuilder -> {
337360
String userName =

integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,15 @@ protected static JSONObject updateClusterSettings(ClusterSetting setting, RestCl
405405
String clusterUrl = System.getProperty("tests.rest.cluster");
406406
boolean isAoss = clusterUrl != null && clusterUrl.contains(".aoss.amazonaws.com");
407407

408+
// Debug logging
409+
System.out.println("=== CLUSTER SETTINGS DEBUG ===");
410+
System.out.println("Cluster URL: " + clusterUrl);
411+
System.out.println("Is AOSS: " + isAoss);
412+
System.out.println("Setting: " + setting.toString());
413+
System.out.println("==============================");
414+
408415
if (isAoss) {
416+
System.out.println("🔄 SKIPPING cluster settings for AOSS");
409417
return new JSONObject("{}"); // Return empty JSON for AOSS
410418
}
411419

0 commit comments

Comments
 (0)