Skip to content

Commit

Permalink
fix #4530: allowing serialization to better handle primitive values
Browse files Browse the repository at this point in the history
  • Loading branch information
shawkins authored and manusa committed Nov 11, 2022
1 parent 3167a41 commit e5e26f9
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -11,6 +11,7 @@

#### Improvements
* Fix #4355: for exec, attach, upload, and copy operations the container id/name will be validated or chosen prior to the remote call. You may also use the kubectl.kubernetes.io/default-container annotation to specify the default container.
* Fix #4530: generalizing the Serialization logic to allow for primitive values and clarifying the type expectations.

#### Dependency Upgrade

Expand Down
Expand Up @@ -23,6 +23,7 @@
import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator;
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
import io.fabric8.kubernetes.api.model.KubernetesResource;
import io.fabric8.kubernetes.api.model.runtime.RawExtension;
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.kubernetes.model.jackson.UnmatchedFieldTypeModule;
import org.yaml.snakeyaml.DumperOptions;
Expand Down Expand Up @@ -158,6 +159,8 @@ public static <T> String asYaml(T object) {

/**
* Unmarshals a stream.
* <p>
* The type is assumed to be {@link KubernetesResource}
*
* @param is The {@link InputStream}.
* @param <T> The target type.
Expand All @@ -170,6 +173,8 @@ public static <T> T unmarshal(InputStream is) {

/**
* Unmarshals a stream optionally performing placeholder substitution to the stream.
* <p>
* The type is assumed to be {@link KubernetesResource}
*
* @param is The {@link InputStream}.
* @param parameters A {@link Map} with parameters for placeholder substitution.
Expand All @@ -190,6 +195,8 @@ public static <T> T unmarshal(InputStream is, Map<String, String> parameters) {

/**
* Unmarshals a stream.
* <p>
* The type is assumed to be {@link KubernetesResource}
*
* @param is The {@link InputStream}.
* @param mapper The {@link ObjectMapper} to use.
Expand All @@ -202,6 +209,8 @@ public static <T> T unmarshal(InputStream is, ObjectMapper mapper) {

/**
* Unmarshals a stream optionally performing placeholder substitution to the stream.
* <p>
* The type is assumed to be {@link KubernetesResource}
*
* @param is The {@link InputStream}.
* @param mapper The {@link ObjectMapper} to use.
Expand Down Expand Up @@ -231,11 +240,15 @@ private static <T> T unmarshal(InputStream is, ObjectMapper mapper, TypeReferenc
bis.reset();

final T result;
if (intch != '{') {
if (intch != '{' && intch != '[') {
final Yaml yaml = new Yaml(new SafeConstructor(), new Representer(), new DumperOptions(),
new CustomYamlTagResolverWithLimit());
final Map<String, Object> obj = yaml.load(bis);
result = mapper.convertValue(obj, type);
final Object obj = yaml.load(bis);
if (obj instanceof Map) {
result = mapper.convertValue(obj, type);
} else {
result = mapper.convertValue(new RawExtension(obj), type);
}
} else {
result = mapper.readerFor(type).readValue(bis);
}
Expand All @@ -247,6 +260,8 @@ private static <T> T unmarshal(InputStream is, ObjectMapper mapper, TypeReferenc

/**
* Unmarshals a {@link String}
* <p>
* The type is assumed to be {@link KubernetesResource}
*
* @param str The {@link String}.
* @param <T> template argument denoting type
Expand Down
Expand Up @@ -45,6 +45,7 @@
import io.fabric8.kubernetes.api.model.coordination.v1.LeaseSpec;
import io.fabric8.kubernetes.api.model.runtime.RawExtension;
import io.fabric8.kubernetes.client.CustomResource;
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.kubernetes.model.annotation.Group;
import io.fabric8.kubernetes.model.annotation.Version;
import org.assertj.core.api.InstanceOfAssertFactories;
Expand All @@ -61,6 +62,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -235,15 +237,37 @@ void unmarshalWithValidCustomResourceShouldReturnGenericCustomResource() {
}

@Test
@DisplayName("unmarshal, with invalid YAML content, should throw Exception")
void unmarshalWithInvalidYamlShouldThrowException() {
// Given
final InputStream is = SerializationTest.class.getResourceAsStream("/serialization/invalid-yaml.yml");
// When
final ClassCastException result = assertThrows(ClassCastException.class, () -> Serialization.unmarshal(is));
RawExtension result = Serialization.unmarshal(is);
// Then
assertThat(result)
.hasMessageContainingAll("java.lang.String", "cannot be cast to", "java.util.Map");
assertEquals("This\nis not\nYAML", result.getValue());
}

@Test
void unmarshalArrays() {
// not valid as KubernetesResource - we'd have to look ahead to know if the array values
// were not hasmetadata
assertThrows(KubernetesClientException.class, () -> Serialization.unmarshal("[1, 2]"));
assertThrows(IllegalArgumentException.class, () -> Serialization.unmarshal("- 1\n- 2"));

assertEquals(Arrays.asList(1, 2), Serialization.unmarshal("[1, 2]", List.class));
assertEquals(Arrays.asList(1, 2), Serialization.unmarshal("- 1\n- 2", List.class));
}

@Test
void unmarshalPrimitives() {
// as json
RawExtension raw = Serialization.unmarshal("\"a\"");
assertEquals("a", raw.getValue());
// as yaml
raw = Serialization.unmarshal("a");
assertEquals("a", raw.getValue());

assertEquals("a", Serialization.unmarshal("\"a\"", String.class));
assertEquals("a", Serialization.unmarshal("a", String.class));
}

@Test
Expand Down
Expand Up @@ -21,7 +21,9 @@
import io.fabric8.kubernetes.api.model.AnyType;
import io.fabric8.kubernetes.api.model.KubernetesResource;
import io.sundr.builder.annotations.Buildable;
import lombok.ToString;

@ToString(callSuper = true)
@JsonDeserialize(using = com.fasterxml.jackson.databind.JsonDeserializer.None.class)
@Buildable(editableEnabled = false, validationEnabled = false, generateBuilderPackage = true, lazyCollectionInitEnabled = false, builderPackage = "io.fabric8.kubernetes.api.builder")
public class RawExtension extends AnyType implements KubernetesResource {
Expand Down
Expand Up @@ -89,9 +89,12 @@ public KubernetesResource deserialize(JsonParser jp, DeserializationContext ctxt
return fromObjectNode(jp, node);
} else if (node.isArray()) {
return fromArrayNode(jp, node);
} else {
}
Object object = node.traverse(jp.getCodec()).readValueAs(Object.class);
if (object == null) {
return null;
}
return new RawExtension(object);
}

private KubernetesResource fromArrayNode(JsonParser jp, JsonNode node) throws IOException {
Expand All @@ -105,6 +108,8 @@ private KubernetesResource fromArrayNode(JsonParser jp, JsonNode node) throws IO
throw new JsonMappingException(jp, "Cannot parse a nested array containing a non-HasMetadata resource");
}
list.add((HasMetadata) resource);
} else {
throw new JsonMappingException(jp, "Cannot parse a nested array containing non-object resource");
}
}
return new KubernetesListBuilder().withItems(list).build();
Expand Down

0 comments on commit e5e26f9

Please sign in to comment.