fix: strip ports and digests from service aliases (#1820)

Author: bidord

src/job.ts

@@ -46,7 +46,7 @@ interface Cache {
     when: "on_success" | "on_failure" | "always";
 }
 
-interface Service {
+export interface Service {
     name: string;
     entrypoint: string[] | null;
     command: string[] | null;
@@ -1583,15 +1583,8 @@ If you know what you're doing and would like to suppress this warning, use one o
             }
         }
 
-        const serviceAlias = service.alias;
         const serviceName = service.name;
-        const serviceNameWithoutVersion = serviceName.replace(/(.*)(:.*)/, "$1");
-        const aliases = new Set<string>();
-        aliases.add(serviceNameWithoutVersion.replaceAll("/", "-"));
-        aliases.add(serviceNameWithoutVersion.replaceAll("/", "__"));
-        if (serviceAlias) {
-            aliases.add(serviceAlias);
-        }
+        const aliases = Utils.getAllServiceAliases(service);
 
         for (const [key, val] of Object.entries(expanded)) {
             // Replacing `'` with `'\''` to correctly handle single quotes(if `val` contains `'`) in shell commands
@@ -1642,23 +1635,14 @@ If you know what you're doing and would like to suppress this warning, use one o
     }
 
     private async serviceHealthCheck (writeStreams: WriteStreams, service: Service, serviceIndex: number, serviceContainerLogFile: string) {
-        const serviceAlias = service.alias;
+        const serviceAlias = Utils.getServiceAlias(service);
         const serviceName = service.name;
         const waitImageName = this.argv.waitImage;
         const waitForServicesTimeout = this.argv.waitForServicesTimeout;
 
         const {stdout} = await Utils.spawn([this.argv.containerExecutable, "image", "inspect", serviceName]);
         const imageInspect = JSON.parse(stdout);
 
-        // Copied from the startService block. Important thing is that the aliases match
-        const serviceNameWithoutVersion = serviceName.replace(/(.*)(:.*)/, "$1");
-        const aliases = [serviceNameWithoutVersion.replaceAll("/", "-"), serviceNameWithoutVersion.replaceAll("/", "__")];
-        if (serviceAlias) {
-            aliases.push(serviceAlias);
-        }
-
-        const uniqueAlias = aliases[aliases.length - 1];
-
         if ((imageInspect[0]?.Config?.ExposedPorts ?? null) === null) {
             return writeStreams.stderr(chalk`${this.formattedJobName} {yellow Could not find exposed tcp ports ${serviceName}}\n`);
         }
@@ -1670,7 +1654,7 @@ If you know what you're doing and would like to suppress this warning, use one o
                 if (!port.endsWith("/tcp")) return;
                 const portNum = parseInt(port.replace("/tcp", ""));
                 const containerName = `gcl-wait-for-it-${this.jobId}-${serviceIndex}-${portNum}`;
-                const spawnCmd = [this.argv.containerExecutable, "run", "--rm", `--name=${containerName}`, "--network", `${this._serviceNetworkId}`, `${waitImageName}`, `${uniqueAlias}:${portNum}`, "-t", `${waitForServicesTimeout}`];
+                const spawnCmd = [this.argv.containerExecutable, "run", "--rm", `--name=${containerName}`, "--network", `${this._serviceNetworkId}`, `${waitImageName}`, `${serviceAlias}:${portNum}`, "-t", `${waitForServicesTimeout}`];
                 this._containersToClean.push(containerName);
                 return Utils.spawn(spawnCmd);
             }));

src/utils.ts

@@ -1,7 +1,7 @@
 import "./global.js";
 import {RE2JS} from "re2js";
 import chalk from "chalk-template";
-import {Job, JobRule, Need} from "./job.js";
+import {Job, JobRule, Need, Service} from "./job.js";
 import {needsComplex} from "./data-expander.js";
 import fs from "fs-extra";
 import checksum from "checksum";
@@ -529,4 +529,26 @@ export class Utils {
             return String(variable);
         }
     }
+
+    static getAllServiceAliases (service: Service): Set<string> {
+        const aliases = new Set<string>();
+
+        if (service.alias) {
+            aliases.add(service.alias);
+        }
+
+        // Strip any port (:443), tag (:1.2.3), or digest (@sha256:...) suffix from each path segment
+        const serviceNameWithoutVersionAndPort = service.name.replaceAll(/[:@][^/]*/g, "");
+        aliases.add(serviceNameWithoutVersionAndPort.replaceAll("/", "-"));
+        aliases.add(serviceNameWithoutVersionAndPort.replaceAll("/", "__"));
+
+        return aliases;
+    }
+
+    static getServiceAlias (service: Service): string {
+        const aliases = Utils.getAllServiceAliases(service);
+
+        // Return the first alias in the set
+        return aliases.values().next().value!;
+    }
 }

tests/test-cases/services/.gitlab-ci.yml

@@ -108,3 +108,15 @@ service-user:
   image: docker.io/library/alpine
   script:
     - echo "bye"
+
+registry-port:
+  services:
+    - registry-1.docker.io:443/library/nginx:1.29.7
+  image: docker.io/curlimages/curl:8.18.0
+  script: curl -sS http://registry-1.docker.io-library-nginx/
+
+image-digest:
+  services:
+    - docker.io/library/nginx@sha256:e7257f1ef28ba17cf7c248cb8ccf6f0c6e0228ab9c315c152f9c203cd34cf6d1
+  image: docker.io/curlimages/curl:8.18.0
+  script: curl -sS http://docker.io-library-nginx/

tests/test-cases/services/integration.test.ts

@@ -206,3 +206,33 @@ test.concurrent("services <unnamed services should be ignored>", async () => {
         stateDir: ".gitlab-ci-local-unnamed",
     }, writeStreams);
 });
+
+test.concurrent("services <registry-port>", async () => {
+    const writeStreams = new WriteStreamsMock();
+    await handler({
+        cwd: "tests/test-cases/services",
+        job: ["registry-port"],
+        stateDir: ".gitlab-ci-local-registry-port",
+    }, writeStreams);
+
+    const expected = [
+        chalk`{blueBright registry-port} {greenBright >} <title>Welcome to nginx!</title>`,
+        chalk`{black.bgGreenBright  PASS } {blueBright registry-port}`,
+    ];
+    expect(writeStreams.stdoutLines).toEqual(expect.arrayContaining(expected));
+});
+
+test.concurrent("services <image-digest>", async () => {
+    const writeStreams = new WriteStreamsMock();
+    await handler({
+        cwd: "tests/test-cases/services",
+        job: ["image-digest"],
+        stateDir: ".gitlab-ci-local-image-digest",
+    }, writeStreams);
+
+    const expected = [
+        chalk`{blueBright image-digest} {greenBright >} <title>Welcome to nginx!</title>`,
+        chalk`{black.bgGreenBright  PASS } {blueBright image-digest}`,
+    ];
+    expect(writeStreams.stdoutLines).toEqual(expect.arrayContaining(expected));
+});

tests/utils.test.ts

@@ -193,3 +193,76 @@ describe("isSubPath where process.cwd() have been mocked to return /home/user/gi
         });
     });
 });
+
+describe("getAllServiceAliases", () => {
+    const tests = [
+        {
+            input: "nginx",
+            expected: ["nginx"],
+        },
+        {
+            input: "library/nginx",
+            expected: ["library-nginx", "library__nginx"],
+        },
+        {
+            input: "docker.io/library/nginx",
+            expected: ["docker.io-library-nginx", "docker.io__library__nginx"],
+        },
+        {
+            input: "registry-1.docker.io/library/nginx",
+            expected: ["registry-1.docker.io-library-nginx", "registry-1.docker.io__library__nginx"],
+        },
+        {
+            input: "registry-1.docker.io:443/library/nginx",
+            expected: ["registry-1.docker.io-library-nginx", "registry-1.docker.io__library__nginx"],
+        },
+    ];
+
+    const suffixes = [
+        "",
+        ":1.29.7",
+        ":1.29.7@sha256:e7257f1ef28ba17cf7c248cb8ccf6f0c6e0228ab9c315c152f9c203cd34cf6d1",
+        "@sha256:e7257f1ef28ba17cf7c248cb8ccf6f0c6e0228ab9c315c152f9c203cd34cf6d1",
+    ];
+
+    tests.forEach(({input, expected}) => {
+        suffixes.forEach((suffix) => {
+            const serviceName = `${input}${suffix}`;
+            test.concurrent(`${serviceName}`, () => {
+                const service = {
+                    name: serviceName,
+                    entrypoint: null,
+                    command: null,
+                    alias: null,
+                    variables: {},
+                };
+                const aliases = Utils.getAllServiceAliases(service);
+                expect([...aliases]).toEqual(expected);
+            });
+        });
+    });
+
+    test.concurrent("should include custom alias when provided", () => {
+        const service = {
+            name: "docker.io/library/nginx:1.29.7",
+            entrypoint: null,
+            command: null,
+            alias: "my-nginx",
+            variables: {},
+        };
+        const aliases = Utils.getAllServiceAliases(service);
+        expect([...aliases]).toEqual(["my-nginx", "docker.io-library-nginx", "docker.io__library__nginx"]);
+    });
+});
+
+describe("getServiceAlias", () => {
+    const base = {entrypoint: null, command: null, variables: {}};
+
+    test.concurrent("returns - variant when no custom alias", () => {
+        expect(Utils.getServiceAlias({...base, name: "library/nginx", alias: null})).toBe("library-nginx");
+    });
+
+    test.concurrent("returns custom alias when provided", () => {
+        expect(Utils.getServiceAlias({...base, name: "library/nginx", alias: "my-nginx"})).toBe("my-nginx");
+    });
+});