Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(validator): guard racy map access with mutex #211

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gnanderson
Copy link

@gnanderson gnanderson commented Feb 2, 2021

Write access to the SecretContext cache maps is racy (from the parallel validator) - see stack trace below which halted the program:

{"level":"info","ts":1612302789.8019516,"logger":"spinvalidate","msg":"Validator *validate.accountValidator detected a fatal error"}
fatal error: concurrent map writes
goroutine 2395 [running]:
runtime.throw(0x19698e4, 0x15)
/usr/local/go/src/runtime/panic.go:774 +0x72 fp=0xc000bb4eb8 sp=0xc000bb4e88 pc=0x430272
runtime.mapassign_faststr(0x16fc180, 0xc00045a720, 0xc000a7e780, 0x2f, 0x0)
/usr/local/go/src/runtime/map_faststr.go:211 +0x417 fp=0xc000bb4f20 sp=0xc000bb4eb8 pc=0x414dd7
github.com/armory/spinnaker-operator/pkg/secrets.Decode(0x1cc2c80, 0xc0007e7b00, 0xc000a7e780, 0x2f, 0x10, 0xc000183a10, 0x4ce300, 0x0, 0xc00014e380)
/opt/spinnaker-operator/build/pkg/secrets/secrets.go:51 +0x59d fp=0xc000bb4fc8 sp=0xc000bb4f20 pc=0x12a044d
github.com/armory/spinnaker-operator/pkg/halyard.sanitizeSecrets.func1(0xc000a7e780, 0x2f, 0x98, 0xc000a7e780, 0x2f, 0x10)
/opt/spinnaker-operator/build/pkg/halyard/validate.go:102 +0x5f fp=0xc000bb5080 sp=0xc000bb4fc8 pc=0x1546f6f
github.com/armory/spinnaker-operator/pkg/inspect.inspectStringReflect(0x1688e60, 0xc00102fb10, 0x98, 0xc000cd77f8, 0xc00102fb10, 0x98, 0xc000183a50, 0xc000908888, 0xc000cd71e8)
/opt/spinnaker-operator/build/pkg/inspect/parse.go:173 +0xddb fp=0xc000bb5178 sp=0xc000bb5080 pc=0x12a559b
github.com/armory/spinnaker-operator/pkg/inspect.inspectStringReflect(0x16f4b00, 0xc000183a50, 0x94, 0xc000cd77f8, 0xc0001839f0, 0x98, 0x16f4b00, 0xc000183a50, 0x94)
/opt/spinnaker-operator/build/pkg/inspect/parse.go:203 +0x735 fp=0xc000bb5270 sp=0xc000bb5178 pc=0x12a4ef5
github.com/armory/spinnaker-operator/pkg/inspect.inspectStringReflect(0x16fb820, 0xc000c05a70, 0x15, 0xc000bb57f8, 0xc000c05a70, 0x15, 0xc0001839c0, 0xc000908988, 0xc000cd73d8)
/opt/spinnaker-operator/build/pkg/inspect/parse.go:195 +0x5a0 fp=0xc000bb5368 sp=0xc000bb5270 pc=0x12a4d60
github.com/armory/spinnaker-operator/pkg/inspect.inspectStringReflect(0x16f4b00, 0xc0001839c0, 0x94, 0xc000cd77f8, 0xc0001839b0, 0x98, 0x16f4b00, 0xc0001839c0, 0x94)
/opt/spinnaker-operator/build/pkg/inspect/parse.go:203 +0x735 fp=0xc000bb5460 sp=0xc000bb5368 pc=0x12a4ef5
github.com/armory/spinnaker-operator/pkg/inspect.inspectStringReflect(0x16fb820, 0xc000c05a40, 0x15, 0xc000bb57f8, 0xc000c05a40, 0x15, 0xc000183990, 0xc000908758, 0xc000cd75c8)
/opt/spinnaker-operator/build/pkg/inspect/parse.go:195 +0x5a0 fp=0xc000bb5558 sp=0xc000bb5460 pc=0x12a4d60
github.com/armory/spinnaker-operator/pkg/inspect.inspectStringReflect(0x16f4b00, 0xc000183990, 0x94, 0xc000cd77f8, 0xc000183930, 0x98, 0x16f4b00, 0xc000183990, 0x94)
/opt/spinnaker-operator/build/pkg/inspect/parse.go:203 +0x735 fp=0xc000bb5650 sp=0xc000bb5558 pc=0x12a4ef5
github.com/armory/spinnaker-operator/pkg/inspect.inspectStringReflect(0x172c120, 0xc000c05770, 0x15, 0xc000bb57f8, 0x17e55c0, 0x400, 0xc000877580, 0x28, 0x28)
/opt/spinnaker-operator/build/pkg/inspect/parse.go:195 +0x5a0 fp=0xc000bb5748 sp=0xc000bb5650 pc=0x12a4d60
github.com/armory/spinnaker-operator/pkg/inspect.InspectStrings(0x172c120, 0xc000c05770, 0xc000cd77f8, 0xc000877580, 0xc000cd7810, 0x40e3d8, 0x30)
/opt/spinnaker-operator/build/pkg/inspect/parse.go:141 +0xb2 fp=0xc000bb57c0 sp=0xc000bb5748 pc=0x12a46f2
github.com/armory/spinnaker-operator/pkg/halyard.sanitizeSecrets(0x1cc2c80, 0xc0007e7b00, 0x172c120, 0xc000c05770, 0xc000767c50, 0xc000767c50, 0xc000cd78a8, 0x46ce56)
/opt/spinnaker-operator/build/pkg/halyard/validate.go:114 +0x74 fp=0xc000bb5820 sp=0xc000bb57c0 pc=0x1544fd4
github.com/armory/spinnaker-operator/pkg/halyard.(*Service).buildValidationRequest(0xc000894e10, 0x1cc2c80, 0xc0007e7b00, 0x1d01780, 0xc00052b680, 0x3064103a600, 0x28e9640, 0x195d420, 0xc)
/opt/spinnaker-operator/build/pkg/halyard/validate.go:50 +0x12a fp=0xc000bb59e8 sp=0xc000bb5820 pc=0x154443a
github.com/armory/spinnaker-operator/pkg/halyard.(*Service).Validate(0xc000894e10, 0x1cc2c80, 0xc0007e7b00, 0x1d01780, 0xc00052b680, 0x0, 0x1cd1ee0, 0xc00021bfa0, 0x0, 0x0)
/opt/spinnaker-operator/build/pkg/halyard/validate.go:30 +0x80 fp=0xc000bb5a78 sp=0xc000bb59e8 pc=0x15441a0
github.com/armory/spinnaker-operator/pkg/validate.(*halValidator).Validate(0x29070e0, 0x1d01780, 0xc00052b680, 0x1cc2c80, 0xc0007e7b00, 0x1cdcbc0, 0xc000a50ae0, 0xc000a7e2a0, 0x24, 0xc000f42d20, ...)
/opt/spinnaker-operator/build/pkg/validate/hal_validation.go:11 +0xba fp=0xc000bb5b30 sp=0xc000bb5a78 pc=0x1551efa
github.com/armory/spinnaker-operator/pkg/validate.(*ParallelValidator).Validate.func1.1(0x1cc2c80, 0xc0007e7b00)
/opt/spinnaker-operator/build/pkg/validate/parallel.go:28 +0x196 fp=0xc000bb5f60 sp=0xc000bb5b30 pc=0x1554106
k8s.io/apimachinery/pkg/util/wait.(*Group).StartWithContext.func1()
/opt/spinnaker-operator/build/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:62 +0x37 fp=0xc000bb5f80 sp=0xc000bb5f60 pc=0x148f067
k8s.io/apimachinery/pkg/util/wait.(*Group).Start.func1(0xc000fa6870, 0xc000924000)
/opt/spinnaker-operator/build/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:71 +0x59 fp=0xc000bb5fd0 sp=0xc000bb5f80 pc=0x148f0d9
runtime.goexit()

@cristhian-castaneda
Copy link
Contributor

Hey @gnanderson, thanks for your contribution.
How many accounts did you configure or what specific configuration you used to produce this error?
I want to know that to see if we could write some tests for this.

@geojaz
Copy link

geojaz commented Feb 5, 2021

I was able to repro with 600 accounts

@gnanderson
Copy link
Author

gnanderson commented Feb 8, 2021

@cristhian-castaneda I actually caught this with blind luck and only two Kubernetes accounts added as a kustomize patch to the SpinnakerService resource - I just happened to be tailing the operator logs on my second screen when i saw the Go stack trace.

Here's the yaml at the time:

k8s.yaml.txt

@geojaz
Copy link

geojaz commented Feb 8, 2021

That reminds me of a great point- so far I've only run into this issue using k8s accounts listed in the halconfig (like before dynamic accounts).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants