From 2d30ddda31c67582d90c5070b901e1d5f3486573 Mon Sep 17 00:00:00 2001 From: Michael Kriese Date: Wed, 19 Jan 2022 14:33:58 +0100 Subject: [PATCH 1/4] fix: use correct syntax for environment variable --- .../plugins/docker/workflow/Docker.groovy | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy b/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy index de5296d6d..b3581f882 100644 --- a/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy +++ b/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy @@ -81,10 +81,16 @@ class Docker implements Serializable { } } + String asEnv(String var) { + node { + script.isUnix() ? "$${var}" : "%${var}%" + } + } + public Image build(String image, String args = '.') { check(image) node { - def commandLine = 'docker build -t "$JD_IMAGE" ' + args + def commandLine = 'docker build -t "' + asEnv('JD_IMAGE') + '" ' + args script.withEnv(["JD_IMAGE=${image}"]) { script."${shell()}" commandLine } @@ -122,11 +128,11 @@ class Docker implements Serializable { docker.node { def toRun = imageName() docker.script.withEnv(["JD_ID=${id}", "JD_TO_RUN=${toRun}"]) { - if (toRun != id && docker.script."${docker.shell()}"(script: 'docker inspect -f . "$JD_ID"', returnStatus: true) == 0) { + if (toRun != id && docker.script."${docker.shell()}"(script: 'docker inspect -f . "' + asEnv('JD_ID') + '"', returnStatus: true) == 0) { // Can run it without registry prefix, because it was locally built. toRun = id } else { - if (docker.script."${docker.shell()}"(script: 'docker inspect -f . "$JD_TO_RUN"', returnStatus: true) != 0) { + if (docker.script."${docker.shell()}"(script: 'docker inspect -f . "' + asEnv('JD_TO_RUN') + '"', returnStatus: true) != 0) { // Not yet present locally. // withDockerContainer requires the image to be available locally, since its start phase is not a durable task. pull() @@ -143,7 +149,7 @@ class Docker implements Serializable { docker.node { def toPull = imageName() docker.script.withEnv(["JD_TO_PULL=${toPull}"]) { - docker.script."${docker.shell()}" 'docker pull "$JD_TO_PULL"' + docker.script."${docker.shell()}" 'docker pull "' + asEnv('JD_TO_PULL') + '"' } } } @@ -170,7 +176,7 @@ class Docker implements Serializable { docker.node { def taggedImageName = toQualifiedImageName(parsedId.userAndRepo + ':' + tagName) docker.script.withEnv(["JD_ID=${id}", "JD_TAGGED_IMAGE_NAME=${taggedImageName}"]) { - docker.script."${docker.shell()}" 'docker tag "$JD_ID" "$JD_TAGGED_IMAGE_NAME"' + docker.script."${docker.shell()}" 'docker tag "' + asEnv('JD_ID') + '" "' + asEnv('JD_TAGGED_IMAGE_NAME') + '"' } return taggedImageName; } @@ -182,7 +188,7 @@ class Docker implements Serializable { // That's ok since tagging is cheap. def taggedImageName = tag(tagName, force) docker.script.withEnv(["JD_TAGGED_IMAGE_NAME=${taggedImageName}"]) { - docker.script."${docker.shell()}" 'docker push "$JD_TAGGED_IMAGE_NAME"' + docker.script."${docker.shell()}" 'docker push "' + asEnv('JD_TAGGED_IMAGE_NAME') + '"' } } } @@ -201,13 +207,13 @@ class Docker implements Serializable { public void stop() { docker.script.withEnv(["JD_ID=${id}"]) { - docker.script."${docker.shell()}" 'docker stop "$JD_ID" && docker rm -f "$JD_ID"' + docker.script."${docker.shell()}" 'docker stop "' + asEnv('JD_ID') + '" && docker rm -f "' + asEnv('JD_ID') + '"' } } public String port(int port) { docker.script.withEnv(["JD_ID=${id}", "JD_PORT=${port}"]) { - docker.script."${docker.shell()}"(script: 'docker port "$JD_ID" "$JD_PORT"', returnStdout: true).trim() + docker.script."${docker.shell()}"(script: 'docker port "' + asEnv('JD_ID') + '" "' + asEnv('JD_PORT') + '"', returnStdout: true).trim() } } } From 822b140f413a9dfd1cf1fee28488b8267a29a3f2 Mon Sep 17 00:00:00 2001 From: Michael Kriese Date: Wed, 19 Jan 2022 14:41:28 +0100 Subject: [PATCH 2/4] fix: wrong method access --- .../plugins/docker/workflow/Docker.groovy | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy b/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy index b3581f882..cbdd8dec0 100644 --- a/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy +++ b/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy @@ -128,11 +128,11 @@ class Docker implements Serializable { docker.node { def toRun = imageName() docker.script.withEnv(["JD_ID=${id}", "JD_TO_RUN=${toRun}"]) { - if (toRun != id && docker.script."${docker.shell()}"(script: 'docker inspect -f . "' + asEnv('JD_ID') + '"', returnStatus: true) == 0) { + if (toRun != id && docker.script."${docker.shell()}"(script: 'docker inspect -f . "' + docker.asEnv('JD_ID') + '"', returnStatus: true) == 0) { // Can run it without registry prefix, because it was locally built. toRun = id } else { - if (docker.script."${docker.shell()}"(script: 'docker inspect -f . "' + asEnv('JD_TO_RUN') + '"', returnStatus: true) != 0) { + if (docker.script."${docker.shell()}"(script: 'docker inspect -f . "' + docker.asEnv('JD_TO_RUN') + '"', returnStatus: true) != 0) { // Not yet present locally. // withDockerContainer requires the image to be available locally, since its start phase is not a durable task. pull() @@ -149,7 +149,7 @@ class Docker implements Serializable { docker.node { def toPull = imageName() docker.script.withEnv(["JD_TO_PULL=${toPull}"]) { - docker.script."${docker.shell()}" 'docker pull "' + asEnv('JD_TO_PULL') + '"' + docker.script."${docker.shell()}" 'docker pull "' + docker.asEnv('JD_TO_PULL') + '"' } } } @@ -176,7 +176,7 @@ class Docker implements Serializable { docker.node { def taggedImageName = toQualifiedImageName(parsedId.userAndRepo + ':' + tagName) docker.script.withEnv(["JD_ID=${id}", "JD_TAGGED_IMAGE_NAME=${taggedImageName}"]) { - docker.script."${docker.shell()}" 'docker tag "' + asEnv('JD_ID') + '" "' + asEnv('JD_TAGGED_IMAGE_NAME') + '"' + docker.script."${docker.shell()}" 'docker tag "' + docker.asEnv('JD_ID') + '" "' + docker.asEnv('JD_TAGGED_IMAGE_NAME') + '"' } return taggedImageName; } @@ -188,7 +188,7 @@ class Docker implements Serializable { // That's ok since tagging is cheap. def taggedImageName = tag(tagName, force) docker.script.withEnv(["JD_TAGGED_IMAGE_NAME=${taggedImageName}"]) { - docker.script."${docker.shell()}" 'docker push "' + asEnv('JD_TAGGED_IMAGE_NAME') + '"' + docker.script."${docker.shell()}" 'docker push "' + docker.asEnv('JD_TAGGED_IMAGE_NAME') + '"' } } } @@ -207,13 +207,13 @@ class Docker implements Serializable { public void stop() { docker.script.withEnv(["JD_ID=${id}"]) { - docker.script."${docker.shell()}" 'docker stop "' + asEnv('JD_ID') + '" && docker rm -f "' + asEnv('JD_ID') + '"' + docker.script."${docker.shell()}" 'docker stop "' + docker.asEnv('JD_ID') + '" && docker rm -f "' + docker.asEnv('JD_ID') + '"' } } public String port(int port) { docker.script.withEnv(["JD_ID=${id}", "JD_PORT=${port}"]) { - docker.script."${docker.shell()}"(script: 'docker port "' + asEnv('JD_ID') + '" "' + asEnv('JD_PORT') + '"', returnStdout: true).trim() + docker.script."${docker.shell()}"(script: 'docker port "' + docker.asEnv('JD_ID') + '" "' + docker.asEnv('JD_PORT') + '"', returnStdout: true).trim() } } } From a7efdb65d4f9278a9dc64d235914bfb0ad33027e Mon Sep 17 00:00:00 2001 From: Michael Kriese Date: Thu, 27 Jan 2022 13:35:33 +0100 Subject: [PATCH 3/4] chore: refactor node calls --- .../plugins/docker/workflow/Docker.groovy | 44 ++++++++++--------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy b/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy index cbdd8dec0..7fc57b030 100644 --- a/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy +++ b/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy @@ -75,24 +75,21 @@ class Docker implements Serializable { new Image(this, id) } - String shell() { - node { - script.isUnix() ? "sh" : "bat" - } + String shell(boolean isUnix) { + isUnix ? "sh" : "bat" } - String asEnv(String var) { - node { - script.isUnix() ? "$${var}" : "%${var}%" - } + String asEnv(boolean isUnix, String var) { + isUnix ? "$${var}" : "%${var}%" } public Image build(String image, String args = '.') { check(image) node { - def commandLine = 'docker build -t "' + asEnv('JD_IMAGE') + '" ' + args + def isUnix = script.isUnix() + def commandLine = 'docker build -t "' + asEnv(isUnix, 'JD_IMAGE') + '" ' + args script.withEnv(["JD_IMAGE=${image}"]) { - script."${shell()}" commandLine + script."${shell(isUnix)}" commandLine } this.image(image) } @@ -127,12 +124,13 @@ class Docker implements Serializable { public V inside(String args = '', Closure body) { docker.node { def toRun = imageName() + def isUnix = docker.script.isUnix() docker.script.withEnv(["JD_ID=${id}", "JD_TO_RUN=${toRun}"]) { - if (toRun != id && docker.script."${docker.shell()}"(script: 'docker inspect -f . "' + docker.asEnv('JD_ID') + '"', returnStatus: true) == 0) { + if (toRun != id && docker.script."${docker.shell(isUnix)}"(script: 'docker inspect -f . "' + docker.asEnv(isUnix, 'JD_ID') + '"', returnStatus: true) == 0) { // Can run it without registry prefix, because it was locally built. toRun = id } else { - if (docker.script."${docker.shell()}"(script: 'docker inspect -f . "' + docker.asEnv('JD_TO_RUN') + '"', returnStatus: true) != 0) { + if (docker.script."${docker.shell(isUnix)}"(script: 'docker inspect -f . "' + docker.asEnv(isUnix, 'JD_TO_RUN') + '"', returnStatus: true) != 0) { // Not yet present locally. // withDockerContainer requires the image to be available locally, since its start phase is not a durable task. pull() @@ -148,16 +146,18 @@ class Docker implements Serializable { public void pull() { docker.node { def toPull = imageName() + def isUnix = docker.script.isUnix() docker.script.withEnv(["JD_TO_PULL=${toPull}"]) { - docker.script."${docker.shell()}" 'docker pull "' + docker.asEnv('JD_TO_PULL') + '"' + docker.script."${docker.shell(isUnix)}" 'docker pull "' + docker.asEnv(isUnix, 'JD_TO_PULL') + '"' } } } public Container run(String args = '', String command = "") { docker.node { - def container = docker.script."${docker.shell()}"(script: "docker run -d${args != '' ? ' ' + args : ''} ${id}${command != '' ? ' ' + command : ''}", returnStdout: true).trim() - new Container(docker, container) + def isUnix = docker.script.isUnix() + def container = docker.script."${docker.shell(isUnix)}"(script: "docker run -d${args != '' ? ' ' + args : ''} ${id}${command != '' ? ' ' + command : ''}", returnStdout: true).trim() + new Container(docker, container, isUnix) } } @@ -175,8 +175,9 @@ class Docker implements Serializable { public void tag(String tagName = parsedId.tag, boolean force = true) { docker.node { def taggedImageName = toQualifiedImageName(parsedId.userAndRepo + ':' + tagName) + def isUnix = docker.script.isUnix() docker.script.withEnv(["JD_ID=${id}", "JD_TAGGED_IMAGE_NAME=${taggedImageName}"]) { - docker.script."${docker.shell()}" 'docker tag "' + docker.asEnv('JD_ID') + '" "' + docker.asEnv('JD_TAGGED_IMAGE_NAME') + '"' + docker.script."${docker.shell(isUnix)}" 'docker tag "' + docker.asEnv(isUnix, 'JD_ID') + '" "' + docker.asEnv(isUnix, 'JD_TAGGED_IMAGE_NAME') + '"' } return taggedImageName; } @@ -187,8 +188,9 @@ class Docker implements Serializable { // The image may have already been tagged, so the tagging may be a no-op. // That's ok since tagging is cheap. def taggedImageName = tag(tagName, force) + def isUnix = docker.script.isUnix() docker.script.withEnv(["JD_TAGGED_IMAGE_NAME=${taggedImageName}"]) { - docker.script."${docker.shell()}" 'docker push "' + docker.asEnv('JD_TAGGED_IMAGE_NAME') + '"' + docker.script."${docker.shell(isUnix)}" 'docker push "' + docker.asEnv(isUnix, 'JD_TAGGED_IMAGE_NAME') + '"' } } } @@ -198,22 +200,24 @@ class Docker implements Serializable { public static class Container implements Serializable { private final Docker docker; + private final boolean isUnix; public final String id; - private Container(Docker docker, String id) { + private Container(Docker docker, String id, boolean isUnix) { this.docker = docker this.id = id + this.isUnix = isUnix; } public void stop() { docker.script.withEnv(["JD_ID=${id}"]) { - docker.script."${docker.shell()}" 'docker stop "' + docker.asEnv('JD_ID') + '" && docker rm -f "' + docker.asEnv('JD_ID') + '"' + docker.script."${docker.shell(isUnix)}" 'docker stop "' + docker.asEnv(isUnix,'JD_ID') + '" && docker rm -f "' + docker.asEnv(isUnix, 'JD_ID') + '"' } } public String port(int port) { docker.script.withEnv(["JD_ID=${id}", "JD_PORT=${port}"]) { - docker.script."${docker.shell()}"(script: 'docker port "' + docker.asEnv('JD_ID') + '" "' + docker.asEnv('JD_PORT') + '"', returnStdout: true).trim() + docker.script."${docker.shell(isUnix)}"(script: 'docker port "' + docker.asEnv(isUnix, 'JD_ID') + '" "' + docker.asEnv(isUnix, 'JD_PORT') + '"', returnStdout: true).trim() } } } From b4d40ba1093584df2e00d2ad828414e7ef248a29 Mon Sep 17 00:00:00 2001 From: Michael Kriese Date: Thu, 27 Jan 2022 14:01:44 +0100 Subject: [PATCH 4/4] fix: escape dollar --- .../org/jenkinsci/plugins/docker/workflow/Docker.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy b/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy index 7fc57b030..fa7b6ead1 100644 --- a/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy +++ b/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy @@ -80,7 +80,7 @@ class Docker implements Serializable { } String asEnv(boolean isUnix, String var) { - isUnix ? "$${var}" : "%${var}%" + isUnix ? "\$${var}" : "%${var}%" } public Image build(String image, String args = '.') {