Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PUT /api/3.x/servers with no xmppId (hashId) results in an internal server error #5724

Closed
mitchell852 opened this issue Apr 8, 2021 · 1 comment · Fixed by #5725
Closed
Assignees
Labels
bug something isn't working as intended Traffic Ops related to Traffic Ops

Comments

@mitchell852
Copy link
Member

mitchell852 commented Apr 8, 2021

I'm submitting a ...

  • bug report

Traffic Control components affected ...

  • Traffic Ops

Current behavior:

PUT /api/3.x/servers with no xmppId (hashId) results in an internal server error

ERROR: api.go:209: 2021-04-08T18:39:19.577714586Z: [ip] original server had no XMPPID

Expected behavior:

No ISE. You can't change xmppId after server create so i'm not even sure why the api is checking for one.

Minimal reproduction of the problem with instructions:

Create a server, in the db, delete the xmppId.

PUT /api/3.x/servers/your-new-server-id and you should get a 500 Internal Server Error.

Anything else:

Workaround: add an xmppId to the db for that server and the error goes away

Also, i see lots of instances of xmppId defined like this in the documentation which is not accurate:

xmppId
An identifier to be used in XMPP communications with the server - in nearly all cases this will be the same as hostName
@mitchell852 mitchell852 added bug something isn't working as intended Traffic Ops related to Traffic Ops labels Apr 8, 2021
@mitchell852 mitchell852 changed the title PUT 3.x/servers with no xmppId (hashId) results in an internal server error PUT /api/3.x/servers with no xmppId (hashId) results in an internal server error Apr 8, 2021
@rawlinp
Copy link
Contributor

rawlinp commented Apr 8, 2021

Yeah, I think the intent was to return an error to the user if they try to change a field that was made immutable. But in that case, the server update code should assume that a nil original XMPPID means that the original XMPPID is the server's host_name for comparison purposes.

Also, we should consider adding a DB migration to set nil/empty xmpp_id to host_name. That is what the CRConfig generation code basically does already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something isn't working as intended Traffic Ops related to Traffic Ops
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants