Skip to content
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

Jump to bottom

[JENKINS-51248] Fix label validation regex #343

Merged
merged 2 commits into from
Jun 13, 2018
Merged

[JENKINS-51248] Fix label validation regex #343

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
231f31a
[JENKINS-51248] Fix label validation regex
carlossg Jun 13, 2018
2c05b81
Changelog for 1.8.0
carlossg Jun 13, 2018
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ Known issues

See the full list of issues at [JIRA](https://issues.jenkins-ci.org/issues/?filter=15575)

1.8.0
-----
* Validate label and container names with regex [#332](https://github.com/jenkinsci/kubernetes-plugin/pull/332) [#343](https://github.com/jenkinsci/kubernetes-plugin/pull/343) [JENKINS-51248](https://issues.jenkins-ci.org/browse/JENKINS-51248)

1.7.1
-----
* Do not print credentials in build output or logs. Only affects certain pipeline steps like `withDockerRegistry`. `sh` step is not affected [SECURITY-883](https://issues.jenkins-ci.org/browse/SECURITY-883)
Expand Down
10 changes: 3 additions & 7 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 @@ -61,6 +61,8 @@ public class PodTemplateUtils {

private static final Logger LOGGER = Logger.getLogger(PodTemplateUtils.class.getName());

private static final Pattern LABEL_VALIDATION = Pattern.compile("[a-zA-Z0-9]([_\\.\\-a-zA-Z0-9]*[a-zA-Z0-9])?");

/**
* Combines a {@link ContainerTemplate} with its parent.
* @param parent The parent container template (nullable).
Expand Down Expand Up @@ -522,13 +524,7 @@ public static boolean validateContainerName(String name) {
* Pulled from https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set
*/
public static boolean validateLabel(String label) {
if (label != null && !label.isEmpty()) {
Pattern labelValidation = Pattern.compile("[a-zA-Z0-9]([_\\.-A-Za-z0-9]*[A-Za-z0-9])?");
if (label.length() > 63 || !labelValidation.matcher(label).matches()) {
return false;
}
}
return true;
return StringUtils.isBlank(label) ? true : label.length() <= 63 && LABEL_VALIDATION.matcher(label).matches();
}

private static List<EnvVar> combineEnvVars(Container parent, Container template) {
Expand Down
84 changes: 67 additions & 17 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 @@ -30,7 +30,6 @@
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;

import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -70,7 +69,6 @@ public class PodTemplateUtilsTest {
private static final ContainerTemplate MAVEN_1 = new ContainerTemplate("maven", "maven:1", "sh -c", "cat");
private static final ContainerTemplate MAVEN_2 = new ContainerTemplate("maven", "maven:2");


@Test
public void shouldReturnContainerTemplateWhenParentIsNull() {
ContainerTemplate result = combine(null, JNLP_2);
Expand All @@ -84,7 +82,6 @@ public void shouldOverrideTheImageAndInheritTheRest() {
assertEquals("cat", result.getArgs());
}


@Test
public void shouldReturnPodTemplateWhenParentIsNull() {
PodTemplate template = new PodTemplate();
Expand All @@ -94,7 +91,6 @@ public void shouldReturnPodTemplateWhenParentIsNull() {
assertEquals(result, template);
}


@Test
public void shouldOverrideServiceAccountIfSpecified() {
PodTemplate parent = new PodTemplate();
Expand Down Expand Up @@ -135,8 +131,6 @@ public void shouldOverrideNodeSelectorIfSpecified() {
assertEquals("key:value", result.getNodeSelector());
}



@Test
public void shouldCombineAllImagePullSecrets() {
PodTemplate parent = new PodTemplate();
Expand Down Expand Up @@ -165,7 +159,6 @@ public void shouldCombineAllImagePullSecrets() {
assertEquals(1, result.getImagePullSecrets().size());
}


@Test
public void shouldCombineAllAnnotations() {
PodTemplate parent = new PodTemplate();
Expand Down Expand Up @@ -212,7 +205,6 @@ public void shouldUnwrapParent() {
template1.setServiceAccount("sa1");
template1.setImagePullSecrets(asList(SECRET_2, SECRET_3));


PodTemplate result = unwrap(template1, asList(parent, template1));
assertEquals(3, result.getImagePullSecrets().size());
assertEquals("sa1", result.getServiceAccount());
Expand All @@ -230,14 +222,14 @@ public void shouldDropNoDataWhenIdentical() {
podTemplate.setNodeUsageMode(Node.Mode.EXCLUSIVE);
podTemplate.setImagePullSecrets(asList(SECRET_1));
podTemplate.setInheritFrom("Inherit");
podTemplate.setInstanceCap(99);
podTemplate.setInstanceCap(99);
podTemplate.setSlaveConnectTimeout(99);
podTemplate.setIdleMinutes(99);
podTemplate.setActiveDeadlineSeconds(99);
podTemplate.setServiceAccount("ServiceAccount");
podTemplate.setCustomWorkspaceVolumeEnabled(true);
podTemplate.setYaml("Yaml");

PodTemplate selfCombined = combine(podTemplate, podTemplate);

assertEquals("Name", podTemplate.getName());
Expand All @@ -248,7 +240,7 @@ public void shouldDropNoDataWhenIdentical() {
assertEquals(Node.Mode.EXCLUSIVE, podTemplate.getNodeUsageMode());
assertEquals(asList(SECRET_1), podTemplate.getImagePullSecrets());
assertEquals("Inherit", podTemplate.getInheritFrom());
assertEquals(99, podTemplate.getInstanceCap());
assertEquals(99, podTemplate.getInstanceCap());
assertEquals(99, podTemplate.getSlaveConnectTimeout());
assertEquals(99, podTemplate.getIdleMinutes());
assertEquals(99, podTemplate.getActiveDeadlineSeconds());
Expand Down Expand Up @@ -285,14 +277,14 @@ public void shouldUnwrapMultipleParents() {
toUnwrap.setName("toUnwrap");
toUnwrap.setInheritFrom("template1 template2");


PodTemplate result = unwrap(toUnwrap, asList(parent, template1, template2));
assertEquals(3, result.getImagePullSecrets().size());
assertEquals("sa1", result.getServiceAccount());
assertEquals("key:value", result.getNodeSelector());
assertEquals(2, result.getContainers().size());

ContainerTemplate mavenTemplate = result.getContainers().stream().filter(c -> c.getName().equals("maven")).findFirst().orElse(null);
ContainerTemplate mavenTemplate = result.getContainers().stream().filter(c -> c.getName().equals("maven"))
.findFirst().orElse(null);
assertNotNull(mavenTemplate);
assertEquals("maven:2", mavenTemplate.getImage());
}
Expand Down Expand Up @@ -403,7 +395,8 @@ public void shouldCombineAllSecretEnvVars() {

ContainerTemplate result = combine(template1, template2);

assertThat(result.getEnvVars(), contains(containerSecretEnvVar1, containerSecretEnvVar2, containerSecretEnvVar3));
assertThat(result.getEnvVars(),
contains(containerSecretEnvVar1, containerSecretEnvVar2, containerSecretEnvVar3));
}

@Test
Expand Down Expand Up @@ -452,15 +445,15 @@ public void shouldCombineAllPodMounts() {

@Test
public void shouldCombineAllTolerations() {
PodSpec podSpec1 = new PodSpec();
PodSpec podSpec1 = new PodSpec();
Pod pod1 = new Pod();
Toleration toleration1 = new Toleration("effect1", "key1", "oper1", Long.parseLong("1"), "val1");
Toleration toleration2 = new Toleration("effect2", "key2", "oper2", Long.parseLong("2"), "val2");
podSpec1.setTolerations(asList(toleration1, toleration2));
pod1.setSpec(podSpec1);
pod1.setMetadata(new ObjectMeta());

PodSpec podSpec2 = new PodSpec();
PodSpec podSpec2 = new PodSpec();
Pod pod2 = new Pod();
Toleration toleration3 = new Toleration("effect3", "key3", "oper3", Long.parseLong("3"), "val3");
Toleration toleration4 = new Toleration("effect4", "key4", "oper4", Long.parseLong("4"), "val4");
Expand Down Expand Up @@ -524,6 +517,63 @@ public void shouldSubstituteMultipleEnvVarsAndNotUseDefaultsForMissing() {
Map<String, String> properties = new HashMap<>();
properties.put("key1", "value1");
properties.put("key2", "value2");
assertEquals("value1 or value2 or ${key3}", substitute("${key1} or ${key2} or ${key3}", properties, "defaultValue"));
assertEquals("value1 or value2 or ${key3}",
substitute("${key1} or ${key2} or ${key3}", properties, "defaultValue"));
}

@Test
public void testValidateLabelA() {
assertTrue(validateLabel("1"));
assertTrue(validateLabel("a"));
}

@Test
public void testValidateLabelAb() {
assertTrue(validateLabel("12"));
assertTrue(validateLabel("ab"));
}

@Test
public void testValidateLabelAbc() {
assertTrue(validateLabel("123"));
assertTrue(validateLabel("abc"));
}

@Test
public void testValidateLabelAbcd() {
assertTrue(validateLabel("1234"));
assertTrue(validateLabel("abcd"));
}

@Test
public void testValidateLabelMypod() {
assertTrue(validateLabel("mypod"));
}

@Test
public void testValidateLabelMyPodNested() {
assertTrue(validateLabel("mypodNested"));
}

@Test
public void testValidateLabelSpecialChars() {
assertTrue(validateLabel("x-_.z"));
}

@Test
public void testValidateLabelStartWithSpecialChars() {
assertFalse(validateLabel("-x"));
}

@Test
public void testValidateLabelLong() {
assertTrue(validateLabel("123456789012345678901234567890123456789012345678901234567890123"));
assertTrue(validateLabel("abcdefghijklmnopqrstuwxyzabcdefghijklmnopqrstuwxyzabcdefghijklm"));
}

@Test
public void testValidateLabelTooLong() {
assertFalse(validateLabel("1234567890123456789012345678901234567890123456789012345678901234"));
assertFalse(validateLabel("abcdefghijklmnopqrstuwxyzabcdefghijklmnopqrstuwxyzabcdefghijklmn"));
}
}