Addressed PR comments, reworked export handling in NewFedTest

Author: Emma Turetsky

client/sharing_url_test.go

@@ -35,6 +35,7 @@ import (
 
 	"github.com/pelicanplatform/pelican/client"
 	"github.com/pelicanplatform/pelican/config"
+	"github.com/pelicanplatform/pelican/param"
 	"github.com/pelicanplatform/pelican/test_utils"
 )
 
@@ -98,9 +99,9 @@ func TestSharingUrl(t *testing.T) {
 	defer os.Unsetenv("PELICAN_SKIP_TERMINAL_CHECK")
 
 	test_utils.InitClient(t, map[string]any{
-		"Logging.Level":           "debug",
-		"TLSSkipVerify":           true,
-		"Federation.DiscoveryUrl": server.URL,
+		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

cmd/plugin_test.go

@@ -561,16 +561,10 @@ func TestPluginDirectRead(t *testing.T) {
 	defer server_utils.ResetTestState()
 
 	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)
@@ -851,14 +845,9 @@ func TestPluginRecursiveDownload(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", "/<THIS WILL BE OVERRIDDEN>")
-	viper.Set("Origin.EnablePublicReads", 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())
 
 	// Drop the testFileContent into the origin directory

cmd/resources/public-test-origin.yml

@@ -1,4 +1,4 @@
-# Origin export configuration to test full multi-export capabilities
+# Origin export configuration for testing
 
 Origin:
   # Things that configure the origin itself
@@ -8,5 +8,4 @@ Origin:
   Exports:
     - StoragePrefix: /<SHOULD BE OVERRIDDEN>
       FederationPrefix: /test
-      # Don't set Reads -- it should be toggled true by setting PublicReads
       Capabilities: ["PublicReads", "Writes", "DirectReads", "Listings"]

config/config.go

@@ -1251,6 +1251,7 @@ func SetServerDefaults(v *viper.Viper) error {
 // is thrown
 func InitServer(ctx context.Context, currentServers server_structs.ServerType) error {
 	InitConfigInternal()
+	logging.FlushLogs(true)
 	setEnabledServer(currentServers)
 
 	// Output warnings before the defaults are set. The SetServerDefaults function sets the default values
@@ -1814,6 +1815,7 @@ func ResetConfig() {
 	fedDiscoveryOnce = &sync.Once{}
 	globalFedInfo = pelican_url.FederationDiscovery{}
 	globalFedErr = nil
+
 	setServerOnce = sync.Once{}
 
 	ResetIssuerPrivateKeys()

director/discovery_test.go

@@ -1,6 +1,6 @@
 /***************************************************************
  *
- * Copyright (C) 2024, Pelican Project, Morgridge Institute for Research
+ * Copyright (C) 2025, 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
@@ -29,7 +29,6 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"github.com/pelicanplatform/pelican/config"
 	"github.com/pelicanplatform/pelican/pelican_url"
 	"github.com/pelicanplatform/pelican/server_structs"
 	"github.com/pelicanplatform/pelican/server_utils"
@@ -215,7 +214,6 @@ func TestOidcDiscoveryHandler(t *testing.T) {
 			test_utils.InitClient(t, map[string]any{
 				"Federation.DirectorUrl": tc.dirUrl,
 			})
-			require.NoError(t, config.InitClient())
 
 			w := httptest.NewRecorder()
 			req, _ := http.NewRequest("GET", "/test"+oidcDiscoveryPath, nil)

director/fed_token_test.go

@@ -175,8 +175,6 @@ func parseJWT(tokenString string) (scopes []string, issuer string, err error) {
 }
 
 func TestCreateFedTok(t *testing.T) {
-	server_utils.ResetTestState()
-	defer server_utils.ResetTestState()
 
 	testCases := []struct {
 		name            string
@@ -219,6 +217,9 @@ func TestCreateFedTok(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
+			server_utils.ResetTestState()
+			defer server_utils.ResetTestState()
+
 			w := httptest.NewRecorder()
 			c, _ := gin.CreateTestContext(w)
 

fed_test_utils/fed.go

@@ -38,6 +38,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/sync/errgroup"
+	"gopkg.in/yaml.v3"
 
 	"github.com/pelicanplatform/pelican/config"
 	"github.com/pelicanplatform/pelican/director"
@@ -157,60 +158,86 @@ func NewFedTest(t *testing.T, originConfig string) (ft *FedTest) {
 	require.NoError(t, err)
 
 	// Read in any config we may have set
+	var originConf interface{}
 	if originConfig != "" {
 		viper.SetConfigType("yaml")
 		err = viper.MergeConfig(strings.NewReader(originConfig))
 		require.NoError(t, err, "error reading config")
+
+		err := yaml.Unmarshal([]byte(originConfig), &originConf)
+		require.NoError(t, err, "error unmarshalling into interface")
+
+		confMap := originConf.(map[string]interface{})
+
+		originRaw, exists := confMap["Origin"]
+
+		if exists {
+			originMap := originRaw.(map[string]interface{})
+
+			// Override the test directory from the config file with our temp directory
+			if exportsRaw, ok := originMap["Exports"]; ok {
+				for i, item := range exportsRaw.([]interface{}) {
+					originDir, err := os.MkdirTemp("", fmt.Sprintf("Export%d", i))
+					assert.NoError(t, err)
+					t.Cleanup(func() {
+						err := os.RemoveAll(originDir)
+						require.NoError(t, err)
+					})
+
+					exportMap := item.(map[string]interface{})
+					exportMap["StoragePrefix"] = originDir
+
+					// Change the permissions of the temporary origin directory
+					permissions = os.FileMode(0755)
+					err = os.Chmod(originDir, permissions)
+					require.NoError(t, err)
+
+					// Change ownership on the temporary origin directory so files can be uploaded
+					uinfo, err := config.GetDaemonUserInfo()
+					require.NoError(t, err)
+					require.NoError(t, os.Chown(originDir, uinfo.Uid, uinfo.Gid))
+
+					// Start off with a Hello World file we can use for testing in each of our exports
+					err = os.WriteFile(filepath.Join(originDir, "hello_world.txt"), []byte("Hello, World!"), os.FileMode(0644))
+					require.NoError(t, err)
+				}
+			} else {
+				originDir, err := os.MkdirTemp("", fmt.Sprintf("Export%s", "test"))
+				assert.NoError(t, err)
+				t.Cleanup(func() {
+					err := os.RemoveAll(originDir)
+					require.NoError(t, err)
+				})
+
+				originMap["StoragePrefix"] = originDir
+
+				permissions = os.FileMode(0755)
+				err = os.Chmod(originDir, permissions)
+				require.NoError(t, err)
+
+				// Change ownership on the temporary origin directory so files can be uploaded
+				uinfo, err := config.GetDaemonUserInfo()
+				require.NoError(t, err)
+				require.NoError(t, os.Chown(originDir, uinfo.Uid, uinfo.Gid))
+
+				// Start off with a Hello World file we can use for testing in the StoragePrefix
+				err = os.WriteFile(filepath.Join(originDir, "hello_world.txt"), []byte("Hello, World!"), os.FileMode(0644))
+				require.NoError(t, err)
+			}
+		}
 	}
-	// Now call GetOriginExports and check the struct
-	exports, err := server_utils.GetOriginExports()
-	require.NoError(t, err, "error getting origin exports")
-	ft.Exports = exports
-
-	// Override the test directory from the config file with our temp directory
-	for i := 0; i < len(ft.Exports); i++ {
-		originDir, err := os.MkdirTemp("", fmt.Sprintf("Export%d", i))
-		assert.NoError(t, err)
-		t.Cleanup(func() {
-			err := os.RemoveAll(originDir)
-			require.NoError(t, err)
-		})
-
-		// Set the storage prefix to the temporary origin directory
-		ft.Exports[i].StoragePrefix = originDir
-		// Our exports object becomes global -- we must reset in between each fed test
-		t.Cleanup(func() {
-			server_utils.ResetOriginExports()
-		})
-
-		// Change the permissions of the temporary origin directory
-		permissions = os.FileMode(0755)
-		err = os.Chmod(originDir, permissions)
-		require.NoError(t, err)
 
-		// Change ownership on the temporary origin directory so files can be uploaded
-		uinfo, err := config.GetDaemonUserInfo()
-		require.NoError(t, err)
-		require.NoError(t, os.Chown(originDir, uinfo.Uid, uinfo.Gid))
+	confDir := t.TempDir()
+	outputPath := filepath.Join(confDir, "tempfile_*.yaml")
 
-		// Start off with a Hello World file we can use for testing in each of our exports
-		err = os.WriteFile(filepath.Join(originDir, "hello_world.txt"), []byte("Hello, World!"), os.FileMode(0644))
-		require.NoError(t, err)
-	}
-	originConf := strings.NewReader(originConfig)
+	outputData, err := yaml.Marshal(&originConf)
+	require.NoError(t, err, "error marshalling struct into yaml format")
 
-	tmpFile, err := os.CreateTemp("", "tempfile_*.yaml")
-	if err != nil {
-		t.Fatalf("Error creating temporary config file for fed test: %v", err)
-	}
-	defer tmpFile.Close()
+	err = os.WriteFile(outputPath, outputData, 0644)
+	require.NoError(t, err, "error writing out temporary config file for fed test")
 
-	_, err = io.Copy(tmpFile, originConf)
-	if err != nil {
-		t.Fatalf("Error writing to temporary config file for fed test: %v", err)
-	}
+	viper.Set("config", outputPath)
 
-	viper.Set("config", tmpFile.Name())
 	servers, _, err := launchers.LaunchModules(ctx, modules)
 	require.NoError(t, err)
 
@@ -275,6 +302,9 @@ func NewFedTest(t *testing.T, originConfig string) (ft *FedTest) {
 
 	ft.Token = token
 
+	ft.Exports, err = server_utils.GetOriginExports()
+	require.NoError(t, err)
+
 	// Explicitly run tmpPath cleanup AFTER cancel and egrp are done -- otherwise we end up
 	// with a race condition where removing tmpPath might happen while the server is still
 	// using it, resulting in "error: unlinkat <tmpPath>: directory not empty"

xrootd/authorization_test.go

@@ -375,9 +375,8 @@ func TestEmitCfg(t *testing.T) {
 
 	defer server_utils.ResetTestState()
 
-	test_utils.InitClient(t, map[string]any{
-		"Origin.RunLocation": dirname,
-	})
+	test_utils.InitClient(t, map[string]any{})
+	viper.Set(param.Origin_RunLocation.GetName(), dirname)
 
 	configTester := func(cfg *ScitokensCfg, configResult string) func(t *testing.T) {
 		return func(t *testing.T) {
@@ -455,9 +454,9 @@ func TestLoadScitokensConfig(t *testing.T) {
 
 	defer server_utils.ResetTestState()
 
-	test_utils.InitClient(t, map[string]any{
-		"Origin.RunLocation": dirname,
-	})
+	test_utils.InitClient(t, map[string]any{})
+
+	viper.Set(param.Origin_RunLocation.GetName(), dirname)
 
 	configTester := func(configResult string) func(t *testing.T) {
 		return func(t *testing.T) {
@@ -490,16 +489,16 @@ func TestMergeConfig(t *testing.T) {
 
 	defer server_utils.ResetTestState()
 
-	viper.Set("Origin.RunLocation", dirname)
-	viper.Set("Origin.Port", 8443)
-	viper.Set("Origin.StoragePrefix", "/")
-	viper.Set("Origin.FederationPrefix", "/")
+	viper.Set(param.Origin_RunLocation.GetName(), dirname)
+	viper.Set(param.Origin_Port.GetName(), 8443)
+	viper.Set(param.Origin_StoragePrefix.GetName(), "/")
+	viper.Set(param.Origin_FederationPrefix.GetName(), "/")
 	// We don't inherit any defaults at this level of code -- in order to recognize
 	// that this is an authorized prefix, we must set either EnableReads && !EnablePublicReads
 	// or EnableWrites
-	viper.Set("Origin.EnableReads", true)
+	viper.Set(param.Origin_EnableReads.GetName(), true)
 	scitokensConfigFile := filepath.Join(dirname, "scitokens-input.cfg")
-	viper.Set("Xrootd.ScitokensConfig", scitokensConfigFile)
+	viper.Set(param.Xrootd_ScitokensConfig.GetName(), scitokensConfigFile)
 
 	configTester := func(configInput string, postProcess func(*testing.T, ScitokensCfg)) func(t *testing.T) {
 		return func(t *testing.T) {

xrootd/xrootd_config_test.go

@@ -48,28 +48,17 @@ import (
 
 func setupXrootd(t *testing.T, ctx context.Context, server server_structs.ServerType) {
 	tmpDir := t.TempDir()
+	server_utils.ResetTestState()
 
 	viper.Set("ConfigDir", tmpDir)
 	viper.Set("Xrootd.RunLocation", tmpDir)
 	viper.Set("Cache.RunLocation", tmpDir)
 	viper.Set("Origin.RunLocation", tmpDir)
-
-	server_utils.ResetTestState()
-
-	dirname, err := os.MkdirTemp("", "tmpDir")
-	require.NoError(t, err)
-	t.Cleanup(func() {
-		os.RemoveAll(dirname)
-	})
-	viper.Set("ConfigDir", dirname)
-	viper.Set("Xrootd.RunLocation", dirname)
-	viper.Set("Cache.RunLocation", dirname)
-	viper.Set("Origin.RunLocation", dirname)
 	viper.Set("Origin.StoragePrefix", "/")
 	viper.Set("Origin.FederationPrefix", "/")
 	viper.Set("Server.IssuerUrl", "https://my-xrootd.com:8444")
 
-	err = config.InitServer(ctx, server)
+	err := config.InitServer(ctx, server)
 	require.NoError(t, err)
 
 }