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

Jump to bottom

[JENKINS-68339] Skip remoting stack on built-in node and improve IPv6 Regex #366

Merged
merged 5 commits into from May 11, 2022
Merged

[JENKINS-68339] Skip remoting stack on built-in node and improve IPv6 Regex #366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9f73e9c
[JENKINS-68339] Skip remoting stack and timeout if running on built-i…
Dohbedoh May 9, 2022
edad3e8
[JENKINS-68339][ci] Remove JDK 8 CI and add JDK 17 CI
Dohbedoh May 9, 2022
e0290c8
[JENKINS-68339] Improve comment
Dohbedoh May 9, 2022
d60afd9
[JENKINS-68339] Add complete tests for IPv6 filter
Dohbedoh May 10, 2022
a7b4ea1
[JENKINS-68339] Improve Regex for IPv6 addresses detection
Dohbedoh May 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions Jenkinsfile
View file
Open in desktop
@@ -1,5 +1,5 @@
buildPlugin(configurations: [
[ platform: "linux", jdk: "8", jenkins: null],
[ platform: "linux", jdk: "11", jenkins: null],
[ platform: "windows", jdk: "8", jenkins: null],
buildPlugin(useContainerAgent: true, configurations: [
[ platform: "linux", jdk: "11"],
[ platform: "linux", jdk: "17", jenkins: '2.342'],
[ platform: "windows", jdk: "11"]
])
45 changes: 23 additions & 22 deletions src/main/java/com/cloudbees/jenkins/support/AsyncResultCache.java
View file
Open in desktop
Expand Up @@ -4,13 +4,13 @@
import hudson.model.Computer;
import hudson.model.Node;
import hudson.remoting.Callable;
import hudson.remoting.Future;
import hudson.remoting.VirtualChannel;
import jenkins.model.Jenkins;

import java.io.IOException;
import java.util.WeakHashMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.logging.Level;
Expand All @@ -37,29 +37,34 @@ public static <V, T extends java.lang.Throwable> V get(Node node, WeakHashMap<No
public static <V, T extends java.lang.Throwable> V get(Node node, WeakHashMap<Node, V> cache, /*MasterToSlave*/Callable<V,T> operation, String name)

throws IOException {

if (node == null) return null;
VirtualChannel channel = node.getChannel();
if (channel == null) {
synchronized (cache) {
return cache.get(node);
Future<V> future;
// If launching execution on the built-in node, no need to use the CallAsyncWrapper
if (node instanceof Jenkins) {
Copy link
Member

Choose a reason for hiding this comment

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

Might be safer to call JenkinsJVM.isJenkinsJVM() if this might run on an agent? Not sure if node instanceof Jenkins will trigger classloading of the main Jenkins class on agents (which would be undesirable).

Copy link
Contributor Author

@Dohbedoh Dohbedoh May 9, 2022 •
edited

Choose a reason for hiding this comment

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

The AsyncResultCache actually always executes on the controller JVM. It accepts the node and the callable and then kind of delegate the asynchronous execution and just retrieve the result. So here we check if the node in which we are going to execute the callable is the controller or not.
Maybe I can rephrase my comment.

future = Computer.threadPoolForRemoting.submit(() -> {
try {
return operation.call();
} catch (Throwable e) {
throw new IOException(e);
}
});
} else {
VirtualChannel channel = node.getChannel();
if (channel == null) {
synchronized (cache) {
return cache.get(node);
}
}
future = CallAsyncWrapper.callAsync(channel, operation);
}
Future<V> future = CallAsyncWrapper.callAsync(channel, operation);
try {
final V result = future.get(SupportPlugin.REMOTE_OPERATION_TIMEOUT_MS, TimeUnit.MILLISECONDS);
synchronized (cache) {
cache.put(node, result);
}
return result;
} catch (InterruptedException e) {
final LogRecord lr = new LogRecord(Level.FINE, "Could not retrieve {0} from {1}");
lr.setParameters(new Object[]{name, getNodeName(node)});
lr.setThrown(e);
LOGGER.log(lr);
synchronized (cache) {
return cache.get(node);
}
} catch (ExecutionException e) {
} catch (InterruptedException | ExecutionException e) {
final LogRecord lr = new LogRecord(Level.FINE, "Could not retrieve {0} from {1}");
lr.setParameters(new Object[]{name, getNodeName(node)});
lr.setThrown(e);
Expand All @@ -72,7 +77,7 @@ public static <V, T extends java.lang.Throwable> V get(Node node, WeakHashMap<No
lr.setParameters(new Object[]{name, getNodeName(node)});
lr.setThrown(e);
LOGGER.log(lr);
Computer.threadPoolForRemoting.submit(new AsyncResultCache<V>(node, cache, future, name));
Computer.threadPoolForRemoting.submit(new AsyncResultCache<>(node, cache, future, name));
synchronized (cache) {
return cache.get(node);
}
Expand All @@ -90,19 +95,15 @@ public AsyncResultCache(Node node, WeakHashMap<Node, T> cache, Future<T> future,
this.name = name;
}

@Override
public void run() {
T result;
try {
result = future.get(SupportPlugin.REMOTE_OPERATION_CACHE_TIMEOUT_SEC, TimeUnit.SECONDS);
synchronized (cache) {
cache.put(node, result);
}
} catch (InterruptedException e1) {
final LogRecord lr = new LogRecord(Level.FINE, "Could not retrieve {0} from {1} for caching");
lr.setParameters(new Object[]{name, getNodeName(node)});
lr.setThrown(e1);
LOGGER.log(lr);
} catch (ExecutionException e1) {
} catch (InterruptedException | ExecutionException e1) {
final LogRecord lr = new LogRecord(Level.FINE, "Could not retrieve {0} from {1} for caching");
lr.setParameters(new Object[]{name, getNodeName(node)});
lr.setThrown(e1);
Expand Down
14 changes: 7 additions & 7 deletions src/main/java/com/cloudbees/jenkins/support/filter/InetAddressContentFilter.java
View file
Open in desktop
Expand Up @@ -53,13 +53,13 @@ public static InetAddressContentFilter get() {
return ExtensionList.lookupSingleton(InetAddressContentFilter.class);
}

// https://blogs.msdn.microsoft.com/oldnewthing/20060522-08/?p=31113
private static final String IPv4 =
"([1-9]?\\d|1\\d\\d|2[0-4]\\d|25[0-5])\\.([1-9]?\\d|1\\d\\d|2[0-4]\\d|25[0-5])\\.([1-9]?\\d|1\\d\\d|2[0-4]\\d|25[0-5])\\.([1-9]?\\d|1\\d\\d|2[0-4]\\d|25[0-5])";
// https://stackoverflow.com/a/17871737
private static final String IPv6 =
"(([0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]+|::(ffff(:0{1,4})?:)?((25[0-5]|(2[0-4]|1?[0-9])?[0-9])\\.){3}(25[0-5]|(2[0-4]|1?[0-9])?[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1?[0-9])?[0-9])\\.){3}(25[0-5]|(2[0-4]|1?[0-9])?[0-9]))";
private static final Pattern IP_ADDRESS = Pattern.compile("\\b(" + IPv4 + '|' + IPv6 + ")\\b");
// http://www.java2s.com/example/java/java.util.regex/is-ipv4-address-by-regex.html
private static final String IPv4 = "(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)(\\.(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)){3}";
// Following is based http://www.java2s.com/example/java/java.util.regex/is-ipv6-address-by-regex.html to which
// we add the mix notation (last 2 octet is IPv4)
private static final String IPv6_STANDARD_AND_MIX = "(?i)(?:[0-9a-f]{1,4}:){6}(?:([0-9a-f]{1,4}):[0-9a-f]{1,4}|" + IPv4 + ")";
private static final String IPv6_COMPRESSED_AND_MIX = "(?i)((?:[0-9a-f]{1,4}(?::[0-9a-f]{1,4})*)?)::(((?:[0-9a-f]{1,4}:){1,5})?(" + IPv4 + ")|((?:[0-9a-f]{1,4}(?::[0-9a-f]{1,4})*)?))";
private static final Pattern IP_ADDRESS = Pattern.compile("(?<![:.\\w])(" + IPv4 + '|' + IPv6_STANDARD_AND_MIX + '|' + IPv6_COMPRESSED_AND_MIX + ")(?![:.\\w])");

@Override
public @NonNull String filter(@NonNull String input) {
Expand Down
126 changes: 118 additions & 8 deletions src/test/java/com/cloudbees/jenkins/support/filter/InetAddressContentFilterTest.java
View file
Open in desktop
Expand Up @@ -25,7 +25,11 @@

import hudson.BulkChange;
import jenkins.model.Jenkins;
import org.junit.*;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.quicktheories.core.Gen;
Expand Down Expand Up @@ -66,9 +70,9 @@ public void shouldFilterInetAddresses() {
ContentMappings mappings = ContentMappings.get();
try (BulkChange ignored = new BulkChange(mappings)) {
qt().forAll(inetAddress()).checkAssert(address ->
assertThat(ContentFilter.filter(filter, address))
.contains("ip_")
.doesNotContain(address)
assertThat(ContentFilter.filter(filter, address))
.contains("ip_")
.doesNotContain(address)
);
}
}
Expand All @@ -80,7 +84,7 @@ public void shouldNotFilterInetAddressMatchingJenkinsVersion() {
resetMappings();
InetAddressContentFilter filter = InetAddressContentFilter.get();
assertThat(ContentFilter.filter(filter, Jenkins.VERSION))
.isEqualTo(Jenkins.VERSION);
.isEqualTo(Jenkins.VERSION);
}

private Gen<String> inetAddress() {
Expand All @@ -93,10 +97,116 @@ private Gen<String> ipv4() {
}

private Gen<String> ipv6() {
return ipv6Standard().mix(ipv6Compressed()).mix(ipv6Mixed()).mix(ipv6MixedCompressed());
}

private Gen<String> ipv6Standard() {
Gen<String> part = integers().between(0, 65535).map(Integer::toHexString);
return IntStream.range(0, 8)
.mapToObj(i -> part)
.reduce((left, right) -> left.zip(right, (s1, s2) -> s1 + ':' + s2))
.orElseThrow(IllegalStateException::new);
.mapToObj(i -> part)
.reduce((left, right) -> left.zip(right, (s1, s2) -> s1 + ':' + s2))
.orElseThrow(IllegalStateException::new);
}

private Gen<String> ipv6Compressed() {
Gen<String> part = integers().between(0, 65535).map(Integer::toHexString);
Gen<String> result = null;
// First pick an index between 0 and 7 where the compression start
// Then pick an index between the start index and 8 where the compression stops
// Generate the left part from 0 to the start index
// Generate the right part from the stop index to 8
// If the start index is 0 everything is compressed until the stop index, the string starts with ::
// If the stop index is 7 everything is compressed before the stop index, the string ends with ::
for (int compStartIndex = 0; compStartIndex < 7; compStartIndex++) {
for (int compStopIndex = compStartIndex + 1; compStopIndex < 8; compStopIndex++) {
Gen<String> compResult;
if(compStartIndex == 0) {
compResult = IntStream.range(compStopIndex, 8)
.mapToObj(i -> part)
.reduce((left, right) -> left.zip(right, (s1, s2) -> s1 + ':' + s2))
.orElseThrow(IllegalStateException::new)
.map(s -> "::" + s);
} else if (compStopIndex == 7) {
compResult = IntStream.range(0, compStartIndex)
.mapToObj(i -> part)
.reduce((left, right) -> left.zip(right, (s1, s2) -> s1 + ':' + s2))
.orElseThrow(IllegalStateException::new)
.map(s -> s + "::");
} else {
Gen<String> leftPart = IntStream.range(0, compStartIndex)
.mapToObj(i -> part)
.reduce((left, right) -> left.zip(right, (s1, s2) -> s1 + ':' + s2))
.orElseThrow(IllegalStateException::new);
Gen<String> rightPart = IntStream.range(compStopIndex, 8)
.mapToObj(i -> part)
.reduce((left, right) -> left.zip(right, (s1, s2) -> s1 + ':' + s2))
.orElseThrow(IllegalStateException::new);
compResult = leftPart.zip(rightPart, (s1, s2) -> s1 + "::" + s2);
}
if(result == null) {
result = compResult;
} else {
result.mix(compResult);
}
}
}
return result;
}

private Gen<String> ipv6Mixed() {
Gen<String> part = integers().between(0, 65535).map(Integer::toHexString);
Gen<String> ipv4 = ipv4();
return IntStream.range(0, 6)
.mapToObj(i -> part)
.reduce((left, right) -> left.zip(right, (s1, s2) -> s1 + ':' + s2))
.orElseThrow(IllegalStateException::new)
.zip(ipv4, (s1, s2) -> s1 + ':' + s2);
}

private Gen<String> ipv6MixedCompressed() {
Gen<String> part = integers().between(0, 65535).map(Integer::toHexString);
Gen<String> ipv4 = ipv4();
Gen<String> result = null;
// First pick an index between 0 and 7 where the compression start
// Then pick an index between the start index and 8 where the compression stops
// Generate the left part from 0 to the start index
// Generate the right part from the stop index to 8
// If the start index is 0 everything is compressed until the stop index, the string starts with ::
// If the stop index is 7 everything is compressed before the stop index, the string ends with ::
for (int compStartIndex = 0; compStartIndex < 6; compStartIndex++) {
for (int compStopIndex = compStartIndex + 1; compStopIndex < 6; compStopIndex++) {
Gen<String> compResult;
if(compStartIndex == 0) {
compResult = IntStream.range(compStopIndex, 6)
.mapToObj(i -> part)
.reduce((left, right) -> left.zip(right, (s1, s2) -> s1 + ':' + s2))
.orElseThrow(IllegalStateException::new)
.zip(ipv4, (s1, s2) -> s1 + ":" + s2)
.map(s -> "::" + s);
} else if (compStopIndex == 5) {
compResult = IntStream.range(0, compStartIndex)
.mapToObj(i -> part)
.reduce((left, right) -> left.zip(right, (s1, s2) -> s1 + ':' + s2))
.orElseThrow(IllegalStateException::new)
.zip(ipv4, (s1, s2) -> s1 + "::" + s2);
} else {
Gen<String> leftPart = IntStream.range(0, compStartIndex)
.mapToObj(i -> part)
.reduce((left, right) -> left.zip(right, (s1, s2) -> s1 + ':' + s2))
.orElseThrow(IllegalStateException::new);
Gen<String> rightPart = IntStream.range(compStopIndex, 6)
.mapToObj(i -> part)
.reduce((left, right) -> left.zip(right, (s1, s2) -> s1 + ':' + s2))
.orElseThrow(IllegalStateException::new);
compResult = leftPart.zip(rightPart, (s1, s2) -> s1 + "::" + s2).zip(ipv4, (s1, s2) -> s1 + ':' + s2);
}
if(result == null) {
result = compResult;
} else {
result.mix(compResult);
}
}
}
return result;
}
}
1 change: 0 additions & 1 deletion src/test/java/com/cloudbees/jenkins/support/impl/FileDescriptorLimitTest.java
View file
Open in desktop
Expand Up @@ -50,7 +50,6 @@ public void addContents() throws Exception {
MatcherAssert.assertThat(output, containsString(SENSITIVE_JOB_NAME));
}

@Ignore("TODO flaky test")
@Test
public void addContentsFiltered() throws Exception {
Assume.assumeTrue(!Functions.isWindows());
Expand Down