From 97bdac4eb781bd91d2d18d452d885144d5af34cd Mon Sep 17 00:00:00 2001 From: Steve Hawkins Date: Tue, 15 Nov 2022 16:33:40 -0500 Subject: [PATCH] fix #3972: removing internal usage of registerCustomKind --- CHANGELOG.md | 1 + .../client/dsl/internal/BaseOperation.java | 14 +++- .../internal/HasMetadataOperationsImpl.java | 17 ----- .../client/dsl/internal/OperationSupport.java | 13 +++- .../client/impl/KubernetesClientImpl.java | 6 +- .../HasMetadataOperationsImplTest.java | 73 +------------------ ...ypedClusterScopeCustomResourceApiTest.java | 6 +- .../build/BuildConfigOperationsImpl.java | 9 ++- 8 files changed, 38 insertions(+), 101 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ee3ebdfdee..6b3978ce951 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ #### _**Note**_: Breaking changes in the API * 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 #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. ### 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 index 9c77ea8b3fd..8d799081565 100755 --- 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; @@ -65,6 +67,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; @@ -389,7 +392,16 @@ public CompletableFuture submitList(ListOptions listOptions) { try { URL fetchListUrl = fetchListUrl(getNamespacedUrl(), defaultListOptions(listOptions, null)); HttpRequest.Builder requestBuilder = httpClient.newHttpRequestBuilder().url(fetchListUrl); - CompletableFuture futureAnswer = handleResponse(httpClient, requestBuilder, listType, getParameters()); + Type refinedType = listType.equals(DefaultKubernetesResourceList.class) + ? Serialization.jsonMapper().getTypeFactory().constructParametricType(listType, type) + : listType; + TypeReference listTypeReference = new TypeReference() { + @Override + public Type getType() { + return refinedType; + } + }; + CompletableFuture futureAnswer = handleResponse(httpClient, requestBuilder, listTypeReference, getParameters()); return futureAnswer.thenApply(l -> { updateApiVersion(l); return l; diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/HasMetadataOperationsImpl.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/HasMetadataOperationsImpl.java index 48a9f7b20c8..fafbc38c932 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/HasMetadataOperationsImpl.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/HasMetadataOperationsImpl.java @@ -15,9 +15,7 @@ */ 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; @@ -25,7 +23,6 @@ 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,16 +47,6 @@ 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) this.listType); - } - } } @Override @@ -67,10 +54,6 @@ public HasMetadataOperationsImpl 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/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/OperationSupport.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/OperationSupport.java index f317e4d60a2..0144e5e2859 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/OperationSupport.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/OperationSupport.java @@ -47,6 +47,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; @@ -565,7 +566,12 @@ protected T handleResponse(HttpRequest.Builder requestBuilder, Class type */ private T handleResponse(HttpRequest.Builder requestBuilder, Class type, Map parameters) throws IOException { - return waitForResult(handleResponse(httpClient, requestBuilder, type, parameters)); + return waitForResult(handleResponse(httpClient, requestBuilder, new TypeReference() { + @Override + public Type getType() { + return type; + } + }, parameters)); } /** @@ -579,7 +585,8 @@ private T handleResponse(HttpRequest.Builder requestBuilder, Class type, * * @return Returns a de-serialized object as api server response of provided type. */ - protected CompletableFuture handleResponse(HttpClient client, HttpRequest.Builder requestBuilder, Class type, + protected CompletableFuture handleResponse(HttpClient client, HttpRequest.Builder requestBuilder, + TypeReference type, Map parameters) { VersionUsageUtils.log(this.resourceT, this.apiGroupVersion); HttpRequest request = requestBuilder.build(); @@ -591,7 +598,7 @@ protected CompletableFuture 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 index 991eee4c53e..b4ad065511f 100644 --- 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 resource(InputStream is) { */ @Override public MixedOperation, Resource> bindings() { - return resources(Binding.class, - (Class>) TypeFactory.rawClass(new TypeReference>() { - }.getType())); + return resources(Binding.class); } /** diff --git a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/internal/HasMetadataOperationsImplTest.java b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/internal/HasMetadataOperationsImplTest.java index 72121cc2a13..d6012d94dd6 100644 --- a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/internal/HasMetadataOperationsImplTest.java +++ b/kubernetes-client/src/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> resourceClazz, - Class> 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 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 { - public static final String GROUP = "custom.group"; - public static final String VERSION = "v1alpha1"; - } - - public static class MyCustomResourceList extends CustomResourceList { - } - @Group("sample.fabric8.io") @Version("v1") public static class Bar extends CustomResource { } - public static class BarList extends CustomResourceList { + public static class BarList extends DefaultKubernetesResourceList { } } diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/TypedClusterScopeCustomResourceApiTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/TypedClusterScopeCustomResourceApiTest.java index 9849f9e8831..bea8dd3842f 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/TypedClusterScopeCustomResourceApiTest.java +++ b/kubernetes-tests/src/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 starList = new CustomResourceList<>(); - ((CustomResourceList) starList).setItems(Collections.singletonList(getStar())); + KubernetesResourceList starList = new DefaultKubernetesResourceList<>(); + ((DefaultKubernetesResourceList) 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/openshift-client/src/main/java/io/fabric8/openshift/client/dsl/internal/build/BuildConfigOperationsImpl.java b/openshift-client/src/main/java/io/fabric8/openshift/client/dsl/internal/build/BuildConfigOperationsImpl.java index 60a8f11a282..2a37bf6a480 100644 --- a/openshift-client/src/main/java/io/fabric8/openshift/client/dsl/internal/build/BuildConfigOperationsImpl.java +++ b/openshift-client/src/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; @@ -52,6 +53,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; @@ -268,7 +270,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() { + @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