diff --git a/packages/k8s/src/k8s/index.ts b/packages/k8s/src/k8s/index.ts index 721bac7..62f5e89 100644 --- a/packages/k8s/src/k8s/index.ts +++ b/packages/k8s/src/k8s/index.ts @@ -516,27 +516,37 @@ class BackOffManager { export function containerPorts( container: ContainerInfo ): k8s.V1ContainerPort[] { - // 8080:8080/tcp - const portFormat = /(\d{1,5})(:(\d{1,5}))?(\/(tcp|udp))?/ - const ports: k8s.V1ContainerPort[] = [] for (const portDefinition of container.portMappings) { - const submatches = portFormat.exec(portDefinition) - if (!submatches) { - throw new Error( - `Port definition "${portDefinition}" is in incorrect format` - ) + const portProtoSplit = portDefinition.split('/') + if (portProtoSplit.length > 2) { + throw new Error(`Unexpected port format: ${portDefinition}`) } + const port = new k8s.V1ContainerPort() - port.hostPort = Number(submatches[1]) - if (submatches[3]) { - port.containerPort = Number(submatches[3]) + port.protocol = + portProtoSplit.length === 2 ? portProtoSplit[1].toUpperCase() : 'TCP' + + const portSplit = portProtoSplit[0].split(':') + if (portSplit.length > 2) { + throw new Error('ports should have at most one ":" separator') } - if (submatches[5]) { - port.protocol = submatches[5].toUpperCase() + + const parsePort = (p: string): number => { + const num = Number(p) + if (!Number.isInteger(num) || num < 1 || num > 65535) { + throw new Error(`invalid container port: ${p}`) + } + return num + } + + if (portSplit.length === 1) { + port.containerPort = parsePort(portSplit[0]) } else { - port.protocol = 'TCP' + port.hostPort = parsePort(portSplit[0]) + port.containerPort = parsePort(portSplit[1]) } + ports.push(port) } return ports diff --git a/packages/k8s/tests/constants-test.ts b/packages/k8s/tests/constants-test.ts index 4fc1eb1..cd053fd 100644 --- a/packages/k8s/tests/constants-test.ts +++ b/packages/k8s/tests/constants-test.ts @@ -4,6 +4,7 @@ import { getSecretName, getStepPodName, getVolumeClaimName, + JOB_CONTAINER_NAME, MAX_POD_NAME_LENGTH, RunnerInstanceLabel, STEP_POD_NAME_SUFFIX_LENGTH @@ -170,4 +171,12 @@ describe('constants', () => { } }) }) + + describe('const values', () => { + it('should have constants set', () => { + expect(JOB_CONTAINER_NAME).toBeTruthy() + expect(MAX_POD_NAME_LENGTH).toBeGreaterThan(0) + expect(STEP_POD_NAME_SUFFIX_LENGTH).toBeGreaterThan(0) + }) + }) }) diff --git a/packages/k8s/tests/k8s-utils-test.ts b/packages/k8s/tests/k8s-utils-test.ts index 3d79256..4341375 100644 --- a/packages/k8s/tests/k8s-utils-test.ts +++ b/packages/k8s/tests/k8s-utils-test.ts @@ -1,5 +1,5 @@ import * as fs from 'fs' -import { POD_VOLUME_NAME } from '../src/k8s' +import { containerPorts, POD_VOLUME_NAME } from '../src/k8s' import { containerVolumes, writeEntryPointScript } from '../src/k8s/utils' import { TestHelper } from './test-setup' @@ -152,5 +152,73 @@ describe('k8s utils', () => { volumes = containerVolumes([], false, false) expect(volumes.every(e => e.name === POD_VOLUME_NAME)).toBeTruthy() }) + + it('should parse container ports', () => { + const tt = [ + { + spec: '8080:80', + want: { + containerPort: 80, + hostPort: 8080, + protocol: 'TCP' + } + }, + { + spec: '8080:80/udp', + want: { + containerPort: 80, + hostPort: 8080, + protocol: 'UDP' + } + }, + { + spec: '8080/udp', + want: { + containerPort: 8080, + hostPort: undefined, + protocol: 'UDP' + } + }, + { + spec: '8080', + want: { + containerPort: 8080, + hostPort: undefined, + protocol: 'TCP' + } + } + ] + + for (const tc of tt) { + const got = containerPorts({ portMappings: [tc.spec] }) + for (const [key, value] of Object.entries(tc.want)) { + expect(got[0][key]).toBe(value) + } + } + }) + + it('should throw when ports are out of range (0, 65536)', () => { + expect(() => containerPorts({ portMappings: ['65536'] })).toThrow() + expect(() => containerPorts({ portMappings: ['0'] })).toThrow() + expect(() => containerPorts({ portMappings: ['65536/udp'] })).toThrow() + expect(() => containerPorts({ portMappings: ['0/udp'] })).toThrow() + expect(() => containerPorts({ portMappings: ['1:65536'] })).toThrow() + expect(() => containerPorts({ portMappings: ['65536:1'] })).toThrow() + expect(() => containerPorts({ portMappings: ['1:65536/tcp'] })).toThrow() + expect(() => containerPorts({ portMappings: ['65536:1/tcp'] })).toThrow() + expect(() => containerPorts({ portMappings: ['1:'] })).toThrow() + expect(() => containerPorts({ portMappings: [':1'] })).toThrow() + expect(() => containerPorts({ portMappings: ['1:/tcp'] })).toThrow() + expect(() => containerPorts({ portMappings: [':1/tcp'] })).toThrow() + }) + + it('should throw on multi ":" splits', () => { + expect(() => containerPorts({ portMappings: ['1:1:1'] })).toThrow() + }) + + it('should throw on multi "/" splits', () => { + expect(() => containerPorts({ portMappings: ['1:1/tcp/udp'] })).toThrow() + expect(() => containerPorts({ portMappings: ['1/tcp/udp'] })).toThrow() + }) }) })