fix #3972: preparing for removal of unneeded serialization logic. (#4683

) * fix #3972: preparing for removal of unneeded serialization logic. * fix #3972: taking the changes further to remove overloaded load all parameter operations are now localized to template logic (overloaded get and load) also fixes kubernetesclientbuilder.withconfig
fabric8io · Dec 22, 2022 · 1d3e263 · 1d3e263
1 parent 4d04bf9
commit 1d3e263
Show file tree

Hide file tree

Showing 19 changed files with 139 additions and 310 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,9 @@
 #### New Features
 
 #### _**Note**_: Breaking changes
+* Fix #3972: deprecated Parameterizable and methods on Serialization accepting parameters - that was only needed as a workaround for non-string parameters.  You should instead include those parameter values in the map passed to processLocally.
+* Fix #3972: OpenShiftClient.load will no longer implicitly process templates.  Use OpenShiftClient.templates().load instead.
+* Fix #3972: WARNING: future client versions will not provide the static yaml and json ObjectMappersSerialization.
 * Fix #4574: fromServer has been deprecated - it no longer needs to be called.  All get() operations will fetch the resource(s) from the api server.  If you need the context item that was passed in from a resource, load, or resourceList methods, use the item or items method.
 * Fix #4633: client.run().withRunConfig was deprecated.  Use withNewRunConfig instead.
 * Fix #4663: Config.maxConcurrentRequests and Config.maxConcurrentRequestsPerHost will no longer be used.  Instead they will default to unlimited for all clients.  Due to the ability of the fabric8 client to start long running requests (either websocket or regular http) and how this is treated by the underlying clients you can easily exhaust these values and enter a state where the client is unresponsive without any additional information on what is occurring.

diff --git a/...rnetes-client-api/src/main/java/io/fabric8/kubernetes/client/KubernetesClientBuilder.java b/...rnetes-client-api/src/main/java/io/fabric8/kubernetes/client/KubernetesClientBuilder.java
@@ -98,12 +98,12 @@ public KubernetesClientBuilder withConfig(Config config) {
   }
 
   public KubernetesClientBuilder withConfig(String config) {
-    this.config = Serialization.unmarshal(config);
+    this.config = Serialization.unmarshal(config, Config.class);
     return this;
   }
 
   public KubernetesClientBuilder withConfig(InputStream config) {
-    this.config = Serialization.unmarshal(config);
+    this.config = Serialization.unmarshal(config, Config.class);
     return this;
   }
 

diff --git a/...es-client-api/src/main/java/io/fabric8/kubernetes/client/dsl/ParameterMixedOperation.java b/...es-client-api/src/main/java/io/fabric8/kubernetes/client/dsl/ParameterMixedOperation.java
@@ -17,12 +17,15 @@
 package io.fabric8.kubernetes.client.dsl;
 
 /**
- * A {@link Parameterizable} {@link MixedOperation}
+ * @deprecated It is no longer necessary to associate parameters prior to deserialization.
+ *             <p>
+ *             reference {@link MixedOperation} instead
  *
  * @param <T> The Kubernetes resource type.
  * @param <L> The list variant of the Kubernetes resource type.
  * @param <R> The resource operations.
  */
+@Deprecated
 public interface ParameterMixedOperation<T, L, R extends Resource<T>>
     extends MixedOperation<T, L, R>, Parameterizable<MixedOperation<T, L, R>> {
 }
diff --git a/...etes/client/dsl/ParameterNamespaceListVisitFromServerGetDeleteRecreateWaitApplicable.java b/...etes/client/dsl/ParameterNamespaceListVisitFromServerGetDeleteRecreateWaitApplicable.java
@@ -16,6 +16,12 @@
 
 package io.fabric8.kubernetes.client.dsl;
 
+/**
+ * @deprecated It is no longer necessary to associate parameters prior to deserialization.
+ *             <p>
+ *             reference {@link NamespaceListVisitFromServerGetDeleteRecreateWaitApplicable} instead
+ */
+@Deprecated
 public interface ParameterNamespaceListVisitFromServerGetDeleteRecreateWaitApplicable<T>
     extends NamespaceListVisitFromServerGetDeleteRecreateWaitApplicable<T>,
     Parameterizable<NamespaceListVisitFromServerGetDeleteRecreateWaitApplicable<T>> {

diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/dsl/Parameterizable.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/dsl/Parameterizable.java
@@ -17,6 +17,11 @@
 
 import java.util.Map;
 
+/**
+ * @deprecated It is no longer necessary to associate parameters prior to deserialization. Please
+ *             provide the parameters to one of the process methods instead
+ */
+@Deprecated
 public interface Parameterizable<T> {
 
   T withParameters(Map<String, String> parameters);

diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/Serialization.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/Serialization.java
@@ -35,7 +35,6 @@
 
 import java.io.BufferedInputStream;
 import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.lang.reflect.Type;
@@ -180,17 +179,13 @@ public static <T> T unmarshal(InputStream is) {
    * @param parameters A {@link Map} with parameters for placeholder substitution.
    * @param <T> The target type.
    * @return returns returns de-serialized object
+   *
+   * @deprecated please directly apply {@link Utils#interpolateString(String, Map)} instead of passing parameters here
    */
+  @Deprecated
   @SuppressWarnings("unchecked")
   public static <T> T unmarshal(InputStream is, Map<String, String> parameters) {
-    String specFile = readSpecFileFromInputStream(is);
-    if (containsMultipleDocuments(specFile)) {
-      return (T) getKubernetesResourceList(parameters, specFile);
-    } else if (specFile.contains(DOCUMENT_DELIMITER)) {
-      specFile = specFile.replaceAll("^---([ \\t].*?)?\\r?\\n", "");
-      specFile = specFile.replaceAll("\\n---([ \\t].*?)?\\r?\\n?$", "\n");
-    }
-    return unmarshal(new ByteArrayInputStream(specFile.getBytes()), JSON_MAPPER, parameters);
+    return unmarshal(is, JSON_MAPPER, parameters);
   }
 
   /**
@@ -217,9 +212,27 @@ public static <T> T unmarshal(InputStream is, ObjectMapper mapper) {
    * @param parameters A {@link Map} with parameters for placeholder substitution.
    * @param <T> The target type.
    * @return returns de-serialized object
+   *
+   * @deprecated please directly apply {@link Utils#interpolateString(String, Map)} instead of passing parameters here
    */
+  @Deprecated
   public static <T> T unmarshal(InputStream is, ObjectMapper mapper, Map<String, String> parameters) {
-    return unmarshal(is, mapper, new TypeReference<T>() {
+    // it's not well documented which Serialization methods are aware of input that can contain
+    // multiple docs
+    String specFile;
+    try {
+      specFile = IOHelpers.readFully(is);
+    } catch (IOException e1) {
+      throw new RuntimeException("Could not read stream");
+    }
+    if (containsMultipleDocuments(specFile)) {
+      return (T) getKubernetesResourceList(Collections.emptyMap(), specFile);
+    } else if (specFile.contains(DOCUMENT_DELIMITER)) {
+      specFile = specFile.replaceAll("^---([ \\t].*?)?\\r?\\n", "");
+      specFile = specFile.replaceAll("\\n---([ \\t].*?)?\\r?\\n?$", "\n");
+    }
+
+    return unmarshal(new ByteArrayInputStream(specFile.getBytes(StandardCharsets.UTF_8)), mapper, new TypeReference<T>() {
       @Override
       public Type getType() {
         return KubernetesResource.class;
@@ -228,9 +241,8 @@ public Type getType() {
   }
 
   private static <T> T unmarshal(InputStream is, ObjectMapper mapper, TypeReference<T> type, Map<String, String> parameters) {
-    try (
-        InputStream wrapped = parameters != null && !parameters.isEmpty() ? ReplaceValueStream.replaceValues(is, parameters)
-            : is;
+    try (InputStream wrapped = parameters != null && !parameters.isEmpty() ? ReplaceValueStream.replaceValues(is, parameters)
+        : is;
         BufferedInputStream bis = new BufferedInputStream(wrapped)) {
       bis.mark(-1);
       int intch;
@@ -296,7 +308,10 @@ public static <T> T unmarshal(String str, final Class<T> type) {
    * @param parameters A hashmap containing parameters
    *
    * @return returns de-serialized object
+   *
+   * @deprecated please directly apply {@link Utils#interpolateString(String, Map)} instead of passing parameters here
    */
+  @Deprecated
   public static <T> T unmarshal(String str, final Class<T> type, Map<String, String> parameters) {
     try (InputStream is = new ByteArrayInputStream(str.getBytes(StandardCharsets.UTF_8))) {
       return unmarshal(is, new TypeReference<T>() {
@@ -330,7 +345,10 @@ public static <T> T unmarshal(InputStream is, final Class<T> type) {
    * @param parameters A {@link Map} with parameters for placeholder substitution.
    * @param <T> Template argument denoting type
    * @return returns de-serialized object
+   *
+   * @deprecated please directly apply {@link Utils#interpolateString(String, Map)} instead of passing parameters here
    */
+  @Deprecated
   public static <T> T unmarshal(InputStream is, final Class<T> type, Map<String, String> parameters) {
     return unmarshal(is, new TypeReference<T>() {
       @Override
@@ -361,7 +379,10 @@ public static <T> T unmarshal(InputStream is, TypeReference<T> type) {
    * @param <T> Template argument denoting type
    *
    * @return returns de-serialized object
+   *
+   * @deprecated please directly apply {@link Utils#interpolateString(String, Map)} instead of passing parameters here
    */
+  @Deprecated
   public static <T> T unmarshal(InputStream is, TypeReference<T> type, Map<String, String> parameters) {
     return unmarshal(is, JSON_MAPPER, type, parameters);
   }
@@ -370,6 +391,7 @@ private static List<KubernetesResource> getKubernetesResourceList(Map<String, St
     return splitSpecFile(specFile).stream().filter(Serialization::validate)
         .map(
             document -> (KubernetesResource) Serialization.unmarshal(new ByteArrayInputStream(document.getBytes()), parameters))
+        .filter(o -> o != null)
         .collect(Collectors.toList());
   }
 
@@ -401,20 +423,6 @@ private static boolean validate(String document) {
     return !document.isEmpty() && keyValueMatcher.find();
   }
 
-  private static String readSpecFileFromInputStream(InputStream inputStream) {
-    ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
-    byte[] buffer = new byte[1024];
-    int length;
-    try {
-      while ((length = inputStream.read(buffer)) != -1) {
-        outputStream.write(buffer, 0, length);
-      }
-      return outputStream.toString();
-    } catch (IOException e) {
-      throw new RuntimeException("Unable to read InputStream." + e);
-    }
-  }
-
   /**
    * Create a copy of the resource via serialization.
    *

diff --git a/...est/java/io/fabric8/kubernetes/client/utils/SerializationSingleDocumentUnmarshalTest.java b/...est/java/io/fabric8/kubernetes/client/utils/SerializationSingleDocumentUnmarshalTest.java
@@ -21,8 +21,6 @@
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
 
-import java.util.Collections;
-
 import static org.assertj.core.api.Assertions.assertThat;
 
 class SerializationSingleDocumentUnmarshalTest {
@@ -36,8 +34,7 @@ class SerializationSingleDocumentUnmarshalTest {
   void unmarshalWithSingleDocumentWithDocumentDelimiterShouldReturnKubernetesResource(String arg) {
     // When
     final KubernetesResource result = Serialization.unmarshal(
-        SerializationTest.class.getResourceAsStream(String.format("/serialization/%s", arg)),
-        Collections.emptyMap());
+        SerializationTest.class.getResourceAsStream(String.format("/serialization/%s", arg)));
     // Then
     assertThat(result)
         .asInstanceOf(InstanceOfAssertFactories.type(Service.class))

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
@@ -411,7 +411,7 @@ public Type getType() {
           return refinedType;
         }
       };
-      CompletableFuture<L> futureAnswer = handleResponse(httpClient, requestBuilder, listTypeReference, getParameters());
+      CompletableFuture<L> futureAnswer = handleResponse(httpClient, requestBuilder, listTypeReference);
       return futureAnswer.thenApply(l -> {
         updateApiVersion(l);
         return l;

diff --git a/...nt/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java b/...nt/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java
@@ -20,7 +20,6 @@
 import io.fabric8.kubernetes.api.builder.Visitor;
 import io.fabric8.kubernetes.api.model.DeletionPropagation;
 import io.fabric8.kubernetes.api.model.HasMetadata;
-import io.fabric8.kubernetes.api.model.KubernetesList;
 import io.fabric8.kubernetes.api.model.KubernetesResourceList;
 import io.fabric8.kubernetes.api.model.StatusDetails;
 import io.fabric8.kubernetes.client.Client;
@@ -85,11 +84,6 @@ public List<HasMetadata> waitUntilReady(final long amount, final TimeUnit timeUn
   List<HasMetadata> getItems() {
     Object item = context.getItem();
 
-    if (item instanceof InputStream) {
-      item = Serialization.unmarshal((InputStream) item, Collections.emptyMap());
-      context = context.withItem(item); // late realization of the inputstream
-    }
-
     return asHasMetadata(item).stream()
         .map(meta -> acceptVisitors(meta,
             Collections.emptyList(), this.context))
@@ -255,17 +249,13 @@ protected Readiness getReadiness() {
 
   protected List<HasMetadata> asHasMetadata(Object item) {
     List<HasMetadata> result = new ArrayList<>();
-    if (item instanceof KubernetesList) {
-      result.addAll(((KubernetesList) item).getItems());
-    } else if (item instanceof KubernetesResourceList) {
+    if (item instanceof KubernetesResourceList) {
       result.addAll(((KubernetesResourceList) item).getItems());
     } else if (item instanceof HasMetadata) {
       result.add((HasMetadata) item);
-    } else if (item instanceof String) {
-      return asHasMetadata(Serialization.unmarshal((String) item));
     } else if (item instanceof Collection) {
       for (Object o : (Collection) item) {
-        if (o instanceof HasMetadata) {
+        if (o != null) {
           result.add((HasMetadata) o);
         }
       }

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
@@ -365,7 +365,7 @@ protected <T> T handleUpdate(T updated, Class<T> type, boolean status) throws IO
     HttpRequest.Builder requestBuilder = httpClient.newHttpRequestBuilder()
         .put(JSON, JSON_MAPPER.writeValueAsString(updated))
         .url(getResourceURLForWriteOperation(getResourceUrl(checkNamespace(updated), checkName(updated), status)));
-    return handleResponse(requestBuilder, type, getParameters());
+    return handleResponse(requestBuilder, type);
   }
 
   /**
@@ -481,11 +481,7 @@ protected Status handleDeploymentRollback(String resourceUrl, DeploymentRollback
    */
   protected <T> T handleGet(URL resourceUrl, Class<T> type) throws InterruptedException, IOException {
     HttpRequest.Builder requestBuilder = httpClient.newHttpRequestBuilder().url(resourceUrl);
-    return handleResponse(requestBuilder, type, getParameters());
-  }
-
-  protected Map<String, String> getParameters() {
-    return Collections.emptyMap();
+    return handleResponse(requestBuilder, type);
   }
 
   protected <T extends HasMetadata> T handleApproveOrDeny(T csr, Class<T> type) throws IOException, InterruptedException {
@@ -556,32 +552,15 @@ protected <T> T waitForResult(CompletableFuture<T> future) throws IOException {
    * @param <T> template argument provided
    *
    * @return Returns a de-serialized object as api server response of provided type.
-   * @throws InterruptedException Interrupted Exception
    * @throws IOException IOException
    */
-  protected <T> T handleResponse(HttpRequest.Builder requestBuilder, Class<T> type) throws InterruptedException, IOException {
-    return handleResponse(requestBuilder, type, getParameters());
-  }
-
-  /**
-   * Send an http request and handle the response, optionally performing placeholder substitution to the response.
-   *
-   * @param requestBuilder request builder
-   * @param type type of object
-   * @param parameters a hashmap containing parameters
-   * @param <T> template argument provided
-   *
-   * @return Returns a de-serialized object as api server response of provided type.
-   * @throws IOException IOException
-   */
-  private <T> T handleResponse(HttpRequest.Builder requestBuilder, Class<T> type, Map<String, String> parameters)
-      throws IOException {
+  protected <T> T handleResponse(HttpRequest.Builder requestBuilder, Class<T> type) throws IOException {
     return waitForResult(handleResponse(httpClient, requestBuilder, new TypeReference<T>() {
       @Override
       public Type getType() {
         return type;
       }
-    }, parameters));
+    }));
   }
 
   /**
@@ -590,14 +569,12 @@ public Type getType() {
    * @param client the client
    * @param requestBuilder Request builder
    * @param type Type of object provided
-   * @param parameters A hashmap containing parameters
    * @param <T> Template argument provided
    *
    * @return Returns a de-serialized object as api server response of provided type.
    */
   protected <T> CompletableFuture<T> handleResponse(HttpClient client, HttpRequest.Builder requestBuilder,
-      TypeReference<T> type,
-      Map<String, String> parameters) {
+      TypeReference<T> type) {
     VersionUsageUtils.log(this.resourceT, this.apiGroupVersion);
     HttpRequest request = requestBuilder.build();
     CompletableFuture<HttpResponse<byte[]>> futureResponse = new CompletableFuture<>();
@@ -609,7 +586,7 @@ protected <T> CompletableFuture<T> handleResponse(HttpClient client, HttpRequest
       try {
         assertResponseCode(request, response);
         if (type != null && type.getType() != null) {
-          return Serialization.unmarshal(new ByteArrayInputStream(response.body()), type, parameters);
+          return Serialization.unmarshal(new ByteArrayInputStream(response.body()), type);
         } else {
           return null;
         }
@@ -810,9 +787,6 @@ public <R1> R1 restCall(Class<R1> result, String... path) {
         throw e;
       }
       return null;
-    } catch (InterruptedException ie) {
-      Thread.currentThread().interrupt();
-      throw KubernetesClientException.launderThrowable(ie);
     } catch (IOException e) {
       throw KubernetesClientException.launderThrowable(e);
     }

diff --git a/...nt/src/main/java/io/fabric8/kubernetes/client/dsl/internal/core/v1/PodOperationsImpl.java b/...nt/src/main/java/io/fabric8/kubernetes/client/dsl/internal/core/v1/PodOperationsImpl.java
@@ -263,9 +263,6 @@ private boolean handleEvict(HasMetadata eviction) {
       return false;
     } catch (IOException exception) {
       throw KubernetesClientException.launderThrowable(forOperationType("evict"), exception);
-    } catch (InterruptedException interruptedException) {
-      Thread.currentThread().interrupt();
-      throw KubernetesClientException.launderThrowable(forOperationType("evict"), interruptedException);
     }
   }