feat: Get all state#15
Conversation
|
Why do we need get all state? |
@monkeyDluffy6017 https://lists.apache.org/thread/2j12c5fxvqpxvhlccsq6gnnpk399kf4d |
| local state_key = key_for(self.TARGET_STATE, target.ip, target.port, target.hostname) | ||
| target.status = INTERNAL_STATES[self.shm:get(state_key)] | ||
| if not target.hostheader then | ||
| target.hostheader = nil |
There was a problem hiding this comment.
Why do we reset the hostheader when the target doesn't have hostheader?
There was a problem hiding this comment.
@spacewander
hostheader may be false, which does not make sense and should be nil or a string value at any time.
It's due to a trivial bug of APISIX, which calculates the argument (host_hdr) to false:
https://github.com/apache/apisix/blob/3b76c45543861a0fb4a7924842d3a47c51ae83b2/apisix/upstream.lua#L129-L131
There was a problem hiding this comment.
It looks so strange here, could we add a validation in add_target?
There was a problem hiding this comment.
@moonming Look here:
apache/apisix#9150
However, the set-nil-if-false guard logic is harmless here. It's even necessary because we need to keep compatibility with old versions of APISIX.
So we could accept PRs independently.
There was a problem hiding this comment.
so we need to fix this bug first
Yep, we should also keep this patch (for those who want to use this feature in other versions of apisix).
5 participants
#14 (comment)