From f9e426b5ed34259f1acff03c493d531fcca01c01 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Moritz=20=27Morty=27=20Str=C3=BCbe?=
 <moritz.struebe@mathema.de>
Date: Mon, 28 Nov 2022 21:07:02 +0100
Subject: [PATCH] Bug: Docker: Do not pull all tags

If the latest tag did not exist locally and no tag was given for an
image, all tags were pulled.
---
 .../tests/DockerServerInteractions.java       | 23 +++++++++++++++++++
 .../docker/launcher/ContainerLaunchUtils.java | 10 +++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/launch/org.eclipse.cdt.docker.launcher.tests/src/org/eclipse/cdt/docker/launcher/tests/DockerServerInteractions.java b/launch/org.eclipse.cdt.docker.launcher.tests/src/org/eclipse/cdt/docker/launcher/tests/DockerServerInteractions.java
index a4952049cf2..61b400ae86f 100644
--- a/launch/org.eclipse.cdt.docker.launcher.tests/src/org/eclipse/cdt/docker/launcher/tests/DockerServerInteractions.java
+++ b/launch/org.eclipse.cdt.docker.launcher.tests/src/org/eclipse/cdt/docker/launcher/tests/DockerServerInteractions.java
@@ -13,6 +13,7 @@ package org.eclipse.cdt.docker.launcher.tests;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 import org.eclipse.cdt.internal.docker.launcher.ContainerLaunchUtils;
@@ -31,6 +32,9 @@ class DockerServerInteractions {
 	// A small image that exists
 	static final String okImage = "alpine:latest";
 	static final String imageBad = "alasdfasdfga32e:latest";
+	static final String tagTestImage = "alpine";
+	static final String tagTestImageTag = "alpine:2.7";
+
 	IDockerConnection4 cnn;
 
 	DockerServerInteractions() {
@@ -64,4 +68,23 @@ class DockerServerInteractions {
 		assertFalse(di.getMessage().isEmpty());
 	}
 
+	// You can clean up your registry using
+	// docker images | grep alpine | awk '{print $3}' | xargs docker rmi
+	@Test
+	void noPullAllTags() throws DockerException, InterruptedException {
+		// Remove latest
+		if (cnn.getImageInfo(tagTestImage + ":latest") != null) {
+			cnn.removeImage(tagTestImage + ":latest");
+		}
+		// Make sure the tag we test exists and remove it
+		{
+			final var di = ContainerLaunchUtils.provideDockerImage(null, cnn.getName(), tagTestImageTag);
+			assertTrue(di.isOK());
+			cnn.removeImage(tagTestImageTag);
+		}
+		// This should pull latest, not all tags
+		final var di = ContainerLaunchUtils.provideDockerImage(null, cnn.getName(), tagTestImage);
+		assertTrue(di.isOK());
+		assertNull(cnn.getImageInfo(tagTestImageTag));
+	}
 }
diff --git a/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/internal/docker/launcher/ContainerLaunchUtils.java b/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/internal/docker/launcher/ContainerLaunchUtils.java
index 4874814f887..3aeacb2c17f 100644
--- a/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/internal/docker/launcher/ContainerLaunchUtils.java
+++ b/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/internal/docker/launcher/ContainerLaunchUtils.java
@@ -31,6 +31,8 @@ import org.eclipse.osgi.util.NLS;
 
 public class ContainerLaunchUtils {
 
+	private static final String LATEST = ":latest"; //$NON-NLS-1$
+
 	/**
 	 * Maps the local path, to a path that is used within a docker container.
 	 * @param path The host path
@@ -103,7 +105,7 @@ public class ContainerLaunchUtils {
 	 *
 	 */
 	public static @NonNull IStatus provideDockerImage(IProgressMonitor monitor, @NonNull final String connectionName,
-			@NonNull final String imageName) {
+			@NonNull String imageName) {
 
 		// Try to pull image, if necessary
 		final var connection = (IDockerConnection4) DockerConnectionManager.getInstance()
@@ -115,6 +117,12 @@ public class ContainerLaunchUtils {
 					NLS.bind(Messages.ContainerCommandLauncher_pullerr_noConn, imageName, connectionName));
 		}
 
+		// Make sure to not pull all images if no tag is found
+		// Nothing found -> -1 ; Make sure the : comes after the last /. If neither exists, both are -1.
+		if (imageName.lastIndexOf(':') <= imageName.lastIndexOf('/')) {
+			imageName = imageName + LATEST;
+		}
+
 		// See if the image already exists
 		if (connection.getImageInfo(imageName) != null)
 			return Status.OK_STATUS;