carvel-dev / kbld

kbld seamlessly incorporates image building and image pushing into your development and deployment workflows
https://carvel.dev/kbld
Apache License 2.0
293 stars 40 forks source link

validate kbld config for unrecognized keys #10

Open cppforlife opened 5 years ago

joaopapereira commented 3 years ago

@cppforlife can you provide some more context about this issue. Also if it is still something that we would like to have implemented or not.

cppforlife commented 3 years ago

this is probably something we should do for kapp, imgpkg and other tools as well. goal is to error when yaml parser encounters a field that kbld config structs do not know about. eg

kind: Config
apiVersion: kbld.k14s.io/v1alpha1
unknownField: true <---- presence of this field should cause an error at parse time since it's not part of Config struct
joaopapereira commented 3 years ago

So I came up with some scenarios for this story. @cppforlife what do you think?

Scenario On field is unknown Given I have the following file

kind: Config
apiVersion: kbld.k14s.io/v1alpha1
unknownField: true

When I execute kbld -f file.yml Then I should see the message

Error: Configuration has the following unknown fields:
  unknownField

Scenario Multiple unknown fields Given I have the following file

kind: Config
apiVersion: kbld.k14s.io/v1alpha1
sources:
- path: /some/path
  unknownSourceKey: should fail
destinations:
- newImage: some.registry.io/some-image
- unknownDestinationKey: fail.registry.io/not-present

When I execute kbld -f file.yml Then I should see the message

Error: Configuration have the following unknown fields:
  sources[0].unknownSourceKey
  destinations[1].unkownDestinationKey
StevenLocke commented 3 years ago

The golang yaml package has an UnmarshalStrict method which seems to accomplish exactly what's asked here.

UnmarshalStrict is like Unmarshal except that any fields that are found in the data that do not have corresponding struct members, or mapping keys that are duplicates, will result in an error.

I'd imagine that the resulting error would include which keys caused a problem and then we could simply exit out with the same error.

cppforlife commented 3 years ago

let's take error message returned by whatever library we are using for yaml unmarshaling (not sure if it's sigs.k8s.io/yaml or yaml.v2).