instrumenta / kubeval

Validate your Kubernetes configuration files, supports multiple Kubernetes versions
https://kubeval.com
Other
3.16k stars 229 forks source link

Improperly validates DaemonSet #24

Closed rothgar closed 7 years ago

rothgar commented 7 years ago

Example

apiVersion: extensions/v1beta1
kind: DaemonSet
metadata:
  name: nginx-ds
spec:
  replicas: 2
  template:
    spec:
      containers:
      - image: nginx
        name: nginx

kubeval verifies the yaml

kubeval test.yaml
The document test.yaml contains a valid DaemonSet

replicas is invalid for a DaemonSet

kubectl create -f test.yaml
error: error validating "test.yaml": error validating data: found invalid field replicas for v1beta1.DaemonSetSpec; if you choose to ignore these errors, turn validation off with --validate=false

I didn't look at the code (I'm assuming it a straight forward fix) but wanted to report it in case I don't get to it.

garethr commented 7 years ago

Thanks for reporting.

I think the answer here is additionalProperties, which should throw an error whenever a key not in the schema is used. This isn't in the upstream schema, but that might be an openapi vs json schema difference. I'll check and hopefully add that to everything.

garethr commented 7 years ago

So, according to the spec https://github.com/garethr/kubernetes-json-schema/blob/master/master-standalone/daemonset.json this should be valid.

Hopefully this is a generic behaviour of the API and I can patch kubeval accordingly. When I get a moment I'll ask on SIG API Machinery and open an upstream bug.

garethr commented 7 years ago

Posted to API SIG Machinery to sanity check https://groups.google.com/forum/#!topic/kubernetes-sig-api-machinery/sz948IbCH2A

garethr commented 7 years ago

So, looking into this further this behaviour is from kubectl not the Kubernetes API: https://github.com/kubernetes/kubernetes/blob/225b9119d6a8f03fcbe3cc3d590c261965d928d0/pkg/kubectl/validation/schema.go#L312

garethr commented 7 years ago

The deal appears to be:

So kubeval is right, in the sense that the document is valid for the API. But it's probably more useful at least to allow this to do the same checks as kubectl. My plan at the moment is:

garethr commented 7 years ago

I have a PR up with a fix for this in #32, I need a bit of time to add a few tests and some documentation but if you want to build from source you could check it out.

On the sample manifest with additional properties this now correctly flags the replica key.

kubeval --strict .\fixtures\extra_property.yaml
The document .\fixtures\extra_property.yaml contains an invalid DaemonSet
---> replicas: Additional property replicas is not allowed