From 8784f7b5765f581eff7751f293b70364d3d2045e Mon Sep 17 00:00:00 2001 From: Haoming Meng Date: Mon, 5 Feb 2024 19:20:39 +0000 Subject: [PATCH] Address code review feedback --- config/config.go | 3 ++- config/resources/defaults.yaml | 2 +- docs/parameters.yaml | 41 +++++++++++++++++----------------- metrics/shoveler.go | 16 ++++++------- param/parameters.go | 12 +++++----- param/parameters_struct.go | 24 ++++++++++---------- 6 files changed, 50 insertions(+), 48 deletions(-) diff --git a/config/config.go b/config/config.go index bfc77c681..4cc8c1df0 100644 --- a/config/config.go +++ b/config/config.go @@ -660,7 +660,6 @@ func InitServer(ctx context.Context, currentServers ServerType) error { viper.SetDefault("OIDC.ClientSecretFile", filepath.Join(configDir, "oidc-client-secret")) viper.SetDefault("Cache.ExportLocation", "/") viper.SetDefault("Registry.RequireKeyChaining", true) - viper.SetDefault("Shoveler.TokenLocation", "/etc/pelican/shoveler-token") if IsRootExecution() { viper.SetDefault("Xrootd.RunLocation", filepath.Join("/run", "pelican", "xrootd", xrootdPrefix)) viper.SetDefault("Cache.DataLocation", "/run/pelican/xcache") @@ -669,11 +668,13 @@ func InitServer(ctx context.Context, currentServers ServerType) error { viper.SetDefault("Registry.DbLocation", "/var/lib/pelican/registry.sqlite") viper.SetDefault("Monitoring.DataLocation", "/var/lib/pelican/monitoring/data") viper.SetDefault("Shoveler.QueueDirectory", "/var/spool/pelican/shoveler/queue") + viper.SetDefault("Shoveler.AMQPTokenLocation", "/etc/pelican/shoveler-token") } else { viper.SetDefault("Director.GeoIPLocation", filepath.Join(configDir, "maxmind", "GeoLite2-City.mmdb")) viper.SetDefault("Registry.DbLocation", filepath.Join(configDir, "ns-registry.sqlite")) viper.SetDefault("Monitoring.DataLocation", filepath.Join(configDir, "monitoring/data")) viper.SetDefault("Shoveler.QueueDirectory", filepath.Join(configDir, "shoveler/queue")) + viper.SetDefault("Shoveler.AMQPTokenLocation", filepath.Join(configDir, "shoveler-token")) if userRuntimeDir := os.Getenv("XDG_RUNTIME_DIR"); userRuntimeDir != "" { runtimeDir := filepath.Join(userRuntimeDir, "pelican", xrootdPrefix) diff --git a/config/resources/defaults.yaml b/config/resources/defaults.yaml index d3387cfd5..a366b45fc 100644 --- a/config/resources/defaults.yaml +++ b/config/resources/defaults.yaml @@ -63,7 +63,7 @@ Shoveler: MessageQueueProtocol: amqp PortLower: 9930 PortHigher: 9999 - Exchange: shoveled-xrd + AMQPExchange: shoveled-xrd Xrootd: Port: 8443 Mount: "" diff --git a/docs/parameters.yaml b/docs/parameters.yaml index 64b63a5b6..ce6a399f3 100644 --- a/docs/parameters.yaml +++ b/docs/parameters.yaml @@ -1342,17 +1342,17 @@ description: >- For amqp, the following configurations are required: - URL: amqps://username:password@example.com/vhost - - Exchange: shoveled-xrd - Topic: mytopic - - TokenLocation: /etc/pelican/xrootd-monitoring-shoveler-token + - AMQPExchange: shoveled-xrd + - AMQPTokenLocation: /etc/pelican/xrootd-monitoring-shoveler-token For stomp, the following configurations are required: - - User: username - - Password: password - URL: messagebroker.org:port - Topic: mytopic - - Cert: path/to/cert/file - - CertKey: path/to/certkey/file + - StompUsername: username + - StompPassword: password + - StompCert: path/to/cert/file + - StompCertKey: path/to/certkey/file type: string default: amqp components: ["origin", "cache"] @@ -1366,34 +1366,35 @@ type: url default: none components: ["origin", "cache"] --- -name: Shoveler.Exchange +name: Shoveler.Topic description: >- - For amqp only. + For amqp and stomp. - The exchange to shovel messages + The topic of the messages. For stomp, it defaults to xrootd.shoveler type: string -default: "shoveled-xrd" +default: none components: ["origin", "cache"] --- -name: Shoveler.Topic +name: Shoveler.AMQPExchange description: >- - For amqp and stomp. + For amqp only. - The topic of the messages. For stomp, it defaults to xrootd.shoveler + The exchange to shovel messages type: string -default: none +default: "shoveled-xrd" components: ["origin", "cache"] --- -name: Shoveler.TokenLocation +name: Shoveler.AMQPTokenLocation description: >- For amqp only. A filepath to the location of the JWT used for authenticating amqp connection type: filename -default: /etc/pelican/shoveler-token +default: $ConfigBase/shoveler-token +root_default: /etc/pelican/shoveler-token components: ["origin", "cache"] --- -name: Shoveler.Username +name: Shoveler.StompUsername description: >- For stomp only. @@ -1402,7 +1403,7 @@ type: string default: none components: ["origin", "cache"] --- -name: Shoveler.Password +name: Shoveler.StompPassword description: >- For stomp only. @@ -1411,7 +1412,7 @@ type: string default: none components: ["origin", "cache"] --- -name: Shoveler.Cert +name: Shoveler.StompCert description: >- For stomp only. @@ -1420,7 +1421,7 @@ type: filename default: none components: ["origin", "cache"] --- -name: Shoveler.CertKey +name: Shoveler.StompCertKey description: >- For stomp only. diff --git a/metrics/shoveler.go b/metrics/shoveler.go index 0b6dd7447..2ff208681 100644 --- a/metrics/shoveler.go +++ b/metrics/shoveler.go @@ -63,28 +63,28 @@ func configShoveler(c *shoveler.Config) error { log.Debugln("AMQP URL:", c.AmqpURL.String()) // Get the AMQP Exchange - c.AmqpExchange = param.Shoveler_Exchange.GetString() + c.AmqpExchange = param.Shoveler_AMQPExchange.GetString() log.Debugln("AMQP Exchange:", c.AmqpExchange) - c.AmqpToken = param.Shoveler_TokenLocation.GetString() + c.AmqpToken = param.Shoveler_AMQPTokenLocation.GetString() log.Debugln("AMQP Token location:", c.AmqpToken) _, err := os.Stat(c.AmqpToken) if err != nil { - return fmt.Errorf("Token in Shoveler.TokenLocation does not exists: %s", err.Error()) + return fmt.Errorf("Token in Shoveler.AMQPTokenLocation does not exists: %s", err.Error()) } tokenContents, err := os.ReadFile(c.AmqpToken) if err != nil { return fmt.Errorf("Unable to read file: %s", c.AmqpToken) } if strings.TrimSpace(string(tokenContents)) == "" { - return fmt.Errorf("Token content is empty. Reading from Shoveler.TokenLocation at %s", c.AmqpToken) + return fmt.Errorf("Token content is empty. Reading from Shoveler.AMQPTokenLocation at %s", c.AmqpToken) } } else { // Stomp viper.SetDefault("Shoveler.Topic", "xrootd.shoveler") - c.StompUser = param.Shoveler_Username.GetString() + c.StompUser = param.Shoveler_StompUsername.GetString() log.Debugln("STOMP User:", c.StompUser) - c.StompPassword = param.Shoveler_Password.GetString() + c.StompPassword = param.Shoveler_StompPassword.GetString() // Get the STOMP URL c.StompURL, err = url.Parse(param.Shoveler_URL.GetString()) @@ -97,11 +97,11 @@ func configShoveler(c *shoveler.Config) error { log.Debugln("STOMP Topic:", c.StompTopic) // Get the STOMP cert - c.StompCert = param.Shoveler_Cert.GetString() + c.StompCert = param.Shoveler_StompCert.GetString() log.Debugln("STOMP CERT:", c.StompCert) // Get the STOMP certkey - c.StompCertKey = param.Shoveler_CertKey.GetString() + c.StompCertKey = param.Shoveler_StompCertKey.GetString() log.Debugln("STOMP CERTKEY:", c.StompCertKey) } diff --git a/param/parameters.go b/param/parameters.go index f199d94f0..c9b26b371 100644 --- a/param/parameters.go +++ b/param/parameters.go @@ -129,16 +129,16 @@ var ( Server_UIActivationCodeFile = StringParam{"Server.UIActivationCodeFile"} Server_UIPasswordFile = StringParam{"Server.UIPasswordFile"} Server_WebHost = StringParam{"Server.WebHost"} - Shoveler_Cert = StringParam{"Shoveler.Cert"} - Shoveler_CertKey = StringParam{"Shoveler.CertKey"} - Shoveler_Exchange = StringParam{"Shoveler.Exchange"} + Shoveler_AMQPExchange = StringParam{"Shoveler.AMQPExchange"} + Shoveler_AMQPTokenLocation = StringParam{"Shoveler.AMQPTokenLocation"} Shoveler_MessageQueueProtocol = StringParam{"Shoveler.MessageQueueProtocol"} - Shoveler_Password = StringParam{"Shoveler.Password"} Shoveler_QueueDirectory = StringParam{"Shoveler.QueueDirectory"} - Shoveler_TokenLocation = StringParam{"Shoveler.TokenLocation"} + Shoveler_StompCert = StringParam{"Shoveler.StompCert"} + Shoveler_StompCertKey = StringParam{"Shoveler.StompCertKey"} + Shoveler_StompPassword = StringParam{"Shoveler.StompPassword"} + Shoveler_StompUsername = StringParam{"Shoveler.StompUsername"} Shoveler_Topic = StringParam{"Shoveler.Topic"} Shoveler_URL = StringParam{"Shoveler.URL"} - Shoveler_Username = StringParam{"Shoveler.Username"} StagePlugin_MountPrefix = StringParam{"StagePlugin.MountPrefix"} StagePlugin_OriginPrefix = StringParam{"StagePlugin.OriginPrefix"} StagePlugin_ShadowOriginPrefix = StringParam{"StagePlugin.ShadowOriginPrefix"} diff --git a/param/parameters_struct.go b/param/parameters_struct.go index c63d78302..a9bbdee8a 100644 --- a/param/parameters_struct.go +++ b/param/parameters_struct.go @@ -161,21 +161,21 @@ type config struct { WebPort int } Shoveler struct { - Cert string - CertKey string + AMQPExchange string + AMQPTokenLocation string Enable bool - Exchange string IPMapping interface{} MessageQueueProtocol string OutputDestinations []string - Password string PortHigher int PortLower int QueueDirectory string - TokenLocation string + StompCert string + StompCertKey string + StompPassword string + StompUsername string Topic string URL string - Username string VerifyHeader bool } StagePlugin struct { @@ -366,21 +366,21 @@ type configWithType struct { WebPort struct { Type string; Value int } } Shoveler struct { - Cert struct { Type string; Value string } - CertKey struct { Type string; Value string } + AMQPExchange struct { Type string; Value string } + AMQPTokenLocation struct { Type string; Value string } Enable struct { Type string; Value bool } - Exchange struct { Type string; Value string } IPMapping struct { Type string; Value interface{} } MessageQueueProtocol struct { Type string; Value string } OutputDestinations struct { Type string; Value []string } - Password struct { Type string; Value string } PortHigher struct { Type string; Value int } PortLower struct { Type string; Value int } QueueDirectory struct { Type string; Value string } - TokenLocation struct { Type string; Value string } + StompCert struct { Type string; Value string } + StompCertKey struct { Type string; Value string } + StompPassword struct { Type string; Value string } + StompUsername struct { Type string; Value string } Topic struct { Type string; Value string } URL struct { Type string; Value string } - Username struct { Type string; Value string } VerifyHeader struct { Type string; Value bool } } StagePlugin struct {