Plumb HTTPS backend through CLI and adjust it to use full storage prefixes

client/fed_test.go

@@ -841,36 +841,37 @@ func TestObjectList(t *testing.T) {
 // This tests object ls but for an origin that supports listings but with an object store that does not support PROPFIND.
 // We should get a 405 returned. This is a separate test since we need a completely different origin
 func TestObjectList405Error(t *testing.T) {
+	server_utils.ResetTestState()
+	defer server_utils.ResetTestState()
 	test_utils.InitClient(t, nil)
 
-	err := config.InitClient()
-	require.NoError(t, err)
+	var storageName string
 
 	// Set up our http backend so that we can return a 405 on a PROPFIND
 	body := "Hello, World!"
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if r.Method == "HEAD" && r.URL.Path == "/test2/hello_world" {
+		if r.Method == "HEAD" && r.URL.Path == storageName {
 			w.Header().Set("Content-Length", strconv.Itoa(len(body)))
 			w.WriteHeader(http.StatusOK)
 			return
-		} else if r.Method == "GET" && r.URL.Path == "/test2/hello_world" {
+		} else if r.Method == "GET" && r.URL.Path == storageName {
 			w.Header().Set("Content-Length", strconv.Itoa(len(body)))
 			w.WriteHeader(http.StatusPartialContent)
 			_, err := w.Write([]byte(body))
 			require.NoError(t, err)
 			return
-		} else if r.Method == "PROPFIND" && r.URL.Path == "/test2/hello_world" {
+		} else if r.Method == "PROPFIND" && r.URL.Path == storageName {
 			w.WriteHeader(http.StatusMethodNotAllowed)
 		}
 	}))
 	defer srv.Close()
-	viper.Set("Origin.HttpServiceUrl", srv.URL+"/test2")
+	viper.Set("Origin.HttpServiceUrl", srv.URL)
 
-	config.InitConfig()
 	fed := fed_test_utils.NewFedTest(t, httpsOriginConfig)
-	host := param.Server_Hostname.GetString() + ":" + strconv.Itoa(param.Server_WebPort.GetInt())
+	storageName = fed.Exports[0].StoragePrefix + "/hello_world"
+	discoveryHost := param.Server_Hostname.GetString() + ":" + strconv.Itoa(param.Server_WebPort.GetInt())
 
-	_, err = client.DoList(fed.Ctx, "pelican://"+host+"/test/hello_world")
+	_, err := client.DoList(fed.Ctx, "pelican://"+discoveryHost+"/my-prefix/hello_world")
 	require.Error(t, err)
 	require.Contains(t, err.Error(), "405: object listings are not supported by the discovered origin")
 }

client/resources/test-https-origin.yml

@@ -4,5 +4,6 @@ Logging:
     Scitokens: debug
 Origin:
   StorageType: https
-  FederationPrefix: /test
+  FederationPrefix: /my-prefix
+  StoragePrefix: /test
   EnablePublicReads: true

cmd/origin.go

@@ -97,7 +97,7 @@ func init() {
 	originCmd.AddCommand(originServeCmd)
 
 	// The -m flag is used to specify what kind of backend we plan to use for the origin.
-	originServeCmd.Flags().StringP("mode", "m", "posix", "Set the mode for the origin service (default is 'posix'). Supported modes are 'posix' and 's3'.")
+	originServeCmd.Flags().StringP("mode", "m", "posix", "Set the mode for the origin service (default is 'posix'). Supported modes are 'posix', 's3, 'https', 'globus' and 'xroot'.")
 	if err := viper.BindPFlag("Origin.StorageType", originServeCmd.Flags().Lookup("mode")); err != nil {
 		panic(err)
 	}
@@ -158,6 +158,16 @@ instead.
 		panic(err)
 	}
 
+	// https backend flags
+	originServeCmd.Flags().String("http-service-url", "", "Specify the http(s) service-url. Only used when an origin is launched in https/globus modes.")
+	if err := viper.BindPFlag("Origin.HttpServiceUrl", originServeCmd.Flags().Lookup("http-service-url")); err != nil {
+		panic(err)
+	}
+	originServeCmd.Flags().String("http-tokenfile", "", "Specify a filepath to use for configuring the http(s) token. See documentation for details.")
+	if err := viper.BindPFlag("Origin.HttpAuthTokenFile", originServeCmd.Flags().Lookup("http-tokenfile")); err != nil {
+		panic(err)
+	}
+
 	// The port any web UI stuff will be served on
 	originServeCmd.Flags().AddFlag(portFlag)
 

cmd/plugin_test.go

@@ -451,18 +451,18 @@ func TestPluginDirectRead(t *testing.T) {
 // We ran into a bug where the start time for the transfer was not recorded correctly and was almost always the same as the end time
 // (since they were set at similar sections of code). This test ensures that they are different and that the start time is before the end time.
 func TestPluginCorrectStartAndEndTime(t *testing.T) {
-	test_utils.InitClient(t, nil)
 	server_utils.ResetOriginExports()
 	defer server_utils.ResetTestState()
+	var storageName string
 
 	// Set up our http backend so that we can sleep during transfer
 	body := "Hello, World!"
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if r.Method == "HEAD" && r.URL.Path == "/test2/hello_world" {
+		if r.Method == "HEAD" && r.URL.Path == storageName {
 			w.Header().Set("Content-Length", strconv.Itoa(len(body)))
 			w.WriteHeader(http.StatusOK)
 			return
-		} else if r.Method == "GET" && r.URL.Path == "/test2/hello_world" {
+		} else if r.Method == "GET" && r.URL.Path == storageName {
 			w.Header().Set("Content-Length", strconv.Itoa(len(body)))
 			w.WriteHeader(http.StatusPartialContent)
 			time.Sleep(1 * time.Second)
@@ -473,20 +473,19 @@ func TestPluginCorrectStartAndEndTime(t *testing.T) {
 		w.WriteHeader(http.StatusNotFound)
 	}))
 	defer srv.Close()
-	viper.Set("Origin.HttpServiceUrl", srv.URL+"/test2")
-
-	config.InitConfig()
-	tmpPath := t.TempDir()
+	viper.Set("Origin.HttpServiceUrl", srv.URL)
 
 	fed := fed_test_utils.NewFedTest(t, httpsOriginConfig)
-	host := param.Server_Hostname.GetString() + ":" + strconv.Itoa(param.Server_WebPort.GetInt())
+	storageName = fed.Exports[0].StoragePrefix + "/hello_world"
+	discoveryHost := param.Server_Hostname.GetString() + ":" + strconv.Itoa(param.Server_WebPort.GetInt())
 
 	downloadUrl := url.URL{
 		Scheme: "pelican",
-		Host:   host,
-		Path:   "/test/hello_world",
+		Host:   discoveryHost,
+		Path:   "/my-prefix/hello_world",
 	}
 
+	tmpPath := t.TempDir()
 	workChan := make(chan PluginTransfer, 2)
 	workChan <- PluginTransfer{url: &downloadUrl, localFile: tmpPath}
 	close(workChan)

cmd/resources/test-https-origin.yml

@@ -3,5 +3,6 @@ Logging:
     Scitokens: debug
 Origin:
   StorageType: https
-  FederationPrefix: /test
+  FederationPrefix: /my-prefix
+  StoragePrefix: /test
   EnablePublicReads: true

docs/parameters.yaml

@@ -919,9 +919,9 @@ components: ["origin"]
 name: Origin.HttpServiceUrl
 description: |+
   If Origin.StorageType is set to `https`, the service URL is used as the base for requests to the backend.  To generate the
-  request, the Origin.FederationPrefix is removed from the object name, then the result is joined with the service URL.
-  For example, if one sets `Origin.HTTPServiceUrl=https://example.com/testfiles` and `Origin.FederationPrefix=/foo`, then a request
-  for an object named `/foo/bar` will generate a request to https://example.com/testfiles/bar.
+  request, the Origin.FederationPrefix is removed from the object name, then the result is joined with the service URL and storage prefix.
+  For example, if one sets `Origin.HTTPServiceUrl=https://example.com`, `Origin.StoragePrefix=/testfiles` and `Origin.FederationPrefix=/foo`,
+  then a request for an object named `/foo/bar` will generate a request to https://example.com/testfiles/bar.
 type: string
 default: none
 components: ["origin"]

server_utils/origin.go

@@ -183,6 +183,10 @@ func validateFederationPrefix(prefix string) error {
 		return errors.Wrapf(ErrInvalidOriginConfig, "prefix %s contains invalid '\\' characters", prefix)
 	}
 
+	if strings.Contains(prefix, "?") {
+		return errors.Wrapf(ErrInvalidOriginConfig, "prefix %s contains invalid '?' characters", prefix)
+	}
+
 	if server_structs.IsCacheNS(prefix) || server_structs.IsOriginNS(prefix) {
 		return errors.Wrapf(ErrInvalidOriginConfig, "prefix %s is a reserved prefix for cache/origin server registration", prefix)
 	}
@@ -238,7 +242,7 @@ func validateBucketName(bucket string) error {
 }
 
 // GetOriginExports is used to parse the config yaml and return a list of OriginExports. It should only touch
-// the yaml the first time it's called, and then return the cached value on subsequent calls.
+// the yaml the first time it's called, and then return the in-memory value on subsequent calls.
 // When the configuration is set up using the older single-prefix style of configuration, the function will
 // convert those values (such as Origin.FederationPrefix, Origin.StoragePrefix, etc.) into the OriginExports
 // struct and return a list of one. Otherwise, we'll base things off the list of exports and ignore the single-prefix
@@ -356,15 +360,110 @@ func GetOriginExports() ([]OriginExport, error) {
 			viper.Set("Origin.EnableReads", capabilities.Reads)
 		}
 	case server_structs.OriginStorageHTTPS:
-		// Storage prefix is unused by HTTPS so we put in a dummy value of /
-		originExport = OriginExport{
-			FederationPrefix: param.Origin_FederationPrefix.GetString(),
-			StoragePrefix:    "/",
-			Capabilities:     capabilities,
+		// clean up the http service URL
+		if strings.HasSuffix(param.Origin_HttpServiceUrl.GetString(), "/") {
+			log.Warningln("Removing trailing '/' from http service URL")
+			viper.Set("Origin.HttpServiceUrl", strings.TrimSuffix(param.Origin_HttpServiceUrl.GetString(), "/"))
+		}
+
+		// Handle exports configured via -v or potentially env vars
+		if len(param.Origin_ExportVolumes.GetStringSlice()) > 0 {
+			log.Infoln("Configuring exports from export volumes passed via command line or via yaml")
+			volumes := param.Origin_ExportVolumes.GetStringSlice()
+			if len(volumes) > 1 {
+				// We don't yet support multiple exports for the HTTPS backend
+				return nil, errors.Errorf("https backend does not yet support multiple exports, but %d were provided: %+v", len(volumes), volumes)
+			}
+
+			volume := volumes[0]
+			// Perform validation of the namespace
+			storagePrefix := filepath.Clean(volume)
+			federationPrefix := filepath.Clean(volume)
+			volumeMountInfo := strings.SplitN(volume, ":", 2)
+			if len(volumeMountInfo) == 2 {
+				storagePrefix = filepath.Clean(volumeMountInfo[0])
+				federationPrefix = filepath.Clean(volumeMountInfo[1])
+			}
+
+			if err = validateExportPaths(storagePrefix, federationPrefix); err != nil {
+				return nil, err
+			}
+
+			// clean up trailing / in the storage prefix
+			if strings.HasSuffix(storagePrefix, "/") {
+				log.Warningln("Removing trailing '/' from storage prefix", storagePrefix)
+				storagePrefix = strings.TrimSuffix(storagePrefix, "/")
+			}
+			originExport := OriginExport{
+				FederationPrefix: federationPrefix,
+				StoragePrefix:    storagePrefix,
+				Capabilities:     capabilities,
+			}
+
+			viper.Set("Origin.FederationPrefix", originExport.FederationPrefix)
+			viper.Set("Origin.StoragePrefix", originExport.StoragePrefix)
+
+			log.Warningln(WarnExportVolumes)
+			originExports := []OriginExport{originExport}
+			return originExports, nil
 		}
 
-		if err = validateExportPaths("/", originExport.FederationPrefix); err != nil {
-			return nil, err
+		if param.Origin_Exports.IsSet() {
+			var tmpExports []OriginExport
+			if err := viper.UnmarshalKey("Origin.Exports", &tmpExports, viper.DecodeHook(StringListToCapsHookFunc())); err != nil {
+				return nil, err
+			}
+			if len(tmpExports) == 0 {
+				err := errors.New("Origin.Exports is defined, but no exports were found")
+				return nil, err
+			} else if len(tmpExports) > 1 {
+				err := errors.New("More than one export found, only one export is currently supported for the https backend")
+				return nil, err
+			}
+
+			// Assume there's only one export
+			export := tmpExports[0]
+
+			// Clean up any path components that might have been added by the user to guarantee the correct
+			// URL is constructed without duplicate or missing slashes
+			if strings.HasSuffix(export.StoragePrefix, "/") {
+				log.Warningln("Removing trailing '/' from storage prefix", export.StoragePrefix)
+				export.StoragePrefix = strings.TrimSuffix(export.StoragePrefix, "/")
+			}
+
+			if err = validateExportPaths(export.StoragePrefix, export.FederationPrefix); err != nil {
+				return nil, err
+			}
+
+			capabilities := export.Capabilities
+			reads := capabilities.Reads || capabilities.PublicReads
+			viper.Set("Origin.FederationPrefix", export.FederationPrefix)
+			viper.Set("Origin.StoragePrefix", export.StoragePrefix)
+			viper.Set("Origin.EnableReads", reads)
+			viper.Set("Origin.EnablePublicReads", capabilities.PublicReads)
+			viper.Set("Origin.EnableWrites", capabilities.Writes)
+			viper.Set("Origin.EnableListings", capabilities.Listings)
+			viper.Set("Origin.EnableDirectReads", capabilities.DirectReads)
+
+			originExports = []OriginExport{export}
+			return originExports, nil
+		} else { // we're using the simple Origin.FederationPrefix
+			log.Infoln("Configuring single-export origin")
+			federationPrefix := param.Origin_FederationPrefix.GetString()
+			storagePrefix := param.Origin_StoragePrefix.GetString()
+			if strings.HasSuffix(storagePrefix, "/") {
+				log.Warningln("Removing trailing '/' from storage prefix", storagePrefix)
+				storagePrefix = strings.TrimSuffix(storagePrefix, "/")
+			}
+			originExport = OriginExport{
+				FederationPrefix: federationPrefix,
+				StoragePrefix:    storagePrefix,
+				Capabilities:     capabilities,
+			}
+
+			if err = validateExportPaths(originExport.StoragePrefix, originExport.FederationPrefix); err != nil {
+				return nil, err
+			}
 		}
 	case server_structs.OriginStorageS3:
 		// Handle exports configured via -v or potentially env vars