Incorporate feedback from review

cmd/origin_serve.go

@@ -64,8 +64,6 @@ type (
 		Port                   int
 		ManagerHost            string
 		ManagerPort            string
-		TLSCertFile            string
-		TLSCertDir             string
 		MacaroonsKeyFile       string
 		RobotsTxtFile          string
 		Sitename               string
@@ -81,8 +79,10 @@ type (
 	}
 
 	ServerConfig struct {
-		TLSCertificate string
-		TLSKey         string
+		TLSCertificate          string
+		TLSKey                  string
+		TLSCertificateDirectory string
+		TLSCertificateFile      string
 	}
 
 	XrootdConfig struct {
@@ -379,11 +379,11 @@ func checkDefaults() error {
 	if originUrlParsed.Port() == "" {
 		// No port was specified, let's tack on whatever was passed in the
 		// command line argument
-		viper.Set("OriginUrl", originUrlParsed.String()+":"+fmt.Sprint(param.Server_WebPort.GetInt()))
-	} else if originUrlParsed.Port() != fmt.Sprint(param.Server_WebPort.GetInt()) {
-		// The web port configured via the config file and the webport configured
+		viper.Set("OriginUrl", originUrlParsed.String()+":"+fmt.Sprint(param.Server_Port.GetInt()))
+	} else if originUrlParsed.Port() != fmt.Sprint(param.Server_Port.GetInt()) {
+		// The web port configured via the config file and the port configured
 		// via commandline don't match. Perhaps the user is confused?
-		return errors.New("Mismatched webports: from command line: " + fmt.Sprint(param.Server_WebPort.GetInt()) +
+		return errors.New("Mismatched ports: from command line: " + fmt.Sprint(param.Server_Port.GetInt()) +
 			", from config file: " + originUrlParsed.Port() + ". Please ensure these match")
 	}
 

cmd/resources/xrootd.cfg

@@ -21,10 +21,10 @@ if exec xrootd
   xrd.protocol http:{{.Xrootd.Port}} libXrdHttp.so
 fi
 xrd.tls {{.Server.TLSCertificate}} {{.Server.TLSKey}}
-{{if .Xrootd.TLSCertDir}}
-xrd.tlsca certdir {{.Xrootd.TLSCertDir}}
+{{if .Server.TLSCertificateDirectory}}
+xrd.tlsca certdir {{.Server.TLSCertificateDirectory}}
 {{else}}
-xrd.tlsca certfile {{.Xrootd.TLSCertFile}}
+xrd.tlsca certfile {{.Server.TLSCertificateFile}}
 {{end}}
 http.exthandler xrdmacaroons libXrdMacaroons.so
 macaroons.secretkey {{.Xrootd.MacaroonsKeyFile}}

cmd/root.go

@@ -46,7 +46,7 @@ with data federations, enabling the sharing of objects and collections
 across multiple dataset providers.`,
 	}
 
-	// We want the value of this port flag to correspond to the WebPort viper key.
+	// We want the value of this port flag to correspond to the Port viper key.
 	// However, only one flag pointer can correspond to the key.  If we define this
 	// in `pelican registry serve` and `pelican director serve`, then whatever init()
 	// function is run second will be the only one that is set (the first definition
@@ -128,7 +128,7 @@ func initConfig() {
 	if err := viper.BindPFlag("Debug", rootCmd.PersistentFlags().Lookup("debug")); err != nil {
 		panic(err)
 	}
-	if err := viper.BindPFlag("Server.WebPort", portFlag); err != nil {
+	if err := viper.BindPFlag("Server.Port", portFlag); err != nil {
 		panic(err)
 	}
 

config/config.go

@@ -230,7 +230,7 @@ func ComputeExternalAddress() string {
 	if config_url != "" {
 		return config_url
 	}
-	return fmt.Sprintf("%v:%v", param.Server_Hostname.GetString(), param.Server_WebPort.GetInt())
+	return fmt.Sprintf("%v:%v", param.Server_Hostname.GetString(), param.Server_Port.GetInt())
 }
 
 func getConfigBase() (string, error) {
@@ -302,18 +302,18 @@ func InitServer() error {
 	viper.SetDefault("Xrootd.MacaroonsKeyFile", filepath.Join(configDir, "macaroons-secret"))
 	viper.SetDefault("IssuerKey", filepath.Join(configDir, "issuer.jwk"))
 	viper.SetDefault("Origin.UIPasswordFile", filepath.Join(configDir, "origin-ui-passwd"))
-	viper.SetDefault("Registry.OIDCClientIDFile", filepath.Join(configDir, "oidc-client-id"))
-	viper.SetDefault("Registry.OIDCClientSecretFile", filepath.Join(configDir, "oidc-client-secret"))
+	viper.SetDefault("OIDC.ClientIDFile", filepath.Join(configDir, "oidc-client-id"))
+	viper.SetDefault("OIDC.ClientSecretFile", filepath.Join(configDir, "oidc-client-secret"))
 	if IsRootExecution() {
 		viper.SetDefault("Xrootd.RunLocation", "/run/pelican/xrootd")
 		viper.SetDefault("Origin.Multiuser", true)
 		viper.SetDefault("Director.GeoIPLocation", "/var/cache/pelican/maxmind/GeoLite2-City.mmdb")
 		viper.SetDefault("Registry.DbLocation", "/var/lib/pelican/registry.sqlite")
-		viper.SetDefault("Prometheus.MonitoringData", "/var/lib/pelican/monitoring/data")
+		viper.SetDefault("Monitoring.DataLocation", "/var/lib/pelican/monitoring/data")
 	} else {
 		viper.SetDefault("Director.GeoIPLocation", filepath.Join(configDir, "maxmind", "GeoLite2-City.mmdb"))
 		viper.SetDefault("Registry.DbLocation", filepath.Join(configDir, "ns-registry.sqlite"))
-		viper.SetDefault("Prometheus.MonitoringData", filepath.Join(configDir, "monitoring/data"))
+		viper.SetDefault("Monitoring.DataLocation", filepath.Join(configDir, "monitoring/data"))
 
 		if userRuntimeDir := os.Getenv("XDG_RUNTIME_DIR"); userRuntimeDir != "" {
 			runtimeDir := filepath.Join(userRuntimeDir, "pelican")
@@ -332,9 +332,9 @@ func InitServer() error {
 		}
 		viper.SetDefault("Origin.Multiuser", false)
 	}
-	viper.SetDefault("Xrootd.TLSCertFile", "/etc/pki/tls/cert.pem")
+	viper.SetDefault("Xrootd.TLSCertificateFile", "/etc/pki/tls/cert.pem")
 
-	err := os.MkdirAll(viper.GetString("Prometheus.MonitoringData"), 0750)
+	err := os.MkdirAll(viper.GetString("Monitoring.DataLocation"), 0750)
 	if err != nil {
 		return errors.Wrapf(err, "Failure when creating a directory for the monitoring data")
 	}
@@ -464,7 +464,19 @@ func InitClient() error {
 		}
 		break
 	}
-	viper.Set("Client.MinimumDownloadSpeed", downloadLimit)
+	if viper.IsSet("MinimumDownloadSpeed") {
+		viper.SetDefault("Client.MinimumDownloadSpeed", viper.GetString("MinimumDownloadSpeed"))
+	} else {
+		viper.Set("Client.MinimumDownloadSpeed", downloadLimit)
+	}
+
+	// Handle more legacy config options
+	if viper.IsSet("DisableProxyFallback") {
+		viper.SetDefault("Client.DisableProxyFallback", viper.GetString("DisableProxyFallback"))
+	}
+	if viper.IsSet("DisableHttpProxy") {
+		viper.SetDefault("Client.DisableHttpProxy", viper.GetString("DisableHttpProxy"))
+	}
 
 	setupTransport()
 

config/resources/defaults.yaml

@@ -15,17 +15,18 @@
 #
 
 Debug: false
-WebAddress: "0.0.0.0"
+
 Server:
-  WebPort: 8444
+  Port: 8444
+  Address: "0.0.0.0"
 Director:
   DefaultResponse: cache
 Origin:
   NamespacePrefix: ""
   Multiuser: false
-Prometheus:
-  MonitoringPortLower: 9930
-  MonitoringPortHigher: 9999
+Monitoring:
+  PortLower: 9930
+  ortHigher: 9999
 Xrootd:
   Port: 8443
   Mount: ""