diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java index 39d802c6bc..dc73941219 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java @@ -18,7 +18,6 @@ import io.fabric8.kubernetes.api.model.ContainerBuilder; import io.fabric8.kubernetes.api.model.EnvFromSource; import io.fabric8.kubernetes.api.model.EnvVar; -import io.fabric8.kubernetes.api.model.KeyToPath; import io.fabric8.kubernetes.api.model.LocalObjectReference; import io.fabric8.kubernetes.api.model.ObjectMeta; import io.fabric8.kubernetes.api.model.Pod; @@ -29,10 +28,6 @@ import io.fabric8.kubernetes.api.model.Toleration; import io.fabric8.kubernetes.api.model.Volume; import io.fabric8.kubernetes.api.model.VolumeMount; -import io.fabric8.kubernetes.client.KubernetesClient; -import io.fabric8.kubernetes.client.KubernetesClientBuilder; -import io.fabric8.kubernetes.client.KubernetesClientException; -import io.fabric8.kubernetes.client.utils.Serialization; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; @@ -70,14 +65,6 @@ public class PodTemplateUtils { @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "tests & emergency admin") public static boolean SUBSTITUTE_ENV = Boolean.getBoolean(PodTemplateUtils.class.getName() + ".SUBSTITUTE_ENV"); - /** - * If true, all modes permissions provided to pods are expected to be provided in decimal notation. - * Otherwise, the plugin will consider they are written in octal notation. - */ - @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "tests & emergency admin") - public static /* almost final*/ boolean DISABLE_OCTAL_MODES = - Boolean.getBoolean(PodTemplateUtils.class.getName() + ".DISABLE_OCTAL_MODES"); - /** * Combines a {@link ContainerTemplate} with its parent. * @param parent The parent container template (nullable). @@ -287,8 +274,8 @@ public static Pod combine(Pod parent, Pod template) { return template; } - LOGGER.finest(() -> "Combining pods, parent: " + Serialization.asYaml(parent) + " template: " - + Serialization.asYaml(template)); + LOGGER.finest(() -> "Combining pods, parent: " + Serialization2.asYaml(parent) + " template: " + + Serialization2.asYaml(template)); Map nodeSelector = mergeMaps(parent.getSpec().getNodeSelector(), template.getSpec().getNodeSelector()); @@ -412,7 +399,7 @@ public static Pod combine(Pod parent, Pod template) { // podTemplate.setYaml(template.getYaml() == null ? parent.getYaml() : template.getYaml()); Pod pod = specBuilder.endSpec().build(); - LOGGER.finest(() -> "Pods combined: " + Serialization.asYaml(pod)); + LOGGER.finest(() -> "Pods combined: " + Serialization2.asYaml(pod)); return pod; } @@ -699,102 +686,27 @@ public static String substitute(String s, Map properties, String public static Pod parseFromYaml(String yaml) { String s = yaml; - try (KubernetesClient client = new KubernetesClientBuilder().build()) { - // JENKINS-57116 - if (StringUtils.isBlank(s)) { - LOGGER.log(Level.WARNING, "[JENKINS-57116] Trying to parse invalid yaml: \"{0}\"", yaml); - s = "{}"; - } - Pod podFromYaml; - try (InputStream is = new ByteArrayInputStream(s.getBytes(UTF_8))) { - podFromYaml = client.pods().load(is).item(); - } catch (IOException | KubernetesClientException e) { - throw new RuntimeException(String.format("Failed to parse yaml: \"%s\"", yaml), e); - } - LOGGER.finest(() -> "Parsed pod template from yaml: " + Serialization.asYaml(podFromYaml)); - // yaml can be just a fragment, avoid NPEs - if (podFromYaml.getMetadata() == null) { - podFromYaml.setMetadata(new ObjectMeta()); - } - if (podFromYaml.getSpec() == null) { - podFromYaml.setSpec(new PodSpec()); - } - if (!DISABLE_OCTAL_MODES) { - fixOctal(podFromYaml); - } - return podFromYaml; + // JENKINS-57116 + if (StringUtils.isBlank(s)) { + LOGGER.log(Level.WARNING, "[JENKINS-57116] Trying to parse invalid yaml: \"{0}\"", yaml); + s = "{}"; } - } - - private static void fixOctal(@NonNull Pod podFromYaml) { - podFromYaml.getSpec().getVolumes().stream().map(Volume::getConfigMap).forEach(configMap -> { - if (configMap != null) { - var defaultMode = configMap.getDefaultMode(); - if (defaultMode != null) { - configMap.setDefaultMode(convertPermissionToOctal(defaultMode)); - } - } - }); - podFromYaml.getSpec().getVolumes().stream().map(Volume::getSecret).forEach(secretVolumeSource -> { - if (secretVolumeSource != null) { - var defaultMode = secretVolumeSource.getDefaultMode(); - if (defaultMode != null) { - secretVolumeSource.setDefaultMode(convertPermissionToOctal(defaultMode)); - } - } - }); - podFromYaml.getSpec().getVolumes().stream().map(Volume::getProjected).forEach(projected -> { - if (projected != null) { - var defaultMode = projected.getDefaultMode(); - if (defaultMode != null) { - projected.setDefaultMode(convertPermissionToOctal(defaultMode)); - } - projected.getSources().forEach(source -> { - var configMap = source.getConfigMap(); - if (configMap != null) { - convertDecimalIntegersToOctal(configMap.getItems()); - } - var secret = source.getSecret(); - if (secret != null) { - convertDecimalIntegersToOctal(secret.getItems()); - } - }); - } - }); - } - - private static void convertDecimalIntegersToOctal(List items) { - items.forEach(i -> { - var mode = i.getMode(); - if (mode != null) { - i.setMode(convertPermissionToOctal(mode)); - } - }); - } - - /** - * Permissions are generally expressed in octal notation, e.g. 0777. - * After parsing, this is stored as the integer 777, but the snakeyaml-engine does not convert to decimal first. - * When the client later sends the pod spec to the server, it sends the integer as is through the json schema, - * however the server expects a decimal, which means an integer between 0 and 511. - * - * The user can also provide permissions as a decimal integer, e.g. 511. - * - * This method attempts to guess whether the user provided a decimal or octal integer, and converts to octal if needed, - * so that the resulting can be submitted to the server. - * - */ - static int convertPermissionToOctal(Integer i) { - // Permissions are expressed as octal integers - // octal goes from 0000 to 0777 - // decimal goes from 0 to 511 - var s = Integer.toString(i, 10); - // If the input has a digit which is 8 or 9, this was likely a decimal input. Best effort support here. - if (s.chars().map(c -> c - '0').anyMatch(a -> a > 7)) { - return i; - } else { - return Integer.parseInt(s, 8); + Pod podFromYaml; + try (InputStream is = new ByteArrayInputStream(s.getBytes(UTF_8))) { + podFromYaml = Serialization2.unmarshal(is, Pod.class); + // podFromYaml = new KubernetesSerialization().unmarshal(is, Pod.class); + } catch (IOException e) { + throw new RuntimeException(String.format("Failed to parse yaml: \"%s\"", yaml), e); + } + LOGGER.finest(() -> "Parsed pod template from yaml: " + Serialization2.asYaml(podFromYaml)); + // yaml can be just a fragment, avoid NPEs + if (podFromYaml.getMetadata() == null) { + podFromYaml.setMetadata(new ObjectMeta()); + } + if (podFromYaml.getSpec() == null) { + podFromYaml.setSpec(new PodSpec()); } + return podFromYaml; } public static Collection validateYamlContainerNames(List yamls) { diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/Serialization2.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/Serialization2.java new file mode 100644 index 0000000000..ec8630012d --- /dev/null +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/Serialization2.java @@ -0,0 +1,58 @@ +package org.csanchez.jenkins.plugins.kubernetes; + +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; +import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator; +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; +import edu.umd.cs.findbugs.annotations.CheckForNull; +import edu.umd.cs.findbugs.annotations.NonNull; +import io.fabric8.kubernetes.api.model.KubernetesResource; +import io.fabric8.kubernetes.model.jackson.UnmatchedFieldTypeModule; +import java.io.IOException; +import java.io.InputStream; + +/** + * Use Jackson for serialization to continue support octal notation `0xxx`. + * + * @see io.fabric8.kubernetes.client.utils.Serialization + * @see io.fabric8.kubernetes.client.utils.KubernetesSerialization + */ +public final class Serialization2 { + + private static final ObjectMapper objectMapper = + new ObjectMapper(new YAMLFactory().disable(YAMLGenerator.Feature.USE_NATIVE_TYPE_ID)); + + static { + objectMapper.registerModules(new JavaTimeModule(), new UnmatchedFieldTypeModule()); + objectMapper.disable(DeserializationFeature.FAIL_ON_INVALID_SUBTYPE); + objectMapper.setDefaultPropertyInclusion( + JsonInclude.Value.construct(JsonInclude.Include.NON_NULL, JsonInclude.Include.ALWAYS)); + } + + private Serialization2() {} + + public static T unmarshal(InputStream is, Class type) throws IOException { + try { + return objectMapper.readerFor(type).readValue(is); + } catch (JsonProcessingException e) { + throw new IOException("Unable to parse InputStream.", e); + } catch (IOException e) { + throw new IOException("Unable to read InputStream.", e); + } + } + + @NonNull + public static String asYaml(@CheckForNull Object model) { + if (model != null) { + try { + return objectMapper.writeValueAsString(model); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + } + return ""; + } +} diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest.java index 7d7a6fc7b5..60b11b6fb8 100644 --- a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest.java +++ b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest.java @@ -898,15 +898,10 @@ public void octalParsing() throws IOException { @Test public void decimalParsing() throws IOException { - try { - DISABLE_OCTAL_MODES = true; - var fileStream = getClass().getResourceAsStream(getClass().getSimpleName() + "/decimal.yaml"); - assertNotNull(fileStream); - var pod = parseFromYaml(IOUtils.toString(fileStream, StandardCharsets.UTF_8)); - checkParsed(pod); - } finally { - DISABLE_OCTAL_MODES = false; - } + var fileStream = getClass().getResourceAsStream(getClass().getSimpleName() + "/decimal.yaml"); + assertNotNull(fileStream); + var pod = parseFromYaml(IOUtils.toString(fileStream, StandardCharsets.UTF_8)); + checkParsed(pod); } private static void checkParsed(Pod pod) {