search

ory / oathkeeper

A cloud native Identity & Access Proxy / API (IAP) and Access Control Decision API that authenticates, authorizes, and mutates incoming HTTP(s) requests. Inspired by the BeyondCorp / Zero Trust white paper. Written in Go.
https://www.ory.sh/?utm_source=github&utm_medium=banner&utm_campaign=hydra
Apache License 2.0
3.23k stars 354 forks source link

I found some data race warnings #574

Open PatrLind opened 3 years ago

PatrLind commented 3 years ago

Describe the bug

While running tests (working on https://github.com/ory/oathkeeper/pull/557) I tried to run the tests with the data race detector on, and it found a few data races. I think some are duplicates.

Reproducing the bug

Steps to reproduce the behavior: go test -timeout 30s -race -count=1 ./...

==================
WARNING: DATA RACE
Read at 0x00c000a3f4c0 by goroutine 31:
  github.com/ory/oathkeeper/driver.(*RegistryMemory).RuleRepository()
      C:/Dev/oathkeeper/driver/registry_memory.go:156 +0x5d5
  github.com/ory/oathkeeper/rule_test.TestFetcherReload()
      C:/Dev/oathkeeper/rule/fetcher_default_test.go:66 +0x625
  testing.tRunner()
      C:/Go/src/testing/testing.go:1127 +0x202

Previous write at 0x00c000a3f4c0 by goroutine 102:
  github.com/ory/oathkeeper/driver.(*RegistryMemory).RuleRepository()
      C:/Dev/oathkeeper/driver/registry_memory.go:157 +0x1b0
  github.com/ory/oathkeeper/rule.(*FetcherDefault).configUpdate()
      C:/Dev/oathkeeper/rule/fetcher_default.go:143 +0x861
  github.com/ory/oathkeeper/rule.(*FetcherDefault).watch()
      C:/Dev/oathkeeper/rule/fetcher_default.go:269 +0xe14
  github.com/ory/oathkeeper/rule.(*FetcherDefault).Watch()
      C:/Dev/oathkeeper/rule/fetcher_default.go:183 +0x10f
  github.com/ory/oathkeeper/rule_test.TestFetcherReload.func2()
      C:/Dev/oathkeeper/rule/fetcher_default_test.go:57 +0xd0

Goroutine 31 (running) created at:
  testing.(*T).Run()
      C:/Go/src/testing/testing.go:1178 +0x796
  testing.runTests.func1()
      C:/Go/src/testing/testing.go:1449 +0xb1
  testing.tRunner()
      C:/Go/src/testing/testing.go:1127 +0x202
  testing.runTests()
      C:/Go/src/testing/testing.go:1447 +0x5b3
  testing.(*M).Run()
      C:/Go/src/testing/testing.go:1357 +0x4f3
  github.com/ory/oathkeeper/rule.TestMain()
      C:/Dev/oathkeeper/rule/repository_test.go:44 +0x4d
  main.main()
      _testmain.go:75 +0x271

Goroutine 102 (running) created at:
  github.com/ory/oathkeeper/rule_test.TestFetcherReload()
      C:/Dev/oathkeeper/rule/fetcher_default_test.go:56 +0x428
  testing.tRunner()
      C:/Go/src/testing/testing.go:1127 +0x202
==================

==================
WARNING: DATA RACE
Read at 0x00c000a3e200 by goroutine 118:
  github.com/ory/oathkeeper/driver.(*RegistryMemory).RuleRepository()
      C:/Dev/oathkeeper/driver/registry_memory.go:156 +0x173
  github.com/ory/oathkeeper/rule_test.TestFetcherWatchConfig.func3()
      C:/Dev/oathkeeper/rule/fetcher_default_test.go:211 +0x1c5
  testing.tRunner()
      C:/Go/src/testing/testing.go:1127 +0x202

Previous write at 0x00c000a3e200 by goroutine 117:
  github.com/ory/oathkeeper/driver.(*RegistryMemory).RuleRepository()
      C:/Dev/oathkeeper/driver/registry_memory.go:157 +0x1b0
  github.com/ory/oathkeeper/rule.(*FetcherDefault).configUpdate()
      C:/Dev/oathkeeper/rule/fetcher_default.go:143 +0x861
  github.com/ory/oathkeeper/rule.(*FetcherDefault).watch()
      C:/Dev/oathkeeper/rule/fetcher_default.go:269 +0xe14
  github.com/ory/oathkeeper/rule.(*FetcherDefault).Watch()
      C:/Dev/oathkeeper/rule/fetcher_default.go:183 +0x10f
  github.com/ory/oathkeeper/rule_test.TestFetcherWatchConfig.func2()
      C:/Dev/oathkeeper/rule/fetcher_default_test.go:156 +0xd0

Goroutine 118 (running) created at:
  testing.(*T).Run()
      C:/Go/src/testing/testing.go:1178 +0x796
  github.com/ory/oathkeeper/rule_test.TestFetcherWatchConfig()
      C:/Dev/oathkeeper/rule/fetcher_default_test.go:207 +0x96e
  testing.tRunner()
      C:/Go/src/testing/testing.go:1127 +0x202

Goroutine 117 (running) created at:
  github.com/ory/oathkeeper/rule_test.TestFetcherWatchConfig()
      C:/Dev/oathkeeper/rule/fetcher_default_test.go:155 +0x428
  testing.tRunner()
      C:/Go/src/testing/testing.go:1127 +0x202
==================

==================
WARNING: DATA RACE
Write at 0x00c00052e3d0 by goroutine 116:
  github.com/ory/viper.toMapStringInterface()
      C:/Users/patrik/go/pkg/mod/github.com/ory/viper@v1.7.5/util.go:240 +0x3b3
  github.com/ory/viper.toMapStringInterface()
      C:/Users/patrik/go/pkg/mod/github.com/ory/viper@v1.7.5/util.go:229 +0x17a
  github.com/ory/viper.toMapStringInterface()
      C:/Users/patrik/go/pkg/mod/github.com/ory/viper@v1.7.5/util.go:229 +0x17a
  github.com/ory/viper.(*Viper).AllSettingsE()
      C:/Users/patrik/go/pkg/mod/github.com/ory/viper@v1.7.5/viper.go:2152 +0x185
  github.com/ory/viper.(*Viper).AllSettings()
      C:/Users/patrik/go/pkg/mod/github.com/ory/viper@v1.7.5/viper.go:2158 +0x64b
  github.com/ory/viper.AllSettings()
      C:/Users/patrik/go/pkg/mod/github.com/ory/viper@v1.7.5/viper.go:2156 +0x62c
time=2020-11-18T15:13:46+08:00 level=debug msg=One or more access rule repositories changed, reloading access rules. audience=application event=repository_change file=ftp://not-valid service_name=ORY Oathkeeper service_version=master source=config_update
  github.com/ory/x/viperx.WatchConfig.func1()
      C:/Users/patrik/go/pkg/mod/github.com/ory/x@v0.0.163/viperx/viper.go:105 +0x659
  github.com/ory/viper.(*Viper).WatchConfig.func1.1()
      C:/Users/patrik/go/pkg/mod/github.com/ory/viper@v1.7.5/viper.go:426 +0x3b3

Previous read at 0x00c00052e3d0 by goroutine 117:
  github.com/spf13/cast.ToStringSliceE()
      C:/Users/patrik/go/pkg/mod/github.com/spf13/cast@v1.3.1/caste.go:1126 +0x138
  github.com/spf13/cast.ToStringSlice()
      C:/Users/patrik/go/pkg/mod/github.com/spf13/cast@v1.3.1/cast.go:157 +0x6f
  github.com/ory/viper.(*Viper).GetStringSlice()
      C:/Users/patrik/go/pkg/mod/github.com/ory/viper@v1.7.5/viper.go:1017 +0x8e
  github.com/ory/viper.GetStringSlice()
      C:/Users/patrik/go/pkg/mod/github.com/ory/viper@v1.7.5/viper.go:1015 +0x8e
  github.com/ory/x/viperx.GetStringSlice()
      C:/Users/patrik/go/pkg/mod/github.com/ory/x@v0.0.163/viperx/helper.go:133 +0x52
  github.com/ory/oathkeeper/driver/configuration.(*ViperProvider).AccessRuleRepositories()
      C:/Dev/oathkeeper/driver/configuration/provider_viper.go:138 +0xa4
  github.com/ory/oathkeeper/rule.(*FetcherDefault).watch()
      C:/Dev/oathkeeper/rule/fetcher_default.go:269 +0xdb1
  github.com/ory/oathkeeper/rule.(*FetcherDefault).Watch()
      C:/Dev/oathkeeper/rule/fetcher_default.go:183 +0x10f
  github.com/ory/oathkeeper/rule_test.TestFetcherWatchConfig.func2()
      C:/Dev/oathkeeper/rule/fetcher_default_test.go:156 +0xd0
time=2020-11-18T15:13:46+08:00 level=debug msg=Fetching access rules from given location because something changed. audience=application location=ftp://not-valid service_name=ORY Oathkeeper service_version=master

Goroutine 116 (running) created at:
  github.com/ory/viper.(*Viper).WatchConfig.func1()
      C:/Users/patrik/go/pkg/mod/github.com/ory/viper@v1.7.5/viper.go:404 +0x407

Goroutine 117 (running) created at:
  github.com/ory/oathkeeper/rule_test.TestFetcherWatchConfig()
      C:/Dev/oathkeeper/rule/fetcher_default_test.go:155 +0x428
  testing.tRunner()
      C:/Go/src/testing/testing.go:1127 +0x202
==================

==================
WARNING: DATA RACE
Read at 0x0000032eab40 by goroutine 117:
  github.com/ory/herodot.(*DefaultError).WithTrace()
      C:/Users/patrik/go/pkg/mod/github.com/ory/herodot@v0.8.4/error_default.go:51 +0x92
  github.com/ory/oathkeeper/pipeline/authn.NewErrAuthenticatorNotEnabled()
      C:/Dev/oathkeeper/pipeline/authn/authenticator.go:31 +0xe6
  github.com/ory/oathkeeper/pipeline/authn.(*AuthenticatorNoOp).Validate()
      C:/Dev/oathkeeper/pipeline/authn/authenticator_noop.go:25 +0x17e
  github.com/ory/oathkeeper/rule.(*ValidatorDefault).validateAuthenticators()
      C:/Dev/oathkeeper/rule/validator.go:66 +0x3f9
  github.com/ory/oathkeeper/rule.(*ValidatorDefault).Validate()
      C:/Dev/oathkeeper/rule/validator.go:138 +0x410
  github.com/ory/oathkeeper/rule.(*RepositoryMemory).Set()
      C:/Dev/oathkeeper/rule/repository_memory.go:110 +0x168
  github.com/ory/oathkeeper/rule.(*FetcherDefault).watch()
      C:/Dev/oathkeeper/rule/fetcher_default.go:295 +0xbc1
  github.com/ory/oathkeeper/rule.(*FetcherDefault).Watch()
      C:/Dev/oathkeeper/rule/fetcher_default.go:183 +0x10f
  github.com/ory/oathkeeper/rule_test.TestFetcherWatchConfig.func2()
      C:/Dev/oathkeeper/rule/fetcher_default_test.go:156 +0xd0

Previous write at 0x0000032eab40 by goroutine 102:
  github.com/ory/herodot.(*DefaultError).Wrap()
      C:/Users/patrik/go/pkg/mod/github.com/ory/herodot@v0.8.4/error_default.go:42 +0x20d
  github.com/ory/herodot.(*DefaultError).WithTrace()
      C:/Users/patrik/go/pkg/mod/github.com/ory/herodot@v0.8.4/error_default.go:54 +0x1ff
  github.com/ory/oathkeeper/pipeline/authn.NewErrAuthenticatorNotEnabled()
      C:/Dev/oathkeeper/pipeline/authn/authenticator.go:31 +0xe6
  github.com/ory/oathkeeper/pipeline/authn.(*AuthenticatorNoOp).Validate()
      C:/Dev/oathkeeper/pipeline/authn/authenticator_noop.go:25 +0x17e
  github.com/ory/oathkeeper/rule.(*ValidatorDefault).validateAuthenticators()
      C:/Dev/oathkeeper/rule/validator.go:66 +0x3f9
  github.com/ory/oathkeeper/rule.(*ValidatorDefault).Validate()
      C:/Dev/oathkeeper/rule/validator.go:138 +0x410
  github.com/ory/oathkeeper/rule.(*RepositoryMemory).Set()
      C:/Dev/oathkeeper/rule/repository_memory.go:110 +0x168
  github.com/ory/oathkeeper/rule.(*FetcherDefault).watch()
      C:/Dev/oathkeeper/rule/fetcher_default.go:295 +0xbc1
  github.com/ory/oathkeeper/rule.(*FetcherDefault).Watch()
      C:/Dev/oathkeeper/rule/fetcher_default.go:183 +0x10f
  github.com/ory/oathkeeper/rule_test.TestFetcherReload.func2()
      C:/Dev/oathkeeper/rule/fetcher_default_test.go:57 +0xd0

Goroutine 117 (running) created at:
  github.com/ory/oathkeeper/rule_test.TestFetcherWatchConfig()
      C:/Dev/oathkeeper/rule/fetcher_default_test.go:155 +0x428
  testing.tRunner()
      C:/Go/src/testing/testing.go:1127 +0x202

Goroutine 102 (running) created at:
  github.com/ory/oathkeeper/rule_test.TestFetcherReload()
      C:/Dev/oathkeeper/rule/fetcher_default_test.go:56 +0x428
  testing.tRunner()
      C:/Go/src/testing/testing.go:1127 +0x202
==================

==================
WARNING: DATA RACE
Read at 0x00c000a3e480 by goroutine 65:
  github.com/ory/oathkeeper/driver.(*RegistryMemory).RuleRepository()
      C:/Dev/oathkeeper/driver/registry_memory.go:156 +0x173
  github.com/ory/oathkeeper/rule_test.TestFetcherWatchRepositoryFromFS.func2()
      C:/Dev/oathkeeper/rule/fetcher_default_test.go:268 +0x1c5
  testing.tRunner()
      C:/Go/src/testing/testing.go:1127 +0x202

Previous write at 0x00c000a3e480 by goroutine 69:
  github.com/ory/oathkeeper/driver.(*RegistryMemory).RuleRepository()
      C:/Dev/oathkeeper/driver/registry_memory.go:157 +0x1b0
  github.com/ory/oathkeeper/rule.(*FetcherDefault).watch()
      C:/Dev/oathkeeper/rule/fetcher_default.go:277 +0x64f
  github.com/ory/oathkeeper/rule.(*FetcherDefault).Watch()
      C:/Dev/oathkeeper/rule/fetcher_default.go:183 +0x10f
  github.com/ory/oathkeeper/rule_test.TestFetcherWatchRepositoryFromFS.func1()
      C:/Dev/oathkeeper/rule/fetcher_default_test.go:252 +0xd0

Goroutine 65 (running) created at:
  testing.(*T).Run()
      C:/Go/src/testing/testing.go:1178 +0x796
  github.com/ory/oathkeeper/rule_test.TestFetcherWatchRepositoryFromFS()
      C:/Dev/oathkeeper/rule/fetcher_default_test.go:264 +0xcb1
  testing.tRunner()
      C:/Go/src/testing/testing.go:1127 +0x202

Goroutine 69 (running) created at:
  github.com/ory/oathkeeper/rule_test.TestFetcherWatchRepositoryFromFS()
      C:/Dev/oathkeeper/rule/fetcher_default_test.go:251 +0x764
  testing.tRunner()
      C:/Go/src/testing/testing.go:1127 +0x202
==================

==================
WARNING: DATA RACE
Read at 0x00c000a3e340 by goroutine 133:
  github.com/ory/oathkeeper/driver.(*RegistryMemory).RuleRepository()
      C:/Dev/oathkeeper/driver/registry_memory.go:156 +0x649
  github.com/ory/oathkeeper/rule_test.TestFetchRulesFromObjectStorage()
      C:/Dev/oathkeeper/rule/fetcher_default_test.go:399 +0x69e
  testing.tRunner()
      C:/Go/src/testing/testing.go:1127 +0x202

Previous write at 0x00c000a3e340 by goroutine 34:
  [failed to restore the stack]

Goroutine 133 (running) created at:
  testing.(*T).Run()
      C:/Go/src/testing/testing.go:1178 +0x796
  testing.runTests.func1()
      C:/Go/src/testing/testing.go:1449 +0xb1
  testing.tRunner()
      C:/Go/src/testing/testing.go:1127 +0x202
  testing.runTests()
      C:/Go/src/testing/testing.go:1447 +0x5b3
  testing.(*M).Run()
      C:/Go/src/testing/testing.go:1357 +0x4f3
  github.com/ory/oathkeeper/rule.TestMain()
      C:/Dev/oathkeeper/rule/repository_test.go:44 +0x4d
  main.main()
      _testmain.go:75 +0x271

Goroutine 34 (running) created at:
  github.com/ory/oathkeeper/rule_test.TestFetchRulesFromObjectStorage()
      C:/Dev/oathkeeper/rule/fetcher_default_test.go:393 +0x61a
  testing.tRunner()
      C:/Go/src/testing/testing.go:1127 +0x202
==================

Environment

aeneasr commented 3 years ago

I am not entirely sure but I think that these races are to be expected because of the way the tests are written, at least that's I think what is going on. However, we should still look into these.