fix #3972: removing internal usage of registerCustomKind

fabric8io · Dec 7, 2022 · 1dab2f8 · 1dab2f8
1 parent 6bcb118
commit 1dab2f8
Show file tree

Hide file tree

Showing 8 changed files with 39 additions and 102 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -29,8 +29,9 @@
 * Fix #4516: added support for blocking delete operations using the withTimeout methods: op.withTimeout(1, TimeUnit.MINUTE).delete() - will wait for up to 1 minute for the resources to be fully deleted. This makes for a more concise replacement of the deletingExisting method.
 
 #### _**Note**_: Breaking changes
-* Fix #4515: files located at the root of jars named model.properties, e.g. core.properties, have been removed
 * Fix #3923: removed KubernetesResourceMappingProvider - a META-INF/services/io.fabric8.kubernetes.api.model.KubernetesResource list of resources is used instead.
+* Fix #4515: files located at the root of jars named model.properties, e.g. core.properties, have been removed
+* Fix #4579: the implicit registration of resource and list types that happens when using the resource(class) methods has been removed. This makes the behavior of the client more predictable as that was an undocumented side-effect.  If you expect to see instances of a custom type from an untyped api call - typically KubernetesClient.load, KubernetesClient.resourceList, KubernetesClient.resource(InputStream|String), then you must either create a META-INF/services/io.fabric8.kubernetes.api.model.KubernetesResource file (see above #3923), or make calls to KubernetesDeserializer.registerCustomKind - however since KubernetesDeserializer is an internal class that mechanism is not preferred.
 * Fix #4597: remove the deprecated support for `javax.validation.constraints.NotNull` in the `crd-generator`, to mark a property as `required` it needs to be annotated with `io.fabric8.generator.annotation.Required`
 
 ### 6.2.0 (2022-10-20)

diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/BaseOperation.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/BaseOperation.java
@@ -15,8 +15,10 @@
  */
 package io.fabric8.kubernetes.client.dsl.internal;
 
+import com.fasterxml.jackson.core.type.TypeReference;
 import io.fabric8.kubernetes.api.builder.TypedVisitor;
 import io.fabric8.kubernetes.api.builder.Visitor;
+import io.fabric8.kubernetes.api.model.DefaultKubernetesResourceList;
 import io.fabric8.kubernetes.api.model.DeletionPropagation;
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.api.model.KubernetesResource;
@@ -66,6 +68,7 @@
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.lang.reflect.Type;
 import java.net.HttpURLConnection;
 import java.net.MalformedURLException;
 import java.net.URL;
@@ -390,7 +393,16 @@ public CompletableFuture<L> submitList(ListOptions listOptions) {
     try {
       URL fetchListUrl = fetchListUrl(getNamespacedUrl(), defaultListOptions(listOptions, null));
       HttpRequest.Builder requestBuilder = httpClient.newHttpRequestBuilder().url(fetchListUrl);
-      CompletableFuture<L> futureAnswer = handleResponse(httpClient, requestBuilder, listType, getParameters());
+      Type refinedType = listType.equals(DefaultKubernetesResourceList.class)
+          ? Serialization.jsonMapper().getTypeFactory().constructParametricType(listType, type)
+          : listType;
+      TypeReference<L> listTypeReference = new TypeReference<L>() {
+        @Override
+        public Type getType() {
+          return refinedType;
+        }
+      };
+      CompletableFuture<L> futureAnswer = handleResponse(httpClient, requestBuilder, listTypeReference, getParameters());
       return futureAnswer.thenApply(l -> {
         updateApiVersion(l);
         return l;

diff --git a/...nt/src/main/java/io/fabric8/kubernetes/client/dsl/internal/HasMetadataOperationsImpl.java b/...nt/src/main/java/io/fabric8/kubernetes/client/dsl/internal/HasMetadataOperationsImpl.java
@@ -15,17 +15,14 @@
  */
 package io.fabric8.kubernetes.client.dsl.internal;
 
-import io.fabric8.kubernetes.api.model.GenericKubernetesResource;
 import io.fabric8.kubernetes.api.model.HasMetadata;
-import io.fabric8.kubernetes.api.model.KubernetesResource;
 import io.fabric8.kubernetes.api.model.KubernetesResourceList;
 import io.fabric8.kubernetes.client.Client;
 import io.fabric8.kubernetes.client.dsl.MixedOperation;
 import io.fabric8.kubernetes.client.dsl.Resource;
 import io.fabric8.kubernetes.client.dsl.base.ResourceDefinitionContext;
 import io.fabric8.kubernetes.client.impl.BaseClient;
 import io.fabric8.kubernetes.client.utils.Utils;
-import io.fabric8.kubernetes.internal.KubernetesDeserializer;
 
 import static io.fabric8.kubernetes.client.utils.KubernetesResourceUtil.inferListType;
 
@@ -50,27 +47,13 @@ public HasMetadataOperationsImpl(OperationContext context, ResourceDefinitionCon
         .withPlural(rdc.getPlural()), type, listType != null ? listType : (Class) inferListType(type));
 
     this.rdc = rdc;
-
-    if (!GenericKubernetesResource.class.isAssignableFrom(type)) {
-      // TODO: the static nature of these registrations is problematic,
-      // we should ensure that we aren't redefining an existing type
-      KubernetesDeserializer.registerCustomKind(apiVersion, kind(rdc), type);
-      if (KubernetesResource.class.isAssignableFrom(this.listType)) {
-        KubernetesDeserializer.registerCustomKind(this.listType.getSimpleName(),
-            (Class<? extends KubernetesResource>) this.listType);
-      }
-    }
   }
 
   @Override
   public HasMetadataOperationsImpl<T, L> newInstance(OperationContext context) {
     return new HasMetadataOperationsImpl<>(context, rdc, type, listType);
   }
 
-  private String kind(ResourceDefinitionContext context) {
-    return context.getKind() != null ? context.getKind() : getKind();
-  }
-
   @Override
   public boolean isResourceNamespaced() {
     return rdc.isNamespaceScoped();

diff --git a/...etes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/OperationSupport.java b/...etes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/OperationSupport.java
@@ -48,6 +48,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InterruptedIOException;
+import java.lang.reflect.Type;
 import java.net.HttpURLConnection;
 import java.net.MalformedURLException;
 import java.net.URL;
@@ -566,7 +567,12 @@ protected <T> T handleResponse(HttpRequest.Builder requestBuilder, Class<T> type
    */
   private <T> T handleResponse(HttpRequest.Builder requestBuilder, Class<T> type, Map<String, String> parameters)
       throws IOException {
-    return waitForResult(handleResponse(httpClient, requestBuilder, type, parameters));
+    return waitForResult(handleResponse(httpClient, requestBuilder, new TypeReference<T>() {
+      @Override
+      public Type getType() {
+        return type;
+      }
+    }, parameters));
   }
 
   /**
@@ -580,7 +586,8 @@ private <T> T handleResponse(HttpRequest.Builder requestBuilder, Class<T> type,
    *
    * @return Returns a de-serialized object as api server response of provided type.
    */
-  protected <T> CompletableFuture<T> handleResponse(HttpClient client, HttpRequest.Builder requestBuilder, Class<T> type,
+  protected <T> CompletableFuture<T> handleResponse(HttpClient client, HttpRequest.Builder requestBuilder,
+      TypeReference<T> type,
       Map<String, String> parameters) {
     VersionUsageUtils.log(this.resourceT, this.apiGroupVersion);
     HttpRequest request = requestBuilder.build();
@@ -592,7 +599,7 @@ protected <T> CompletableFuture<T> handleResponse(HttpClient client, HttpRequest
     return futureResponse.thenApply(response -> {
       try {
         assertResponseCode(request, response);
-        if (type != null) {
+        if (type != null && type.getType() != null) {
           return Serialization.unmarshal(new ByteArrayInputStream(response.body()), type, parameters);
         } else {
           return null;

diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/impl/KubernetesClientImpl.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/impl/KubernetesClientImpl.java
@@ -15,8 +15,6 @@
  */
 package io.fabric8.kubernetes.client.impl;
 
-import com.fasterxml.jackson.core.type.TypeReference;
-import com.fasterxml.jackson.databind.type.TypeFactory;
 import io.fabric8.kubernetes.api.model.APIGroup;
 import io.fabric8.kubernetes.api.model.APIGroupBuilder;
 import io.fabric8.kubernetes.api.model.APIResource;
@@ -386,9 +384,7 @@ public NamespaceableResource<HasMetadata> resource(InputStream is) {
    */
   @Override
   public MixedOperation<Binding, KubernetesResourceList<Binding>, Resource<Binding>> bindings() {
-    return resources(Binding.class,
-        (Class<KubernetesResourceList<Binding>>) TypeFactory.rawClass(new TypeReference<KubernetesResourceList<Binding>>() {
-        }.getType()));
+    return resources(Binding.class);
   }
 
   /**

diff --git a/...rc/test/java/io/fabric8/kubernetes/client/dsl/internal/HasMetadataOperationsImplTest.java b/...rc/test/java/io/fabric8/kubernetes/client/dsl/internal/HasMetadataOperationsImplTest.java
@@ -15,26 +15,17 @@
  */
 package io.fabric8.kubernetes.client.dsl.internal;
 
+import io.fabric8.kubernetes.api.model.DefaultKubernetesResourceList;
 import io.fabric8.kubernetes.api.model.GenericKubernetesResource;
 import io.fabric8.kubernetes.api.model.KubernetesResource;
-import io.fabric8.kubernetes.api.model.apiextensions.v1beta1.CustomResourceDefinition;
 import io.fabric8.kubernetes.client.CustomResource;
-import io.fabric8.kubernetes.client.CustomResourceList;
 import io.fabric8.kubernetes.client.dsl.MixedOperation;
 import io.fabric8.kubernetes.client.dsl.Resource;
-import io.fabric8.kubernetes.client.dsl.base.CustomResourceDefinitionContext;
-import io.fabric8.kubernetes.client.dsl.base.ResourceDefinitionContext;
 import io.fabric8.kubernetes.client.impl.KubernetesClientImpl;
 import io.fabric8.kubernetes.client.utils.Serialization;
 import io.fabric8.kubernetes.model.annotation.Group;
 import io.fabric8.kubernetes.model.annotation.Version;
-import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.Arguments;
-import org.junit.jupiter.params.provider.MethodSource;
-
-import java.util.stream.Stream;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -61,71 +52,11 @@ void shouldReturnGenericKubernetesResourceWhenNotRegistered() {
         .hasFieldOrPropertyWithValue("apiVersion", "sample.fabric8.io/v1");
   }
 
-  @DisplayName("HasMetadataOperationsImpl registers custom kind")
-  @ParameterizedTest(name = "{index}: {1}")
-  @MethodSource("registerCustomKindInput")
-  void hasMetadataOperationsImplRegistersCustomKind(
-      String description,
-      ResourceDefinitionContext resourceDefinitionContext,
-      Class<? extends CustomResource<?, ?>> resourceClazz,
-      Class<? extends CustomResourceList<?>> resourceListClazz) {
-    // Given
-    new HasMetadataOperationsImpl(
-        new OperationContext(),
-        resourceDefinitionContext,
-        resourceClazz,
-        resourceListClazz);
-    // When
-    final KubernetesResource resource = Serialization.unmarshal("{\n" +
-        "    \"apiVersion\": \"custom.group/v1alpha1\",\n" +
-        "    \"kind\": \"MyCustomResource\"\n" +
-        "}");
-    // Then
-    assertThat(resource)
-        .isInstanceOf(MyCustomResource.class)
-        .hasFieldOrPropertyWithValue("apiVersion", "custom.group/v1alpha1");
-  }
-
-  static Stream<Arguments> registerCustomKindInput() {
-    final CustomResourceDefinition myCustomResourceCrd = CustomResourceDefinitionContext
-        .v1beta1CRDFromCustomResourceType(MyCustomResource.class).build();
-    return Stream.of(
-        Arguments.arguments(
-            "with typed custom resource and list",
-            CustomResourceDefinitionContext.fromCrd(myCustomResourceCrd),
-            MyCustomResource.class,
-            MyCustomResourceList.class),
-        Arguments.arguments(
-            "with typed custom resource and generic list",
-            CustomResourceDefinitionContext.fromCrd(myCustomResourceCrd),
-            MyCustomResource.class,
-            CustomResourceList.class),
-        Arguments.arguments(
-            "with manual ResourceDefinitionContext",
-            new ResourceDefinitionContext.Builder()
-                .withGroup("custom.group")
-                .withVersion("v1alpha1")
-                .withPlural("mycustomresources")
-                .build(),
-            MyCustomResource.class,
-            MyCustomResourceList.class));
-  }
-
-  @Group(MyCustomResource.GROUP)
-  @Version(MyCustomResource.VERSION)
-  public static class MyCustomResource extends CustomResource<Object, Object> {
-    public static final String GROUP = "custom.group";
-    public static final String VERSION = "v1alpha1";
-  }
-
-  public static class MyCustomResourceList extends CustomResourceList<MyCustomResource> {
-  }
-
   @Group("sample.fabric8.io")
   @Version("v1")
   public static class Bar extends CustomResource<Object, Object> {
   }
 
-  public static class BarList extends CustomResourceList<Bar> {
+  public static class BarList extends DefaultKubernetesResourceList<Bar> {
   }
 }
diff --git a/...c/test/java/io/fabric8/kubernetes/client/mock/TypedClusterScopeCustomResourceApiTest.java b/...c/test/java/io/fabric8/kubernetes/client/mock/TypedClusterScopeCustomResourceApiTest.java
@@ -15,10 +15,10 @@
  */
 package io.fabric8.kubernetes.client.mock;
 
+import io.fabric8.kubernetes.api.model.DefaultKubernetesResourceList;
 import io.fabric8.kubernetes.api.model.DeletionPropagation;
 import io.fabric8.kubernetes.api.model.KubernetesResourceList;
 import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
-import io.fabric8.kubernetes.client.CustomResourceList;
 import io.fabric8.kubernetes.client.KubernetesClient;
 import io.fabric8.kubernetes.client.dsl.MixedOperation;
 import io.fabric8.kubernetes.client.dsl.Resource;
@@ -58,8 +58,8 @@ void create() {
 
   @Test
   void list() {
-    KubernetesResourceList<Star> starList = new CustomResourceList<>();
-    ((CustomResourceList<Star>) starList).setItems(Collections.singletonList(getStar()));
+    KubernetesResourceList<Star> starList = new DefaultKubernetesResourceList<>();
+    ((DefaultKubernetesResourceList<Star>) starList).setItems(Collections.singletonList(getStar()));
     server.expect().get().withPath("/apis/example.crd.com/v1alpha1/stars").andReturn(200, starList).once();
     starClient = client.resources(Star.class);
 

diff --git a/...c/main/java/io/fabric8/openshift/client/dsl/internal/build/BuildConfigOperationsImpl.java b/...c/main/java/io/fabric8/openshift/client/dsl/internal/build/BuildConfigOperationsImpl.java
@@ -15,6 +15,7 @@
  */
 package io.fabric8.openshift.client.dsl.internal.build;
 
+import com.fasterxml.jackson.core.type.TypeReference;
 import io.fabric8.kubernetes.api.model.Event;
 import io.fabric8.kubernetes.api.model.EventList;
 import io.fabric8.kubernetes.client.Client;
@@ -51,6 +52,7 @@
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.lang.reflect.Type;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.List;
@@ -262,7 +264,12 @@ protected Build submitToApiServer(InputStream inputStream, long contentLength) {
           .post("application/octet-stream", inputStream, contentLength)
           .expectContinue()
           .uri(getQueryParameters());
-      return waitForResult(handleResponse(newClient, requestBuilder, Build.class, null));
+      return waitForResult(handleResponse(newClient, requestBuilder, new TypeReference<Build>() {
+        @Override
+        public Type getType() {
+          return Build.class;
+        }
+      }, null));
     } catch (Throwable e) {
       // TODO: better determine which exception this should occur on
       // otherwise we need to have the httpclient api open up to the notion