diff --git a/config/config.go b/config/config.go index c22e4e1a5..cca9eeb16 100644 --- a/config/config.go +++ b/config/config.go @@ -1443,6 +1443,9 @@ func SetServerDefaults(v *viper.Viper) error { func InitServer(ctx context.Context, currentServers server_structs.ServerType) error { InitConfigInternal(log.InfoLevel) + // Bind any legacy env vars to their new config params -- will generate warnings about deprecated env vars + bindLegacyServerEnv() + setEnabledServer(currentServers) // Output warnings before the defaults are set. The SetServerDefaults function sets the default values diff --git a/config/env.go b/config/env.go index 57a24abf5..af6472b12 100644 --- a/config/env.go +++ b/config/env.go @@ -140,3 +140,24 @@ func bindLegacyClientEnv() { } } } + +// Bind any legacy/deprecated server environment variables to their new config parameters. +// The function should log any warnings about deprecation; this class of env vars is so old +// these config parameters are not included in the params table, meaning +// `config/config.go::handleDeprecatedConfig` will not catch them/log warnings. +func bindLegacyServerEnv() { + prefixes := GetAllPrefixes() + + // MAXMINDKEY + for _, prefix := range prefixes { + if val, isSet := os.LookupEnv(prefix.String() + "_MAXMINDKEY"); isSet { + log.Warningf("You are using a legacy/deprecated parameter '%s_MAXMINDKEY' to indicate the MaxMind license key. "+ + "Support for this environment variable will be removed in a future release. Please use %s instead", prefix.String(), + param.Director_MaxMindKeyFile.GetName()) + + // Use SetDefault so that if both the env var and the config param are set, the config param takes precedence + viper.SetDefault(param.Director_MaxMindKeyFile.GetName(), val) + break + } + } +} diff --git a/config/env_test.go b/config/env_test.go index d37816184..887c29d11 100644 --- a/config/env_test.go +++ b/config/env_test.go @@ -28,6 +28,8 @@ import ( "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/pelicanplatform/pelican/param" ) func TestOsdfEnvToPelican(t *testing.T) { @@ -154,3 +156,31 @@ func TestOsdfEnvToPelican(t *testing.T) { assert.Contains(t, hook.LastEntry().Message, "Environment variables with OSDF prefix will be deprecated in the next feature release. Please use PELICAN prefix instead.") }) } + +func TestBindLegacyServerEnv(t *testing.T) { + ResetConfig() + t.Cleanup(func() { + ResetConfig() + }) + + t.Setenv("PELICAN_MAXMINDKEY", "/path/to/keyfile") + + // Two things to test here: + // 1. That the legacy env var PELICAN_MAXMINDKEY correctly binds to Director.MaxMindKeyFile + // 2. Director.MaxMindKeyFile takes precedence over the legacy env var when both are set + + // Test 1 + t.Run("binds-legacy-env-var", func(t *testing.T) { + assert.Empty(t, viper.GetString(param.Director_MaxMindKeyFile.GetName())) + bindLegacyServerEnv() + assert.Equal(t, "/path/to/keyfile", viper.GetString(param.Director_MaxMindKeyFile.GetName())) + }) + + // Test 2 + t.Run("does-not-overwrite-existing-config", func(t *testing.T) { + viper.Set(param.Director_MaxMindKeyFile.GetName(), "/existing/config/keyfile") + bindLegacyServerEnv() + assert.Equal(t, "/path/to/keyfile", os.Getenv("PELICAN_MAXMINDKEY")) // env var is still set + assert.Equal(t, "/existing/config/keyfile", viper.GetString(param.Director_MaxMindKeyFile.GetName())) + }) +} diff --git a/director/maxmind.go b/director/maxmind.go index 8f1e0c6af..c3ae44181 100644 --- a/director/maxmind.go +++ b/director/maxmind.go @@ -36,7 +36,6 @@ import ( "github.com/oschwald/geoip2-golang/v2" "github.com/pkg/errors" log "github.com/sirupsen/logrus" - "github.com/spf13/viper" "github.com/pelicanplatform/pelican/param" "github.com/pelicanplatform/pelican/server_structs" @@ -76,19 +75,17 @@ func downloadDB(localFile string) error { var licenseKey string keyFile := param.Director_MaxMindKeyFile.GetString() - keyFromEnv := viper.GetString("MAXMINDKEY") - if keyFile != "" { - contents, err := os.ReadFile(keyFile) - if err != nil { - return err - } - licenseKey = strings.TrimSpace(string(contents)) - } else if keyFromEnv != "" { - licenseKey = keyFromEnv - } else { - return errors.New("A MaxMind key file must be specified in the config (Director.MaxMindKeyFile), in the environment (PELICAN_DIRECTOR_MAXMINDKEYFILE), or the key must be provided via the environment variable PELICAN_MAXMINDKEY)") + if keyFile == "" { + return errors.Errorf("A MaxMind API key file must be specified in the config (%s)", param.Director_MaxMindKeyFile.GetName()) } + contents, err := os.ReadFile(keyFile) + if err != nil { + return errors.Wrapf(err, "unable to read MaxMind license key file %q", keyFile) + } + + licenseKey = strings.TrimSpace(string(contents)) + url := fmt.Sprintf(maxMindURL, licenseKey) localDir := filepath.Dir(localFile) fileHandle, err := os.CreateTemp(localDir, filepath.Base(localFile)+".tmp") @@ -169,7 +166,7 @@ func InitializeGeoIPDB(ctx context.Context) { localFile := param.Director_GeoIPLocation.GetString() localReader, err := geoip2.Open(localFile) if err != nil { - log.Warningln("Local GeoIP database file not present; will attempt a download.", err) + log.Infoln("Local GeoIP database file not present; will attempt a download.", err) err = downloadDB(localFile) if err != nil { log.Errorln("Failed to download GeoIP database! Will not be available:", err)