Hi Amaury! Sorry for the HTML message, I have to use a *** well-known enterprise MUA :(
Le 22/03/2024 09:09, « Amaury Denoyelle » <adenoye...@haproxy.com> a écrit : This patch raises several interrogations. First, dynamic servers are currently intended to not support cookies, hence why the keyword is disabled for them. It has been done as a convenience but maybe it would be a good time to review it carefully and see if whole cookie support can be enabled. Indeed, there could be an opportunity to revisit this. What we observed is that, even with dynamic servers, calling “set server bkd1/srv1 addr a.b.c.d” would re-add the dynamic cookie for this server, and calling “enable dynamic-cookie backend bkd1” would re-compute cookie for all servers in the backend. It is expected as in these calls code path there is a call to srv_set_dyncookie(). So there currently is at least a partial support for dynamic cookies on dynamic servers, even if it’s not expected :) Second, I'm unsure srv_set_dyncookie() should be called on _srv_parse_init(). This function is also called for configuration file servers. In particular, I do not know how we should handled duplicate cookie values in this case. Not sure we really create duplicates here as we basically reset the same cookie on the server, not on another one in the backend, at least I didn’t observe such warnings in my logs while testing this patch yet. But I agree that in the context of haproxy startup there would be 2 calls to srv_set_dyncookie() per server which is useless and could create issues. Maybe I could move that at the end of cli_parse_add_server()? Feel free to suggest any better option. Cheers, Damien
