Fix SD mechanism source prefix handling. #1151
Conversation
| ch <- tg | ||
| // Make a copy of the target group in case the discovery mechanism reuses | ||
| // the original object. | ||
| ptg := *tg |
There was a problem hiding this comment.
At this stage the value could already be modified, no?
Wondering whether we should change the interface to only use values to begin with – or forbid the re-use in the interface contract, i.e. remove it from the Consul implementation.
juliusv
force-pushed
the
fix-sd-source-handling
branch
5 times, most recently
from
d0ddf0c to
60b974e
Compare
|
@fabxc Changed the PR to do (minimal) depointerization instead and updated the PR/commit descriptions to reflect and explain that. |
juliusv
force-pushed
the
fix-sd-source-handling
branch
2 times, most recently
from
94b938c to
50b552f
Compare
The prefixed target provider changed a pointerized target group that was reused in the wrapped target provider, causing an ever-increasing chain of source prefixes in target groups from the Consul target provider. We now make this bug generally impossible by switching the target group channel from pointer to value type and thus ensuring that target groups are copied before being passed on to other parts of the system. I tried to not let the depointerization leak too far outside of the channel handling (both upstream and downstream) because I tried that initially and caused some nasty bugs, which I want to minimize. Fixes #1083
juliusv
force-pushed
the
fix-sd-source-handling
branch
from
50b552f to
d88aea7
Compare
|
👍 |
|
@beorn7 and I noticed that this still could cause problems if an SD mechanism mutated the labels map or targets slice included in the copied target group (because no deep copy happens). However, at least in the Consul SD, these fields always get replaced completely rather than being mutated in place. So we should be safe, or at least not more unsafe, for now, with this. |
Fix SD mechanism source prefix handling.
The prefixed target provider changed a pointerized target group that was
reused in the wrapped target provider, causing an ever-increasing chain
of source prefixes in target groups from the Consul target provider.
We now make this bug generally impossible by switching the target group
channel from pointer to value type and thus ensuring that target groups
are copied before being passed on to other parts of the system.
I tried to not let the depointerization leak too far outside of the
channel handling (both upstream and downstream) because I tried that
initially and caused some nasty bugs, which I want to minimize.
Fixes #1083