Merge pull request #1503 from Vlatombe/JENKINS-71956-octal-mode-impro…

…vements

[JENKINS-71956] Octal notation fixes

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,14 @@ 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).
@@ -705,24 +711,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 +756,37 @@ private static void fixOctal(@NonNull Pod podFromYaml) {
     }
 
     private static void convertDecimalIntegersToOctal(List<KeyToPath> 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<String> validateYamlContainerNames(List<String> yamls) {

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

@@ -47,13 +47,16 @@
 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;
 import java.util.HashMap;
 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());
+    }
 }

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"

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"