Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ private URI getUri(URI uri, String supportedScheme,
int port = uri.getPort();
port = (port == -1 ? defaultPort : port);
if (port == -1) { // no port supplied and default port is not specified
return new URI(supportedScheme, authority, "/", null);
return URI.create(supportedScheme + "://" + authority + "/");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change?

}
return new URI(supportedScheme + "://" + uri.getHost() + ":" + port);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -573,8 +573,11 @@ private static void addDeprecatedKeys() {
*/
public void initialize(URI name, Configuration originalConf)
throws IOException {
// get the host; this is guaranteed to be non-null, non-empty
// get the host; fallback to authority if getHost() returns null
bucket = name.getHost();
if (bucket == null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pull this out, stick it in S3AUtils, add unit tests that now try to break things. Use everywhere

bucket = name.getAuthority();
}
AuditSpan span = null;
// track initialization duration; will only be set after
// statistics are set up.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ public int run(final String[] args, final PrintStream out)
final String bucketPath = parsedArgs.get(0);
final Path source = new Path(bucketPath);
URI fsURI = source.toUri();
String bucket = fsURI.getHost();
String bucket = fsURI.getHost() != null ? fsURI.getHost() : fsURI.getAuthority();

println(out, "Filesystem %s", fsURI);
if (!"s3a".equals(fsURI.getScheme())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,12 @@ public static URI buildFSURI(URI uri) {
// look for login secrets and fail if they are present.
Objects.requireNonNull(uri, "null uri");
Objects.requireNonNull(uri.getScheme(), "null uri.getScheme()");
Objects.requireNonNull(uri.getHost(), "null uri host.");
return URI.create(uri.getScheme() + "://" + uri.getHost());
String host = uri.getHost();
if (host == null) {
host = uri.getAuthority();
}
Objects.requireNonNull(host, "null uri host.");
return URI.create(uri.getScheme() + "://" + host);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ before 2021.
Consult [S3A and Directory Markers](directory_markers.html) for
full details.

### <a name="bucket-name-compatibility"></a> S3 Bucket Name Compatibility

This release adds support for S3 bucket names containing dots followed by numbers
(e.g., `my-bucket-v1.1`, `data-store.v2.3`). Previous versions of the Hadoop S3A
client failed to initialize such buckets due to URI parsing limitations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • highlight that per-bucket settings do not work for dotted buckets (they don't, do they?), so the ability to use them is still very much downgraded.
  • Explain that AWS do not recommend dotted buckets for anything other than web site serving
  • highlight that path style access is needed to access (correct? never tried)

## <a name="documents"></a> Documents

* [Connecting](./connecting.html)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,11 @@ private static <T> T getField(Object target, Class<T> fieldType,
public void testConfOptionPropagationToFS() throws Exception {
Configuration config = new Configuration();
String testFSName = config.getTrimmed(TEST_FS_S3A_NAME, "");
String bucket = new URI(testFSName).getHost();
URI uri = new URI(testFSName);
String bucket = uri.getHost();
if (bucket == null) {
bucket = uri.getAuthority();
}
setBucketOption(config, bucket, "propagation", "propagated");
fs = S3ATestUtils.createTestFileSystem(config);
Configuration updated = fs.getConf();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -883,7 +883,11 @@ public static <T extends Writable> T roundTrip(
public static String getTestBucketName(final Configuration conf) {
String bucket = checkNotNull(conf.get(TEST_FS_S3A_NAME),
"No test bucket");
return URI.create(bucket).getHost();
URI uri = URI.create(bucket);
if (uri.getHost() != null) {
return uri.getHost();
}
return uri.getAuthority();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@
import org.junit.jupiter.api.io.TempDir;

import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.s3a.auth.delegation.EncryptionSecrets;
import org.apache.hadoop.fs.s3native.S3xLoginHelper;
import org.apache.hadoop.security.ProviderUtils;
import org.apache.hadoop.security.alias.CredentialProvider;
import org.apache.hadoop.security.alias.CredentialProviderFactory;
Expand Down Expand Up @@ -75,6 +77,55 @@
S3AFileSystem.initializeClass();
}

@Test
public void testS3xLoginHelperWithDotInBucketName() throws Throwable {
// Test buildFSURI with bucket name containing dot followed by number
URI uri = URI.create("s3a://bucket-v1.1/path");
URI result = S3xLoginHelper.buildFSURI(uri);
assertEquals("s3a://bucket-v1.1", result.toString());

// Test with normal bucket name
URI normalUri = URI.create("s3a://normal-bucket/path");
URI normalResult = S3xLoginHelper.buildFSURI(normalUri);
assertEquals("s3a://normal-bucket", normalResult.toString());

// Test edge case with multiple dots
URI multiDotUri = URI.create("s3a://bucket.v1.2.test/path");
URI multiDotResult = S3xLoginHelper.buildFSURI(multiDotUri);
assertEquals("s3a://bucket.v1.2.test", multiDotResult.toString());
}

@Test
public void testBucketNameWithDotAndNumber() throws Exception {
Configuration config = new Configuration();
org.apache.hadoop.fs.Path path =
new org.apache.hadoop.fs.Path("s3a://test-bucket-v1.1");
try (FileSystem fs = path.getFileSystem(config)) {
assertThat(fs)
.describedAs("FileSystem should be S3AFileSystem instance")
.isInstanceOf(S3AFileSystem.class);
}
}

@Test
public void testFileSystemCacheForBucketWithDotAndNumber() throws Exception {
Configuration config = new Configuration();
URI uri1 = URI.create("s3a://test-bucket-v1.1");
URI uri2 = URI.create("s3a://test-bucket-v1.2");

Check failure on line 115 in hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestBucketConfiguration.java

View check run for this annotation

ASF Cloudbees Jenkins ci-hadoop / Apache Yetus

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestBucketConfiguration.java#L115

blanks: end of line
FileSystem fs1a = FileSystem.get(uri1, config);
FileSystem fs1b = FileSystem.get(uri1, config);
FileSystem fs2 = FileSystem.get(uri2, config);

assertThat(fs1a)
.describedAs("The call should return same cached instance for same URI")
.isSameAs(fs1b);

assertThat(fs1a)
.describedAs("The call should return different instance for different bucket")
.isNotSameAs(fs2);
}

@Test
public void testBucketConfigurationPropagation() throws Throwable {
Configuration config = new Configuration(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,17 @@ public SdkHttpFullRequest sign(SdkHttpFullRequest request,

String host = request.host();
String bucketName = parseBucketFromHost(host);
// If host-based parsing fails (path-style requests), extract from path
if (bucketName.equals("s3")) {
String path = request.encodedPath();
if (path != null && path.startsWith("/") && path.length() > 1) {
String[] pathParts = path.substring(1).split("/", 2);
if (pathParts.length > 0 && !pathParts[0].isEmpty()) {
bucketName = pathParts[0];
}
}
}

try {
lastStoreValue = CustomSignerInitializer
.getStoreValue(bucketName, UserGroupInformation.getCurrentUser());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,12 @@ private String run(CredentialShell cs, String expected, String... args)
*/
private String toJceksProvider(Path keystore) {
final URI uri = keystore.toUri();
String bucket = uri.getHost();
if (bucket == null) {
bucket = uri.getAuthority();
}
return String.format("jceks://%s@%s%s",
uri.getScheme(), uri.getHost(), uri.getPath());
uri.getScheme(), bucket, uri.getPath());
}

}