Repository: cdk8s-team/cdk8s-plus
Pull request #1110 (2022-09-08 07:30:25)

feat(container): multiple ports and advanced config properties
- Support for all configuration properties of a port (e.g protocol, name...not just the number)
- Support for multiple container ports

Resolves https://github.com/cdk8s-team/cdk8s-plus/issues/28


Patch:
diff --git a/src/_action.ts b/src/_action.ts
index c9082229..a1c7bc6c 100644
--- a/src/_action.ts
+++ b/src/_action.ts
@@ -12,7 +12,7 @@ export class Action {
 
   public static fromTcpSocket(container: Container, options: { port?: number; host?: string } = {}): k8s.TcpSocketAction {
     return {
-      port: k8s.IntOrString.fromNumber(options.port ?? container.port ?? 80),
+      port: k8s.IntOrString.fromNumber(options.port ?? container.portNumber ?? 80),
       host: options.host,
     };
   }
@@ -24,7 +24,7 @@ export class Action {
   public static fromHttpGet(container: Container, path: string, options: { port?: number; scheme?: ConnectionScheme } = { }): k8s.HttpGetAction {
     return {
       path,
-      port: k8s.IntOrString.fromNumber(options.port ?? container.port ?? 80),
+      port: k8s.IntOrString.fromNumber(options.port ?? container.portNumber ?? 80),
       scheme: options.scheme ?? ConnectionScheme.HTTP,
     };
   }
diff --git a/src/container.ts b/src/container.ts
index 6576ecf5..c556132d 100644
--- a/src/container.ts
+++ b/src/container.ts
@@ -55,6 +55,71 @@ export interface ContainerSecurityContextProps {
 
 }
 
+/**
+ * Represents a network port in a single container.
+ */
+export interface ContainerPort {
+
+  /**
+   * Number of port to expose on the pod's IP address. This must be a valid port number, 0 < x < 65536.
+   */
+  readonly number: number;
+
+  /**
+   * What host IP to bind the external port to.
+   *
+   * @default - 127.0.0.1.
+   */
+  readonly hostIp?: string;
+
+  /**
+   * Number of port to expose on the host. If specified, this must be a valid port number, 0 < x < 65536.
+   * Most containers do not need this.
+   *
+   * @default - auto generated by kubernetes and might change on restarts.
+   */
+  readonly hostPort?: number;
+
+  /**
+   * If specified, this must be an IANA_SVC_NAME and unique within the pod.
+   * Each named port in a pod must have a unique name.
+   * Name for the port that can be referred to by services.
+   *
+   * @default - port is not named.
+   */
+  readonly name?: string;
+
+  /**
+   * Protocol for port. Must be UDP, TCP, or SCTP. Defaults to "TCP".
+   *
+   * @default Protocol.TCP
+   */
+  readonly protocol?: Protocol;
+
+}
+
+/**
+ * Network protocols.
+ */
+export enum Protocol {
+
+  /**
+   * TCP.
+   */
+  TCP = 'TCP',
+
+  /**
+   * UDP.
+   */
+  UDP = 'UDP',
+
+  /**
+   * SCTP.
+   */
+  SCTP = 'SCTP'
+}
+
+
 /**
  * Container security attributes and settings.
  */
@@ -434,12 +499,29 @@ export interface ContainerProps {
    */
   readonly name?: string;
 
+  /**
+   * @deprecated - use `portNumber`.
+   */
+  readonly port?: number;
+
   /**
    * Number of port to expose on the pod's IP address. This must be a valid port number, 0 < x < 65536.
    *
-   * @default - No port is exposed.
+   * This is a convinience property if all you need a single TCP numbered port.
+   * In case more advanced configuartion is required, use the `ports` property.
+   *
+   * This port is added to the list of ports mentioned in the `ports` property.
+   *
+   * @default - Only the ports mentiond in the `ports` property are exposed.
    */
-  readonly port?: number;
+  readonly portNumber?: number;
+
+  /**
+   * List of ports to expose from this container.
+   *
+   * @default - Only the port mentioned in the `portNumber` property is exposed.
+   */
+  readonly ports?: ContainerPort[];
 
   /**
    * Entrypoint array. Not executed within a shell. The docker image's ENTRYPOINT is used if this is not provided. Variable references $(VAR_NAME) are expanded using the container's environment.
@@ -564,9 +646,11 @@ export interface ContainerProps {
 export class Container {
 
   /**
-   * The port this container exposes.
+   * The port number that was configured for this container.
+   * If undefined, either the container doesn't expose a port, or its
+   * port configuration is stored in the `ports` field.
    */
-  public readonly port?: number;
+  public readonly portNumber?: number;
 
   /**
    * Volume mounts configured for this container.
@@ -611,6 +695,7 @@ export class Container {
 
   private readonly _command?: readonly string[];
   private readonly _args?: readonly string[];
+  private readonly _ports: ContainerPort[] = [];
   private readonly _readiness?: probe.Probe;
   private readonly _liveness?: probe.Probe;
   private readonly _startup?: probe.Probe;
@@ -632,9 +717,13 @@ export class Container {
       },
     };
 
+    if (props.port && props.portNumber) {
+      throw new Error('Either \'port\' or \'portNumber\' is allowed. Use \'portNumber\' since \'port\' is deprecated');
+    }
+
     this.name = props.name ?? 'main';
     this.image = props.image;
-    this.port = props.port;
+    this.portNumber = props.portNumber ?? props.port;
     this._command = props.command;
     this._args = props.args;
     this._readiness = props.readiness;
@@ -647,6 +736,31 @@ export class Container {
     this.imagePullPolicy = props.imagePullPolicy ?? ImagePullPolicy.ALWAYS;
     this.securityContext = new ContainerSecurityContext(props.securityContext);
     this.env = new Env(props.envFrom ?? [], props.envVariables ?? {});
+
+    if (this.portNumber) {
+      this.addPort({
+        number: this.portNumber,
+      });
+    }
+
+    for (const port of props.ports ?? []) {
+      this.addPort(port);
+    }
+  }
+
+  /**
+   * @deprecated - use `portNumber`.
+   */
+  public get port(): number | undefined {
+    return this.portNumber;
+  }
+
+  /**
+   * Ports exposed by this containers.
+   * Returns a copy, use `addPort` to modify.
+   */
+  public get ports(): ContainerPort[] {
+    return [...this._ports];
   }
 
   /**
@@ -677,6 +791,25 @@ export class Container {
     this.mounts.push({ path, volume: storage.asVolume(), ...options });
   }
 
+  /**
+   * Add a port to expose from this container.
+   */
+  public addPort(port: ContainerPort) {
+
+    const names = this._ports.map(p => p.name).filter(x => x);
+    const numbers = this._ports.map(p => p.number);
+
+    if (port.name && names.includes(port.name)) {
+      throw new Error(`Port with name ${port.name} already exists`);
+    }
+
+    if (numbers.includes(port.number)) {
+      throw new Error(`Port with number ${port.number} already exists`);
+    }
+
+    this._ports.push(port);
+  }
+
   /**
    * @internal
    */
@@ -696,9 +829,13 @@ export class Container {
 
     const ports = new Array<k8s.ContainerPort>();
 
-    if (this.port) {
+    for (const port of this.ports) {
       ports.push({
-        containerPort: this.port,
+        containerPort: port.number,
+        protocol: port.protocol?.toString(),
+        name: port.name,
+        hostPort: port.hostPort,
+        hostIp: port.hostIp,
       });
     }
 
@@ -952,23 +1089,15 @@ export class EnvFrom {
 
 }
 
-export function extractContainerPorts(selector?: any): number[] {
+export function extractContainerPorts(selector?: any): ContainerPort[] {
 
   if (!selector) { return []; }
 
-  const ports = [];
-
   // we don't use instanceof intentionally since it can create
   // cyclic import problems.
-  const containers: Container[] = (selector as any).containers;
-
-  for (const con of containers ?? []) {
-    if (con.port) {
-      ports.push(con.port);
-    }
-  }
+  const containers: Container[] = (selector as any).containers ?? [];
 
-  return ports;
+  return containers.flatMap(c => c.ports);
 }
 
 /**
diff --git a/src/deployment.ts b/src/deployment.ts
index 958af7e6..5c642fdc 100644
--- a/src/deployment.ts
+++ b/src/deployment.ts
@@ -168,13 +168,23 @@ export class Deployment extends workload.Workload {
    * @param options Options to determine details of the service and port exposed.
    */
   public exposeViaService(options: DeploymentExposeViaServiceOptions = {}): service.Service {
-    const ports = options.ports ?? this.extractPorts();
+    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 }));
     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');
     }
 
+    // validate the ports are owned by our containers
+    for (const port of ports) {
+      const targetPort = port.targetPort ?? port.port;
+      if (!myPortNumbers.includes(targetPort)) {
+        throw new Error(`Unable to expose deployment ${this.name} via a service: Port ${targetPort} is not exposed by any container`);
+      }
+    }
+
     const metadata: any = { namespace: this.metadata.namespace };
     if (options.name) {
       metadata.name = options.name;
@@ -200,10 +210,6 @@ export class Deployment extends workload.Workload {
     return ser.exposeViaIngress(path, options);
   }
 
-  private extractPorts(): service.ServicePort[] {
-    return container.extractContainerPorts(this).map(port => ({ targetPort: port, port }));
-  }
-
   /**
    * @internal
    */
diff --git a/src/pod.ts b/src/pod.ts
index 8984e115..d5dc3723 100644
--- a/src/pod.ts
+++ b/src/pod.ts
@@ -1751,6 +1751,6 @@ export class PodConnections {
   }
 
   private extractPorts(selector?: networkpolicy.INetworkPolicyPeer): networkpolicy.NetworkPolicyPort[] {
-    return container.extractContainerPorts(selector).map(n => networkpolicy.NetworkPolicyPort.tcp(n));
+    return container.extractContainerPorts(selector).map(n => networkpolicy.NetworkPolicyPort.tcp(n.number));
   }
 }
diff --git a/src/service.ts b/src/service.ts
index 5d0463d8..713fa8fb 100644
--- a/src/service.ts
+++ b/src/service.ts
@@ -1,6 +1,7 @@
 import { ApiObject, Lazy } from 'cdk8s';
 import { Construct } from 'constructs';
 import * as base from './base';
+import * as container from './container';
 import * as k8s from './imports/k8s';
 import * as ingress from './ingress';
 import * as pod from './pod';
@@ -336,12 +337,6 @@ export class Service extends base.Resource {
   }
 }
 
-export enum Protocol {
-  TCP = 'TCP',
-  UDP = 'UDP',
-  SCTP = 'SCTP'
-}
-
 /**
  * Options for `Service.bind`.
  */
@@ -372,7 +367,7 @@ export interface ServiceBindOptions {
    *
    * @default Protocol.TCP
    */
-  readonly protocol?: Protocol;
+  readonly protocol?: container.Protocol;
 
   /**
    * The port number the service will redirect to.


Test patch:
diff --git a/test/__snapshots__/deployment.test.ts.snap b/test/__snapshots__/deployment.test.ts.snap
index 2c6ef51e..f7fcecfb 100644
--- a/test/__snapshots__/deployment.test.ts.snap
+++ b/test/__snapshots__/deployment.test.ts.snap
@@ -123,6 +123,107 @@ Array [
 ]
 `;
 
+exports[`expose captures all container ports 1`] = `
+Array [
+  Object {
+    "apiVersion": "apps/v1",
+    "kind": "Deployment",
+    "metadata": Object {
+      "name": "test-deployment-c898c72d",
+    },
+    "spec": Object {
+      "minReadySeconds": 0,
+      "progressDeadlineSeconds": 600,
+      "replicas": 1,
+      "selector": Object {
+        "matchLabels": Object {
+          "cdk8s.io/metadata.addr": "test-Deployment-c83f5e59",
+        },
+      },
+      "strategy": Object {
+        "rollingUpdate": Object {
+          "maxSurge": "25%",
+          "maxUnavailable": "25%",
+        },
+        "type": "RollingUpdate",
+      },
+      "template": Object {
+        "metadata": Object {
+          "labels": Object {
+            "cdk8s.io/metadata.addr": "test-Deployment-c83f5e59",
+          },
+        },
+        "spec": Object {
+          "automountServiceAccountToken": true,
+          "containers": Array [
+            Object {
+              "image": "image",
+              "imagePullPolicy": "Always",
+              "name": "main",
+              "ports": Array [
+                Object {
+                  "containerPort": 8080,
+                },
+                Object {
+                  "containerPort": 9090,
+                },
+              ],
+              "resources": Object {
+                "limits": Object {
+                  "cpu": "1500m",
+                  "memory": "2048Mi",
+                },
+                "requests": Object {
+                  "cpu": "1000m",
+                  "memory": "512Mi",
+                },
+              },
+              "securityContext": Object {
+                "privileged": false,
+                "readOnlyRootFilesystem": false,
+                "runAsGroup": 26000,
+                "runAsNonRoot": false,
+                "runAsUser": 25000,
+              },
+            },
+          ],
+          "dnsPolicy": "ClusterFirst",
+          "securityContext": Object {
+            "fsGroupChangePolicy": "Always",
+            "runAsNonRoot": false,
+          },
+          "setHostnameAsFQDN": false,
+        },
+      },
+    },
+  },
+  Object {
+    "apiVersion": "v1",
+    "kind": "Service",
+    "metadata": Object {
+      "name": "test-deployment-service-c870ff98",
+    },
+    "spec": Object {
+      "externalIPs": Array [],
+      "ports": Array [
+        Object {
+          "port": 8080,
+          "targetPort": 8080,
+        },
+        Object {
+          "port": 9090,
+          "targetPort": 9090,
+        },
+      ],
+      "selector": Object {
+        "cdk8s.io/metadata.addr": "test-Deployment-c83f5e59",
+      },
+      "type": "ClusterIP",
+    },
+  },
+]
+`;
+
 exports[`exposing via a service preserves deployment namespace 1`] = `
 Array [
   Object {
diff --git a/test/container.test.ts b/test/container.test.ts
index 577d0acc..00a487ff 100644
--- a/test/container.test.ts
+++ b/test/container.test.ts
@@ -151,6 +151,81 @@ describe('EnvValue', () => {
 
 describe('Container', () => {
 
+  test('cannot configure identical ports at instantiation', () => {
+
+    expect(() => new kplus.Container({
+      image: 'image',
+      ports: [
+        {
+          number: 8080,
+        },
+        {
+          number: 8080,
+        },
+      ],
+    })).toThrowError('Port with number 8080 already exists');
+
+  });
+
+  test('cannot add an already existing port number', () => {
+
+    const container = new kplus.Container({
+      image: 'image',
+      ports: [{
+        number: 8080,
+      }],
+    });
+
+    expect(() => container.addPort({ number: 8080 })).toThrowError('Port with number 8080 already exists');
+
+  });
+
+  test('cannot add an already existing port name', () => {
+
+    const container = new kplus.Container({
+      image: 'image',
+      ports: [{
+        number: 8080,
+        name: 'port1',
+      }],
+    });
+
+    expect(() => container.addPort({ number: 9090, name: 'port1' })).toThrowError('Port with name port1 already exists');
+
+  });
+
+  test('can configure multiple ports', () => {
+
+    const container = new kplus.Container({
+      image: 'image',
+      ports: [{
+        number: 8080,
+      }],
+    });
+
+    container.addPort({ number: 9090 });
+    expect(container._toKube().ports).toEqual([{ containerPort: 8080 }, { containerPort: 9090 }]);
+    expect(container.ports).toEqual([{ number: 8080 }, { number: 9090 }]);
+
+  });
+
+  test('portNumber is equivalent to port', () => {
+
+    const container1 = new kplus.Container({
+      image: 'image',
+      port: 9000,
+    });
+
+    const container2 = new kplus.Container({
+      image: 'image',
+      portNumber: 9000,
+    });
+
+    expect(container1._toKube()).toEqual(container2._toKube());
+    expect(container1.portNumber).toEqual(container1.port);
+
+  });
+
   test('Instantiation properties are all respected', () => {
 
     const container = new kplus.Container({
diff --git a/test/deployment.test.ts b/test/deployment.test.ts
index d822241e..026e21a7 100644
--- a/test/deployment.test.ts
+++ b/test/deployment.test.ts
@@ -125,7 +125,7 @@ test('Expose can set service and port details', () => {
     containers: [
       {
         image: 'image',
-        port: 9300,
+        port: 9500,
       },
     ],
   });
@@ -632,4 +632,53 @@ test('exposing via a service preserves deployment namespace', () => {
   deployment.exposeViaService();
   expect(Testing.synth(chart)).toMatchSnapshot();
 
+});
+
+test('expose captures all container ports', () => {
+
+  const chart = Testing.chart();
+
+  const deployment = new kplus.Deployment(chart, 'Deployment', {
+    containers: [{
+      image: 'image',
+      ports: [
+        {
+          number: 8080,
+        },
+        {
+          number: 9090,
+        },
+      ],
+    }],
+  });
+
+  deployment.exposeViaService();
+  expect(Testing.synth(chart)).toMatchSnapshot();
+
+});
+
+test('cannot expose with a port not owned by the container', () => {
+
+  const chart = Testing.chart();
+
+  const deployment = new kplus.Deployment(chart, 'Deployment', {
+    containers: [{
+      image: 'image',
+      ports: [
+        {
+          number: 8080,
+        },
+        {
+          number: 9090,
+        },
+      ],
+    }],
+  });
+
+  // this should fail because none of the containers in this
+  // deployment expose port 2020.
+  expect(() => deployment.exposeViaService({
+    ports: [{ port: 2020 }],
+  })).toThrowError('Unable to expose deployment');
+
 });
\ No newline at end of file