feat(new VM): add ssh key to vm creation form by Elise-FZI · Pull Request #9413 · vatesfr/xen-orchestra

Elise-FZI · 2026-01-21T08:23:45Z

Description

Add ssh key field to VM creation form and updating the Cloud_config display on the site/task side panel

A quick note:
Regarding the TaskPropertiesCard.vue file, I'm not entirely happy with disabling ESLint for my code.
I found another solution (below), but I'm not convinced.


const propertiesOtherWithoutCloudConfig = computed(() => {
  const other = properties.value.other
  const args = other?.args as { cloud_config?: string } | undefined

  if (!args?.cloud_config) {
    return other
  }

  const argsCopy = { ...args }
  delete argsCopy.cloud_config

  return {
    ...other,
    args: argsCopy,
  }
})

Screenshots :

Create VM view:

Site/task sidepanel view:

Checklist

Commit
- Title follows commit conventions
- Reference the relevant issue (Fixes #007, See xoa-support#42, See https://...)
- If bug fix, add Introduced by
Changelog
- If visible by XOA users, add changelog entry
- Update "Packages to release" in CHANGELOG.unreleased.md
PR
- If UI changes, add screenshots
- If not finished or not tested, open as Draft

Review process

If you are an external contributor, you can skip this part. Simply create the pull request, and we'll get back to you as soon as possible.

This 2-passes review process aims to:

develop skills of junior reviewers

limit the workload for senior reviewers

limit the number of unnecessary changes by the author

The author creates a PR.
Review process:
1. The author assigns the junior reviewer.
2. The junior reviewer conducts their review:
  - Resolves their comments if they are addressed.
  - Adds comments if necessary or approves the PR.
3. The junior reviewer assigns the senior reviewer.
4. The senior reviewer conducts their review:
  - If there are no unresolved comments on the PR → merge.
  - Otherwise, we continue with 3.
The author responds to comments and/or makes corrections, and we go back to 2.

Notes:

The author can request a review at any time, even if the PR is still a Draft.
In theory, there should not be more than one reviewer at a time.
The author should not make any changes:
- When a reviewer is assigned.
- Between the junior and senior reviews.

plane-sync-vates · 2026-01-21T08:23:53Z

Linked to Plane Work Item(s)

References

[XO-1521] [XO-6][VM] New VM - Add missing ssh key to VM creation form

This comment was auto-generated by Plane

UnelDev · 2026-02-06T10:47:56Z

@xen-orchestra/web/src/pages/vm/new.vue

+const addSshKey = () => {
+  if (!vmState.ssh_key.trim()) {
+    isSshKeyEmpty.value = true
+    return
+  }
+  vmState.sshKeys.push(vmState.ssh_key.trim())
+  vmState.ssh_key = ''
+  isSshKeyEmpty.value = false
+}
+
+const removeSshKey = (index: number) => {
+  vmState.sshKeys.splice(index, 1)
+}
+


For consistency with XO5, additional validation like duplication detection is needed. And maybe check if it starts with ssh-rsa for validating the key.

Suggested change

const addSshKey = () => {

if (!vmState.ssh_key.trim()) {

isSshKeyEmpty.value = true

return

}

vmState.sshKeys.push(vmState.ssh_key.trim())

vmState.ssh_key = ''

isSshKeyEmpty.value = false

}

const removeSshKey = (index: number) => {

vmState.sshKeys.splice(index, 1)

}

const addSshKey = () => {

const sshKey = vmState.ssh_key.trim()

if (!sshKey) {

isSshKeyError.value = 'empty'

return

}

if (vmState.sshKeys.includes(sshKey)) {

isSshKeyError.value = 'alreadyExist'

return

}

if (!sshKey.startsWith('ssh-rsa ')) {

isSshKeyError.value = 'invalidKey'

return

}

vmState.sshKeys.push(sshKey)

vmState.ssh_key = ''

isSshKeyError.value = false

}

I agree on the duplication detection.
But, ⚠️ not all SSH keys start with ssh-rsa!

~~Perhaps you should check for all types of SSH keys, or just if start with ssh-?~~

The ssh-keygen can only generate ecdsa | ecdsa-sk | ed25519 | ed25519-sk | rsa types, which makes it small enough to test all of them.

UnelDev · 2026-02-06T10:49:48Z

@xen-orchestra/web/src/pages/vm/new.vue

+                  <div class="ssh-chips">
+                    <div v-for="(key, index) in vmState.sshKeys" :key="index" class="ssh-chip-wrapper">
+                      <UiChip accent="info" @remove="removeSshKey(index)">
+                        {{ key }}


For consistency with XO5, add an sshKey parser to extract the name of the key.

const parseSshKey = (key: string) => { const parts = key.trim().split(' ') if (parts.length >= 3) { return parts[2] } return key }

After discussion, we are keeping the Figma design for now

UnelDev · 2026-02-06T10:59:02Z

@xen-orchestra/web/src/pages/vm/new.vue

+                  <div class="ssh-key-area">
+                    <UiTextarea v-model="vmState.ssh_key" required :accent="isSshKeyEmpty ? 'danger' : 'brand'">
+                      {{ t('public-key') }}
+                      <template v-if="isSshKeyEmpty" #info>
+                        {{ t('public-key-mandatory') }}
+                      </template>
+                    </UiTextarea>
+                  </div>


For greater clarity regarding error detection, additional error messages should be added.

Suggested change

<div class="ssh-key-area">

<UiTextarea v-model="vmState.ssh_key" required :accent="isSshKeyEmpty ? 'danger' : 'brand'">

{{ t('public-key') }}

<template v-if="isSshKeyEmpty" #info>

{{ t('public-key-mandatory') }}

</template>

</UiTextarea>

</div>

<div class="ssh-key-area">

<UiTextarea

v-model="vmState.ssh_key"

required

:accent="isSshKeyError !== false ? 'danger' : 'brand'"

>

{{ t('public-key') }}

<template v-if="isSshKeyError == 'empty'" #info>

{{ t('public-key-mandatory') }}

</template>

<template v-else-if="isSshKeyError == 'alreadyExist'" #info>

{{ t('public-key-already-exist') }}

</template>

<template v-else-if="isSshKeyError == 'invalidKey'" #info>

{{ t('public-key-invalid') }}

</template>

</UiTextarea>

</div>

⚠️ isSshKeyError == 'empty'

UnelDev · 2026-02-06T12:35:01Z

@xen-orchestra/web/src/pages/vm/new.vue

 // Toaster
 const errorMessage = ref('')
 const isToasterOpen = ref(false)
+const isSshKeyEmpty = ref(false)


Suggested change

const isSshKeyEmpty = ref(false)

const isSshKeyError = ref<'empty' | 'alreadyExist' | 'invalidKey' | false>(false)

I'm not a fan of having mixed types for the same variable (boolean and string here).
Plus, the naming is confusing.
Maybe a better approach would be to have two variables, one boolean for the "state" and another for the "value".

UnelDev · 2026-02-06T12:49:42Z

@xen-orchestra/web/src/modules/task/components/list/panel/cards/TaskPropertiesCard.vue

 const { properties } = useXoTaskPropertiesUtils(() => task)
+
+const cloudConfig = computed(() => {
+  const args = properties.value.other?.args as { cloud_config?: string } | undefined


I think you can change this type on xo.d.ts

UnelDev · 2026-02-06T12:57:51Z

@xen-orchestra/web/src/modules/task/components/list/panel/cards/TaskPropertiesCard.vue

-      <VtsRecursiveFields :fields="properties.other" />
+      <VtsRecursiveFields :fields="propertiesOtherWithoutCloudConfig" />
    </div>
+    <UiLogEntryViewer :content="cloudConfig" :label="t('cloud-config')" size="small" accent="info" />


The other fields on this card is not translated, so maybe you don't have to translate this too. Or translate all fields.

to discuss

Suggested change

<UiLogEntryViewer :content="cloudConfig" :label="t('cloud-config')" size="small" accent="info" />

<UiLogEntryViewer :content="cloudConfig" :label="cloud_config" size="small" accent="info" />

Since we know we want to display this property explicitly, it makes sense to have a translation. However, I'm not sure of the underscore in the translation value (Could_config -> Could config).

OlivierFL

Remember to add an entry in the changelog.

OlivierFL · 2026-02-18T15:21:48Z

@xen-orchestra/web/src/pages/vm/new.vue

+    .install-ssh-key-container {
+      margin-block-start: 3rem;
+    }


I don't think this space is needed: while the design in Figma is newer, there is only 2.4rem of gap, which what we already have in .install-settings-container.
Plus, you can remove the unnecessary <div> in the template.

OlivierFL · 2026-02-19T07:10:43Z

@xen-orchestra/web/src/pages/vm/new.vue

+    .ssh-chips {
+      display: flex;
+      flex-wrap: wrap;
+      gap: 0.5rem;


Suggested change

gap: 0.5rem;

gap: 0.4rem;

OlivierFL · 2026-02-19T07:43:00Z

@xen-orchestra/web/src/pages/vm/new.vue

+    }
+  }
+)
+watch(() => [vmState.installMode, vmState.name, vmState.sshKeys], buildCloudConfig)


As discussed, I think you're watching too many properties, watching only vmState.sshKeys should be enough.
Here the problem is that buildCloudConfig is triggered too many times, even when it shouldn't, and from my tests, sometimes it is not triggered when it should.

vmState.installMode is not useful here, since you already check its value in buildCloudConfig

same for vmState.name I think

OlivierFL · 2026-02-19T07:56:17Z

@xen-orchestra/web/src/pages/vm/new.vue

+const addSshKey = () => {
+  if (!vmState.ssh_key.trim()) {
+    isSshKeyEmpty.value = true
+    return
+  }
+  vmState.sshKeys.push(vmState.ssh_key.trim())
+  vmState.ssh_key = ''
+  isSshKeyEmpty.value = false
+}
+
+const removeSshKey = (index: number) => {
+  vmState.sshKeys.splice(index, 1)
+}
+


I agree on the duplication detection.
But, ⚠️ not all SSH keys start with ssh-rsa!

OlivierFL · 2026-02-19T08:00:02Z

@xen-orchestra/web/src/pages/vm/new.vue

-        method: vmState.installMode,
-        repository: vmState.installMode === 'network' ? '' : vmState.selectedVdi,
+    vmState.installMode !== 'no-config' &&
+      vmState.installMode !== 'ssh-key' && {


It may be not a change to do on this PR, but it will be nice to extract the installMode values in a constant instead of hardcoding them in multiple places.

OlivierFL · 2026-02-19T09:35:40Z

@xen-orchestra/web/src/pages/vm/new.vue

+    .ssh-chips {
+      display: flex;
+      flex-wrap: wrap;
+      gap: 0.5rem;
+      margin-block-end: 1rem;
+      width: 100%;
+
+      .ssh-chip-wrapper {
+        min-width: 0;
+        max-width: 40rem;
+        display: flex;
+      }
+    }


Not to be done on this PR, but I see in Figma that we should have a UiChipsList component, that would act as the wrapper here.
Maybe add a TODO to update this part when we have the component ready.
@UnelDev it can be done on your XO-1890 card.

OlivierFL · 2026-02-19T09:42:19Z

@xen-orchestra/web/src/pages/vm/new.vue

+                  <div class="ssh-key-area">
+                    <UiTextarea v-model="vmState.ssh_key" required :accent="isSshKeyEmpty ? 'danger' : 'brand'">
+                      {{ t('public-key') }}
+                      <template v-if="isSshKeyEmpty" #info>
+                        {{ t('public-key-mandatory') }}
+                      </template>
+                    </UiTextarea>
+                  </div>


⚠️ isSshKeyError == 'empty'

OlivierFL · 2026-02-20T07:52:29Z

@xen-orchestra/web/src/modules/task/components/list/panel/cards/TaskPropertiesCard.vue

+const propertiesOtherWithoutCloudConfig = computed(() => {
+  const other = properties.value.other
+  const args = other?.args as { cloud_config?: string } | undefined
+
+  if (!args?.cloud_config) {
+    return other
+  }
+
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  const { cloud_config, ...argsCopy } = args
+
+  return {
+    ...other,
+    args: argsCopy,
+  }
+})


What about something like this, using omit from Lodash:

Suggested change

const propertiesOtherWithoutCloudConfig = computed(() => {

const other = properties.value.other

const args = other?.args as { cloud_config?: string } | undefined

if (!args?.cloud_config) {

return other

}

// eslint-disable-next-line @typescript-eslint/no-unused-vars

const { cloud_config, ...argsCopy } = args

return {

...other,

args: argsCopy,

}

})

const propertiesOtherWithoutCloudConfig = computed(() => properties.value.other?.args ? omit(properties.value.other.args, 'cloud_config') : undefined)

OlivierFL · 2026-02-20T07:56:08Z

@xen-orchestra/web/src/modules/task/components/list/panel/cards/TaskPropertiesCard.vue

-      <VtsRecursiveFields :fields="properties.other" />
+      <VtsRecursiveFields :fields="propertiesOtherWithoutCloudConfig" />
    </div>
+    <UiLogEntryViewer :content="cloudConfig" :label="t('cloud-config')" size="small" accent="info" />


Careful here, is cloudConfig is undefined, the component will be displayed as empty.

Suggested change

<UiLogEntryViewer :content="cloudConfig" :label="t('cloud-config')" size="small" accent="info" />

<UiLogEntryViewer v-if="cloudConfig !== undefined" :content="cloudConfig" :label="t('cloud-config')" size="small" accent="info" />

OlivierFL · 2026-02-20T07:57:52Z

@xen-orchestra/web/src/modules/task/components/list/panel/cards/TaskPropertiesCard.vue

-      <VtsRecursiveFields :fields="properties.other" />
+      <VtsRecursiveFields :fields="propertiesOtherWithoutCloudConfig" />
    </div>
+    <UiLogEntryViewer :content="cloudConfig" :label="t('cloud-config')" size="small" accent="info" />


Since we know we want to display this property explicitly, it makes sense to have a translation. However, I'm not sure of the underscore in the translation value (Could_config -> Could config).

Elise-FZI self-assigned this Jan 21, 2026

Elise-FZI added the XO Web 6 label Jan 21, 2026

Elise-FZI added the Highlight label Feb 4, 2026

Elise-FZI added 2 commits February 5, 2026 16:16

feat(new VM): add ssh key to vm creation form

4285574

feat(sidepanel task): cloud_config display update

04bfe1b

Elise-FZI force-pushed the xo6/add-ssh-key-to-vm-creation branch from f0d8f6b to 04bfe1b Compare February 5, 2026 15:17

Elise-FZI changed the title ~~WIP : feat(new VM): add ssh key to vm creation form~~ feat(new VM): add ssh key to vm creation form Feb 5, 2026

Elise-FZI marked this pull request as ready for review February 5, 2026 16:06

Elise-FZI requested a review from UnelDev February 5, 2026 16:07

UnelDev requested changes Feb 6, 2026

View reviewed changes

UnelDev requested a review from OlivierFL February 16, 2026 10:15

OlivierFL requested changes Feb 20, 2026

View reviewed changes

feedbacks number one

19ee2a9

	const isSshKeyEmpty = ref(false)
	const isSshKeyError = ref<'empty' \| 'alreadyExist' \| 'invalidKey' \| false>(false)

	<UiLogEntryViewer :content="cloudConfig" :label="t('cloud-config')" size="small" accent="info" />
	<UiLogEntryViewer :content="cloudConfig" :label="cloud_config" size="small" accent="info" />

	<UiLogEntryViewer :content="cloudConfig" :label="t('cloud-config')" size="small" accent="info" />
	<UiLogEntryViewer v-if="cloudConfig !== undefined" :content="cloudConfig" :label="t('cloud-config')" size="small" accent="info" />

Comments

Conversation

Elise-FZI commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Review process

Uh oh!

plane-sync-vates bot commented Jan 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

UnelDev Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OlivierFL left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Elise-FZI commented Jan 21, 2026 •

edited

Loading

UnelDev Feb 20, 2026 •

edited

Loading