Panic from invalid CLI -var if a valid variable is referenced in env{}

[gulducat]

Nomad version

Nomad v1.0.3 (08741d9f2003ec26e44c72a2c0e27cdf0eadb6ee)

Operating system and Environment details

macOS Catalina, but I suspect this is not os-specific.

Issue

Normally when providing an invalid variable name with nomad run -var='somevar=someval',
it will output a mostly sensible error saying the variable is invalid.

In this case it panics with "invalid memory address or nil pointer dereference"

It seems to be on the cli side, a server/agent doesn't even need to be running.
I am not sure if this can occur in other jobspec contexts; I ran across it in env{}

Reproduction steps

Run this with the below jobspec, named bug.nomad here.

nomad run -var='invalid=yes' bug.nomad

or any variable name aside from the valid "one".

Job file

variable "one" {}

job "bug" {
  task "bug" {
    env {
      ONE = var.one # this is what does it.
    }
  }
}

Error output

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

goroutine 1 [running]:
github.com/zclconf/go-cty/cty.Type.Equals(...)
        github.com/zclconf/go-cty@v1.4.1/cty/type.go:39
github.com/zclconf/go-cty/cty/convert.Convert(0x0, 0x0, 0x6847400, 0x8ab2580, 0x74a1f20, 0xc00030fb69, 0x0, 0x0, 0x0, 0x0, ...)
        github.com/zclconf/go-cty@v1.4.1/cty/convert/public.go:43 +0x26
github.com/hashicorp/hcl/v2/gohcl.(*Decoder).DecodeExpression(0x8a76630, 0x74a10e0, 0xc000429aa0, 0xc000206220, 0x660f080, 0xc000466190, 0xa7a3308, 0xa760bb8, 0x100000000000340)
        github.com/hashicorp/hcl/v2@v2.7.1-0.20201020204811-68a97f93bb48/gohcl/decode.go:413 +0x1b6
github.com/hashicorp/hcl/v2/gohcl.DecodeExpression(...)
        github.com/hashicorp/hcl/v2@v2.7.1-0.20201020204811-68a97f93bb48/gohcl/decode.go:384
github.com/hashicorp/hcl/v2/gohcl.(*Decoder).decodeBodyToMap(0xc000466180, 0x74a0ce0, 0xc00022d2f0, 0xc000206220, 0x688d9a0, 0xc00079e1d0, 0x195, 0x101, 0xc000400000, 0xc000a682d8)
        github.com/hashicorp/hcl/v2@v2.7.1-0.20201020204811-68a97f93bb48/gohcl/decode.go:322 +0x6e5
github.com/hashicorp/hcl/v2/gohcl.(*Decoder).decodeBodyToValue(0xc000466180, 0x74a0ce0, 0xc00022d2f0, 0xc000206220, 0x688d9a0, 0xc00079e1d0, 0x195, 0xc0001bdcd0, 0xf, 0x15)
        github.com/hashicorp/hcl/v2@v2.7.1-0.20201020204811-68a97f93bb48/gohcl/decode.go:94 +0x179
github.com/hashicorp/hcl/v2/gohcl.(*Decoder).decodeBlockToValue(0xc000466180, 0xc000798dd0, 0xc000206220, 0x688d9a0, 0xc00079e1d0, 0x195, 0x195, 0x688d9a0, 0x640ba93)
        github.com/hashicorp/hcl/v2@v2.7.1-0.20201020204811-68a97f93bb48/gohcl/decode.go:349 +0x3b2
github.com/hashicorp/hcl/v2/gohcl.(*Decoder).decodeBodyToStruct(0xc000466180, 0x74a0ce0, 0xc00022c870, 0xc000206220, 0x6f1a380, 0xc00079e160, 0x199, 0x6c194a0, 0xc00003c640, 0x6ca87a0)
        github.com/hashicorp/hcl/v2@v2.7.1-0.20201020204811-68a97f93bb48/gohcl/decode.go:296 +0x1f90
github.com/hashicorp/hcl/v2/gohcl.(*Decoder).decodeBodyToValue(0xc000466180, 0x74a0ce0, 0xc00022c870, 0xc000206220, 0x6f1a380, 0xc00079e160, 0x199, 0x0, 0x0, 0x0)
        github.com/hashicorp/hcl/v2@v2.7.1-0.20201020204811-68a97f93bb48/gohcl/decode.go:92 +0xe9
github.com/hashicorp/hcl/v2/gohcl.(*Decoder).DecodeBody(0xc000466180, 0x74a0ce0, 0xc00022c870, 0xc000206220, 0x6c01700, 0xc00079e160, 0x0, 0x0, 0x6c01700)
        github.com/hashicorp/hcl/v2@v2.7.1-0.20201020204811-68a97f93bb48/gohcl/decode.go:85 +0x11e
github.com/hashicorp/nomad/jobspec2.decodeTask(0x74a0ce0, 0xc00022c5d0, 0xc000206220, 0x6c01700, 0xc00079e160, 0xc00079e160, 0x7ffeefbff80f, 0x9)
        github.com/hashicorp/nomad/jobspec2/hcl_conversions.go:303 +0x265
github.com/hashicorp/hcl/v2/gohcl.(*Decoder).decodeBodyToStruct(0xc000980a60, 0x74a0ce0, 0xc00022c5d0, 0xc000206220, 0x6f1a380, 0xc00079e160, 0x199, 0x160, 0x160, 0x6f1a380)
        github.com/hashicorp/hcl/v2@v2.7.1-0.20201020204811-68a97f93bb48/gohcl/decode.go:102 +0x34a2
github.com/hashicorp/hcl/v2/gohcl.(*Decoder).decodeBodyToValue(0xc000980a60, 0x74a0ce0, 0xc00022c5d0, 0xc000206220, 0x6f1a380, 0xc00079e160, 0x199, 0xc000466160, 0xc000a69348, 0x4010c38)
        github.com/hashicorp/hcl/v2@v2.7.1-0.20201020204811-68a97f93bb48/gohcl/decode.go:92 +0xe9
github.com/hashicorp/hcl/v2/gohcl.(*Decoder).DecodeBody(0xc000980a60, 0x74a0ce0, 0xc00022c5d0, 0xc000206220, 0x6c01700, 0xc00079e160, 0x0, 0x203000, 0x203000)
        github.com/hashicorp/hcl/v2@v2.7.1-0.20201020204811-68a97f93bb48/gohcl/decode.go:85 +0x11e
github.com/hashicorp/nomad/jobspec2.(*jobConfig).decodeTopLevelExtras(0xc0002f80e0, 0xc000429e60, 0xc000206220, 0x0, 0x0, 0x0)
        github.com/hashicorp/nomad/jobspec2/types.config.go:162 +0x134
github.com/hashicorp/nomad/jobspec2.(*jobConfig).decodeJob(0xc0002f80e0, 0xc000429ce0, 0xc000206220, 0x0, 0x0, 0x0)
        github.com/hashicorp/nomad/jobspec2/types.config.go:272 +0x3ca
github.com/hashicorp/nomad/jobspec2.(*jobConfig).decodeBody(0xc0002f80e0, 0x74a0fa0, 0xc00021af20, 0x7ffeefbff80f, 0x9, 0xc00003c580)
        github.com/hashicorp/nomad/jobspec2/types.config.go:76 +0x4ed
github.com/hashicorp/nomad/jobspec2.decode(0xc0002f80e0, 0xc0002f80e0, 0x6f73335)
        github.com/hashicorp/nomad/jobspec2/parse.go:95 +0x258
github.com/hashicorp/nomad/jobspec2.ParseWithConfig(0xc00021a420, 0xc00021a420, 0x1c, 0xc000010040)
        github.com/hashicorp/nomad/jobspec2/parse.go:44 +0xa8
github.com/hashicorp/nomad/command.(*JobGetter).ApiJobWithArgs(0xc000799ab0, 0x7ffeefbff80f, 0x9, 0xc000372060, 0x1, 0x1, 0x0, 0x0, 0x0, 0x0, ...)
        github.com/hashicorp/nomad/command/helpers.go:455 +0x41f
github.com/hashicorp/nomad/command.(*JobRunCommand).Run(0xc000799a00, 0xc000138020, 0x2, 0x2, 0xc000981410)
        github.com/hashicorp/nomad/command/job_run.go:197 +0x6ef
github.com/mitchellh/cli.(*CLI).Run(0xc000350b40, 0xc000350b40, 0xc000410920, 0x37)
        github.com/mitchellh/cli@v1.1.0/cli.go:260 +0x1cf
main.RunCustom(0xc000138010, 0x3, 0x3, 0xc00010e058)
        github.com/hashicorp/nomad/main.go:142 +0x4a5
main.Run(...)
        github.com/hashicorp/nomad/main.go:87
main.main()
        github.com/hashicorp/nomad/main.go:83 +0x65

[tgross]

Thanks for opening this @gulducat! You're right that the crash is entirely constrained to the CLI; all the HCL2 parsing happens there and the server just gets the "rendered" JSON API body. But we definitely should fix this!

[tgross]

Hi @gulducat I did some quick reproduction work and I don't think the error here is actually because of the extra variable (-var="invalid=true"). I get the panic even if I do nomad validate ./example.nomad without that extra variable set. But the variable declaration you have for variable "one" is incomplete -- it's missing a type.

If we use the following jobspec:

variable "one" {
  type = string
}

job "bug" {
  task "bug" {
    env {
      ONE = var.one
    }
  }
}

Then we get the following validation errors:

$ nomad validate -var='invalid=yes' ./example.nomad
Error getting job struct: Error parsing job file from ./example.nomad:
<nil>: Undefined -var variable; A "invalid" variable was passed in the command line but was not found in known variables. To declare variable "invalid", place this block in your job file
<nil>: Unset variable "one"; A used variable must be set or have a default value; see https://packer.io/docs/configuration/from-1.5/syntax for details.
example.nomad:8,13-20: Unsuitable value type; Unsuitable value: value must be known

$ nomad validate -var='one=foo' ./example.nomad
Job validation errors:
2 errors occurred:
        * Missing job datacenters
        * Task group bug validation failed: 1 error occurred:
        * Task bug validation failed: 1 error occurred:
        * Missing task driver

LOL on this error message though (copy and pasted from packer 😊 ):

A used variable must be set or have a default value; see https://packer.io/docs/configuration/from-1.5/syntax for details.

Same results with the following jobspec:

variables {
  one = "foo"
}

job "bug" {
  task "bug" {
    env {
      ONE = var.one
    }
  }
}

Which implicitly declares the type. The docs say:

The variable block can optionally include a type argument to specify what value types are accepted for the variable, as described in the following section.

So that definitely could use some improvement. And in any case, we shouldn't be panicking.

[tgross]

I've got a fix in #10045. The bug was sort of interesting: we were catching the error during validation but then merrily continuing on with the invalid state to decode the job, which panicked and obscured the error message. That patch will ship in 1.0.4

[gulducat]

Huzzah! I was hoping this would be a pretty easy one, thanks for the extra info and the fix @tgross!

[github-actions]

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.