Add very basic unit tests for S3 and HTTP

CMakeLists.txt

@@ -2,6 +2,9 @@ cmake_minimum_required( VERSION 3.13 )
 
 project( xrootd-http/s3 )
 
+option( XROOTD_PLUGINS_BUILD_UNITTESTS "Build the scitokens-cpp unit tests" OFF )
+option( XROOTD_PLUGINS_EXTERNAL_GTEST "Use an external/pre-installed copy of GTest" OFF )
+
 set( CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake )
 set( CMAKE_BUILD_TYPE Debug )
 
@@ -73,6 +76,19 @@ install(
   LIBRARY DESTINATION ${LIB_INSTALL_DIR}
 )
 
+if( XROOTD_PLUGINS_BUILD_UNITTESTS )
+	if( NOT XROOTD_PLUGINS_EXTERNAL_GTEST )
+include(ExternalProject)
+ExternalProject_Add(gtest
+    PREFIX external/gtest
+    URL ${CMAKE_CURRENT_SOURCE_DIR}/vendor/gtest
+    INSTALL_COMMAND :
+)
+endif()
+enable_testing()
+add_subdirectory(test)
+endif()
+
 #install(
 #  FILES ${CMAKE_SOURCE_DIR}/configs/60-s3.cfg
 #  DESTINATION ${CMAKE_INSTALL_PREFIX}/etc/xrootd/config.d/

README.md

@@ -1,31 +1,46 @@
 
-S3/HTTP filesystem plugins for XRootD
-================================
+# S3/HTTP filesystem plugins for XRootD
+These filesystem plugins for [XRootD](https://github.com/xrootd/xrootd) allow you to serve objects from S3 and HTTP backends through an XRootD server.
 
-These filesystem plugins are intended to demonstrate the ability to have an S3 bucket
-or raw HTTP server as an underlying "filesystem" for an XRootD server.
-
-They are currently quite experimental, aiming to show a "minimum viable product".
-
-Building and Installing
------------------------
+## Building and Installing
 Assuming XRootD, CMake>=3.13 and gcc>=8 are already installed, run:
 
 ```
 mkdir build
 cd build
 cmake ..
-make install
-```
+make
 
+# For system installation, uncomment:
+# make install
+```
 
 If building XRootD from source instead, add `-DXROOTD_DIR` to the CMake command line
 to point it at the installed directory.
 
-Configuration
--------------
+### Building with Tests
+
+Unit tests for this repository require `gtest`, which for RHEL-based linux distributions can be installed with:
+```bash
+dnf install gtest
+```
+
+Once `gtest` is installed, the unit tests can be compiled with a slight modification to your build command:
+
+```
+mkdir build
+cd build
+cmake -DXROOTD_PLUGINS_BUILD_UNITTESTS=ON -DXROOTD_PLUGINS_EXTERNAL_GTEST=ON ..
+make
+```
 
-## Configure an HTTP Server Backend
+This creates the directory `build/test` with two unit test executables that can be run:
+- `build/test/s3-gtest`
+- `build/test/http-gtest`
+
+## Configuration
+
+### Configure an HTTP Server Backend
 
 To configure the HTTP server plugin, add the following line to the Xrootd configuration file:
 
@@ -36,29 +51,9 @@ ofs.osslib </path/to/libXrdHTTPServer.so>
 Here's a minimal config file
 
 ```
-###########################################################################
-# This is a very simple sample configuration file sufficient to start an  #
-# xrootd file caching proxy server using the default port 1094. This      #
-# server runs by itself (stand-alone) and does not assume it is part of a #
-# cluster. You can then connect to this server to access files in '/tmp'. #
-# Consult the the reference manuals on how to create more complicated     #
-# configurations.                                                         #
-#                                                                         #
-# On successful start-up you will see 'initialization completed' in the   #
-# last message. You can now connect to the xrootd server.                 #
-#                                                                         #
-# Note: You should always create a *single* configuration file for all    #
-# daemons related to xrootd.                                              #
-###########################################################################
-
-# The adminpath and pidpath variables indicate where the pid and various
-# IPC files should be placed.  These can be placed in /tmp for a developer-
-# quality server.
-#
-all.adminpath /var/spool/xrootd
-all.pidpath /run/xrootd
-
-# Enable the HTTP protocol on port 1094 (same as the default XRootD port):
+# Enable the HTTP protocol on port 1094 (same as the default XRootD port)
+# NOTE: This is NOT the HTTP plugin -- it is the library XRootD uses to
+# speak the HTTP protocol, as opposed to the root protocol, for incoming requests
 xrd.protocol http:1094 libXrdHttp.so
 
 # Allow access to path with given prefix.
@@ -73,15 +68,12 @@ ofs.osslib libXrdHTTPServer.so
 # Upon last testing, the plugin did not yet work in async mode
 xrootd.async off
 
-
-
 # Configure the upstream HTTP server that XRootD is to treat as a filesystem
 httpserver.host_name <hostname of HTTP server>
 httpserver.host_url <host url>
 ```
 
-
-## Configure an S3 Backend
+### Configure an S3 Backend
 
 To configure the S3 plugin, add the following line to the Xrootd configuration file:
 
@@ -92,29 +84,8 @@ ofs.osslib </path/to/libXrdS3.so>
 Here's a minimal config file
 
 ```
-###########################################################################
-# This is a very simple sample configuration file sufficient to start an  #
-# xrootd file caching proxy server using the default port 1094. This      #
-# server runs by itself (stand-alone) and does not assume it is part of a #
-# cluster. You can then connect to this server to access files in '/tmp'. #
-# Consult the the reference manuals on how to create more complicated     #
-# configurations.                                                         #
-#                                                                         #
-# On successful start-up you will see 'initialization completed' in the   #
-# last message. You can now connect to the xrootd server.                 #
-#                                                                         #
-# Note: You should always create a *single* configuration file for all    #
-# daemons related to xrootd.                                              #
-###########################################################################
-
-# The adminpath and pidpath variables indicate where the pid and various
-# IPC files should be placed.  These can be placed in /tmp for a developer-
-# quality server.
-#
-all.adminpath /var/spool/xrootd
-all.pidpath /run/xrootd
-
-# Enable the HTTP protocol on port 1094 (same as the default XRootD port):
+# Enable the HTTP protocol on port 1094 (same as the default XRootD port)
+# The S3 plugin use
 xrd.protocol http:1094 libXrdHttp.so
 
 # Allow access to path with given prefix.
@@ -161,10 +132,9 @@ s3.url_style        virtual
 ```
 
 
-Startup and Testing
--------------------
+## Startup and Testing
 
-## HTTP Server Backend
+### HTTP Server Backend
 
 Assuming you named the config file `xrootd-http.cfg`, as a non-rootly user run:
 
@@ -178,11 +148,8 @@ In a separate terminal, run
 curl -v http://localhost:1094/<host name>/<URL path to object>
 ```
 
-## S3 Server Backend
+### S3 Server Backend
 Startup and Testing
--------------------
-
-## HTTP Server Backend
 
 Assuming you named the config file `xrootd-s3.cfg`, as a non-rootly user run:
 
@@ -193,5 +160,5 @@ xrootd -d -c xrootd-s3.cfg
 In a separate terminal, run
 
 ```
-curl -v http://localhost:1094/<service_name>/<region>/<path to bucket/object>
+curl -v http://localhost:1094/<path name>/<object name>
 ```

src/HTTPCommands.cc

@@ -81,21 +81,6 @@ bool HTTPRequest::parseProtocol(const std::string &url, std::string &protocol) {
 	}
 	protocol = substring(url, 0, i);
 
-	// This func used to parse the entire URL according
-	// to the Amazon canonicalURI specs, but that functionality
-	// has since been moved to the Amazon subclass. Now it just
-	// grabs the protocol. Leaving the old stuff commented for
-	// now, just in case...
-
-	// auto j = url.find( "/", i + 3 );
-	// if( j == std::string::npos ) {
-	//     host = substring( url, i + 3 );
-	//     path = "/";
-	//     return true;
-	// }
-
-	// host = substring( url, i + 3, j );
-	// path = substring( url, j );
 	return true;
 }
 

src/S3Commands.cc

@@ -55,8 +55,8 @@ std::string AmazonRequest::canonicalizeQueryString() {
 }
 
 // Takes in the configured `s3.service_url` and uses the bucket/object requested
-// to generate the virtual host URL, as well as the canonical URI (which is the
-// path to the object).
+// to generate the host URL, as well as the canonical URI (which is the path to
+// the object).
 bool AmazonRequest::parseURL(const std::string &url, std::string &path) {
 	auto i = url.find("://");
 	if (i == std::string::npos) {
@@ -71,7 +71,14 @@ bool AmazonRequest::parseURL(const std::string &url, std::string &path) {
 			// :// and the last /
 			host = substring(url, i + 3);
 			// Likewise, the path is going to be /bucket/object
-			path = "/" + bucket + "/" + object;
+			// Sometimes we intentionally configure the plugin with no bucket because we
+			// assume the incoming object request already encodes the bucket. This is used
+			// for exporting many buckets from a single endpoint.
+			if (bucket.empty()) {
+				path = "/" + object;
+			} else {
+				path = "/" + bucket + "/" + object;
+			}
 		} else {
 			// In virtual-style requests, the host should be determined as
 			// everything between

test/CMakeLists.txt

@@ -0,0 +1,51 @@
+add_executable( s3-gtest s3_tests.cc
+  ../src/AWSv4-impl.cc
+  ../src/logging.cc
+  ../src/S3AccessInfo.cc
+  ../src/S3File.cc
+  ../src/S3FileSystem.cc
+  ../src/shortfile.cc
+  ../src/stl_string_utils.cc
+  ../src/HTTPCommands.cc 
+  ../src/S3Commands.cc
+)
+
+add_executable( http-gtest http_tests.cc
+  ../src/HTTPFile.cc
+  ../src/HTTPFileSystem.cc
+  ../src/HTTPCommands.cc
+  ../src/stl_string_utils.cc
+  ../src/shortfile.cc
+  ../src/logging.cc
+)
+
+
+if( NOT XROOTD_PLUGINS_EXTERNAL_GTEST )
+    add_dependencies(s3-gtest gtest)
+    add_dependencies(http-gtest gtest)
+    include_directories("${PROJECT_SOURCE_DIR}/vendor/gtest/googletest/include")
+endif()
+
+if(XROOTD_PLUGINS_EXTERNAL_GTEST)
+    set(LIBGTEST "gtest")
+else()
+    set(LIBGTEST "${CMAKE_BINARY_DIR}/external/gtest/src/gtest-build/lib/libgtest.a")
+endif()
+
+target_link_libraries(s3-gtest XrdS3 "${LIBGTEST}" pthread)
+target_link_libraries(http-gtest XrdHTTPServer "${LIBGTEST}" pthread)
+
+
+add_test(
+  NAME
+    s3-unit
+  COMMAND
+    ${CMAKE_CURRENT_BINARY_DIR}/s3-gtest
+)
+
+add_test(
+  NAME
+    http-unit
+  COMMAND
+    ${CMAKE_CURRENT_BINARY_DIR}/http-gtest
+)

test/http_tests.cc

@@ -0,0 +1,50 @@
+/***************************************************************
+ *
+ * Copyright (C) 2024, Pelican Project, Morgridge Institute for Research
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you
+ * may not use this file except in compliance with the License.  You may
+ * obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ ***************************************************************/
+
+#include "../src/HTTPCommands.hh"
+
+#include <XrdSys/XrdSysError.hh>
+#include <XrdSys/XrdSysLogger.hh>
+#include <gtest/gtest.h>
+
+class TestHTTPRequest : public HTTPRequest {
+  public:
+	XrdSysLogger log{};
+	XrdSysError err{&log, "TestS3CommandsLog"};
+
+	TestHTTPRequest(const std::string &url) : HTTPRequest(url, err) {}
+};
+
+TEST(TestHTTPParseProtocol, Test1) {
+	const std::string httpURL = "https://my-test-url.com:443";
+	TestHTTPRequest req{httpURL};
+
+	// Test parsing of https
+	std::string protocol;
+	req.parseProtocol("https://my-test-url.com:443", protocol);
+	ASSERT_EQ(protocol, "https");
+
+	// Test parsing for http
+	req.parseProtocol("http://my-test-url.com:443", protocol);
+	ASSERT_EQ(protocol, "http");
+}
+
+int main(int argc, char **argv) {
+	::testing::InitGoogleTest(&argc, argv);
+	return RUN_ALL_TESTS();
+}