Skip to content

Merge supplementalGroups when combining pod YAML with builder spec #2850

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

Open
amarkdotdev wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Merge supplementalGroups when combining pod YAML with builder spec #2850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
68 changes: 36 additions & 32 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 @@ -22,6 +22,7 @@
import io.fabric8.kubernetes.api.model.ObjectMeta;
import io.fabric8.kubernetes.api.model.Pod;
import io.fabric8.kubernetes.api.model.PodBuilder;
import io.fabric8.kubernetes.api.model.PodSecurityContext;
import io.fabric8.kubernetes.api.model.PodSpec;
import io.fabric8.kubernetes.api.model.Quantity;
import io.fabric8.kubernetes.api.model.ResourceRequirements;
Expand Down Expand Up @@ -209,6 +210,30 @@ private static Map<String, Quantity> combineResources(
);
}

private static Long resolvePodSecurityContextLong(
PodSecurityContext parent,
PodSecurityContext template,
Function<PodSecurityContext, Long> getter) {
Long templateValue = template != null ? getter.apply(template) : null;
if (templateValue != null) {
return templateValue;
}
return parent != null ? getter.apply(parent) : null;
}

private static List<Long> resolvePodSecurityContextSupplementalGroups(
PodSecurityContext parent, PodSecurityContext template) {
List<Long> templateValue = template != null ? template.getSupplementalGroups() : null;
if (templateValue != null && !templateValue.isEmpty()) {
return templateValue;
}
List<Long> parentValue = parent != null ? parent.getSupplementalGroups() : null;
if (parentValue != null && !parentValue.isEmpty()) {
return parentValue;
}
return null;
}

private static Capabilities combineCapabilities(Container parent, Container template) {
Capabilities parentCapabilities = parent.getSecurityContext() != null
? parent.getSecurityContext().getCapabilities()
Expand Down Expand Up @@ -343,40 +368,19 @@ public static Pod combine(Pod parent, Pod template) {

// Security context
if (template.getSpec().getSecurityContext() != null || parent.getSpec().getSecurityContext() != null) {
PodSecurityContext parentSecurityContext = parent.getSpec().getSecurityContext();
PodSecurityContext templateSecurityContext = template.getSpec().getSecurityContext();
Long runAsUser = resolvePodSecurityContextLong(
parentSecurityContext, templateSecurityContext, PodSecurityContext::getRunAsUser);
Long runAsGroup = resolvePodSecurityContextLong(
parentSecurityContext, templateSecurityContext, PodSecurityContext::getRunAsGroup);
List<Long> supplementalGroups =
resolvePodSecurityContextSupplementalGroups(parentSecurityContext, templateSecurityContext);
specBuilder
.editOrNewSecurityContext()
.withRunAsUser(
template.getSpec().getSecurityContext() != null
&& template.getSpec()
.getSecurityContext()
.getRunAsUser()
!= null
? template.getSpec().getSecurityContext().getRunAsUser()
: (parent.getSpec().getSecurityContext() != null
&& parent.getSpec()
.getSecurityContext()
.getRunAsUser()
!= null
? parent.getSpec()
.getSecurityContext()
.getRunAsUser()
: null))
.withRunAsGroup(
template.getSpec().getSecurityContext() != null
&& template.getSpec()
.getSecurityContext()
.getRunAsGroup()
!= null
? template.getSpec().getSecurityContext().getRunAsGroup()
: (parent.getSpec().getSecurityContext() != null
&& parent.getSpec()
.getSecurityContext()
.getRunAsGroup()
!= null
? parent.getSpec()
.getSecurityContext()
.getRunAsGroup()
: null))
.withRunAsUser(runAsUser)
.withRunAsGroup(runAsGroup)
.withSupplementalGroups(supplementalGroups)
.endSecurityContext();
}

Expand Down
27 changes: 27 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 @@ -461,6 +461,33 @@ void shouldCombineActiveDeadlineSeconds() {
assertEquals(combinedPod.getSpec().getActiveDeadlineSeconds(), Long.valueOf(1L));
}

@Test
void shouldCombinePodSupplementalGroupsFromTemplate() {
Pod parentPod = new PodBuilder()
.withNewMetadata()
.endMetadata()
.withNewSpec()
.withContainers(new ContainerBuilder().withName("jnlp").build())
.endSpec()
.build();
Pod childPod = new PodBuilder()
.withNewMetadata()
.endMetadata()
.withNewSpec()
.withNewSecurityContext()
.withRunAsUser(1000L)
.withRunAsGroup(1000L)
.withSupplementalGroups(5001L, 5002L)
.endSecurityContext()
.endSpec()
.build();

Pod combinedPod = combine(parentPod, childPod);
assertEquals(asList(5001L, 5002L), combinedPod.getSpec().getSecurityContext().getSupplementalGroups());
assertEquals(Long.valueOf(1000L), combinedPod.getSpec().getSecurityContext().getRunAsUser());
assertEquals(Long.valueOf(1000L), combinedPod.getSpec().getSecurityContext().getRunAsGroup());
}

@Test
void shouldFilterOutNullOrEmptyPodKeyValueEnvVars() {
PodTemplate template1 = new PodTemplate();
Expand Down
99 changes: 99 additions & 0 deletions ...java/org/csanchez/jenkins/plugins/kubernetes/SupplementalGroupsYamlOverrideReproTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
package org.csanchez.jenkins.plugins.kubernetes;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.when;

import io.fabric8.kubernetes.api.model.Pod;
import io.fabric8.kubernetes.api.model.PodSecurityContext;
import java.util.Arrays;
import org.csanchez.jenkins.plugins.kubernetes.pod.yaml.Overrides;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.JenkinsRule;
import org.mockito.Mock;
import org.mockito.Spy;
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;

/**
* Verifies supplementalGroups from pod template UI/CasC fields survive yaml override merge.
* Related to jenkinsci/kubernetes-plugin#2444.
*/
public class SupplementalGroupsYamlOverrideReproTest {

private static final String AGENT_NAME = "jenkins-agent";
private static final String AGENT_SECRET = "xxx";
private static final String JENKINS_URL = "http://jenkins.example.com";

@Rule
public JenkinsRule r = new JenkinsRule();

@Rule
public MockitoRule mockitoRule = MockitoJUnit.rule();

@Spy
private KubernetesCloud cloud = new KubernetesCloud("test");

@Mock
private KubernetesSlave slave;

@Mock
private KubernetesComputer computer;

@Before
public void setUp() {
when(slave.getKubernetesCloud()).thenReturn(cloud);
doReturn(JENKINS_URL).when(cloud).getJenkinsUrlOrDie();
when(computer.getName()).thenReturn(AGENT_NAME);
when(computer.getJnlpMac()).thenReturn(AGENT_SECRET);
when(slave.getComputer()).thenReturn(computer);
}

@Test
public void supplementalGroupsPresentWithoutRawYaml() throws Exception {
PodTemplate template = baseTemplate();
Pod pod = new PodTemplateBuilder(template, slave).build();

PodSecurityContext sc = pod.getSpec().getSecurityContext();
assertNotNull(sc);
assertEquals(Arrays.asList(5001L, 5002L), sc.getSupplementalGroups());
}

@Test
public void supplementalGroupsPreservedWhenRawYamlOverrideIsUsed() throws Exception {
PodTemplate template = baseTemplate();
template.setYamlMergeStrategy(new Overrides());
template.setYaml(
"""
apiVersion: v1
kind: Pod
spec:
containers:
- name: jnlp
image: jenkins/inbound-agent:latest
command: ['cat']
tty: true
""");

Pod pod = new PodTemplateBuilder(template, slave).build();

PodSecurityContext sc = pod.getSpec().getSecurityContext();
assertNotNull(sc);
assertEquals(
"expected supplementalGroups [5001, 5002] from pod template fields",
Arrays.asList(5001L, 5002L),
sc.getSupplementalGroups());
}

private static PodTemplate baseTemplate() {
PodTemplate template = new PodTemplate();
template.setRunAsUser("1000");
template.setRunAsGroup("1000");
template.setSupplementalGroups("5001,5002");
return template;
}
}
Loading