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.

Sign up for GitHub

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

[JENKINS-41730] - Add option to ignore the X-Jenkins-Agent-Protocols header #146

Merged
merged 4 commits into from
Feb 13, 2017
Merged

[JENKINS-41730] - Add option to ignore the X-Jenkins-Agent-Protocols header #146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
09c0d9b
[JENKINS-41730] - Add option to ignore the X-Jenkins-Agent-Protocols …
oleg-nenashev Feb 5, 2017
7d7237f
[JENKINS-41730] - Switch to org.jenkinsci.remoting.engine.JnlpAgentEn…
oleg-nenashev Feb 6, 2017
bc7a1a1
[JENKINS-41730] - Pront the list of supported protocols iff it is not…
oleg-nenashev Feb 13, 2017
5177e40
[JENKINS-41730] - Get rid of unnecessary line breaks
oleg-nenashev Feb 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions docs/configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,18 @@ These properties require independent configuration on both sides of the channel.
<td>org.jenkinsci.remoting.nio.NioChannelHub.disabled</td>
<td>false</td>
<td>2.62.3</td>
<td>?</td>
<td>TODO</td>
<td><a href="https://issues.jenkins-ci.org/browse/JENKINS-39290">JENKINS-39290</a></td>
<td>Boolean flag to disable NIO-based socket connection handling, and switch back to classic IO. Used to isolate the problem.</td>
</tr>-
</tr>
<tr>
<td>org.jenkinsci.remoting.engine.JnlpAgentEndpointResolver.protocolNamesToTry</td>
<td>false</td>
<td>TODO</td>
<td>TODO</td>
<td><a href="https://issues.jenkins-ci.org/browse/JENKINS-41730">JENKINS-41730</a></td>
<td>If specified, only the protocols from the list will be tried during the connection. The option provides protocol names, but the order of the check is defined internally and cannot be changed.</td>
</tr>
<!--Template
<tr>
<td></td>
Expand Down
35 changes: 35 additions & 0 deletions src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,16 @@ public class JnlpAgentEndpointResolver {

private String tunnel;

/**
* If specified, only the protocols from the list will be tried during the connection.
* The option provides protocol names, but the order of the check is defined internally and cannot be changed.
* This option can be also used in order to workaround issues when the headers cannot be delivered
* from the server due to whatever reason (e.g. JENKINS-41730).
* @since TODO
*/
private static String PROTOCOL_NAMES_TO_TRY =
System.getProperty(JnlpAgentEndpointResolver.class.getName() + ".protocolNamesToTry");

public JnlpAgentEndpointResolver(String... jenkinsUrls) {
this.jenkinsUrls = new ArrayList<String>(Arrays.asList(jenkinsUrls));
}
Expand Down Expand Up @@ -164,6 +174,7 @@ public JnlpAgentEndpoint resolve() throws IOException {
host = defaultString(first(header(con, "X-Jenkins-JNLP-Host")), salURL.getHost());
List<String> protocols = header(con, "X-Jenkins-Agent-Protocols");
if (protocols != null) {
// Take the list of protocols to try from the headers
agentProtocolNames = new HashSet<String>();
for (String names : protocols) {
for (String name : names.split(",")) {
Expand All @@ -173,7 +184,31 @@ public JnlpAgentEndpoint resolve() throws IOException {
}
}
}

if (agentProtocolNames.isEmpty()) {
LOGGER.log(Level.WARNING, "Received the empty list of supported protocols from the server. " +
"All protocols are disabled on the master side OR the 'X-Jenkins-Agent-Protocols' header is corrupted (JENKINS-41730). " +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the problem is that the header is set, but empty? Who the hell does that!?

Shouldn't this also apply to the case that the header isn't set, or do we handle that better anyway?

"In the case of the header corruption as a workaround you can use the " +
"'org.jenkinsci.remoting.engine.JnlpAgentEndpointResolver.protocolNamesToTry' system property " +
"to define the supported protocols.");
} else {
LOGGER.log(Level.INFO, "Remoting server accepts the following protocols: {0}", agentProtocolNames);
}
}

if (PROTOCOL_NAMES_TO_TRY != null) {
// Take a list of protocols to try from the system property
agentProtocolNames = new HashSet<String>();
LOGGER.log(Level.INFO, "Ignoring the list of supported remoting protocols provided by the server, because the " +
"'org.jenkinsci.remoting.engine.JnlpAgentEndpointResolver.protocolNamesToTry' property is defined. Will try {0}", PROTOCOL_NAMES_TO_TRY);
for (String name : PROTOCOL_NAMES_TO_TRY.split(",")) {
name = name.trim();
if (!name.isEmpty()) {
agentProtocolNames.add(name);
}
}
}

String idHeader = first(header(con, "X-Instance-Identity"));
RSAPublicKey identity = null;
if (idHeader != null) {
Expand Down