Changing the message type is going to cause all existing clients and agents to fail. As is, they'll fail with an error that most users won't understand about failing to decode JSON.

All CLI users will have to update to a new CLI to tunnel to a workspace after Coderd upgrade, and all workspaces will have to be rebuilt.

This is pretty disruptive. We should talk about what it would take to do this in a backward compatible way and make a decision about whether it's worthwhile.

The other thing to say is that I also think we need to make another change to the protocol here such that we can withdraw nodes, not just add them.#7960

If we're going to change the protocol here, I'd prefer we only change it once, or at least convince ourselves we'll be able to make the later change in a back compatible way.

cc@kylecarbs

This is very easy to ensure that it's backwards compatible so I've made the changes required. This struct now has omitempty and the compare function now considers the derpmap unchanged if the second derpmap is nil

JSON parsing ignores fields in the JSON that aren't in the struct and leaves them as their default value.

The one thing I can't fix is agents never getting a new derpmap, but the derpmap only changes if you're using proxies so we can just tell customers that are using proxies to restart workspaces if they haven't been restarted recently

Even with your change it's not backward compatible because the initial protocol expected an array of nodes[{...}, {...}], and the new protocol sends an object as the outmost element:{"nodes": [{...}, {...}]}

Damn, I forgot it wasn't originally an object...

You're right this kinda does block merge until we figure out what we want to do about it

One idea is to leave this off from the coordinator endpoint entirely, and stream DERPMap updates from some other endpoint.

It's kinda bolted on to the coordinator, which tingles the Spidey senses anyway about this not being in the right place, even back-compat aside.

Adding more websockets or streaming endpoints seems unnecessary if we already have this websocket open, so this does feel like the right place for it IMO, but I guess we'll have to use another way to get updates because this was originally written as an array :/

Do you propose another websocket or something? It made sense for the coordinator to send it since both peers need updates in order to maintain a connection.

Isn't there already an endpoint where they query the DERPMap today?

Maybe we don't need to stream DERPMap updates at all. If I'm understanding correctly, we add new DERP IDs and don't reuse them. So peers only need the new DERP map if they get a Node update with a DERP ID they don't recognize, at which point they could query.