kubernetes
/
git-sync
mirror of https://github.com/kubernetes/git-sync.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Env var handling should prefer old names 

For back-compat, if we find both an old and new env var for the same
flag, prefer the old.  This matters because the docker image sets
GITSYNC_ROOT but some users still set GIT_SYNC_ROOT.
This commit is contained in:
Tim Hockin 2024-12-12 15:15:13 -08:00
parent cffb28d447
commit ea2acba375
No known key found for this signature in database
4 changed files with 284 additions and 109 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
README.md
View File

@ -587,4 +587,3 @@ HOOKS
if a hook fails and a new hash is synced during the backoff period, the if a hook fails and a new hash is synced during the backoff period, the
retried hook will fire for the newest hash. retried hook will fire for the newest hash.
``` ```

168
env.go
View File

@ -29,17 +29,42 @@ import (
"github.com/spf13/pflag" "github.com/spf13/pflag"
) )
// Tests can set this or set it to nil.
var envWarnfOverride func(format string, args ...any)
func envWarnf(format string, args ...any) {
if envWarnfOverride != nil {
envWarnfOverride(format, args...)
} else {
fmt.Fprintf(os.Stderr, format, args...)
}
}
func envString(def string, key string, alts ...string) string { func envString(def string, key string, alts ...string) string {
if val := os.Getenv(key); val != "" { found := 0
return val result := ""
resultKey := ""
if val, ok := os.LookupEnv(key); ok {
found++
result = val
resultKey = key
} }
for _, alt := range alts { for _, alt := range alts {
if val := os.Getenv(alt); val != "" { if val, ok := os.LookupEnv(alt); ok {
fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key) envWarnf("env $%s has been deprecated, use $%s instead\n", alt, key)
return val found++
result = val
resultKey = alt
} }
} }
return def if found == 0 {
return def
}
if found > 1 {
envWarnf("env $%s was overridden by $%s\n", key, resultKey)
}
return result
} }
func envFlagString(key string, def string, usage string, alts ...string) *string { func envFlagString(key string, def string, usage string, alts ...string) *string {
registerEnvFlag(key, "string", usage) registerEnvFlag(key, "string", usage)
@ -60,16 +85,31 @@ func envStringArray(def string, key string, alts ...string) []string {
return strings.Split(s, ":") return strings.Split(s, ":")
} }
if val := os.Getenv(key); val != "" { found := 0
return parse(val) result := ""
resultKey := ""
if val, ok := os.LookupEnv(key); ok {
found++
result = val
resultKey = key
} }
for _, alt := range alts { for _, alt := range alts {
if val := os.Getenv(alt); val != "" { if val, ok := os.LookupEnv(alt); ok {
fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key) envWarnf("env $%s has been deprecated, use $%s instead\n", alt, key)
return parse(val) found++
result = val
resultKey = key
} }
} }
return parse(def) if found == 0 {
return parse(def)
}
if found > 1 {
envWarnf("env $%s was overridden by $%s\n", key, resultKey)
}
return parse(result)
} }
func envBoolOrError(def bool, key string, alts ...string) (bool, error) { func envBoolOrError(def bool, key string, alts ...string) (bool, error) {
@ -81,16 +121,30 @@ func envBoolOrError(def bool, key string, alts ...string) (bool, error) {
return false, fmt.Errorf("invalid bool env %s=%q: %w", key, val, err) return false, fmt.Errorf("invalid bool env %s=%q: %w", key, val, err)
} }
if val := os.Getenv(key); val != "" { found := 0
return parse(key, val) result := ""
resultKey := ""
if val, ok := os.LookupEnv(key); ok {
found++
result = val
resultKey = key
} }
for _, alt := range alts { for _, alt := range alts {
if val := os.Getenv(alt); val != "" { if val, ok := os.LookupEnv(alt); ok {
fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key) envWarnf("env $%s has been deprecated, use $%s instead\n", alt, key)
return parse(alt, val) found++
result = val
resultKey = key
} }
} }
return def, nil if found == 0 {
return def, nil
}
if found > 1 {
envWarnf("env $%s was overridden by $%s\n", key, resultKey)
}
return parse(resultKey, result)
} }
func envBool(def bool, key string, alts ...string) bool { func envBool(def bool, key string, alts ...string) bool {
val, err := envBoolOrError(def, key, alts...) val, err := envBoolOrError(def, key, alts...)
@ -111,16 +165,30 @@ func envIntOrError(def int, key string, alts ...string) (int, error) {
return 0, fmt.Errorf("invalid int env %s=%q: %w", key, val, err) return 0, fmt.Errorf("invalid int env %s=%q: %w", key, val, err)
} }
if val := os.Getenv(key); val != "" { found := 0
return parse(key, val) result := ""
resultKey := ""
if val, ok := os.LookupEnv(key); ok {
found++
result = val
resultKey = key
} }
for _, alt := range alts { for _, alt := range alts {
if val := os.Getenv(alt); val != "" { if val, ok := os.LookupEnv(alt); ok {
fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key) envWarnf("env $%s has been deprecated, use $%s instead\n", alt, key)
return parse(alt, val) found++
result = val
resultKey = key
} }
} }
return def, nil if found == 0 {
return def, nil
}
if found > 1 {
envWarnf("env $%s was overridden by $%s\n", key, resultKey)
}
return parse(resultKey, result)
} }
func envInt(def int, key string, alts ...string) int { func envInt(def int, key string, alts ...string) int {
val, err := envIntOrError(def, key, alts...) val, err := envIntOrError(def, key, alts...)
@ -141,16 +209,30 @@ func envFloatOrError(def float64, key string, alts ...string) (float64, error) {
return 0, fmt.Errorf("invalid float env %s=%q: %w", key, val, err) return 0, fmt.Errorf("invalid float env %s=%q: %w", key, val, err)
} }
if val := os.Getenv(key); val != "" { found := 0
return parse(key, val) result := ""
resultKey := ""
if val, ok := os.LookupEnv(key); ok {
found++
result = val
resultKey = key
} }
for _, alt := range alts { for _, alt := range alts {
if val := os.Getenv(alt); val != "" { if val, ok := os.LookupEnv(alt); ok {
fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key) envWarnf("env $%s has been deprecated, use $%s instead\n", alt, key)
return parse(alt, val) found++
result = val
resultKey = key
} }
} }
return def, nil if found == 0 {
return def, nil
}
if found > 1 {
envWarnf("env $%s was overridden by $%s\n", key, resultKey)
}
return parse(resultKey, result)
} }
func envFloat(def float64, key string, alts ...string) float64 { func envFloat(def float64, key string, alts ...string) float64 {
val, err := envFloatOrError(def, key, alts...) val, err := envFloatOrError(def, key, alts...)
@ -171,16 +253,30 @@ func envDurationOrError(def time.Duration, key string, alts ...string) (time.Dur
return 0, fmt.Errorf("invalid duration env %s=%q: %w", key, val, err) return 0, fmt.Errorf("invalid duration env %s=%q: %w", key, val, err)
} }
if val := os.Getenv(key); val != "" { found := 0
return parse(key, val) result := ""
resultKey := ""
if val, ok := os.LookupEnv(key); ok {
found++
result = val
resultKey = key
} }
for _, alt := range alts { for _, alt := range alts {
if val := os.Getenv(alt); val != "" { if val, ok := os.LookupEnv(alt); ok {
fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key) envWarnf("env $%s has been deprecated, use $%s instead\n", alt, key)
return parse(alt, val) found++
result = val
resultKey = key
} }
} }
return def, nil if found == 0 {
return def, nil
}
if found > 1 {
envWarnf("env $%s was overridden by $%s\n", key, resultKey)
}
return parse(resultKey, result)
} }
func envDuration(def time.Duration, key string, alts ...string) time.Duration { func envDuration(def time.Duration, key string, alts ...string) time.Duration {
val, err := envDurationOrError(def, key, alts...) val, err := envDurationOrError(def, key, alts...)

219
env_test.go
View File

@ -24,120 +24,187 @@ import (
const ( const (
testKey = "KEY" testKey = "KEY"
alt1Key = "ALT1"
alt2Key = "ALT2"
) )
func setupEnv(val, alt1, alt2 string) {
if val != "" {
os.Setenv(testKey, val)
}
if alt1 != "" {
os.Setenv(alt1Key, alt1)
}
if alt2 != "" {
os.Setenv(alt2Key, alt2)
}
}
func resetEnv() {
os.Unsetenv(testKey)
os.Unsetenv(alt1Key)
os.Unsetenv(alt2Key)
}
func TestEnvBool(t *testing.T) { func TestEnvBool(t *testing.T) {
envWarnfOverride = func(format string, args ...any) {
t.Logf(format, args...)
}
defer func() { envWarnfOverride = nil }()
cases := []struct { cases := []struct {
value string value string
alt1 string
alt2 string
def bool def bool
exp bool exp bool
err bool err bool
}{ }{
{"true", true, true, false}, {"true", "", "", true, true, false},
{"true", false, true, false}, {"true", "", "", false, true, false},
{"", true, true, false}, {"", "", "", true, true, false},
{"", false, false, false}, {"", "", "", false, false, false},
{"false", true, false, false}, {"false", "", "", true, false, false},
{"false", false, false, false}, {"false", "", "", false, false, false},
{"", true, true, false}, {"", "", "", true, true, false},
{"", false, false, false}, {"", "", "", false, false, false},
{"no true", false, false, true}, {"invalid", "", "", false, false, true},
{"no false", false, false, true}, {"invalid", "true", "", false, true, false},
{"true", "invalid", "", false, false, true},
{"invalid", "invalid", "true", false, true, false},
{"true", "true", "invalid", false, false, true},
{"invalid", "invalid", "invalid", false, false, true},
} }
for _, testCase := range cases { for i, tc := range cases {
os.Setenv(testKey, testCase.value) resetEnv()
val, err := envBoolOrError(testCase.def, testKey) setupEnv(tc.value, tc.alt1, tc.alt2)
if err != nil && !testCase.err { val, err := envBoolOrError(tc.def, testKey, alt1Key, alt2Key)
t.Fatalf("%q: unexpected error: %v", testCase.value, err) if err != nil && !tc.err {
t.Fatalf("%d: %q: unexpected error: %v", i, tc.value, err)
} }
if err == nil && testCase.err { if err == nil && tc.err {
t.Fatalf("%q: unexpected success", testCase.value) t.Fatalf("%d: %q: unexpected success", i, tc.value)
} }
if val != testCase.exp { if val != tc.exp {
t.Fatalf("%q: expected %v but %v returned", testCase.value, testCase.exp, val) t.Fatalf("%d: expected: %v, got: %v", i, tc.exp, val)
} }
} }
} }
func TestEnvString(t *testing.T) { func TestEnvString(t *testing.T) {
envWarnfOverride = func(format string, args ...any) {
t.Logf(format, args...)
}
defer func() { envWarnfOverride = nil }()
cases := []struct { cases := []struct {
value string value string
alt1 string
alt2 string
def string def string
exp string exp string
}{ }{
{"true", "true", "true"}, {"foo", "", "", "foo", "foo"},
{"true", "false", "true"}, {"foo", "", "", "bar", "foo"},
{"", "true", "true"}, {"", "", "", "foo", "foo"},
{"", "false", "false"}, {"", "", "", "bar", "bar"},
{"false", "true", "false"}, {"bar", "", "", "foo", "bar"},
{"false", "false", "false"}, {"bar", "", "", "bar", "bar"},
{"", "true", "true"}, {"", "", "", "foo", "foo"},
{"", "false", "false"}, {"", "", "", "bar", "bar"},
{"foo1", "foo2", "", "bar", "foo2"},
{"foo1", "foo2", "foo3", "bar", "foo3"},
} }
for _, testCase := range cases { for i, tc := range cases {
os.Setenv(testKey, testCase.value) resetEnv()
val := envString(testCase.def, testKey) setupEnv(tc.value, tc.alt1, tc.alt2)
if val != testCase.exp { val := envString(tc.def, testKey, alt1Key, alt2Key)
t.Fatalf("%q: expected %v but %v returned", testCase.value, testCase.exp, val) if val != tc.exp {
t.Fatalf("%d: expected: %q, got: %q", i, tc.exp, val)
} }
} }
} }
func TestEnvInt(t *testing.T) { func TestEnvInt(t *testing.T) {
envWarnfOverride = func(format string, args ...any) {
t.Logf(format, args...)
}
defer func() { envWarnfOverride = nil }()
cases := []struct { cases := []struct {
value string value string
alt1 string
alt2 string
def int def int
exp int exp int
err bool err bool
}{ }{
{"0", 1, 0, false}, {"0", "", "", 1, 0, false},
{"", 0, 0, false}, {"", "", "", 0, 0, false},
{"-1", 0, -1, false}, {"-1", "", "", 0, -1, false},
{"abcd", 0, 0, true}, {"invalid", "", "", 0, 0, true},
{"abcd", 0, 0, true}, {"invalid", "0", "", 1, 0, false},
{"0", "invalid", "", 0, 0, true},
{"invalid", "invalid", "0", 1, 0, false},
{"0", "0", "invalid", 0, 0, true},
{"invalid", "invalid", "invalid", 0, 0, true},
} }
for _, testCase := range cases { for i, tc := range cases {
os.Setenv(testKey, testCase.value) resetEnv()
val, err := envIntOrError(testCase.def, testKey) setupEnv(tc.value, tc.alt1, tc.alt2)
if err != nil && !testCase.err { val, err := envIntOrError(tc.def, testKey, alt1Key, alt2Key)
t.Fatalf("%q: unexpected error: %v", testCase.value, err) if err != nil && !tc.err {
t.Fatalf("%d: %q: unexpected error: %v", i, tc.value, err)
} }
if err == nil && testCase.err { if err == nil && tc.err {
t.Fatalf("%q: unexpected success", testCase.value) t.Fatalf("%d: %q: unexpected success", i, tc.value)
} }
if val != testCase.exp { if val != tc.exp {
t.Fatalf("%q: expected %v but %v returned", testCase.value, testCase.exp, val) t.Fatalf("%d: expected: %v, got: %v", i, tc.exp, val)
} }
} }
} }
func TestEnvFloat(t *testing.T) { func TestEnvFloat(t *testing.T) {
envWarnfOverride = func(format string, args ...any) {
t.Logf(format, args...)
}
defer func() { envWarnfOverride = nil }()
cases := []struct { cases := []struct {
value string value string
alt1 string
alt2 string
def float64 def float64
exp float64 exp float64
err bool err bool
}{ }{
{"0.5", 0, 0.5, false}, {"0.5", "", "", 0, 0.5, false},
{"", 0.5, 0.5, false}, {"", "", "", 0.5, 0.5, false},
{"-0.5", 0, -0.5, false}, {"-0.5", "", "", 0, -0.5, false},
{"abcd", 0, 0, true}, {"invalid", "", "", 0, 0, true},
{"invalid", "0.5", "", 0, 0.5, false},
{"0.5", "invalid", "", 0, 0, true},
{"invalid", "invalid", "0.5", 0, 0.5, false},
{"0.5", "0.5", "invalid", 0, 0, true},
{"invalid", "invalid", "invalid", 0, 0, true},
} }
for _, testCase := range cases { for i, tc := range cases {
os.Setenv(testKey, testCase.value) resetEnv()
val, err := envFloatOrError(testCase.def, testKey) setupEnv(tc.value, tc.alt1, tc.alt2)
if err != nil && !testCase.err { val, err := envFloatOrError(tc.def, testKey, alt1Key, alt2Key)
t.Fatalf("%q: unexpected error: %v", testCase.value, err) if err != nil && !tc.err {
t.Fatalf("%d: %q: unexpected error: %v", i, tc.value, err)
} }
if err == nil && testCase.err { if err == nil && tc.err {
t.Fatalf("%q: unexpected success", testCase.value) t.Fatalf("%d: %q: unexpected success", i, tc.value)
} }
if val != testCase.exp { if val != tc.exp {
t.Fatalf("%q: expected %v but %v returned", testCase.value, testCase.exp, val) t.Fatalf("%d: expected: %v, got: %v", i, tc.exp, val)
} }
} }
} }
@ -145,27 +212,35 @@ func TestEnvFloat(t *testing.T) {
func TestEnvDuration(t *testing.T) { func TestEnvDuration(t *testing.T) {
cases := []struct { cases := []struct {
value string value string
alt1 string
alt2 string
def time.Duration def time.Duration
exp time.Duration exp time.Duration
err bool err bool
}{ }{
{"1s", 0, time.Second, false}, {"1s", "", "", 0, time.Second, false},
{"", time.Minute, time.Minute, false}, {"", "", "", time.Minute, time.Minute, false},
{"1h", 0, time.Hour, false}, {"1h", "", "", 0, time.Hour, false},
{"abcd", 0, 0, true}, {"invalid", "", "", 0, 0, true},
{"invalid", "1s", "", 0, time.Second, false},
{"1s", "invalid", "", 0, 0, true},
{"invalid", "invalid", "1s", 0, time.Second, false},
{"1s", "1s", "invalid", 0, 0, true},
{"invalid", "invalid", "invalid", 0, 0, true},
} }
for _, testCase := range cases { for i, tc := range cases {
os.Setenv(testKey, testCase.value) resetEnv()
val, err := envDurationOrError(testCase.def, testKey) setupEnv(tc.value, tc.alt1, tc.alt2)
if err != nil && !testCase.err { val, err := envDurationOrError(tc.def, testKey, alt1Key, alt2Key)
t.Fatalf("%q: unexpected error: %v", testCase.value, err) if err != nil && !tc.err {
t.Fatalf("%d: %q: unexpected error: %v", i, tc.value, err)
} }
if err == nil && testCase.err { if err == nil && tc.err {
t.Fatalf("%q: unexpected success", testCase.value) t.Fatalf("%d: %q: unexpected success", i, tc.value)
} }
if val != testCase.exp { if val != tc.exp {
t.Fatalf("%q: expected %v but %v returned", testCase.value, testCase.exp, val) t.Fatalf("%d: expected: %v, got: %v", i, tc.exp, val)
} }
} }
} }

5
v3-to-v4.md
View File

@ -159,6 +159,11 @@ Most flags can also be configured by environment variables. In v3 the
variables all start with `GIT_SYNC_`. In v4 they all start with `GITSYNC_`, variables all start with `GIT_SYNC_`. In v4 they all start with `GITSYNC_`,
though the old names are still accepted for compatibility. though the old names are still accepted for compatibility.
If both an old (`GIT_SYNC_*`) name and a new (`GITSYNC_*`) name are specified,
the behavior is:
* v4.0.x - v4.3.x: the new name is used
* v4.4.x and up: the old name is used
## Defaults ## Defaults
### Depth ### Depth