Send Node-Name HTTP header in JnlpAgentEndpointResolver

[jglick]

remoting/src/main/java/org/jenkinsci/remoting/engine/JnlpConnectionState.java

Line 65 in b329c48

public static final String CLIENT_NAME_KEY = "Node-Name";

is passed as an HTTP header in WebSocket mode

remoting/src/main/java/hudson/remoting/Engine.java

Line 555 in b329c48

addedHeaders.put(JnlpConnectionState.CLIENT_NAME_KEY, List.of(agentName));

and as a protocol header in TCP mode

remoting/src/main/java/hudson/remoting/Engine.java

Line 740 in b329c48

headers.put(JnlpConnectionState.CLIENT_NAME_KEY, agentName);

This patch just passes it also as an HTTP header in the handshake phase of TCP mode, when the agent checks https://jenkins/tcpSlaveAgentListener/ to determine a host and port to connect to. Jenkins core will ignore the header; CloudBees CI can use it to adjust the TCP connection information according to the identity of the agent.

[jglick]

These overloads appear unused, so I did not bother retaining compatibility for them.

[basil]

Making it clear that I am blocking this PR until it does not increase the divergence between multiple copies of this code.

[jglick]

This PR does not actually increase the divergence between copies, since it adds an independent line of code after the common portions; if #687 is merged then I would resolve the merge conflict which would make that more apparent.

[basil]

This PR does not actually increase the divergence between copies, since it adds an independent line of code after the common portions

I'm having a hard time following this point, so maybe you could clarify. Are you saying that even in the context of the more comprehensive refactoring done in #677 (which condenses all openURLConnection methods into one), you'd still want to send this header only in the request to /tcpSlaveAgentListener/ and not the request to /computer/${NODE_NAME}/jenkins-agent.jnlp because in the former case the server has no way of getting the agent name without the header but in the latter case the server can get the agent name from the URL?

[jglick]

Are you saying that…

Yes, exactly. EncryptedSlaveAgentJnlpFile.slaveName is already determined by Stapler ancestry. I guess it does no harm to pass a redundant header if the new caller of JnlpAgentEndpointResolver.openURLConnection in Launcher has access to name (or it is an optional parameter).

[basil]

OK, I could also see an argument for always passing the header for greater consistency among the client's requests. But I don't feel strongly either way. Approving this PR.

[jglick]

I do not feel strongly either way either. Will wait for #677 to be merged and resolve conflicts with that.