From 17aab60dba3e53571f5ae2bf1d83192bdce9cfe5 Mon Sep 17 00:00:00 2001 From: Rohan Kumar Date: Mon, 3 Aug 2020 16:07:58 +0530 Subject: [PATCH] Fix #2354: Fix NullPointerException in ResourceCompare when no resource is returned from fromServer.get() Somehow resource was being returned as `null` from `fromServer.get()` call due to no namespace being provided. Added a case of Null items in `ResourceCompare#equals` --- CHANGELOG.md | 1 + .../io/fabric8/knative/test/RouteTest.java | 79 +++++++++++++++++++ .../{RouteTest.java => RouteCrudTest.java} | 2 +- .../client/utils/ResourceCompare.java | 7 ++ .../client/utils/ResourceCompareTest.java | 20 +++++ 5 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 extensions/knative/tests/src/test/java/io/fabric8/knative/test/RouteTest.java rename extensions/knative/tests/src/test/java/io/fabric8/knative/test/crud/{RouteTest.java => RouteCrudTest.java} (98%) diff --git a/CHANGELOG.md b/CHANGELOG.md index bbd9422fa50..9814b6e4d0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ #### Bugs * Fix #2373: Unable to create a Template on OCP3 * Fix #2316: Cannot load resource from stream without apiVersion +* Fix #2354: Fix NullPointerException in ResourceCompare when no resource is returned from fromServer.get() #### Improvements * Fix #2331: Fixed documentation for namespaced informer for all custom types implementing `Namespaced` interface diff --git a/extensions/knative/tests/src/test/java/io/fabric8/knative/test/RouteTest.java b/extensions/knative/tests/src/test/java/io/fabric8/knative/test/RouteTest.java new file mode 100644 index 00000000000..8a4324790dc --- /dev/null +++ b/extensions/knative/tests/src/test/java/io/fabric8/knative/test/RouteTest.java @@ -0,0 +1,79 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.knative.test; + +import io.fabric8.knative.client.KnativeClient; +import io.fabric8.knative.mock.KnativeServer; +import io.fabric8.knative.serving.v1.Route; +import io.fabric8.knative.serving.v1.RouteBuilder; +import org.junit.Rule; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.migrationsupport.rules.EnableRuleMigrationSupport; + +import java.net.HttpURLConnection; + +import static org.junit.jupiter.api.Assertions.assertNotNull; + +@EnableRuleMigrationSupport +class RouteTest { + @Rule + public KnativeServer server = new KnativeServer(); + + @Test + void testCreateOrReplace() { + // Given + Route route = new RouteBuilder() + .withNewMetadata() + .withName("helloworld-nodejs-red-blue1") + .withNamespace("test") + .endMetadata() + .withNewSpec() + .addNewTraffic() + .withConfigurationName("greeter") + .withPercent(100L) + .endTraffic() + .endSpec() + .build(); + server.expect().post().withPath("/apis/serving.knative.dev/v1/namespaces/test/routes") + .andReturn(HttpURLConnection.HTTP_CONFLICT, route) + .once(); + server.expect().get().withPath("/apis/serving.knative.dev/v1/namespaces/test/routes/helloworld-nodejs-red-blue1") + .andReturn(HttpURLConnection.HTTP_OK, route) + .times(2); + server.expect().put().withPath("/apis/serving.knative.dev/v1/namespaces/test/routes/helloworld-nodejs-red-blue1") + .andReturn(HttpURLConnection.HTTP_OK, route) + .once(); + KnativeClient kn = server.getKnativeClient(); + + // When + route = kn.routes().createOrReplaceWithNew() + .withNewMetadata() + .withName("helloworld-nodejs-red-blue1") + .addToAnnotations("foo", "bar") + .withNamespace("test") + .endMetadata() + .withNewSpec() + .addNewTraffic() + .withConfigurationName("greeter") + .withPercent(100L) + .endTraffic() + .endSpec() + .done(); + + // Then + assertNotNull(route); + } +} diff --git a/extensions/knative/tests/src/test/java/io/fabric8/knative/test/crud/RouteTest.java b/extensions/knative/tests/src/test/java/io/fabric8/knative/test/crud/RouteCrudTest.java similarity index 98% rename from extensions/knative/tests/src/test/java/io/fabric8/knative/test/crud/RouteTest.java rename to extensions/knative/tests/src/test/java/io/fabric8/knative/test/crud/RouteCrudTest.java index 2ce03da63a2..5821ee45c36 100644 --- a/extensions/knative/tests/src/test/java/io/fabric8/knative/test/crud/RouteTest.java +++ b/extensions/knative/tests/src/test/java/io/fabric8/knative/test/crud/RouteCrudTest.java @@ -29,7 +29,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; @EnableRuleMigrationSupport -public class RouteTest { +public class RouteCrudTest { @Rule public KnativeServer server = new KnativeServer(true, true); diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/ResourceCompare.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/ResourceCompare.java index 519ecb205e3..18da084157f 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/ResourceCompare.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/ResourceCompare.java @@ -48,6 +48,13 @@ private ResourceCompare() {} */ public static boolean equals(T left, T right) { ObjectMapper jsonMapper = Serialization.jsonMapper(); + if (left == null && right == null) { + return true; + } else if (left == null) { + return false; + } else if (right == null) { + return false; + } Map leftJson = jsonMapper.convertValue(left, TYPE_REF); Map rightJson = jsonMapper.convertValue(right, TYPE_REF); diff --git a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/utils/ResourceCompareTest.java b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/utils/ResourceCompareTest.java index 89d2b844902..3c0ac03dbd5 100644 --- a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/utils/ResourceCompareTest.java +++ b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/utils/ResourceCompareTest.java @@ -17,6 +17,7 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import io.fabric8.kubernetes.api.model.IntOrString; @@ -24,6 +25,8 @@ import io.fabric8.kubernetes.api.model.KubernetesListBuilder; import io.fabric8.kubernetes.api.model.Pod; import io.fabric8.kubernetes.api.model.PodBuilder; +import io.fabric8.kubernetes.api.model.PodList; +import io.fabric8.kubernetes.api.model.PodListBuilder; import io.fabric8.kubernetes.api.model.ReplicationController; import io.fabric8.kubernetes.api.model.ReplicationControllerBuilder; import io.fabric8.kubernetes.api.model.Service; @@ -32,6 +35,7 @@ import org.junit.jupiter.api.Test; import java.util.Collections; +import java.util.List; public class ResourceCompareTest { @@ -130,4 +134,20 @@ void testServiceDifferenceFromClusterAndAsObject() { assertTrue(result); } + @Test + void testEqualsWhenOneResourceIsNull() { + // Given + Pod pod2 = new PodBuilder().withNewMetadata().withName("foo").endMetadata().build(); + + // When + boolean result = ResourceCompare.equals(null, pod2); + + // Then + assertFalse(result); + } + + @Test + void testEqualsWhenBothNull() { + assertTrue(ResourceCompare.equals(null, null)); + } }