From c91e951eaebd823be6ad34478eba9fd50e6a631d Mon Sep 17 00:00:00 2001 From: Antonio Muniz Date: Thu, 19 Dec 2024 17:43:12 +0100 Subject: [PATCH] Avoid NPE on informers (#1630) I found out when working on a CloudBees proprietary feature that `KubernetesCloud#readResolve` is not always called on deserialization. I think this will not happen in OSS, but let's stay on the safe side and initialize the field before using it. --- .../plugins/kubernetes/KubernetesCloud.java | 14 ++++++++++---- .../kubernetes/watch/PodStatusEventHandler.java | 4 +++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java index 037887e47..09e2bb3f6 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java @@ -172,7 +172,7 @@ public class KubernetesCloud extends Cloud implements PodTemplateGroup { * namespace -> informer * Use to watch pod events per namespace. */ - private transient Map> informers = new ConcurrentHashMap<>(); + private transient volatile Map> informers = new ConcurrentHashMap<>(); @DataBoundConstructor public KubernetesCloud(String name) { @@ -1306,9 +1306,6 @@ private Object readResolve() { if (containerCap != null && containerCap == 0) { containerCap = null; } - if (informers == null) { - informers = new ConcurrentHashMap<>(); - } return this; } @@ -1321,6 +1318,15 @@ public Cloud reconfigure(@NonNull StaplerRequest req, JSONObject form) throws De } public void registerPodInformer(KubernetesSlave node) { + // even having readResolve initializing informers is not enough, there are some special cases where XStream will + // not call it, so let us make sure it is initialized before using + if (informers == null) { + synchronized (this) { + if (informers == null) { + informers = new ConcurrentHashMap<>(); + } + } + } informers.computeIfAbsent(node.getNamespace(), (n) -> { KubernetesClient client; try { diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java index cac846b50..694adc690 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java @@ -50,7 +50,9 @@ private String formatPodStatus(PodCondition c, String phase, StringBuilder sb) { // not interesting return ""; } - String formatted = String.format("%n\tPod [%s][%s] %s", phase, c.getReason(), c.getMessage()); + String message = c.getMessage(); + String formatted = + String.format("%n\tPod [%s][%s] %s", phase, c.getReason(), message != null ? message : "No message"); return sb.indexOf(formatted) == -1 ? formatted : ""; }