Repository: cdk8s-team/cdk8s-plus
Pull request #2390 (2023-07-25 13:27:20)

fix(deployment): `exposeViaService` does not expose port names (#2170)
# Backport

This will backport the following commits from `k8s-26/main` to `k8s-24/main`:
 - [fix(deployment): `exposeViaService` does not expose port names (#2170)](https://github.com/cdk8s-team/cdk8s-plus/pull/2170)

<!--- Backport version: 8.5.0 -->

### Questions ?
Please refer to the [Backport tool documentation](https://github.com/sqren/backport)


Patch:
diff --git a/src/deployment.ts b/src/deployment.ts
index 1c784e41..b0a8eefd 100644
--- a/src/deployment.ts
+++ b/src/deployment.ts
@@ -177,12 +177,19 @@ export class Deployment extends workload.Workload implements IScalable {
   public exposeViaService(options: DeploymentExposeViaServiceOptions = {}): service.Service {
     const myPorts = container.extractContainerPorts(this);
     const myPortNumbers = myPorts.map(p => p.number);
-    const ports: service.ServicePort[] = options.ports ?? myPorts.map(p => ({ port: p.number, targetPort: p.number, protocol: p.protocol }));
+    const ports: service.ServicePort[] = options.ports ?? myPorts.map(p => ({
+      port: p.number, targetPort: p.number, protocol: p.protocol, name: p.name,
+    }));
     if (ports.length === 0) {
       throw new Error(`Unable to expose deployment ${this.name} via a service: `
         + 'Deployment port cannot be determined.'
         + 'Either pass \'ports\', or configure ports on the containers of the deployment');
     }
+    const portNames = ports.map(p => p.name);
+    if (ports.length > 1 && portNames.includes(undefined)) {
+      throw new Error(`Unable to expose deployment ${this.name} via a service: `
+        + 'When using multiple ports for a service, all ports must have port names so they are unambiguous.');
+    }
 
     // validate the ports are owned by our containers
     for (const port of ports) {


Test patch:
diff --git a/test/__snapshots__/deployment.test.ts.snap b/test/__snapshots__/deployment.test.ts.snap
index 77b95800..4a656e0c 100644
--- a/test/__snapshots__/deployment.test.ts.snap
+++ b/test/__snapshots__/deployment.test.ts.snap
@@ -260,9 +260,11 @@ Array [
               "ports": Array [
                 Object {
                   "containerPort": 8080,
+                  "name": "port1",
                 },
                 Object {
                   "containerPort": 9090,
+                  "name": "port2",
                 },
               ],
               "resources": Object {
@@ -306,10 +308,12 @@ Array [
       "externalIPs": Array [],
       "ports": Array [
         Object {
+          "name": "port1",
           "port": 8080,
           "targetPort": 8080,
         },
         Object {
+          "name": "port2",
           "port": 9090,
           "targetPort": 9090,
         },
diff --git a/test/deployment.test.ts b/test/deployment.test.ts
index 67b333d0..96d8eab5 100644
--- a/test/deployment.test.ts
+++ b/test/deployment.test.ts
@@ -676,9 +676,11 @@ test('expose captures all container ports', () => {
       ports: [
         {
           number: 8080,
+          name: 'port1',
         },
         {
           number: 9090,
+          name: 'port2',
         },
       ],
     }],
@@ -713,4 +715,42 @@ test('cannot expose with a port not owned by the container', () => {
     ports: [{ port: 2020 }],
   })).toThrowError('Unable to expose deployment');
 
+});
+
+test('expose via service with multiple ports throws error when names are not provided', () => {
+
+  const chart = Testing.chart();
+
+  const deployment = new kplus.Deployment(chart, 'Deployment', {
+    containers: [{
+      image: 'image',
+      ports: [
+        {
+          number: 8080,
+          name: 'port1',
+        },
+        {
+          number: 9090,
+        },
+      ],
+    }],
+  });
+
+  expect(() => deployment.exposeViaService()).toThrowError('Unable to expose deployment');
+
+  const deployment1 = new kplus.Deployment(chart, 'Deployment2', {
+    containers: [{
+      image: 'image',
+      ports: [
+        {
+          number: 8080,
+        },
+        {
+          number: 9090,
+        },
+      ],
+    }],
+  });
+
+  expect(() => deployment1.exposeViaService()).toThrowError('Unable to expose deployment');
 });
\ No newline at end of file