Added ;sn param check to loose.c is_myself() so that loose_route() would recognize such a Route URI denoting itself. You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/2643
-- Commit Summary --
* Added ;sn param check to loose.c is_myself()
-- File Changes --
M src/modules/rr/loose.c (27)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/2643.patch https://github.com/kamailio/kamailio/pull/2643.diff
The commit message first line has to start with the name of the module ("rr: ...") like per contributing guide:
* https://github.com/kamailio/kamailio/blob/master/.github/CONTRIBUTING.md#com...
It is important for generating changelogs and releases. I made a similar remark to your latest commit as well.
Then, related to the commit itself, `myself` matching is related to routing address (ip/domain) matching of the server, this should be the default behaviour. Otherwise, on this case, it can happen that is matching headers added by another node using same socket names, which can have unexpected side effects in processing .
Personally I do not really see the need for what this patch brings, because you can specify the domains associated with the sever on several ways: config alias parameters, domain module (with register myself) for dynamic or large number of domains and corex module for subdomains.
However, I am fine to add the behaviour of the patch, but controlled by a new rr module parameter (e.g, myself_mode) , default being disabled (e.g., myself_mode=0) so the match is done like so far, on address elements only.
Daniel-Constantin Mierla writes:
The commit message first line has to start with the name of the module ("rr: ...") like per contributing guide:
It is important for generating changelogs and releases. I made a similar remark to your latest commit as well.
OK, will add if this leads to anywhere.
Then, related to the commit itself, `myself` matching is related to routing address (ip/domain) matching of the server, this should be the default behaviour. Otherwise, on this case, it can happen that is matching headers added by another node using same socket names, which can have unexpected side effects in processing .
If such conflicts can happen or not, depends on the proxy deployment environment.
Personally I do not really see the need for what this patch brings, because you can specify the domains associated with the sever on several ways: config alias parameters, domain module (with register myself) for dynamic or large number of domains and corex module for subdomains.
In multi-tenant setup, MS Teams does not know which tenant INVITE from Kamailio tries to reach if first R-R URI does not contain FQDN of the tenant and, like everyone knows, MS does not care sh*t about standards compatibility. So the FQDN needs to be there and also loose_route() needs to work. I don't think that the other options you mention in the above can help here.
However, I am fine to add the behaviour of the patch, but controlled by a new rr module parameter (e.g, myself_mode) , default being disabled (e.g., myself_mode=0) so the match is done like so far, on address elements only.
OK, I'll try to add if the patch otherwise is OK.
-- Juha
What I said that the list of FQDNs for tenants can be specified via alias global params, corex or domain modules to Kamailio, then this patch is not needed, the loose route should do its job. It is irrelevant for what loose route is supposed to do that you need it to interconnect with MS Teams.
And of course conflicts can happen depending on the deployment, but IP addresses and FQDN are global resources owned through a standard process. Socket names are random values that everyone can choose, and many go with suggestions from documentation. The it is likely to see interconnect providers using `s1`, `s2`, ... on their route headers, like it happens for `did` parameter added by dialog module. But to know it is the route header for the instance, the match is done on address, not custom parameters, because the did value from another provider can match another local dialog.
One of the benefits of open source is flexibility and trying to help solving easier various needs, but as a general remark, the default behaviour should be the one closest to the standards or the very common use cases. For specific needs, it has to be a parameter to turn it on. As I said, from my perspective it is ok to add it if controlled by a modparam.
Daniel-Constantin Mierla writes:
What I said that the list of FQDNs for tenants can be specified via alias global params, corex or domain modules to Kamailio, then this patch is not needed, the loose route should do its job. It is irrelevant for what loose route is supposed to do that you need it to interconnect with MS Teams.
I'll give corex alias_subdomains a try and close the pull request if it works as expected.
--- Juha
Can be replaced by corex alias_subdomains.
Closed #2643.
-
Daniel-Constantin Mierla -
juha-h