From 5014a63bd7428a0fb21df0f6faaefc167581202a Mon Sep 17 00:00:00 2001
From: shawkins <shawkins@redhat.com>
Date: Fri, 25 Jun 2021 06:58:44 -0400
Subject: [PATCH] fix #3272: addressing npe and index logic in general

---
 CHANGELOG.md                                   |  1 +
 .../client/informers/cache/Cache.java          | 18 +++++++++---------
 .../client/informers/cache/CacheTest.java      |  5 +++++
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e34c3798b4f..e245efdd7e6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,7 @@
 * Fix #3216: made the mock server aware of apiVersions
 * Fix #3225: Pod metric does not have corresponding label selector variant
 * Fix #3243: pipes provided to exec command are no longer closed on connection close, so that client can fully read the buffer after the command finishes.
+* Fix #3272: prevent index npe after informer sees an empty list
 
 #### Improvements
 * Fix #3078: adding javadocs to further clarify patch, edit, replace, etc. and note the possibility of items being modified.
diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/informers/cache/Cache.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/informers/cache/Cache.java
index 31f4fd99fb6..d08dbd17097 100644
--- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/informers/cache/Cache.java
+++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/informers/cache/Cache.java
@@ -44,13 +44,13 @@ public class Cache<T> implements Indexer<T> {
   public static final String NAMESPACE_INDEX = "namespace";
 
   // indexers stores index functions by their names
-  private Map<String, Function<T, List<String>>> indexers = new HashMap<>();
+  private final Map<String, Function<T, List<String>>> indexers = new HashMap<>();
 
   // items stores object instances
   private volatile ConcurrentHashMap<String, T> items = new ConcurrentHashMap<>();
 
   // indices stores objects' key by their indices
-  private Map<String, Map<String, Set<String>>> indices = new HashMap<>();
+  private final Map<String, Map<String, Set<String>>> indices = new HashMap<>();
   
   private BooleanSupplier isRunning = () -> false;
 
@@ -74,12 +74,12 @@ public void setIsRunning(BooleanSupplier isRunning) {
    * @return registered indexers
    */
   @Override
-  public Map<String, Function<T, List<String>>> getIndexers() {
-    return indexers;
+  public synchronized Map<String, Function<T, List<String>>> getIndexers() {
+    return Collections.unmodifiableMap(indexers);
   }
 
   @Override
-  public void addIndexers(Map<String, Function<T, List<String>>> indexersNew) {
+  public synchronized void addIndexers(Map<String, Function<T, List<String>>> indexersNew) {
     if (isRunning.getAsBoolean()) {
       throw new IllegalStateException("Cannot add indexers to a running informer.");
     }  
@@ -147,7 +147,7 @@ public synchronized Map<String, T> replace(List<T> list) {
     this.items = newItems;
 
     // rebuild any index
-    this.indices = new HashMap<>();
+    this.indices.values().stream().forEach(Map::clear);
     for (Map.Entry<String, T> itemEntry : items.entrySet()) {
       this.updateIndices(null, itemEntry.getValue(), itemEntry.getKey());
     }
@@ -294,7 +294,7 @@ public synchronized List<T> byIndex(String indexName, String indexKey) {
    * @param newObj new object
    * @param key the key
    */
-  public void updateIndices(T oldObj, T newObj, String key) {
+  void updateIndices(T oldObj, T newObj, String key) {
     if (oldObj != null) {
       deleteFromIndices(oldObj, key);
     }
@@ -307,7 +307,7 @@ public void updateIndices(T oldObj, T newObj, String key) {
         continue;
       }
 
-      Map<String, Set<String>> index = this.indices.computeIfAbsent(indexName, k -> new HashMap<>());
+      Map<String, Set<String>> index = this.indices.get(indexName);
       for (String indexValue : indexValues) {
         Set<String> indexSet = index.computeIfAbsent(indexValue, k -> new HashSet<>());
         indexSet.add(key);
@@ -350,7 +350,7 @@ private void deleteFromIndices(T oldObj, String key) {
    * @param indexName the index name
    * @param indexFunc the index func
    */
-  public void addIndexFunc(String indexName, Function<T, List<String>> indexFunc) {
+  public synchronized void addIndexFunc(String indexName, Function<T, List<String>> indexFunc) {
     this.indices.put(indexName, new HashMap<>());
     this.indexers.put(indexName, indexFunc);
   }
diff --git a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/informers/cache/CacheTest.java b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/informers/cache/CacheTest.java
index 9c7df9c99b7..0810e03cc38 100644
--- a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/informers/cache/CacheTest.java
+++ b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/informers/cache/CacheTest.java
@@ -20,12 +20,14 @@
 import org.junit.jupiter.api.Test;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Function;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 
 class CacheTest {
   private static Cache cache = new Cache("mock", CacheTest::mockIndexFunction, CacheTest::mockKeyFunction);
@@ -49,6 +51,9 @@ void testCacheIndex() {
     List<String> allExistingKeys = cache.listKeys();
     assertEquals(1, allExistingKeys.size());
     assertEquals(key, allExistingKeys.get(0));
+    
+    cache.replace(Collections.emptyList());
+    assertEquals(0, cache.byIndex("mock", "y").size());
   }
 
   @Test