Make K8S integration more configurable (#552)

* Define pod using k8s yaml manifest

The Kubernetes plugin allows to define pods directly via the Kubernetes
API specification:
https://github.com/jenkinsci/kubernetes-plugin#using-yaml-to-define-pod-templates

This has the advantage of unlocking Kubernetes features which are not
exposed via the Kubernetes plugin, including all Kubernetes security
featues.

Using the Kubernetes API directly is also better from development
point of view because it is stable and better desgined then the API the
plugin offers.

* Make the Kubernetes ns configurable

If one Jenkins Master is used by multiple Teams, it is desirable to
schedule K8S workloads in seperatae workspaces.

* Add securityContext to define uid and fsGroup

In the context of the Jenkins k8s plugin it is uids and fsGroups play an
important role, because the containers share a common file system.

Therefore it is benefical to configure this data centraly.

* fix indention

* Undo format changes

* Extend and fix unit tests

* Fix port mapping

* Don't set uid globally

This does not work with jaas due to permissions problems.

* Fix sidecar test

* Make security context configurable at stage level

* Extract json serialization

* Cleanup unit tests

resources/default_pipeline_environment.yml

@@ -27,6 +27,10 @@ general:
   gitSshKeyCredentialsId: '' #needed to allow sshagent to run with local ssh key
   jenkinsKubernetes:
     jnlpAgent: 's4sdk/jenkins-agent-k8s:latest'
+    securityContext:
+    # Setting security context globally is currently not working with jaas
+    #  runAsUser: 1000
+    #  fsGroup: 1000
   manualConfirmation: true
   productiveBranch: 'master'
 

test/groovy/DockerExecuteOnKubernetesTest.groovy

@@ -7,6 +7,7 @@ import org.junit.rules.ExpectedException
 import org.junit.rules.RuleChain
 
 
+import groovy.json.JsonSlurper
 import util.BasePiperTest
 import util.JenkinsDockerExecuteRule
 import util.JenkinsLoggingRule
@@ -54,7 +55,8 @@ class DockerExecuteOnKubernetesTest extends BasePiperTest {
     def portList = []
     def containerCommands = []
     def pullImageMap = [:]
-
+    def namespace
+    def securityContext
 
     @Before
     void init() {
@@ -71,21 +73,25 @@ class DockerExecuteOnKubernetesTest extends BasePiperTest {
         helper.registerAllowedMethod('podTemplate', [Map.class, Closure.class], { Map options, Closure body ->
             podName = options.name
             podLabel = options.label
-            options.containers.each { option ->
-                containersList.add(option.name)
-                imageList.add(option.image.toString())
-                envList.add(option.envVars)
-                portList.add(option.ports)
-                if (option.command) {
-                    containerCommands.add(option.command)
+            namespace = options.namespace
+            def podSpec = new JsonSlurper().parseText(options.yaml)  // this yaml is actually json
+            def containers = podSpec.spec.containers
+            securityContext = podSpec.spec.securityContext
+
+            containers.each { container ->
+                containersList.add(container.name)
+                imageList.add(container.image.toString())
+                envList.add(container.env)
+                if(container.ports) {
+                    portList.add(container.ports)
                 }
-                pullImageMap.put(option.image.toString(), option.alwaysPullImage)
+                if (container.command) {
+                    containerCommands.add(container.command)
+                }
+                pullImageMap.put(container.image.toString(), container.imagePullPolicy == "Always")
             }
             body()
         })
-        helper.registerAllowedMethod('node', [String.class, Closure.class], { String nodeName, Closure body -> body() })
-        helper.registerAllowedMethod('envVar', [Map.class], { Map option -> return option })
-        helper.registerAllowedMethod('containerTemplate', [Map.class], { Map option -> return option })
     }
 
     @Test
@@ -257,10 +263,10 @@ class DockerExecuteOnKubernetesTest extends BasePiperTest {
             hasItem('maven:3.5-jdk-8-alpine'),
             hasItem('selenium/standalone-chrome'),
         ))
-        assertThat(portList, hasItem(hasItem([name: 'selenium0', containerPort: 4444, hostPort: 4444])))
-        assertThat(portMapping, hasItem([name: 'selenium0', containerPort: 4444, hostPort: 4444]))
+        // assertThat(portList, is(null))
+        assertThat(portList, hasItem([[name: 'selenium0', containerPort: 4444, hostPort: 4444]]))
         assertThat(containerCommands.size(), is(1))
-        assertThat(envList, hasItem(hasItem(allOf(hasEntry('key', 'customEnvKey'), hasEntry ('value','customEnvValue')))))
+        assertThat(envList, hasItem(hasItem(allOf(hasEntry('name', 'customEnvKey'), hasEntry ('value','customEnvValue')))))
     }
 
     @Test
@@ -286,7 +292,7 @@ class DockerExecuteOnKubernetesTest extends BasePiperTest {
         ) {
             //nothing to exeute
         }
-        assertThat(containerCommands, hasItem('/busybox/tail -f /dev/null'))
+        assertThat(containerCommands, hasItem(['/bin/sh', '-c', '/busybox/tail -f /dev/null']))
     }
 
     @Test
@@ -334,6 +340,36 @@ class DockerExecuteOnKubernetesTest extends BasePiperTest {
         assertTrue(bodyExecuted)
     }
 
+    @Test
+    void testDockerExecuteOnKubernetesWithCustomNamespace() {
+        def expectedNamespace = "sandbox"
+        nullScript.commonPipelineEnvironment.configuration = [general: [jenkinsKubernetes: [namespace: expectedNamespace]]]
+
+        stepRule.step.dockerExecuteOnKubernetes(
+                script: nullScript,
+                juStabUtils: utils,
+                dockerImage: 'maven:3.5-jdk-8-alpine',
+                ) { bodyExecuted = true }
+        assertTrue(bodyExecuted)
+        assertThat(namespace, is(equalTo(expectedNamespace)))
+    }
+
+    @Test
+    void testDockerExecuteOnKubernetesWithSecurityContext() {
+        def expectedSecurityContext = [ runAsUser: 1000, fsGroup: 1000 ]
+        nullScript.commonPipelineEnvironment.configuration = [general: [jenkinsKubernetes: [
+                    securityContext: expectedSecurityContext]]]
+
+        stepRule.step.dockerExecuteOnKubernetes(
+                script: nullScript,
+                juStabUtils: utils,
+                dockerImage: 'maven:3.5-jdk-8-alpine',
+                ) { bodyExecuted = true }
+        assertTrue(bodyExecuted)
+        assertThat(securityContext, is(equalTo(expectedSecurityContext)))
+    }
+
+
     private container(options, body) {
         containerName = options.name
         containerShell = options.shell

vars/dockerExecuteOnKubernetes.groovy

@@ -5,6 +5,8 @@ import com.sap.piper.GenerateDocumentation
 import com.sap.piper.JenkinsUtils
 import com.sap.piper.Utils
 import com.sap.piper.k8s.SystemEnv
+import com.sap.piper.JsonUtils
+
 import groovy.transform.Field
 import hudson.AbortException
 
@@ -81,7 +83,13 @@ import hudson.AbortException
     /**
      *
      */
-    'stashIncludes'
+    'stashIncludes',
+    /**
+     * Kubernetes Security Context used for the pod.
+     * Can be used to specify uid and fsGroup.
+     * See: https://kubernetes.io/docs/tasks/configure-pod-container/security-context/
+     */
+    'securityContext'
 ])
 @Field Set PARAMETER_KEYS = STEP_CONFIG_KEYS.minus([
     'stashIncludes',
@@ -124,9 +132,16 @@ void call(Map parameters = [:], body) {
 }
 
 def getOptions(config) {
-    return [name      : 'dynamic-agent-' + config.uniqueId,
-            label     : config.uniqueId,
-            containers: getContainerList(config)]
+    def namespace = config.jenkinsKubernetes.namespace
+    def options = [
+        name      : 'dynamic-agent-' + config.uniqueId,
+        label     : config.uniqueId,
+        yaml      : generatePodSpec(config)
+    ]
+    if (namespace) {
+        options.namespace = namespace
+    }
+    return options
 }
 
 void executeOnPod(Map config, utils, Closure body) {
@@ -171,13 +186,33 @@ void executeOnPod(Map config, utils, Closure body) {
     }
 }
 
+private String generatePodSpec(Map config) {
+    def containers = getContainerList(config)
+    def podSpec = [
+        apiVersion: "v1",
+        kind: "Pod",
+        metadata: [
+            lables: config.uniqueId
+        ],
+        spec: [
+            containers: containers
+        ]
+    ]
+    podSpec.spec.securityContext = getSecurityContext(config)
+
+    return new JsonUtils().getPrettyJsonString(podSpec)
+}
+
+
 private String stashWorkspace(config, prefix, boolean chown = false) {
     def stashName = "${prefix}-${config.uniqueId}"
     try {
-        // Every dockerImage used in the dockerExecuteOnKubernetes should have user id 1000
         if (chown)  {
+            def securityContext = getSecurityContext(config)
+            def runAsUser = securityContext?.runAsUser ?: 1000
+            def fsGroup = securityContext?.fsGroup ?: 1000
             sh """#!${config.containerShell?:'/bin/sh'}
-chown -R 1000:1000 ."""
+chown -R ${runAsUser}:${fsGroup} ."""
         }
         stash(
             name: stashName,
@@ -191,6 +226,10 @@ chown -R 1000:1000 ."""
     return null
 }
 
+private Map getSecurityContext(Map config) {
+    return config.securityContext ?: config.jenkinsKubernetes.securityContext ?: [:]
+}
+
 private void unstashWorkspace(config, prefix) {
     try {
         unstash "${prefix}-${config.uniqueId}"
@@ -200,25 +239,46 @@ private void unstashWorkspace(config, prefix) {
 }
 
 private List getContainerList(config) {
-    result = []
-    result.push(containerTemplate(
+    def result = [[
         name: 'jnlp',
         image: config.jenkinsKubernetes.jnlpAgent
-    ))
+    ]]
     config.containerMap.each { imageName, containerName ->
         def containerPullImage = config.containerPullImageFlags?.get(imageName)
-        def templateParameters = [
+        def containerSpec = [
             name: containerName.toLowerCase(),
             image: imageName,
-            alwaysPullImage: containerPullImage != null ? containerPullImage : config.dockerPullImage,
-            envVars: getContainerEnvs(config, imageName)
+            imagePullPolicy: containerPullImage ? "Always" : "IfNotPresent",
+            env: getContainerEnvs(config, imageName)
         ]
 
-        if (!config.containerCommands?.get(imageName)?.isEmpty()) {
-            templateParameters.command = config.containerCommands?.get(imageName)?: '/usr/bin/tail -f /dev/null'
+        def configuredCommand = config.containerCommands?.get(imageName)
+        def shell = config.containerShell ?: '/bin/sh'
+        if (configuredCommand == null) {
+            containerSpec['command'] = [
+                '/usr/bin/tail',
+                '-f',
+                '/dev/null'
+            ]
+        } else if(configuredCommand != "") {
+            // apparently "" is used as a flag for not settings container commands !?
+            containerSpec['command'] =
+                    (configuredCommand in List) ? configuredCommand : [
+                        shell,
+                        '-c',
+                        configuredCommand
+                    ]
         }
 
         if (config.containerPortMappings?.get(imageName)) {
+            def portMapping = { m ->
+                [
+                    name: m.name,
+                    containerPort: m.containerPort,
+                    hostPort: m.hostPort
+                ]
+            }
+
             def ports = []
             def portCounter = 0
             config.containerPortMappings.get(imageName).each {mapping ->
@@ -226,9 +286,9 @@ private List getContainerList(config) {
                 ports.add(portMapping(mapping))
                 portCounter ++
             }
-            templateParameters.ports = ports
+            containerSpec.ports = ports
         }
-        result.push(containerTemplate(templateParameters))
+        result.push(containerSpec)
     }
     return result
 }
@@ -244,6 +304,10 @@ private List getContainerEnvs(config, imageName) {
     def dockerEnvVars = config.containerEnvVars?.get(imageName) ?: config.dockerEnvVars ?: [:]
     def dockerWorkspace = config.containerWorkspaces?.get(imageName) != null ? config.containerWorkspaces?.get(imageName) : config.dockerWorkspace ?: ''
 
+    def envVar = { e ->
+        [ name: e.key, value: e.value ]
+    }
+
     if (dockerEnvVars) {
         for (String k : dockerEnvVars.keySet()) {
             containerEnv << envVar(key: k, value: dockerEnvVars[k].toString())
@@ -260,10 +324,5 @@ private List getContainerEnvs(config, imageName) {
         containerEnv << envVar(key: env, value: systemEnv.get(env))
     }
 
-    // ContainerEnv array can't be empty. Using a stub to avoid failure.
-    if (!containerEnv) {
-        containerEnv << envVar(key: "EMPTY_VAR", value: " ")
-    }
-
     return containerEnv
 }