Repository navigation
Clean up GEOIPUPDATE_PROXY_USER_PASSWORD_FILE support #571
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a7f03cf
5ea9930
b23157e
b16dcc6
0df07b6
952fca1
2d15b7c
4304fd4
9d412c0
2da284c
a76f87b
a1946d8
56cc185
cb7ba22
28698ef
99ac625
e44e59c
ca6c272
403f561
0306d30
b06fe04
1b71dd2
5cbe6ef
d80e86d
7fd6194
0bcb6c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -287,39 +287,14 @@ func setConfigFromFile(config *Config, path string) error { | |
| return nil | ||
| } | ||
|
|
||
| // getValueFromEnvFile returns the value associated to an env variable or from a file containing it. | ||
| func getValueFromEnvFile(key string) (string, bool, error) { | ||
| var res string | ||
| var found bool | ||
|
|
||
| if value, ok := os.LookupEnv(key); ok { | ||
| res = value | ||
| found = true | ||
| } | ||
|
|
||
| if value := os.Getenv(key + "_FILE"); value != "" { | ||
| var err error | ||
|
|
||
| content, err := os.ReadFile(filepath.Clean(value)) | ||
| if err != nil { | ||
| return "", true, fmt.Errorf("failed to open %s_FILE: %w", key, err) | ||
| } | ||
|
|
||
| found = true | ||
| res = strings.TrimSpace(string(content)) | ||
| } | ||
|
|
||
| return res, found, nil | ||
| } | ||
|
|
||
| // setConfigFromEnv sets Config fields based on environment variables. | ||
| func setConfigFromEnv(config *Config) error { | ||
| if value, found, err := getValueFromEnvFile("GEOIPUPDATE_ACCOUNT_ID"); found { | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| config.AccountID, err = strconv.Atoi(value) | ||
| accountID, found, err := lookupFileOrEnv("GEOIPUPDATE_ACCOUNT_ID") | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if found { | ||
| config.AccountID, err = strconv.Atoi(accountID) | ||
| if err != nil { | ||
| return errors.New("invalid account ID format") | ||
| } | ||
|
|
@@ -344,12 +319,12 @@ func setConfigFromEnv(config *Config) error { | |
| config.URL = u.String() | ||
| } | ||
|
|
||
| if value, found, err := getValueFromEnvFile("GEOIPUPDATE_LICENSE_KEY"); found { | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| config.LicenseKey = value | ||
| licenseKey, found, err := lookupFileOrEnv("GEOIPUPDATE_LICENSE_KEY") | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if found { | ||
| config.LicenseKey = licenseKey | ||
| } | ||
|
|
||
| if value, ok := os.LookupEnv("GEOIPUPDATE_LOCK_FILE"); ok { | ||
|
|
@@ -378,12 +353,12 @@ func setConfigFromEnv(config *Config) error { | |
| config.proxyURL = value | ||
| } | ||
|
|
||
| if value, found, err := getValueFromEnvFile("GEOIPUPDATE_PROXY_USER_PASSWORD"); found { | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| config.proxyUserInfo = value | ||
| proxyUserInfo, found, err := lookupFileOrEnv("GEOIPUPDATE_PROXY_USER_PASSWORD") | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if found { | ||
| config.proxyUserInfo = proxyUserInfo | ||
| } | ||
|
|
||
| if value, ok := os.LookupEnv("GEOIPUPDATE_RETRY_FOR"); ok { | ||
|
|
@@ -404,6 +379,33 @@ func setConfigFromEnv(config *Config) error { | |
| return nil | ||
| } | ||
|
|
||
| // lookupFileOrEnv returns the value of the environment variable named by key | ||
| // and whether it is set, as os.LookupEnv does. If the variable named by | ||
| // key+"_FILE" is not empty, lookupFileOrEnv instead reads the file at that | ||
| // path. It then returns the file contents, with leading and trailing white | ||
| // space removed, and true. If nothing remains, it returns an error. The file | ||
| // takes precedence over the variable named by key. | ||
| func lookupFileOrEnv(key string) (string, bool, error) { | ||
| if path := os.Getenv(key + "_FILE"); path != "" { | ||
| content, err := os.ReadFile(filepath.Clean(path)) | ||
| if err != nil { | ||
| return "", false, fmt.Errorf("failed to read %s_FILE: %w", key, err) | ||
| } | ||
| value := strings.TrimSpace(string(content)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The docs do not say that
Claude Code (an AI agent) wrote this comment for @oschwald.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| if value == "" { | ||
| return "", false, fmt.Errorf( | ||
| "%s_FILE is set to %q, but that file is empty or contains only white space", | ||
| key, | ||
| path, | ||
| ) | ||
| } | ||
| return value, true, nil | ||
| } | ||
|
|
||
| value, found := os.LookupEnv(key) | ||
| return value, found, nil | ||
| } | ||
|
|
||
| // setConfigFromFlags sets Config fields based on option flags. | ||
| func setConfigFromFlags(config *Config, flagOptions ...Option) error { | ||
| for _, option := range flagOptions { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compared with 8.0.0, this release also changes
GEOIPUPDATE_ACCOUNT_ID_FILEandGEOIPUPDATE_LICENSE_KEY_FILEin 3 ways that users can see:LicenseKeyoption is required".GEOIPUPDATE_ACCOUNT_IDno longer causes an error whenGEOIPUPDATE_ACCOUNT_ID_FILEis set.Should the changelog mention these? Users who match on the error text, or who relied on the old error, get no notice.
Claude Code (an AI agent) wrote this comment for @oschwald.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We decided not to add these. They are too minor for the changelog.
Claude Code (an AI agent) wrote this reply for @horgh.