From 67b1360eb53b42e953c9aebd0b462db2445bd843 Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Tue, 20 Jun 2023 10:42:56 -0500 Subject: [PATCH 01/25] Refactor config code to better accommodate multiple config sources The current config code mixes constructing values, setting defaults, validation, etc. which makes it difficult to reason about how a configuration is constructed. This refactors the config package to more clearly separate the various steps of config construction, as well as providing a way for e.g. a Proxy url from one config to work with the ProxyUserPassword from an overriding config (which itself does not provide a Proxy url). It also makes it easier to add additional configuration fields and/or normalize those fields since those steps now come after applying any overrides. --- pkg/geoipupdate/config.go | 135 ++++++++++++++------- pkg/geoipupdate/config_test.go | 210 +++++++++++++++++++++++++++++++++ 2 files changed, 300 insertions(+), 45 deletions(-) diff --git a/pkg/geoipupdate/config.go b/pkg/geoipupdate/config.go index 13074d13..38d1c8a5 100644 --- a/pkg/geoipupdate/config.go +++ b/pkg/geoipupdate/config.go @@ -39,6 +39,10 @@ type Config struct { Parallelism int // Proxy is host name or IP address of a proxy server. Proxy *url.URL + // proxyURL is the host value of Proxy + proxyURL string + // proxyUserainfo is the userinfo value of Proxy + proxyUserInfo string // RetryFor is the retry timeout for HTTP requests. It defaults // to 5 minutes. RetryFor time.Duration @@ -101,17 +105,10 @@ func WithOutput(val bool) Option { // NewConfig parses the configuration file. // flagOptions is provided to provide optional flag overrides to the config // file. -func NewConfig( //nolint: gocyclo // long but breaking it up may be worse +func NewConfig( path string, flagOptions ...Option, ) (*Config, error) { - fh, err := os.Open(filepath.Clean(path)) - if err != nil { - return nil, fmt.Errorf("error opening file: %w", err) - } - - defer fh.Close() - // config defaults config := &Config{ URL: "https://updates.maxmind.com", @@ -120,10 +117,61 @@ func NewConfig( //nolint: gocyclo // long but breaking it up may be worse Parallelism: 1, } + // Override config with values from the config file. + err := setConfigFromFile(config, path) + if err != nil { + return nil, err + } + + // Override config with values from option flags. + err = setConfigFromFlags(config, flagOptions...) + if err != nil { + return nil, err + } + + // Set config values that depend on other config values. For instance + // proxyURL may have been set by the default config, and proxyUserInfo + // by config file. Both of these values need to be combined to create + // the public Proxy field that is a *url.URL. + + config.Proxy, err = parseProxy(config.proxyURL, config.proxyUserInfo) + if err != nil { + return nil, err + } + + if config.LockFile == "" { + config.LockFile = filepath.Join(config.DatabaseDirectory, ".geoipupdate.lock") + } + + // Validate config values now that all config sources have been considered and + // any value that may need to be created from other values has been set. + + err = validateConfig(config) + if err != nil { + return nil, err + } + + // Reset values that were only needed to communicate information between + // config overrides. + + config.proxyURL = "" + config.proxyUserInfo = "" + + return config, nil +} + +// setConfigFromFile sets Config fields based on the configuration file. +func setConfigFromFile(config *Config, path string) error { + fh, err := os.Open(filepath.Clean(path)) + if err != nil { + return fmt.Errorf("error opening file: %w", err) + } + + defer fh.Close() + scanner := bufio.NewScanner(fh) lineNumber := 0 keysSeen := map[string]struct{}{} - var proxy, proxyUserPassword string for scanner.Scan() { lineNumber++ line := strings.TrimSpace(scanner.Text()) @@ -133,13 +181,13 @@ func NewConfig( //nolint: gocyclo // long but breaking it up may be worse fields := strings.Fields(line) if len(fields) < 2 { - return nil, fmt.Errorf("invalid format on line %d", lineNumber) + return fmt.Errorf("invalid format on line %d", lineNumber) } key := fields[0] value := strings.Join(fields[1:], " ") if _, ok := keysSeen[key]; ok { - return nil, fmt.Errorf("`%s' is in the config multiple times", key) + return fmt.Errorf("`%s' is in the config multiple times", key) } keysSeen[key] = struct{}{} @@ -147,7 +195,7 @@ func NewConfig( //nolint: gocyclo // long but breaking it up may be worse case "AccountID", "UserId": accountID, err := strconv.Atoi(value) if err != nil { - return nil, fmt.Errorf("invalid account ID format: %w", err) + return fmt.Errorf("invalid account ID format: %w", err) } config.AccountID = accountID keysSeen["AccountID"] = struct{}{} @@ -166,79 +214,76 @@ func NewConfig( //nolint: gocyclo // long but breaking it up may be worse config.LockFile = filepath.Clean(value) case "PreserveFileTimes": if value != "0" && value != "1" { - return nil, errors.New("`PreserveFileTimes' must be 0 or 1") + return errors.New("`PreserveFileTimes' must be 0 or 1") } if value == "1" { config.PreserveFileTimes = true } case "Proxy": - proxy = value + config.proxyURL = value case "ProxyUserPassword": - proxyUserPassword = value + config.proxyUserInfo = value case "Protocol", "SkipHostnameVerification", "SkipPeerVerification": // Deprecated. case "RetryFor": dur, err := time.ParseDuration(value) if err != nil || dur < 0 { - return nil, fmt.Errorf("'%s' is not a valid duration", value) + return fmt.Errorf("'%s' is not a valid duration", value) } config.RetryFor = dur case "Parallelism": parallelism, err := strconv.Atoi(value) if err != nil { - return nil, fmt.Errorf("'%s' is not a valid parallelism value: %w", value, err) + return fmt.Errorf("'%s' is not a valid parallelism value: %w", value, err) } if parallelism <= 0 { - return nil, fmt.Errorf("parallelism should be greater than 0, got '%d'", parallelism) + return fmt.Errorf("parallelism should be greater than 0, got '%d'", parallelism) } config.Parallelism = parallelism default: - return nil, fmt.Errorf("unknown option on line %d", lineNumber) + return fmt.Errorf("unknown option on line %d", lineNumber) } } if err := scanner.Err(); err != nil { - return nil, fmt.Errorf("error reading file: %w", err) + return fmt.Errorf("error reading file: %w", err) } - // Mandatory values. - if _, ok := keysSeen["EditionIDs"]; !ok { - return nil, fmt.Errorf("the `EditionIDs` option is required") - } - - if _, ok := keysSeen["AccountID"]; !ok { - return nil, fmt.Errorf("the `AccountID` option is required") - } - - if _, ok := keysSeen["LicenseKey"]; !ok { - return nil, fmt.Errorf("the `LicenseKey` option is required") - } + return nil +} - // Overrides. +// setConfigFromFlags sets Config fields based on option flags. +func setConfigFromFlags(config *Config, flagOptions ...Option) error { for _, option := range flagOptions { if err := option(config); err != nil { - return nil, fmt.Errorf("error applying flag to config: %w", err) + return fmt.Errorf("error applying flag to config: %w", err) } } + return nil +} - if config.LockFile == "" { - config.LockFile = filepath.Join(config.DatabaseDirectory, ".geoipupdate.lock") - } - - config.Proxy, err = parseProxy(proxy, proxyUserPassword) - if err != nil { - return nil, err - } - +func validateConfig(config *Config) error { // We used to recommend using 999999 / 000000000000 for free downloads // and many people still use this combination. With a real account id // and license key now being required, we want to give those people a // sensible error message. if (config.AccountID == 0 || config.AccountID == 999999) && config.LicenseKey == "000000000000" { - return nil, errors.New("geoipupdate requires a valid AccountID and LicenseKey combination") + return errors.New("geoipupdate requires a valid AccountID and LicenseKey combination") } - return config, nil + if len(config.EditionIDs) == 0 { + return fmt.Errorf("the `EditionIDs` option is required") + } + + if config.AccountID == 0 { + return fmt.Errorf("the `AccountID` option is required") + } + + if config.LicenseKey == "" { + return fmt.Errorf("the `LicenseKey` option is required") + } + + return nil } var schemeRE = regexp.MustCompile(`(?i)\A([a-z][a-z0-9+\-.]*)://`) diff --git a/pkg/geoipupdate/config_test.go b/pkg/geoipupdate/config_test.go index 49d09ca5..37e14989 100644 --- a/pkg/geoipupdate/config_test.go +++ b/pkg/geoipupdate/config_test.go @@ -195,6 +195,8 @@ Parallelism 3 User: url.UserPassword("username", "password"), Host: "127.0.0.1:8888", }, + proxyURL: "", + proxyUserInfo: "", PreserveFileTimes: true, URL: "https://updates.example.com", RetryFor: 10 * time.Minute, @@ -449,6 +451,214 @@ EditionIDs GeoLite2-City GeoLite2-Country } } +func TestSetConfigFromFile(t *testing.T) { + tests := []struct { + Description string + Input string + Expected Config + Err string + }{ + { + Description: "All config file related variables", + Input: `AccountID 1 + DatabaseDirectory /tmp/db + EditionIDs GeoLite2-Country GeoLite2-City + Host updates.maxmind.com + LicenseKey 000000000001 + LockFile /tmp/lock + Parallelism 2 + PreserveFileTimes 1 + Proxy 127.0.0.1:8888 + ProxyUserPassword username:password + RetryFor 1m + `, + Expected: Config{ + AccountID: 1, + DatabaseDirectory: filepath.Clean("/tmp/db"), + EditionIDs: []string{"GeoLite2-Country", "GeoLite2-City"}, + LicenseKey: "000000000001", + LockFile: filepath.Clean("/tmp/lock"), + Parallelism: 2, + PreserveFileTimes: true, + proxyURL: "127.0.0.1:8888", + proxyUserInfo: "username:password", + RetryFor: 1 * time.Minute, + URL: "https://updates.maxmind.com", + }, + }, + { + Description: "Empty config", + Input: "", + Expected: Config{}, + }, + { + Description: "Invalid account ID", + Input: "AccountID 1a", + Err: `invalid account ID format: strconv.Atoi: parsing "1a": invalid syntax`, + }, + { + Description: "Invalid PreserveFileTimes", + Input: "PreserveFileTimes 1a", + Err: "`PreserveFileTimes' must be 0 or 1", + }, + { + Description: "RetryFor needs a unit", + Input: "RetryFor 5", + Err: "'5' is not a valid duration", + }, + { + Description: "RetryFor needs to be non-negative", + Input: "RetryFor -5m", + Err: "'-5m' is not a valid duration", + }, + { + Description: "Parallelism should be a number", + Input: "Parallelism a", + Err: "'a' is not a valid parallelism value: strconv.Atoi: parsing \"a\": invalid syntax", + }, + { + Description: "Parallelism should be a positive number", + Input: "Parallelism 0", + Err: "parallelism should be greater than 0, got '0'", + }, + } + + tempFh, err := os.CreateTemp("", "conf-test") + require.NoError(t, err) + tempName := tempFh.Name() + require.NoError(t, tempFh.Close()) + defer func() { + _ = os.Remove(tempName) + }() + + for _, test := range tests { + t.Run(test.Description, func(t *testing.T) { + require.NoError(t, os.WriteFile(tempName, []byte(test.Input), 0o600)) + + var config Config + + err := setConfigFromFile(&config, tempName) + if test.Err == "" { + assert.NoError(t, err, test.Description) + } else { + assert.EqualError(t, err, test.Err, test.Description) + } + assert.Equal(t, test.Expected, config, test.Description) + }) + } +} + +func TestSetConfigFromFlags(t *testing.T) { + tests := []struct { + Description string + Flags []Option + Expected Config + Err string + }{ + { + Description: "All option flag related config set", + Flags: []Option{ + WithDatabaseDirectory("/tmp/db"), + WithOutput(true), + WithParallelism(2), + WithVerbose(true), + }, + Expected: Config{ + DatabaseDirectory: filepath.Clean("/tmp/db"), + Output: true, + Parallelism: 2, + Verbose: true, + }, + }, + { + Description: "Empty config", + Flags: []Option{}, + Expected: Config{}, + }, + { + Description: "Parallelism should be a positive number", + Flags: []Option{WithParallelism(-1)}, + Err: "error applying flag to config: parallelism can't be negative, got '-1'", + }, + } + + for _, test := range tests { + t.Run(test.Description, func(t *testing.T) { + var config Config + + err := setConfigFromFlags(&config, test.Flags...) + if test.Err == "" { + assert.NoError(t, err, test.Description) + } else { + assert.EqualError(t, err, test.Err, test.Description) + } + assert.Equal(t, test.Expected, config, test.Description) + }) + } +} + +func TestValidateConfig(t *testing.T) { + tests := []struct { + Description string + Config Config + Err string + }{ + { + Description: "Basic config", + Config: Config{ + AccountID: 42, + LicenseKey: "000000000001", + DatabaseDirectory: "/tmp/db", + EditionIDs: []string{"GeoLite2-Country", "GeoLite2-City"}, + LockFile: "/tmp/lock", + URL: "https://updates.maxmind.com", + RetryFor: 5 * time.Minute, + Parallelism: 1, + }, + Err: "", + }, + { + Description: "EditionIDs required", + Config: Config{}, + Err: "the `EditionIDs` option is required", + }, + { + Description: "AccountID required", + Config: Config{ + EditionIDs: []string{"GeoLite2-Country", "GeoLite2-City"}, + }, + Err: "the `AccountID` option is required", + }, + { + Description: "LicenseKey required", + Config: Config{ + AccountID: 42, + EditionIDs: []string{"GeoLite2-Country", "GeoLite2-City"}, + }, + Err: "the `LicenseKey` option is required", + }, + { + Description: "Valid AccountID + LicenseKey combination", + Config: Config{ + AccountID: 999999, + LicenseKey: "000000000000", + }, + Err: "geoipupdate requires a valid AccountID and LicenseKey combination", + }, + } + + for _, test := range tests { + t.Run(test.Description, func(t *testing.T) { + err := validateConfig(&test.Config) + if test.Err == "" { + assert.NoError(t, err, test.Description) + } else { + assert.EqualError(t, err, test.Err, test.Description) + } + }) + } +} + func TestParseProxy(t *testing.T) { tests := []struct { Proxy string From afa60928218f3d326a65ad3a23ee6370591e8b19 Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Tue, 20 Jun 2023 10:55:11 -0500 Subject: [PATCH 02/25] Add environment variable configuration options Previous various parts of the config could only be set in a config file (and some parts by the config file or command line options). This updates the config package to also set config options based on environment variables after the config file and before command line options are used. --- pkg/geoipupdate/config.go | 91 +++++++++++++++- pkg/geoipupdate/config_test.go | 188 +++++++++++++++++++++++++++++++-- 2 files changed, 268 insertions(+), 11 deletions(-) diff --git a/pkg/geoipupdate/config.go b/pkg/geoipupdate/config.go index 38d1c8a5..a62cdbc0 100644 --- a/pkg/geoipupdate/config.go +++ b/pkg/geoipupdate/config.go @@ -102,9 +102,10 @@ func WithOutput(val bool) Option { } } -// NewConfig parses the configuration file. -// flagOptions is provided to provide optional flag overrides to the config -// file. +// NewConfig create a new configuration and populates it based on a config +// file point to by 'path', then by various environment variables, and then +// finally by flag overrides provided by flagOptions. Values from the later +// override the former. func NewConfig( path string, flagOptions ...Option, @@ -123,6 +124,12 @@ func NewConfig( return nil, err } + // Override config with values from environment variables. + err = setConfigFromEnv(config) + if err != nil { + return nil, err + } + // Override config with values from option flags. err = setConfigFromFlags(config, flagOptions...) if err != nil { @@ -252,6 +259,84 @@ func setConfigFromFile(config *Config, path string) error { return nil } +// setConfigFromEnv sets Config fields based on environment variables. +func setConfigFromEnv(config *Config) error { + if value, ok := os.LookupEnv("GEOIPUPDATE_ACCOUNT_ID"); ok { + var err error + config.AccountID, err = strconv.Atoi(value) + if err != nil { + return fmt.Errorf("invalid account ID format: %w", err) + } + } + + if value, ok := os.LookupEnv("GEOIPUPDATE_DB_DIR"); ok { + config.DatabaseDirectory = value + } + + if value, ok := os.LookupEnv("GEOIPUPDATE_EDITION_IDS"); ok { + config.EditionIDs = strings.Fields(value) + } + + if value, ok := os.LookupEnv("GEOIPUPDATE_HOST"); ok { + config.URL = "https://" + value + } + + if value, ok := os.LookupEnv("GEOIPUPDATE_LICENSE_KEY"); ok { + config.LicenseKey = value + } + + if value, ok := os.LookupEnv("GEOIPUPDATE_LOCK_FILE"); ok { + config.LockFile = value + } + + if value, ok := os.LookupEnv("GEOIPUPDATE_PARALLELISM"); ok { + parallelism, err := strconv.Atoi(value) + if err != nil { + return fmt.Errorf("'%s' is not a valid parallelism value: %w", value, err) + } + if parallelism <= 0 { + return fmt.Errorf("parallelism should be greater than 0, got '%d'", parallelism) + } + config.Parallelism = parallelism + } + + if value, ok := os.LookupEnv("GEOIPUPDATE_PRESERVE_FILE_TIMES"); ok { + if value != "0" && value != "1" { + return errors.New("`PreserveFileTimes' must be 0 or 1") + } + if value == "1" { + config.PreserveFileTimes = true + } + } + + if value, ok := os.LookupEnv("GEOIPUPDATE_PROXY"); ok { + config.proxyURL = value + } + + if value, ok := os.LookupEnv("GEOIPUPDATE_PROXY_USER_PASSWORD"); ok { + config.proxyUserInfo = value + } + + if value, ok := os.LookupEnv("GEOIPUPDATE_RETRY_FOR"); ok { + dur, err := time.ParseDuration(value) + if err != nil || dur < 0 { + return fmt.Errorf("'%s' is not a valid duration", value) + } + config.RetryFor = dur + } + + if value, ok := os.LookupEnv("GEOIPUPDATE_VERBOSE"); ok { + if value != "0" && value != "1" { + return errors.New("'Verbose' must be 0 or 1") + } + if value == "1" { + config.Verbose = true + } + } + + return nil +} + // setConfigFromFlags sets Config fields based on option flags. func setConfigFromFlags(config *Config, flagOptions ...Option) error { for _, option := range flagOptions { diff --git a/pkg/geoipupdate/config_test.go b/pkg/geoipupdate/config_test.go index 37e14989..9d45ce0d 100644 --- a/pkg/geoipupdate/config_test.go +++ b/pkg/geoipupdate/config_test.go @@ -5,6 +5,7 @@ import ( "net/url" "os" "path/filepath" + "strings" "testing" "time" @@ -17,6 +18,7 @@ func TestNewConfig(t *testing.T) { tests := []struct { Description string Input string + Env map[string]string Flags []Option Output *Config Err string @@ -427,6 +429,41 @@ EditionIDs GeoLite2-City GeoLite2-Country Parallelism: 1, }, }, + { + Description: "Config flags override env vars override config file", + Input: "AccountID\t\t123\nLicenseKey\t\t456\nParallelism\t\t1\n", + Env: map[string]string{ + "GEOIPUPDATE_DB_DIR": "/tmp/db", + "GEOIPUPDATE_EDITION_IDS": "GeoLite2-Country GeoLite2-City", + "GEOIPUPDATE_HOST": "updates.maxmind.com", + "GEOIPUPDATE_LICENSE_KEY": "000000000001", + "GEOIPUPDATE_LOCK_FILE": "/tmp/lock", + "GEOIPUPDATE_PARALLELISM": "2", + "GEOIPUPDATE_PRESERVE_FILE_TIMES": "1", + "GEOIPUPDATE_PROXY": "127.0.0.1:8888", + "GEOIPUPDATE_PROXY_USER_PASSWORD": "username:password", + "GEOIPUPDATE_RETRY_FOR": "1m", + "GEOIPUPDATE_VERBOSE": "1", + }, + Flags: []Option{WithParallelism(3)}, + Output: &Config{ + AccountID: 123, + DatabaseDirectory: "/tmp/db", + EditionIDs: []string{"GeoLite2-Country", "GeoLite2-City"}, + LicenseKey: "000000000001", + LockFile: "/tmp/lock", + Parallelism: 3, + PreserveFileTimes: true, + Proxy: &url.URL{ + Scheme: "http", + User: url.UserPassword("username", "password"), + Host: "127.0.0.1:8888", + }, + RetryFor: 1 * time.Minute, + URL: "https://updates.maxmind.com", + Verbose: true, + }, + }, } tempFh, err := os.CreateTemp("", "conf-test") @@ -439,14 +476,16 @@ EditionIDs GeoLite2-City GeoLite2-Country for _, test := range tests { t.Run(test.Description, func(t *testing.T) { - require.NoError(t, os.WriteFile(tempName, []byte(test.Input), 0o600)) - config, err := NewConfig(tempName, test.Flags...) - if test.Err == "" { - assert.NoError(t, err, test.Description) - } else { - assert.EqualError(t, err, test.Err, test.Description) - } - assert.Equal(t, test.Output, config, test.Description) + withEnvVars(t, test.Env, func() { + require.NoError(t, os.WriteFile(tempName, []byte(test.Input), 0o600)) + config, err := NewConfig(tempName, test.Flags...) + if test.Err == "" { + assert.NoError(t, err, test.Description) + } else { + assert.EqualError(t, err, test.Err, test.Description) + } + assert.Equal(t, test.Output, config, test.Description) + }) }) } } @@ -548,6 +587,117 @@ func TestSetConfigFromFile(t *testing.T) { } } +func TestSetConfigFromEnv(t *testing.T) { + tests := []struct { + Description string + Env map[string]string + Expected Config + Err string + }{ + { + Description: "All config related environment variables", + Env: map[string]string{ + "GEOIPUPDATE_ACCOUNT_ID": "1", + "GEOIPUPDATE_DB_DIR": "/tmp/db", + "GEOIPUPDATE_EDITION_IDS": "GeoLite2-Country GeoLite2-City", + "GEOIPUPDATE_HOST": "updates.maxmind.com", + "GEOIPUPDATE_LICENSE_KEY": "000000000001", + "GEOIPUPDATE_LOCK_FILE": "/tmp/lock", + "GEOIPUPDATE_PARALLELISM": "2", + "GEOIPUPDATE_PRESERVE_FILE_TIMES": "1", + "GEOIPUPDATE_PROXY": "127.0.0.1:8888", + "GEOIPUPDATE_PROXY_USER_PASSWORD": "username:password", + "GEOIPUPDATE_RETRY_FOR": "1m", + "GEOIPUPDATE_VERBOSE": "1", + }, + Expected: Config{ + AccountID: 1, + DatabaseDirectory: "/tmp/db", + EditionIDs: []string{"GeoLite2-Country", "GeoLite2-City"}, + LicenseKey: "000000000001", + LockFile: "/tmp/lock", + Parallelism: 2, + PreserveFileTimes: true, + proxyURL: "127.0.0.1:8888", + proxyUserInfo: "username:password", + RetryFor: 1 * time.Minute, + URL: "https://updates.maxmind.com", + Verbose: true, + }, + }, + { + Description: "Empty config", + Env: map[string]string{}, + Expected: Config{}, + }, + { + Description: "Invalid account ID", + Env: map[string]string{ + "GEOIPUPDATE_ACCOUNT_ID": "1a", + }, + Err: `invalid account ID format: strconv.Atoi: parsing "1a": invalid syntax`, + }, + { + Description: "Invalid PreserveFileTimes", + Env: map[string]string{ + "GEOIPUPDATE_PRESERVE_FILE_TIMES": "1a", + }, + Err: "`PreserveFileTimes' must be 0 or 1", + }, + { + Description: "RetryFor needs a unit", + Env: map[string]string{ + "GEOIPUPDATE_RETRY_FOR": "5", + }, + Err: "'5' is not a valid duration", + }, + { + Description: "RetryFor needs to be non-negative", + Env: map[string]string{ + "GEOIPUPDATE_RETRY_FOR": "-5m", + }, + Err: "'-5m' is not a valid duration", + }, + { + Description: "Parallelism should be a number", + Env: map[string]string{ + "GEOIPUPDATE_PARALLELISM": "a", + }, + Err: "'a' is not a valid parallelism value: strconv.Atoi: parsing \"a\": invalid syntax", + }, + { + Description: "Parallelism should be a positive number", + Env: map[string]string{ + "GEOIPUPDATE_PARALLELISM": "0", + }, + Err: "parallelism should be greater than 0, got '0'", + }, + { + Description: "Invalid Verbose", + Env: map[string]string{ + "GEOIPUPDATE_VERBOSE": "1a", + }, + Err: "'Verbose' must be 0 or 1", + }, + } + + for _, test := range tests { + t.Run(test.Description, func(t *testing.T) { + withEnvVars(t, test.Env, func() { + var config Config + + err := setConfigFromEnv(&config) + if test.Err == "" { + assert.NoError(t, err, test.Description) + } else { + assert.EqualError(t, err, test.Err, test.Description) + } + assert.Equal(t, test.Expected, config, test.Description) + }) + }) + } +} + func TestSetConfigFromFlags(t *testing.T) { tests := []struct { Description string @@ -745,3 +895,25 @@ func TestParseProxy(t *testing.T) { ) } } + +func withEnvVars(t *testing.T, newEnvVars map[string]string, f func()) { + origEnv := os.Environ() + + for key, val := range newEnvVars { + err := os.Setenv(key, val) + require.NoError(t, err) + } + + // Execute the test + f() + + // Clean the environment + os.Clearenv() + + // Reset the original environment variables + for _, pair := range origEnv { + parts := strings.SplitN(pair, "=", 2) + err := os.Setenv(parts[0], parts[1]) + require.NoError(t, err) + } +} From 37011a68a4d0bb5713a72decbb5840f3dce3d3ca Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Tue, 20 Jun 2023 15:39:21 -0500 Subject: [PATCH 03/25] Use environment variables in docker entry instead of config file --- docker/entry.sh | 30 ++++-------------------------- 1 file changed, 4 insertions(+), 26 deletions(-) diff --git a/docker/entry.sh b/docker/entry.sh index 85ea22a0..1c690a7e 100755 --- a/docker/entry.sh +++ b/docker/entry.sh @@ -42,33 +42,11 @@ if [ -z "$GEOIPUPDATE_ACCOUNT_ID" ] || [ -z "$GEOIPUPDATE_LICENSE_KEY" ] || [ - exit 1 fi -# Create configuration file +# Create an empty configuration file. All configuration is provided via +# environment variables or command line options, but geoipupdate still +# expects a configuration file to exist. echo "# STATE: Creating configuration file at $conf_file" -cat < "$conf_file" -AccountID $GEOIPUPDATE_ACCOUNT_ID -LicenseKey $GEOIPUPDATE_LICENSE_KEY -EditionIDs $GEOIPUPDATE_EDITION_IDS -EOF - -if [ ! -z "$GEOIPUPDATE_HOST" ]; then - echo "Host $GEOIPUPDATE_HOST" >> "$conf_file" -fi - -if [ ! -z "$GEOIPUPDATE_PROXY" ]; then - echo "Proxy $GEOIPUPDATE_PROXY" >> "$conf_file" -fi - -if [ ! -z "$GEOIPUPDATE_PROXY_USER_PASSWORD" ]; then - echo "ProxyUserPassword $GEOIPUPDATE_PROXY_USER_PASSWORD" >> "$conf_file" -fi - -if [ ! -z "$GEOIPUPDATE_PRESERVE_FILE_TIMES" ]; then - echo "PreserveFileTimes $GEOIPUPDATE_PRESERVE_FILE_TIMES" >> "$conf_file" -fi - -if [ "$GEOIPUPDATE_VERBOSE" ]; then - flags="$flags -v" -fi +touch "$conf_file" mkdir -p $log_dir From acb13ed14fa4553329633775d2cf909cff363b40 Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Wed, 21 Jun 2023 08:40:35 -0500 Subject: [PATCH 04/25] Update docs to reference enviroment variable configuration --- conf/GeoIP.env.default | 49 ++++++++++++++++++++++ dev-bin/make-man-pages.pl | 6 +++ doc/GeoIP.conf.md | 42 ++++++++++++------- doc/GeoIP.env.md | 88 +++++++++++++++++++++++++++++++++++++++ doc/geoipupdate.md | 8 ++-- 5 files changed, 176 insertions(+), 17 deletions(-) create mode 100644 conf/GeoIP.env.default create mode 100644 doc/GeoIP.env.md diff --git a/conf/GeoIP.env.default b/conf/GeoIP.env.default new file mode 100644 index 00000000..a6d0a8ac --- /dev/null +++ b/conf/GeoIP.env.default @@ -0,0 +1,49 @@ +# Note that environment variables override any associated config setting from +# a config file. + +# Replace YOUR_ACCOUNT_ID_HERE and YOUR_LICENSE_KEY_HERE with an active account +# ID and license key combination associated with your MaxMind account. These +# are available from https://www.maxmind.com/en/my_license_key. +GEOIPUPDATE_ACCOUNT_ID="YOUR_ACCOUNT_ID_HERE" +GEOIPUPDATE_LICENSE_KEY="YOUR_LICENSE_KEY_HERE" + +# Enter the edition IDs of the databases you would like to update. +# Multiple edition IDs are separated by spaces. +GEOIPUPDATE_EDITION_IDS="GeoLite2-Country GeoLite2-City" + +# The remaining settings are OPTIONAL. + +# The directory to store the database files. Defaults to $DATADIR +# GEOIPUPDATE_DB_DIR="$DATADIR" + +# The server to use. Defaults to "updates.maxmind.com". +# GEOIPUPDATE_HOST="updates.maxmind.com" + +# The proxy host name or IP address. You may optionally specify a +# port number, e.g., 127.0.0.1:8888. If no port number is specified, 1080 +# will be used. +# GEOIPUPDATE_PROXY="127.0.0.1:8888" + +# The user name and password to use with your proxy server. +# GEOIPUPDATE_PROXY_USER_PASSWORD="username:password" + +# Whether to preserve modification times of files downloaded from the server. +# Defaults to "0". +# GEOIPUPDATE_PRESERVE_FILE_TIMES=0 + +# The lock file to use. This ensures only one geoipupdate process can run at a +# time. +# Note: Once created, this lockfile is not removed from the filesystem. +# Defaults to ".geoipupdate.lock" under the $GEOIPUPDATE_DB_DIR. +# GEOIPUPDATE_LOCK_FILE="$GEOIPUPDATE_DB_DIR/.geoipupdate.lock + +# The amount of time to retry for when errors during HTTP transactions are +# encountered. It can be specified as a (possibly fractional) decimal number +# followed by a unit suffix. Valid time units are "ns", "us" (or "µs"), "ms", +# "s", "m", "h". +# Defaults to "5m" (5 minutes). +# GEOIPUPDATE_RETRY_FOR="5m" + +# The number of parallel database downloads. +# Defaults to "1". +# GEOIPUPDATE_PARALLELISM=1 diff --git a/dev-bin/make-man-pages.pl b/dev-bin/make-man-pages.pl index 82acb072..99bcf88b 100755 --- a/dev-bin/make-man-pages.pl +++ b/dev-bin/make-man-pages.pl @@ -19,6 +19,12 @@ sub main { "$build_dir/GeoIP.conf.md", "$build_dir/GeoIP.conf.5", ); + _make_man( + 'GeoIP.env', + 9, + "$build_dir/GeoIP.env.md", + "$build_dir/GeoIP.env.9", + ); return 1; } diff --git a/doc/GeoIP.conf.md b/doc/GeoIP.conf.md index 272997a9..7a61614b 100644 --- a/doc/GeoIP.conf.md +++ b/doc/GeoIP.conf.md @@ -17,66 +17,79 @@ sensitive. `AccountID` -: Your MaxMind account ID. This was formerly known as `UserId`. +: Your MaxMind account ID. This was formerly known as `UserId`. This can be + overridden at run time by the `GEOIPUPDATE_ACCOUNT_ID` environment + variable. `LicenseKey` -: Your case-sensitive MaxMind license key. +: Your case-sensitive MaxMind license key. This can be overriden at run time + by the `GEOIPUPDATE_LICENSE_KEY` environment variables. `EditionIDs` : List of space-separated database edition IDs. Edition IDs may consist of letters, digits, and dashes. For example, `GeoIP2-City` would - download the GeoIP2 City database (`GeoIP2-City`). Note: this was - formerly called `ProductIds`. + download the GeoIP2 City database (`GeoIP2-City`). This can be overridden + at run time by the `GEOIPUPDATE_EDITION_IDS` environment variable. Note: + this was formerly called `ProductIds`. ## Optional settings: `DatabaseDirectory` : The directory to store the database files. If not set, the default is - DATADIR. This can be overridden at run time by the `-d` command line - argument. + DATADIR. This can be overridden at run time by the `GEOIPUPDATE_DB_DIR` + environment variable or the `-d` command line argument. `Host` : The host name of the server to use. The default is `updates.maxmind.com`. + This can be overridden at run time by the `GEOIPUPDATE_HOST` environment + variable. `Proxy` -: The proxy host name or IP address. You may optionally specify - a port number, e.g., `127.0.0.1:8888`. If no port number is specified, - 1080 will be used. +: The proxy host name or IP address. You may optionally specify a port + number, e.g., `127.0.0.1:8888`. If no port number is specified, 1080 + will be used. This can be overridden at run time by the + `GEOIPUPDATE_PROXY` environment variable. `ProxyUserPassword` : The proxy user name and password, separated by a colon. For instance, - `username:password`. + `username:password`. This can be overridden at run time by the + `GEOIPUPDATE_USER_PASSWORD` environment variable. `PreserveFileTimes` : Whether to preserve modification times of files downloaded from the - server. This option is either `0` or `1`. The default is `0`. + server. This option is either `0` or `1`. The default is `0`. This + can be overridden at run time by the `GEOIPUPDATE_PRESERVE_FILE_TIMES` + environment variable. `LockFile` : The lock file to use. This ensures only one `geoipupdate` process can run at a time. Note: Once created, this lockfile is not removed from the filesystem. The default is `.geoipupdate.lock` under the - `DatabaseDirectory`. + `DatabaseDirectory`. This can be overridden at run time by the + `GEOIPUPDATE_LOCK_FILE` environment variable. `RetryFor` : The amount of time to retry for when errors during HTTP transactions are encountered. It can be specified as a (possibly fractional) decimal number followed by a unit suffix. Valid time units are `ns`, `us` (or `µs`), `ms`, - `s`, `m`, `h`. The default is `5m` (5 minutes). + `s`, `m`, `h`. The default is `5m` (5 minutes). This can be overridden at + run time by the `GEOIPUPDATE_RETRY_FOR` environment variable. `Parallelism` : The maximum number of parallel database downloads. The default is 1, which means that databases will be downloaded sequentially. This can be - overriden at runtime by the `--parallelism` command line argument. + overriden at runtime by the `GEOIPUPDATE_PARALLELISM` environment variable + or the `--parallelism` command line argument. ## Deprecated settings: @@ -91,3 +104,4 @@ The following are deprecated and will be ignored if present: # SEE ALSO `geoipupdate`(1) +`GeoIP.env`(9) diff --git a/doc/GeoIP.env.md b/doc/GeoIP.env.md new file mode 100644 index 00000000..cb297842 --- /dev/null +++ b/doc/GeoIP.env.md @@ -0,0 +1,88 @@ +# NAME + +GeoIP.env - Configuration environment variables for geoipupdate + +# SYNOPSIS + +These environment variables allow you to configure your `geoipupdate` +program to download GeoIP2 and GeoLite2 databases. + +# DESCRIPTION + +Environment variables override any associated configuration that may +have been set from the config file. + +## Required settings: + +`GEOIPUPDATE_ACCOUNT_ID` + +: Your MaxMind account ID. This can be overriden at runtime by the +`--parallelism` command line argument. + +`GEOIPUPDATE_LICENSE_KEY` + +: Your case-sensitive MaxMind license key. + +`GEOIPUPDATE_EDITION_IDS` + +: List of space-separated database edition IDs. Edition IDs may consist + of letters, digits, and dashes. For example, `GeoIP2-City` would + download the GeoIP2 City database (`GeoIP2-City`). + +## Optional settings: + +`GEOIPUPDATE_DB_DIR` + +: The directory to store the database files. If not set, the default is + DATADIR. This can be overridden at run time by the `-d` command line + argument. + +`GEOIPUPDATE_HOST` + +: The host name of the server to use. The default is `updates.maxmind.com`. + +`GEOUPUPDATE_PROXY` + +: The proxy host name or IP address. You may optionally specify + a port number, e.g., `127.0.0.1:8888`. If no port number is specified, + 1080 will be used. + +`GEOIPUPDATE_PROXY_USER_PASSWORD` + +: The proxy user name and password, separated by a colon. For instance, + `username:password`. + +`GEOIPUPDATE_PRESERVE_FILE_TIMES` + +: Whether to preserve modification times of files downloaded from the + server. This option is either `0` or `1`. The default is `0`. + +`GEOIPUPDATE_LOCK_FILE` + +: The lock file to use. This ensures only one `geoipupdate` process can run + at a time. Note: Once created, this lockfile is not removed from the + filesystem. The default is `.geoipupdate.lock` under the + `GEOIPUPDATE_DB_DIR`. + +`GEOIPUPDATE_RETRY_FOR` + +: The amount of time to retry for when errors during HTTP transactions are + encountered. It can be specified as a (possibly fractional) decimal number + followed by a unit suffix. Valid time units are `ns`, `us` (or `µs`), `ms`, + `s`, `m`, `h`. The default is `5m` (5 minutes). + +`GEOIPUPDATE_PARALLELISM` + +: The maximum number of parallel database downloads. The default is + 1, which means that databases will be downloaded sequentially. This can be + overriden at runtime by the `--parallelism` command line argument. + +`VERBOSE` + +: Enable verbose mode. Prints out the steps that `geoipupdate` takes. This + can be overriden at runtime by the `--verbose` command line argument. + +# SEE ALSO + +`geoipupdate`(1) +`GeoIP.conf`(5) diff --git a/doc/geoipupdate.md b/doc/geoipupdate.md index 0cc7f0f2..850d8b4f 100644 --- a/doc/geoipupdate.md +++ b/doc/geoipupdate.md @@ -72,9 +72,10 @@ runs `geoipupdate` on each Wednesday at noon: # end of crontab -To use with a proxy server, update your `GeoIP.conf` file as specified -in the `GeoIP.conf` man page or set the `http_proxy` environment -variable. +To use with a proxy server, update your `GeoIP.conf` file as specified in +the `GeoIP.conf` man page or set the `GEOIPUPDATE_PROXY` environment variable +as specified in the `GeoIP.env` man page. Alternatively, set the `http_proxy` +environment variable. # BUGS @@ -97,3 +98,4 @@ to learn more about the GeoIP2 databases or to sign up for a subscription. # SEE ALSO `GeoIP.conf`(5) +`GeoIP.env`(9) From 20ab1a62b7b4eca6d8ddd17020899d9e8e1864d8 Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Fri, 23 Jun 2023 08:28:20 -0500 Subject: [PATCH 05/25] Remove environment config specific documentation The various environment variables are referenced in the config file document already, so we can remove these as mostly duplicative. --- conf/GeoIP.env.default | 49 ---------------------- dev-bin/make-man-pages.pl | 6 --- doc/GeoIP.conf.md | 1 - doc/GeoIP.env.md | 88 --------------------------------------- doc/geoipupdate.md | 1 - 5 files changed, 145 deletions(-) delete mode 100644 conf/GeoIP.env.default delete mode 100644 doc/GeoIP.env.md diff --git a/conf/GeoIP.env.default b/conf/GeoIP.env.default deleted file mode 100644 index a6d0a8ac..00000000 --- a/conf/GeoIP.env.default +++ /dev/null @@ -1,49 +0,0 @@ -# Note that environment variables override any associated config setting from -# a config file. - -# Replace YOUR_ACCOUNT_ID_HERE and YOUR_LICENSE_KEY_HERE with an active account -# ID and license key combination associated with your MaxMind account. These -# are available from https://www.maxmind.com/en/my_license_key. -GEOIPUPDATE_ACCOUNT_ID="YOUR_ACCOUNT_ID_HERE" -GEOIPUPDATE_LICENSE_KEY="YOUR_LICENSE_KEY_HERE" - -# Enter the edition IDs of the databases you would like to update. -# Multiple edition IDs are separated by spaces. -GEOIPUPDATE_EDITION_IDS="GeoLite2-Country GeoLite2-City" - -# The remaining settings are OPTIONAL. - -# The directory to store the database files. Defaults to $DATADIR -# GEOIPUPDATE_DB_DIR="$DATADIR" - -# The server to use. Defaults to "updates.maxmind.com". -# GEOIPUPDATE_HOST="updates.maxmind.com" - -# The proxy host name or IP address. You may optionally specify a -# port number, e.g., 127.0.0.1:8888. If no port number is specified, 1080 -# will be used. -# GEOIPUPDATE_PROXY="127.0.0.1:8888" - -# The user name and password to use with your proxy server. -# GEOIPUPDATE_PROXY_USER_PASSWORD="username:password" - -# Whether to preserve modification times of files downloaded from the server. -# Defaults to "0". -# GEOIPUPDATE_PRESERVE_FILE_TIMES=0 - -# The lock file to use. This ensures only one geoipupdate process can run at a -# time. -# Note: Once created, this lockfile is not removed from the filesystem. -# Defaults to ".geoipupdate.lock" under the $GEOIPUPDATE_DB_DIR. -# GEOIPUPDATE_LOCK_FILE="$GEOIPUPDATE_DB_DIR/.geoipupdate.lock - -# The amount of time to retry for when errors during HTTP transactions are -# encountered. It can be specified as a (possibly fractional) decimal number -# followed by a unit suffix. Valid time units are "ns", "us" (or "µs"), "ms", -# "s", "m", "h". -# Defaults to "5m" (5 minutes). -# GEOIPUPDATE_RETRY_FOR="5m" - -# The number of parallel database downloads. -# Defaults to "1". -# GEOIPUPDATE_PARALLELISM=1 diff --git a/dev-bin/make-man-pages.pl b/dev-bin/make-man-pages.pl index 99bcf88b..82acb072 100755 --- a/dev-bin/make-man-pages.pl +++ b/dev-bin/make-man-pages.pl @@ -19,12 +19,6 @@ sub main { "$build_dir/GeoIP.conf.md", "$build_dir/GeoIP.conf.5", ); - _make_man( - 'GeoIP.env', - 9, - "$build_dir/GeoIP.env.md", - "$build_dir/GeoIP.env.9", - ); return 1; } diff --git a/doc/GeoIP.conf.md b/doc/GeoIP.conf.md index 7a61614b..8b917d68 100644 --- a/doc/GeoIP.conf.md +++ b/doc/GeoIP.conf.md @@ -104,4 +104,3 @@ The following are deprecated and will be ignored if present: # SEE ALSO `geoipupdate`(1) -`GeoIP.env`(9) diff --git a/doc/GeoIP.env.md b/doc/GeoIP.env.md deleted file mode 100644 index cb297842..00000000 --- a/doc/GeoIP.env.md +++ /dev/null @@ -1,88 +0,0 @@ -# NAME - -GeoIP.env - Configuration environment variables for geoipupdate - -# SYNOPSIS - -These environment variables allow you to configure your `geoipupdate` -program to download GeoIP2 and GeoLite2 databases. - -# DESCRIPTION - -Environment variables override any associated configuration that may -have been set from the config file. - -## Required settings: - -`GEOIPUPDATE_ACCOUNT_ID` - -: Your MaxMind account ID. This can be overriden at runtime by the -`--parallelism` command line argument. - -`GEOIPUPDATE_LICENSE_KEY` - -: Your case-sensitive MaxMind license key. - -`GEOIPUPDATE_EDITION_IDS` - -: List of space-separated database edition IDs. Edition IDs may consist - of letters, digits, and dashes. For example, `GeoIP2-City` would - download the GeoIP2 City database (`GeoIP2-City`). - -## Optional settings: - -`GEOIPUPDATE_DB_DIR` - -: The directory to store the database files. If not set, the default is - DATADIR. This can be overridden at run time by the `-d` command line - argument. - -`GEOIPUPDATE_HOST` - -: The host name of the server to use. The default is `updates.maxmind.com`. - -`GEOUPUPDATE_PROXY` - -: The proxy host name or IP address. You may optionally specify - a port number, e.g., `127.0.0.1:8888`. If no port number is specified, - 1080 will be used. - -`GEOIPUPDATE_PROXY_USER_PASSWORD` - -: The proxy user name and password, separated by a colon. For instance, - `username:password`. - -`GEOIPUPDATE_PRESERVE_FILE_TIMES` - -: Whether to preserve modification times of files downloaded from the - server. This option is either `0` or `1`. The default is `0`. - -`GEOIPUPDATE_LOCK_FILE` - -: The lock file to use. This ensures only one `geoipupdate` process can run - at a time. Note: Once created, this lockfile is not removed from the - filesystem. The default is `.geoipupdate.lock` under the - `GEOIPUPDATE_DB_DIR`. - -`GEOIPUPDATE_RETRY_FOR` - -: The amount of time to retry for when errors during HTTP transactions are - encountered. It can be specified as a (possibly fractional) decimal number - followed by a unit suffix. Valid time units are `ns`, `us` (or `µs`), `ms`, - `s`, `m`, `h`. The default is `5m` (5 minutes). - -`GEOIPUPDATE_PARALLELISM` - -: The maximum number of parallel database downloads. The default is - 1, which means that databases will be downloaded sequentially. This can be - overriden at runtime by the `--parallelism` command line argument. - -`VERBOSE` - -: Enable verbose mode. Prints out the steps that `geoipupdate` takes. This - can be overriden at runtime by the `--verbose` command line argument. - -# SEE ALSO - -`geoipupdate`(1) -`GeoIP.conf`(5) diff --git a/doc/geoipupdate.md b/doc/geoipupdate.md index 850d8b4f..cf7d8e10 100644 --- a/doc/geoipupdate.md +++ b/doc/geoipupdate.md @@ -98,4 +98,3 @@ to learn more about the GeoIP2 databases or to sign up for a subscription. # SEE ALSO `GeoIP.conf`(5) -`GeoIP.env`(9) From 98b41b313e67e013fc74445e86458c795ac34837 Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Fri, 23 Jun 2023 08:33:53 -0500 Subject: [PATCH 06/25] Fix typos --- doc/GeoIP.conf.md | 8 ++++---- pkg/geoipupdate/config.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/GeoIP.conf.md b/doc/GeoIP.conf.md index 8b917d68..2f19ba02 100644 --- a/doc/GeoIP.conf.md +++ b/doc/GeoIP.conf.md @@ -23,8 +23,8 @@ sensitive. `LicenseKey` -: Your case-sensitive MaxMind license key. This can be overriden at run time - by the `GEOIPUPDATE_LICENSE_KEY` environment variables. +: Your case-sensitive MaxMind license key. This can be overridden at run time + by the `GEOIPUPDATE_LICENSE_KEY` environment variable. `EditionIDs` @@ -59,7 +59,7 @@ sensitive. : The proxy user name and password, separated by a colon. For instance, `username:password`. This can be overridden at run time by the - `GEOIPUPDATE_USER_PASSWORD` environment variable. + `GEOIPUPDATE_PROXY_USER_PASSWORD` environment variable. `PreserveFileTimes` @@ -88,7 +88,7 @@ sensitive. : The maximum number of parallel database downloads. The default is 1, which means that databases will be downloaded sequentially. This can be - overriden at runtime by the `GEOIPUPDATE_PARALLELISM` environment variable + overridden at runtime by the `GEOIPUPDATE_PARALLELISM` environment variable or the `--parallelism` command line argument. ## Deprecated settings: diff --git a/pkg/geoipupdate/config.go b/pkg/geoipupdate/config.go index a62cdbc0..bbf60a50 100644 --- a/pkg/geoipupdate/config.go +++ b/pkg/geoipupdate/config.go @@ -41,7 +41,7 @@ type Config struct { Proxy *url.URL // proxyURL is the host value of Proxy proxyURL string - // proxyUserainfo is the userinfo value of Proxy + // proxyUserInfo is the userinfo value of Proxy proxyUserInfo string // RetryFor is the retry timeout for HTTP requests. It defaults // to 5 minutes. @@ -103,7 +103,7 @@ func WithOutput(val bool) Option { } // NewConfig create a new configuration and populates it based on a config -// file point to by 'path', then by various environment variables, and then +// file poinedt to by 'path', then by various environment variables, and then // finally by flag overrides provided by flagOptions. Values from the later // override the former. func NewConfig( From 0b89f05afb8ffe053551f04b0bd6b8b66eaa0066 Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Fri, 23 Jun 2023 08:36:26 -0500 Subject: [PATCH 07/25] Remove confusing message While it is true we are still creating a configuration file, we aren't actually writing anything to it anymore and thus anyone who reads that message probably wouldn't be interested in it any longer. --- docker/entry.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/docker/entry.sh b/docker/entry.sh index 1c690a7e..f05a9d25 100755 --- a/docker/entry.sh +++ b/docker/entry.sh @@ -45,7 +45,6 @@ fi # Create an empty configuration file. All configuration is provided via # environment variables or command line options, but geoipupdate still # expects a configuration file to exist. -echo "# STATE: Creating configuration file at $conf_file" touch "$conf_file" mkdir -p $log_dir From 0352d5fc529396d365688705337f8eeff449c37a Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Fri, 23 Jun 2023 08:43:29 -0500 Subject: [PATCH 08/25] Simplify some boolean config assignment --- pkg/geoipupdate/config.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/pkg/geoipupdate/config.go b/pkg/geoipupdate/config.go index bbf60a50..dc8a1d3e 100644 --- a/pkg/geoipupdate/config.go +++ b/pkg/geoipupdate/config.go @@ -223,9 +223,7 @@ func setConfigFromFile(config *Config, path string) error { if value != "0" && value != "1" { return errors.New("`PreserveFileTimes' must be 0 or 1") } - if value == "1" { - config.PreserveFileTimes = true - } + config.PreserveFileTimes = value == "1" case "Proxy": config.proxyURL = value case "ProxyUserPassword": @@ -304,9 +302,7 @@ func setConfigFromEnv(config *Config) error { if value != "0" && value != "1" { return errors.New("`PreserveFileTimes' must be 0 or 1") } - if value == "1" { - config.PreserveFileTimes = true - } + config.PreserveFileTimes = value == "1" } if value, ok := os.LookupEnv("GEOIPUPDATE_PROXY"); ok { @@ -329,9 +325,7 @@ func setConfigFromEnv(config *Config) error { if value != "0" && value != "1" { return errors.New("'Verbose' must be 0 or 1") } - if value == "1" { - config.Verbose = true - } + config.Verbose = value == "1" } return nil From cbeaef4538cc715d8c4e51a34ea0af218993ffda Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Fri, 23 Jun 2023 08:53:25 -0500 Subject: [PATCH 09/25] Use t.TempDir() instead of os.CreateTemp() --- pkg/geoipupdate/config_test.go | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/pkg/geoipupdate/config_test.go b/pkg/geoipupdate/config_test.go index 9d45ce0d..dcd9b06a 100644 --- a/pkg/geoipupdate/config_test.go +++ b/pkg/geoipupdate/config_test.go @@ -466,17 +466,10 @@ EditionIDs GeoLite2-City GeoLite2-Country }, } - tempFh, err := os.CreateTemp("", "conf-test") - require.NoError(t, err) - tempName := tempFh.Name() - require.NoError(t, tempFh.Close()) - defer func() { - _ = os.Remove(tempName) - }() - for _, test := range tests { t.Run(test.Description, func(t *testing.T) { withEnvVars(t, test.Env, func() { + tempName := filepath.Join(t.TempDir(), "/GeoIP-test.conf") require.NoError(t, os.WriteFile(tempName, []byte(test.Input), 0o600)) config, err := NewConfig(tempName, test.Flags...) if test.Err == "" { @@ -562,16 +555,9 @@ func TestSetConfigFromFile(t *testing.T) { }, } - tempFh, err := os.CreateTemp("", "conf-test") - require.NoError(t, err) - tempName := tempFh.Name() - require.NoError(t, tempFh.Close()) - defer func() { - _ = os.Remove(tempName) - }() - for _, test := range tests { t.Run(test.Description, func(t *testing.T) { + tempName := filepath.Join(t.TempDir(), "/GeoIP-test.conf") require.NoError(t, os.WriteFile(tempName, []byte(test.Input), 0o600)) var config Config From f134315a727c49958f827f27ad9a5cec23378f52 Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Fri, 23 Jun 2023 09:09:51 -0500 Subject: [PATCH 10/25] Mention environment config options in changelog --- CHANGELOG.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7ef438e..75f468f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,25 @@ # CHANGELOG +## 5.2.0 + +* `geoipupdate` now supports configuration via environment variables. Any + configuration set this way will override any value from the config file, + but still be overridden by any associated command line option (if any). + The following new environment variables are supported: + + * `GEOIPUPDATE_ACCOUNT_ID` + * `GEOIPUPDATE_DB_DIR` + * `GEOIPUPDATE_EDITION_IDS` + * `GEOIPUPDATE_HOST` + * `GEOIPUPDATE_LICENSE_KEY` + * `GEOIPUPDATE_LOCK_FILE` + * `GEOIPUPDATE_PARALLELISM` + * `GEOIPUPDATE_PRESERVE_FILE_TIMES` + * `GEOIPUPDATE_PROXY` + * `GEOIPUPDATE_PROXY_USER_PASSWORD` + * `GEOIPUPDATE_RETRY_FOR` + * `GEOIPUPDATE_VERBOSE` + ## 5.1.1 (2023-05-08) * Based on feedback, the change to use a non-root user in 5.1.0 From 928b738a48f7f212a453507569faf4817e5844ce Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Fri, 23 Jun 2023 09:18:23 -0500 Subject: [PATCH 11/25] Update expected value in documentation --- doc/docker.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/docker.md b/doc/docker.md index 569817eb..af065628 100644 --- a/doc/docker.md +++ b/doc/docker.md @@ -33,7 +33,7 @@ The following are optional: of files downloaded from the server. This option is either `0` or `1`. The default is `0`. * `GEOIPUPDATE_VERBOSE` - Enable verbose mode. Prints out the steps that - `geoipupdate` takes. Set to **anything** (e.g., `1`) to enable. + `geoipupdate` takes. Set to `1` to enable. * `GEOIPUPDATE_CONF_FILE` - The path where the configuration file will be written. The default is `/etc/GeoIP.conf`. * `GEOIPUPDATE_DB_DIR` - The directory where geoipupdate will download the From 34a4203eb4b8a694e4a56becb1e428222e0abb16 Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Fri, 23 Jun 2023 12:59:12 -0500 Subject: [PATCH 12/25] Support $GEOIPUPDATE_CONF_FILE in geoipupdate --- CHANGELOG.md | 1 + cmd/geoipupdate/args.go | 7 ++++++- doc/docker.md | 5 +++-- doc/geoipupdate.md | 3 ++- docker/entry.sh | 8 ++++---- 5 files changed, 16 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75f468f6..6d0ea8a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ The following new environment variables are supported: * `GEOIPUPDATE_ACCOUNT_ID` + * `GEOIPUPDATE_CONF_FILE` * `GEOIPUPDATE_DB_DIR` * `GEOIPUPDATE_EDITION_IDS` * `GEOIPUPDATE_HOST` diff --git a/cmd/geoipupdate/args.go b/cmd/geoipupdate/args.go index d1b3a74c..8310c723 100644 --- a/cmd/geoipupdate/args.go +++ b/cmd/geoipupdate/args.go @@ -19,10 +19,15 @@ type Args struct { } func getArgs() *Args { + confFileDefault := vars.DefaultConfigFile + if value, ok := os.LookupEnv("GEOIPUPDATE_CONF_FILE"); ok { + confFileDefault = value + } + configFile := flag.StringP( "config-file", "f", - vars.DefaultConfigFile, + confFileDefault, "Configuration file", ) databaseDirectory := flag.StringP( diff --git a/doc/docker.md b/doc/docker.md index af065628..fd11cb41 100644 --- a/doc/docker.md +++ b/doc/docker.md @@ -34,8 +34,9 @@ The following are optional: default is `0`. * `GEOIPUPDATE_VERBOSE` - Enable verbose mode. Prints out the steps that `geoipupdate` takes. Set to `1` to enable. -* `GEOIPUPDATE_CONF_FILE` - The path where the configuration file will be - written. The default is `/etc/GeoIP.conf`. +* `GEOIPUPDATE_CONF_FILE` - The path of a configuration file to be used by + `geoipupdate`. If this file does not exist, an empty file will be created. + The default is `/etc/GeoIP.conf`. * `GEOIPUPDATE_DB_DIR` - The directory where geoipupdate will download the databases. The default is `/usr/share/GeoIP`. diff --git a/doc/geoipupdate.md b/doc/geoipupdate.md index cf7d8e10..1bab6c1a 100644 --- a/doc/geoipupdate.md +++ b/doc/geoipupdate.md @@ -26,7 +26,8 @@ open. `-f`, `--config-file` : The configuration file to use. See `GeoIP.conf` and its documentation for - more information. This is optional. It defaults to CONFFILE. + more information. This is optional. It defaults to the environment variable + `GEOIPUPDATE_CONF_FILE` if it is set, or CONFFILE otherwise. `--parallelism` diff --git a/docker/entry.sh b/docker/entry.sh index f05a9d25..9c899a7e 100755 --- a/docker/entry.sh +++ b/docker/entry.sh @@ -21,8 +21,8 @@ log_file="$log_dir/.healthcheck" flags="--output" frequency=$((GEOIPUPDATE_FREQUENCY * 60 * 60)) -if ! [ -z "$GEOIPUPDATE_CONF_FILE" ]; then - conf_file=$GEOIPUPDATE_CONF_FILE +if [ -z "$GEOIPUPDATE_CONF_FILE" ]; then + GEOIPUPDATE_CONF_FILE="$conf_file" fi if ! [ -z "$GEOIPUPDATE_DB_DIR" ]; then @@ -45,13 +45,13 @@ fi # Create an empty configuration file. All configuration is provided via # environment variables or command line options, but geoipupdate still # expects a configuration file to exist. -touch "$conf_file" +touch "$GEOIPUPDATE_CONF_FILE" mkdir -p $log_dir while true; do echo "# STATE: Running geoipupdate" - /usr/bin/geoipupdate -d "$database_dir" -f "$conf_file" $flags 1>$log_file + /usr/bin/geoipupdate -d "$database_dir" $flags 1>$log_file if [ "$frequency" -eq 0 ]; then break fi From 443d6155cf7fecd6db32b9000442fd2d06c87075 Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Fri, 23 Jun 2023 13:02:57 -0500 Subject: [PATCH 13/25] Update error to reflect non-file config errors --- cmd/geoipupdate/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/geoipupdate/main.go b/cmd/geoipupdate/main.go index 5f5ebcf5..c6f71005 100644 --- a/cmd/geoipupdate/main.go +++ b/cmd/geoipupdate/main.go @@ -48,7 +48,7 @@ func main() { geoipupdate.WithOutput(args.Output), ) if err != nil { - fatalLogger(fmt.Sprintf("error loading configuration file %s", args.ConfigFile), err) + fatalLogger(fmt.Sprintf("error loading configuration %s", args.ConfigFile), err) } if config.Verbose { From 62070e2d287c26312c9b5940f36c905ddb27d42d Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Fri, 23 Jun 2023 13:20:19 -0500 Subject: [PATCH 14/25] Use GEOIPUPDATE_DB_DIR instead of --database-directory --- docker/entry.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docker/entry.sh b/docker/entry.sh index 9c899a7e..12b7f55a 100755 --- a/docker/entry.sh +++ b/docker/entry.sh @@ -25,8 +25,8 @@ if [ -z "$GEOIPUPDATE_CONF_FILE" ]; then GEOIPUPDATE_CONF_FILE="$conf_file" fi -if ! [ -z "$GEOIPUPDATE_DB_DIR" ]; then - database_dir=$GEOIPUPDATE_DB_DIR +if [ -z "$GEOIPUPDATE_DB_DIR" ]; then + GEOIPUPDATE_DB_DIR="$database_dir" fi if [ ! -z "$GEOIPUPDATE_ACCOUNT_ID_FILE" ]; then @@ -51,7 +51,7 @@ mkdir -p $log_dir while true; do echo "# STATE: Running geoipupdate" - /usr/bin/geoipupdate -d "$database_dir" $flags 1>$log_file + /usr/bin/geoipupdate $flags 1>$log_file if [ "$frequency" -eq 0 ]; then break fi From 8f864e5f7ecbe2729d6b0f9ee9d40eb9f4a61851 Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Wed, 28 Jun 2023 09:42:25 -0500 Subject: [PATCH 15/25] Make config file path optional Previously a configuration file was required, even if all required configuration was supplied through other means. This removes that requirement, making the configuration file optional. --- cmd/geoipupdate/args.go | 5 ----- cmd/geoipupdate/main.go | 2 +- doc/docker.md | 2 +- docker/entry.sh | 10 --------- pkg/geoipupdate/config.go | 38 +++++++++++++++++++++++++++------- pkg/geoipupdate/config_test.go | 3 ++- 6 files changed, 35 insertions(+), 25 deletions(-) diff --git a/cmd/geoipupdate/args.go b/cmd/geoipupdate/args.go index 8310c723..bfcb0243 100644 --- a/cmd/geoipupdate/args.go +++ b/cmd/geoipupdate/args.go @@ -54,11 +54,6 @@ func getArgs() *Args { os.Exit(0) } - if *configFile == "" { - log.Printf("You must provide a configuration file.") - printUsage() - } - if *parallelism < 0 { log.Printf("Parallelism must be a positive number") printUsage() diff --git a/cmd/geoipupdate/main.go b/cmd/geoipupdate/main.go index c6f71005..f42a02a8 100644 --- a/cmd/geoipupdate/main.go +++ b/cmd/geoipupdate/main.go @@ -41,7 +41,7 @@ func main() { } config, err := geoipupdate.NewConfig( - args.ConfigFile, + geoipupdate.WithConfigFile(args.ConfigFile), geoipupdate.WithDatabaseDirectory(args.DatabaseDirectory), geoipupdate.WithParallelism(args.Parallelism), geoipupdate.WithVerbose(args.Verbose), diff --git a/doc/docker.md b/doc/docker.md index fd11cb41..312631d7 100644 --- a/doc/docker.md +++ b/doc/docker.md @@ -35,7 +35,7 @@ The following are optional: * `GEOIPUPDATE_VERBOSE` - Enable verbose mode. Prints out the steps that `geoipupdate` takes. Set to `1` to enable. * `GEOIPUPDATE_CONF_FILE` - The path of a configuration file to be used by - `geoipupdate`. If this file does not exist, an empty file will be created. + `geoipupdate`. The default is `/etc/GeoIP.conf`. * `GEOIPUPDATE_DB_DIR` - The directory where geoipupdate will download the databases. The default is `/usr/share/GeoIP`. diff --git a/docker/entry.sh b/docker/entry.sh index 12b7f55a..6eb8b2c4 100755 --- a/docker/entry.sh +++ b/docker/entry.sh @@ -14,17 +14,12 @@ term_handler() { trap 'kill ${!}; term_handler' SIGTERM pid=0 -conf_file=/var/lib/geoipupdate/GeoIP.conf database_dir=/usr/share/GeoIP log_dir="/var/lib/geoipupdate" log_file="$log_dir/.healthcheck" flags="--output" frequency=$((GEOIPUPDATE_FREQUENCY * 60 * 60)) -if [ -z "$GEOIPUPDATE_CONF_FILE" ]; then - GEOIPUPDATE_CONF_FILE="$conf_file" -fi - if [ -z "$GEOIPUPDATE_DB_DIR" ]; then GEOIPUPDATE_DB_DIR="$database_dir" fi @@ -42,11 +37,6 @@ if [ -z "$GEOIPUPDATE_ACCOUNT_ID" ] || [ -z "$GEOIPUPDATE_LICENSE_KEY" ] || [ - exit 1 fi -# Create an empty configuration file. All configuration is provided via -# environment variables or command line options, but geoipupdate still -# expects a configuration file to exist. -touch "$GEOIPUPDATE_CONF_FILE" - mkdir -p $log_dir while true; do diff --git a/pkg/geoipupdate/config.go b/pkg/geoipupdate/config.go index dc8a1d3e..2ab69cca 100644 --- a/pkg/geoipupdate/config.go +++ b/pkg/geoipupdate/config.go @@ -19,6 +19,9 @@ import ( type Config struct { // AccountID is the account ID. AccountID int + // confFile is the path to any configuration file used when + // potentially populating Config fields. + configFile *string // DatabaseDirectory is where database files are going to be // stored. DatabaseDirectory string @@ -102,12 +105,23 @@ func WithOutput(val bool) Option { } } -// NewConfig create a new configuration and populates it based on a config -// file poinedt to by 'path', then by various environment variables, and then -// finally by flag overrides provided by flagOptions. Values from the later -// override the former. +// WithConfigFile returns an Option that sets the configuration +// file to be used. +func WithConfigFile(file string) Option { + return func(c *Config) error { + if file != "" { + cleanedPath := filepath.Clean(file) + c.configFile = &cleanedPath + } + return nil + } +} + +// NewConfig creates a new configuration and populates it based on an optional +// config file pointed to by an option set with WithConfigFile, then by various +// environment variables, and then finally by flag overrides provided by +// flagOptions. Values from the later override the former. func NewConfig( - path string, flagOptions ...Option, ) (*Config, error) { // config defaults @@ -118,12 +132,21 @@ func NewConfig( Parallelism: 1, } - // Override config with values from the config file. - err := setConfigFromFile(config, path) + // Potentially populate config.configFilePath. We will rerun this function + // again later to ensure the flag values override env variables. + err := setConfigFromFlags(config, flagOptions...) if err != nil { return nil, err } + // Override config with values from the config file. + if confFile := config.configFile; confFile != nil { + err = setConfigFromFile(config, *confFile) + if err != nil { + return nil, err + } + } + // Override config with values from environment variables. err = setConfigFromEnv(config) if err != nil { @@ -161,6 +184,7 @@ func NewConfig( // Reset values that were only needed to communicate information between // config overrides. + config.configFile = nil config.proxyURL = "" config.proxyUserInfo = "" diff --git a/pkg/geoipupdate/config_test.go b/pkg/geoipupdate/config_test.go index dcd9b06a..8f18a754 100644 --- a/pkg/geoipupdate/config_test.go +++ b/pkg/geoipupdate/config_test.go @@ -471,7 +471,8 @@ EditionIDs GeoLite2-City GeoLite2-Country withEnvVars(t, test.Env, func() { tempName := filepath.Join(t.TempDir(), "/GeoIP-test.conf") require.NoError(t, os.WriteFile(tempName, []byte(test.Input), 0o600)) - config, err := NewConfig(tempName, test.Flags...) + testFlags := append([]Option{WithConfigFile(tempName)}, test.Flags...) + config, err := NewConfig(testFlags...) if test.Err == "" { assert.NoError(t, err, test.Description) } else { From 2fd246e1a5a06c2b501d4b0c0e60b92726bd50f0 Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Wed, 28 Jun 2023 09:44:05 -0500 Subject: [PATCH 16/25] Use original truth criteria when setting Verbose The original verbose option behavior enabled it if any value was used, and would not error if a value besides 0 or 1 was used. This returns to the original behavior of allowing any value to be used. --- doc/docker.md | 2 +- pkg/geoipupdate/config.go | 5 +---- pkg/geoipupdate/config_test.go | 7 ------- 3 files changed, 2 insertions(+), 12 deletions(-) diff --git a/doc/docker.md b/doc/docker.md index 312631d7..abdc5b6f 100644 --- a/doc/docker.md +++ b/doc/docker.md @@ -33,7 +33,7 @@ The following are optional: of files downloaded from the server. This option is either `0` or `1`. The default is `0`. * `GEOIPUPDATE_VERBOSE` - Enable verbose mode. Prints out the steps that - `geoipupdate` takes. Set to `1` to enable. + `geoipupdate` takes. Set to **anything** (e.g., `1`) to enable. * `GEOIPUPDATE_CONF_FILE` - The path of a configuration file to be used by `geoipupdate`. The default is `/etc/GeoIP.conf`. diff --git a/pkg/geoipupdate/config.go b/pkg/geoipupdate/config.go index 2ab69cca..4b8718c4 100644 --- a/pkg/geoipupdate/config.go +++ b/pkg/geoipupdate/config.go @@ -346,10 +346,7 @@ func setConfigFromEnv(config *Config) error { } if value, ok := os.LookupEnv("GEOIPUPDATE_VERBOSE"); ok { - if value != "0" && value != "1" { - return errors.New("'Verbose' must be 0 or 1") - } - config.Verbose = value == "1" + config.Verbose = value != "" } return nil diff --git a/pkg/geoipupdate/config_test.go b/pkg/geoipupdate/config_test.go index 8f18a754..d533e825 100644 --- a/pkg/geoipupdate/config_test.go +++ b/pkg/geoipupdate/config_test.go @@ -659,13 +659,6 @@ func TestSetConfigFromEnv(t *testing.T) { }, Err: "parallelism should be greater than 0, got '0'", }, - { - Description: "Invalid Verbose", - Env: map[string]string{ - "GEOIPUPDATE_VERBOSE": "1a", - }, - Err: "'Verbose' must be 0 or 1", - }, } for _, test := range tests { From 39a410383440bb59f50520dda44755ab433dce6b Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Wed, 28 Jun 2023 09:53:14 -0500 Subject: [PATCH 17/25] Fix typos and spacing --- doc/GeoIP.conf.md | 8 ++++---- doc/docker.md | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/doc/GeoIP.conf.md b/doc/GeoIP.conf.md index 2f19ba02..3e013156 100644 --- a/doc/GeoIP.conf.md +++ b/doc/GeoIP.conf.md @@ -86,10 +86,10 @@ sensitive. `Parallelism` -: The maximum number of parallel database downloads. The default is - 1, which means that databases will be downloaded sequentially. This can be - overridden at runtime by the `GEOIPUPDATE_PARALLELISM` environment variable - or the `--parallelism` command line argument. +: The maximum number of parallel database downloads. The default is + 1, which means that databases will be downloaded sequentially. This can be + overridden at run time by the `GEOIPUPDATE_PARALLELISM` environment + variable or the `--parallelism` command line argument. ## Deprecated settings: diff --git a/doc/docker.md b/doc/docker.md index abdc5b6f..74021b0d 100644 --- a/doc/docker.md +++ b/doc/docker.md @@ -36,7 +36,6 @@ The following are optional: `geoipupdate` takes. Set to **anything** (e.g., `1`) to enable. * `GEOIPUPDATE_CONF_FILE` - The path of a configuration file to be used by `geoipupdate`. - The default is `/etc/GeoIP.conf`. * `GEOIPUPDATE_DB_DIR` - The directory where geoipupdate will download the databases. The default is `/usr/share/GeoIP`. From d1abad8ce41a57e80cd6c589cd7f412eb1dd24ed Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Wed, 28 Jun 2023 13:33:37 -0500 Subject: [PATCH 18/25] Add related environment variables in CLI docs --- doc/geoipupdate.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/geoipupdate.md b/doc/geoipupdate.md index 1bab6c1a..1fa5147d 100644 --- a/doc/geoipupdate.md +++ b/doc/geoipupdate.md @@ -21,7 +21,8 @@ open. `-d`, `--database-directory` : Install databases to a custom directory. This is optional. If provided, it - overrides any `DatabaseDirectory` set in the configuration file. + overrides the `DatabaseDirectory` value from the configuration file and the + `GEOIPUPDATE_DB_DIR` environment variable. `-f`, `--config-file` @@ -48,7 +49,8 @@ open. `-v`, `--verbose` -: Enable verbose mode. Prints out the steps that `geoipupdate` takes. +: Enable verbose mode. Prints out the steps that `geoipupdate` takes. If + provided, it overrides any `GEOIPUPDATE_VERBOSE` environment variable. `-o`, `--output` From 6e8aee629f8fc49a448c0e2d2ce2547814d76657 Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Wed, 28 Jun 2023 13:55:51 -0500 Subject: [PATCH 19/25] Mention breaking changes in changelog --- CHANGELOG.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d0ea8a1..4a076f9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # CHANGELOG -## 5.2.0 +## 6.0.0 * `geoipupdate` now supports configuration via environment variables. Any configuration set this way will override any value from the config file, @@ -21,6 +21,11 @@ * `GEOIPUPDATE_RETRY_FOR` * `GEOIPUPDATE_VERBOSE` +* Changed the signature of `NewConfig` in `pkg/geoipupdate` to no longer accept + a positional config file path argument, which can now be passed in using the + option from `WithConfigFile` along with the other optional parameters. +* `geoipupdate` and `NewConfig` no longer require a config file to exist. + ## 5.1.1 (2023-05-08) * Based on feedback, the change to use a non-root user in 5.1.0 From 3b9b147fb36b80432563bc57dcce9ba45e1250be Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Thu, 6 Jul 2023 07:06:18 -0500 Subject: [PATCH 20/25] Use non-pointer --- pkg/geoipupdate/config.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/pkg/geoipupdate/config.go b/pkg/geoipupdate/config.go index 4b8718c4..d010d79e 100644 --- a/pkg/geoipupdate/config.go +++ b/pkg/geoipupdate/config.go @@ -21,7 +21,7 @@ type Config struct { AccountID int // confFile is the path to any configuration file used when // potentially populating Config fields. - configFile *string + configFile string // DatabaseDirectory is where database files are going to be // stored. DatabaseDirectory string @@ -110,8 +110,7 @@ func WithOutput(val bool) Option { func WithConfigFile(file string) Option { return func(c *Config) error { if file != "" { - cleanedPath := filepath.Clean(file) - c.configFile = &cleanedPath + c.configFile = filepath.Clean(file) } return nil } @@ -140,8 +139,8 @@ func NewConfig( } // Override config with values from the config file. - if confFile := config.configFile; confFile != nil { - err = setConfigFromFile(config, *confFile) + if confFile := config.configFile; confFile != "" { + err = setConfigFromFile(config, confFile) if err != nil { return nil, err } @@ -184,7 +183,7 @@ func NewConfig( // Reset values that were only needed to communicate information between // config overrides. - config.configFile = nil + config.configFile = "" config.proxyURL = "" config.proxyUserInfo = "" From 66f47bb1989ba85990234025bf9fd252fba5b96f Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Thu, 6 Jul 2023 07:49:03 -0500 Subject: [PATCH 21/25] Add _ACCOUNT_ID_FILE and _LICENSE_KEY_FILE env vars --- CHANGELOG.md | 2 ++ doc/GeoIP.conf.md | 7 ++-- doc/docker.md | 14 +++++--- docker/entry.sh | 14 ++++---- pkg/geoipupdate/config.go | 25 +++++++++++++++ pkg/geoipupdate/config_test.go | 58 +++++++++++++++++++++++++++++++--- 6 files changed, 103 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a076f9e..46525ba2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,11 +8,13 @@ The following new environment variables are supported: * `GEOIPUPDATE_ACCOUNT_ID` + * `GEOIPUPDATE_ACCOUNT_ID_FILE` * `GEOIPUPDATE_CONF_FILE` * `GEOIPUPDATE_DB_DIR` * `GEOIPUPDATE_EDITION_IDS` * `GEOIPUPDATE_HOST` * `GEOIPUPDATE_LICENSE_KEY` + * `GEOIPUPDATE_LICENSE_KEY_FILE` * `GEOIPUPDATE_LOCK_FILE` * `GEOIPUPDATE_PARALLELISM` * `GEOIPUPDATE_PRESERVE_FILE_TIMES` diff --git a/doc/GeoIP.conf.md b/doc/GeoIP.conf.md index 3e013156..6ba9ca57 100644 --- a/doc/GeoIP.conf.md +++ b/doc/GeoIP.conf.md @@ -18,13 +18,14 @@ sensitive. `AccountID` : Your MaxMind account ID. This was formerly known as `UserId`. This can be - overridden at run time by the `GEOIPUPDATE_ACCOUNT_ID` environment - variable. + overridden at run time by either the `GEOIPUPDATE_ACCOUNT_ID` or the + `GEOIPUPDATE_ACCOUNT_ID_FILE` environment variables. `LicenseKey` : Your case-sensitive MaxMind license key. This can be overridden at run time - by the `GEOIPUPDATE_LICENSE_KEY` environment variable. + by either the `GEOIPUPDATE_LICENSE_KEY` or `GEOIPUPDATE_LICENSE_KEY_FILE` + environment variables. `EditionIDs` diff --git a/doc/docker.md b/doc/docker.md index 74021b0d..ec4bc250 100644 --- a/doc/docker.md +++ b/doc/docker.md @@ -10,16 +10,22 @@ The source code is available on [GitHub](https://github.com/maxmind/geoipupdate) The Docker image is configured by environment variables. The following variables are required: -* `GEOIPUPDATE_ACCOUNT_ID` - Your MaxMind account ID. -* `GEOIPUPDATE_LICENSE_KEY` - Your case-sensitive MaxMind license key. + * `GEOIPUPDATE_EDITION_IDS` - List of space-separated database edition IDs. Edition IDs may consist of letters, digits, and dashes. For example, `GeoIP2-City` would download the GeoIP2 City database (`GeoIP2-City`). +One of: +* `GEOIPUPDATE_ACCOUNT_ID` - Your MaxMind account ID. +* `GEOIPUPDATE_ACCOUNT_IDFILE` - A file containing your MaxMind account ID. + +One of: +* `GEOIPUPDATE_LICENSE_KEY` - Your case-sensitive MaxMind license key. +* `GEOIPUPDATE_LICENSE_KEY_FILE` - A file containing your case-sensitive + MaxMind license key. + The following are optional: -* `GEOIPUPDATE_ACCOUNT_ID_FILE` - The path to a file containing your MaxMind account ID. This is intended to be used with Docker secrets (example below). -* `GEOIPUPDATE_LICENSE_KEY_FILE` - The path to a file containing your case-sensitive MaxMind license key. This is intended to be used with Docker secrets (example below). * `GEOIPUPDATE_FREQUENCY` - The number of hours between `geoipupdate` runs. If this is not set or is set to `0`, `geoipupdate` will run once and exit. * `GEOIPUPDATE_HOST` - The host name of the server to use. The default is diff --git a/docker/entry.sh b/docker/entry.sh index 6eb8b2c4..c2b5ebca 100755 --- a/docker/entry.sh +++ b/docker/entry.sh @@ -24,16 +24,18 @@ if [ -z "$GEOIPUPDATE_DB_DIR" ]; then GEOIPUPDATE_DB_DIR="$database_dir" fi -if [ ! -z "$GEOIPUPDATE_ACCOUNT_ID_FILE" ]; then - GEOIPUPDATE_ACCOUNT_ID=$( cat "$GEOIPUPDATE_ACCOUNT_ID_FILE" ) +if [ -z "$GEOIPUPDATE_ACCOUNT_ID" ] && [ -z "$GEOIPUPDATE_ACCOUNT_ID_FILE" ]; then + echo "ERROR: You must set the environment variable GEOIPUPDATE_ACCOUNT_ID or GEOIPUPDATE_ACCOUNT_ID_FILE!" + exit 1 fi -if [ ! -z "$GEOIPUPDATE_LICENSE_KEY_FILE" ]; then - GEOIPUPDATE_LICENSE_KEY=$( cat "$GEOIPUPDATE_LICENSE_KEY_FILE" ) +if [ -z "$GEOIPUPDATE_LICENSE_KEY" ] && [ -z "$GEOIPUPDATE_LICENSE_KEY_FILE" ]; then + echo "ERROR: You must set the environment variable GEOIPUPDATE_LICENSE_KEY or GEOIPUPDATE_LICENSE_KEY_FILE!" + exit 1 fi -if [ -z "$GEOIPUPDATE_ACCOUNT_ID" ] || [ -z "$GEOIPUPDATE_LICENSE_KEY" ] || [ -z "$GEOIPUPDATE_EDITION_IDS" ]; then - echo "ERROR: You must set the environment variables GEOIPUPDATE_ACCOUNT_ID, GEOIPUPDATE_LICENSE_KEY, and GEOIPUPDATE_EDITION_IDS!" +if [ -z "$GEOIPUPDATE_EDITION_IDS" ]; then + echo "ERROR: You must set the environment variable GEOIPUPDATE_EDITION_IDS!" exit 1 fi diff --git a/pkg/geoipupdate/config.go b/pkg/geoipupdate/config.go index d010d79e..17a2c7d6 100644 --- a/pkg/geoipupdate/config.go +++ b/pkg/geoipupdate/config.go @@ -290,6 +290,20 @@ func setConfigFromEnv(config *Config) error { } } + if value := os.Getenv("GEOIPUPDATE_ACCOUNT_ID_FILE"); value != "" { + var err error + + accountID, err := os.ReadFile(filepath.Clean(value)) + if err != nil { + return fmt.Errorf("failed to open GEOIPUPDATE_ACCOUNT_ID_FILE: %w", err) + } + + config.AccountID, err = strconv.Atoi(string(accountID)) + if err != nil { + return fmt.Errorf("invalid account ID format: %w", err) + } + } + if value, ok := os.LookupEnv("GEOIPUPDATE_DB_DIR"); ok { config.DatabaseDirectory = value } @@ -306,6 +320,17 @@ func setConfigFromEnv(config *Config) error { config.LicenseKey = value } + if value := os.Getenv("GEOIPUPDATE_LICENSE_KEY_FILE"); value != "" { + var err error + + licenseKey, err := os.ReadFile(filepath.Clean(value)) + if err != nil { + return fmt.Errorf("failed to open GEOIPUPDATE_LICENSE_KEY_FILE: %w", err) + } + + config.LicenseKey = string(licenseKey) + } + if value, ok := os.LookupEnv("GEOIPUPDATE_LOCK_FILE"); ok { config.LockFile = value } diff --git a/pkg/geoipupdate/config_test.go b/pkg/geoipupdate/config_test.go index d533e825..41108f92 100644 --- a/pkg/geoipupdate/config_test.go +++ b/pkg/geoipupdate/config_test.go @@ -576,19 +576,23 @@ func TestSetConfigFromFile(t *testing.T) { func TestSetConfigFromEnv(t *testing.T) { tests := []struct { - Description string - Env map[string]string - Expected Config - Err string + Description string + AccountIDFileContents string + LicenseKeyFileContents string + Env map[string]string + Expected Config + Err string }{ { Description: "All config related environment variables", Env: map[string]string{ "GEOIPUPDATE_ACCOUNT_ID": "1", + "GEOIPUPDATE_ACCOUNT_ID_FILE": "", "GEOIPUPDATE_DB_DIR": "/tmp/db", "GEOIPUPDATE_EDITION_IDS": "GeoLite2-Country GeoLite2-City", "GEOIPUPDATE_HOST": "updates.maxmind.com", "GEOIPUPDATE_LICENSE_KEY": "000000000001", + "GEOIPUPDATE_LICENSE_KEY_FILE": "", "GEOIPUPDATE_LOCK_FILE": "/tmp/lock", "GEOIPUPDATE_PARALLELISM": "2", "GEOIPUPDATE_PRESERVE_FILE_TIMES": "1", @@ -612,6 +616,41 @@ func TestSetConfigFromEnv(t *testing.T) { Verbose: true, }, }, + { + Description: "ACCOUNT_ID_FILE and LICENSE_KEY_FILE override", + AccountIDFileContents: "2", + LicenseKeyFileContents: "000000000002", + Env: map[string]string{ + "GEOIPUPDATE_ACCOUNT_ID": "1", + "GEOIPUPDATE_ACCOUNT_ID_FILE": filepath.Join(t.TempDir(), "accountIDFile"), + "GEOIPUPDATE_DB_DIR": "/tmp/db", + "GEOIPUPDATE_EDITION_IDS": "GeoLite2-Country GeoLite2-City", + "GEOIPUPDATE_HOST": "updates.maxmind.com", + "GEOIPUPDATE_LICENSE_KEY": "000000000001", + "GEOIPUPDATE_LICENSE_KEY_FILE": filepath.Join(t.TempDir(), "licenseKeyFile"), + "GEOIPUPDATE_LOCK_FILE": "/tmp/lock", + "GEOIPUPDATE_PARALLELISM": "2", + "GEOIPUPDATE_PRESERVE_FILE_TIMES": "1", + "GEOIPUPDATE_PROXY": "127.0.0.1:8888", + "GEOIPUPDATE_PROXY_USER_PASSWORD": "username:password", + "GEOIPUPDATE_RETRY_FOR": "1m", + "GEOIPUPDATE_VERBOSE": "1", + }, + Expected: Config{ + AccountID: 2, + DatabaseDirectory: "/tmp/db", + EditionIDs: []string{"GeoLite2-Country", "GeoLite2-City"}, + LicenseKey: "000000000002", + LockFile: "/tmp/lock", + Parallelism: 2, + PreserveFileTimes: true, + proxyURL: "127.0.0.1:8888", + proxyUserInfo: "username:password", + RetryFor: 1 * time.Minute, + URL: "https://updates.maxmind.com", + Verbose: true, + }, + }, { Description: "Empty config", Env: map[string]string{}, @@ -663,6 +702,17 @@ func TestSetConfigFromEnv(t *testing.T) { for _, test := range tests { t.Run(test.Description, func(t *testing.T) { + accountIDFile := test.Env["GEOIPUPDATE_ACCOUNT_ID_FILE"] + licenseKeyFile := test.Env["GEOIPUPDATE_LICENSE_KEY_FILE"] + + if test.AccountIDFileContents != "" { + require.NoError(t, os.WriteFile(accountIDFile, []byte(test.AccountIDFileContents), 0o600)) + } + + if test.LicenseKeyFileContents != "" { + require.NoError(t, os.WriteFile(licenseKeyFile, []byte(test.LicenseKeyFileContents), 0o600)) + } + withEnvVars(t, test.Env, func() { var config Config From f0334b4c3f5b18c0e708fb7a137d57b996963702 Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Thu, 6 Jul 2023 07:54:26 -0500 Subject: [PATCH 22/25] Remove leftover reference to non-existant GeoIP.env --- doc/geoipupdate.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/doc/geoipupdate.md b/doc/geoipupdate.md index 1fa5147d..e39b88db 100644 --- a/doc/geoipupdate.md +++ b/doc/geoipupdate.md @@ -76,9 +76,8 @@ runs `geoipupdate` on each Wednesday at noon: To use with a proxy server, update your `GeoIP.conf` file as specified in -the `GeoIP.conf` man page or set the `GEOIPUPDATE_PROXY` environment variable -as specified in the `GeoIP.env` man page. Alternatively, set the `http_proxy` -environment variable. +the `GeoIP.conf` man page. Alternatively, set the `GEOIPUPDATE_PROXY` or +`http_proxy` environment variable. # BUGS From 08dce57dce968ea1338c39b9e8107b7b3718340d Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Fri, 7 Jul 2023 07:36:13 -0500 Subject: [PATCH 23/25] Make configuration error message more generic Loading a configuration can fail in ways that do not involve a config file. This removes part of an error message that implies otherwise. --- cmd/geoipupdate/main.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/geoipupdate/main.go b/cmd/geoipupdate/main.go index f42a02a8..0a9bf0a5 100644 --- a/cmd/geoipupdate/main.go +++ b/cmd/geoipupdate/main.go @@ -3,7 +3,6 @@ package main import ( "context" - "fmt" "log" "os" @@ -48,7 +47,7 @@ func main() { geoipupdate.WithOutput(args.Output), ) if err != nil { - fatalLogger(fmt.Sprintf("error loading configuration %s", args.ConfigFile), err) + fatalLogger("error loading configuration", err) } if config.Verbose { From ad02665fde576026ce74cca5928d3ce3eafd3559 Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Mon, 10 Jul 2023 08:12:28 -0500 Subject: [PATCH 24/25] Avoid leaking account id in error message --- pkg/geoipupdate/config.go | 6 +++--- pkg/geoipupdate/config_test.go | 9 ++++----- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/pkg/geoipupdate/config.go b/pkg/geoipupdate/config.go index 17a2c7d6..1cb18505 100644 --- a/pkg/geoipupdate/config.go +++ b/pkg/geoipupdate/config.go @@ -225,7 +225,7 @@ func setConfigFromFile(config *Config, path string) error { case "AccountID", "UserId": accountID, err := strconv.Atoi(value) if err != nil { - return fmt.Errorf("invalid account ID format: %w", err) + return fmt.Errorf("invalid account ID format") } config.AccountID = accountID keysSeen["AccountID"] = struct{}{} @@ -286,7 +286,7 @@ func setConfigFromEnv(config *Config) error { var err error config.AccountID, err = strconv.Atoi(value) if err != nil { - return fmt.Errorf("invalid account ID format: %w", err) + return fmt.Errorf("invalid account ID format") } } @@ -300,7 +300,7 @@ func setConfigFromEnv(config *Config) error { config.AccountID, err = strconv.Atoi(string(accountID)) if err != nil { - return fmt.Errorf("invalid account ID format: %w", err) + return fmt.Errorf("invalid account ID format") } } diff --git a/pkg/geoipupdate/config_test.go b/pkg/geoipupdate/config_test.go index 41108f92..6cdb182d 100644 --- a/pkg/geoipupdate/config_test.go +++ b/pkg/geoipupdate/config_test.go @@ -231,7 +231,7 @@ UserId 456 Description: "Invalid account ID", Input: `AccountID 1a `, - Err: `invalid account ID format: strconv.Atoi: parsing "1a": invalid syntax`, + Err: `invalid account ID format`, }, { Description: "Invalid PreserveFileTimes", @@ -394,8 +394,7 @@ SkipPeerVerification 1 { Description: "CR line ending does not work", Input: "AccountID 0\rLicenseKey 123\rEditionIDs GeoIP2-City\r", - //nolint: lll - Err: `invalid account ID format: strconv.Atoi: parsing "0 LicenseKey 123 EditionIDs GeoIP2-City": invalid syntax`, + Err: `invalid account ID format`, }, { Description: "Multiple spaces between option and value works", @@ -527,7 +526,7 @@ func TestSetConfigFromFile(t *testing.T) { { Description: "Invalid account ID", Input: "AccountID 1a", - Err: `invalid account ID format: strconv.Atoi: parsing "1a": invalid syntax`, + Err: `invalid account ID format`, }, { Description: "Invalid PreserveFileTimes", @@ -661,7 +660,7 @@ func TestSetConfigFromEnv(t *testing.T) { Env: map[string]string{ "GEOIPUPDATE_ACCOUNT_ID": "1a", }, - Err: `invalid account ID format: strconv.Atoi: parsing "1a": invalid syntax`, + Err: `invalid account ID format`, }, { Description: "Invalid PreserveFileTimes", From 007a68c7261a765f76f4e3b49117332b2925dbb5 Mon Sep 17 00:00:00 2001 From: Nick Logan Date: Mon, 10 Jul 2023 08:15:52 -0500 Subject: [PATCH 25/25] Tidy whitespace, fix typo --- doc/docker.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/docker.md b/doc/docker.md index ec4bc250..3197e271 100644 --- a/doc/docker.md +++ b/doc/docker.md @@ -10,16 +10,17 @@ The source code is available on [GitHub](https://github.com/maxmind/geoipupdate) The Docker image is configured by environment variables. The following variables are required: - * `GEOIPUPDATE_EDITION_IDS` - List of space-separated database edition IDs. Edition IDs may consist of letters, digits, and dashes. For example, `GeoIP2-City` would download the GeoIP2 City database (`GeoIP2-City`). One of: + * `GEOIPUPDATE_ACCOUNT_ID` - Your MaxMind account ID. -* `GEOIPUPDATE_ACCOUNT_IDFILE` - A file containing your MaxMind account ID. +* `GEOIPUPDATE_ACCOUNT_ID_FILE` - A file containing your MaxMind account ID. One of: + * `GEOIPUPDATE_LICENSE_KEY` - Your case-sensitive MaxMind license key. * `GEOIPUPDATE_LICENSE_KEY_FILE` - A file containing your case-sensitive MaxMind license key.