Skip to content

Commit

Permalink
fix: less flaky rule tests (#973)
Browse files Browse the repository at this point in the history 
Instead of (flaky) fixed sleeps, we now use assert.Eventually
to wait until the rule changes were propagated.

Co-authored-by: hperl <34397+hperl@users.noreply.github.com>
  • Loading branch information
@hperl
hperl and hperl authored Jun 10, 2022
1 parent f714cd3 commit 6ee6a73
Showing 1 changed file with 42 additions and 49 deletions.
91 changes: 42 additions & 49 deletions rule/fetcher_default_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,20 @@ import (
"github.com/ory/oathkeeper/driver/configuration"
"github.com/ory/oathkeeper/internal"
"github.com/ory/oathkeeper/internal/cloudstorage"
"github.com/ory/oathkeeper/rule"
)

const testRule = `[{"id":"test-rule-5","upstream":{"preserve_host":true,"strip_path":"/api","url":"mybackend.com/api"},"match":{"url":"myproxy.com/api","methods":["GET","POST"]},"authenticators":[{"handler":"noop"},{"handler":"anonymous"}],"authorizer":{"handler":"allow"},"mutators":[{"handler":"noop"}]}]`

func TestFetcherReload(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
viper.Reset()
conf := internal.NewConfigurationWithDefaults() // this resets viper and must be at the top
r := internal.NewRegistry(conf)
testConfigPath := "../test/update"

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
_, _ = w.Write([]byte(testRule))
}))
defer ts.Close()
Expand All @@ -54,80 +57,66 @@ func TestFetcherReload(t *testing.T) {
viperx.WatchConfig(l, nil)

go func() {
require.NoError(t, r.RuleFetcher().Watch(context.TODO()))
require.NoError(t, r.RuleFetcher().Watch(ctx))
}()

// initial config without a repo and without a matching strategy
config, err := ioutil.ReadFile(path.Join(testConfigPath, "config_no_repo.yaml"))
require.NoError(t, err)
require.NoError(t, ioutil.WriteFile(configFile, config, 0666))
time.Sleep(time.Millisecond * 500)

rules, err := r.RuleRepository().List(context.Background(), 500, 0)
require.NoError(t, err)
rules := eventuallyListRules(ctx, t, r, 0)
require.Empty(t, rules)

strategy, err := r.RuleRepository().MatchingStrategy(context.Background())
strategy, err := r.RuleRepository().MatchingStrategy(ctx)
require.NoError(t, err)
require.Equal(t, configuration.MatchingStrategy(""), strategy)

// config with a repo and without a matching strategy
config, err = ioutil.ReadFile(path.Join(testConfigPath, "config_default.yaml"))
require.NoError(t, err)
require.NoError(t, ioutil.WriteFile(configFile, config, 0666))
time.Sleep(time.Millisecond * 500)

rules, err = r.RuleRepository().List(context.Background(), 500, 0)
require.NoError(t, err)
require.Equal(t, 1, len(rules))
rules = eventuallyListRules(ctx, t, r, 1)
require.Equal(t, "test-rule-1-glob", rules[0].ID)

strategy, err = r.RuleRepository().MatchingStrategy(context.Background())
strategy, err = r.RuleRepository().MatchingStrategy(ctx)
require.NoError(t, err)
require.Equal(t, configuration.MatchingStrategy(""), strategy)

// config with a glob matching strategy
config, err = ioutil.ReadFile(path.Join(testConfigPath, "config_glob.yaml"))
require.NoError(t, err)
require.NoError(t, ioutil.WriteFile(configFile, config, 0666))
time.Sleep(time.Millisecond * 500)

rules, err = r.RuleRepository().List(context.Background(), 500, 0)
require.NoError(t, err)
require.Equal(t, 1, len(rules))
rules = eventuallyListRules(ctx, t, r, 1)
require.Equal(t, "test-rule-1-glob", rules[0].ID)

strategy, err = r.RuleRepository().MatchingStrategy(context.Background())
strategy, err = r.RuleRepository().MatchingStrategy(ctx)
require.NoError(t, err)
require.Equal(t, configuration.Glob, strategy)

// config with unknown matching strategy
config, err = ioutil.ReadFile(path.Join(testConfigPath, "config_error.yaml"))
require.NoError(t, err)
require.NoError(t, ioutil.WriteFile(configFile, config, 0666))
time.Sleep(time.Millisecond * 500)

rules, err = r.RuleRepository().List(context.Background(), 500, 0)
require.NoError(t, err)
require.Equal(t, 1, len(rules))
rules = eventuallyListRules(ctx, t, r, 1)
require.Equal(t, "test-rule-1-glob", rules[0].ID)

strategy, err = r.RuleRepository().MatchingStrategy(context.Background())
strategy, err = r.RuleRepository().MatchingStrategy(ctx)
require.NoError(t, err)
require.Equal(t, configuration.MatchingStrategy("UNKNOWN"), strategy)

// config with regexp matching strategy
config, err = ioutil.ReadFile(path.Join(testConfigPath, "config_regexp.yaml"))
require.NoError(t, err)
require.NoError(t, ioutil.WriteFile(configFile, config, 0666))
time.Sleep(time.Millisecond * 500)

rules, err = r.RuleRepository().List(context.Background(), 500, 0)
require.NoError(t, err)
require.Equal(t, 1, len(rules))
rules = eventuallyListRules(ctx, t, r, 1)
require.Equal(t, "test-rule-1-glob", rules[0].ID)

strategy, err = r.RuleRepository().MatchingStrategy(context.Background())
strategy, err = r.RuleRepository().MatchingStrategy(ctx)
require.NoError(t, err)
require.Equal(t, configuration.Regexp, strategy)
}
Expand All @@ -136,8 +125,10 @@ func TestFetcherWatchConfig(t *testing.T) {
viper.Reset()
conf := internal.NewConfigurationWithDefaults() // this resets viper and must be at the top
r := internal.NewRegistry(conf)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
_, _ = w.Write([]byte(testRule))
}))
defer ts.Close()
Expand All @@ -153,7 +144,7 @@ func TestFetcherWatchConfig(t *testing.T) {
viperx.WatchConfig(l, nil)

go func() {
require.NoError(t, r.RuleFetcher().Watch(context.TODO()))
require.NoError(t, r.RuleFetcher().Watch(ctx))
}()

for k, tc := range []struct {
Expand Down Expand Up @@ -206,13 +197,9 @@ access_rules:
} {
t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
require.NoError(t, ioutil.WriteFile(configFile, []byte(tc.config), 0666))
time.Sleep(time.Millisecond * 500)

rules, err := r.RuleRepository().List(context.Background(), 500, 0)
require.NoError(t, err)
require.Len(t, rules, len(tc.expectIDs))

strategy, err := r.RuleRepository().MatchingStrategy(context.Background())
rules := eventuallyListRules(ctx, t, r, len(tc.expectIDs))
strategy, err := r.RuleRepository().MatchingStrategy(ctx)
require.NoError(t, err)
require.Equal(t, tc.expectedStrategy, strategy)

Expand All @@ -229,6 +216,8 @@ access_rules:
}

func TestFetcherWatchRepositoryFromFS(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
if runtime.GOOS == "windows" {
t.Skip("Skipping watcher tests on windows")
}
Expand All @@ -253,7 +242,7 @@ access_rules:
viperx.WatchConfig(nil, nil)

go func() {
require.NoError(t, r.RuleFetcher().Watch(context.TODO()))
require.NoError(t, r.RuleFetcher().Watch(ctx))
}()

for k, tc := range []struct {
Expand All @@ -269,15 +258,13 @@ access_rules:
require.NoError(t, ioutil.WriteFile(repository, []byte(tc.content), 0777))
time.Sleep(time.Millisecond * 500)

rules, err := r.RuleRepository().List(context.Background(), 500, 0)
require.NoError(t, err)
rules := eventuallyListRules(ctx, t, r, len(tc.expectIDs))

ids := make([]string, len(rules))
for k, r := range rules {
ids[k] = r.ID
}

assert.Len(t, ids, len(tc.expectIDs), "%+v", rules)
for _, id := range tc.expectIDs {
assert.True(t, stringslice.Has(ids, id), "\nexpected: %v\nactual: %v", tc.expectIDs, ids)
}
Expand All @@ -286,6 +273,8 @@ access_rules:
}

func TestFetcherWatchRepositoryFromKubernetesConfigMap(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
if runtime.GOOS == "windows" {
t.Skip()
}
Expand Down Expand Up @@ -348,17 +337,14 @@ func TestFetcherWatchRepositoryFromKubernetesConfigMap(t *testing.T) {
var cleanup func()

go func() {
require.NoError(t, r.RuleFetcher().Watch(context.TODO()))
require.NoError(t, r.RuleFetcher().Watch(ctx))
}()

for i := 0; i < 10; i++ {
t.Run(fmt.Sprintf("case=%d", i), func(t *testing.T) {
cleanup = configMapUpdate(t, fmt.Sprintf(`[{"id":"%d"}]`, i), cleanup)

time.Sleep(time.Millisecond * 100) // give it a bit of time to reload everything

rules, err := r.RuleRepository().List(context.Background(), 500, 0)
require.NoError(t, err)
rules := eventuallyListRules(ctx, t, r, 1)

require.Len(t, rules, 1)
require.Equal(t, fmt.Sprintf("%d", i), rules[0].ID)
Expand All @@ -367,6 +353,8 @@ func TestFetcherWatchRepositoryFromKubernetesConfigMap(t *testing.T) {
}

func TestFetchRulesFromObjectStorage(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
t.Cleanup(func() {
cloudstorage.SetCurrentTest(nil)
})
Expand Down Expand Up @@ -395,13 +383,18 @@ access_rules:
viperx.WatchConfig(nil, nil)

go func() {
require.NoError(t, r.RuleFetcher().Watch(context.TODO()))
require.NoError(t, r.RuleFetcher().Watch(ctx))
}()

time.Sleep(time.Second * 2) // give it a bit of time to reload everything

rules, err := r.RuleRepository().List(context.Background(), 500, 0)
require.NoError(t, err)
eventuallyListRules(ctx, t, r, 9)
}

assert.Equal(t, 9, len(rules))
func eventuallyListRules(ctx context.Context, t *testing.T, r rule.Registry, expectedLen int) (rules []rule.Rule) {
var err error
assert.Eventually(t, func() bool {
rules, err = r.RuleRepository().List(ctx, 500, 0)
require.NoError(t, err)
return len(rules) == expectedLen
}, 5*time.Second, 10*time.Millisecond)
return
}

0 comments on commit 6ee6a73

Please sign in to comment.