fix: TOOLS-2569 correctly parse --s3-bucket option

src/file_proxy_s3.cc

@@ -467,7 +467,7 @@ off_t s3_get_backup_files(const char* prefix, as_vector* file_vec)
 
 	const Aws::S3::S3Client& client = g_api.GetS3Client();
 
-	size_t prefix_len = strlen(S3_PREFIX) + 1;
+	size_t prefix_len = strlen(S3_PREFIX) + path.GetBucket().size() + 1;
 
 	Aws::S3::Model::ListObjectsV2Request req;
 	req.SetBucket(path.GetBucket());
@@ -491,7 +491,7 @@ off_t s3_get_backup_files(const char* prefix, as_vector* file_vec)
 			}
 
 			snprintf(elem, prefix_len + obj_key.size() + 1,
-					S3_PREFIX "%s", obj_key.c_str());
+					S3_PREFIX "%s/%s", path.GetBucket().c_str(), obj_key.c_str());
 
 			as_vector_append(file_vec, &elem);
 

src/s3_api.cc

@@ -222,8 +222,9 @@ S3API::S3Path::GetBucket() const
 bool
 S3API::S3Path::ParsePath(const std::string& bucket, const std::string& path)
 {
+
+	// path must be in the format "s3://<bucket>/<key>"
 	if (bucket.size() == 0) {
-		// path is in the format "s3://<bucket>/<key>"
 		size_t delim = path.find('/', S3_PREFIX_LEN);
 
 		if (delim == std::string::npos) {
@@ -235,9 +236,19 @@ S3API::S3Path::ParsePath(const std::string& bucket, const std::string& path)
 		this->bucket = path.substr(S3_PREFIX_LEN, delim - S3_PREFIX_LEN);
 		this->key = path.substr(delim + 1);
 	}
+	// path could be in the format "s3://<bucket>/<key>" or "s3://<key>"
+	// we always want to omit the bucket from the key so that the s3 api
+	// doesn't repeat it when it concatenates this->bucket with this->key
 	else {
 		this->bucket = bucket;
-		this->key = path.substr(S3_PREFIX_LEN);
+		// if the path starts with s3://<bucket>/ then set the key to what comes after
+		std::string bucket_substring = bucket + "/";
+		if (path.compare(S3_PREFIX_LEN, bucket_substring.size(), bucket_substring) == 0) {
+			this->key = path.substr(S3_PREFIX_LEN + bucket_substring.size());
+		}
+		else {
+			this->key = path.substr(S3_PREFIX_LEN);
+		}
 	}
 
 	return true;

test/integration/test_s3.py

@@ -26,7 +26,8 @@
 
 
 def do_s3_backup(max_interrupts, n_records=10000, backup_opts=None,
-		restore_opts=None, state_file_dir=False, state_file_explicit=False, backup_cout=0):
+		restore_opts=None, state_file_dir=False, state_file_explicit=False,
+		backup_cout=0, s3_bucket=False):
 	if backup_opts == None:
 		backup_opts = []
 	if restore_opts == None:
@@ -41,6 +42,10 @@ def do_s3_backup(max_interrupts, n_records=10000, backup_opts=None,
 	# make the bucket
 	os.mkdir(backup_files_loc + "/" + S3_BUCKET, 0o755)
 
+	if s3_bucket:
+		backup_opts += ["--s3-bucket", S3_BUCKET]
+		restore_opts += ["--s3-bucket", S3_BUCKET]
+
 	for comp_enc_mode in [
 			[],
 			['--compress=zstd'],
@@ -56,10 +61,16 @@ def do_s3_backup(max_interrupts, n_records=10000, backup_opts=None,
 		prev_rec_total = 0
 		prev_bytes_total = 0
 
-		if lib.is_dir_mode():
-			path = "s3://" + S3_BUCKET + "/test_dir"
+		if s3_bucket:
+			if lib.is_dir_mode():
+				path = "s3://" + "test_dir"
+			else:
+				path = "s3://" + "test_file.asb"
 		else:
-			path = "s3://" + S3_BUCKET + "/test_file.asb"
+			if lib.is_dir_mode():
+				path = "s3://" + S3_BUCKET + "/test_dir"
+			else:
+				path = "s3://" + S3_BUCKET + "/test_file.asb"
 
 		env = S3_ENV
 
@@ -161,15 +172,24 @@ def do_s3_backup(max_interrupts, n_records=10000, backup_opts=None,
 def test_s3_backup_small():
 	do_s3_backup(0, n_records=100, backup_opts=['--s3-region', S3_REGION])
 
+def test_s3_backup_small_s3_bucket_opt():
+	do_s3_backup(0, n_records=100, backup_opts=['--s3-region', S3_REGION], s3_bucket=True)
+
 def test_s3_backup():
 	do_s3_backup(0)
 
 def test_s3_backup_multiple_files():
 	do_s3_backup(0, n_records=10000, backup_opts=['--file-limit', '1', '--s3-connect-timeout', '2000'], restore_opts=['--s3-connect-timeout', '2000'])
 
+def test_s3_backup_multiple_files_s3_bucket_opt():
+	do_s3_backup(0, n_records=10000, backup_opts=['--file-limit', '1', '--s3-connect-timeout', '2000'], restore_opts=['--s3-connect-timeout', '2000'], s3_bucket=True)
+
 def test_s3_backup_interrupt():
 	do_s3_backup(1, n_records=10000, backup_opts=['--records-per-second', '200'])
 
+def test_s3_backup_interrupt_s3_bucket_opt():
+	do_s3_backup(1, n_records=10000, backup_opts=['--records-per-second', '200'], s3_bucket=True)
+
 def test_s3_backup_multiple_files_interrupt():
 	do_s3_backup(10, n_records=10000, backup_opts=['--file-limit', '1'])