Skip to content

Commit

Permalink
Ignore process environment variables in Flare test (#8270)
Browse files Browse the repository at this point in the history 
The unit test for Flare uses and modifies _actual_ environment variables!
This makes it:

- impossible to parallelize, and
- fail when any environment variable matches the Flare regexen

In particular the second means that a user who has set
`LAKEFS_STATS_ENABLED=0` cannot run unit tests.  So they disable it in their
environment... and the next time they run lakeFS in that shell they send
useless stats.  Or their Linux distribution likes to set `HOSTNAME`.
  • Loading branch information
@arielshaqed
arielshaqed authored Oct 9, 2024
1 parent d28387c commit e133951
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 124 deletions.
11 changes: 10 additions & 1 deletion pkg/flare/flare.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ type WithTime struct {
}

type Flare struct {
envVariables []string
envVarPrefixes []string
envVarBlacklist *regexp.Regexp
// LogDateLayout is the layout used by time.Parse to parse dates in log lines.
Expand Down Expand Up @@ -79,6 +80,7 @@ func NewFlare(options ...Option) (*Flare, error) {
return nil, fmt.Errorf("failed to init secrets scanner: %w", err)
}
flare := &Flare{
envVariables: os.Environ(),
envVarPrefixes: defaultEnvVarPrefixes,
envVarBlacklist: nil,
replacerFunc: defaultSecretReplacer,
Expand Down Expand Up @@ -127,6 +129,13 @@ func WithSecretReplacerFunc(fn RedactedValueReplacer) Option {
}
}

// WithEnv replaces the process environment variables vars.
func WithEnv(vars []string) Option {
return func(f *Flare) {
f.envVariables = vars
}
}

// ProcessConfig takes a config struct, marshals it to YAML and writes it out to outputPath
func (f *Flare) ProcessConfig(cfg interface{}, outputPath, fileName string, getWriterFunc GetFileWriterFunc) (retErr error) {
yamlCfg, err := yaml.Marshal(cfg)
Expand Down Expand Up @@ -178,7 +187,7 @@ func SetBaselinePermissions(mask int) {
}

func (f *Flare) processEnvVars(w io.Writer) error {
for _, e := range os.Environ() {
for _, e := range f.envVariables {
for _, p := range f.envVarPrefixes {
if strings.HasPrefix(e, p) {
var re string
Expand Down
179 changes: 56 additions & 123 deletions pkg/flare/flare_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,88 +11,59 @@ import (
"github.com/stretchr/testify/assert"
)

type EnvVarKV struct {
Key string
Value string
}

func TestEnvVarHandler(t *testing.T) {
testCases := []struct {
Name string
PrefixOverrides []string
PrefixAdditions []string
EnvVars []EnvVarKV
EnvVars []string
Expected string
}{
{
Name: "no env vars",
EnvVars: []EnvVarKV{},
EnvVars: []string{},
Expected: "",
},
{
Name: "single env var with prefix",
EnvVars: []EnvVarKV{
{
Key: "LAKEFS_TEST_ENV_VAR",
Value: "test",
},
EnvVars: []string{
"LAKEFS_TEST_ENV_VAR=test",
},
Expected: `LAKEFS_TEST_ENV_VAR=test
`,
},
{
Name: "multiple env vars with prefix",
EnvVars: []EnvVarKV{
{
Key: "LAKEFS_TEST_ENV_VAR",
Value: "test",
},
{
Key: "LAKEFS_OTHER_TEST_ENV_VAR",
Value: "test2",
},
EnvVars: []string{
"LAKEFS_TEST_ENV_VAR=test",
"LAKEFS_OTHER_TEST_ENV_VAR=test2",
},
Expected: `LAKEFS_TEST_ENV_VAR=test
LAKEFS_OTHER_TEST_ENV_VAR=test2
`,
},
{
Name: "postgres connection string",
EnvVars: []EnvVarKV{
{
Key: "LAKEFS_DATABASE_POSTGRES_CONNECTION_STRING",
Value: "postgresql://lakefs:lakefs@localhost:5432/postgres?sslmode=disable",
},
EnvVars: []string{
"LAKEFS_DATABASE_POSTGRES_CONNECTION_STRING=postgresql://lakefs:lakefs@localhost:5432/postgres?sslmode=disable",
},
Expected: `LAKEFS_DATABASE_POSTGRES_CONNECTION_STRING=<REDACTED>
`,
},
{
Name: "env var with secret",
EnvVars: []EnvVarKV{
{
Key: "LAKEFS_SOME_API_KEY",
Value: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
},
EnvVars: []string{
"LAKEFS_SOME_API_KEY=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
},
Expected: `LAKEFS_SOME_API_KEY=<REDACTED>
`,
},
{
Name: "multiple env vars with secrets",
EnvVars: []EnvVarKV{
{
Key: "LAKEFS_DB_CONNECTION_STRING",
Value: "postgresql://lakefs:password@localhost:5432/lakefe_db",
},
{
Key: "LAKEFS_AWS_SECRET_KEY",
Value: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
},
{
Key: "LAKEFS_AWS_ACCESS_KEY_ID",
Value: "AKIAIOSFODNN7EXAMPLE",
},
EnvVars: []string{
"LAKEFS_DB_CONNECTION_STRING=postgresql://lakefs:password@localhost:5432/lakefe_db",
"LAKEFS_AWS_SECRET_KEY=wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
"LAKEFS_AWS_ACCESS_KEY_ID=AKIAIOSFODNN7EXAMPLE",
},
Expected: `LAKEFS_DB_CONNECTION_STRING=<REDACTED>
LAKEFS_AWS_SECRET_KEY=<REDACTED>
Expand All @@ -101,39 +72,34 @@ LAKEFS_AWS_ACCESS_KEY_ID=<REDACTED>
},
{
Name: "low-entropy value",
EnvVars: []EnvVarKV{
{
Key: "LAKEFS_AUTH_ENCRYPT_SECRET_KEY",
Value: "12e3wadasd",
},
EnvVars: []string{
"LAKEFS_AUTH_ENCRYPT_SECRET_KEY=12e3wadasd",
},
Expected: `LAKEFS_AUTH_ENCRYPT_SECRET_KEY=<REDACTED>
`,
},
{
Name: "high-entropy value",
EnvVars: []EnvVarKV{
{
Key: "LAKEFS_AUTH_ENCRYPT_SECRET_KEY",
Value: "h8vkOauR6Ptt2cvM8WEVsaexZ1IsX55s",
},
EnvVars: []string{
"LAKEFS_AUTH_ENCRYPT_SECRET_KEY=h8vkOauR6Ptt2cvM8WEVsaexZ1IsX55s",
},
Expected: `LAKEFS_AUTH_ENCRYPT_SECRET_KEY=<REDACTED>
`,
},
}

flr, err := NewFlare(WithSecretReplacerFunc(func(value string) string {
return "<REDACTED>"
}))
assert.NoError(t, err)
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
flr, err := NewFlare(
WithSecretReplacerFunc(func(value string) string {
return "<REDACTED>"
}),
WithEnv(tc.EnvVars),
)
assert.NoError(t, err)

b := new(bytes.Buffer)
bw := bufio.NewWriter(b)
for _, kv := range tc.EnvVars {
t.Setenv(kv.Key, kv.Value)
}
flr.processEnvVars(bw)
bw.Flush()
assert.Equal(t, tc.Expected, b.String())
Expand All @@ -145,45 +111,33 @@ func TestEnvVarBlacklist(t *testing.T) {
testCases := []struct {
Name string
Blacklist []string
EnvVars []EnvVarKV
EnvVars []string
Expected string
}{
{
Name: "empty blacklist",
Blacklist: []string{},
EnvVars: []EnvVarKV{
{
Key: "LAKEFS_TEST_ENV_VAR",
Value: "test",
},
EnvVars: []string{
"LAKEFS_TEST_ENV_VAR=test",
},
Expected: `LAKEFS_TEST_ENV_VAR=test
`,
},
{
Name: "single blacklisted",
Blacklist: []string{"LAKEFS_TEST"},
EnvVars: []EnvVarKV{
{
Key: "LAKEFS_TEST",
Value: "test",
},
EnvVars: []string{
"LAKEFS_TEST=test",
},
Expected: `LAKEFS_TEST=<REDACTED>
`,
},
{
Name: "Blacklisted and non-blacklisted",
Blacklist: []string{"LAKEFS_TEST"},
EnvVars: []EnvVarKV{
{
Key: "LAKEFS_TEST",
Value: "test",
},
{
Key: "LAKEFS_TEST_OTHER",
Value: "test2",
},
EnvVars: []string{
"LAKEFS_TEST=test",
"LAKEFS_TEST_OTHER=test2",
},
Expected: `LAKEFS_TEST=<REDACTED>
LAKEFS_TEST_OTHER=test2
Expand All @@ -196,13 +150,14 @@ LAKEFS_TEST_OTHER=test2
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
flr, err := NewFlare(WithEnvVarBlacklist(tc.Blacklist), WithSecretReplacerFunc(replacerFunc))
flr, err := NewFlare(
WithEnvVarBlacklist(tc.Blacklist),
WithSecretReplacerFunc(replacerFunc),
WithEnv(tc.EnvVars),
)
assert.NoError(t, err)
b := new(bytes.Buffer)
bw := bufio.NewWriter(b)
for _, kv := range tc.EnvVars {
t.Setenv(kv.Key, kv.Value)
}
flr.processEnvVars(bw)
bw.Flush()
assert.Equal(t, tc.Expected, b.String())
Expand All @@ -213,64 +168,42 @@ LAKEFS_TEST_OTHER=test2
func TestDefaultReplacerFunc(t *testing.T) {
testCases := []struct {
Name string
EnvVars []EnvVarKV
EnvVars []string
}{
{
Name: "single env var",
EnvVars: []EnvVarKV{
{
Key: "LAKEFS_AUTH_ENCRYPT_SECRET_KEY",
Value: "12e3wadasd",
},
EnvVars: []string{
"LAKEFS_AUTH_ENCRYPT_SECRET_KEY=12e3wadasd",
},
},
{
Name: "multiple env vars",
EnvVars: []EnvVarKV{
{
Key: "LAKEFS_DB_CONNECTION_STRING",
Value: "postgresql://lakefs:password@localhost:5432/lakefe_db",
},
{
Key: "LAKEFS_AWS_SECRET_KEY",
Value: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
},
{
Key: "LAKEFS_AWS_ACCESS_KEY_ID",
Value: "AKIAIOSFODNN7EXAMPLE",
},
EnvVars: []string{
"LAKEFS_DB_CONNECTION_STRING=postgresql://lakefs:password@localhost:5432/lakefe_db",
"LAKEFS_AWS_SECRET_KEY=wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
"LAKEFS_AWS_ACCESS_KEY_ID=AKIAIOSFODNN7EXAMPLE",
},
},
}

flr, err := NewFlare()
assert.NoError(t, err)
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
expected := make([]EnvVarKV, 0, len(tc.EnvVars))
flr, err := NewFlare(WithEnv(tc.EnvVars))
assert.NoError(t, err)

expected := ""
b := new(bytes.Buffer)
bw := bufio.NewWriter(b)
for _, kv := range tc.EnvVars {
t.Setenv(kv.Key, kv.Value)
hasher := sha512.New()
hasher.Write([]byte(kv.Value))
expected = append(expected, EnvVarKV{
Key: kv.Key,
Value: fmt.Sprintf("%x", hasher.Sum(nil)),
})
v := strings.SplitN(kv, "=", 2)
hasher.Write([]byte(v[1]))
expected = expected +
fmt.Sprintf("%s=%x\n", v[0], hasher.Sum(nil))
}
flr.processEnvVars(bw)
bw.Flush()
expectedString := fmt.Sprintf("%s%s", strings.Join(joinKeyValue(expected), "\n"), "\n")
assert.Equal(t, expectedString, b.String())
assert.Equal(t, expected, b.String())
})
}
}

func joinKeyValue(kv []EnvVarKV) []string {
res := make([]string, 0, len(kv))
for _, itm := range kv {
res = append(res, fmt.Sprintf("%s=%s", itm.Key, itm.Value))
}
return res
}

0 comments on commit e133951

Please sign in to comment.