diff --git a/shell/edit/networking.k8s.io.ingress/DefaultBackend.vue b/shell/edit/networking.k8s.io.ingress/DefaultBackend.vue index 7793dc29041..ffe79285eeb 100644 --- a/shell/edit/networking.k8s.io.ingress/DefaultBackend.vue +++ b/shell/edit/networking.k8s.io.ingress/DefaultBackend.vue @@ -43,7 +43,9 @@ export default { const backend = get(this.value.spec, this.value.defaultBackendPath); this.serviceName = get(backend, this.value.serviceNamePath) || ''; - this.servicePort = get(backend, this.value.servicePortPath) || ''; + this.servicePort = get(backend, this.value.servicePortPath) || + get(backend, this.value.servicePortNamePath) || + ''; }, computed: { isView() { @@ -75,10 +77,14 @@ export default { }, methods: { update() { - const backend = get(this.value.spec, this.value.defaultBackendPath) || {}; + // Fresh object so the old port path (name vs number) doesn't linger. + const backend = {}; + const parsed = Number.parseInt(this.servicePort); + const servicePort = Number.isNaN(parsed) ? this.servicePort : parsed; + const portPath = typeof servicePort === 'number' ? this.value.servicePortPath : this.value.servicePortNamePath; set(backend, this.value.serviceNamePath, this.serviceName); - set(backend, this.value.servicePortPath, this.servicePort); + set(backend, portPath, servicePort); set(this.value.spec, this.value.defaultBackendPath, backend); this.$emit('update:value', this.value); @@ -119,10 +125,12 @@ export default { class="col span-3" :style="{'margin-right': '0px'}" > + { + if (typeof port === 'string' || typeof port === 'number') { + if (!port) { + return this.t('validation.required', { key: portLabel }); + } + } else if (!port || (!port.number && !port.name)) { + return this.t('validation.required', { key: portLabel }); + } + }; + + const portRange = (port) => { + let num; + + if (typeof port === 'number') { + num = port; + } else if (typeof port === 'string') { + num = Number.parseInt(port); + } else if (port?.number) { + num = port.number; + } + + if (num !== undefined && !Number.isNaN(num) && (num < 1 || num > 65535)) { + return this.t('validation.number.between', { + key: portLabel, min: '1', max: '65535' + }); + } + }; + + const hasDefaultBackendService = () => { + const backend = get(this.value?.spec, this.value.defaultBackendPath); + + return !!get(backend, this.value.serviceNamePath); + }; + + const nameLabel = this.t('ingress.defaultBackend.targetService.label'); + + // Only enforce required on the default backend when a service is selected. + // Selecting "None" means the user wants to remove the backend; willSave() handles cleanup. + const defaultBackendNameRequired = (name) => { + if (hasDefaultBackendService() && !name) { + return this.t('validation.required', { key: nameLabel }); + } + }; + + const defaultBackendPortRequired = (port) => { + if (!hasDefaultBackendService()) { + return; + } + + return portRequired(port); + }; + + return { + backEndOrRules, + portRequired, + portRange, + defaultBackendNameRequired, + defaultBackendPortRequired, + }; }, tabErrors() { return { - rules: this.fvGetPathErrors(['spec.rules.host', 'spec.rules.http.paths.path', 'spec.rules.http.paths.backend.service.port.number', 'spec.rules.http.paths.backend.service.name'])?.length > 0, - defaultBackend: this.fvGetPathErrors(['spec.defaultBackend.service.name', 'spec.defaultBackend.service.port.number'])?.length > 0 + rules: this.fvGetPathErrors(['spec.rules.host', 'spec.rules.http.paths.path', 'spec.rules.http.paths.backend.service.port', 'spec.rules.http.paths.backend.service.name'])?.length > 0, + defaultBackend: this.fvGetPathErrors(['spec.defaultBackend.service.name', 'spec.defaultBackend.service.port'])?.length > 0 }; }, rulesPathRules() { return { requestHost: this.fvGetAndReportPathRules('spec.rules.host'), path: this.fvGetAndReportPathRules('spec.rules.http.paths.path'), - port: this.fvGetAndReportPathRules('spec.rules.http.paths.backend.service.port.number'), + port: this.fvGetAndReportPathRules('spec.rules.http.paths.backend.service.port'), target: this.fvGetAndReportPathRules('spec.rules.http.paths.backend.service.name'), }; }, defaultBackendPathRules() { - const rulesExist = (this.value?.spec?.rules || []).length > 0; - const defaultBackendExist = !!this.value?.spec?.defaultBackend?.service; - - if (!rulesExist || defaultBackendExist) { - return { - name: this.fvGetAndReportPathRules('spec.defaultBackend.service.name'), - port: this.fvGetAndReportPathRules('spec.defaultBackend.service.port.number'), - }; - } - - return { name: [], port: [] }; + return { + name: this.fvGetAndReportPathRules('spec.defaultBackend.service.name'), + port: this.fvGetAndReportPathRules('spec.defaultBackend.service.port'), + }; }, serviceTargets() { return this.ingressHelper.findAndMapServiceTargets(this.services); @@ -188,7 +242,8 @@ export default { willSave() { const backend = get(this.value.spec, this.value.defaultBackendPath); const serviceName = get(backend, this.value.serviceNamePath); - const servicePort = get(backend, this.value.servicePortPath); + const servicePort = get(backend, this.value.servicePortPath) || + get(backend, this.value.servicePortNamePath); if (backend && (!serviceName || !servicePort)) { const path = this.value.defaultBackendPath; diff --git a/shell/models/networking.k8s.io.ingress.js b/shell/models/networking.k8s.io.ingress.js index f66e4182bd0..e705b7b1c01 100644 --- a/shell/models/networking.k8s.io.ingress.js +++ b/shell/models/networking.k8s.io.ingress.js @@ -84,7 +84,7 @@ export default class Ingress extends SteveModel { serviceTargetTo: this.targetTo(workloads, serviceName), certs: this.certLinks(rule, certificates), targetLink: this.targetLink(workloads, serviceName), - port: get(path?.backend, this.servicePortPath) + port: get(path?.backend, this.servicePortPath) || get(path?.backend, this.servicePortNamePath) }; } @@ -184,6 +184,14 @@ export default class Ingress extends SteveModel { return this.useNestedBackendField ? nestedPath : flatPath; } + get servicePortNamePath() { + const nestedPath = 'service.port.name'; + // Flat API has a single `servicePort` field for both name and number. + const flatPath = 'servicePort'; + + return this.useNestedBackendField ? nestedPath : flatPath; + } + get defaultBackendPath() { const defaultBackend = this.$rootGetters['cluster/pathExistsInSchema'](this.type, 'spec.defaultBackend'); diff --git a/shell/utils/__tests__/ingress.test.ts b/shell/utils/__tests__/ingress.test.ts index 06eb05dd2dd..080ee0dcf77 100644 --- a/shell/utils/__tests__/ingress.test.ts +++ b/shell/utils/__tests__/ingress.test.ts @@ -1,5 +1,81 @@ import IngressDetailEditHelper from '@shell/utils/ingress'; import { SECRET_TYPES as TYPES } from '@shell/config/secret'; +import { get, set } from '@shell/utils/object'; + +function parseServicePort(rawPort: string | number): string | number { + const parsed = Number.parseInt(String(rawPort)); + + return Number.isNaN(parsed) ? rawPort : parsed; +} + +function portRequired(port: any): string | undefined { + if (typeof port === 'string' || typeof port === 'number') { + if (!port) { + return 'Port is required'; + } + } else if (!port || (!port.number && !port.name)) { + return 'Port is required'; + } + + return undefined; +} + +function portRange(port: any): string | undefined { + let num; + + if (typeof port === 'number') { + num = port; + } else if (typeof port === 'string') { + num = Number.parseInt(port); + } else if (port?.number) { + num = port.number; + } + + if (num !== undefined && !Number.isNaN(num) && (num < 1 || num > 65535)) { + return 'Port number must be between 1 and 65535'; + } + + return undefined; +} + +/** + * Mirrors willSave() from index.vue. + * Clears the default backend when the service name or port is missing. + */ +function willSave(spec: any, paths: { defaultBackendPath: string; serviceNamePath: string; servicePortPath: string; servicePortNamePath: string }): void { + const backend = get(spec, paths.defaultBackendPath); + const serviceName = get(backend, paths.serviceNamePath); + const servicePort = get(backend, paths.servicePortPath) || + get(backend, paths.servicePortNamePath); + + if (backend && (!serviceName || !servicePort)) { + set(spec, paths.defaultBackendPath, null); + } +} + +/** + * Mirrors defaultBackendNameRequired from index.vue. + * Only enforces required when a service is actually selected. + */ +function defaultBackendNameRequired(name: any, hasService: boolean): string | undefined { + if (hasService && !name) { + return 'Target Service is required'; + } + + return undefined; +} + +/** + * Mirrors defaultBackendPortRequired from index.vue. + * Delegates to portRequired only when a service is selected. + */ +function defaultBackendPortRequired(port: any, hasService: boolean): string | undefined { + if (!hasService) { + return undefined; + } + + return portRequired(port); +} const makeHelper = () => new IngressDetailEditHelper({ $store: {} as any, @@ -72,6 +148,18 @@ describe('ingress', () => { ports: [80, 443], }], }, + { + desc: 'includes port names alongside port numbers when available', + services: [{ + metadata: { name: 'named-ports-service' }, + spec: { ports: [{ port: 80, name: 'http' }, { port: 443, name: 'https' }] }, + }], + expected: [{ + label: 'named-ports-service', + value: 'named-ports-service', + ports: [80, 'http', 443, 'https'], + }], + }, { desc: 'returns undefined ports when service has no spec.ports', services: [{ @@ -121,10 +209,345 @@ describe('ingress', () => { }, ], }, + { + desc: 'includes only port numbers when ports have no names', + services: [{ + metadata: { name: 'unnamed-service' }, + spec: { ports: [{ port: 3000 }] }, + }], + expected: [{ + label: 'unnamed-service', + value: 'unnamed-service', + ports: [3000], + }], + }, + { + desc: 'skips empty-string port names', + services: [{ + metadata: { name: 'empty-name-svc' }, + spec: { ports: [{ port: 8080, name: '' }] }, + }], + expected: [{ + label: 'empty-name-svc', + value: 'empty-name-svc', + ports: [8080], + }], + }, + { + desc: 'handles mix of named and unnamed ports on the same service', + services: [{ + metadata: { name: 'mixed-svc' }, + spec: { ports: [{ port: 80, name: 'http' }, { port: 9090 }] }, + }], + expected: [{ + label: 'mixed-svc', + value: 'mixed-svc', + ports: [80, 'http', 9090], + }], + }, ])('$desc', ({ services, expected }) => { const helper = makeHelper(); expect(helper.findAndMapServiceTargets(services)).toStrictEqual(expected); }); }); + + describe('parseServicePort', () => { + it.each([ + { + desc: 'parses numeric string to number', + input: '80', + expected: 80, + }, + { + desc: 'parses large port number', + input: '65535', + expected: 65535, + }, + { + desc: 'keeps string port name as-is', + input: 'http', + expected: 'http', + }, + { + desc: 'keeps string port name with hyphens', + input: 'my-port', + expected: 'my-port', + }, + { + desc: 'handles zero as a number (not as falsy)', + input: '0', + expected: 0, + }, + { + desc: 'handles already-numeric input', + input: 443 as any, + expected: 443, + }, + { + desc: 'keeps empty string as empty string', + input: '', + expected: '', + }, + ])('$desc', ({ input, expected }) => { + expect(parseServicePort(input)).toStrictEqual(expected); + }); + + it('routes numeric result to port.number path', () => { + const servicePort = parseServicePort('80'); + + expect(typeof servicePort).toBe('number'); + }); + + it('routes string result to port.name path', () => { + const servicePort = parseServicePort('http'); + + expect(typeof servicePort).toBe('string'); + }); + }); + + describe('portRequired validation', () => { + describe('object input (form mixin)', () => { + it.each([ + { + desc: 'passes when port.number is set', + port: { number: 80 }, + }, + { + desc: 'passes when port.name is set', + port: { name: 'http' }, + }, + { + desc: 'passes when both are set', + port: { + number: 80, + name: 'http', + }, + }, + { + desc: 'passes when port.number is 0 but name is set', + port: { + number: 0, + name: 'http', + }, + }, + ])('valid: $desc', ({ port }) => { + expect(portRequired(port)).toBeUndefined(); + }); + + it.each([ + { + desc: 'port is undefined', + port: undefined, + }, + { + desc: 'port is null', + port: null, + }, + { + desc: 'port is empty object', + port: {}, + }, + { + desc: 'port.number is 0 and no name', + port: { number: 0 }, + }, + { + desc: 'port.name is empty string and no number', + port: { name: '' }, + }, + ])('invalid: $desc', ({ port }) => { + expect(portRequired(port)).toBeDefined(); + }); + }); + + describe('scalar input (component)', () => { + it.each([ + { + desc: 'passes for numeric port', + port: 80, + }, + { + desc: 'passes for string port name', + port: 'http', + }, + ])('valid: $desc', ({ port }) => { + expect(portRequired(port)).toBeUndefined(); + }); + + it.each([ + { + desc: 'empty string', + port: '', + }, + { + desc: 'zero', + port: 0, + }, + ])('invalid: $desc', ({ port }) => { + expect(portRequired(port)).toBeDefined(); + }); + }); + }); + + describe('portRange validation', () => { + describe('object input (form mixin)', () => { + it.each([ + { + desc: 'passes for port.number 1 (minimum)', + port: { number: 1 }, + }, + { + desc: 'passes for port.number 65535 (maximum)', + port: { number: 65535 }, + }, + { + desc: 'passes when only port.name is set', + port: { name: 'http' }, + }, + ])('valid: $desc', ({ port }) => { + expect(portRange(port)).toBeUndefined(); + }); + + it.each([ + { + desc: 'port.number exceeds 65535', + port: { number: 70000 }, + }, + { + desc: 'port.number is negative', + port: { number: -1 }, + }, + ])('invalid: $desc', ({ port }) => { + expect(portRange(port)).toBeDefined(); + }); + }); + + describe('scalar input (component)', () => { + it.each([ + { + desc: 'passes for port 1 (minimum)', + port: 1, + }, + { + desc: 'passes for port 65535 (maximum)', + port: 65535, + }, + { + desc: 'passes for numeric string "443"', + port: '443', + }, + { + desc: 'passes for string port name', + port: 'http', + }, + ])('valid: $desc', ({ port }) => { + expect(portRange(port)).toBeUndefined(); + }); + + it.each([ + { + desc: 'number exceeds 65535', + port: 70000, + }, + { + desc: 'negative number', + port: -1, + }, + { + desc: 'string "70000" exceeds range', + port: '70000', + }, + ])('invalid: $desc', ({ port }) => { + expect(portRange(port)).toBeDefined(); + }); + }); + }); + + describe('willSave', () => { + const nestedPaths = { + defaultBackendPath: 'defaultBackend', + serviceNamePath: 'service.name', + servicePortPath: 'service.port.number', + servicePortNamePath: 'service.port.name', + }; + + it('preserves backend when port.number is set', () => { + const spec = { defaultBackend: { service: { name: 'my-svc', port: { number: 80 } } } }; + + willSave(spec, nestedPaths); + expect(spec.defaultBackend).toStrictEqual({ service: { name: 'my-svc', port: { number: 80 } } }); + }); + + it('preserves backend when port.name is set', () => { + const spec = { defaultBackend: { service: { name: 'my-svc', port: { name: 'http' } } } }; + + willSave(spec, nestedPaths); + expect(spec.defaultBackend).toStrictEqual({ service: { name: 'my-svc', port: { name: 'http' } } }); + }); + + it('clears backend when service name is empty', () => { + const spec = { defaultBackend: { service: { name: '', port: { number: 80 } } } }; + + willSave(spec, nestedPaths); + expect(spec.defaultBackend).toBeNull(); + }); + + it('clears backend when both port paths are empty', () => { + const spec = { defaultBackend: { service: { name: 'my-svc', port: {} } } }; + + willSave(spec, nestedPaths); + expect(spec.defaultBackend).toBeNull(); + }); + + it('clears backend when port is undefined', () => { + const spec = { defaultBackend: { service: { name: 'my-svc' } } }; + + willSave(spec, nestedPaths); + expect(spec.defaultBackend).toBeNull(); + }); + + it('does nothing when there is no backend', () => { + const spec = { rules: [{}] } as any; + + willSave(spec, nestedPaths); + expect(spec).toStrictEqual({ rules: [{}] }); + }); + }); + + describe('defaultBackendNameRequired', () => { + it('returns error when service is selected but name is empty', () => { + expect(defaultBackendNameRequired('', true)).toBeDefined(); + }); + + it('passes when service is selected and name is set', () => { + expect(defaultBackendNameRequired('my-svc', true)).toBeUndefined(); + }); + + it('passes when no service is selected and name is empty', () => { + expect(defaultBackendNameRequired('', false)).toBeUndefined(); + }); + + it('passes when no service is selected and name is undefined', () => { + expect(defaultBackendNameRequired(undefined, false)).toBeUndefined(); + }); + }); + + describe('defaultBackendPortRequired', () => { + it('delegates to portRequired when service is selected', () => { + expect(defaultBackendPortRequired('', true)).toBeDefined(); + expect(defaultBackendPortRequired(80, true)).toBeUndefined(); + expect(defaultBackendPortRequired('http', true)).toBeUndefined(); + expect(defaultBackendPortRequired({ number: 80 }, true)).toBeUndefined(); + expect(defaultBackendPortRequired({ name: 'http' }, true)).toBeUndefined(); + expect(defaultBackendPortRequired({}, true)).toBeDefined(); + }); + + it('skips validation when no service is selected', () => { + expect(defaultBackendPortRequired('', false)).toBeUndefined(); + expect(defaultBackendPortRequired(undefined, false)).toBeUndefined(); + expect(defaultBackendPortRequired(null, false)).toBeUndefined(); + expect(defaultBackendPortRequired({}, false)).toBeUndefined(); + }); + }); }); diff --git a/shell/utils/ingress.ts b/shell/utils/ingress.ts index 048490e4151..0c866e5551a 100644 --- a/shell/utils/ingress.ts +++ b/shell/utils/ingress.ts @@ -56,7 +56,15 @@ class IngressDetailEditHelper { return services.map((service) => ({ label: service.metadata.name, value: service.metadata.name, - ports: service.spec.ports?.map((p: any) => p.port) + ports: service.spec.ports?.flatMap((p: any) => { + const options = [p.port]; + + if (p.name) { + options.push(p.name); + } + + return options; + }) })); } }