Skip to content

Commit

Permalink
fix #3972: removing internal usage of registerCustomKind
Browse files Browse the repository at this point in the history
  • Loading branch information
shawkins committed Nov 21, 2022
1 parent e86646a commit 4aa9614
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 101 deletions.
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -389,7 +392,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;
Expand Down
Expand Up @@ -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;

Expand All @@ -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();
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -565,7 +566,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));
}

/**
Expand All @@ -579,7 +585,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();
Expand All @@ -591,7 +598,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;
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

/**
Expand Down
Expand Up @@ -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;
Expand All @@ -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> {
}
}
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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<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
Expand Down

0 comments on commit 4aa9614

Please sign in to comment.