From 1dab2f8f569a3482838cfe9185dd78ad9b83490d 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 | 3 +- .../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, 39 insertions(+), 102 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d3277afd64..e5eaf45fa96 100644 --- 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 index 3ac99664972..1e3d982da6d 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; @@ -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 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 55dfa177356..da119314150 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 @@ -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 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)); } /** @@ -580,7 +586,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(); @@ -592,7 +599,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 165872e6ecc..b3b31b0453c 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; @@ -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() { + @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