kubevirt / kubevirt

Kubernetes Virtualization API and runtime in order to define and manage virtual machines.
https://kubevirt.io
Apache License 2.0
5.67k stars 1.35k forks source link

`virtctl create vm` doesn't validate that name can be used as part of disk name #13340

Open lyarwood opened 1 week ago

lyarwood commented 1 week ago

/cc @0xFelix

What happened:

$ virtctl create vm --instancetype cx1.medium --volume-containerdisk src:quay.io/containerdisks/fedora:latest --name fedora-cx1.medium  
apiVersion: kubevirt.io/v1
kind: VirtualMachine
metadata:
  creationTimestamp: null
  name: fedora-cx1.medium
spec:
  instancetype:
    name: cx1.medium
  runStrategy: Always
  template:
    metadata:
      creationTimestamp: null
    spec:
      domain:
        devices: {}
        resources: {}
      terminationGracePeriodSeconds: 180
      volumes:
      - containerDisk:
          image: quay.io/containerdisks/fedora:latest
        name: fedora-cx1.medium-containerdisk-0
status: {}
$ virtctl create vm --instancetype cx1.medium --volume-containerdisk src:quay.io/containerdisks/fedora:latest --name fedora-cx1.medium  | kubectl apply -f -
The request is invalid: spec.template.spec.domain.devices.disks.domain.devices.disks[0].name: must not contain dots

What you expected to happen: A clear and concise description of what you expected to happen.

How to reproduce it (as minimally and precisely as possible): Steps to reproduce the behavior.

Additional context: Add any other context about the problem here.

Environment:

dosubot[bot] commented 1 week ago

The issue you're encountering is due to the fact that disk names in KubeVirt must conform to DNS label rules, which do not allow dots in the names. The virtctl create vm command does not currently validate the VM name to ensure it can be used as part of a disk name, leading to the error you experienced.

To address this, you can modify the VM name to avoid using dots or any other characters that are not allowed in DNS labels. This will ensure that the disk names derived from the VM name are valid.

Additionally, there are existing validation mechanisms in virtctl that could be applied to disk names, such as the validateDiskNameAsContainerName function, which ensures that disk names conform to DNS label rules [1]. Implementing similar validation for VM names during the creation process could prevent such issues in the future.

To continue talking to Dosu, mention @dosu.

SWITCHin2 commented 2 days ago

@lyarwood despite adding the validation in the admiter( like validateDiskNameAsContainerName as the dosubot suggested), can we add a validation for vm names in the createVM() function itself?

0xFelix commented 18 hours ago

@SWITCHin2 If you want to address this, this is the right place to add the validation:

https://github.com/kubevirt/kubevirt/blob/be188de70b4466b9f1c74de0eef37d95ed998531/pkg/virtctl/create/vm/vm.go#L522

SWITCHin2 commented 18 hours ago

Ohkay. thank you @0xFelix