Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #4530: allowing serialization to better handle primitive values #4533

Merged
merged 3 commits into from
Nov 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -286,18 +286,18 @@ MixedOperation<GenericKubernetesResource, GenericKubernetesResourceList, Resourc
NonNamespaceOperation<ComponentStatus, ComponentStatusList, Resource<ComponentStatus>> componentstatuses();

/**
* Load a Kubernetes resource object from file InputStream
* Load Kubernetes resource object(s) from the provided InputStream.
*
* @param is File input stream object containing json/yaml content
* @return deserialized object
* @param is the input stream containing JSON/YAML content
* @return an operation instance to work on the list of Kubernetes Resource objects
*/
ParameterNamespaceListVisitFromServerGetDeleteRecreateWaitApplicable<HasMetadata> load(InputStream is);

/**
* Load a Kubernetes list object
*
* @param s kubernetes list as string
* @return deserialized KubernetesList object
* @return an operation instance to work on the deserialized KubernetesList objects
*/
ParameterNamespaceListVisitFromServerGetDeleteRecreateWaitApplicable<HasMetadata> resourceList(String s);

Expand Down Expand Up @@ -340,11 +340,20 @@ NamespaceListVisitFromServerGetDeleteRecreateWaitApplicable<HasMetadata> resourc
* KubernetesResource operations. You can pass any Kubernetes resource as string object and do
* all operations
*
* @param s Kubernetes resource object as string
* @param s a Kubernetes resource object as string
* @return operations object for Kubernetes resource
*/
NamespaceableResource<HasMetadata> resource(String s);

/**
* KubernetesResource operations. You can pass any Kubernetes resource as an InputStream object and perform
* all operations
*
* @param is the InputStream containing a serialized Kubernetes resource.
* @return operations object for Kubernetes resource.
*/
NamespaceableResource<HasMetadata> resource(InputStream is);

/**
* Operations for Binding resource in APIgroup core/v1
*
Expand Down Expand Up @@ -514,7 +523,7 @@ NamespaceListVisitFromServerGetDeleteRecreateWaitApplicable<HasMetadata> resourc

/**
* Visit all resources with the given {@link ApiVisitor}.
*
*
* @param visitor
*/
void visitResources(ApiVisitor visitor);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,11 @@ public NamespaceableResource<HasMetadata> resource(String s) {
return getClient().resource(s);
}

@Override
public NamespaceableResource<HasMetadata> resource(InputStream is) {
return getClient().resource(is);
}

@Override
public MixedOperation<Binding, KubernetesResourceList<Binding>, Resource<Binding>> bindings() {
return getClient().bindings();
Expand Down
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -27,24 +27,28 @@

class SerializationSingleDocumentUnmarshalTest {
@ParameterizedTest(name = "#{index} - unmarshal {0}")
@ValueSource(strings = {"document-with-trailing-document-delimiter.yml", "document-with-leading-document-delimiter.yml", "document-with-leading-and-trailing-document-delimiter.yml", "document-with-no-document-delimiter.yml"})
@ValueSource(strings = {
"document-with-trailing-document-delimiter.yml",
"document-with-leading-document-delimiter.yml",
"document-with-leading-and-trailing-document-delimiter.yml",
"document-with-no-document-delimiter.yml"
})
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)),
Collections.emptyMap());
// Then
assertThat(result)
.asInstanceOf(InstanceOfAssertFactories.type(Service.class))
.hasFieldOrPropertyWithValue("apiVersion", "v1")
.hasFieldOrPropertyWithValue("kind", "Service")
.hasFieldOrPropertyWithValue("metadata.name", "redis-master")
.hasFieldOrPropertyWithValue("metadata.labels.app", "redis")
.hasFieldOrPropertyWithValue("metadata.labels.tier", "backend")
.hasFieldOrPropertyWithValue("metadata.labels.role", "master")
.hasFieldOrPropertyWithValue("spec.selector.app", "redis")
.hasFieldOrPropertyWithValue("spec.selector.tier", "backend")
.hasFieldOrPropertyWithValue("spec.selector.role", "master");
.asInstanceOf(InstanceOfAssertFactories.type(Service.class))
.hasFieldOrPropertyWithValue("apiVersion", "v1")
.hasFieldOrPropertyWithValue("kind", "Service")
.hasFieldOrPropertyWithValue("metadata.name", "redis-master")
.hasFieldOrPropertyWithValue("metadata.labels.app", "redis")
.hasFieldOrPropertyWithValue("metadata.labels.tier", "backend")
.hasFieldOrPropertyWithValue("metadata.labels.role", "master")
.hasFieldOrPropertyWithValue("spec.selector.app", "redis")
.hasFieldOrPropertyWithValue("spec.selector.tier", "backend")
.hasFieldOrPropertyWithValue("spec.selector.role", "master");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,16 @@
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;
import org.assertj.core.groups.Tuple;
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.io.File;
import java.io.IOException;
Expand All @@ -61,12 +65,15 @@
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;
import java.util.stream.Stream;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

class SerializationTest {

Expand Down Expand Up @@ -193,7 +200,7 @@ void testSerializeYamlWithAlias() {
}

@Test
void testClone() {
void cloneKubernetesResourceReturnsDifferentInstance() {
// Given
Pod pod = new PodBuilder().withNewMetadata().withName("pod").endMetadata().build();
// When
Expand All @@ -206,7 +213,7 @@ void testClone() {
}

@Test
void testCloneNonResource() {
void cloneNonResourceReturnsDifferentInstance() {
// Given
Map<String, String> value = Collections.singletonMap("key", "value");
// When
Expand Down Expand Up @@ -235,15 +242,69 @@ void unmarshalWithValidCustomResourceShouldReturnGenericCustomResource() {
}

@Test
@DisplayName("unmarshal, with invalid YAML content, should throw Exception")
void unmarshalWithInvalidYamlShouldThrowException() {
void unmarshalWithInvalidYamlShouldReturnRawExtension() {
// 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 unmarshalYamlArrayShouldThrowException() {
assertThatIllegalArgumentException()
.isThrownBy(() -> Serialization.unmarshal("- 1\n- 2"))
.withMessageStartingWith("Cannot parse a nested array containing non-object resource");
}

@Test
void unmarshalJsonArrayShouldThrowException() {
assertThatExceptionOfType(KubernetesClientException.class)
.isThrownBy(() -> Serialization.unmarshal("[1, 2]"))
.withMessage("An error has occurred.")
.havingCause()
.withMessageStartingWith("Cannot parse a nested array containing non-object resource");
}

@Test
void unmarshalYamlArrayWithProvidedTypeShouldDeserialize() {
// not valid as KubernetesResource - we'd have to look ahead to know if the array values
// were not hasmetadata
assertEquals(Arrays.asList(1, 2), Serialization.unmarshal("- 1\n- 2", List.class));
}

@Test
void unmarshalJsonArrayWithProvidedTypeShouldDeserialize() {
// not valid as KubernetesResource - we'd have to look ahead to know if the array values
// were not hasmetadata
assertEquals(Arrays.asList(1, 2), Serialization.unmarshal("[1, 2]", List.class));
}

@ParameterizedTest(name = "''{0}'' should be deserialized as ''{1}''")
@MethodSource("unmarshalPrimitivesInput")
void unmarshalPrimitives(String input, Object expected) {
assertThat(Serialization.<RawExtension> unmarshal(input))
.extracting(RawExtension::getValue)
.isEqualTo(expected);
assertEquals("a", Serialization.unmarshal("\"a\"", String.class));
assertEquals("a", Serialization.unmarshal("a", String.class));
}

@ParameterizedTest(name = "''{0}'' and ''{2}'' target type should be deserialized as ''{1}''")
@MethodSource("unmarshalPrimitivesInput")
void unmarshalPrimitivesWithType(String input, Object expected, Class<?> targetType) {
assertThat(Serialization.unmarshal(input, targetType))
.isEqualTo(expected);
}

static Stream<Arguments> unmarshalPrimitivesInput() {
return Stream.of(
Arguments.arguments("\"a\"", "a", String.class), // JSON
Arguments.arguments("a", "a", String.class), // YAML
Arguments.arguments("1", 1, Integer.class),
Arguments.arguments("true", true, Boolean.class),
Arguments.arguments("1.2", 1.2, Double.class));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,12 +357,28 @@ public <T extends HasMetadata> NamespaceableResource<T> resource(T item) {
return new NamespaceableResourceAdapter<>(item, op);
}

private NamespaceableResource<HasMetadata> resource(Object resource) {
if (resource instanceof HasMetadata) {
return resource((HasMetadata) resource);
}
throw new KubernetesClientException("Unable to create a valid resource from the provided object (" +
resource.getClass().getName() + ")");
}

/**
* {@inheritDoc}
*/
@Override
public NamespaceableResource<HasMetadata> resource(String s) {
return resource((HasMetadata) Serialization.unmarshal(s));
return resource(Serialization.<Object> unmarshal(s));
}

/**
* {@inheritDoc}
*/
@Override
public NamespaceableResource<HasMetadata> resource(InputStream is) {
return resource(Serialization.<Object> unmarshal(is));
}

/**
Expand Down
Loading