Skip to content

Commit 4017ba2

Browse files
DohbedohDohbedoh
authored
[JENKINS-71956] Switching YAML serializer to Jackson to support Octal values (#1537)
1 parent 40d4fb2 commit 4017ba2

File tree

3 files changed

+84
-119
lines changed

3 files changed

+84
-119
lines changed

‎src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java

+22-110
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import io.fabric8.kubernetes.api.model.ContainerBuilder;
1919
import io.fabric8.kubernetes.api.model.EnvFromSource;
2020
import io.fabric8.kubernetes.api.model.EnvVar;
21-
import io.fabric8.kubernetes.api.model.KeyToPath;
2221
import io.fabric8.kubernetes.api.model.LocalObjectReference;
2322
import io.fabric8.kubernetes.api.model.ObjectMeta;
2423
import io.fabric8.kubernetes.api.model.Pod;
@@ -29,10 +28,6 @@
2928
import io.fabric8.kubernetes.api.model.Toleration;
3029
import io.fabric8.kubernetes.api.model.Volume;
3130
import io.fabric8.kubernetes.api.model.VolumeMount;
32-
import io.fabric8.kubernetes.client.KubernetesClient;
33-
import io.fabric8.kubernetes.client.KubernetesClientBuilder;
34-
import io.fabric8.kubernetes.client.KubernetesClientException;
35-
import io.fabric8.kubernetes.client.utils.Serialization;
3631
import java.io.ByteArrayInputStream;
3732
import java.io.IOException;
3833
import java.io.InputStream;
@@ -70,14 +65,6 @@ public class PodTemplateUtils {
7065
@SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "tests & emergency admin")
7166
public static boolean SUBSTITUTE_ENV = Boolean.getBoolean(PodTemplateUtils.class.getName() + ".SUBSTITUTE_ENV");
7267

73-
/**
74-
* If true, all modes permissions provided to pods are expected to be provided in decimal notation.
75-
* Otherwise, the plugin will consider they are written in octal notation.
76-
*/
77-
@SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "tests & emergency admin")
78-
public static /* almost final*/ boolean DISABLE_OCTAL_MODES =
79-
Boolean.getBoolean(PodTemplateUtils.class.getName() + ".DISABLE_OCTAL_MODES");
80-
8168
/**
8269
* Combines a {@link ContainerTemplate} with its parent.
8370
* @param parent The parent container template (nullable).
@@ -287,8 +274,8 @@ public static Pod combine(Pod parent, Pod template) {
287274
return template;
288275
}
289276

290-
LOGGER.finest(() -> "Combining pods, parent: " + Serialization.asYaml(parent) + " template: "
291-
+ Serialization.asYaml(template));
277+
LOGGER.finest(() -> "Combining pods, parent: " + Serialization2.asYaml(parent) + " template: "
278+
+ Serialization2.asYaml(template));
292279

293280
Map<String, String> nodeSelector =
294281
mergeMaps(parent.getSpec().getNodeSelector(), template.getSpec().getNodeSelector());
@@ -412,7 +399,7 @@ public static Pod combine(Pod parent, Pod template) {
412399
// podTemplate.setYaml(template.getYaml() == null ? parent.getYaml() : template.getYaml());
413400

414401
Pod pod = specBuilder.endSpec().build();
415-
LOGGER.finest(() -> "Pods combined: " + Serialization.asYaml(pod));
402+
LOGGER.finest(() -> "Pods combined: " + Serialization2.asYaml(pod));
416403
return pod;
417404
}
418405

@@ -699,102 +686,27 @@ public static String substitute(String s, Map<String, String> properties, String
699686

700687
public static Pod parseFromYaml(String yaml) {
701688
String s = yaml;
702-
try (KubernetesClient client = new KubernetesClientBuilder().build()) {
703-
// JENKINS-57116
704-
if (StringUtils.isBlank(s)) {
705-
LOGGER.log(Level.WARNING, "[JENKINS-57116] Trying to parse invalid yaml: \"{0}\"", yaml);
706-
s = "{}";
707-
}
708-
Pod podFromYaml;
709-
try (InputStream is = new ByteArrayInputStream(s.getBytes(UTF_8))) {
710-
podFromYaml = client.pods().load(is).item();
711-
} catch (IOException | KubernetesClientException e) {
712-
throw new RuntimeException(String.format("Failed to parse yaml: \"%s\"", yaml), e);
713-
}
714-
LOGGER.finest(() -> "Parsed pod template from yaml: " + Serialization.asYaml(podFromYaml));
715-
// yaml can be just a fragment, avoid NPEs
716-
if (podFromYaml.getMetadata() == null) {
717-
podFromYaml.setMetadata(new ObjectMeta());
718-
}
719-
if (podFromYaml.getSpec() == null) {
720-
podFromYaml.setSpec(new PodSpec());
721-
}
722-
if (!DISABLE_OCTAL_MODES) {
723-
fixOctal(podFromYaml);
724-
}
725-
return podFromYaml;
689+
// JENKINS-57116
690+
if (StringUtils.isBlank(s)) {
691+
LOGGER.log(Level.WARNING, "[JENKINS-57116] Trying to parse invalid yaml: \"{0}\"", yaml);
692+
s = "{}";
726693
}
727-
}
728-
729-
private static void fixOctal(@NonNull Pod podFromYaml) {
730-
podFromYaml.getSpec().getVolumes().stream().map(Volume::getConfigMap).forEach(configMap -> {
731-
if (configMap != null) {
732-
var defaultMode = configMap.getDefaultMode();
733-
if (defaultMode != null) {
734-
configMap.setDefaultMode(convertPermissionToOctal(defaultMode));
735-
}
736-
}
737-
});
738-
podFromYaml.getSpec().getVolumes().stream().map(Volume::getSecret).forEach(secretVolumeSource -> {
739-
if (secretVolumeSource != null) {
740-
var defaultMode = secretVolumeSource.getDefaultMode();
741-
if (defaultMode != null) {
742-
secretVolumeSource.setDefaultMode(convertPermissionToOctal(defaultMode));
743-
}
744-
}
745-
});
746-
podFromYaml.getSpec().getVolumes().stream().map(Volume::getProjected).forEach(projected -> {
747-
if (projected != null) {
748-
var defaultMode = projected.getDefaultMode();
749-
if (defaultMode != null) {
750-
projected.setDefaultMode(convertPermissionToOctal(defaultMode));
751-
}
752-
projected.getSources().forEach(source -> {
753-
var configMap = source.getConfigMap();
754-
if (configMap != null) {
755-
convertDecimalIntegersToOctal(configMap.getItems());
756-
}
757-
var secret = source.getSecret();
758-
if (secret != null) {
759-
convertDecimalIntegersToOctal(secret.getItems());
760-
}
761-
});
762-
}
763-
});
764-
}
765-
766-
private static void convertDecimalIntegersToOctal(List<KeyToPath> items) {
767-
items.forEach(i -> {
768-
var mode = i.getMode();
769-
if (mode != null) {
770-
i.setMode(convertPermissionToOctal(mode));
771-
}
772-
});
773-
}
774-
775-
/**
776-
* Permissions are generally expressed in octal notation, e.g. 0777.
777-
* After parsing, this is stored as the integer 777, but the snakeyaml-engine does not convert to decimal first.
778-
* When the client later sends the pod spec to the server, it sends the integer as is through the json schema,
779-
* however the server expects a decimal, which means an integer between 0 and 511.
780-
*
781-
* The user can also provide permissions as a decimal integer, e.g. 511.
782-
*
783-
* This method attempts to guess whether the user provided a decimal or octal integer, and converts to octal if needed,
784-
* so that the resulting can be submitted to the server.
785-
*
786-
*/
787-
static int convertPermissionToOctal(Integer i) {
788-
// Permissions are expressed as octal integers
789-
// octal goes from 0000 to 0777
790-
// decimal goes from 0 to 511
791-
var s = Integer.toString(i, 10);
792-
// If the input has a digit which is 8 or 9, this was likely a decimal input. Best effort support here.
793-
if (s.chars().map(c -> c - '0').anyMatch(a -> a > 7)) {
794-
return i;
795-
} else {
796-
return Integer.parseInt(s, 8);
694+
Pod podFromYaml;
695+
try (InputStream is = new ByteArrayInputStream(s.getBytes(UTF_8))) {
696+
podFromYaml = Serialization2.unmarshal(is, Pod.class);
697+
// podFromYaml = new KubernetesSerialization().unmarshal(is, Pod.class);
698+
} catch (IOException e) {
699+
throw new RuntimeException(String.format("Failed to parse yaml: \"%s\"", yaml), e);
700+
}
701+
LOGGER.finest(() -> "Parsed pod template from yaml: " + Serialization2.asYaml(podFromYaml));
702+
// yaml can be just a fragment, avoid NPEs
703+
if (podFromYaml.getMetadata() == null) {
704+
podFromYaml.setMetadata(new ObjectMeta());
705+
}
706+
if (podFromYaml.getSpec() == null) {
707+
podFromYaml.setSpec(new PodSpec());
797708
}
709+
return podFromYaml;
798710
}
799711

800712
public static Collection<String> validateYamlContainerNames(List<String> yamls) {

‎src/main/java/org/csanchez/jenkins/plugins/kubernetes/Serialization2.java

+58
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
package org.csanchez.jenkins.plugins.kubernetes;
2+
3+
import com.fasterxml.jackson.annotation.JsonInclude;
4+
import com.fasterxml.jackson.core.JsonProcessingException;
5+
import com.fasterxml.jackson.databind.DeserializationFeature;
6+
import com.fasterxml.jackson.databind.ObjectMapper;
7+
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
8+
import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator;
9+
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
10+
import edu.umd.cs.findbugs.annotations.CheckForNull;
11+
import edu.umd.cs.findbugs.annotations.NonNull;
12+
import io.fabric8.kubernetes.api.model.KubernetesResource;
13+
import io.fabric8.kubernetes.model.jackson.UnmatchedFieldTypeModule;
14+
import java.io.IOException;
15+
import java.io.InputStream;
16+
17+
/**
18+
* Use Jackson for serialization to continue support octal notation `0xxx`.
19+
*
20+
* @see io.fabric8.kubernetes.client.utils.Serialization
21+
* @see io.fabric8.kubernetes.client.utils.KubernetesSerialization
22+
*/
23+
public final class Serialization2 {
24+
25+
private static final ObjectMapper objectMapper =
26+
new ObjectMapper(new YAMLFactory().disable(YAMLGenerator.Feature.USE_NATIVE_TYPE_ID));
27+
28+
static {
29+
objectMapper.registerModules(new JavaTimeModule(), new UnmatchedFieldTypeModule());
30+
objectMapper.disable(DeserializationFeature.FAIL_ON_INVALID_SUBTYPE);
31+
objectMapper.setDefaultPropertyInclusion(
32+
JsonInclude.Value.construct(JsonInclude.Include.NON_NULL, JsonInclude.Include.ALWAYS));
33+
}
34+
35+
private Serialization2() {}
36+
37+
public static <T extends KubernetesResource> T unmarshal(InputStream is, Class<T> type) throws IOException {
38+
try {
39+
return objectMapper.readerFor(type).readValue(is);
40+
} catch (JsonProcessingException e) {
41+
throw new IOException("Unable to parse InputStream.", e);
42+
} catch (IOException e) {
43+
throw new IOException("Unable to read InputStream.", e);
44+
}
45+
}
46+
47+
@NonNull
48+
public static String asYaml(@CheckForNull Object model) {
49+
if (model != null) {
50+
try {
51+
return objectMapper.writeValueAsString(model);
52+
} catch (JsonProcessingException e) {
53+
throw new RuntimeException(e);
54+
}
55+
}
56+
return "";
57+
}
58+
}

‎src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest.java

+4-9
Original file line numberDiff line numberDiff line change
@@ -898,15 +898,10 @@ public void octalParsing() throws IOException {
898898

899899
@Test
900900
public void decimalParsing() throws IOException {
901-
try {
902-
DISABLE_OCTAL_MODES = true;
903-
var fileStream = getClass().getResourceAsStream(getClass().getSimpleName() + "/decimal.yaml");
904-
assertNotNull(fileStream);
905-
var pod = parseFromYaml(IOUtils.toString(fileStream, StandardCharsets.UTF_8));
906-
checkParsed(pod);
907-
} finally {
908-
DISABLE_OCTAL_MODES = false;
909-
}
901+
var fileStream = getClass().getResourceAsStream(getClass().getSimpleName() + "/decimal.yaml");
902+
assertNotNull(fileStream);
903+
var pod = parseFromYaml(IOUtils.toString(fileStream, StandardCharsets.UTF_8));
904+
checkParsed(pod);
910905
}
911906

912907
private static void checkParsed(Pod pod) {

0 commit comments

Comments
 (0)