Fix namespace issue #319
Fix namespace issue #319
Conversation
pkg/popeye.go
Outdated
| @@ -256,6 +257,8 @@ func (p *Popeye) lint() (int, int, error) { | |||
| continue | |||
| } | |||
|
|
|||
| ctx = context.WithValue(ctx, internal.KeyNamespace, orig_ns) | |||
There was a problem hiding this comment.
Context is immutable. This call will create a brand new context every time.
I think it's best to only alter the original context if we have a clusterwide resource.
There was a problem hiding this comment.
Thanks - being new to Go in general I hope I am not heading off in the wrong direction but if I test conditionally within the loop to see if the namespace has been reset in ctx, and only if it has, mutate it using a function like I have included here seems to work. Was this what you had in mind?
There was a problem hiding this comment.
@dcarrolloreilly Thanks for the update. Please see comment below
| @@ -256,6 +262,11 @@ func (p *Popeye) lint() (int, int, error) { | |||
| continue | |||
| } | |||
|
|
|||
| // mutate ctx for namespace only if resource is clusterwide | |||
There was a problem hiding this comment.
I think we want to keep the original context here and only create a new context with the new namespace when we detect a cluster wide resource.
FYI context in go is immutable ie when you alter a context in any way you will get a new context.
Does this make sense?
There was a problem hiding this comment.
So I was experimenting with this a bit more and I think there is a indeed a bigger bug :(
The issue here is the context switch timing is wrong. We need to alter/reset the context further down the stack.
I'll take a closer peek...
|
@dcarrolloreilly I took a pass on this and think this issue was resolved on v0.21.5. Please reopen if that's not the case. |
When running popeye from cmd line and passing the -n option to specify a namespace the lint is run for the entire cluster. It appears the namespace is set in context correctly but as we enter the loop to run thru the scrubbers if we encounter a resource that is not namespaced the context is updated to be cluster-wide with a -. This never gets reset back so the next time through the loop we just keep scanning every namespace. This fix grabs the namespace set, stores it in a var and resets it within the loop so that the namespace flag will work correctly limiting the display.