Skip to content

Commit b8a477d

Browse files
authored
fix: NPE in handleRecentResourceUpdate for an unknown secondary resource (#3519)
`ExternalResourceCachingEventSource.handleRecentResourceUpdate` checks that the primary has an entry in the cache, but then dereferences the per-secondary lookup without checking it: R actualResource = actualValues.get(resourceId); if (actualResource.equals(previousVersionOfResource)) { `actualValues.get(resourceId)` returns null whenever the primary has a cache entry but that particular secondary id is not in it, which throws a NullPointerException. This is reachable from `AbstractEventSourceHolderDependentResource.onUpdated` for any `RecentOperationCacheFiller` event source, e.g. after an update whose resource id is not the one currently cached for that primary. Skips the cache update when the resource is not tracked, which matches the intent of the surrounding "only overwrite if we still hold the version the caller saw" check. Adds a regression test that fails with NullPointerException without this change.
1 parent 96e59cd commit b8a477d

2 files changed

Lines changed: 11 additions & 1 deletion

File tree

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/ExternalResourceCachingEventSource.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ public synchronized void handleRecentResourceUpdate(
238238
if (actualValues != null) {
239239
var resourceId = resourceIDMapper.idFor(resource);
240240
R actualResource = actualValues.get(resourceId);
241-
if (actualResource.equals(previousVersionOfResource)) {
241+
if (actualResource != null && actualResource.equals(previousVersionOfResource)) {
242242
actualValues.put(resourceId, resource);
243243
}
244244
}

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/ExternalResourceCachingEventSourceTest.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,16 @@ void genericFilteringEvents() {
212212
}
213213

214214
@Test
215+
void recentResourceUpdateIsIgnoredForUnknownSecondaryResource() {
216+
source.handleResources(primaryID1(), Set.of(testResource1()));
217+
218+
// testResource2 has a different id, so it is not present in the cache for primaryID1
219+
var unknown = testResource2();
220+
source.handleRecentResourceUpdate(primaryID1(), unknown, unknown);
221+
222+
assertThat(source.getSecondaryResources(primaryID1())).containsExactly(testResource1());
223+
}
224+
215225
void onlyGenericFilterSetDoesNotFailOnAdd() {
216226
var eventSource = new TestExternalCachingEventSource();
217227
eventSource.setGenericFilter(res -> true);

0 commit comments

Comments
 (0)