From 6b7a2f9b4bd88df33e3aea87fd5262b53f16fcbe Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Mon, 27 Nov 2023 22:20:43 +0000 Subject: [PATCH 01/14] Make Pelican able to use S3 xrootd plugin for unauthenticated S3 endpoints This is the first step in giving Pelican the ability to use an S3 backend. For now, it only covers unauthenticated buckets. Importantly, this doesn't do anything to actually make the plugin available to xrootd unless its installed by hand. That will come in a later commit. Add plugin to containers Right now, the RPM for the S3/HTTP plugin isn't functional, and we don't have that repo's GH set up to publish anything automatically. Until those can be sorted out, we'll clone from source to install. Concurrent to this commit, I'm opening a few issues at the plugin repo to handle getting the various GHA set up to take care of this. --- cmd/origin.go | 67 +++++++++++- cmd/origin_serve.go | 9 ++ docs/parameters.yaml | 35 +++++++ images/Dockerfile | 10 ++ origin_ui/origin.go | 41 ++++++++ utils/server_utils.go | 27 +++++ xrootd/authorization.go | 13 ++- xrootd/resources/xrootd-origin.cfg | 9 ++ xrootd/xrootd_config.go | 161 ++++++++++++++++------------- 9 files changed, 300 insertions(+), 72 deletions(-) diff --git a/cmd/origin.go b/cmd/origin.go index edfdddd1d..7105dafa3 100644 --- a/cmd/origin.go +++ b/cmd/origin.go @@ -109,12 +109,77 @@ func initOrigin() error { func init() { originCmd.AddCommand(originConfigCmd) originCmd.AddCommand(originServeCmd) - originServeCmd.Flags().StringP("volume", "v", "", "Setting the volue to /SRC:/DEST will export the contents of /SRC as /DEST in the Pelican federation") + + // 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')") + if err := viper.BindPFlag("Origin.Mode", originServeCmd.Flags().Lookup("mode")); err != nil { + panic(err) + } + + // The -v flag is used when an origin is served in POSIX mode + originServeCmd.Flags().StringP("volume", "v", "", "Setting the volume to /SRC:/DEST will export the contents of /SRC as /DEST in the Pelican federation") if err := viper.BindPFlag("Origin.ExportVolume", originServeCmd.Flags().Lookup("volume")); err != nil { panic(err) } + + // A variety of flags we add for S3 mode. These are ultimately required for configuring the S3 xrootd plugin + originServeCmd.Flags().String("service-name", "", "Specify the S3 service-name. Only used when an origin is launched in S3 mode.") + originServeCmd.Flags().String("region", "", "Specify the S3 region. Only used when an origin is launched in S3 mode.") + originServeCmd.Flags().String("bucket", "", "Specify the S3 bucket. Only used when an origin is launched in S3 mode.") + originServeCmd.Flags().String("service-url", "", "Specify the S3 service-url. Only used when an origin is launched in S3 mode.") + if err := viper.BindPFlag("Origin.S3ServiceName", originServeCmd.Flags().Lookup("service-name")); err != nil { + panic(err) + } + if err := viper.BindPFlag("Origin.S3Region", originServeCmd.Flags().Lookup("region")); err != nil { + panic(err) + } + if err := viper.BindPFlag("Origin.S3Bucket", originServeCmd.Flags().Lookup("bucket")); err != nil { + panic(err) + } + if err := viper.BindPFlag("Origin.S3ServiceUrl", originServeCmd.Flags().Lookup("service-url")); err != nil { + panic(err) + } + + // Would be nice to make these mutually exclusive to mode=posix instead of to --volume, but cobra + // doesn't seem to have something that can make the value of a flag exclusive to other flags + // Anyway, we never want to run the S3 flags with the -v flag. + originServeCmd.MarkFlagsMutuallyExclusive("volume", "service-name") + originServeCmd.MarkFlagsMutuallyExclusive("volume", "region") + originServeCmd.MarkFlagsMutuallyExclusive("volume", "bucket") + originServeCmd.MarkFlagsMutuallyExclusive("volume", "service-url") + originServeCmd.MarkFlagsRequiredTogether("service-name", "region", "bucket", "service-url") + + // Use PreRunE to mark certain flags as required based on the mode we're running in + originServeCmd.PreRunE = func(cmd *cobra.Command, args []string) error { + mode, _ := cmd.Flags().GetString("mode") + + switch mode { + case "posix": + err := cmd.MarkFlagRequired("volume") + cobra.CheckErr(err) + + case "s3": + err := cmd.MarkFlagRequired("service-name") + cobra.CheckErr(err) + err = cmd.MarkFlagRequired("region") + cobra.CheckErr(err) + err = cmd.MarkFlagRequired("bucket") + cobra.CheckErr(err) + err = cmd.MarkFlagRequired("service-url") + cobra.CheckErr(err) + + default: + return fmt.Errorf("unsupported mode: %s. Supported modes are 'posix' and 's3'.", mode) + } + + return nil + } + + // The port any web UI stuff will be served on originServeCmd.Flags().AddFlag(portFlag) + // origin token, used for creating and verifying tokens with + // the origin's signing jwk. originCmd.AddCommand(originTokenCmd) originTokenCmd.AddCommand(originTokenCreateCmd) originTokenCmd.PersistentFlags().String("profile", "wlcg", "Passing a profile ensures the token adheres to the profile's requirements. Accepted values are scitokens2 and wlcg") diff --git a/cmd/origin_serve.go b/cmd/origin_serve.go index 096e2e1db..f794e63e7 100644 --- a/cmd/origin_serve.go +++ b/cmd/origin_serve.go @@ -80,6 +80,15 @@ func serveOrigin( /*cmd*/ *cobra.Command /*args*/, []string) error { return err } wg.Add(1) + + // In posix mode, we rely on xrootd to export keys. When we run the origin with + // different backends, we instead export the keys via the Pelican process + if param.Origin_Mode.GetString() != "posix" { + if err = origin_ui.ConfigIssJWKS(engine.Group("/.well-known")); err != nil { + return err + } + } + if err = server_ui.RegisterNamespaceWithRetry(); err != nil { return err } diff --git a/docs/parameters.yaml b/docs/parameters.yaml index c7311bff5..814107c4a 100644 --- a/docs/parameters.yaml +++ b/docs/parameters.yaml @@ -355,6 +355,41 @@ description: >- downloads to work properly and for directories to be visable. type: bool default: false +--- +name: Origin.Mode +description: >- + The backend mode to be used by an origin. Current values that can be selected from + are either "posix" or "s3". +type: string +default: posix +components: ["origin"] +--- +name: Origin.S3ServiceName +description: >- + The S3 Service Name to be used by the XRootD plugin. +type: string +default: none +components: ["origin"] +--- +name: Origin.S3Region +description: >- + The S3 region to be used by the XRootD plugin. +type: string +default: none +components: ["origin"] +--- +name: Origin.S3Bucket +description: >- + The S3 bucket to be used by the XRootD plugin. +type: string +default: none +components: ["origin"] +--- +name: Origin.S3ServiceUrl +description: >- + The S3 service URL to be used by the XRootD plugin. +type: string +default: none components: ["origin"] --- diff --git a/images/Dockerfile b/images/Dockerfile index b7a8c5ce5..67942bfa7 100644 --- a/images/Dockerfile +++ b/images/Dockerfile @@ -122,6 +122,16 @@ ENV JAVA_HOME=/usr/lib/jvm/jre \ QDL_HOME="/opt/qdl" \ PATH="${ST_HOME}/bin:${QDL_HOME}/bin:${PATH}" +# Install the S3 and HTTP server plugins for XRootD. For now we do this from source +# until we can sort out the RPMs. +RUN \ + git clone https://github.com/PelicanPlatform/xrootd-s3-http.git && \ + cd xrootd-s3-http && \ + mkdir build && cd build && \ + cmake .. && \ + make install && \ + echo "/usr/local/lib" > /etc/ld.so.conf.d/xrdplugins.conf && ldconfig + RUN chmod +x /pelican/osdf-client \ && chmod +x /entrypoint.sh diff --git a/origin_ui/origin.go b/origin_ui/origin.go index d1ab7f23d..bd574de53 100644 --- a/origin_ui/origin.go +++ b/origin_ui/origin.go @@ -19,6 +19,16 @@ package origin_ui import ( + "embed" + "encoding/json" + "mime" + "net/http" + "net/url" + "os" + "path/filepath" + "strings" + + "github.com/gin-gonic/gin" "github.com/pelicanplatform/pelican/config" "github.com/pelicanplatform/pelican/param" "github.com/pkg/errors" @@ -56,3 +66,34 @@ func ConfigureXrootdMonitoringDir() error { return nil } + +func ConfigIssJWKS(router *gin.RouterGroup) error { + if router == nil { + return errors.New("Origin configuration passed a nil pointer") + } + + router.GET("/openid-configuration", ExportOpenIDConfig) + router.GET("/issuer.jwks", ExportIssuerJWKS) + return nil +} + +func ExportOpenIDConfig(c *gin.Context) { + issuerURL := url.URL{} + issuerURL.Scheme = "https" + issuerURL.Host = param.Server_ExternalWebUrl.GetString() + + jwksUri, _ := url.JoinPath(issuerURL.String(), "/.well-known/issuer.jwks") + jsonData := gin.H{ + "issuer": issuerURL.String(), + "jwks_uri": jwksUri, + } + + c.JSON(http.StatusOK, jsonData) +} + +func ExportIssuerJWKS(c *gin.Context) { + keys, _ := config.GetIssuerPublicJWKS() + buf, _ := json.MarshalIndent(keys, "", " ") + + c.Data(http.StatusOK, "application/json; charset=utf-8", buf) +} diff --git a/utils/server_utils.go b/utils/server_utils.go index 62ccfd215..fea5029eb 100644 --- a/utils/server_utils.go +++ b/utils/server_utils.go @@ -23,8 +23,10 @@ import ( "encoding/json" "io" "net/http" + "net/url" "github.com/pelicanplatform/pelican/config" + "github.com/pelicanplatform/pelican/param" "github.com/pkg/errors" ) @@ -58,3 +60,28 @@ func MakeRequest(url string, method string, data map[string]interface{}, headers return body, nil } + +func GetIssuerURL() (*url.URL, error) { + // If Origin.Mode is set to anything that isn't "posix" or "", assume we're running a plugin and + // that the origin's issuer URL actually uses the same port as OriginUI instead of XRootD. This is + // because under that condition, keys are being served by the Pelican process instead of by XRootD + originMode := param.Origin_Mode.GetString() + if originMode == "" || originMode == "posix" { + // In this case, we use the default set up by config.go, which uses the xrootd port + issuerUrl, err := url.Parse(param.Origin_Url.GetString()) + if err != nil { + return nil, errors.Wrap(err, "Failed to parse the issuer URL from the default origin URL") + } + return issuerUrl, nil + } else { + // to parse the URL, we first must prepend it with a scheme + issuerUrlStr := "https://" + param.Server_ExternalWebUrl.GetString() + + issuerUrl, err := url.Parse(issuerUrlStr) + if err != nil { + return nil, errors.Wrap(err, "Failed to parse the issuer URL generated using ComputeExternalAddress") + } + + return issuerUrl, nil + } +} diff --git a/xrootd/authorization.go b/xrootd/authorization.go index 13cd73ce0..96ca9e729 100644 --- a/xrootd/authorization.go +++ b/xrootd/authorization.go @@ -40,6 +40,7 @@ import ( "github.com/pelicanplatform/pelican/director" "github.com/pelicanplatform/pelican/param" "github.com/pelicanplatform/pelican/server_utils" + "github.com/pelicanplatform/pelican/utils" "github.com/pkg/errors" ) @@ -277,7 +278,11 @@ func GenerateMonitoringIssuer() (issuer Issuer, err error) { return } issuer.Name = "Built-in Monitoring" - issuer.Issuer = param.Origin_Url.GetString() + issuerUrl, err := utils.GetIssuerURL() + if err != nil { + return + } + issuer.Issuer = issuerUrl.String() issuer.BasePaths = []string{"/pelican/monitoring"} issuer.DefaultUser = "xrootd" @@ -290,7 +295,11 @@ func GenerateOriginIssuer(exportedPaths []string) (issuer Issuer, err error) { return } issuer.Name = "Origin" - issuer.Issuer = param.Origin_Url.GetString() + issuerUrl, err := utils.GetIssuerURL() + if err != nil { + return + } + issuer.Issuer = issuerUrl.String() issuer.BasePaths = exportedPaths issuer.RestrictedPaths = param.Origin_ScitokensRestrictedPaths.GetStringSlice() issuer.MapSubject = param.Origin_ScitokensMapSubject.GetBool() diff --git a/xrootd/resources/xrootd-origin.cfg b/xrootd/resources/xrootd-origin.cfg index 2cdc8559e..a86baad3b 100644 --- a/xrootd/resources/xrootd-origin.cfg +++ b/xrootd/resources/xrootd-origin.cfg @@ -50,7 +50,16 @@ xrd.report {{.Xrootd.SummaryMonitoringHost}}:{{.Xrootd.SummaryMonitoringPort}},1 xrootd.monitor all auth flush 30s window 5s fstat 60 lfn ops xfr 5 {{if .Xrootd.DetailedMonitoringHost -}} dest redir fstat info files user pfc tcpmon ccm {{.Xrootd.DetailedMonitoringHost}}:{{.Xrootd.DetailedMonitoringPort}} {{- end}} dest redir fstat info files user pfc tcpmon ccm 127.0.0.1:{{.Xrootd.LocalMonitoringPort}} all.adminpath {{.Xrootd.RunLocation}} all.pidpath {{.Xrootd.RunLocation}} +{{if eq .Origin.Mode "posix"}} oss.localroot {{.Xrootd.Mount}} +{{else if eq .Origin.Mode "s3"}} +ofs.osslib libXrdS3.so +# The S3 plugin doesn't currently support async mode +xrootd.async off +s3.service_name {{.Origin.S3ServiceName}} +s3.region {{.Origin.S3Region}} +s3.service_url {{.Origin.S3ServiceUrl}} +{{end}} xrootd.seclib libXrdSec.so sec.protocol ztn ofs.authorize 1 diff --git a/xrootd/xrootd_config.go b/xrootd/xrootd_config.go index cf23ea65c..8a843df96 100644 --- a/xrootd/xrootd_config.go +++ b/xrootd/xrootd_config.go @@ -39,13 +39,18 @@ var ( type ( OriginConfig struct { - Multiuser bool - EnableCmsd bool - EnableMacaroons bool - EnableVoms bool + Multiuser bool + EnableCmsd bool + EnableMacaroons bool + EnableVoms bool EnableDirListing bool - SelfTest bool - NamespacePrefix string + SelfTest bool + NamespacePrefix string + Mode string + S3Bucket string + S3Region string + S3ServiceName string + S3ServiceUrl string } CacheConfig struct { @@ -90,73 +95,91 @@ type ( ) func CheckOriginXrootdEnv(exportPath string, uid int, gid int, groupname string) (string, error) { - // If we use "volume mount" style options, configure the export directories. - volumeMount := param.Origin_ExportVolume.GetString() - if volumeMount != "" { - volumeMount, err := filepath.Abs(volumeMount) - if err != nil { - return exportPath, err - } - volumeMountSrc := volumeMount - volumeMountDst := volumeMount - volumeMountInfo := strings.SplitN(volumeMount, ":", 2) - if len(volumeMountInfo) == 2 { - volumeMountSrc = volumeMountInfo[0] - volumeMountDst = volumeMountInfo[1] - } - volumeMountDst = filepath.Clean(volumeMountDst) - if volumeMountDst == "" { - return exportPath, fmt.Errorf("Export volume %v has empty destination path", volumeMount) - } - if volumeMountDst[0:1] != "/" { - return "", fmt.Errorf("Export volume %v has a relative destination path", - volumeMountDst) - } - destPath := path.Clean(filepath.Join(exportPath, volumeMountDst[1:])) - err = config.MkdirAll(filepath.Dir(destPath), 0755, uid, gid) - if err != nil { - return exportPath, errors.Wrapf(err, "Unable to create export directory %v", - filepath.Dir(destPath)) - } - err = os.Symlink(volumeMountSrc, destPath) - if err != nil { - return exportPath, errors.Wrapf(err, "Failed to create export symlink") - } - viper.Set("Origin.NamespacePrefix", volumeMountDst) - } else { - mountPath := param.Xrootd_Mount.GetString() - namespacePrefix := param.Origin_NamespacePrefix.GetString() - if mountPath == "" || namespacePrefix == "" { - return exportPath, errors.New(`Export information was not provided. - Add command line flag: + originMode := param.Origin_Mode.GetString() + if originMode == "posix" { + // If we use "volume mount" style options, configure the export directories. + volumeMount := param.Origin_ExportVolume.GetString() + if volumeMount != "" { + volumeMount, err := filepath.Abs(volumeMount) + if err != nil { + return exportPath, err + } + volumeMountSrc := volumeMount + volumeMountDst := volumeMount + volumeMountInfo := strings.SplitN(volumeMount, ":", 2) + if len(volumeMountInfo) == 2 { + volumeMountSrc = volumeMountInfo[0] + volumeMountDst = volumeMountInfo[1] + } + volumeMountDst = filepath.Clean(volumeMountDst) + if volumeMountDst == "" { + return exportPath, fmt.Errorf("Export volume %v has empty destination path", volumeMount) + } + if volumeMountDst[0:1] != "/" { + return "", fmt.Errorf("Export volume %v has a relative destination path", + volumeMountDst) + } + destPath := path.Clean(filepath.Join(exportPath, volumeMountDst[1:])) + err = config.MkdirAll(filepath.Dir(destPath), 0755, uid, gid) + if err != nil { + return exportPath, errors.Wrapf(err, "Unable to create export directory %v", + filepath.Dir(destPath)) + } + err = os.Symlink(volumeMountSrc, destPath) + if err != nil { + return exportPath, errors.Wrapf(err, "Failed to create export symlink") + } + viper.Set("Origin.NamespacePrefix", volumeMountDst) + } else { + mountPath := param.Xrootd_Mount.GetString() + namespacePrefix := param.Origin_NamespacePrefix.GetString() + if mountPath == "" || namespacePrefix == "" { + return exportPath, errors.New(`Export information was not provided. + Add command line flag: - -v /mnt/foo:/bar + -v /mnt/foo:/bar - to export the directory /mnt/foo to the path /bar in the data federation`) - } - mountPath, err := filepath.Abs(mountPath) - if err != nil { - return exportPath, err - } - mountPath = filepath.Clean(mountPath) - namespacePrefix = filepath.Clean(namespacePrefix) - if namespacePrefix[0:1] != "/" { - return exportPath, fmt.Errorf("Namespace prefix %v must have an absolute path", - namespacePrefix) - } - destPath := path.Clean(filepath.Join(exportPath, namespacePrefix[1:])) - err = config.MkdirAll(filepath.Dir(destPath), 0755, uid, gid) - if err != nil { - return exportPath, errors.Wrapf(err, "Unable to create export directory %v", - filepath.Dir(destPath)) - } - srcPath := filepath.Join(mountPath, namespacePrefix[1:]) - err = os.Symlink(srcPath, destPath) - if err != nil { - return exportPath, errors.Wrapf(err, "Failed to create export symlink") + to export the directory /mnt/foo to the path /bar in the data federation`) + } + mountPath, err := filepath.Abs(mountPath) + if err != nil { + return exportPath, err + } + mountPath = filepath.Clean(mountPath) + namespacePrefix = filepath.Clean(namespacePrefix) + if namespacePrefix[0:1] != "/" { + return exportPath, fmt.Errorf("Namespace prefix %v must have an absolute path", + namespacePrefix) + } + destPath := path.Clean(filepath.Join(exportPath, namespacePrefix[1:])) + err = config.MkdirAll(filepath.Dir(destPath), 0755, uid, gid) + if err != nil { + return exportPath, errors.Wrapf(err, "Unable to create export directory %v", + filepath.Dir(destPath)) + } + srcPath := filepath.Join(mountPath, namespacePrefix[1:]) + err = os.Symlink(srcPath, destPath) + if err != nil { + return exportPath, errors.Wrapf(err, "Failed to create export symlink") + } } + viper.Set("Xrootd.Mount", exportPath) + + } else if originMode == "s3" { + // Our "namespace prefix" is actually just + // /// + nsPrefix := filepath.Join("/", param.Origin_S3ServiceName.GetString(), + param.Origin_S3Region.GetString(), param.Origin_S3Bucket.GetString()) + viper.Set("Origin.NamespacePrefix", nsPrefix) } - viper.Set("Xrootd.Mount", exportPath) + + + + + + + + if param.Origin_SelfTest.GetBool() { if err := origin_ui.ConfigureXrootdMonitoringDir(); err != nil { From 27a97aac93fa3909548700f67d31ccc936e50fa9 Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Tue, 28 Nov 2023 22:24:53 +0000 Subject: [PATCH 02/14] Make origins servable via supervisor in production container This adds a supervisor script for serving origins, as well as updates some of the dependencies needed to install the s3/http plugin and get an origin running with all of its other plugins. --- images/Dockerfile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/images/Dockerfile b/images/Dockerfile index 67942bfa7..232da4ee0 100644 --- a/images/Dockerfile +++ b/images/Dockerfile @@ -130,9 +130,11 @@ RUN \ mkdir build && cd build && \ cmake .. && \ make install && \ + # For now, until the RPM is set up, we install the libraries here, but + # we need to add to LD_LIBRARY_PATH so XRootD knows where to look echo "/usr/local/lib" > /etc/ld.so.conf.d/xrdplugins.conf && ldconfig RUN chmod +x /pelican/osdf-client \ && chmod +x /entrypoint.sh -ENTRYPOINT ["/entrypoint.sh"] +#ENTRYPOINT ["/entrypoint.sh"] From 0696d8aaaa0a2113c12dade0c2a0aee423be285a Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Tue, 28 Nov 2023 23:27:49 +0000 Subject: [PATCH 03/14] More cleanup from rebase --- origin_ui/origin.go | 5 +---- utils/server_utils.go | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/origin_ui/origin.go b/origin_ui/origin.go index bd574de53..aa90cfe17 100644 --- a/origin_ui/origin.go +++ b/origin_ui/origin.go @@ -78,10 +78,7 @@ func ConfigIssJWKS(router *gin.RouterGroup) error { } func ExportOpenIDConfig(c *gin.Context) { - issuerURL := url.URL{} - issuerURL.Scheme = "https" - issuerURL.Host = param.Server_ExternalWebUrl.GetString() - + issuerURL, _ := url.Parse(param.Server_ExternalWebUrl.GetString()) jwksUri, _ := url.JoinPath(issuerURL.String(), "/.well-known/issuer.jwks") jsonData := gin.H{ "issuer": issuerURL.String(), diff --git a/utils/server_utils.go b/utils/server_utils.go index fea5029eb..cb7ae6144 100644 --- a/utils/server_utils.go +++ b/utils/server_utils.go @@ -75,7 +75,7 @@ func GetIssuerURL() (*url.URL, error) { return issuerUrl, nil } else { // to parse the URL, we first must prepend it with a scheme - issuerUrlStr := "https://" + param.Server_ExternalWebUrl.GetString() + issuerUrlStr := param.Server_ExternalWebUrl.GetString() issuerUrl, err := url.Parse(issuerUrlStr) if err != nil { From 5d550c5c4cb1b6c8239c43785735a185b4e73192 Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Wed, 29 Nov 2023 14:57:09 +0000 Subject: [PATCH 04/14] Cleanup naming and remove outdated comment --- utils/server_utils.go | 4 +--- xrootd/authorization.go | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/utils/server_utils.go b/utils/server_utils.go index cb7ae6144..da1cd8635 100644 --- a/utils/server_utils.go +++ b/utils/server_utils.go @@ -61,7 +61,7 @@ func MakeRequest(url string, method string, data map[string]interface{}, headers return body, nil } -func GetIssuerURL() (*url.URL, error) { +func GetLocalIssuerURL() (*url.URL, error) { // If Origin.Mode is set to anything that isn't "posix" or "", assume we're running a plugin and // that the origin's issuer URL actually uses the same port as OriginUI instead of XRootD. This is // because under that condition, keys are being served by the Pelican process instead of by XRootD @@ -74,9 +74,7 @@ func GetIssuerURL() (*url.URL, error) { } return issuerUrl, nil } else { - // to parse the URL, we first must prepend it with a scheme issuerUrlStr := param.Server_ExternalWebUrl.GetString() - issuerUrl, err := url.Parse(issuerUrlStr) if err != nil { return nil, errors.Wrap(err, "Failed to parse the issuer URL generated using ComputeExternalAddress") diff --git a/xrootd/authorization.go b/xrootd/authorization.go index 96ca9e729..ea9c9edf6 100644 --- a/xrootd/authorization.go +++ b/xrootd/authorization.go @@ -278,7 +278,7 @@ func GenerateMonitoringIssuer() (issuer Issuer, err error) { return } issuer.Name = "Built-in Monitoring" - issuerUrl, err := utils.GetIssuerURL() + issuerUrl, err := utils.GetLocalIssuerURL() if err != nil { return } @@ -295,7 +295,7 @@ func GenerateOriginIssuer(exportedPaths []string) (issuer Issuer, err error) { return } issuer.Name = "Origin" - issuerUrl, err := utils.GetIssuerURL() + issuerUrl, err := utils.GetLocalIssuerURL() if err != nil { return } From a0b3e583a821e3b7e8b345abd4dd4be29da59b3d Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Wed, 29 Nov 2023 22:40:23 +0000 Subject: [PATCH 05/14] Add boilerplate to allow an authenticated bucket This adds what we need to set an access/secret key for the S3 plugin. HOWEVER, upon trying to test this locally with MinIO, I found some bugs in the actual plugin that prevent authenticated buckets from working in the first place. That being said, that's a problem with the plugin and not with Pelican. All Pelican needs to do is to run xrootd with the right `s3` directives, and that appears to be the case with these changes. --- cmd/origin.go | 12 ++++++++++++ docs/parameters.yaml | 15 +++++++++++++++ xrootd/resources/xrootd-origin.cfg | 6 ++++++ xrootd/xrootd_config.go | 2 ++ 4 files changed, 35 insertions(+) diff --git a/cmd/origin.go b/cmd/origin.go index 7105dafa3..a48b797c3 100644 --- a/cmd/origin.go +++ b/cmd/origin.go @@ -127,6 +127,8 @@ func init() { originServeCmd.Flags().String("region", "", "Specify the S3 region. Only used when an origin is launched in S3 mode.") originServeCmd.Flags().String("bucket", "", "Specify the S3 bucket. Only used when an origin is launched in S3 mode.") originServeCmd.Flags().String("service-url", "", "Specify the S3 service-url. Only used when an origin is launched in S3 mode.") + originServeCmd.Flags().String("bucket-access-keyfile", "", "Specify a filepath to use for configuring the bucket's access key.") + originServeCmd.Flags().String("bucket-secret-keyfile", "", "Specify a filepath to use for configuring the bucket's access key.") if err := viper.BindPFlag("Origin.S3ServiceName", originServeCmd.Flags().Lookup("service-name")); err != nil { panic(err) } @@ -139,6 +141,12 @@ func init() { if err := viper.BindPFlag("Origin.S3ServiceUrl", originServeCmd.Flags().Lookup("service-url")); err != nil { panic(err) } + if err := viper.BindPFlag("Origin.S3AccessKeyfile", originServeCmd.Flags().Lookup("bucket-access-keyfile")); err != nil { + panic(err) + } + if err := viper.BindPFlag("Origin.S3SecretKeyfile", originServeCmd.Flags().Lookup("bucket-secret-keyfile")); err != nil { + panic(err) + } // Would be nice to make these mutually exclusive to mode=posix instead of to --volume, but cobra // doesn't seem to have something that can make the value of a flag exclusive to other flags @@ -147,7 +155,11 @@ func init() { originServeCmd.MarkFlagsMutuallyExclusive("volume", "region") originServeCmd.MarkFlagsMutuallyExclusive("volume", "bucket") originServeCmd.MarkFlagsMutuallyExclusive("volume", "service-url") + originServeCmd.MarkFlagsMutuallyExclusive("volume", "bucket-access-keyfile") + originServeCmd.MarkFlagsMutuallyExclusive("volume", "bucket-secret-keyfile") + // We don't require the bucket access and secret keyfiles as they're not needed for unauthenticated buckets originServeCmd.MarkFlagsRequiredTogether("service-name", "region", "bucket", "service-url") + originServeCmd.MarkFlagsRequiredTogether("bucket-access-keyfile", "bucket-secret-keyfile") // Use PreRunE to mark certain flags as required based on the mode we're running in originServeCmd.PreRunE = func(cmd *cobra.Command, args []string) error { diff --git a/docs/parameters.yaml b/docs/parameters.yaml index 814107c4a..5ee843a55 100644 --- a/docs/parameters.yaml +++ b/docs/parameters.yaml @@ -392,6 +392,21 @@ type: string default: none components: ["origin"] --- +name: Origin.S3AccessKeyfile +description: >- + A path to a file containing an S3 access keyfile for authenticated buckets when an origin is run in S3 mode. +type: filename +default: none +components: ["origin"] +--- +name: Origin.S3SecretKeyfile +description: >- + A path to a file containing an S3 secret keyfile for authenticated buckets when an origin is run in S3 mode. +type: filename +default: none +components: ["origin"] +--- + ############################ # Cache-level configs # diff --git a/xrootd/resources/xrootd-origin.cfg b/xrootd/resources/xrootd-origin.cfg index a86baad3b..e7d257f38 100644 --- a/xrootd/resources/xrootd-origin.cfg +++ b/xrootd/resources/xrootd-origin.cfg @@ -59,6 +59,12 @@ xrootd.async off s3.service_name {{.Origin.S3ServiceName}} s3.region {{.Origin.S3Region}} s3.service_url {{.Origin.S3ServiceUrl}} +{{- if .Origin.S3AccessKeyfile}} +s3.access_key_file {{.Origin.S3AccessKeyfile}} +{{- end -}} +{{if .Origin.S3SecretKeyfile}} +s3.secret_key_file {{.Origin.S3SecretKeyfile}} +{{- end}} {{end}} xrootd.seclib libXrdSec.so sec.protocol ztn diff --git a/xrootd/xrootd_config.go b/xrootd/xrootd_config.go index 8a843df96..06e5ecddd 100644 --- a/xrootd/xrootd_config.go +++ b/xrootd/xrootd_config.go @@ -51,6 +51,8 @@ type ( S3Region string S3ServiceName string S3ServiceUrl string + S3AccessKeyfile string + S3SecretKeyfile string } CacheConfig struct { From 50235d9769f880fdd0159af948810b7b03c66a02 Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Mon, 4 Dec 2023 21:26:19 +0000 Subject: [PATCH 06/14] Add very basic test for S3 origin and add dependency to dev.Dockerfile This adds a very basic test for the S3 origin -- basically it only checks that we can start an xrootd S3 server with the provided configuration. The reason for this punt on the E2E testing is that I'm waiting for things like `originServe` to be unwrapped from the cobra cli, because otherwise there's an awful lot of setup to do. Since I don't actually test a file download, there's technically no need to set up the MinIO server in the test, but that's there and ready when those other pieces are figured out. The last bit is that for these tests to run, the minio server needs to be installed. With changes to `dev.Dockerfile`, the tests should be runnable. --- cmd/origin.go | 26 ---- go.mod | 8 +- go.sum | 20 ++- images/Dockerfile | 2 +- xrootd/origin_linux_test.go | 243 ++++++++++++++++++++++++++++++++++++ xrootd/origin_test.go | 137 -------------------- 6 files changed, 264 insertions(+), 172 deletions(-) create mode 100644 xrootd/origin_linux_test.go delete mode 100644 xrootd/origin_test.go diff --git a/cmd/origin.go b/cmd/origin.go index a48b797c3..331ba94fc 100644 --- a/cmd/origin.go +++ b/cmd/origin.go @@ -161,32 +161,6 @@ func init() { originServeCmd.MarkFlagsRequiredTogether("service-name", "region", "bucket", "service-url") originServeCmd.MarkFlagsRequiredTogether("bucket-access-keyfile", "bucket-secret-keyfile") - // Use PreRunE to mark certain flags as required based on the mode we're running in - originServeCmd.PreRunE = func(cmd *cobra.Command, args []string) error { - mode, _ := cmd.Flags().GetString("mode") - - switch mode { - case "posix": - err := cmd.MarkFlagRequired("volume") - cobra.CheckErr(err) - - case "s3": - err := cmd.MarkFlagRequired("service-name") - cobra.CheckErr(err) - err = cmd.MarkFlagRequired("region") - cobra.CheckErr(err) - err = cmd.MarkFlagRequired("bucket") - cobra.CheckErr(err) - err = cmd.MarkFlagRequired("service-url") - cobra.CheckErr(err) - - default: - return fmt.Errorf("unsupported mode: %s. Supported modes are 'posix' and 's3'.", mode) - } - - return nil - } - // The port any web UI stuff will be served on originServeCmd.Flags().AddFlag(portFlag) diff --git a/go.mod b/go.mod index 5ec6ebcc9..c8439058d 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/jellydator/ttlcache/v3 v3.1.0 github.com/jsipprell/keyctl v1.0.4-0.20211208153515-36ca02672b6c github.com/lestrrat-go/jwx/v2 v2.0.16 + github.com/minio/minio-go/v7 v7.0.65 github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f github.com/oklog/run v1.1.0 github.com/opensaucerer/grab/v3 v3.0.1 @@ -21,7 +22,7 @@ require ( github.com/prometheus/client_golang v1.16.0 github.com/prometheus/common v0.44.0 github.com/prometheus/prometheus v0.46.0 - github.com/sirupsen/logrus v1.8.1 + github.com/sirupsen/logrus v1.9.3 github.com/spf13/cobra v1.7.0 github.com/spf13/pflag v1.0.5 github.com/spf13/viper v1.16.0 @@ -95,7 +96,7 @@ require ( github.com/julienschmidt/httprouter v1.3.0 // indirect github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect github.com/klauspost/compress v1.16.7 // indirect - github.com/klauspost/cpuid/v2 v2.2.4 // indirect + github.com/klauspost/cpuid/v2 v2.2.5 // indirect github.com/kylelemons/godebug v1.1.0 // indirect github.com/leodido/go-urn v1.2.4 // indirect github.com/lestrrat-go/blackmagic v1.0.2 // indirect @@ -109,6 +110,8 @@ require ( github.com/mattn/go-runewidth v0.0.15 // indirect github.com/mattn/go-sqlite3 v1.14.17 // indirect github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect + github.com/minio/md5-simd v1.1.2 // indirect + github.com/minio/sha256-simd v1.0.1 // indirect github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect @@ -126,6 +129,7 @@ require ( github.com/prometheus/procfs v0.11.0 // indirect github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect github.com/rivo/uniseg v0.4.4 // indirect + github.com/rs/xid v1.5.0 // indirect github.com/segmentio/asm v1.2.0 // indirect github.com/shurcooL/httpfs v0.0.0-20230704072500-f1e31cf0ba5c // indirect github.com/spf13/afero v1.9.5 // indirect diff --git a/go.sum b/go.sum index 390ac07d7..22404a592 100644 --- a/go.sum +++ b/go.sum @@ -377,9 +377,10 @@ github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+o github.com/klauspost/compress v1.13.6/go.mod h1:/3/Vjq9QcHkK5uEr5lBEmyoZ1iFhe47etQ6QUkpK6sk= github.com/klauspost/compress v1.16.7 h1:2mk3MPGNzKyxErAw8YaohYh69+pa4sIQSC0fPGCFR9I= github.com/klauspost/compress v1.16.7/go.mod h1:ntbaceVETuRiXiv4DpjP66DpAtAGkEQskQzEyD//IeE= +github.com/klauspost/cpuid/v2 v2.0.1/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= github.com/klauspost/cpuid/v2 v2.0.9/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= -github.com/klauspost/cpuid/v2 v2.2.4 h1:acbojRNwl3o09bUq+yDCtZFc1aiwaAAxtcn8YkZXnvk= -github.com/klauspost/cpuid/v2 v2.2.4/go.mod h1:RVVoqg1df56z8g3pUjL/3lE5UfnlrJX8tyFgg4nqhuY= +github.com/klauspost/cpuid/v2 v2.2.5 h1:0E5MSMDEoAulmXNFquVs//DdoomxaoTY1kUhbc/qbZg= +github.com/klauspost/cpuid/v2 v2.2.5/go.mod h1:Lcz8mBdAVJIBVzewtcLocK12l3Y+JytZYpaMropDUws= github.com/kolo/xmlrpc v0.0.0-20220921171641-a4b6fa1dd06b h1:udzkj9S/zlT5X367kqJis0QP7YMxobob6zhzq6Yre00= github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/konsorten/go-windows-terminal-sequences v1.0.2/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= @@ -431,6 +432,12 @@ github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5 github.com/matttproud/golang_protobuf_extensions v1.0.4 h1:mmDVorXM7PCGKw94cs5zkfA9PSy5pEvNWRP0ET0TIVo= github.com/matttproud/golang_protobuf_extensions v1.0.4/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4= github.com/miekg/dns v1.1.55 h1:GoQ4hpsj0nFLYe+bWiCToyrBEJXkQfOOIvFGFy0lEgo= +github.com/minio/md5-simd v1.1.2 h1:Gdi1DZK69+ZVMoNHRXJyNcxrMA4dSxoYHZSQbirFg34= +github.com/minio/md5-simd v1.1.2/go.mod h1:MzdKDxYpY2BT9XQFocsiZf/NKVtR7nkE4RoEpN+20RM= +github.com/minio/minio-go/v7 v7.0.65 h1:sOlB8T3nQK+TApTpuN3k4WD5KasvZIE3vVFzyyCa0go= +github.com/minio/minio-go/v7 v7.0.65/go.mod h1:R4WVUR6ZTedlCcGwZRauLMIKjgyaWxhs4Mqi/OMPmEc= +github.com/minio/sha256-simd v1.0.1 h1:6kaan5IFmwTNynnKKpDHe6FWHohJOHhCPchzK49dzMM= +github.com/minio/sha256-simd v1.0.1/go.mod h1:Pz6AKMiUdngCLpeTL/RJY1M9rUuPMYujV5xJjtbRSN8= github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y= github.com/mitchellh/mapstructure v1.3.3/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo= github.com/mitchellh/mapstructure v1.4.1/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo= @@ -520,6 +527,8 @@ github.com/rogpeppe/go-internal v1.1.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFR github.com/rogpeppe/go-internal v1.2.2/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ= +github.com/rs/xid v1.5.0 h1:mKX4bl4iPYJtEIxp6CYiUuLQ/8DYMoz0PUdtGgMFRVc= +github.com/rs/xid v1.5.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/scaleway/scaleway-sdk-go v1.0.0-beta.19 h1:+1H+N9QFl2Sfvia0FBYfMrHYHYhmpZxhSE0wpPL2lYs= github.com/segmentio/asm v1.2.0 h1:9BQrFxC+YOHJlTlHGkTrFWf59nbL3XnCoFLTwDCI7ys= @@ -531,8 +540,8 @@ github.com/sirupsen/logrus v1.4.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPx github.com/sirupsen/logrus v1.4.1/go.mod h1:ni0Sbl8bgC9z8RoU9G6nDWqqs/fq4eDPysMBDgk/93Q= github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE= github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrfsX/uA88= -github.com/sirupsen/logrus v1.8.1 h1:dJKuHgqk1NNQlqoA6BTlM1Wf9DOH3NBjQyu0h9+AZZE= -github.com/sirupsen/logrus v1.8.1/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0= +github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ= +github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= github.com/spf13/afero v1.9.5 h1:stMpOSZFs//0Lv29HduCmli3GUfpFoF3Y1Q/aXj/wVM= github.com/spf13/afero v1.9.5/go.mod h1:UBogFpq8E9Hx+xc5CNTTEpTnuHVmXDwZcZcE1eb/UhQ= github.com/spf13/cast v1.5.1 h1:R+kOtfhWQE6TVQzY+4D7wJLBgkdVasCEFxSUBYBYIlA= @@ -762,7 +771,6 @@ golang.org/x/sys v0.0.0-20190606165138-5da285871e9c/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20190624142023-c5567b49c5d0/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190726091711-fc99dfbffb4e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191001151750-bb3f8db39f24/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191204072324-ce4227a45e2e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191228213918-04cbcbbfeed8/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200106162015-b016eb3dc98e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -797,7 +805,7 @@ golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20210616045830-e2b7044e8c71/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220704084225-05e143d24a9e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= diff --git a/images/Dockerfile b/images/Dockerfile index 232da4ee0..aa503cf33 100644 --- a/images/Dockerfile +++ b/images/Dockerfile @@ -137,4 +137,4 @@ RUN \ RUN chmod +x /pelican/osdf-client \ && chmod +x /entrypoint.sh -#ENTRYPOINT ["/entrypoint.sh"] +ENTRYPOINT ["/entrypoint.sh"] diff --git a/xrootd/origin_linux_test.go b/xrootd/origin_linux_test.go new file mode 100644 index 000000000..63347c445 --- /dev/null +++ b/xrootd/origin_linux_test.go @@ -0,0 +1,243 @@ +/*************************************************************** + * + * Copyright (C) 2023, 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. + * + ***************************************************************/ + +package xrootd + +import ( + "bytes" + "context" + "crypto/elliptic" + "fmt" + "net/http" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/minio/minio-go/v7" + "github.com/minio/minio-go/v7/pkg/credentials" + "os/exec" + + "github.com/pelicanplatform/pelican/config" + "github.com/pelicanplatform/pelican/daemon" + "github.com/pelicanplatform/pelican/origin_ui" + "github.com/pelicanplatform/pelican/param" + log "github.com/sirupsen/logrus" + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func originMockup(t *testing.T) context.CancelFunc { + // Create our own temp directory (for some reason t.TempDir() does not play well with xrootd) + tmpPath := "/tmp/XRootD-Test_Origin" + viper.Set("ConfigDir", tmpPath) + viper.Set("Xrootd.RunLocation", filepath.Join(tmpPath, "xrootd")) + t.Cleanup(func() { + os.RemoveAll(tmpPath) + }) + // Increase the log level; otherwise, its difficult to debug failures + viper.Set("Logging.Level", "Debug") + config.InitConfig() + err := config.InitServer() + + require.NoError(t, err) + + err = config.GeneratePrivateKey(param.Server_TLSKey.GetString(), elliptic.P256()) + require.NoError(t, err) + err = config.GenerateCert() + require.NoError(t, err) + + err = CheckXrootdEnv(true, nil) + require.NoError(t, err) + + err = SetUpMonitoring() + require.NoError(t, err) + + configPath, err := ConfigXrootd(true) + require.NoError(t, err) + + launchers, err := ConfigureLaunchers(false, configPath, false) + require.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + go func() { + _ = daemon.LaunchDaemons(ctx, launchers) + }() + return cancel + +} + +func TestOrigin(t *testing.T) { + viper.Reset() + + viper.Set("Origin.ExportVolume", t.TempDir()+":/test") + viper.Set("Origin.Mode", "posix") + // Disable functionality we're not using (and is difficult to make work on Mac) + viper.Set("Origin.EnableCmsd", false) + viper.Set("Origin.EnableMacaroons", false) + viper.Set("Origin.EnableVoms", false) + viper.Set("TLSSkipVerify", true) + + cancel := originMockup(t) + defer cancel() + + testExpiry := time.Now().Add(10 * time.Second) + testSuccess := false + for !(testSuccess || time.Now().After(testExpiry)) { + time.Sleep(50 * time.Millisecond) + req, err := http.NewRequest("GET", param.Origin_Url.GetString(), nil) + require.NoError(t, err) + httpClient := http.Client{ + Transport: config.GetTransport(), + Timeout: 50 * time.Millisecond, + } + _, err = httpClient.Do(req) + if err != nil { + log.Infoln("Failed to send request to XRootD; likely, server is not up (will retry in 50ms):", err) + } else { + testSuccess = true + log.Debugln("XRootD server appears to be functioning; will proceed with test") + } + } + + if testSuccess { + url, err := origin_ui.UploadTestfile() + require.NoError(t, err) + err = origin_ui.DownloadTestfile(url) + require.NoError(t, err) + err = origin_ui.DeleteTestfile(url) + require.NoError(t, err) + } else { + t.Fatalf("Unsucessful test: timeout when trying to send request to xrootd") + } + viper.Reset() +} + +func TestS3OriginConfig(t *testing.T) { + viper.Reset() + tmpDir := t.TempDir() + + // We need to start up a minio server. Open to better ways to do this! + minIOServerCmd := exec.Command("minio", "server", "--quiet", tmpDir) + minIOServerCmd.Env = []string{fmt.Sprintf("HOME=%s", tmpDir)} + + // Create a few buffers to grab outputs + var outb, errb bytes.Buffer + minIOServerCmd.Stdout = &outb + minIOServerCmd.Stderr = &errb + + err := minIOServerCmd.Start() + // Wait for the server to initialize. Hopefully this always happens in under 2 seconds! + time.Sleep(time.Second * 2) + + // Check for any errors in the outputs + if strings.Contains(strings.ToLower(outb.String()), "error") { + t.Fatalf("Could not start the MinIO server: %s", outb.String()) + } else if errb.String() != "" { + t.Fatalf("Could not start the MinIO server: %s", errb.String()) + } + // Check for other types of errors that might have been passed back through the process + assert.NoError(t, err) + defer func() { + err = minIOServerCmd.Process.Kill() + assert.NoError(t, err) + }() + + // MinIO is running (by default at localhost:9000), let's create an unauthenticated bucket + // First we set up a client instance + endpoint := "localhost:9000" + // By default, the endpoint will require access/secret key with these values. Note that this doesn't + // necessarily mean the bucket we create needs those keys, as the bucket will have its own IAM + accessKey := "minioadmin" + secretKey := "minioadmin" + useSSL := false + + minioClient, err := minio.New(endpoint, &minio.Options{ + Creds: credentials.NewStaticV4(accessKey, secretKey, ""), + Secure: useSSL, + }) + assert.NoError(t, err) + + // Create a new unauthenticated bucket. Under the hood, this will access the minio server endpoint + // and do a PUT + bucketName := "test-bucket" + regionName := "test-region" + serviceName := "test-name" + err = minioClient.MakeBucket(context.Background(), bucketName, minio.MakeBucketOptions{}) + assert.NoError(t, err) + + // Set bucket policy for public read access + policy := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":"*","Action":["s3:GetObject"],"Resource":["arn:aws:s3:::` + bucketName + `/*"]}]}` + err = minioClient.SetBucketPolicy(context.Background(), bucketName, policy) + assert.NoError(t, err) + + // Upload a test file to the bucket + testFilePath := filepath.Join(tmpDir, "test-file.txt") + content := []byte("This is the content of the test file.") + err = os.WriteFile(testFilePath, content, 0644) + assert.NoError(t, err) + + objectName := "test-file.txt" + contentType := "application/octet-stream" + _, err = minioClient.FPutObject(context.Background(), bucketName, objectName, testFilePath, minio.PutObjectOptions{ContentType: contentType}) + assert.NoError(t, err) + + // All the setup to create the S3 server, add a bucket with a publicly-readable object is done. Now onto Pelican stuff + // Set up the origin and try to pull a file + viper.Set("Origin.Mode", "s3") + viper.Set("Origin.S3Region", regionName) + viper.Set("Origin.S3Bucket", bucketName) + viper.Set("Origin.S3ServiceName", serviceName) + viper.Set("Origin.S3ServiceUrl", fmt.Sprintf("http://%s", endpoint)) + // Disable functionality we're not using (and is difficult to make work on Mac) + viper.Set("Origin.EnableCmsd", false) + viper.Set("Origin.EnableMacaroons", false) + viper.Set("Origin.EnableVoms", false) + viper.Set("Origin.SelfTest", false) + viper.Set("TLSSkipVerify", true) + + cancel := originMockup(t) + defer cancel() + + // FOR NOW, we consider the test a success if the origin's xrootd server boots. + // TODO: When we've made it easier to come back and test whole pieces of this process by disentangling our + // *serve* commands, come back and make this e2e. The reason I'm punting is that in S3 mode, we also need all + // of the web_ui stuff initialized to export the public signing keys (as we can't export via XRootD) and we + // need a real token. These become much easier when we have an internally workable set of commands to do so. + + // Wait for xrootd to initialize -- hopefully this always happens in 2 seconds! + time.Sleep(2 * time.Second) + // The file is accessed at ${OriginURL}//// + req, err := http.NewRequest("GET", fmt.Sprintf("%s/%s/%s/%s/%s", param.Origin_Url.GetString(), serviceName, regionName, bucketName, objectName), nil) + require.NoError(t, err) + httpClient := http.Client{ + Transport: config.GetTransport(), + } + resp, err := httpClient.Do(req) + assert.NoError(t, err) + + // Until we sort out the things we mentioned above, we should expect a 403 because we don't try to pass tokens + // and we don't actually export any keys for token validation. + assert.Equal(t, 403, resp.StatusCode) + + // One other quick check to do is that the namespace was correctly parsed: + assert.Equal(t, fmt.Sprintf("/%s/%s/%s", serviceName, regionName, bucketName), param.Origin_NamespacePrefix.GetString()) + viper.Reset() +} diff --git a/xrootd/origin_test.go b/xrootd/origin_test.go deleted file mode 100644 index 734b90917..000000000 --- a/xrootd/origin_test.go +++ /dev/null @@ -1,137 +0,0 @@ -//go:build linux - -/*************************************************************** - * - * Copyright (C) 2023, 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. - * - ***************************************************************/ - -package xrootd - -import ( - "context" - "crypto/elliptic" - "net/http" - "os" - "path/filepath" - "sync" - "testing" - "time" - - "github.com/pelicanplatform/pelican/config" - "github.com/pelicanplatform/pelican/daemon" - "github.com/pelicanplatform/pelican/origin_ui" - "github.com/pelicanplatform/pelican/param" - "github.com/pelicanplatform/pelican/utils" - log "github.com/sirupsen/logrus" - "github.com/spf13/viper" - "github.com/stretchr/testify/require" -) - -func TestOrigin(t *testing.T) { - viper.Reset() - - originServer := &origin_ui.OriginServer{} - - viper.Set("Origin.ExportVolume", t.TempDir()+":/test") - // Disable functionality we're not using (and is difficult to make work on Mac) - viper.Set("Origin.EnableCmsd", false) - viper.Set("Origin.EnableMacaroons", false) - viper.Set("Origin.EnableVoms", false) - viper.Set("TLSSkipVerify", true) - - // Create our own temp directory (for some reason t.TempDir() does not play well with xrootd) - tmpPathPattern := "XRootD-Test_Origin*" - tmpPath, err := os.MkdirTemp("", tmpPathPattern) - require.NoError(t, err) - viper.Set("ConfigDir", tmpPath) - viper.Set("Xrootd.RunLocation", filepath.Join(tmpPath, "xrootd")) - t.Cleanup(func() { - os.RemoveAll(tmpPath) - }) - // Increase the log level; otherwise, its difficult to debug failures - viper.Set("Logging.Level", "Debug") - config.InitConfig() - err = config.InitServer(config.OriginType) - require.NoError(t, err) - - // Ensure the xrootd user can access the directory - userInfo, err := config.GetDaemonUserInfo() - require.NoError(t, err) - err = os.Chown(tmpPath, userInfo.Uid, userInfo.Gid) - require.NoError(t, err) - - require.NoError(t, err) - - err = config.GeneratePrivateKey(param.Server_TLSKey.GetString(), elliptic.P256()) - require.NoError(t, err) - err = config.GenerateCert() - require.NoError(t, err) - - err = CheckXrootdEnv(originServer) - require.NoError(t, err) - - shutdownCtx, shutdownCancel := context.WithCancel(context.Background()) - var wg sync.WaitGroup - - defer func() { - shutdownCancel() - wg.Wait() - }() - - err = SetUpMonitoring(shutdownCtx, &wg) - require.NoError(t, err) - wg.Add(1) - - configPath, err := ConfigXrootd(true) - require.NoError(t, err) - - launchers, err := ConfigureLaunchers(false, configPath, false) - require.NoError(t, err) - - ctx, cancel := context.WithCancel(context.Background()) - go func() { - _ = daemon.LaunchDaemons(ctx, launchers) - }() - defer cancel() - - testExpiry := time.Now().Add(10 * time.Second) - testSuccess := false - for !(testSuccess || time.Now().After(testExpiry)) { - time.Sleep(50 * time.Millisecond) - req, err := http.NewRequest("GET", param.Origin_Url.GetString(), nil) - require.NoError(t, err) - httpClient := http.Client{ - Transport: config.GetTransport(), - Timeout: 50 * time.Millisecond, - } - _, err = httpClient.Do(req) - if err != nil { - log.Infoln("Failed to send request to XRootD; likely, server is not up (will retry in 50ms):", err) - } else { - testSuccess = true - log.Debugln("XRootD server appears to be functioning; will proceed with test") - } - } - - if testSuccess { - fileTests := utils.TestFileTransferImpl{} - ok, err := fileTests.RunTests(param.Origin_Url.GetString(), param.Origin_Url.GetString(), utils.OriginSelfFileTest) - require.NoError(t, err) - require.True(t, ok) - } else { - t.Fatalf("Unsucessful test: timeout when trying to send request to xrootd") - } -} From 38f7b905ee7521e8a9655a882fa2f794dfd76aec Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Mon, 4 Dec 2023 21:51:39 +0000 Subject: [PATCH 07/14] Cleanup after rebase --- .../{origin_linux_test.go => origin_test.go} | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) rename xrootd/{origin_linux_test.go => origin_test.go} (97%) diff --git a/xrootd/origin_linux_test.go b/xrootd/origin_test.go similarity index 97% rename from xrootd/origin_linux_test.go rename to xrootd/origin_test.go index 63347c445..a20d2bfcf 100644 --- a/xrootd/origin_linux_test.go +++ b/xrootd/origin_test.go @@ -25,19 +25,20 @@ import ( "fmt" "net/http" "os" + "os/exec" "path/filepath" "strings" + "sync" "testing" "time" - "github.com/minio/minio-go/v7" - "github.com/minio/minio-go/v7/pkg/credentials" - "os/exec" - "github.com/pelicanplatform/pelican/config" "github.com/pelicanplatform/pelican/daemon" "github.com/pelicanplatform/pelican/origin_ui" "github.com/pelicanplatform/pelican/param" + + "github.com/minio/minio-go/v7" + "github.com/minio/minio-go/v7/pkg/credentials" log "github.com/sirupsen/logrus" "github.com/spf13/viper" "github.com/stretchr/testify/assert" @@ -67,7 +68,16 @@ func originMockup(t *testing.T) context.CancelFunc { err = CheckXrootdEnv(true, nil) require.NoError(t, err) - err = SetUpMonitoring() + shutdownCtx, shutdownCancel := context.WithCancel(context.Background()) + var wg sync.WaitGroup + wg.Add(1) + + defer func() { + shutdownCancel() + wg.Wait() + }() + + err = SetUpMonitoring(shutdownCtx, &wg) require.NoError(t, err) configPath, err := ConfigXrootd(true) From 4c4a4f4825b69848c8e0f488775ca54c36b00253 Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Tue, 5 Dec 2023 17:05:45 +0000 Subject: [PATCH 08/14] More cleanup after rebase --- origin_ui/origin.go | 8 ++------ xrootd/origin_test.go | 41 +++++++++++++++++++++++++---------------- xrootd/xrootd_config.go | 9 --------- 3 files changed, 27 insertions(+), 31 deletions(-) diff --git a/origin_ui/origin.go b/origin_ui/origin.go index aa90cfe17..da051913c 100644 --- a/origin_ui/origin.go +++ b/origin_ui/origin.go @@ -19,21 +19,17 @@ package origin_ui import ( - "embed" "encoding/json" - "mime" "net/http" "net/url" "os" "path/filepath" - "strings" - "github.com/gin-gonic/gin" "github.com/pelicanplatform/pelican/config" "github.com/pelicanplatform/pelican/param" + + "github.com/gin-gonic/gin" "github.com/pkg/errors" - "os" - "path/filepath" ) // Configure XrootD directory for both self-based and director-based file transfer tests diff --git a/xrootd/origin_test.go b/xrootd/origin_test.go index a20d2bfcf..ae751871e 100644 --- a/xrootd/origin_test.go +++ b/xrootd/origin_test.go @@ -41,23 +41,32 @@ import ( "github.com/minio/minio-go/v7/pkg/credentials" log "github.com/sirupsen/logrus" "github.com/spf13/viper" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func originMockup(t *testing.T) context.CancelFunc { + originServer := &origin_ui.OriginServer{} + // Create our own temp directory (for some reason t.TempDir() does not play well with xrootd) - tmpPath := "/tmp/XRootD-Test_Origin" + tmpPathPattern := "XRootD-Test_Origin*" + tmpPath, err := os.MkdirTemp("", tmpPathPattern) + require.NoError(t, err) + + // Need to set permissions or the xrootd process we spawn won't be able to write PID/UID files + permissions := os.FileMode(0777) + err = os.Chmod(tmpPath, permissions) + require.NoError(t, err) + viper.Set("ConfigDir", tmpPath) viper.Set("Xrootd.RunLocation", filepath.Join(tmpPath, "xrootd")) t.Cleanup(func() { os.RemoveAll(tmpPath) }) + // Increase the log level; otherwise, its difficult to debug failures viper.Set("Logging.Level", "Debug") config.InitConfig() - err := config.InitServer() - + err = config.InitServer(config.OriginType) require.NoError(t, err) err = config.GeneratePrivateKey(param.Server_TLSKey.GetString(), elliptic.P256()) @@ -65,7 +74,7 @@ func originMockup(t *testing.T) context.CancelFunc { err = config.GenerateCert() require.NoError(t, err) - err = CheckXrootdEnv(true, nil) + err = CheckXrootdEnv(originServer) require.NoError(t, err) shutdownCtx, shutdownCancel := context.WithCancel(context.Background()) @@ -144,7 +153,7 @@ func TestS3OriginConfig(t *testing.T) { viper.Reset() tmpDir := t.TempDir() - // We need to start up a minio server. Open to better ways to do this! + // We need to start up a minio server, which is how we emulate S3. Open to better ways to do this! minIOServerCmd := exec.Command("minio", "server", "--quiet", tmpDir) minIOServerCmd.Env = []string{fmt.Sprintf("HOME=%s", tmpDir)} @@ -164,10 +173,10 @@ func TestS3OriginConfig(t *testing.T) { t.Fatalf("Could not start the MinIO server: %s", errb.String()) } // Check for other types of errors that might have been passed back through the process - assert.NoError(t, err) + require.NoError(t, err) defer func() { err = minIOServerCmd.Process.Kill() - assert.NoError(t, err) + require.NoError(t, err) }() // MinIO is running (by default at localhost:9000), let's create an unauthenticated bucket @@ -183,7 +192,7 @@ func TestS3OriginConfig(t *testing.T) { Creds: credentials.NewStaticV4(accessKey, secretKey, ""), Secure: useSSL, }) - assert.NoError(t, err) + require.NoError(t, err) // Create a new unauthenticated bucket. Under the hood, this will access the minio server endpoint // and do a PUT @@ -191,23 +200,23 @@ func TestS3OriginConfig(t *testing.T) { regionName := "test-region" serviceName := "test-name" err = minioClient.MakeBucket(context.Background(), bucketName, minio.MakeBucketOptions{}) - assert.NoError(t, err) + require.NoError(t, err) // Set bucket policy for public read access policy := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":"*","Action":["s3:GetObject"],"Resource":["arn:aws:s3:::` + bucketName + `/*"]}]}` err = minioClient.SetBucketPolicy(context.Background(), bucketName, policy) - assert.NoError(t, err) + require.NoError(t, err) // Upload a test file to the bucket testFilePath := filepath.Join(tmpDir, "test-file.txt") content := []byte("This is the content of the test file.") err = os.WriteFile(testFilePath, content, 0644) - assert.NoError(t, err) + require.NoError(t, err) objectName := "test-file.txt" contentType := "application/octet-stream" _, err = minioClient.FPutObject(context.Background(), bucketName, objectName, testFilePath, minio.PutObjectOptions{ContentType: contentType}) - assert.NoError(t, err) + require.NoError(t, err) // All the setup to create the S3 server, add a bucket with a publicly-readable object is done. Now onto Pelican stuff // Set up the origin and try to pull a file @@ -241,13 +250,13 @@ func TestS3OriginConfig(t *testing.T) { Transport: config.GetTransport(), } resp, err := httpClient.Do(req) - assert.NoError(t, err) + require.NoError(t, err) // Until we sort out the things we mentioned above, we should expect a 403 because we don't try to pass tokens // and we don't actually export any keys for token validation. - assert.Equal(t, 403, resp.StatusCode) + require.Equal(t, 403, resp.StatusCode) // One other quick check to do is that the namespace was correctly parsed: - assert.Equal(t, fmt.Sprintf("/%s/%s/%s", serviceName, regionName, bucketName), param.Origin_NamespacePrefix.GetString()) + require.Equal(t, fmt.Sprintf("/%s/%s/%s", serviceName, regionName, bucketName), param.Origin_NamespacePrefix.GetString()) viper.Reset() } diff --git a/xrootd/xrootd_config.go b/xrootd/xrootd_config.go index 06e5ecddd..0f2f3f527 100644 --- a/xrootd/xrootd_config.go +++ b/xrootd/xrootd_config.go @@ -166,7 +166,6 @@ func CheckOriginXrootdEnv(exportPath string, uid int, gid int, groupname string) } } viper.Set("Xrootd.Mount", exportPath) - } else if originMode == "s3" { // Our "namespace prefix" is actually just // /// @@ -175,14 +174,6 @@ func CheckOriginXrootdEnv(exportPath string, uid int, gid int, groupname string) viper.Set("Origin.NamespacePrefix", nsPrefix) } - - - - - - - - if param.Origin_SelfTest.GetBool() { if err := origin_ui.ConfigureXrootdMonitoringDir(); err != nil { return exportPath, err From 75b9e7fe4f53f465fb72eaa07133fe94977fd35c Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Tue, 5 Dec 2023 17:08:48 +0000 Subject: [PATCH 09/14] Re-add linux requirement to origin_test.go --- xrootd/origin_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xrootd/origin_test.go b/xrootd/origin_test.go index ae751871e..e819187e9 100644 --- a/xrootd/origin_test.go +++ b/xrootd/origin_test.go @@ -1,3 +1,5 @@ +//go:build linux + /*************************************************************** * * Copyright (C) 2023, Pelican Project, Morgridge Institute for Research From 6c30ed653317e8929b78d06b741944d2be86037f Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Tue, 5 Dec 2023 17:14:55 +0000 Subject: [PATCH 10/14] Fixup whitespace to attempt new test kickoff --- xrootd/origin_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/xrootd/origin_test.go b/xrootd/origin_test.go index e819187e9..ff8453081 100644 --- a/xrootd/origin_test.go +++ b/xrootd/origin_test.go @@ -102,7 +102,6 @@ func originMockup(t *testing.T) context.CancelFunc { _ = daemon.LaunchDaemons(ctx, launchers) }() return cancel - } func TestOrigin(t *testing.T) { From 2e02cfcdf4aac435d0f591b62a8d346f0f888cbe Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Wed, 6 Dec 2023 16:52:46 +0000 Subject: [PATCH 11/14] Cleanup linter issues --- cmd/origin_serve.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/origin_serve.go b/cmd/origin_serve.go index f794e63e7..fbd8191ab 100644 --- a/cmd/origin_serve.go +++ b/cmd/origin_serve.go @@ -80,7 +80,7 @@ func serveOrigin( /*cmd*/ *cobra.Command /*args*/, []string) error { return err } wg.Add(1) - + // In posix mode, we rely on xrootd to export keys. When we run the origin with // different backends, we instead export the keys via the Pelican process if param.Origin_Mode.GetString() != "posix" { From 4f9cdadcf7d48622befc3f78591f2b613ea7a5c2 Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Wed, 6 Dec 2023 22:23:40 +0000 Subject: [PATCH 12/14] Fix test after rebase --- xrootd/origin_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/xrootd/origin_test.go b/xrootd/origin_test.go index ff8453081..855f8ec50 100644 --- a/xrootd/origin_test.go +++ b/xrootd/origin_test.go @@ -38,6 +38,7 @@ import ( "github.com/pelicanplatform/pelican/daemon" "github.com/pelicanplatform/pelican/origin_ui" "github.com/pelicanplatform/pelican/param" + "github.com/pelicanplatform/pelican/utils" "github.com/minio/minio-go/v7" "github.com/minio/minio-go/v7/pkg/credentials" @@ -138,12 +139,10 @@ func TestOrigin(t *testing.T) { } if testSuccess { - url, err := origin_ui.UploadTestfile() - require.NoError(t, err) - err = origin_ui.DownloadTestfile(url) - require.NoError(t, err) - err = origin_ui.DeleteTestfile(url) + fileTests := utils.TestFileTransferImpl{} + ok, err := fileTests.RunTests(param.Origin_Url.GetString(), param.Origin_Url.GetString(), utils.OriginSelfFileTest) require.NoError(t, err) + require.True(t, ok) } else { t.Fatalf("Unsucessful test: timeout when trying to send request to xrootd") } From edd52cbe8e1d4f51d5e4211b1dd0d78f7db4d42d Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Wed, 6 Dec 2023 22:27:02 +0000 Subject: [PATCH 13/14] I really wish pre-commit linted things for me --- xrootd/xrootd_config.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/xrootd/xrootd_config.go b/xrootd/xrootd_config.go index 0f2f3f527..dcd019427 100644 --- a/xrootd/xrootd_config.go +++ b/xrootd/xrootd_config.go @@ -39,20 +39,20 @@ var ( type ( OriginConfig struct { - Multiuser bool - EnableCmsd bool - EnableMacaroons bool - EnableVoms bool + Multiuser bool + EnableCmsd bool + EnableMacaroons bool + EnableVoms bool EnableDirListing bool - SelfTest bool - NamespacePrefix string - Mode string - S3Bucket string - S3Region string - S3ServiceName string - S3ServiceUrl string - S3AccessKeyfile string - S3SecretKeyfile string + SelfTest bool + NamespacePrefix string + Mode string + S3Bucket string + S3Region string + S3ServiceName string + S3ServiceUrl string + S3AccessKeyfile string + S3SecretKeyfile string } CacheConfig struct { From aa2070a018153ed390db7ebe801029508305e9cc Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Fri, 8 Dec 2023 19:58:06 +0000 Subject: [PATCH 14/14] Incorporate review feedback --- config/config.go | 62 +++++++++++++ docs/parameters.yaml | 24 +++++ server_utils/server_utils.go | 40 +++++++++ utils/{server_utils.go => web_utils.go} | 25 ------ xrootd/authorization.go | 6 +- xrootd/origin_test.go | 112 ++++++++++++++---------- 6 files changed, 194 insertions(+), 75 deletions(-) create mode 100644 server_utils/server_utils.go rename utils/{server_utils.go => web_utils.go} (64%) diff --git a/config/config.go b/config/config.go index 2bd333377..185011eab 100644 --- a/config/config.go +++ b/config/config.go @@ -323,6 +323,60 @@ func setupTransport() { } } +func parseServerIssuerURL(sType ServerType) error { + if param.Server_IssuerUrl.GetString() != "" { + _, err := url.Parse(param.Server_IssuerUrl.GetString()) + if err != nil { + return errors.Wrapf(err, "Failed to parse the Server.IssuerUrl %s loaded from config", param.Server_IssuerUrl.GetString()) + } + return nil + } + + if param.Server_IssuerHostname.GetString() != "" { + if param.Server_IssuerPort.GetInt() != 0 { // Will be the default if not set + // We assume any issuer is running https, otherwise we're crazy + issuerUrl := url.URL{ + Scheme: "https", + Host: fmt.Sprintf("%s:%d", param.Server_IssuerHostname.GetString(), param.Server_IssuerPort.GetInt()), + } + viper.Set("Server.IssuerUrl", issuerUrl.String()) + return nil + } + return errors.New("If Server.IssuerHostname is configured, you must provide a valid port") + } + + if sType == OriginType { + // If Origin.Mode is set to anything that isn't "posix" or "", assume we're running a plugin and + // that the origin's issuer URL actually uses the same port as OriginUI instead of XRootD. This is + // because under that condition, keys are being served by the Pelican process instead of by XRootD + originMode := param.Origin_Mode.GetString() + if originMode == "" || originMode == "posix" { + // In this case, we use the default set up by config.go, which uses the xrootd port + issuerUrl, err := url.Parse(param.Origin_Url.GetString()) + if err != nil { + return errors.Wrap(err, "Failed to parse the issuer URL from the default origin URL") + } + viper.Set("Server.IssuerUrl", issuerUrl.String()) + return nil + } else { + issuerUrl, err := url.Parse(param.Server_ExternalWebUrl.GetString()) + if err != nil { + return errors.Wrap(err, "Failed to parse the issuer URL generated from Server.ExternalWebUrl") + } + viper.Set("Server.IssuerUrl", issuerUrl.String()) + return nil + } + } else { + issuerUrlStr := param.Server_ExternalWebUrl.GetString() + issuerUrl, err := url.Parse(issuerUrlStr) + if err != nil { + return errors.Wrap(err, "Failed to parse the issuer URL generated using the parsed Server.ExternalWebUrl") + } + viper.Set("Server.IssuerUrl", issuerUrl.String()) + return nil + } +} + // function to get/setup the transport (only once) func GetTransport() *http.Transport { onceTransport.Do(func() { @@ -543,6 +597,14 @@ func InitServer(sType ServerType) error { // After we know we have the certs we need, call setupTransport (which uses those certs for its TLSConfig) setupTransport() + + // Set up the server's issuer URL so we can access that data wherever we need to find keys and whatnot + // This populates Server.IssuerUrl, and can be safely fetched using server_utils.GetServerIssuerURL() + err = parseServerIssuerURL(sType) + if err != nil { + return err + } + return DiscoverFederation() } diff --git a/docs/parameters.yaml b/docs/parameters.yaml index 5ee843a55..7a3886f3f 100644 --- a/docs/parameters.yaml +++ b/docs/parameters.yaml @@ -602,6 +602,30 @@ type: string default: none components: ["origin", "director", "nsregistry"] --- +name: Server.IssuerUrl +description: >- + The URL and port at which the server's issuer can be accessed. +type: string +# Setting default to none for now because it changes based on server type and server mode. +default: none +components: ["origin", "director", "nsregistry"] +--- +name: Server.IssuerHostname +description: >- + The hostname at which the server's issuer can be accessed. +type: string +# Setting default to none for now because it changes based on server type and server mode. +default: none +components: ["origin", "director", "nsregistry"] +--- +name: Server.IssuerPort +description: >- + The port at which the server's issuer can be accessed. +type: int +# Setting default to none for now because it changes based on server type and server mode. +default: none +components: ["origin", "director", "nsregistry"] +--- name: Server.IssuerJwks description: >- A filepath indicating where the server's public JSON web keyset can be found. diff --git a/server_utils/server_utils.go b/server_utils/server_utils.go new file mode 100644 index 000000000..2cc3ccfc8 --- /dev/null +++ b/server_utils/server_utils.go @@ -0,0 +1,40 @@ +/*************************************************************** + * + * Copyright (C) 2023, 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. + * + ***************************************************************/ + +package server_utils + +import ( + "net/url" + + "github.com/pelicanplatform/pelican/param" + "github.com/pkg/errors" +) + +// For calling from within the server. Returns the server's issuer URL/port +func GetServerIssuerURL() (*url.URL, error) { + if param.Server_IssuerUrl.GetString() == "" { + return nil, errors.New("The server failed to determine its own issuer url. Something is wrong!") + } + + issuerUrl, err := url.Parse(param.Server_IssuerUrl.GetString()) + if err != nil { + return nil, errors.Wrapf(err, "The server's issuer URL is malformed: %s. Something is wrong!", param.Server_IssuerUrl.GetString()) + } + + return issuerUrl, nil +} diff --git a/utils/server_utils.go b/utils/web_utils.go similarity index 64% rename from utils/server_utils.go rename to utils/web_utils.go index da1cd8635..62ccfd215 100644 --- a/utils/server_utils.go +++ b/utils/web_utils.go @@ -23,10 +23,8 @@ import ( "encoding/json" "io" "net/http" - "net/url" "github.com/pelicanplatform/pelican/config" - "github.com/pelicanplatform/pelican/param" "github.com/pkg/errors" ) @@ -60,26 +58,3 @@ func MakeRequest(url string, method string, data map[string]interface{}, headers return body, nil } - -func GetLocalIssuerURL() (*url.URL, error) { - // If Origin.Mode is set to anything that isn't "posix" or "", assume we're running a plugin and - // that the origin's issuer URL actually uses the same port as OriginUI instead of XRootD. This is - // because under that condition, keys are being served by the Pelican process instead of by XRootD - originMode := param.Origin_Mode.GetString() - if originMode == "" || originMode == "posix" { - // In this case, we use the default set up by config.go, which uses the xrootd port - issuerUrl, err := url.Parse(param.Origin_Url.GetString()) - if err != nil { - return nil, errors.Wrap(err, "Failed to parse the issuer URL from the default origin URL") - } - return issuerUrl, nil - } else { - issuerUrlStr := param.Server_ExternalWebUrl.GetString() - issuerUrl, err := url.Parse(issuerUrlStr) - if err != nil { - return nil, errors.Wrap(err, "Failed to parse the issuer URL generated using ComputeExternalAddress") - } - - return issuerUrl, nil - } -} diff --git a/xrootd/authorization.go b/xrootd/authorization.go index ea9c9edf6..213118e39 100644 --- a/xrootd/authorization.go +++ b/xrootd/authorization.go @@ -40,7 +40,6 @@ import ( "github.com/pelicanplatform/pelican/director" "github.com/pelicanplatform/pelican/param" "github.com/pelicanplatform/pelican/server_utils" - "github.com/pelicanplatform/pelican/utils" "github.com/pkg/errors" ) @@ -278,7 +277,7 @@ func GenerateMonitoringIssuer() (issuer Issuer, err error) { return } issuer.Name = "Built-in Monitoring" - issuerUrl, err := utils.GetLocalIssuerURL() + issuerUrl, err := server_utils.GetServerIssuerURL() if err != nil { return } @@ -295,7 +294,7 @@ func GenerateOriginIssuer(exportedPaths []string) (issuer Issuer, err error) { return } issuer.Name = "Origin" - issuerUrl, err := utils.GetLocalIssuerURL() + issuerUrl, err := server_utils.GetServerIssuerURL() if err != nil { return } @@ -359,7 +358,6 @@ func makeSciTokensCfg() (cfg ScitokensCfg, err error) { // Writes out the origin's scitokens.cfg configuration func WriteOriginScitokensConfig(exportedPaths []string) error { - cfg, err := makeSciTokensCfg() if err != nil { return err diff --git a/xrootd/origin_test.go b/xrootd/origin_test.go index 855f8ec50..a313899f1 100644 --- a/xrootd/origin_test.go +++ b/xrootd/origin_test.go @@ -42,6 +42,7 @@ import ( "github.com/minio/minio-go/v7" "github.com/minio/minio-go/v7/pkg/credentials" + "github.com/pkg/errors" log "github.com/sirupsen/logrus" "github.com/spf13/viper" "github.com/stretchr/testify/require" @@ -105,6 +106,40 @@ func originMockup(t *testing.T) context.CancelFunc { return cancel } +// Provide the method (eg GET) and endpoint to ping while waiting for +// server pieces to spin up. Returns true when +func serverRunning(t *testing.T, method string, endpoint string, successStatus int) (bool, error) { + testExpiry := time.Now().Add(10 * time.Second) + testSuccess := false + logged := false + for !(time.Now().After(testExpiry)) { + time.Sleep(50 * time.Millisecond) + req, err := http.NewRequest(method, endpoint, nil) + if err != nil { + return false, errors.Errorf("Failed to make request to endpoint %s: %v", endpoint, err) + } + httpClient := http.Client{ + Transport: config.GetTransport(), + Timeout: 50 * time.Millisecond, + } + resp, err := httpClient.Do(req) + if err != nil { + if !logged { + log.Infof("Failed to send request to %s; likely, server is not up. Will continue to retry: %v", endpoint, err) + logged = true + } + } else { + if resp.StatusCode == successStatus { + testSuccess = true + break + } + // We didn't get a success status + return false, errors.Errorf("Received bad status code in reply to server ping: %d. Expected %d,", resp.StatusCode, successStatus) + } + } + return testSuccess, nil +} + func TestOrigin(t *testing.T) { viper.Reset() @@ -119,33 +154,18 @@ func TestOrigin(t *testing.T) { cancel := originMockup(t) defer cancel() - testExpiry := time.Now().Add(10 * time.Second) - testSuccess := false - for !(testSuccess || time.Now().After(testExpiry)) { - time.Sleep(50 * time.Millisecond) - req, err := http.NewRequest("GET", param.Origin_Url.GetString(), nil) - require.NoError(t, err) - httpClient := http.Client{ - Transport: config.GetTransport(), - Timeout: 50 * time.Millisecond, - } - _, err = httpClient.Do(req) - if err != nil { - log.Infoln("Failed to send request to XRootD; likely, server is not up (will retry in 50ms):", err) - } else { - testSuccess = true - log.Debugln("XRootD server appears to be functioning; will proceed with test") - } + // In this case a 403 means its running + running, err := serverRunning(t, "GET", param.Origin_Url.GetString(), 403) + if err != nil { + t.Fatalf("Unsucessful test: Server encountered an error: %v", err) + } else if !running { + t.Fatalf("Unsucessful test: timeout while waiting for xrootd") } + fileTests := utils.TestFileTransferImpl{} + ok, err := fileTests.RunTests(param.Origin_Url.GetString(), param.Origin_Url.GetString(), utils.OriginSelfFileTest) + require.NoError(t, err) + require.True(t, ok) - if testSuccess { - fileTests := utils.TestFileTransferImpl{} - ok, err := fileTests.RunTests(param.Origin_Url.GetString(), param.Origin_Url.GetString(), utils.OriginSelfFileTest) - require.NoError(t, err) - require.True(t, ok) - } else { - t.Fatalf("Unsucessful test: timeout when trying to send request to xrootd") - } viper.Reset() } @@ -163,25 +183,30 @@ func TestS3OriginConfig(t *testing.T) { minIOServerCmd.Stderr = &errb err := minIOServerCmd.Start() - // Wait for the server to initialize. Hopefully this always happens in under 2 seconds! - time.Sleep(time.Second * 2) - - // Check for any errors in the outputs + require.NoError(t, err) + // Check for any other errors in the outputs if strings.Contains(strings.ToLower(outb.String()), "error") { t.Fatalf("Could not start the MinIO server: %s", outb.String()) } else if errb.String() != "" { t.Fatalf("Could not start the MinIO server: %s", errb.String()) } - // Check for other types of errors that might have been passed back through the process - require.NoError(t, err) + + // Check if MinIO is running (by default at localhost:9000) + endpoint := "localhost:9000" + // Expect a 403 from this endpoint -- that means it's running + running, err := serverRunning(t, "GET", fmt.Sprintf("http://%s", endpoint), 403) + if err != nil { + t.Fatalf("Unsucessful test: Server encountered an error: %v", err) + } else if !running { + t.Fatalf("Unsucessful test: timeout while waiting for xrootd") + } + defer func() { err = minIOServerCmd.Process.Kill() require.NoError(t, err) }() - // MinIO is running (by default at localhost:9000), let's create an unauthenticated bucket - // First we set up a client instance - endpoint := "localhost:9000" + // Let's create an unauthenticated bucket. First we set up a client instance // By default, the endpoint will require access/secret key with these values. Note that this doesn't // necessarily mean the bucket we create needs those keys, as the bucket will have its own IAM accessKey := "minioadmin" @@ -241,20 +266,15 @@ func TestS3OriginConfig(t *testing.T) { // of the web_ui stuff initialized to export the public signing keys (as we can't export via XRootD) and we // need a real token. These become much easier when we have an internally workable set of commands to do so. - // Wait for xrootd to initialize -- hopefully this always happens in 2 seconds! - time.Sleep(2 * time.Second) - // The file is accessed at ${OriginURL}//// - req, err := http.NewRequest("GET", fmt.Sprintf("%s/%s/%s/%s/%s", param.Origin_Url.GetString(), serviceName, regionName, bucketName, objectName), nil) - require.NoError(t, err) - httpClient := http.Client{ - Transport: config.GetTransport(), - } - resp, err := httpClient.Do(req) - require.NoError(t, err) - + originEndpoint := fmt.Sprintf("%s/%s/%s/%s/%s", param.Origin_Url.GetString(), serviceName, regionName, bucketName, objectName) // Until we sort out the things we mentioned above, we should expect a 403 because we don't try to pass tokens // and we don't actually export any keys for token validation. - require.Equal(t, 403, resp.StatusCode) + running, err = serverRunning(t, "GET", originEndpoint, 403) + if err != nil { + t.Fatalf("Unsucessful test: Server encountered an error: %v", err) + } else if !running { + t.Fatalf("Unsucessful test: timeout while waiting for xrootd") + } // One other quick check to do is that the namespace was correctly parsed: require.Equal(t, fmt.Sprintf("/%s/%s/%s", serviceName, regionName, bucketName), param.Origin_NamespacePrefix.GetString())