Init config refactor

broker/broker_test.go

@@ -79,7 +79,6 @@ func Setup(t *testing.T, ctx context.Context, egrp *errgroup.Group) {
 	server_utils.ResetTestState()
 	viper.Set("Logging.Level", "Debug")
 	viper.Set("ConfigDir", filepath.Join(dirpath, "config"))
-	config.InitConfig()
 	viper.Set("Server.WebPort", "0")
 	viper.Set("Registry.DbLocation", filepath.Join(dirpath, "ns-registry.sqlite"))
 	viper.Set("Origin.FederationPrefix", "/foo")

broker/token_utils_test.go

@@ -27,15 +27,13 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"github.com/pelicanplatform/pelican/config"
 	"github.com/pelicanplatform/pelican/server_utils"
+	"github.com/pelicanplatform/pelican/test_utils"
 )
 
 func TestGetCacheHostnameFromToken(t *testing.T) {
 	server_utils.ResetTestState()
-	viper.Set("ConfigDir", t.TempDir())
-	config.InitConfig()
-	require.NoError(t, config.InitClient())
+	test_utils.InitClient(t, nil)
 
 	viper.Set("Federation.RegistryUrl", "https://your-registry.com")
 

client/director_test.go

@@ -36,6 +36,7 @@ import (
 	"github.com/pelicanplatform/pelican/pelican_url"
 	"github.com/pelicanplatform/pelican/server_structs"
 	"github.com/pelicanplatform/pelican/server_utils"
+	"github.com/pelicanplatform/pelican/test_utils"
 	"github.com/pelicanplatform/pelican/utils"
 )
 
@@ -104,7 +105,11 @@ func TestParseDirectorInfo(t *testing.T) {
 func TestQueryDirector(t *testing.T) {
 	server_utils.ResetTestState()
 	defer server_utils.ResetTestState()
-	viper.Set(param.Client_DirectorRetries.GetName(), 3)
+	// This test assumes the debugging level is at debug or higher
+	test_utils.InitClient(t, map[string]any{
+		param.Client_DirectorRetries.GetName(): 3,
+		param.Debug.GetName():                  true,
+	})
 
 	type testCase struct {
 		name             string

client/fed_long_test.go

@@ -23,7 +23,6 @@ package client_test
 import (
 	_ "embed"
 	"fmt"
-	"io/fs"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -442,21 +441,22 @@ func TestRecursiveUploadsAndDownloadsWithQuery(t *testing.T) {
 func TestFailureOnOriginDisablingListings(t *testing.T) {
 	server_utils.ResetTestState()
 
-	viper.Set("Logging.Level", "debug")
-	viper.Set("Origin.StorageType", "posix")
-	viper.Set("Origin.ExportVolumes", "/test")
-	viper.Set("Origin.EnablePublicReads", true)
-	viper.Set("Origin.EnableListings", false)
-	fed := fed_test_utils.NewFedTest(t, "")
-
-	destDir := filepath.Join(fed.Exports[0].StoragePrefix, "test")
-	require.NoError(t, os.MkdirAll(destDir, os.FileMode(0755)))
-	log.Debugln("Will create origin file at", destDir)
-	err := os.WriteFile(filepath.Join(destDir, "test.txt"), []byte("test file content"), fs.FileMode(0644))
-	require.NoError(t, err)
-	downloadURL := fmt.Sprintf("pelican://%s:%s%s/%s", param.Server_Hostname.GetString(), strconv.Itoa(param.Server_WebPort.GetInt()),
-		fed.Exports[0].FederationPrefix, "test")
+	fed := fed_test_utils.NewFedTest(t, pubExportNoDirectRead)
+
+	testFileContent := "test file content"
+	// Drop the testFileContent into the origin directory
+	tempFile, err := os.Create(filepath.Join(fed.Exports[0].StoragePrefix, "test.txt"))
+	require.NoError(t, err, "Error creating temp file")
+	defer os.Remove(tempFile.Name())
+	_, err = tempFile.WriteString(testFileContent)
+	require.NoError(t, err, "Error writing to temp file")
+	tempFile.Close()
+
+	// Set path for object to upload/download
+	downloadURL := fmt.Sprintf("pelican://%s:%s%s/", param.Server_Hostname.GetString(), strconv.Itoa(param.Server_WebPort.GetInt()),
+		fed.Exports[0].FederationPrefix)
 
+	// Download the directory with get, should fail due to no collections URL
 	_, err = client.DoGet(fed.Ctx, downloadURL, t.TempDir(), true)
 	require.Error(t, err)
 	require.Contains(t, err.Error(), "no collections URL found in director response")

client/fed_test.go

@@ -900,16 +900,14 @@ func TestObjectList405Error(t *testing.T) {
 // end-to-end
 func TestClientUnpack(t *testing.T) {
 	server_utils.ResetTestState()
-
-	err := config.InitClient()
-	require.NoError(t, err)
+	test_utils.InitClient(t, nil)
 
 	fed := fed_test_utils.NewFedTest(t, bothPublicOriginCfg)
 	export := fed.Exports[0]
 
 	tmpDir := t.TempDir()
 	fooLocation := filepath.Join(tmpDir, "foo.txt")
-	err = os.WriteFile(fooLocation, []byte("hello world"), os.FileMode(0600))
+	err := os.WriteFile(fooLocation, []byte("hello world"), os.FileMode(0600))
 	require.NoError(t, err)
 	fi, err := os.Stat(fooLocation)
 	require.NoError(t, err)

client/sharing_url_test.go

@@ -30,12 +30,13 @@ import (
 	"testing"
 
 	log "github.com/sirupsen/logrus"
-	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/pelicanplatform/pelican/client"
 	"github.com/pelicanplatform/pelican/config"
+	"github.com/pelicanplatform/pelican/param"
+	"github.com/pelicanplatform/pelican/test_utils"
 )
 
 func TestSharingUrl(t *testing.T) {
@@ -93,17 +94,15 @@ func TestSharingUrl(t *testing.T) {
 
 	_, err := config.SetPreferredPrefix(config.PelicanPrefix)
 	assert.NoError(t, err)
-	viper.Set("ConfigDir", t.TempDir())
-	viper.Set("Logging.Level", "debug")
-	viper.Set("TLSSkipVerify", true)
-	config.InitConfig()
 
 	os.Setenv("PELICAN_SKIP_TERMINAL_CHECK", "password")
 	defer os.Unsetenv("PELICAN_SKIP_TERMINAL_CHECK")
-	viper.Set("Federation.DiscoveryUrl", server.URL)
-	viper.Set("ConfigDir", t.TempDir())
-	err = config.InitClient()
-	assert.NoError(t, err)
+
+	test_utils.InitClient(t, map[string]any{
+		param.Logging_Level.GetName():           "debug",
+		param.TLSSkipVerify.GetName():           true,
+		param.Federation_DiscoveryUrl.GetName(): server.URL,
+	})
 
 	// Call QueryDirector with the test server URL and a source path
 	testObj, err := url.Parse("/test/foo/bar")

cmd/config_printer/config_printer_test.go

@@ -51,7 +51,8 @@ func setupMockConfig(t *testing.T) error {
 
 	// Set default config
 	viper.Set("ConfigDir", t.TempDir())
-	config.InitConfig()
+
+	config.InitConfigInternal()
 	if err := config.SetServerDefaults(viper.GetViper()); err != nil {
 		return err
 	}

cmd/fed_serve_test.go

@@ -80,7 +80,6 @@ func TestFedServePosixOrigin(t *testing.T) {
 
 	// Increase the log level; otherwise, its difficult to debug failures
 	viper.Set("Logging.Level", "Debug")
-	config.InitConfig()
 
 	viper.Set("Origin.StoragePrefix", t.TempDir())
 	viper.Set("Origin.FederationPrefix", "/test")

cmd/origin_reset_password_test.go

@@ -45,7 +45,6 @@ func TestResetPassword(t *testing.T) {
 	dirName := t.TempDir()
 	server_utils.ResetTestState()
 	viper.Set("ConfigDir", dirName)
-	config.InitConfig()
 	viper.Set("Server.WebPort", 8444)
 	viper.Set("Origin.Port", 8443)
 	err := config.InitServer(ctx, server_structs.OriginType)

cmd/plugin.go

@@ -114,7 +114,6 @@ func stashPluginMain(args []string) {
 	}()
 
 	var isConfigErr = false
-	config.InitConfig()
 	configErr := config.InitClient()
 	if configErr != nil {
 		log.Errorf("Problem initializing the Pelican client config: %v", configErr)

cmd/plugin_test.go

@@ -66,6 +66,9 @@ import (
 var (
 	//go:embed resources/test-https-origin.yml
 	httpsOriginConfig string
+
+	//go:embed resources/public-test-origin.yml
+	publicTestOrigin string
 )
 
 // TestReadMultiTransfer test if we can read multiple transfers from stdin
@@ -163,7 +166,6 @@ func (f *FedTest) Spinup() {
 
 	viper.Set("ConfigDir", tmpPath)
 
-	config.InitConfig()
 	// Create a file to capture output from commands
 	output, err := os.CreateTemp(f.T.TempDir(), "output")
 	assert.NoError(f.T, err)
@@ -501,11 +503,7 @@ func TestPluginMulti(t *testing.T) {
 
 	dirName := t.TempDir()
 
-	viper.Set("Logging.Level", "debug")
-	viper.Set("Origin.StorageType", "posix")
-	viper.Set("Origin.ExportVolumes", "/test")
-	viper.Set("Origin.EnablePublicReads", true)
-	fed := fed_test_utils.NewFedTest(t, "")
+	fed := fed_test_utils.NewFedTest(t, publicTestOrigin)
 	host := param.Server_Hostname.GetString() + ":" + strconv.Itoa(param.Server_WebPort.GetInt())
 
 	// Drop the testFileContent into the origin directory
@@ -594,15 +592,7 @@ func TestPluginDirectRead(t *testing.T) {
 
 	dirName := t.TempDir()
 
-	viper.Set("Logging.Level", "debug")
-	viper.Set("Origin.StorageType", "posix")
-	viper.Set("Origin.FederationPrefix", "/test")
-	viper.Set("Origin.StoragePrefix", "/<SOMETHING THAT WILL BE OVERRIDDEN>")
-	viper.Set("Origin.EnablePublicReads", true)
-	viper.Set("Origin.EnableDirectReads", true)
-	// We are purposely creating a test with a config of a single-space.
-	// The empty string indicates using the default config, which we don't want.
-	fed := fed_test_utils.NewFedTest(t, " ")
+	fed := fed_test_utils.NewFedTest(t, publicTestOrigin)
 	host := param.Server_Hostname.GetString() + ":" + strconv.Itoa(param.Server_WebPort.GetInt())
 
 	log.Debugln("Will create origin file at", fed.Exports[0].StoragePrefix)
@@ -883,14 +873,7 @@ func TestPluginRecursiveDownload(t *testing.T) {
 
 	dirName := t.TempDir()
 
-	viper.Set("Origin.StorageType", "posix")
-	viper.Set("Origin.FederationPrefix", "/test")
-	viper.Set("Origin.StoragePrefix", "/<THIS WILL BE OVERRIDDEN>")
-	viper.Set("Origin.EnablePublicReads", true)
-	viper.Set(param.Director_AssumePresenceAtSingleOrigin.GetName(), false)
-	// We are purposely creating a test with a config of a single-space.
-	// The empty string indicates using the default config, which we don't want.
-	fed := fed_test_utils.NewFedTest(t, " ")
+	fed := fed_test_utils.NewFedTest(t, publicTestOrigin)
 	host := param.Server_Hostname.GetString() + ":" + strconv.Itoa(param.Server_WebPort.GetInt())
 
 	// Drop the testFileContent into the origin directory
@@ -1285,7 +1268,6 @@ func TestTransferError404(t *testing.T) {
 
 	// Isolate the test so it doesn't use system config
 	viper.Set("ConfigDir", t.TempDir())
-	config.InitConfig()
 	err := config.InitClient()
 	require.NoError(t, err)
 
@@ -1383,7 +1365,6 @@ func TestTransferErrorSlowTransfer(t *testing.T) {
 
 	// Isolate the test so it doesn't use system config
 	viper.Set("ConfigDir", t.TempDir())
-	config.InitConfig()
 	err := config.InitClient()
 	require.NoError(t, err)
 
@@ -1499,7 +1480,6 @@ func TestTransferErrorHeaderTimeout(t *testing.T) {
 
 	// Isolate the test so it doesn't use system config
 	viper.Set("ConfigDir", t.TempDir())
-	config.InitConfig()
 	err := config.InitClient()
 	require.NoError(t, err)
 

cmd/resources/public-test-origin.yml

@@ -0,0 +1,11 @@
+# Origin export configuration for testing
+
+Origin:
+  # Things that configure the origin itself
+  StorageType: "posix"
+  EnableDirectReads: true
+  # The actual namespaces we export
+  Exports:
+    - StoragePrefix: /<SHOULD BE OVERRIDDEN>
+      FederationPrefix: /test
+      Capabilities: ["PublicReads", "Writes", "DirectReads", "Listings"]

cmd/root.go

@@ -132,7 +132,6 @@ func restartProgram() error {
 }
 
 func init() {
-	cobra.OnInitialize(config.InitConfig)
 	rootCmd.AddCommand(objectCmd)
 	objectCmd.CompletionOptions.DisableDefaultCmd = true
 	rootCmd.AddCommand(directorCmd)

config/config.go

@@ -29,6 +29,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"runtime"
 	"sort"
 	"strconv"
 	"strings"
@@ -502,8 +503,12 @@ func CleanupTempResources() (err error) {
 func getConfigBase() string {
 	home, err := os.UserHomeDir()
 	if err != nil {
-		// We currently don't handle this case in Windows (and it may not even occur)
-		// This will be revisited in the future
+		os := runtime.GOOS
+		if os == "windows" {
+			windowsPath := filepath.Join("C:", "ProgramData", "pelican")
+			log.Warningln("No home directory found for user -- will check for configuration yaml in ", windowsPath)
+			return windowsPath
+		}
 		log.Warningln("No home directory found for user -- will check for configuration yaml in /etc/pelican/")
 		return filepath.Join("/etc", "pelican")
 	}
@@ -671,6 +676,10 @@ func handleContinuedCfg() error {
 // Read config file from web UI changes, and call viper.Set() to explicitly override the value
 // so that env wouldn't take precedence
 func setWebConfigOverride(v *viper.Viper, configPath string) error {
+	if configPath == "" {
+		return nil
+	}
+
 	webConfigFile, err := os.OpenFile(configPath, os.O_RDONLY|os.O_CREATE, 0644)
 	if err != nil {
 		return err
@@ -746,7 +755,12 @@ func InitConfigDir(v *viper.Viper) {
 	configDir := v.GetString("ConfigDir")
 	if configDir == "" {
 		if IsRootExecution() {
-			configDir = "/etc/pelican" // We currently don't handle this case in windows, will be revisited in the future
+			os := runtime.GOOS
+			if os == "windows" {
+				configDir = filepath.Join("C:", "ProgramData", "pelican")
+			} else {
+				configDir = filepath.Join("/etc", "pelican")
+			}
 		} else {
 			configDir = getConfigBase()
 		}
@@ -755,9 +769,9 @@ func InitConfigDir(v *viper.Viper) {
 	v.SetConfigName("pelican")
 }
 
-// InitConfig sets up the global Viper instance by loading defaults and
+// InitConfigInternal sets up the global Viper instance by loading defaults and
 // user-defined config files, validates config params, and initializes logging.
-func InitConfig() {
+func InitConfigInternal() {
 	// Set a prefix so Viper knows how to parse PELICAN_* env vars
 	// This must happen before config dir initialization so that Pelican
 	// can pick up setting the config dir with PELICAN_CONFIGDIR
@@ -1251,6 +1265,8 @@ func SetServerDefaults(v *viper.Viper) error {
 // Note not all configurations are supported: currently, if you enable both cache and origin then an error
 // is thrown
 func InitServer(ctx context.Context, currentServers server_structs.ServerType) error {
+	InitConfigInternal()
+
 	setEnabledServer(currentServers)
 
 	// Output warnings before the defaults are set. The SetServerDefaults function sets the default values
@@ -1864,6 +1880,7 @@ func SetClientDefaults(v *viper.Viper) error {
 }
 
 func InitClient() error {
+	InitConfigInternal()
 	logging.FlushLogs(true)
 	if err := SetClientDefaults(viper.GetViper()); err != nil {
 		return err
@@ -1931,6 +1948,8 @@ func ResetConfig() {
 	globalFedInfo = pelican_url.FederationDiscovery{}
 	globalFedErr = nil
 
+	setServerOnce = sync.Once{}
+
 	ResetIssuerPrivateKeys()
 
 	ResetClientInitialized()