From f6eeaa09698b87900088bc277cd26d4a6d541737 Mon Sep 17 00:00:00 2001 From: Rich Wellner Date: Sun, 22 Dec 2024 15:56:49 -0600 Subject: [PATCH] repair region handling (issue #67) --- src/S3Commands.hh | 61 ++++++++--------------------------------- test/s3-xrootd-test.cfg | 2 +- test/s3_tests.cc | 24 ++++++++-------- 3 files changed, 25 insertions(+), 62 deletions(-) diff --git a/src/S3Commands.hh b/src/S3Commands.hh index 271dd5e..dbb6b6e 100644 --- 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 &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 index 8ec8d49..46a785a 100644 --- 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 index bc3f695..6481177 100644 --- 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"); }