fix fabric8io#3972 load should not be overloaded

CHANGELOG.md

@@ -34,6 +34,7 @@
 * 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`
 * Fix #3973: removed support for Parameterizable - 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 #3973: openShiftClient.templates().load and openShiftClient.load will no longer automatically process / create templates.  Use the method openshiftClient.templateFrom as a helper to get a template from any stream.
 
 ### 6.2.0 (2022-10-20)
 

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/extended/leaderelection/resourcelock/ConfigMapLock.java

@@ -15,13 +15,11 @@
  */
 package io.fabric8.kubernetes.client.extended.leaderelection.resourcelock;
 
-import com.fasterxml.jackson.core.JsonProcessingException;
-import com.fasterxml.jackson.core.type.TypeReference;
-import com.fasterxml.jackson.databind.ObjectMapper;
 import io.fabric8.kubernetes.api.model.ConfigMap;
 import io.fabric8.kubernetes.api.model.ConfigMapBuilder;
 import io.fabric8.kubernetes.api.model.ObjectMeta;
 import io.fabric8.kubernetes.client.KubernetesClient;
+import io.fabric8.kubernetes.client.KubernetesClientException;
 import io.fabric8.kubernetes.client.utils.Serialization;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -36,13 +34,11 @@ public class ConfigMapLock implements Lock {
   private final String configMapNamespace;
   private final String configMapName;
   private final String identity;
-  private final ObjectMapper objectMapper;
 
   public ConfigMapLock(String configMapNamespace, String configMapName, String identity) {
     this.configMapNamespace = Objects.requireNonNull(configMapNamespace, "configMapNamespace is required");
     this.configMapName = Objects.requireNonNull(configMapName, "configMapName is required");
     this.identity = Objects.requireNonNull(identity, "identity is required");
-    objectMapper = Serialization.jsonMapper();
   }
 
   /**
@@ -58,9 +54,8 @@ public LeaderElectionRecord get(KubernetesClient client) {
         .map(annotations -> annotations.get(LEADER_ELECTION_RECORD_ANNOTATION_KEY))
         .map(annotation -> {
           try {
-            return objectMapper.readValue(annotation, new TypeReference<LeaderElectionRecord>() {
-            });
-          } catch (JsonProcessingException ex) {
+            return Serialization.unmarshal(annotation, LeaderElectionRecord.class);
+          } catch (KubernetesClientException ex) {
             LOGGER.error("Error deserializing LeaderElectionRecord from ConfigMap", ex);
             return null;
           }
@@ -80,11 +75,11 @@ public void create(
       KubernetesClient client, LeaderElectionRecord leaderElectionRecord) throws LockException {
 
     try {
-      client.configMaps().inNamespace(configMapNamespace).withName(configMapName).create(new ConfigMapBuilder()
+      client.configMaps().inNamespace(configMapNamespace).resource(new ConfigMapBuilder()
           .editOrNewMetadata().withNamespace(configMapNamespace).withName(configMapName)
-          .addToAnnotations(LEADER_ELECTION_RECORD_ANNOTATION_KEY, objectMapper.writeValueAsString(leaderElectionRecord))
+          .addToAnnotations(LEADER_ELECTION_RECORD_ANNOTATION_KEY, Serialization.asJson(leaderElectionRecord))
           .endMetadata()
-          .build());
+          .build()).create();
     } catch (Exception e) {
       throw new LockException("Unable to create ConfigMapLock", e);
     }
@@ -100,11 +95,11 @@ public void update(
     try {
       final ConfigMap toReplace = client.configMaps().inNamespace(configMapNamespace).withName(configMapName).get();
       toReplace.getMetadata().getAnnotations()
-          .put(LEADER_ELECTION_RECORD_ANNOTATION_KEY, objectMapper.writeValueAsString(leaderElectionRecord));
+          .put(LEADER_ELECTION_RECORD_ANNOTATION_KEY, Serialization.asJson(leaderElectionRecord));
       // Use replace instead of edit to avoid concurrent modifications, resourceVersion is locked to original record version
-      client.configMaps().inNamespace(configMapNamespace).withName(configMapName)
+      client.configMaps().inNamespace(configMapNamespace).resource(toReplace)
           .lockResourceVersion((String) Objects.requireNonNull(leaderElectionRecord.getVersion()))
-          .replace(toReplace);
+          .replace();
     } catch (Exception e) {
       throw new LockException("Unable to update ConfigMapLock", e);
     }

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/IOHelpers.java

@@ -75,7 +75,11 @@ public static boolean isJSONValid(String json) {
   }
 
   public static String convertYamlToJson(String yaml) throws IOException {
-    return Serialization.asJson(Serialization.unmarshal(yaml));
+    ObjectMapper yamlReader = Serialization.yamlMapper();
+    Object obj = yamlReader.readValue(yaml, Object.class);
+
+    ObjectMapper jsonWriter = Serialization.jsonMapper();
+    return jsonWriter.writeValueAsString(obj);
   }
 
   public static String convertToJson(String jsonOrYaml) throws IOException {

kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/extended/leaderelection/resourcelock/ConfigMapLockTest.java

@@ -21,13 +21,11 @@
 import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
 import io.fabric8.kubernetes.client.KubernetesClient;
 import io.fabric8.kubernetes.client.dsl.MixedOperation;
-import io.fabric8.kubernetes.client.dsl.ReplaceDeletable;
 import io.fabric8.kubernetes.client.dsl.Resource;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.function.Executable;
-import org.mockito.Answers;
 import org.mockito.ArgumentMatchers;
 import org.mockito.Mockito;
 
@@ -42,7 +40,6 @@
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
-import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
@@ -128,26 +125,25 @@ void createWithValidLeaderElectionRecordShouldSendPostRequest() throws Exception
     // When
     lock.create(kc, record);
     // Then
-    verify(configMaps.withName("name"), times(1)).create(any(ConfigMap.class));
+    verify(configMaps.resource(any(ConfigMap.class)), times(1)).create();
   }
 
   @Test
   void updateWithValidLeaderElectionRecordShouldSendPutRequest() throws Exception {
     // Given
     final Resource<ConfigMap> configMapResource = configMaps.withName("name");
-    final ReplaceDeletable<ConfigMap> replaceable = mock(ReplaceDeletable.class, Answers.RETURNS_DEEP_STUBS);
-    when(configMapResource.lockResourceVersion(any())).thenReturn(replaceable);
     final ConfigMap configMapInTheCluster = new ConfigMap();
     configMapInTheCluster.setMetadata(new ObjectMetaBuilder().withAnnotations(new HashMap<>()).build());
     when(configMapResource.get()).thenReturn(configMapInTheCluster);
     final LeaderElectionRecord record = new LeaderElectionRecord(
         "1337", Duration.ofSeconds(1), ZonedDateTime.now(), ZonedDateTime.now(), 0);
     record.setVersion("313373");
     final ConfigMapLock lock = new ConfigMapLock("namespace", "name", "1337");
+
     // When
     lock.update(kc, record);
     // Then
-    verify(replaceable, times(1)).replace(eq(configMapInTheCluster));
+    verify(configMaps.resource(any()).lockResourceVersion("313373")).replace();
   }
 
   @Test

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/OperationSupport.java

@@ -474,7 +474,6 @@ protected Status handleDeploymentRollback(String resourceUrl, DeploymentRollback
    * @param <T> template argument provided
    *
    * @return returns a deserialized object as api server response of provided type.
-   * @throws InterruptedException Interrupted Exception
    * @throws IOException IOException
    */
   protected <T> T handleGet(URL resourceUrl, Class<T> type) throws IOException {

kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/LeaderElectionTest.java

@@ -64,6 +64,7 @@ public void singleLeaderConfigMapLockUpdateTest() throws Exception {
     server.expect().get().withPath("/api/v1/namespaces/namespace/configmaps/name")
         .andReturn(200, new ConfigMapBuilder()
             .withNewMetadata()
+            .withName("name")
             .withResourceVersion("1")
             .withAnnotations(Collections.singletonMap(
                 "control-plane.alpha.kubernetes.io/leader",

kubernetes-tests/src/test/java/io/fabric8/openshift/client/server/mock/LoadAsTemplateTest.java

@@ -55,7 +55,7 @@ void tearDown() {
 
   @Test
   void shouldLoadPodAsTemplate() {
-    KubernetesList list = client.templates().load(getClass().getResourceAsStream("/test-pod-create-from-load.yml"))
+    KubernetesList list = client.templateFrom(getClass().getResourceAsStream("/test-pod-create-from-load.yml"))
         .processLocally();
     assertNotNull(list);
     assertNotNull(list.getItems());

kubernetes-tests/src/test/java/io/fabric8/openshift/client/server/mock/OpenShiftLoadTest.java

@@ -16,31 +16,35 @@
 package io.fabric8.openshift.client.server.mock;
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.fabric8.kubernetes.client.server.mock.EnableKubernetesMockClient;
+import io.fabric8.kubernetes.client.server.mock.KubernetesMockServer;
 import io.fabric8.openshift.client.OpenShiftClient;
 import org.junit.jupiter.api.Test;
 
+import java.util.Collections;
 import java.util.List;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 
-@EnableOpenShiftMockClient
+@EnableKubernetesMockClient
 class OpenShiftLoadTest {
 
-  OpenShiftMockServer server;
+  KubernetesMockServer server;
   OpenShiftClient client;
 
   @Test
   void testResourceGetFromLoadWhenSingleDocumentsWithStartingDelimiter() {
 
     // when
-    List<HasMetadata> result = client.load(getClass().getResourceAsStream("/test-template.yml")).get();
+    List<HasMetadata> result = client.templateFrom(getClass().getResourceAsStream("/test-template.yml"))
+        .processLocally(Collections.emptyMap()).getItems();
 
     // then
     assertNotNull(result);
     assertEquals(5, result.size());
     HasMetadata deploymentResource = result.get(2);
-    assertEquals("image.openshift.io/v1", deploymentResource.getApiVersion());
+    assertEquals("v1", deploymentResource.getApiVersion());
     assertEquals("ImageStream", deploymentResource.getKind());
     assertEquals("eap-app", deploymentResource.getMetadata().getName());
   }
@@ -49,7 +53,8 @@ void testResourceGetFromLoadWhenSingleDocumentsWithStartingDelimiter() {
   void testResourceGetFromLoadWhenSingleDocumentsWithoutDelimiter() {
 
     // when
-    List<HasMetadata> result = client.load(getClass().getResourceAsStream("/template-with-params.yml")).get();
+    List<HasMetadata> result = client.templateFrom(getClass().getResourceAsStream("/template-with-params.yml")).get()
+        .getObjects();
 
     // then
     assertNotNull(result);

openshift-client-api/src/main/java/io/fabric8/openshift/client/OpenShiftClient.java

@@ -16,12 +16,14 @@
 
 package io.fabric8.openshift.client;
 
+import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.api.model.KubernetesList;
 import io.fabric8.kubernetes.client.KubernetesClient;
 import io.fabric8.kubernetes.client.RequestConfig;
 import io.fabric8.kubernetes.client.VersionInfo;
 import io.fabric8.kubernetes.client.dsl.*;
 import io.fabric8.kubernetes.client.extension.SupportTestingClient;
+import io.fabric8.kubernetes.client.utils.Utils;
 import io.fabric8.openshift.api.model.*;
 import io.fabric8.openshift.api.model.miscellaneous.apiserver.v1.APIRequestCount;
 import io.fabric8.openshift.api.model.miscellaneous.apiserver.v1.APIRequestCountList;
@@ -37,7 +39,9 @@
 import io.fabric8.openshift.api.model.miscellaneous.network.operator.v1.OperatorPKIList;
 import io.fabric8.openshift.client.dsl.*;
 
+import java.io.InputStream;
 import java.net.URL;
+import java.util.List;
 
 public interface OpenShiftClient extends KubernetesClient, SupportTestingClient {
 
@@ -450,6 +454,35 @@ public interface OpenShiftClient extends KubernetesClient, SupportTestingClient
    */
   MixedOperation<Template, TemplateList, TemplateResource<Template, KubernetesList>> templates();
 
+  /**
+   * Create a template from the given stream.  If it is already a template, that will be returned unchanged.
+   * Any other HasMetadata will be returned as items in a Template with a randomly generated name.
+   *
+   * @param is
+   * @return the Template
+   */
+  default TemplateResource<Template, KubernetesList> templateFrom(InputStream is) {
+    Template template = null;
+    // TODO: should be item
+    List<HasMetadata> items = load(is).get();
+    Object item = items;
+    if (items.size() == 1) {
+      item = items.get(0);
+    }
+    if (item instanceof Template) {
+      template = (Template) item;
+    } else {
+      String generatedName = "template-" + Utils.randomString(5);
+      template = new TemplateBuilder()
+          .withNewMetadata()
+          .withName(generatedName)
+          .endMetadata()
+          .withObjects(items).build();
+    }
+
+    return templates().resource(template);
+  }
+
   /**
    * API entrypoint for TemplateInstance(template.openshift.io/v1)
    *

openshift-client/src/main/java/io/fabric8/openshift/client/dsl/internal/core/TemplateOperationsImpl.java

@@ -20,19 +20,16 @@
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.api.model.KubernetesList;
 import io.fabric8.kubernetes.api.model.KubernetesListBuilder;
-import io.fabric8.kubernetes.api.model.KubernetesResourceList;
 import io.fabric8.kubernetes.client.Client;
 import io.fabric8.kubernetes.client.KubernetesClientException;
 import io.fabric8.kubernetes.client.dsl.MixedOperation;
 import io.fabric8.kubernetes.client.dsl.internal.HasMetadataOperation;
 import io.fabric8.kubernetes.client.dsl.internal.HasMetadataOperationsImpl;
 import io.fabric8.kubernetes.client.dsl.internal.OperationContext;
 import io.fabric8.kubernetes.client.http.HttpRequest;
-import io.fabric8.kubernetes.client.utils.Serialization;
 import io.fabric8.kubernetes.client.utils.Utils;
 import io.fabric8.openshift.api.model.Parameter;
 import io.fabric8.openshift.api.model.Template;
-import io.fabric8.openshift.api.model.TemplateBuilder;
 import io.fabric8.openshift.api.model.TemplateList;
 import io.fabric8.openshift.client.ParameterValue;
 import io.fabric8.openshift.client.dsl.TemplateResource;
@@ -45,8 +42,6 @@
 import java.io.InputStream;
 import java.net.MalformedURLException;
 import java.net.URL;
-import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -202,49 +197,4 @@ private URL getProcessUrl() throws MalformedURLException {
     return getNamespacedUrl(getNamespace(), "processedtemplates");
   }
 
-  @Override
-  public TemplateResource<Template, KubernetesList> load(InputStream is) {
-    String generatedName = "template-" + Utils.randomString(5);
-    Template template = null;
-    Object item = Serialization.unmarshal(is);
-    if (item instanceof Template) {
-      template = (Template) item;
-    } else if (item instanceof HasMetadata) {
-      HasMetadata h = (HasMetadata) item;
-      template = new TemplateBuilder()
-          .withNewMetadata()
-          .withName(generatedName)
-          .withNamespace(h.getMetadata() != null ? h.getMetadata().getNamespace() : null)
-          .endMetadata()
-          .withObjects(h).build();
-    } else if (item instanceof KubernetesResourceList) {
-      List<HasMetadata> list = ((KubernetesResourceList<HasMetadata>) item).getItems();
-      template = new TemplateBuilder()
-          .withNewMetadata()
-          .withName(generatedName)
-          .endMetadata()
-          .withObjects(list.toArray(new HasMetadata[list.size()])).build();
-    } else if (item instanceof HasMetadata[]) {
-      template = new TemplateBuilder()
-          .withNewMetadata()
-          .withName(generatedName)
-          .endMetadata()
-          .withObjects((HasMetadata[]) item).build();
-    } else if (item instanceof Collection) {
-      List<HasMetadata> items = new ArrayList<>();
-      for (Object o : (Collection) item) {
-        if (o instanceof HasMetadata) {
-          items.add((HasMetadata) o);
-        }
-      }
-      template = new TemplateBuilder()
-          .withNewMetadata()
-          .withName(generatedName)
-          .endMetadata()
-          .withObjects(items.toArray(new HasMetadata[items.size()])).build();
-    }
-
-    return newInstance(context.withItem(template));
-  }
-
 }