Add strict yaml unmarshal option

[DannyBrito]

This PR adds strict option for yaml unmarshal, any unknow field or typo field in a dalec spec will result in error
This should solve Issue #81

example output when trying to build example with unknow field.

go-md2man-2.yml:1
--------------------
   1 | >>> # syntax=local/dalec/frontend:latest
   2 |     name: go-md2man
   3 |     version: 2.0.3
--------------------
ERROR: failed to solve: error loading spec: error unmarshalling spec: [9:1] unknown field "fake-field"
       6 | license: MIT
       7 | description: A tool to convert markdown into man pages (roff).
       8 | website: https://github.com/cpuguy83/go-md2man
    >  9 | fake-field: test
           ^
      10 | sources:
      11 |   src:
      12 |     git:

[pmengelbert]

I think there's errors in some of the test fixtures. That's causing them to not unmarshal in strict mode -- which is why it's good to have this change.

[DannyBrito]

I looked at what is going on here and this might be a hierarchy problem when doing the unmarshall of the Spec, and this is why when unmarshalling it's not recognizing CheckOutput with TestStep. Could someone please help me validate this?

type Spec struct {
  // ...
  Tests []*TestSpec `yaml:"tests,omitempty" json:"tests,omitempty"`
}

type TestSpec struct {
  Name    string `yaml:"name" json:"name" jsonschema:"required"`
 // Seems like this might be taking presendece over TestStep
  Command `yaml:",inline"`
  Steps []TestStep `yaml:"steps" json:"steps" jsonschema:"required"`
}

type Command struct {
  // ...
  // Steps - as BuildStep instead of TestStep
  Steps []*BuildStep `yaml:"steps" json:"steps" jsonschema:"required"`
}

// TestStep is a wrapper for [BuildStep] to include checks on stdio streams
type TestStep struct {
  BuildStep `yaml:",inline"`
  // Stdout is the expected output on stdout
  Stdout CheckOutput `yaml:"stdout,omitempty" json:"stdout,omitempty"`
  // Stderr is the expected output on stderr
  Stderr CheckOutput `yaml:"stderr,omitempty" json:"stderr,omitempty"`
  // Stdin is the input to pass to stdin for the command
  Stdin string `yaml:"stdin,omitempty" json:"stdin,omitempty"`
}

Seems as we are embedding Command struct in TestSpec the Steps field in command struct is taking precedence, so it is of type BuildStep instead of the explict field Steps as TestStep wrapper

[cpuguy83]

The weird thing is, that is a valid field.

[DannyBrito]

@adamperlin and I had the change to explore this issue a bit more, here is some of the behavior we saw:

TestSpec struct is conflicting during unmarshall, as it is embedded with fields from Command which has an existing Steps field of type []*BuildStep but TestSpec also has explicit written field Steps of type []TestStep.

Unmarshalling behavior without Strict gives precedence to Steps field declared within the TestSpec instead of Steps field embedded in Command effectively making it of type []TestStep.

The observed behavior with Strict is to not recognize the Steps field causing it to fail. So really sure if this is part of the intended behavior of the library or is a bug.

One question about the TestSpec, is the embedded fields from Command Struct ever used? If not, maybe we could do some clean-up of this TestSpec struct and can prevent this issue?

[adamperlin]

Just to add on to what Danny said, if we add an omitempty tag to the embedded Command this issue is resolved for now. If the embedded Command is required, then if a command is NOT specified, in strict mode the unmarshaler will always try and unmarshal a Command, which will partly work if a TestSpec is present due to the steps key being in the yaml for both Command and TestSpec. It will confusingly fail on fields such as stdout, etc. which are specific to TestSpec, which seems to be what happened here.

What was the original purpose for embedded Command?

[cpuguy83]

wrt embedded:

I wonder if we can use yaml:",inline" but without embedding in go? (but this is just a side note to investigate)
That said, it looks like we've got some structs that need to be split out (or fields duplicated, which is ok).

[cpuguy83]

One minor change, and I opened #138 to deal with the command stuff.
I think we just should not inline these data types, its too confusing.

[cpuguy83]

Can you use //go:embed instead?

[cpuguy83]

In case you haven't seen that before: https://pkg.go.dev/embed