Skip to content

Commit

Permalink
feat: Serialization mappers use UnmatchedFieldTypeModule
Browse files Browse the repository at this point in the history
- Reverted KubernetesDeserializer that fall-backed to a
GenericKubernetesResource to deal with InvalidFormatException
in Templates
  • Loading branch information
manusa committed Nov 8, 2021
1 parent 5a76fb9 commit fd331b8
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 30 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#### Improvements
* Fix #3562: Kubernetes Mock Server improvements
* Fix #3574: support for deserialization of properties that don't match the target field's type

#### Dependency Upgrade
* Fix #3562: Bump MockWebServer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,24 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import io.fabric8.kubernetes.client.utils.serialization.UnmatchedFieldTypeModule;
import org.yaml.snakeyaml.Yaml;

public class Serialization {
private Serialization() { }

private static final ObjectMapper JSON_MAPPER = new ObjectMapper();
static {
JSON_MAPPER.registerModule(new JavaTimeModule());
JSON_MAPPER.registerModules(new JavaTimeModule(), new UnmatchedFieldTypeModule());
}

private static final ObjectMapper YAML_MAPPER = new ObjectMapper(
new YAMLFactory().disable(YAMLGenerator.Feature.USE_NATIVE_TYPE_ID)
);
static {
YAML_MAPPER.registerModules(new UnmatchedFieldTypeModule());
}

private static final String DOCUMENT_DELIMITER = "---";

Expand Down Expand Up @@ -144,7 +149,7 @@ public static <T> T unmarshal(InputStream is, ObjectMapper mapper, Map<String, S
throw KubernetesClientException.launderThrowable(e);
}
}

/**
* Unmarshals a {@link String}
* @param str The {@link String}.
Expand Down Expand Up @@ -326,7 +331,7 @@ private static <T> T unmarshalJsonStr(String jsonString, TypeReference<T> type)
}
return JSON_MAPPER.readerFor(KubernetesResource.class).readValue(jsonString);
}

/**
* Create a copy of the resource via serialization.
* @return a deep clone of the resource
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
/**
* Copyright (C) 2015 Red Hat, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.fabric8.kubernetes.client.utils;

import io.fabric8.kubernetes.api.model.ConfigMap;
import io.fabric8.kubernetes.api.model.ConfigMapBuilder;
import io.fabric8.kubernetes.api.model.KubernetesResource;
import io.fabric8.kubernetes.api.model.apps.Deployment;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;

class SerializationAdditionalPropertiesTest {

@Test
@DisplayName("unmarshal, with unknown fields, values are set in additionalProperties map")
void unmarshalWithUnknownFields() {
// Given
final String marshalled =
"{\"kind\": \"ConfigMap\"," +
"\"apiVersion\": \"v1\"," +
"\"metadata\":{\"name\":\"the-name\"}," +
"\"data\":{\"key\":\"value\"}," +
"\"unknownField\":\"unknownValue\"}";
// When
final KubernetesResource result = Serialization.unmarshal(marshalled);
// Then
assertThat(result)
.isInstanceOf(ConfigMap.class)
.hasFieldOrPropertyWithValue("metadata.name", "the-name")
.hasFieldOrPropertyWithValue("data.key", "value")
.hasFieldOrPropertyWithValue("additionalProperties.unknownField", "unknownValue");
}

@Test
@DisplayName("unmarshal, with unmatched type fields, values are set in additionalProperties map")
void unmarshalWithUnmatchedTypeFields() {
// Given
final String marshalled =
"{\"kind\": \"ConfigMap\"," +
"\"apiVersion\": \"v1\"," +
"\"metadata\":{\"name\":\"the-name\"}," +
"\"data\":{\"key\":\"value\"}," +
"\"unknownField\":\"unknownValue\"," +
"\"immutable\":\"${immutable}\"}";
// When
final KubernetesResource result = Serialization.unmarshal(marshalled);
// Then
assertThat(result)
.isInstanceOf(ConfigMap.class)
.hasFieldOrPropertyWithValue("metadata.name", "the-name")
.hasFieldOrPropertyWithValue("immutable", null)
.hasFieldOrPropertyWithValue("additionalProperties.unknownField", "unknownValue")
.hasFieldOrPropertyWithValue("additionalProperties.immutable", "${immutable}");
}

@Test
@DisplayName("unmarshal, with unmatched type nested fields, values are set in additionalProperties map")
void unmarshalWithUnmatchedTypeNestedFields() {
// Given
final String marshalled =
"{\"kind\": \"Deployment\"," +
"\"apiVersion\": \"apps/v1\"," +
"\"metadata\":{\"name\":\"deployment\", \"annotations\": \"${annotations}\"}," +
"\"spec\":{\"replicas\":\"${replicas}\",\"paused\":true}," +
"\"unknownField\":\"unknownValue\"}";
// When
final KubernetesResource result = Serialization.unmarshal(marshalled);
// Then
assertThat(result)
.isInstanceOf(Deployment.class)
.hasFieldOrPropertyWithValue("metadata.name", "deployment")
.hasFieldOrPropertyWithValue("metadata.annotations", null)
.hasFieldOrPropertyWithValue("metadata.additionalProperties.annotations", "${annotations}")
.hasFieldOrPropertyWithValue("spec.paused", true)
.hasFieldOrPropertyWithValue("spec.replicas", null)
.hasFieldOrPropertyWithValue("spec.additionalProperties.replicas", "${replicas}")
.hasFieldOrPropertyWithValue("additionalProperties.unknownField", "unknownValue");
}

@Test
@DisplayName("marshal, with unmatched type fields, values are set in additionalProperties map")
void marshalWithAdditionalPropertiesOverridingFields() {
// Given
final ConfigMap configMap = new ConfigMapBuilder()
.withNewMetadata().withName("name").addToAnnotations("key", "value").endMetadata()
.withImmutable(true)
.build();
configMap.getAdditionalProperties().put("unknownField", "unknownValue");
configMap.getAdditionalProperties().put("immutable", "${immutable}");
configMap.getMetadata().getAdditionalProperties().put("annotations", "${annotations}");
// When
final String result = Serialization.asJson(configMap);
// Then
assertThat(result).isEqualTo("{" +
"\"apiVersion\":\"v1\"," +
"\"kind\":\"ConfigMap\"," +
"\"metadata\":{\"name\":\"name\",\"annotations\":\"${annotations}\"}," +
"\"immutable\":\"${immutable}\"," +
"\"unknownField\":\"unknownValue\"" +
"}");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ void readValueWithNoAnySetterShouldThrowException() {

@Test
@DisplayName("writeValueAsString, with unmatched type fields, values are set in additionalProperties map")
void marshalWithAdditionalPropertiesOverridingFields() throws JsonProcessingException {
void writeValueAsStringWithAdditionalPropertiesOverridingFields() throws JsonProcessingException {
// Given
final ConfigMap configMap = new ConfigMapBuilder()
.withNewMetadata().withName("name").addToAnnotations("key", "ignored").addToLabels("lKey", "value").endMetadata()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.exc.InvalidFormatException;
import io.fabric8.kubernetes.api.KubernetesResourceMappingProvider;
import io.fabric8.kubernetes.api.model.GenericKubernetesResource;
import io.fabric8.kubernetes.api.model.HasMetadata;
Expand All @@ -50,9 +49,6 @@ public class KubernetesDeserializer extends JsonDeserializer<KubernetesResource>

private static final Mapping mapping = new Mapping();

// template deserialization can fail if unless we use generic resources
private static ThreadLocal<Boolean> IN_TEMPLATE = ThreadLocal.withInitial(() -> false);

@Override
public KubernetesResource deserialize(JsonParser jp, DeserializationContext ctxt) throws IOException {
JsonNode node = jp.readValueAsTree();
Expand Down Expand Up @@ -87,23 +83,7 @@ private static KubernetesResource fromObjectNode(JsonParser jp, JsonNode node) t
if (resourceType == null) {
return jp.getCodec().treeToValue(node, GenericKubernetesResource.class);
} else if (KubernetesResource.class.isAssignableFrom(resourceType)){
boolean inTemplate = IN_TEMPLATE.get();
if (!inTemplate && "io.fabric8.openshift.api.model.Template".equals(resourceType.getName())) {
inTemplate = true;
IN_TEMPLATE.set(true);
}
try {
return jp.getCodec().treeToValue(node, resourceType);
} catch (InvalidFormatException e) {
if (!inTemplate) {
throw e;
}
return jp.getCodec().treeToValue(node, GenericKubernetesResource.class);
} finally {
if (inTemplate) {
IN_TEMPLATE.remove();
}
}
return jp.getCodec().treeToValue(node, resourceType);
}
}
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package io.fabric8.openshift.client.server.mock;

import io.fabric8.kubernetes.api.model.APIGroupListBuilder;
import io.fabric8.kubernetes.api.model.GenericKubernetesResource;
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.api.model.KubernetesList;
import io.fabric8.kubernetes.api.model.KubernetesListBuilder;
Expand All @@ -27,7 +26,6 @@
import io.fabric8.kubernetes.api.model.ServiceSpec;
import io.fabric8.kubernetes.api.model.StatusBuilder;
import io.fabric8.kubernetes.client.utils.IOHelpers;
import io.fabric8.kubernetes.client.utils.Serialization;
import io.fabric8.openshift.api.model.ParameterBuilder;
import io.fabric8.openshift.api.model.Template;
import io.fabric8.openshift.api.model.TemplateBuilder;
Expand Down Expand Up @@ -199,9 +197,9 @@ void shouldGetTemplateWithNumberParameters() {
assertThat(template)
.extracting(Template::getObjects).asList()
.singleElement()
.isInstanceOf(GenericKubernetesResource.class)
.extracting("additionalProperties.spec.ports").asList().singleElement()
.hasFieldOrPropertyWithValue("port", "${PORT}");
.isInstanceOf(Service.class)
.extracting("spec.ports").asList().singleElement()
.hasFieldOrPropertyWithValue("additionalProperties.port", "${PORT}");
}

protected void assertListIsServiceWithPort8080(KubernetesList list) {
Expand Down

0 comments on commit fd331b8

Please sign in to comment.