Skip to content

[JENKINS-71956] Octal notation fixes #1503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Vlatombe merged 2 commits into from
Jan 19, 2024
Merged

[JENKINS-71956] Octal notation fixes #1503

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7bed8c8
[JENKINS-71956] Octal notation fixes
Vlatombe Jan 19, 2024
d876d6e
Fix spotbugs
Vlatombe Jan 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 58 additions & 13 deletions src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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());
}
Expand All @@ -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,
Copy link
Member

@jglick jglick Jan 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could make this more reliable by checking the octal representation (reference). Actual modes will normally have the group bits match either the user bits or the other bits (it would be weird to have three different octets). And normally u ≥ g ≥ o and w ≥ r ≥ x right? For example 777 satisfies all constraints whereas 511 in octal would be -r-x--x--x which violates r ≥ x for g/o and is thus technically legal for exotic scenarios but unlikely to be what the user meant.

* 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) {
Expand Down
53 changes: 53 additions & 0 deletions src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
}
29 changes: 29 additions & 0 deletions src/test/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest/decimal.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
29 changes: 29 additions & 0 deletions src/test/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest/octal.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"