From a314e01434e7010e4999faa93d9bd017a92742a1 Mon Sep 17 00:00:00 2001 From: ericlapier <157530298+ericlapier@users.noreply.github.com> Date: Tue, 23 Jan 2024 17:14:39 -0500 Subject: [PATCH 1/4] Port Bindings is not respected for SSH port 1- Dashboard > Manage Jenkins > Configure Clouds 2- Add a new Cloud 3- Connect method SSH 4- Container settings 5- Port Bindings 0.0.0.0:20000-20100:22 Expected Results All docker-proxy process should use a port between 20000 and 20100 Tested for a year now with this fix and docker-proxy always gets a port in the specified range. --- .../docker/connector/DockerComputerSSHConnector.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/jenkins/docker/connector/DockerComputerSSHConnector.java b/src/main/java/io/jenkins/docker/connector/DockerComputerSSHConnector.java index 029d2bc2d..b816e3fc4 100644 --- a/src/main/java/io/jenkins/docker/connector/DockerComputerSSHConnector.java +++ b/src/main/java/io/jenkins/docker/connector/DockerComputerSSHConnector.java @@ -278,8 +278,10 @@ public void beforeContainerCreated(DockerAPI api, String workdir, CreateContaine } final Ports portBindings = hostConfig.getPortBindings(); if (portBindings != null) { - portBindings.add(sshPortBinding); - hostConfig.withPortBindings(portBindings); + if(portBindings.getBindings().get(sshPortBinding.getExposedPort()).length == 0) { + portBindings.add(sshPortBinding); + hostConfig.withPortBindings(portBindings); + } } else { hostConfig.withPortBindings(sshPortBinding); } From d97402bbdc5a5a1e19c2134b64f97f0ab249ff77 Mon Sep 17 00:00:00 2001 From: ericlapier <157530298+ericlapier@users.noreply.github.com> Date: Mon, 3 Jun 2024 13:34:06 -0400 Subject: [PATCH 2/4] Update DockerComputerSSHConnector.java --- .../docker/connector/DockerComputerSSHConnector.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/jenkins/docker/connector/DockerComputerSSHConnector.java b/src/main/java/io/jenkins/docker/connector/DockerComputerSSHConnector.java index b816e3fc4..59b130f7e 100644 --- a/src/main/java/io/jenkins/docker/connector/DockerComputerSSHConnector.java +++ b/src/main/java/io/jenkins/docker/connector/DockerComputerSSHConnector.java @@ -278,10 +278,12 @@ public void beforeContainerCreated(DockerAPI api, String workdir, CreateContaine } final Ports portBindings = hostConfig.getPortBindings(); if (portBindings != null) { - if(portBindings.getBindings().get(sshPortBinding.getExposedPort()).length == 0) { - portBindings.add(sshPortBinding); - hostConfig.withPortBindings(portBindings); + final Ports.Binding[] portBinding = portBindings.getBindings().get(sshPortBinding.getExposedPort()); + // Only add default ssh port Binding when it is not already configured + if (portBinding == null || portBinding.length == 0) { + portBindings.add(sshPortBinding); } + hostConfig.withPortBindings(portBindings); } else { hostConfig.withPortBindings(sshPortBinding); } From 68cede631f7df87ba942c0664b3e801c92a279e0 Mon Sep 17 00:00:00 2001 From: ericlapier <157530298+ericlapier@users.noreply.github.com> Date: Mon, 3 Jun 2024 13:35:46 -0400 Subject: [PATCH 3/4] Update DockerComputerSSHConnectorTest.java --- .../DockerComputerSSHConnectorTest.java | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/test/java/io/jenkins/docker/connector/DockerComputerSSHConnectorTest.java b/src/test/java/io/jenkins/docker/connector/DockerComputerSSHConnectorTest.java index 27a0fdaee..f94a09903 100644 --- a/src/test/java/io/jenkins/docker/connector/DockerComputerSSHConnectorTest.java +++ b/src/test/java/io/jenkins/docker/connector/DockerComputerSSHConnectorTest.java @@ -128,4 +128,46 @@ public void testPortBinding() throws IOException, InterruptedException { final String actualHostPortSpecForPort22 = actualBindingsForPort22[0].getHostPortSpec(); Assert.assertNull(actualHostPortSpecForPort22); } + + @Test + public void testPortBindingPort22() throws IOException, InterruptedException { + // Given + DockerComputerSSHConnector connector = + new DockerComputerSSHConnector(Mockito.mock(DockerComputerSSHConnector.SSHKeyStrategy.class)); + CreateContainerCmdImpl cmd = new CreateContainerCmdImpl( + Mockito.mock(CreateContainerCmd.Exec.class), Mockito.mock(AuthConfig.class), ""); + HostConfig hostConfig = cmd.getHostConfig(); + if (hostConfig == null) { + hostConfig = new HostConfig(); + cmd.withHostConfig(hostConfig); + } + final PortBinding exportContainerPort42 = PortBinding.parse("42:42"); + final PortBinding exportContainerPort22 = PortBinding.parse("3022:22"); + hostConfig.withPortBindings(exportContainerPort42, exportContainerPort22); + final ExposedPort port42 = new ExposedPort(42); + final ExposedPort port22 = new ExposedPort(22); + + // When + connector.setPort(22); + connector.beforeContainerCreated(Mockito.mock(DockerAPI.class), "/workdir", cmd); + + // Then + final Ports actualPortBindings = cmd.getHostConfig().getPortBindings(); + Assert.assertNotNull(actualPortBindings); + final Map actualBindingMap = actualPortBindings.getBindings(); + Assert.assertNotNull(actualBindingMap); + Assert.assertEquals(2, actualBindingMap.size()); + + final Ports.Binding[] actualBindingsForPort42 = actualBindingMap.get(port42); + Assert.assertNotNull(actualBindingsForPort42); + Assert.assertEquals(1, actualBindingsForPort42.length); + final String actualHostPortSpecForPort42 = actualBindingsForPort42[0].getHostPortSpec(); + Assert.assertEquals("42", actualHostPortSpecForPort42); + + final Ports.Binding[] actualBindingsForPort22 = actualBindingMap.get(port22); + Assert.assertNotNull(actualBindingsForPort22); + Assert.assertEquals(1, actualBindingsForPort22.length); + final String actualHostPortSpecForPort22 = actualBindingsForPort22[0].getHostPortSpec(); + Assert.assertEquals("3022",actualHostPortSpecForPort22); + } } From e0b69e40ba9b8e9777b88cf129d5e4eea709d4fa Mon Sep 17 00:00:00 2001 From: ericlapier <157530298+ericlapier@users.noreply.github.com> Date: Mon, 3 Jun 2024 13:58:39 -0400 Subject: [PATCH 4/4] Update DockerComputerSSHConnectorTest.java --- .../docker/connector/DockerComputerSSHConnectorTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/io/jenkins/docker/connector/DockerComputerSSHConnectorTest.java b/src/test/java/io/jenkins/docker/connector/DockerComputerSSHConnectorTest.java index f94a09903..0cf6a4761 100644 --- a/src/test/java/io/jenkins/docker/connector/DockerComputerSSHConnectorTest.java +++ b/src/test/java/io/jenkins/docker/connector/DockerComputerSSHConnectorTest.java @@ -128,7 +128,7 @@ public void testPortBinding() throws IOException, InterruptedException { final String actualHostPortSpecForPort22 = actualBindingsForPort22[0].getHostPortSpec(); Assert.assertNull(actualHostPortSpecForPort22); } - + @Test public void testPortBindingPort22() throws IOException, InterruptedException { // Given @@ -168,6 +168,6 @@ public void testPortBindingPort22() throws IOException, InterruptedException { Assert.assertNotNull(actualBindingsForPort22); Assert.assertEquals(1, actualBindingsForPort22.length); final String actualHostPortSpecForPort22 = actualBindingsForPort22[0].getHostPortSpec(); - Assert.assertEquals("3022",actualHostPortSpecForPort22); + Assert.assertEquals("3022", actualHostPortSpecForPort22); } }