mirror of
https://github.com/harvester/harvester-ui-extension.git
synced 2026-11-15 13:26:15 +08:00
fix: block VM creation/save with invalid CloudInit YAML (#1040)
* fix(vm): block VM creation with invalid CloudInit YAML content * fix(vm): add isWindows guard to validateCloudInit in template version * fix: validate Windows CloudInit YAML on save Co-authored-by: a110605 <5744158+a110605@users.noreply.github.com> * fix: validate cloud-init by format in the VM mixin Cloud-init picks the user data format from the first line, so scripts, MIME messages and Jinja templates are valid without being YAML. Parsing all user data blocked them, and it stopped any existing VM holding one from being saved, even with nothing changed. Only parse user data that starts with #cloud-config, and require a mapping for it and for network data, because plain words are valid YAML and were still saved. Both forms already use the VM mixin, so the check moves there and shares the header pattern with hasCloudConfigComment. Signed-off-by: Marcelo Fukumoto <marcelo.fukumoto@suse.com> * fix: save other user data formats as typed and check text without a header The form added #cloud-config to any user data that parses as YAML, so a script was saved with the header and its lines folded together, and Edit Config then failed the check on it. User data that starts with a script, MIME or another cloud-init or Cloudbase-Init format is now saved as typed, and any other text is checked as cloud-config, with or without the header, because the form adds the header when saving it. Editing only checks cloud-init that changed, so a VM saved before this change can still be edited with the cloud-init it already has. Signed-off-by: Marcelo Fukumoto <marcelo.fukumoto@suse.com> --------- Signed-off-by: Marcelo Fukumoto <marcelo.fukumoto@suse.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: a110605 <5744158+a110605@users.noreply.github.com> Co-authored-by: Marcelo Fukumoto <marcelo.fukumoto@suse.com>
This commit is contained in:
Copilot
co-authored by
a110605
Marcelo Fukumoto
a110605
Marcelo Fukumoto
parent
67ff2777d6
commit
aa6d11a840
@@ -0,0 +1,36 @@
|
||||
import { HCI } from '@pkg/harvester/types';
|
||||
import VM_MIXIN from '@pkg/harvester/mixins/harvester-vm';
|
||||
import HarvesterEditTemplateVersion from '../harvesterhci.io.virtualmachinetemplateversion.vue';
|
||||
|
||||
async function saveWith(userScript) {
|
||||
const buttonCb = jest.fn();
|
||||
const form = {
|
||||
userScript,
|
||||
errors: [],
|
||||
t: (key) => key,
|
||||
parseVM: jest.fn(),
|
||||
validateCloudInit: VM_MIXIN.methods.validateCloudInit,
|
||||
templateValue: { metadata: {} },
|
||||
$store: { dispatch: jest.fn().mockResolvedValue([]) },
|
||||
};
|
||||
|
||||
await HarvesterEditTemplateVersion.methods.saveVMT.call(form, buttonCb);
|
||||
|
||||
return { form, buttonCb };
|
||||
}
|
||||
|
||||
describe('page: harvesterhci.io.virtualmachinetemplateversion edit', () => {
|
||||
it('should stop the save when the cloud config is invalid', async() => {
|
||||
const { form, buttonCb } = await saveWith('#cloud-config\nsome words\n');
|
||||
|
||||
expect(form.errors).toStrictEqual(['harvester.virtualMachine.cloudConfig.user.invalidYaml']);
|
||||
expect(buttonCb).toHaveBeenCalledWith(false);
|
||||
expect(form.$store.dispatch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should go on with the save when the user data is a script', async() => {
|
||||
const { form } = await saveWith('#!/bin/bash\napt-get update\n# start it\nsystemctl enable --now nginx\n');
|
||||
|
||||
expect(form.$store.dispatch).toHaveBeenCalledWith('harvester/findAll', { type: HCI.VM_TEMPLATE });
|
||||
});
|
||||
});
|
||||
@@ -179,8 +179,17 @@ export default {
|
||||
},
|
||||
|
||||
async saveVMT(buttonCb) {
|
||||
this.errors = [];
|
||||
this.parseVM();
|
||||
|
||||
this.validateCloudInit();
|
||||
|
||||
if (this.errors.length) {
|
||||
buttonCb(false);
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
const templates = await this.$store.dispatch('harvester/findAll', { type: HCI.VM_TEMPLATE });
|
||||
const template = templates.find( (O) => O.metadata.name === this.templateValue.metadata.name);
|
||||
|
||||
|
||||
@@ -0,0 +1,37 @@
|
||||
import VM_MIXIN from '@pkg/harvester/mixins/harvester-vm';
|
||||
import HarvesterEditVM from '../index.vue';
|
||||
|
||||
function saveWith(userScript) {
|
||||
const buttonCb = jest.fn();
|
||||
const form = {
|
||||
userScript,
|
||||
errors: [],
|
||||
isSingle: true,
|
||||
t: (key) => key,
|
||||
validateCPUMemory: jest.fn(),
|
||||
validateWindowsSysprep: jest.fn(),
|
||||
validateCloudInit: VM_MIXIN.methods.validateCloudInit,
|
||||
saveSingle: jest.fn(),
|
||||
};
|
||||
|
||||
HarvesterEditVM.methods.saveVM.call(form, buttonCb);
|
||||
|
||||
return { form, buttonCb };
|
||||
}
|
||||
|
||||
describe('page: kubevirt.io.virtualmachine edit', () => {
|
||||
it('should stop the save when the cloud config is invalid', () => {
|
||||
const { form, buttonCb } = saveWith('#cloud-config\nsome words\n');
|
||||
|
||||
expect(form.errors).toStrictEqual(['harvester.virtualMachine.cloudConfig.user.invalidYaml']);
|
||||
expect(buttonCb).toHaveBeenCalledWith(false);
|
||||
expect(form.saveSingle).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should save when the user data is a script', () => {
|
||||
const { form, buttonCb } = saveWith('#!/bin/bash\napt-get update\n# start it\nsystemctl enable --now nginx\n');
|
||||
|
||||
expect(form.errors).toStrictEqual([]);
|
||||
expect(form.saveSingle).toHaveBeenCalledWith(buttonCb);
|
||||
});
|
||||
});
|
||||
@@ -381,6 +381,7 @@ export default {
|
||||
|
||||
this.validateCPUMemory();
|
||||
this.validateWindowsSysprep();
|
||||
this.validateCloudInit();
|
||||
|
||||
// block create VM flow if has validation errors
|
||||
if (this.errors.length) {
|
||||
|
||||
@@ -938,10 +938,12 @@ harvester:
|
||||
label: User Data Template
|
||||
title: "User Data:"
|
||||
tip: "You can specify user data to configure an instance or run a configuration script during launch. If you launch more than one instance at a time, the user data is available to all the instances in that reservation. <a href='https://cloudinit.readthedocs.io/en/latest/topics/examples.html' target='_blank'>Learn more</a>"
|
||||
invalidYaml: 'Invalid User Data YAML syntax'
|
||||
network:
|
||||
label: Network Data Template
|
||||
title: "Network Data:"
|
||||
tip: "The network-data configuration allows you to customize the instance's networking interfaces by assigning subnet configuration, virtual device creation (bonds, bridges, VLANs) routes and DNS configuration. <a href='https://cloudinit.readthedocs.io/en/latest/reference/network-config-format-v1.html' target='_blank'>Learn more</a>"
|
||||
invalidYaml: 'Invalid Network Data YAML syntax'
|
||||
sysprep:
|
||||
title: Windows Unattended & Sysprep Configuration
|
||||
description: "Configure Windows automated installation using autounattend.xml. The configuration will be stored in a Kubernetes Secret and mounted as a CD-ROM during installation. <a href='https://docs.microsoft.com/en-us/windows-hardware/manufacture/desktop/automate-windows-setup' target='_blank'>Learn more</a>"
|
||||
|
||||
@@ -0,0 +1,140 @@
|
||||
import VM_MIXIN from '../index';
|
||||
import impl from '../impl';
|
||||
|
||||
const USER_ERROR = 'harvester.virtualMachine.cloudConfig.user.invalidYaml';
|
||||
const NETWORK_ERROR = 'harvester.virtualMachine.cloudConfig.network.invalidYaml';
|
||||
const CLOUD_CONFIG = '#cloud-config\npackage_update: true\npackages:\n - qemu-guest-agent\n';
|
||||
const YAML_SCRIPT = '#!/bin/bash\napt-get update\napt-get install -y nginx\n';
|
||||
|
||||
function validate({
|
||||
userScript, networkScript, isEdit = false, savedCloudInit = {}
|
||||
}) {
|
||||
const form = {
|
||||
userScript,
|
||||
networkScript,
|
||||
isEdit,
|
||||
savedCloudInit,
|
||||
errors: [],
|
||||
t: (key) => key,
|
||||
};
|
||||
|
||||
VM_MIXIN.methods.validateCloudInit.call(form);
|
||||
|
||||
return form.errors;
|
||||
}
|
||||
|
||||
function userDataFor(userScript) {
|
||||
const form = {
|
||||
userScript,
|
||||
sshKey: [],
|
||||
$store: { getters: { 'harvester/byId': () => undefined } },
|
||||
...impl.methods,
|
||||
...VM_MIXIN.methods,
|
||||
};
|
||||
|
||||
return form.getUserData({ osType: 'linux', installAgent: false });
|
||||
}
|
||||
|
||||
describe('mixin: harvester-vm', () => {
|
||||
describe('validateCloudInit', () => {
|
||||
it.each([
|
||||
['a word after the mapping', `${ CLOUD_CONFIG }some words\n`],
|
||||
['only words', '#cloud-config\nsome words\n'],
|
||||
['only words and no header', 'some words\n'],
|
||||
['a list', '#cloud-config\n- a\n- b\n'],
|
||||
['bad indentation', '#cloud-config\nusers:\n - name: a\n bad: b\n'],
|
||||
['an unclosed quote', '#cloud-config\npassword: "abc\n'],
|
||||
])('should block cloud-config user data with %s', (_, userScript) => {
|
||||
expect(validate({ userScript })).toStrictEqual([USER_ERROR]);
|
||||
});
|
||||
|
||||
it.each([
|
||||
['undefined', undefined],
|
||||
['empty', ''],
|
||||
['only the header', '#cloud-config\n'],
|
||||
['a valid cloud-config', CLOUD_CONFIG],
|
||||
['a mapping without the header', 'password: test\n'],
|
||||
])('should allow user data that is %s', (_, userScript) => {
|
||||
expect(validate({ userScript })).toStrictEqual([]);
|
||||
});
|
||||
|
||||
it.each([
|
||||
['a script that is not valid YAML', '#!/bin/bash\napt-get update\n# start it\nsystemctl enable --now nginx\n'],
|
||||
['a script that parses as YAML', YAML_SCRIPT],
|
||||
['a MIME message with a cloud-config part', 'Content-Type: multipart/mixed; boundary="b"\nMIME-Version: 1.0\n\n--b\nContent-Type: text/cloud-config\n\n#cloud-config\npackages:\n - nginx\n--b--\n'],
|
||||
['a Jinja template', '## template: jinja\n#cloud-config\n{% if v1.region == "us" %}\npackages:\n - a\n{% endif %}\n'],
|
||||
['a Cloudbase-Init PowerShell script', '#ps1_sysnative\n# enable RDP\nWrite-Host "done: ok"\n'],
|
||||
['a Cloudbase-Init batch script', 'rem cmd\nnet user admin Passw0rd /add\n'],
|
||||
['a cloud-config archive', '#cloud-config-archive\n- type: text/x-shellscript\n content: |\n #!/bin/sh\n'],
|
||||
])('should not parse user data that is %s', (_, userScript) => {
|
||||
expect(validate({ userScript })).toStrictEqual([]);
|
||||
});
|
||||
|
||||
it.each([
|
||||
['words', 'some words\n'],
|
||||
['a list', '- a\n- b\n'],
|
||||
['bad indentation', 'network:\n version: 1\n bad: x\n'],
|
||||
])('should block network data with %s', (_, networkScript) => {
|
||||
expect(validate({ networkScript })).toStrictEqual([NETWORK_ERROR]);
|
||||
});
|
||||
|
||||
it.each([
|
||||
['undefined', undefined],
|
||||
['empty', ''],
|
||||
['only whitespace', ' \n'],
|
||||
['only a comment', '# nothing yet\n'],
|
||||
['a valid config', 'network:\n version: 1\n config:\n - type: physical\n name: eth0\n'],
|
||||
])('should allow network data that is %s', (_, networkScript) => {
|
||||
expect(validate({ networkScript })).toStrictEqual([]);
|
||||
});
|
||||
|
||||
it('should report both fields when both are invalid', () => {
|
||||
const errors = validate({ userScript: '#cloud-config\nsome words\n', networkScript: 'some words\n' });
|
||||
|
||||
expect(errors).toStrictEqual([USER_ERROR, NETWORK_ERROR]);
|
||||
});
|
||||
|
||||
it('should save an edited VM with the cloud-init it already has', () => {
|
||||
const savedCloudInit = {
|
||||
userScript: '#cloud-config\n#!/bin/bash\napt-get update apt-get install -y nginx\n',
|
||||
networkScript: 'some words\n',
|
||||
};
|
||||
|
||||
const errors = validate({
|
||||
...savedCloudInit, isEdit: true, savedCloudInit
|
||||
});
|
||||
|
||||
expect(errors).toStrictEqual([]);
|
||||
});
|
||||
|
||||
it('should check cloud-init changed while editing', () => {
|
||||
const savedCloudInit = { userScript: CLOUD_CONFIG, networkScript: '' };
|
||||
const errors = validate({
|
||||
userScript: '#cloud-config\nsome words\n', networkScript: 'some words\n', isEdit: true, savedCloudInit
|
||||
});
|
||||
|
||||
expect(errors).toStrictEqual([USER_ERROR, NETWORK_ERROR]);
|
||||
});
|
||||
|
||||
it('should check cloud-init a new VM takes from a template', () => {
|
||||
const savedCloudInit = { userScript: '#cloud-config\nsome words\n' };
|
||||
|
||||
expect(validate({ ...savedCloudInit, savedCloudInit })).toStrictEqual([USER_ERROR]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getUserData', () => {
|
||||
it.each([
|
||||
['a script that parses as YAML', YAML_SCRIPT],
|
||||
['a Cloudbase-Init batch script', 'rem cmd\nnet user admin Passw0rd /add\n'],
|
||||
['a Cloudbase-Init PowerShell script', '#ps1_sysnative\nWrite-Host done\n'],
|
||||
['a cloud-config archive', '#cloud-config-archive\n- type: text/x-shellscript\n content: echo hi\n'],
|
||||
])('should save %s as typed', (_, userScript) => {
|
||||
expect(userDataFor(userScript)).toStrictEqual(userScript);
|
||||
});
|
||||
|
||||
it('should add the header to cloud-config typed without one', () => {
|
||||
expect(userDataFor('password: test\n')).toBe('#cloud-config\npassword: test\n');
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -35,6 +35,18 @@ export const SSH_EXISTING_TYPE = {
|
||||
EXISTING_ONLY_CLOUD: 'EXISTING_CLOUD',
|
||||
};
|
||||
|
||||
// Cloud-init and Cloudbase-Init run user data starting with one of these as a script, MIME or another format, not as cloud-config.
|
||||
const OTHER_USER_DATA_FORMATS = [
|
||||
'#!', '#include', '#cloud-boothook', '#part-handler', '#cloud-config-archive', '#cloud-config-jsonp', '## template:',
|
||||
'#ps1', 'rem cmd', '<script>', '<powershell>', 'content-type:', 'mime-version:'
|
||||
];
|
||||
|
||||
export function isCloudConfig(userScript) {
|
||||
const start = (userScript || '').trimStart().toLowerCase();
|
||||
|
||||
return !OTHER_USER_DATA_FORMATS.some((format) => start.startsWith(format));
|
||||
}
|
||||
|
||||
export default {
|
||||
methods: {
|
||||
hasCloudConfigComment(userScript) {
|
||||
|
||||
@@ -2,6 +2,8 @@ import YAML from 'yaml';
|
||||
import jsyaml from 'js-yaml';
|
||||
import isEqual from 'lodash/isEqual';
|
||||
import isEmpty from 'lodash/isEmpty';
|
||||
import isNil from 'lodash/isNil';
|
||||
import isPlainObject from 'lodash/isPlainObject';
|
||||
import difference from 'lodash/difference';
|
||||
import { sortBy } from '@shell/utils/sort';
|
||||
import { set } from '@shell/utils/object';
|
||||
@@ -23,7 +25,7 @@ import {
|
||||
import { HCI_SETTING } from '../../config/settings';
|
||||
import { HCI } from '../../types';
|
||||
import { parseVolumeClaimTemplates, EMPTY_IMAGE } from '../../utils/vm';
|
||||
import impl, { QGA_JSON, USB_TABLET } from './impl';
|
||||
import impl, { isCloudConfig, QGA_JSON, USB_TABLET } from './impl';
|
||||
import { GIBIBYTE } from '../../utils/unit';
|
||||
import { VOLUME_MODE, ACCESS_MODE, FILESYSTEM_SOURCE_TYPE } from '@pkg/harvester/config/types';
|
||||
|
||||
@@ -157,6 +159,8 @@ export default {
|
||||
installUSBTablet: true,
|
||||
networkScript: '',
|
||||
userScript: '',
|
||||
// The cloud-init the form loaded, so editing a VM never blocks saving it unchanged.
|
||||
savedCloudInit: {},
|
||||
imageId: '',
|
||||
diskRows: [],
|
||||
networkRows: [],
|
||||
@@ -442,6 +446,7 @@ export default {
|
||||
this['accessCredentials'] = accessCredentials;
|
||||
this['userScript'] = userData;
|
||||
this['networkScript'] = networkData;
|
||||
this['savedCloudInit'] = { userScript: userData, networkScript: networkData };
|
||||
|
||||
this['sshKey'] = sshKey;
|
||||
this['osType'] = osType;
|
||||
@@ -1291,6 +1296,11 @@ export default {
|
||||
* @param config
|
||||
*/
|
||||
getUserData(config) {
|
||||
// Scripts, MIME and the other formats are saved as typed: only cloud-config gets the SSH keys, the guest agent and the header.
|
||||
if (!isCloudConfig(this.userScript)) {
|
||||
return this.userScript;
|
||||
}
|
||||
|
||||
try {
|
||||
// https://github.com/eemeli/yaml/issues/136
|
||||
let userDataDoc = this.userScript ? YAML.parseDocument(this.userScript) : YAML.parseDocument({});
|
||||
@@ -1483,6 +1493,30 @@ export default {
|
||||
this.networkScript = value;
|
||||
},
|
||||
|
||||
validateCloudInit() {
|
||||
const isYamlMapping = (text) => {
|
||||
try {
|
||||
const doc = jsyaml.load(text);
|
||||
|
||||
return isNil(doc) || isPlainObject(doc);
|
||||
} catch (e) {
|
||||
return false;
|
||||
}
|
||||
};
|
||||
|
||||
// Editing only checks what changed, so a VM can still be saved with the cloud-init it already has.
|
||||
const isChanged = (key) => !this.isEdit || this[key] !== this.savedCloudInit[key];
|
||||
|
||||
// Scripts, MIME and the other formats are valid without being YAML. Anything else is saved as cloud-config.
|
||||
if (this.userScript && isChanged('userScript') && isCloudConfig(this.userScript) && !isYamlMapping(this.userScript)) {
|
||||
this.errors.push(this.t('harvester.virtualMachine.cloudConfig.user.invalidYaml'));
|
||||
}
|
||||
|
||||
if (this.networkScript && isChanged('networkScript') && !isYamlMapping(this.networkScript)) {
|
||||
this.errors.push(this.t('harvester.virtualMachine.cloudConfig.network.invalidYaml'));
|
||||
}
|
||||
},
|
||||
|
||||
mergeSSHAuthorizedKeys(yaml) {
|
||||
try {
|
||||
const sshAuthorizedKeys = YAML.parseDocument(yaml)
|
||||
|
||||
Reference in New Issue
Block a user