[GitHub] [ozone] errose28 commented on a diff in pull request #5123: HDDS-7035. With virtual style addressing do not append bucketname from host to canonical URI for signature validation.

["errose28 (via GitHub)" <gi...@apache.org>]

errose28 commented on code in PR #5123:
URL: https://github.com/apache/ozone/pull/5123#discussion_r1294082950


##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/TestStringToSignProducer.java:
##########
@@ -269,4 +271,60 @@ public void testValidateCanonicalHeaders(
 
     Assert.assertEquals(expectedResult, actualResult);
   }
+
+  @Test
+  public void testVirtualStyleAddressURI() throws Exception {

Review Comment:
   Can we rename the other test in this class to `testPathStyleAddressURI` to clarify the difference?



##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/util/S3Consts.java:
##########
@@ -64,4 +64,9 @@ private S3Consts() {
 
   public static final String CUSTOM_METADATA_HEADER_PREFIX = "x-amz-meta-";
 
+  public static final String ENDPOINT_STYLE_PARAM = "endpoint-style";
+
+  public static final String ENDPOINT_STYLE_PATH = "path";
+
+  public static final String ENDPOINT_STYLE_VIRTUAL = "virtual";

Review Comment:
   Where did these values come from? I can't find them documented by AWS anywhere, for example at https://docs.aws.amazon.com/AmazonS3/latest/userguide/VirtualHosting.html



##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestVirtualHostStyleFilter.java:
##########
@@ -106,7 +107,8 @@ public void testVirtualHostStyle() throws  Exception {
             ".localhost:9878", "/myfile", null, true);
     virtualHostStyleFilter.filter(containerRequest);
     URI expected = new URI("http://" + s3HttpAddr +
-        "/mybucket/myfile");
+        "/mybucket/myfile?" + S3Consts.ENDPOINT_STYLE_PARAM + "=" +
+        S3Consts.ENDPOINT_STYLE_VIRTUAL);

Review Comment:
   According to examples in [s3 docs](https://docs.aws.amazon.com/AmazonS3/latest/userguide/VirtualHosting.html), no extra query parameters are required. The tests should run the same way.



-- 
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-unsubscribe@ozone.apache.org

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


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