Skip to content

Commit 4f84eb3

Browse files
codyranchercodyrancher
authored
Fix Ingress form editor to handle backend.service.port.name (#17631)
* Fix Ingress form editor to handle backend.service.port.name The form editor only read/wrote backend.service.port.number, leaving the port field blank when an Ingress used port.name instead. Now both port.name and port.number are supported for reading, writing, validation, and port dropdown options. Fixes #17105 * Harden port validation and fix parseInt edge case - Fix Number.parseInt fallthrough when port is 0 (falsy but valid parse) - Split portNumberOrName into portRequired and portRange validators - Handle both scalar (component-level) and object (mixin-level) inputs - Restore required asterisk on DefaultBackend port fields - Add unit tests for port parsing, portRequired, and portRange * Add reviewer-facing comments on non-obvious changes * Fix willSave() deleting default backend when port uses a name willSave() only checked servicePortPath (port.number), missing backends that store the port under servicePortNamePath (port.name). * Allow removing default backend by selecting None Default backend validation rules now only enforce required checks when a service is actually selected. Selecting "None" skips validation so the user can save, and willSave() cleans up the empty backend object.
1 parent 140bf8d commit 4f84eb3

6 files changed

Lines changed: 537 additions & 30 deletions

File tree

‎shell/edit/networking.k8s.io.ingress/DefaultBackend.vue‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@ export default {
4343
const backend = get(this.value.spec, this.value.defaultBackendPath);
4444
4545
this.serviceName = get(backend, this.value.serviceNamePath) || '';
46-
this.servicePort = get(backend, this.value.servicePortPath) || '';
46+
this.servicePort = get(backend, this.value.servicePortPath) ||
47+
get(backend, this.value.servicePortNamePath) ||
48+
'';
4749
},
4850
computed: {
4951
isView() {
@@ -75,10 +77,14 @@ export default {
7577
},
7678
methods: {
7779
update() {
78-
const backend = get(this.value.spec, this.value.defaultBackendPath) || {};
80+
// Fresh object so the old port path (name vs number) doesn't linger.
81+
const backend = {};
82+
const parsed = Number.parseInt(this.servicePort);
83+
const servicePort = Number.isNaN(parsed) ? this.servicePort : parsed;
84+
const portPath = typeof servicePort === 'number' ? this.value.servicePortPath : this.value.servicePortNamePath;
7985
8086
set(backend, this.value.serviceNamePath, this.serviceName);
81-
set(backend, this.value.servicePortPath, this.servicePort);
87+
set(backend, portPath, servicePort);
8288
set(this.value.spec, this.value.defaultBackendPath, backend);
8389
8490
this.$emit('update:value', this.value);
@@ -119,10 +125,12 @@ export default {
119125
class="col span-3"
120126
:style="{'margin-right': '0px'}"
121127
>
128+
<!-- :required drives the asterisk; portRequired doesn't have .name === 'required' -->
122129
<LabeledInput
123130
v-if="portOptions.length === 0 || isView"
124-
v-model:value.number="servicePort"
131+
v-model:value="servicePort"
125132
:mode="mode"
133+
:required="true"
126134
:label="t('ingress.defaultBackend.port.label')"
127135
:placeholder="t('ingress.defaultBackend.port.placeholder')"
128136
:rules="rules.port"
@@ -132,6 +140,7 @@ export default {
132140
v-else
133141
v-model:value="servicePort"
134142
:mode="mode"
143+
:required="true"
135144
:options="portOptions"
136145
:label="t('ingress.defaultBackend.port.label')"
137146
:placeholder="t('ingress.defaultBackend.port.placeholder')"

‎shell/edit/networking.k8s.io.ingress/RulePath.vue‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,20 +79,24 @@ export default {
7979
set(this.value, 'path', this.value.path || '');
8080
set(this.value, 'pathType', this.value.pathType || this.pathTypes[0]);
8181
set(this.value.backend, this.ingress.serviceNamePath, get(this.value.backend, this.ingress.serviceNamePath) || '');
82-
set(this.value.backend, this.ingress.servicePortPath, get(this.value.backend, this.ingress.servicePortPath) || '');
8382
8483
this.serviceName = get(this.value.backend, this.ingress.serviceNamePath);
85-
this.servicePort = get(this.value.backend, this.ingress.servicePortPath);
84+
this.servicePort = get(this.value.backend, this.ingress.servicePortPath) ||
85+
get(this.value.backend, this.ingress.servicePortNamePath) ||
86+
'';
8687
},
8788
methods: {
8889
update() {
89-
const servicePort = Number.parseInt(this.servicePort) || this.servicePort;
90+
const parsed = Number.parseInt(this.servicePort);
91+
const servicePort = Number.isNaN(parsed) ? this.servicePort : parsed;
9092
const serviceName = this.serviceName.label || this.serviceName;
9193
const out = {
9294
id: this.value.id, backend: {}, path: this.path, pathType: this.pathType
9395
};
9496
95-
set(out.backend, this.ingress.servicePortPath, servicePort);
97+
const portPath = typeof servicePort === 'number' ? this.ingress.servicePortPath : this.ingress.servicePortNamePath;
98+
99+
set(out.backend, portPath, servicePort);
96100
set(out.backend, this.ingress.serviceNamePath, serviceName);
97101
98102
this.$emit('update:value', out);

‎shell/edit/networking.k8s.io.ingress/index.vue‎

Lines changed: 75 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -87,21 +87,21 @@ export default {
8787
path: 'spec.rules.http.paths.path', rules: ['absolutePath'], translationKey: 'ingress.rules.path.label'
8888
},
8989
{
90-
path: 'spec.rules.http.paths.backend.service.port.number', rules: ['required'], translationKey: 'ingress.rules.port.label'
90+
path: 'spec.rules.http.paths.backend.service.port', rules: ['portRequired', 'portRange'], translationKey: 'ingress.rules.port.label'
9191
},
9292
{
9393
path: 'spec.rules.http.paths.backend.service.name', rules: ['required'], translationKey: 'ingress.rules.target.label'
9494
},
9595
{ path: 'spec', rules: ['backEndOrRules'] },
9696
{
97-
path: 'spec.defaultBackend.service.name', rules: ['required'], translationKey: 'ingress.defaultBackend.targetService.label'
97+
path: 'spec.defaultBackend.service.name', rules: ['defaultBackendNameRequired'], translationKey: 'ingress.defaultBackend.targetService.label'
9898
},
9999
{
100-
path: 'spec.defaultBackend.service.port.number', rules: ['required', 'requiredInt', 'portNumber'], translationKey: 'ingress.defaultBackend.port.label'
100+
path: 'spec.defaultBackend.service.port', rules: ['defaultBackendPortRequired', 'portRange'], translationKey: 'ingress.defaultBackend.port.label'
101101
},
102102
{ path: 'spec.tls.hosts', rules: ['required', 'wildcardHostname'] }
103103
],
104-
fvReportedValidationPaths: ['spec.rules.http.paths.backend.service.port.number', 'spec.rules.http.paths.path', 'spec.rules.http.paths.backend.service.name']
104+
fvReportedValidationPaths: ['spec.rules.http.paths.backend.service.port', 'spec.rules.http.paths.path', 'spec.rules.http.paths.backend.service.name']
105105
};
106106
},
107107
@@ -125,35 +125,89 @@ export default {
125125
}
126126
};
127127
128-
return { backEndOrRules };
128+
const portLabel = this.t('ingress.rules.port.label');
129+
130+
// Built-in `required` won't work: it passes for empty objects like {} or { name: '' }.
131+
const portRequired = (port) => {
132+
if (typeof port === 'string' || typeof port === 'number') {
133+
if (!port) {
134+
return this.t('validation.required', { key: portLabel });
135+
}
136+
} else if (!port || (!port.number && !port.name)) {
137+
return this.t('validation.required', { key: portLabel });
138+
}
139+
};
140+
141+
const portRange = (port) => {
142+
let num;
143+
144+
if (typeof port === 'number') {
145+
num = port;
146+
} else if (typeof port === 'string') {
147+
num = Number.parseInt(port);
148+
} else if (port?.number) {
149+
num = port.number;
150+
}
151+
152+
if (num !== undefined && !Number.isNaN(num) && (num < 1 || num > 65535)) {
153+
return this.t('validation.number.between', {
154+
key: portLabel, min: '1', max: '65535'
155+
});
156+
}
157+
};
158+
159+
const hasDefaultBackendService = () => {
160+
const backend = get(this.value?.spec, this.value.defaultBackendPath);
161+
162+
return !!get(backend, this.value.serviceNamePath);
163+
};
164+
165+
const nameLabel = this.t('ingress.defaultBackend.targetService.label');
166+
167+
// Only enforce required on the default backend when a service is selected.
168+
// Selecting "None" means the user wants to remove the backend; willSave() handles cleanup.
169+
const defaultBackendNameRequired = (name) => {
170+
if (hasDefaultBackendService() && !name) {
171+
return this.t('validation.required', { key: nameLabel });
172+
}
173+
};
174+
175+
const defaultBackendPortRequired = (port) => {
176+
if (!hasDefaultBackendService()) {
177+
return;
178+
}
179+
180+
return portRequired(port);
181+
};
182+
183+
return {
184+
backEndOrRules,
185+
portRequired,
186+
portRange,
187+
defaultBackendNameRequired,
188+
defaultBackendPortRequired,
189+
};
129190
},
130191
tabErrors() {
131192
return {
132-
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,
133-
defaultBackend: this.fvGetPathErrors(['spec.defaultBackend.service.name', 'spec.defaultBackend.service.port.number'])?.length > 0
193+
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,
194+
defaultBackend: this.fvGetPathErrors(['spec.defaultBackend.service.name', 'spec.defaultBackend.service.port'])?.length > 0
134195
};
135196
},
136197
rulesPathRules() {
137198
return {
138199
requestHost: this.fvGetAndReportPathRules('spec.rules.host'),
139200
path: this.fvGetAndReportPathRules('spec.rules.http.paths.path'),
140-
port: this.fvGetAndReportPathRules('spec.rules.http.paths.backend.service.port.number'),
201+
port: this.fvGetAndReportPathRules('spec.rules.http.paths.backend.service.port'),
141202
target: this.fvGetAndReportPathRules('spec.rules.http.paths.backend.service.name'),
142203
143204
};
144205
},
145206
defaultBackendPathRules() {
146-
const rulesExist = (this.value?.spec?.rules || []).length > 0;
147-
const defaultBackendExist = !!this.value?.spec?.defaultBackend?.service;
148-
149-
if (!rulesExist || defaultBackendExist) {
150-
return {
151-
name: this.fvGetAndReportPathRules('spec.defaultBackend.service.name'),
152-
port: this.fvGetAndReportPathRules('spec.defaultBackend.service.port.number'),
153-
};
154-
}
155-
156-
return { name: [], port: [] };
207+
return {
208+
name: this.fvGetAndReportPathRules('spec.defaultBackend.service.name'),
209+
port: this.fvGetAndReportPathRules('spec.defaultBackend.service.port'),
210+
};
157211
},
158212
serviceTargets() {
159213
return this.ingressHelper.findAndMapServiceTargets(this.services);
@@ -188,7 +242,8 @@ export default {
188242
willSave() {
189243
const backend = get(this.value.spec, this.value.defaultBackendPath);
190244
const serviceName = get(backend, this.value.serviceNamePath);
191-
const servicePort = get(backend, this.value.servicePortPath);
245+
const servicePort = get(backend, this.value.servicePortPath) ||
246+
get(backend, this.value.servicePortNamePath);
192247
193248
if (backend && (!serviceName || !servicePort)) {
194249
const path = this.value.defaultBackendPath;

‎shell/models/networking.k8s.io.ingress.js‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ export default class Ingress extends SteveModel {
8484
serviceTargetTo: this.targetTo(workloads, serviceName),
8585
certs: this.certLinks(rule, certificates),
8686
targetLink: this.targetLink(workloads, serviceName),
87-
port: get(path?.backend, this.servicePortPath)
87+
port: get(path?.backend, this.servicePortPath) || get(path?.backend, this.servicePortNamePath)
8888
};
8989
}
9090

@@ -184,6 +184,14 @@ export default class Ingress extends SteveModel {
184184
return this.useNestedBackendField ? nestedPath : flatPath;
185185
}
186186

187+
get servicePortNamePath() {
188+
const nestedPath = 'service.port.name';
189+
// Flat API has a single `servicePort` field for both name and number.
190+
const flatPath = 'servicePort';
191+
192+
return this.useNestedBackendField ? nestedPath : flatPath;
193+
}
194+
187195
get defaultBackendPath() {
188196
const defaultBackend = this.$rootGetters['cluster/pathExistsInSchema'](this.type, 'spec.defaultBackend');
189197

0 commit comments

Comments
 (0)