Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions shell/components/form/Security.vue
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,12 @@ export default {
this.afterPrivilegedTickedMessage = this.t('workload.container.security.privileged.afterTick.false');
}

if (securityContext.fsGroup === '') {
// Drop empty values so we don't send a string for int64 fields.
if (securityContext.fsGroup === '' || securityContext.fsGroup === null) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could this also be undefined by any chance?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it looks like it could be. I went ahead and put an explicit check in for undefined as well. We could combine the checks into a simpler expression but I thought being explicit would make the intentions more clear.

delete securityContext.fsGroup;
}

if (securityContext.runAsUser === '') {
if (securityContext.runAsUser === '' || securityContext.runAsUser === null) {
delete securityContext.runAsUser;
}

Expand Down Expand Up @@ -175,6 +176,7 @@ export default {
ref="firstFocusable"
v-model:value.number="securityContext.fsGroup"
type="number"
min="0"
:mode="mode"
:label="t('workload.container.security.fsGroup')"
@update:value="update"
Expand Down Expand Up @@ -283,6 +285,8 @@ export default {
</legend>
<LabeledInput
v-model:value.number="securityContext.runAsUser"
type="number"
min="0"
:label="t('workload.container.security.runAsUser.label')"
:mode="mode"
@update:value="update"
Expand Down
39 changes: 39 additions & 0 deletions shell/components/form/__tests__/Security.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,25 @@ describe('component: Security', () => {
expect(wrapper.emitted('update:value')).toHaveLength(1);
});

// Regression: clearing runAsUser must drop the key from the emitted object.
// Otherwise the spec is sent with `runAsUser: ""` and the API rejects with
// "cannot unmarshal string ... of type int64". See issue #9601.
it('should omit runAsUser from the emitted value when the input is cleared', async() => {
const wrapper = mount(Security, {
props: {
mode: _EDIT, formType: FORM_TYPES.CONTAINER, value: { runAsUser: 33 }
}
});
const input = wrapper.find('[data-testid="input-security-runAsUser"]').find('input');

await input.setValue('');

const events = wrapper.emitted('update:value') ?? [];
const last = events[events.length - 1][0] as Record<string, unknown>;

expect(Object.prototype.hasOwnProperty.call(last, 'runAsUser')).toBe(false);
});

it.each([
'privileged',
'allowPrivilegeEscalation',
Expand Down Expand Up @@ -139,5 +158,25 @@ describe('component: Security', () => {

expect(wrapper.emitted('update:value')).toHaveLength(1);
});

// Regression for #9601 — see equivalent container-level test above.
it.each([
['runAsUser', { runAsUser: 33 }],
['fsGroup', { fsGroup: 100 }],
])('should omit %p from the emitted value when the input is cleared', async(field, initial) => {
const wrapper = mount(Security, {
props: {
mode: _EDIT, formType: FORM_TYPES.POD, value: initial
}
});
const input = wrapper.find(`[data-testid="input-security-${ field }"]`).find('input');

await input.setValue('');

const events = wrapper.emitted('update:value') ?? [];
const last = events[events.length - 1][0] as Record<string, unknown>;

expect(Object.prototype.hasOwnProperty.call(last, field)).toBe(false);
});
});
});
Loading