Add validation for resource schemas

[HumairAK]

[tumido]

Compare oc --local=true , oc --dry-run=true, https://www.kubeval.com/ etc

[tumido]

I came to a conclusion that kubeval is the best solution. After that I actually found out we're not the only ones with the same problem (a very recent article from Andrew from the GitOps team):
https://www.openshift.com/blog/validating-openshift-manifests-in-a-gitops-world

We can either use Andrew's repo here:
https://github.com/sabre1041/k8s-manifest-validation

Or use kubeval on our own instead. Andrew is using Tekton based CI, we probably want to defer this role to prow's presubmit job.

We will also need to generate our own collection of schemas for CRDs so we don't have to ignore them. Andrew already has ArgoCD schemas in his repo, we may want to collaborate on that - if we can provide a centralized community controlled schema store for CRDs that would be ideal.

[sesheta]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

/lifecycle stale

[HumairAK]

/remove-lifecycle stale

[larsks]

I've been playing with kubeval recently, and it's fairly easy to:

1. Grab all the OpenAPI schemas from the cluster's /openapi/v2 endpoint.
2. Generate JSON schemas using openapi2jsonschema

The above steps could be tossed into a job that runs periodically to keep a per-cluster schema repository up-to-date. The advantage of this over a shared schema repository is that the schemas exactly match the resources available on the target cluster.

Of course, the problem with the above sequence is that it breaks down when you're adding new CRDs to the cluster, because if we're getting the schemas from the cluster, and if the same PR that adds the CRDs also uses the CRDs, the validation would fail. Workarounds include:

• Always running kubeval with --ignore-missing-schemas
• Extracting schemas from the CRDs and updating a schema repository prior to making the PR

I would probably vote for the first option as being the path of least resistance, particularly since it would bring us back to the Bad Old Days of having to make commits to multiple repositories when adding something new to the apps repository.

[tumido]

Thanks @larsks, that got me thinking...

... a per-cluster schema repository up-to-date. The advantage of this over a shared schema repository is that the schemas exactly match the resources available on the target cluster.

I don't think we should have a per-cluster schema repository simply because most manifests are shared between anyways and we don't always know if the manifests are applied to one cluster or another. I think we should rather use a single scheme repository containing union of all per-cluster scheme sets. I don't think we ever got into troubles because our manifests matched the schema on one cluster and didn't match on another. I think we can even take this one more step further and host the schemas in the apps repository, because that would allow us to easily run local validations on-repo.

In addition to the periodic job you mentioned, Prow can watch for modifications to cluster-scope/base/apiextensions.k8s.io/customresourcedefinitions and trigger a job to update the schema store.

Of course, the problem with the above sequence is that it breaks down when you're adding new CRDs to the cluster, because if we're getting the schemas from the cluster, and if the same PR that adds the CRDs also uses the CRDs, the validation would fail.

I think otherwise. I think FAIL is the right result in this case. --ignore-missing-schemas would PASS on kind/apiVersion typos. I'd rather see the CI to FAIL on resources unknown to our schema store. I don't know how about you but I never look at PASSing CI check logs, on the other hand I do care if it FAILs. In such cases we can easily determine that the PR is the case of adding a CRD + CR and there's missing schema so we can override it in the PR via /override

Notes for @mimotej:

• Kubeval needs to be executed against manifests rendered by Kustomize, because overlay patches are partials not matching the scheme.

[larsks]

Kubeval needs to be executed against manifests rendered by Kustomize, because overlay patches are partials not matching the scheme.

And even regular manifests may be invalid before being processed by kustomize build -- e.g., A Deployment or Service may not have a selector until kustomize applies the commonLabels.

[larsks]

In addition to the periodic job you mentioned, Prow can watch for modifications to cluster-scope/base/apiextensions.k8s.io/customresourcedefinitions and trigger a job to update the schema store.

This is a neat idea, and could potentially remove (most of) the need for manually managing schemas if we can teach ourselves to submit CRDs as a separate PR.