Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Kubernetes Lists #53

Closed
maxbrunet opened this issue Jul 2, 2021 · 8 comments · Fixed by #54
Closed

Support Kubernetes Lists #53

maxbrunet opened this issue Jul 2, 2021 · 8 comments · Fixed by #54

Comments

@maxbrunet
Copy link
Contributor

Validating Lists does not seem to be supported:

stdin - failed validation: could not find schema for List

kubeconform should recognize the kind and validate items individually.

We mostly use it to validate Jsonnet deployments, this format allows the output to be directly passed to kubectl apply -f - (since JSON does not have an equivalent to YAML document streams). An example out there if that can clarify:
https://github.com/brancz/kubernetes-grafana/blob/master/examples/basic.jsonnet

This is supported in kubeval 0.15.0+. Here is the initial implementation and follow up fixes if that helps:
instrumenta/kubeval#221
instrumenta/kubeval#283
instrumenta/kubeval#284


From a quick look at the code, it seems we can only identify Kind: List from this line:

if err := yaml.Unmarshal(res.Bytes, &r); err != nil {

We would probably need to unmarshall before ValidateResource(), if we want to benefit of the parallel workers:

for i := 0; i < cfg.NumberOfWorkers; i++ {
wg.Add(1)
go func(resources <-chan resource.Resource, validationResults chan<- validator.Result, v validator.Validator) {
for res := range resources {
validationResults <- v.ValidateResource(res)
}
wg.Done()
}(resourcesChan, validationResults, v)
}

@yannh
Copy link
Owner

yannh commented Jul 3, 2021

Hi @maxbrunet , thanks for the report. Kubeconform should work as a dropin replacement for kubeval and therefore support Lists. I am looking into this 👍

Note from looking at kubeval support: if this requires unmarshalling to detect, it might have a significant impact on performance :(

@yannh
Copy link
Owner

yannh commented Jul 3, 2021

WIP: https://github.com/yannh/kubeconform/pull/54/files
The results of the signature call is actually cached, so this should not have an impact in most cases. We'll need a second unmarshall when the resource is a list, but that is likely unavoidable.

@yannh yannh mentioned this issue Jul 3, 2021
@yannh
Copy link
Owner

yannh commented Jul 3, 2021

@maxbrunet this should be good to go https://github.com/yannh/kubeconform/pull/54/files
Want to review?

Note: Like kubeval, I am assuming noone is weird enough to make recursive lists ;)

@yannh yannh closed this as completed in #54 Jul 3, 2021
@yannh yannh reopened this Jul 3, 2021
@yannh
Copy link
Owner

yannh commented Jul 3, 2021

Went ahead and released v0.4.8, lmk if this works for you!

@maxbrunet
Copy link
Contributor Author

Wow thank you @yannh, that was fast!

I had started doing some work, but not much, mostly tried to get around with a single unmarshall.

I had spotted this:

const maxResourceSize = 4 * 1024 * 1024 // 4MB ought to be enough for everybody

It works for small regular deployments, but I have a 17MB output for our Grafana deployment, mainly dozens of dashboards in ConfigMaps: 😛

Summary: 0 resource found parsing stdin - Valid: 0, Invalid: 0, Errors: 0, Skipped: 0

Note: Like kubeval, I am assuming noone is weird enough to make recursive lists ;)

Good news, lists cannot be nested, items must be APIResources: https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#APIResourceList
Although there are other types of APIResourceLists. For example, RoleBindingList and RoleList are used here:
https://github.com/prometheus-operator/kube-prometheus/blob/f9fd5bd499482486de1485abc73effbb6a51f35f/jsonnet/kube-prometheus/components/prometheus.libsonnet#L135-L139

I used to flatten those in my deployment, until I reconfigured it differently and stopped needing them. They have schemas, but I guess kubeconform (or kubeval as of now) would not fail if it sees them inside a List and does not validate the resources inside (likely just a nitpick).

@yannh
Copy link
Owner

yannh commented Jul 3, 2021

@maxbrunet the results of the unmarshall to get the signature are cached, so it should save an unmarshall down the line and be ok in the end. The second unmarshall... If you have an idea to remove it go for it :)
Regarding the maxResourceSize: I think to remember that making this too big actually might allocate quite a lot of memory. How about I make this configurable through an environment variable? Could you configure first that it works if you strip down your file to less than 4MB?

Thanks!

@maxbrunet
Copy link
Contributor Author

Yes, I confirm, as I said it works for an output of less than 4MB.
And yes, a configurable maxResourceSize could do the trick.

But have you tried to use a default buffer with io.Copy() and let it resize itself like in instrumenta/kubeval#220? Does that use a lot more memory?

If you want to reproduce, here's a couple quick and dirty scripts to generate large files:

json.go
package main

import "fmt"

const (
	N = 20000
)

func main() {
	fmt.Println(
		`{
    "apiVersion": "v1",
    "kind": "List",
    "items": [`,
	)

	for i := 0; i < N; i++ {
		fmt.Printf(
			`        {
            "apiVersion": "v1",
            "kind": "ConfigMap",
            "metadata": {
                "name": "yet-another-confimap-%d"
            },
            "data": {
                "key": "value"
            }
        }`,
			i,
		)
		if i == N - 1 {
			fmt.Println()
		} else {
			fmt.Println(",")
		}
	}

	fmt.Println(`    ]
}
`)
}
yaml.go
package main

import "fmt"

func main() {
	fmt.Println(
		`apiVersion: v1
kind: List
items:`,
	)

	for i := 0; i < 50000; i++ {
		fmt.Printf(
			`- apiVersion: v1
  kind: ConfigMap
  metadata:
    name: yet-another-confimap-%d
  data:
    key: value
`,
			i,
		)
	}
}

@yannh
Copy link
Owner

yannh commented Jul 4, 2021

@maxbrunet I think the problem is slightly different - if the support for lists works, would you mind opening a new ticket for large resource files? 🙏
I might review whether there might be a way to avoid a maximum file size, but not sure when :)

@yannh yannh closed this as completed Jul 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants