Skip to content

Commit

Permalink
fix(model): octal numbers properly handled for file structs (mode, de…
Browse files Browse the repository at this point in the history
…faultMode) (6148)

fix (kubernetes-model-generator) : Handle deserialization of `defaultMode` field in custom deserializers for VolumeSource types

Add custom deserializers to handle correct octal deserialization for
these types:
- ConfigMapVolumeSource
- SecretVolumeSource
- DownwardAPIVolumeSource
- ProjectedVolumeSource

Signed-off-by: Rohan Kumar <[email protected]>
---
feat: Jackson module to handle Go (de)serialization nuances

Signed-off-by: Marc Nuri <[email protected]>
  • Loading branch information
rohanKanojia authored Jul 18, 2024
1 parent 541f1fa commit 9293e0a
Show file tree
Hide file tree
Showing 16 changed files with 565 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#### Bugs
* Fix #6038: Support for Gradle configuration cache
* Fix #6110: VolumeSource (and other file mode fields) in Octal are correctly interpreted

#### Improvements
* Fix #6008: removing the optional dependency on bouncy castle
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import io.fabric8.kubernetes.api.model.runtime.RawExtension;
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.kubernetes.internal.KubernetesDeserializer;
import io.fabric8.kubernetes.model.jackson.GoCompatibilityModule;
import io.fabric8.kubernetes.model.jackson.UnmatchedFieldTypeModule;
import org.snakeyaml.engine.v2.api.Dump;
import org.snakeyaml.engine.v2.api.DumpSettings;
Expand Down Expand Up @@ -89,7 +90,7 @@ public KubernetesSerialization(ObjectMapper mapper, boolean searchClassloaders)
}

protected void configureMapper(ObjectMapper mapper) {
mapper.registerModules(new JavaTimeModule(), unmatchedFieldTypeModule);
mapper.registerModules(new JavaTimeModule(), new GoCompatibilityModule(), unmatchedFieldTypeModule);
mapper.disable(DeserializationFeature.FAIL_ON_INVALID_SUBTYPE);
mapper.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS);
mapper.disable(SerializationFeature.WRITE_DURATIONS_AS_TIMESTAMPS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator;
import io.fabric8.kubernetes.api.model.KubernetesResource;
import io.fabric8.kubernetes.model.jackson.GoCompatibilityModule;
import io.fabric8.kubernetes.model.jackson.UnmatchedFieldTypeModule;

import java.io.InputStream;
Expand Down Expand Up @@ -88,7 +89,7 @@ public static ObjectMapper yamlMapper() {
if (YAML_MAPPER == null) {
YAML_MAPPER = new ObjectMapper(
new YAMLFactory().disable(YAMLGenerator.Feature.USE_NATIVE_TYPE_ID));
YAML_MAPPER.registerModules(UNMATCHED_FIELD_TYPE_MODULE);
YAML_MAPPER.registerModules(new GoCompatibilityModule(), UNMATCHED_FIELD_TYPE_MODULE);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,28 @@
import io.fabric8.kubernetes.api.model.AnyType;
import io.fabric8.kubernetes.api.model.Config;
import io.fabric8.kubernetes.api.model.ConfigMap;
import io.fabric8.kubernetes.api.model.ConfigMapVolumeSource;
import io.fabric8.kubernetes.api.model.GenericKubernetesResource;
import io.fabric8.kubernetes.api.model.KeyToPath;
import io.fabric8.kubernetes.api.model.KubernetesList;
import io.fabric8.kubernetes.api.model.KubernetesResource;
import io.fabric8.kubernetes.api.model.Namespace;
import io.fabric8.kubernetes.api.model.ObjectMeta;
import io.fabric8.kubernetes.api.model.Pod;
import io.fabric8.kubernetes.api.model.PodBuilder;
import io.fabric8.kubernetes.api.model.PodSpec;
import io.fabric8.kubernetes.api.model.PodTemplateSpec;
import io.fabric8.kubernetes.api.model.Quantity;
import io.fabric8.kubernetes.api.model.Service;
import io.fabric8.kubernetes.api.model.Toleration;
import io.fabric8.kubernetes.api.model.Volume;
import io.fabric8.kubernetes.api.model.apiextensions.v1beta1.CustomResourceDefinition;
import io.fabric8.kubernetes.api.model.apiextensions.v1beta1.JSONSchemaProps;
import io.fabric8.kubernetes.api.model.apps.Deployment;
import io.fabric8.kubernetes.api.model.batch.v1.CronJob;
import io.fabric8.kubernetes.api.model.batch.v1.CronJobSpec;
import io.fabric8.kubernetes.api.model.batch.v1.JobSpec;
import io.fabric8.kubernetes.api.model.batch.v1.JobTemplateSpec;
import io.fabric8.kubernetes.api.model.coordination.v1.Lease;
import io.fabric8.kubernetes.api.model.coordination.v1.LeaseSpec;
import io.fabric8.kubernetes.api.model.runtime.RawExtension;
Expand Down Expand Up @@ -310,6 +318,33 @@ void unmarshalWithValidListShouldReturnKubernetesList() {
new Tuple(Pod.class, "v1", "Pod", "a-pod"));
}

@Test
@DisplayName("unmarshal, when integer value has octal literal value, then octal literal value correctly parsed")
void unmarshal_whenIntegerValueHasOctalLiteralValue_thenCorrectlyDeserializeInteger() {
// When
final CronJob cronJob = Serialization.unmarshal(getClass().getResourceAsStream("/serialization/cronjob-octal.yml"),
CronJob.class);
// Then
assertThat(cronJob)
.extracting(CronJob::getSpec)
.extracting(CronJobSpec::getJobTemplate)
.extracting(JobTemplateSpec::getSpec)
.extracting(JobSpec::getTemplate)
.extracting(PodTemplateSpec::getSpec)
.extracting(PodSpec::getVolumes)
.asInstanceOf(InstanceOfAssertFactories.list(Volume.class))
.singleElement()
.extracting(Volume::getConfigMap)
.hasFieldOrPropertyWithValue("defaultMode", Integer.valueOf("0555", 8))
.hasFieldOrPropertyWithValue("name", "conf")
.extracting(ConfigMapVolumeSource::getItems)
.asInstanceOf(InstanceOfAssertFactories.list(KeyToPath.class))
.singleElement()
.hasFieldOrPropertyWithValue("key", "key1")
.hasFieldOrPropertyWithValue("path", "target")
.hasFieldOrPropertyWithValue("mode", Integer.valueOf("0555", 8));
}

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.EXISTING_PROPERTY, property = "type")
@JsonSubTypes(@JsonSubTypes.Type(value = Typed.class, name = "x"))
public interface Typeable {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#
# 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.
#

apiVersion: batch/v1beta1
kind: CronJob
metadata:
name: update-db
spec:
schedule: "*/1 * * * *"
jobTemplate:
spec:
template:
spec:
containers:
- name: update-fingerprints
image: python:3.6.2-slim
command: ["/bin/bash"]
args: ["-c", "python /client/test.py"]
volumeMounts:
- name: application-code
mountPath: /where/ever
restartPolicy: OnFailure
volumes:
- name: application-code
configMap:
name: conf
defaultMode: 0o555
items:
- key: key1
path: target
mode: 0555
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* 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.model.jackson;

import com.fasterxml.jackson.databind.module.SimpleModule;

public class GoCompatibilityModule extends SimpleModule {

public GoCompatibilityModule() {
addDeserializer(Integer.class, new GoIntegerDeserializer());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* 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.model.jackson;

import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.databind.BeanProperty;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.deser.ContextualDeserializer;
import com.fasterxml.jackson.databind.deser.std.StdDeserializer;

import java.io.IOException;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class GoIntegerDeserializer extends StdDeserializer<Integer> implements ContextualDeserializer {

private static final Pattern OCTAL = Pattern.compile("(0[oO]?)([0-7]+)");
private static final GoIntegerDeserializer APPLICABLE_INSTANCE = new GoIntegerDeserializer(true);
private static final Set<String> APPLICABLE_FIELDS = new HashSet<>(Arrays.asList("mode", "defaultMode"));

private final boolean applicable;

protected GoIntegerDeserializer() {
this(false);
}

private GoIntegerDeserializer(boolean applicable) {
super(Integer.class);
this.applicable = applicable;
}

@Override
public Integer deserialize(JsonParser p, DeserializationContext ctxt) throws IOException {
final String value = p.getText();
if (value == null) {
return null;
}
if (applicable) {
final Matcher matcher = OCTAL.matcher(value);
if (matcher.find()) {
return Integer.valueOf(matcher.group(2), 8);
}
}
return _parseInteger(ctxt, value);
}

@Override
public JsonDeserializer<?> createContextual(DeserializationContext ctxt, BeanProperty property) {
if (property != null && APPLICABLE_FIELDS.contains(property.getName())) {
return APPLICABLE_INSTANCE;
}
return this;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* 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.model.jackson;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.exc.InvalidFormatException;
import lombok.Data;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import java.util.stream.Stream;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS;

class GoIntegerDeserializerTest {

private ObjectMapper context;

@BeforeEach
void setUp() {
context = new ObjectMapper();
context.registerModule(new GoCompatibilityModule());
}

@Nested
@TestInstance(PER_CLASS)
class Applicable {

@ParameterizedTest(name = "{index}: with '{'\"{0}\": {1}'}' parses as {2}")
@MethodSource
void parsesOctals(String fieldName, String content, Integer expected) throws Exception {
final IntegerFieldsContainer result = context
.readValue(String.format("{\"%s\": %s}", fieldName, content), IntegerFieldsContainer.class);
assertThat(result).hasFieldOrPropertyWithValue(fieldName, expected);
}

private Stream<Arguments> parsesOctals() {
return Stream.of("mode", "defaultMode")
.flatMap(field -> Stream.of(
Arguments.of(field, "null", null),
Arguments.of(field, "\"0555\"", 365),
Arguments.of(field, "\"0o555\"", 365),
Arguments.of(field, "\"0O555\"", 365),
Arguments.of(field, "\"555\"", 555),
Arguments.of(field, "\"0888\"", 888),
Arguments.of(field, "\"0o12\"", 10),
Arguments.of(field, "\"0O12\"", 10)));
}
}

//
@Nested
class NotApplicable {

@Test
void parsesOctalsAsDecimal() throws Exception {
final IntegerFieldsContainer result = context
.readValue("{\"notApplicable\": \"0555\"}", IntegerFieldsContainer.class);
assertThat(result).hasFieldOrPropertyWithValue("notApplicable", 555);
}

@Test
void throwsExceptionForInvalidOctal() {
assertThatThrownBy(() -> context.readValue("{\"mode\": \"0o955\"}", IntegerFieldsContainer.class))
.isInstanceOf(InvalidFormatException.class);
}

@Test
void throwsExceptionForOctalWithSeparator() {
assertThatThrownBy(() -> context.readValue("{\"notApplicable\": \"0o555\"}", IntegerFieldsContainer.class))
.isInstanceOf(InvalidFormatException.class);
}
}

@Data
private static final class IntegerFieldsContainer {
private Integer mode;
private Integer defaultMode;
private Integer notApplicable;
}
}
Loading

0 comments on commit 9293e0a

Please sign in to comment.