From 7bed8c83c50eb72da478fe0e2b2167cee8eacfe8 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Fri, 19 Jan 2024 11:36:11 +0100 Subject: [PATCH 1/2] [JENKINS-71956] Octal notation fixes * Support more paths to convert modes * Allows to disable conversion for people who converted their manifests to decimal notation. --- .../plugins/kubernetes/PodTemplateUtils.java | 70 +++++++++++++++---- .../kubernetes/PodTemplateUtilsTest.java | 53 ++++++++++++++ .../PodTemplateUtilsTest/decimal.yaml | 29 ++++++++ .../PodTemplateUtilsTest/octal.yaml | 29 ++++++++ 4 files changed, 168 insertions(+), 13 deletions(-) create mode 100644 src/test/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest/decimal.yaml create mode 100644 src/test/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest/octal.yaml 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 ea8863b52d..6297e6109b 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java @@ -14,7 +14,6 @@ import hudson.model.Label; import hudson.model.Node; import hudson.slaves.NodeProperty; -import io.fabric8.kubernetes.api.model.ConfigMapProjection; import io.fabric8.kubernetes.api.model.Container; import io.fabric8.kubernetes.api.model.ContainerBuilder; import io.fabric8.kubernetes.api.model.EnvFromSource; @@ -27,7 +26,6 @@ import io.fabric8.kubernetes.api.model.PodSpec; import io.fabric8.kubernetes.api.model.Quantity; import io.fabric8.kubernetes.api.model.ResourceRequirements; -import io.fabric8.kubernetes.api.model.SecretProjection; import io.fabric8.kubernetes.api.model.Toleration; import io.fabric8.kubernetes.api.model.Volume; import io.fabric8.kubernetes.api.model.VolumeMount; @@ -72,6 +70,13 @@ 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. + */ + 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). @@ -705,24 +710,42 @@ public static Pod parseFromYaml(String yaml) { if (podFromYaml.getSpec() == null) { podFromYaml.setSpec(new PodSpec()); } - fixOctal(podFromYaml); + if (!DISABLE_OCTAL_MODES) { + fixOctal(podFromYaml); + } return podFromYaml; } } 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) { - Integer defaultMode = projected.getDefaultMode(); + var defaultMode = projected.getDefaultMode(); if (defaultMode != null) { - projected.setDefaultMode(convertToOctal(defaultMode)); + projected.setDefaultMode(convertPermissionToOctal(defaultMode)); } - projected.getSources().stream().forEach(source -> { - ConfigMapProjection configMap = source.getConfigMap(); + projected.getSources().forEach(source -> { + var configMap = source.getConfigMap(); if (configMap != null) { convertDecimalIntegersToOctal(configMap.getItems()); } - SecretProjection secret = source.getSecret(); + var secret = source.getSecret(); if (secret != null) { convertDecimalIntegersToOctal(secret.getItems()); } @@ -732,16 +755,37 @@ private static void fixOctal(@NonNull Pod podFromYaml) { } private static void convertDecimalIntegersToOctal(List items) { - items.stream().forEach(i -> { - Integer mode = i.getMode(); + items.forEach(i -> { + var mode = i.getMode(); if (mode != null) { - i.setMode(convertToOctal(mode)); + i.setMode(convertPermissionToOctal(mode)); } }); } - private static int convertToOctal(Integer defaultMode) { - return Integer.parseInt(Integer.toString(defaultMode, 10), 8); + /** + * 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); + } } public static Collection validateYamlContainerNames(List yamls) { 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 432994d4c8..eee3c262b1 100644 --- a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest.java +++ b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest.java @@ -47,6 +47,8 @@ import io.fabric8.kubernetes.api.model.Toleration; import io.fabric8.kubernetes.api.model.VolumeMount; import io.fabric8.kubernetes.api.model.VolumeMountBuilder; +import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -54,6 +56,7 @@ import java.util.List; import java.util.Map; import java.util.stream.Collectors; +import org.apache.commons.io.IOUtils; import org.csanchez.jenkins.plugins.kubernetes.model.KeyValueEnvVar; import org.csanchez.jenkins.plugins.kubernetes.model.SecretEnvVar; import org.csanchez.jenkins.plugins.kubernetes.volumes.HostPathVolume; @@ -884,4 +887,54 @@ public void testParseYaml() { PodTemplateUtils.parseFromYaml(null); PodTemplateUtils.parseFromYaml(""); } + + @Test + public void octalParsing() throws IOException { + var fileStream = getClass().getResourceAsStream(getClass().getSimpleName() + "/octal.yaml"); + assertNotNull(fileStream); + var pod = parseFromYaml(IOUtils.toString(fileStream, StandardCharsets.UTF_8)); + checkParsed(pod); + } + + @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; + } + } + + private static void checkParsed(Pod pod) { + assertEquals( + Integer.valueOf("755", 8), + pod.getSpec().getVolumes().get(0).getConfigMap().getDefaultMode()); + assertEquals( + Integer.valueOf("744", 8), + pod.getSpec().getVolumes().get(1).getSecret().getDefaultMode()); + var projectedVolume = pod.getSpec().getVolumes().get(2).getProjected(); + assertEquals(Integer.valueOf("644", 8), projectedVolume.getDefaultMode()); + assertEquals( + Integer.valueOf("400", 8), + projectedVolume + .getSources() + .get(0) + .getConfigMap() + .getItems() + .get(0) + .getMode()); + assertEquals( + Integer.valueOf("600", 8), + projectedVolume + .getSources() + .get(1) + .getSecret() + .getItems() + .get(0) + .getMode()); + } } diff --git a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest/decimal.yaml b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest/decimal.yaml new file mode 100644 index 0000000000..75ccbb0872 --- /dev/null +++ b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest/decimal.yaml @@ -0,0 +1,29 @@ +# Identical to octal.yaml with values converted to decimal +apiVersion: "v1" +kind: "Pod" +spec: + volumes: + - configMap: + name: cm1 + defaultMode: 493 + name: "volume1" + - secret: + secretName: secret1 + defaultMode: 484 + name: "volume2" + - projected: + sources: + - configMap: + name: cm2 + items: + - key: username + path: my-group/my-username + mode: 256 + - secret: + name: secret2 + items: + - key: username + path: my-group/my-username + mode: 384 + defaultMode: 420 + name: "volume3" diff --git a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest/octal.yaml b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest/octal.yaml new file mode 100644 index 0000000000..f85c994d95 --- /dev/null +++ b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest/octal.yaml @@ -0,0 +1,29 @@ +# Octal modes +apiVersion: "v1" +kind: "Pod" +spec: + volumes: + - configMap: + name: cm1 + defaultMode: 0755 + name: "volume1" + - secret: + secretName: secret1 + defaultMode: 0744 + name: "volume2" + - projected: + sources: + - configMap: + name: cm2 + items: + - key: username + path: my-group/my-username + mode: 0400 + - secret: + name: secret2 + items: + - key: username + path: my-group/my-username + mode: 0600 + defaultMode: 0644 + name: "volume3" From d876d6ea4763be7f7e04740c531947142497f148 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Fri, 19 Jan 2024 14:10:29 +0100 Subject: [PATCH 2/2] Fix spotbugs --- .../csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java | 1 + 1 file changed, 1 insertion(+) 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 6297e6109b..b3875c0b97 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java @@ -74,6 +74,7 @@ public class PodTemplateUtils { * 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");