unikraft / kraftkit

Build and use highly customized and ultra-lightweight unikernel VMs.
https://unikraft.org/docs/cli
BSD 3-Clause "New" or "Revised" License
234 stars 63 forks source link

Nil Pointer Dereference when volume driver is provided in kraft file #1488

Closed alanpjohn closed 6 months ago

alanpjohn commented 7 months ago

Describe the bug

If user uses the long-hand syntax for volume specification in the kraftfile, kraft run panics but it works fine if the short syntax is used. The issue arises due to a nil pointer dereference in the parseKraftfileVolumes function

  controllers := map[string]volumeapi.VolumeService{}
  if machine.Spec.Volumes == nil {
    machine.Spec.Volumes = make([]volumeapi.Volume, 0)
  }

  for _, volcfg := range project.Volumes() {
    driver := volcfg.Driver()

    if len(driver) == 0 {
      for sname, strategy := range volume.Strategies() {
        if ok, _ := strategy.IsCompatible(volcfg.Source(), nil); !ok || err != nil {
          continue
        }

        if _, ok := controllers[sname]; !ok {
          controllers[sname], err = strategy.NewVolumeV1alpha1(ctx)
          if err != nil {
            return fmt.Errorf("could not prepare %s volume service: %w", sname, err)
          }
        }

        driver = sname
      }
    }

    if len(driver) == 0 {
      return fmt.Errorf("could not find compatible volume driver for %s", volcfg.Source())
    }

    vol, err := controllers[driver].Create(ctx, &volumeapi.Volume{

When a driver is specified, then the controllers map is not filled with compatible volume services.

I have implemented a quick fix at https://github.com/alanpjohn/kraftkit/blob/volume_fix/internal/cli/kraft/run/utils.go which mitigates the issue and would be happy to clean to up to contribute :)

Steps to reproduce

Using the long-hand syntax described here to attach a volume through kraft file specification

...

volumes:
  - source: "/tmp/mnt"
    dest: "/tmp"
    driver: 9pfs
    readOnly: false

...

If we now run kraft run

$ kraft run .
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x1818e4c]

goroutine 1 [running]:
kraftkit.sh/internal/cli/kraft/run.(*RunOptions).parseKraftfileVolumes(0xc0004bc780?, {0x206f9f8, 0xc0004fce40}, {0x208e7a0, 0xc000368700}, 0xc0002ef608)
    /__w/kraftkit/kraftkit/internal/cli/kraft/run/utils.go:284 +0x42c
kraftkit.sh/internal/cli/kraft/run.(*runnerKraftfileUnikraft).Prepare(0xc000400780, {0x206f9f8, 0xc0004fce40}, 0xc000216000, 0xc0002ef608, {0x1?, 0xb?, 0xc00061d550?})
    /__w/kraftkit/kraftkit/internal/cli/kraft/run/runner_kraftfile_unikraft.go:201 +0xb8b
kraftkit.sh/internal/cli/kraft/run.(*RunOptions).Run(0xc000216000, {0x206f9f8, 0xc0004fce40}, {0xc00061e790, 0x1, 0xb})
    /__w/kraftkit/kraftkit/internal/cli/kraft/run/run.go:337 +0xe5a
kraftkit.sh/cmdfactory.New.func1(0xc000034308?, {0xc00061e790?, 0x0?, 0x0?})
    /__w/kraftkit/kraftkit/cmdfactory/builder.go:367 +0x43
kraftkit.sh/cmdfactory.AttributeFlags.bind.func3(0xc000034308, {0xc00061e790, 0x1, 0xb})
    /__w/kraftkit/kraftkit/cmdfactory/builder.go:549 +0x1cb
github.com/spf13/cobra.(*Command).execute(0xc000034308, {0xc00061e6e0, 0xb, 0xb})
    /go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:983 +0xaca
github.com/spf13/cobra.(*Command).ExecuteC(0xc000498608)
    /go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1115 +0x3ff
github.com/spf13/cobra.(*Command).Execute(...)
    /go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1039
github.com/spf13/cobra.(*Command).ExecuteContext(...)
    /go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1032
kraftkit.sh/cmdfactory.Main({0x206f9f8, 0xc00045ced0}, 0xc000498608)
    /__w/kraftkit/kraftkit/cmdfactory/builder.go:183 +0x6f
kraftkit.sh/internal/cli/kraft.Main({0x7ffe206197b4?, 0x2e8ca18?, 0xc0000061c0?})
    /__w/kraftkit/kraftkit/internal/cli/kraft/kraft.go:186 +0x606
main.main()
    /__w/kraftkit/kraftkit/cmd/kraft/main.go:19 +0x79

Here is the kraft version used

$ kraft version
kraft 0.8.1 (752a90c6d780070467ecad4f30e1ce746c3a4fb9) go1.22.0 2024-03-26T18:52:55Z

Expected behavior

The expected behavior is for kraft to use the volume driver specified without panicking

Which architectures were you using or does this bug affect?

No response

Which operating system were you using or does this bug affect?

No response

Relevant log output

D  kraftkit 0.8.1
 T  initializing package manager format=oci
 T  using containerd handler addr=/run/containerd/containerd.sock namespace=default
 T  initializing package manager format=manifest
 T  checking if source is compatible with the manifest manager source=lwip
 T  trying manifest provider path=lwip
 T  trying index provider path=lwip
 T  trying directory provider path=lwip
 T  trying git provider path=lwip
 T  checking if source is compatible with the manifest manager source=zlib
 T  trying manifest provider path=zlib
 T  trying index provider path=zlib
 T  trying directory provider path=zlib
 T  trying git provider path=zlib
 D  using platform=qemu
 D  determining how to proceed given provided input and context
 T  checking runnability runner=linuxu
 T  candidate is not runnable because: first positional argument is a directory: . candidate=linuxu
 T  checking runnability runner=kernel
 T  candidate is not runnable because: first positional argument is a directory: /home/alanpjohn/Documents/unikernel/benchmarks/06.file-uploader/unikernel candidate=kernel
 T  checking runnability runner=kraftfile-unikraft
 T  checking runnability runner=kraftfile-runtime
 T  candidate is not runnable because: cannot run project without runtime directive candidate=kraftfile-runtime
 T  checking runnability runner=oci
 T  candidate is not runnable because: arguments represent a file or directory candidate=oci
 T  checking runnability runner=manifest
 T  candidate is not runnable because: arguments represent a file or directory candidate=manifest
 D  using compatible context candidate=kraftfile-unikraft
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x1818e4c]

goroutine 1 [running]:
kraftkit.sh/internal/cli/kraft/run.(*RunOptions).parseKraftfileVolumes(0xc000151980?, {0x206f9f8, 0xc0003c64e0}, {0x208e7a0, 0xc0005b9300}, 0xc000486008)
    /__w/kraftkit/kraftkit/internal/cli/kraft/run/utils.go:284 +0x42c
kraftkit.sh/internal/cli/kraft/run.(*runnerKraftfileUnikraft).Prepare(0xc0006852c0, {0x206f9f8, 0xc0003c64e0}, 0xc000236480, 0xc000486008, {0x1?, 0xd?, 0xc000717550?})
    /__w/kraftkit/kraftkit/internal/cli/kraft/run/runner_kraftfile_unikraft.go:201 +0xb8b
kraftkit.sh/internal/cli/kraft/run.(*RunOptions).Run(0xc000236480, {0x206f9f8, 0xc0003c64e0}, {0xc0002a88f0, 0x1, 0xd})
    /__w/kraftkit/kraftkit/internal/cli/kraft/run/run.go:337 +0xe5a
kraftkit.sh/cmdfactory.New.func1(0xc000528308?, {0xc0002a88f0?, 0x0?, 0x0?})
    /__w/kraftkit/kraftkit/cmdfactory/builder.go:367 +0x43
kraftkit.sh/cmdfactory.AttributeFlags.bind.func3(0xc000528308, {0xc0002a88f0, 0x1, 0xd})
    /__w/kraftkit/kraftkit/cmdfactory/builder.go:549 +0x1cb
github.com/spf13/cobra.(*Command).execute(0xc000528308, {0xc0002a8820, 0xd, 0xd})
    /go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:983 +0xaca
github.com/spf13/cobra.(*Command).ExecuteC(0xc000214f08)
    /go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1115 +0x3ff
github.com/spf13/cobra.(*Command).Execute(...)
    /go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1039
github.com/spf13/cobra.(*Command).ExecuteContext(...)
    /go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1032
kraftkit.sh/cmdfactory.Main({0x206f9f8, 0xc00071c0c0}, 0xc000214f08)
    /__w/kraftkit/kraftkit/cmdfactory/builder.go:183 +0x6f
kraftkit.sh/internal/cli/kraft.Main({0x7ffc71b8476e?, 0x2e8ca18?, 0xc0000061c0?})
    /__w/kraftkit/kraftkit/internal/cli/kraft/kraft.go:186 +0x606
main.main()
    /__w/kraftkit/kraftkit/cmd/kraft/main.go:19 +0x79
craciunoiuc commented 7 months ago

Nice catch, will investigate

craciunoiuc commented 7 months ago

Ok, nice catch, if you want to open a PR, go ahead and I can review it 🙏

alanpjohn commented 7 months ago

@craciunoiuc thank you, will do