repair region handling (issue #67)

PelicanPlatform · Dec 22, 2024 · f6eeaa0 · f6eeaa0
1 parent 573987e
commit f6eeaa0
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 62 deletions.
diff --git a/src/S3Commands.hh b/src/S3Commands.hh
@@ -32,17 +32,19 @@ class AmazonRequest : public HTTPRequest {
   public:
 	AmazonRequest(const S3AccessInfo &ai, const std::string objectName,
 				  XrdSysError &log, bool ro = true)
-		: AmazonRequest(ai.getS3ServiceUrl(), ai.getS3AccessKeyFile(),
-						ai.getS3SecretKeyFile(), ai.getS3BucketName(),
-						objectName, ai.getS3UrlStyle(),
+		: AmazonRequest(ai.getS3ServiceUrl(), ai.getS3Region(),
+						ai.getS3AccessKeyFile(), ai.getS3SecretKeyFile(),
+						ai.getS3BucketName(), objectName, ai.getS3UrlStyle(),
 						ai.getS3SignatureVersion(), log, ro) {}
 
-	AmazonRequest(const std::string &s, const std::string &akf,
-				  const std::string &skf, const std::string &b,
-				  const std::string &o, const std::string &style, int sv,
-				  XrdSysError &log, bool ro = true)
+	AmazonRequest(const std::string &s, const std::string &r,
+				  const std::string &akf, const std::string &skf,
+				  const std::string &b, const std::string &o,
+				  const std::string &style, int sv, XrdSysError &log,
+				  bool ro = true)
 		: HTTPRequest(s, log, nullptr), accessKeyFile(akf), secretKeyFile(skf),
-		  signatureVersion(sv), bucket(b), object(o), m_style(style) {
+		  signatureVersion(sv), bucket(b), object(o), region(r),
+		  m_style(style) {
 		requiresSignature = true;
 		retainObject = ro;
 		// Start off by parsing the hostUrl, which we use in conjunction with
@@ -70,11 +72,7 @@ class AmazonRequest : public HTTPRequest {
 		// requests.
 		hostUrl = getProtocol() + "://" + host + canonicalURI;
 
-		// If we can, set the region based on the host.
-		size_t secondDot = host.find(".", 2 + 1);
-		if (host.find("s3.") == 0) {
-			region = host.substr(3, secondDot - 2 - 1);
-		}
+		region = r;
 	}
 	virtual ~AmazonRequest();
 
@@ -154,12 +152,6 @@ class AmazonS3Upload final : public AmazonRequest {
 				   XrdSysError &log)
 		: AmazonRequest(ai, objectName, log) {}
 
-	AmazonS3Upload(const std::string &s, const std::string &akf,
-				   const std::string &skf, const std::string &b,
-				   const std::string &o, const std::string &style,
-				   XrdSysError &log)
-		: AmazonRequest(s, akf, skf, b, o, style, 4, log) {}
-
 	virtual ~AmazonS3Upload();
 
 	bool SendRequest(const std::string_view &payload);
@@ -177,12 +169,6 @@ class AmazonS3CreateMultipartUpload final : public AmazonRequest {
 								  XrdSysError &log)
 		: AmazonRequest(ai, objectName, log) {}
 
-	AmazonS3CreateMultipartUpload(const std::string &s, const std::string &akf,
-								  const std::string &skf, const std::string &b,
-								  const std::string &o,
-								  const std::string &style, XrdSysError &log)
-		: AmazonRequest(s, akf, skf, b, o, style, 4, log) {}
-
 	bool Results(std::string &uploadId, std::string &errMsg);
 
 	virtual ~AmazonS3CreateMultipartUpload();
@@ -202,13 +188,6 @@ class AmazonS3CompleteMultipartUpload : public AmazonRequest {
 									XrdSysError &log)
 		: AmazonRequest(ai, objectName, log) {}
 
-	AmazonS3CompleteMultipartUpload(const std::string &s,
-									const std::string &akf,
-									const std::string &skf,
-									const std::string &b, const std::string &o,
-									const std::string &style, XrdSysError &log)
-		: AmazonRequest(s, akf, skf, b, o, style, 4, log) {}
-
 	virtual ~AmazonS3CompleteMultipartUpload();
 
 	virtual bool SendRequest(const std::vector<std::string> &eTags,
@@ -225,12 +204,6 @@ class AmazonS3SendMultipartPart : public AmazonRequest {
 							  const std::string &objectName, XrdSysError &log)
 		: AmazonRequest(ai, objectName, log) {}
 
-	AmazonS3SendMultipartPart(const std::string &s, const std::string &akf,
-							  const std::string &skf, const std::string &b,
-							  const std::string &o, const std::string &style,
-							  XrdSysError &log)
-		: AmazonRequest(s, akf, skf, b, o, style, 4, log) {}
-
 	bool Results(std::string &uploadId, std::string &errMsg);
 
 	virtual ~AmazonS3SendMultipartPart();
@@ -262,12 +235,6 @@ class AmazonS3Download : public AmazonRequest {
 					 XrdSysError &log, char *buffer)
 		: AmazonRequest(ai, objectName, log), m_buffer(buffer) {}
 
-	AmazonS3Download(const std::string &s, const std::string &akf,
-					 const std::string &skf, const std::string &b,
-					 const std::string &o, const std::string &style,
-					 XrdSysError &log, char *buffer)
-		: AmazonRequest(s, akf, skf, b, o, style, 4, log), m_buffer(buffer) {}
-
 	virtual ~AmazonS3Download();
 
 	virtual bool SendRequest(off_t offset, size_t size);
@@ -310,12 +277,6 @@ class AmazonS3Head final : public AmazonRequest {
 				 XrdSysError &log)
 		: AmazonRequest(ai, objectName, log) {}
 
-	AmazonS3Head(const std::string &s, const std::string &akf,
-				 const std::string &skf, const std::string &b,
-				 const std::string &o, const std::string &style,
-				 XrdSysError &log)
-		: AmazonRequest(s, akf, skf, b, o, style, 4, log) {}
-
 	virtual ~AmazonS3Head();
 
 	virtual bool SendRequest();

diff --git a/test/s3-xrootd-test.cfg b/test/s3-xrootd-test.cfg
@@ -8,9 +8,9 @@ s3.bucket_name noaa-wod-pds
 s3.service_name s3
 s3.region us-east-1
 s3.service_url https://s3.us-east-1.amazonaws.com
+s3.url_style        virtual
 s3.end
 
-s3.url_style        virtual
 
 ofs.trace all
 xrd.trace all -sched

diff --git a/test/s3_tests.cc b/test/s3_tests.cc
@@ -29,38 +29,40 @@ class TestAmazonRequest : public AmazonRequest {
 	XrdSysLogger log{};
 	XrdSysError err{&log, "TestS3CommandsLog"};
 
-	TestAmazonRequest(const std::string &url, const std::string &akf,
-					  const std::string &skf, const std::string &bucket,
-					  const std::string &object, const std::string &path,
-					  int sigVersion)
-		: AmazonRequest(url, akf, skf, bucket, object, path, sigVersion, err) {}
+	TestAmazonRequest(const S3AccessInfo &ai, const std::string objectName,
+					  bool ro = true)
+		: AmazonRequest(ai, objectName, err) {}
 
 	// For getting access to otherwise-protected members
 	std::string getHostUrl() const { return hostUrl; }
 };
 
 TEST(TestS3URLGeneration, Test1) {
-	const std::string serviceUrl = "https://s3-service.com:443";
-	const std::string b = "test-bucket";
+	S3AccessInfo ai;
+	ai.setS3ServiceUrl("https://s3-service.com:443");
+	ai.setS3BucketName("test-bucket");
 	const std::string o = "test-object";
 
 	// Test path-style URL generation
-	TestAmazonRequest pathReq{serviceUrl, "akf", "skf", b, o, "path", 4};
+	ai.setS3UrlStyle("path");
+	TestAmazonRequest pathReq{ai, o};
 	std::string generatedHostUrl = pathReq.getHostUrl();
 	ASSERT_EQ(generatedHostUrl,
 			  "https://s3-service.com:443/test-bucket/test-object")
 		<< "generatedURL: " << generatedHostUrl;
 
 	// Test virtual-style URL generation
-	TestAmazonRequest virtReq{serviceUrl, "akf", "skf", b, o, "virtual", 4};
+	ai.setS3UrlStyle("virtual");
+	TestAmazonRequest virtReq{ai, o};
 	generatedHostUrl = virtReq.getHostUrl();
 	ASSERT_EQ(generatedHostUrl,
 			  "https://test-bucket.s3-service.com:443/test-object");
 
 	// Test path-style with empty bucket (which we use for exporting an entire
 	// endpoint)
-	TestAmazonRequest pathReqNoBucket{serviceUrl, "akf",  "skf", "",
-									  o,		  "path", 4};
+	ai.setS3BucketName("");
+	ai.setS3UrlStyle("path");
+	TestAmazonRequest pathReqNoBucket{ai, o};
 	generatedHostUrl = pathReqNoBucket.getHostUrl();
 	ASSERT_EQ(generatedHostUrl, "https://s3-service.com:443/test-object");
 }