Fix #34843: improve parser of k8s resources and limits

Author: LEDfan

src/main/java/eu/openanalytics/containerproxy/backend/kubernetes/KubernetesBackend.java

@@ -266,10 +266,26 @@ public Proxy startContainer(Authentication user, Container initialContainer, Con
                 .build();
 
             ResourceRequirementsBuilder resourceRequirementsBuilder = new ResourceRequirementsBuilder();
-            resourceRequirementsBuilder.addToRequests("cpu", spec.getCpuRequest().mapOrNull(Quantity::new));
-            resourceRequirementsBuilder.addToLimits("cpu", spec.getCpuLimit().mapOrNull(Quantity::new));
-            resourceRequirementsBuilder.addToRequests("memory", spec.getMemoryRequest().mapOrNull(Quantity::new));
-            resourceRequirementsBuilder.addToLimits("memory", spec.getMemoryLimit().mapOrNull(Quantity::new));
+            try {
+                resourceRequirementsBuilder.addToRequests("cpu", parseCpuQuantity(spec.getCpuRequest().getValueOrNull()));
+            } catch (IllegalArgumentException ex) {
+                throw new IllegalArgumentException("Invalid container-cpu-request: " + ex.getMessage());
+            }
+            try {
+                resourceRequirementsBuilder.addToLimits("cpu", parseCpuQuantity(spec.getCpuLimit().getValueOrNull()));
+            } catch (IllegalArgumentException ex) {
+                throw new IllegalArgumentException("Invalid container-cpu-limit: " + ex.getMessage());
+            }
+            try {
+                resourceRequirementsBuilder.addToRequests("memory", parseMemoryQuantity(spec.getMemoryRequest().getValueOrNull()));
+            } catch (IllegalArgumentException ex) {
+                throw new IllegalArgumentException("Invalid container-memory-request: " + ex.getMessage());
+            }
+            try {
+                resourceRequirementsBuilder.addToLimits("memory", parseMemoryQuantity(spec.getMemoryLimit().getValueOrNull()));
+            } catch (IllegalArgumentException ex) {
+                throw new IllegalArgumentException("Invalid container-memory-limit: " + ex.getMessage());
+            }
 
             List<ContainerPort> containerPorts = spec.getPortMapping().stream()
                 .map(p -> new ContainerPortBuilder().withContainerPort(p.getPort()).build())
@@ -932,4 +948,42 @@ private Optional<Pod> getPod(BackendContainerName podInfo) {
         return Optional.ofNullable(kubeClient.pods().inNamespace(podInfo.getNamespace()).withName(podInfo.getName()).get());
     }
 
+    private Quantity parseCpuQuantity(String value) {
+        if (value == null) {
+            return null;
+        }
+        // convert upper case suffix automatically to a value accepted by k8s
+        if (value.endsWith("M")) {
+            value = value.substring(0, value.length() - 1) + "m";
+        }
+        Quantity quantity = new Quantity(value);
+        quantity.getNumericalAmount(); // validates the quantity
+        if (!quantity.getFormat().equals("m") && !quantity.getFormat().isEmpty()) {
+            throw new IllegalArgumentException("Invalid format for CPU resources");
+        }
+        return quantity;
+    }
+
+    private Quantity parseMemoryQuantity(String value) {
+        if (value == null) {
+            return null;
+        }
+        // convert lower case suffixes automatically to a value accepted by k8s
+        for (String suffix : List.of("p", "t", "g", "m", "k")) {
+            if (value.endsWith(suffix)) {
+                value = value.substring(0, value.length() - 1) + suffix.toUpperCase();
+                break;
+            } else if (value.endsWith(suffix + "i")) {
+                value = value.substring(0, value.length() - 2) + suffix.toUpperCase() + "i";
+                break;
+            }
+        }
+        Quantity quantity = new Quantity(value);
+        quantity.getNumericalAmount(); // validates the quantity
+        if (quantity.getFormat().equals("m") || quantity.getFormat().equals("mi") || quantity.getFormat() == "") {
+            throw new IllegalArgumentException("Invalid format for memory resources");
+        }
+        return quantity;
+    }
+
 }

src/test/java/eu/openanalytics/containerproxy/test/proxy/TestIntegrationOnKube.java

@@ -52,6 +52,8 @@
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.authority.SimpleGrantedAuthority;
@@ -233,10 +235,11 @@ public void launchProxyWithSecretRef() {
         }
     }
 
-    @Test
-    public void launchProxyWithResources() {
+    @ValueSource(strings = {"01_hello_limits2", "01_hello_limits"})
+    @ParameterizedTest
+    public void launchProxyWithResources(String file) {
         try (ContainerSetup k8s = new ContainerSetup("kubernetes")) {
-            String id = inst.client.startProxy("01_hello_limits");
+            String id = inst.client.startProxy(file);
             Proxy proxy = inst.proxyService.getProxy(id);
 
             PodList podList = k8s.client.pods().inNamespace(k8s.namespace).list();

src/test/resources/application-test-kubernetes.yml

@@ -70,6 +70,17 @@ proxy:
           cpu-limit: 2
           memory-request: "1Gi"
           memory-limit: "2Gi"
+    - id: 01_hello_limits2
+      container-specs:
+        - image: "openanalytics/shinyproxy-integration-test-app"
+          cmd: [ "R", "-e", "shinyproxy::run_01_hello()" ]
+          port-mapping:
+            - name: default
+              port: 3838
+          cpu-request: 1000M
+          cpu-limit: 2000m
+          memory-request: "1024mi"
+          memory-limit: "2gi"
     - id: 01_hello_priv
       container-specs:
         - image: "openanalytics/shinyproxy-integration-test-app"