Renaming a host in TC, does not impact hash_id

[rimashah25]

What does this PR (Pull Request) do?

• This PR fixes When renaming a host in TC, the hashId retains the value of the previous hostname #2156
• On server create, PR sets xmppId = UUID and removes hostname from the equation.
• On server update, PR prevents xmppId (removing it from update sql query) from ever being changed once set as it is the source of hashId used by traffic router for consistent hashing.

Which Traffic Control components are affected by this PR?

• Documentation
• Traffic Ops

What is the best way to verify this PR?

1. Create a server in traffic_portal. Observe the xmpp_id value in db. It should be set to uuid.
2. Edit the hostname of the created server (from step 1)
3. Observe the change of hostname in DB but xmpp_id remains unaffected.

If this is a bug fix, what versions of Traffic Control are affected?

• master
• 4.1.0

The following criteria are ALL met by this PR

• This PR includes tests
• This PR includes documentation
• This PR includes an update to CHANGELOG.md
• This PR includes any and all required license headers
• This PR ensures that database migration sequence is correct OR this PR does not include a database migration
• This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

Additional Information

A new package, uuid, was added to traffic_ops_golang vendor folder.

[mitchell852]

@rimashah25 - so you probably want to update the api docs that say:

xmppId: An identifier to be used in XMPP communications with the server - in nearly all cases this will be the same as hostName

to something like:

xmppId: A system-generated UUID used to generate a server hash id for use in traffic router's consistent hashing algorithm. This value is set when a server is created and cannot be changed afterwards.

or something like that.

[rimashah25]

@rimashah25 - so you probably want to update the api docs that say:

xmppId: An identifier to be used in XMPP communications with the server - in nearly all cases this will be the same as hostName

to something like:

xmppId: A system-generated UUID used to generate a server hash id for use in traffic router's consistent hashing algorithm. This value is set when a server is created and cannot be changed afterwards.

or something like that.

@mitchell852: Updated the documentation wrt xmppId

[rimashah25]

Using UUID to set xmppid to a random uuid

[srijeet0406]

Noticing an issue in the following case:

1.) Create a server
2.) Change the host name
3.) The xmpp_id doesn't change -> so far it looks great
4.) Do a PUT to change the server's xmpp ID with a json payload that has a different xmpp ID than the one in the database
5.) The Change doesn't take place in the DB (which is the expected behavior), however, we get back a message saying that the server was updated with the new xmpp ID. This is what the response looks like:

{"alerts":[{"text":"Server updated","level":"success"}],"response":{"cachegroup":"edgeserver","cachegroupId":2,"cdnId":1,"cdnName":"ALL","domainName":"rima2","guid":null,"hostName":"rimatest2","httpsPort":443,"id":10,"iloIpAddress":null,"iloIpGateway":null,"iloIpNetmask":null,"iloPassword":null,"iloUsername":null,"lastUpdated":"2020-07-17 14:51:47-06","mgmtIpAddress":null,"mgmtIpGateway":null,"mgmtIpNetmask":null,"offlineReason":null,"physLocation":"phys2","physLocationId":2,"profile":"TRAFFIC_OPS","profileDesc":"Traffic Ops profile","profileId":3,"rack":null,"revalPending":false,"routerHostName":null,"routerPortName":null,"status":"ONLINE","statusId":2,"tcpPort":80,"type":"EDGE","typeId":11,"updPending":false,"xmppId":"5fa51f60-6606-4c9d-8b60-2d78f96018zf","xmppPasswd":null,"interfaces":[{"ipAddresses":[{"address":"192.23.27.67","gateway":"255.255.245.0","serviceAddress":true},{"address":"123.45.67.89","gateway":"255.255.255.0","serviceAddress":false}],"maxBandwidth":5,"monitor":false,"mtu":1500,"name":"rima-interface"}]}}

The xmpp ID before this update was 5fa51f60-6606-4c9d-8b60-2d78f96018ae in the database.
This remains the same even after the PUT (which is what we want), however, notice how the response says that the server was updated with the new xmpp ID of 5fa51f60-6606-4c9d-8b60-2d78f96018zf. We do not want this. The user should be notified that the server couldn't be updated because change of xmpp_id is not allowed, or something to that effect.

[mitchell852]

Noticing an issue in the following case:

1.) Create a server
2.) Change the host name
3.) The xmpp_id doesn't change -> so far it looks great
4.) Do a PUT to change the server's xmpp ID with a json payload that has a different xmpp ID than the one in the database
5.) The Change doesn't take place in the DB (which is the expected behavior), however, we get back a message saying that the server was updated with the new xmpp ID. This is what the response looks like:

{"alerts":[{"text":"Server updated","level":"success"}],"response":{"cachegroup":"edgeserver","cachegroupId":2,"cdnId":1,"cdnName":"ALL","domainName":"rima2","guid":null,"hostName":"rimatest2","httpsPort":443,"id":10,"iloIpAddress":null,"iloIpGateway":null,"iloIpNetmask":null,"iloPassword":null,"iloUsername":null,"lastUpdated":"2020-07-17 14:51:47-06","mgmtIpAddress":null,"mgmtIpGateway":null,"mgmtIpNetmask":null,"offlineReason":null,"physLocation":"phys2","physLocationId":2,"profile":"TRAFFIC_OPS","profileDesc":"Traffic Ops profile","profileId":3,"rack":null,"revalPending":false,"routerHostName":null,"routerPortName":null,"status":"ONLINE","statusId":2,"tcpPort":80,"type":"EDGE","typeId":11,"updPending":false,"xmppId":"5fa51f60-6606-4c9d-8b60-2d78f96018zf","xmppPasswd":null,"interfaces":[{"ipAddresses":[{"address":"192.23.27.67","gateway":"255.255.245.0","serviceAddress":true},{"address":"123.45.67.89","gateway":"255.255.255.0","serviceAddress":false}],"maxBandwidth":5,"monitor":false,"mtu":1500,"name":"rima-interface"}]}}

The xmpp ID before this update was 5fa51f60-6606-4c9d-8b60-2d78f96018ae in the database.
This remains the same even after the PUT (which is what we want), however, notice how the response says that the server was updated with the new xmpp ID of 5fa51f60-6606-4c9d-8b60-2d78f96018zf. We do not want this. The user should be notified that the server couldn't be updated because change of xmpp_id is not allowed, or something to that effect.

I'm wondering if rather than simply ignoring the xmppId in the update query (which I believe is what it is doing) maybe it makes sense to return a 400 bad request if request.xmppId does not equal the xmpp_id found in the database for that server. i.e. if the user is trying to update xmppId, return 400 bad request with an alert of type error with a message like "xmppId cannot be changed".

@ocket8888 - what do you think? xmppId is now immutable. if the user tries to change it on PUT which of the following should happen?

• 400 bad request with error alert along the lines of xmppId is immutable or
• simply ignore the xmppId in the update query and continue w/ the update?

[ocket8888]

To keep with our API pattern, the fields should only be ignored if we're gonna remove it from responses. Changing it would be an invalid action on a field we admit exists, so it should be a 4XX error.

[rimashah25]

Noticing an issue in the following case:

1.) Create a server
2.) Change the host name
3.) The xmpp_id doesn't change -> so far it looks great
4.) Do a PUT to change the server's xmpp ID with a json payload that has a different xmpp ID than the one in the database
5.) The Change doesn't take place in the DB (which is the expected behavior), however, we get back a message saying that the server was updated with the new xmpp ID. This is what the response looks like:

{"alerts":[{"text":"Server updated","level":"success"}],"response":{"cachegroup":"edgeserver","cachegroupId":2,"cdnId":1,"cdnName":"ALL","domainName":"rima2","guid":null,"hostName":"rimatest2","httpsPort":443,"id":10,"iloIpAddress":null,"iloIpGateway":null,"iloIpNetmask":null,"iloPassword":null,"iloUsername":null,"lastUpdated":"2020-07-17 14:51:47-06","mgmtIpAddress":null,"mgmtIpGateway":null,"mgmtIpNetmask":null,"offlineReason":null,"physLocation":"phys2","physLocationId":2,"profile":"TRAFFIC_OPS","profileDesc":"Traffic Ops profile","profileId":3,"rack":null,"revalPending":false,"routerHostName":null,"routerPortName":null,"status":"ONLINE","statusId":2,"tcpPort":80,"type":"EDGE","typeId":11,"updPending":false,"xmppId":"5fa51f60-6606-4c9d-8b60-2d78f96018zf","xmppPasswd":null,"interfaces":[{"ipAddresses":[{"address":"192.23.27.67","gateway":"255.255.245.0","serviceAddress":true},{"address":"123.45.67.89","gateway":"255.255.255.0","serviceAddress":false}],"maxBandwidth":5,"monitor":false,"mtu":1500,"name":"rima-interface"}]}}

The xmpp ID before this update was 5fa51f60-6606-4c9d-8b60-2d78f96018ae in the database.
This remains the same even after the PUT (which is what we want), however, notice how the response says that the server was updated with the new xmpp ID of 5fa51f60-6606-4c9d-8b60-2d78f96018zf. We do not want this. The user should be notified that the server couldn't be updated because change of xmpp_id is not allowed, or something to that effect.

@mitchell852 @srijeet0406: I have updated the PR to reflect the changes based on this comment.

[mitchell852]

{"alerts":[{"text":"Bad Request","level":"error"}]}

@Rima - can you add message to let the user know what was wrong with the request? i.e. xmppId is immutable or something like that?

[rimashah25]

{"alerts":[{"text":"Bad Request","level":"error"}]}

@Rima - can you add message to let the user know what was wrong with the request? i.e. xmppId is immutable or something like that?

@mitchell852 Done.
{"alerts":[{"text":"server cannot be updated due to requested XMPPID change. XMPIDD is immutable","level":"error"}],"response":null}

[srijeet0406]

Had a couple of comments on the code. The PUT issue is fixed though, so good job on that.

[srijeet0406]

Code changes look good.
Unit tests and API tests pass.
Tested by changing the hostname of a server, made sure the xmpp_id doesn't change.
Tested by trying to PUT an update to an existing server, by trying to change the xmpp_id, made sure the correct error message was received, and that the update didnt take place.

[zrhoffman]

In 2294f8f:

traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid
Submodule uuid added at bd4515

The actual files should be added, since the project does not use submodules.

[zrhoffman]

Since xmppId is always no longer the hostname, CDN-in-a-Box won't finish starting up, because for some reason we were using xmppIds as hostnames:

current_servers_json=$(to-get 'api/'$TO_API_VERSION'/servers' 2>/dev/null | jq -c -e '[.response[] | .xmppId] | sort')

Would you please change .xmppId there to .hostName?

[rimashah25]

Since xmppId is always no longer the hostname, CDN-in-a-Box won't finish starting up, because for some reason we were using xmppIds as hostnames:

current_servers_json=$(to-get 'api/'$TO_API_VERSION'/servers' 2>/dev/null | jq -c -e '[.response[] | .xmppId] | sort')

Would you please change .xmppId there to .hostName?

Done

[zrhoffman]

Looks like our OSS license compatibility checker complains about github.com/google/uuid:

https://github.com/apache/trafficcontrol/runs/896246823

Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/.travis.yml
Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/CONTRIBUTORS
Error                            BSD-3-Clause! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/LICENSE
Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/dce.go
Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/doc.go
Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/go.mod
Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/hash.go
Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/json_test.go
Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/marshal.go
Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/node.go
Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/node_js.go
Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/node_net.go
Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/seq_test.go
Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/sql.go
Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/sql_test.go
Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/time.go
Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/util.go
Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/uuid.go
Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/uuid_test.go
Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/version1.go
Error                           BSD-3-Clause~! traffic_ops/traffic_ops_golang/vendor/github.com/google/uuid/version4.go

From @alficles:

The punctuation at the end of the licenses means this:
~ : This license wasn't located in the file itself, but in a nearby LICENSE file and we assumed it applied.
! : This license is fine, but you need to make sure that it's properly documented in the LICENSE file.

So, the uuid dependency needs to be mentioned at the bottom of https://github.com/apache/trafficcontrol/blob/master/LICENSE
and https://github.com/google/uuid/blob/master/LICENSE needs to be added to https://github.com/apache/trafficcontrol/tree/master/licenses .

[zrhoffman]

API and unit tests pass, code is formatted correctly, weasel passes, code is clean

LGTM!

[mitchell852]

lgtm. creating a new server sets xmpp_id = uuid which is ultimately used for the hash id by traffic router. using a unique value like this will prevent any collisions between caches. also, checked that xmppId is immutable as changing it (and thus the hash id) would essentially invalidate a cache's cache which is not a very good thing.