I think you're right, and it is confusing (and you should open a JIRA). I was just pointing out that in general, you'll see the "right" hostname there anyway, because of the way we write it to the filesystem.
Rgds, JvD > On Dec 26, 2016, at 08:49, Oren Shemesh <[email protected]> wrote: > > Hi Jan, > > A small clarification: When you wrote: "is done on the TM host", you meant > that CRConfig is generated on the Traffic Ops host, right ? > > And to the point, I have no problem with the fact that the file is stored > on the Traffic Ops (TO) filesystem for later delivery to the Traffic > Monitor (TM). > The issue is that using the hostname found in the HTTP request from the > management client to the TO, yields a host name which is not necessarily > the host name through which the traffic TM accesses the TO. > And in general, making the "Snapshot CRConfig" functionality depend on the > fact that it is activated from an HTTP request, seems bad to me. > I would expect that the name of the TO (Which is written to " > $data_obj->{'stats'}->{'tm_host'}", note the confusion between TO and TM > here) would come from some configuration file or the DB, not from a value > which happened to be found in the HTTP request. > > What am I missing ? > > On Mon, Dec 26, 2016 at 5:06 PM, Jan van Doorn <[email protected]> wrote: > >> Hi Oren, >> >> I believe the (client) Host header is still correct because it gets put in >> when we write the CRConfig.json to the filesystem >> (./public/CRConfig-Snapshots/<cdn>) using "Tools->SnapShot CRConfig" and >> that is done on the TM host. We always serve the CRConfig.json from the >> filesystem to TM, because it takes to long to generate it on the fly. >> >> Rgds, >> JvD >> >> >> >>> On Dec 26, 2016, at 00:42, Oren Shemesh <[email protected]> wrote: >>> >>> I believe that the code that generates the CrConfig has a problem when >>> creating the "stat" section. >>> It fills values for that section, such as "tm_host", based on the HTTP >>> headers found in the request that triggered the CrConfig snapshot: >>> >>> Here is a snippet from traffic_ops/app/lib/UI/Topology.pm: >>> >>> $data_obj->{'stats'}->{'tm_path'} = $self->req->url->path->{'path' >> }; >>> $data_obj->{'stats'}->{'tm_host'} = $self->req->headers->host; >>> >>> I find this to be a problem for two reasons: >>> >>> >>> 1. This code assumes that it is being run from the context of an HTTP >>> transaction, which to me sounds like contaminating the logic of >> actually >>> creating the CrConfig with details from the upper layer which currently >>> uses this logic. >>> For example, if one would want to run this code from a different path >>> (Maybe in a unit test), it would be a problem. >>> >>> 2. If the traffic ops is accessed through a NAT, then the host name >>> known to the HTTP client issuing the 'Snapshot CrConfig' request is not >>> necessarily the same as the traffic ops host name known to the other >>> components of the system, e.g. the traffic router that uses this >>> information. >>> This is how I found out about this problem - we are experimenting with >>> deploying TC in the cloud (Amazon EC2) and the host name visible to the >>> outside world is not the same as the host names used internally. >>> >>> I believe that tm_host should come from the database (Currently there is >> no >>> entry from the traffic ops itself, but such an entry can be created for >>> this purpose), or from cdn.conf. >>> >>> Shall I report this as an issue in JIRA ? >>> >>> -- >>> >>> *Oren Shemesh* >>> Qwilt | Work: +972-72-2221637 <+972%2072-222-1637>| Mobile: >> +972-50-2281168 >>> <+972%2050-228-1168> | [email protected] <[email protected]> >> >> > > > -- > > *Oren Shemesh* > Qwilt | Work: +972-72-2221637| Mobile: +972-50-2281168 | [email protected] > <[email protected]>
