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

add yaml validation #2970

Merged
merged 1 commit into from
Nov 9, 2022
Merged

add yaml validation #2970

merged 1 commit into from
Nov 9, 2022

Conversation

chanwit
Copy link
Member

@chanwit chanwit commented Nov 7, 2022

This PR adds yaml validation at an early stage before uploading YAML files to the bucket.
It validates yaml files under the targetDir only.

Fixes #2887

Signed-off-by: Chanwit Kaewkasi chanwit@weave.works

@chanwit chanwit self-assigned this Nov 8, 2022
@chanwit chanwit force-pushed the kube-validate branch 6 times, most recently from 138ed9b to 532b16d Compare November 8, 2022 13:25
@chanwit chanwit marked this pull request as ready for review November 8, 2022 13:27
Copy link
Contributor

@ozamosi ozamosi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Tested "standard podinfo stuff" (create namespace, create flux resources): works ✔️
  • Tested incorrect kustomization schema: rejects my change ✔️
  • Tested other CRD I'm not installing here: document is ignored ✔️
    • This opened my eyes to why my idea "why don't we just ask the cluster for the CRD" is nonsense - I might be installing the CRD together with the resource 🤦
  • Tested a document that was just this: "is not a resource": rejected ✔️
  • kustomization.yaml includes are documented as being out of scope, not tested those.

So overall: ✔️

💡 The output is e.g.

► Validating files under test/ ...
test/podinfo.yaml - Kustomization podinfo is invalid: For field spec: prune is required
✗ Error validating: validation failed

I had a second where I read the last line saying "validation failed" and assumed that was the start of the error output so I should be looking for the error after it - there's nothing after it, so I thought "well that's unhelpful". A possible enhancement would be that if the error is "validation failed" specifically, the error line could be changed to imply it's a summary - "Validation failed - review any errors and try again".

❓ This feature is not optional - is there any risk of false positive where we'd get in the user's way instead of helping them? For example, if we're using different schema versions from the cluster.

Signed-off-by: Chanwit Kaewkasi <chanwit@weave.works>
@chanwit
Copy link
Member Author

chanwit commented Nov 9, 2022

This feature is not optional - is there any risk of false positive where we'd get in the user's way instead of helping them? For example, if we're using different schema versions from the cluster.

Good question. We'll have to pin down specific K8s and Flux versions of schemas in the next ticket.

@chanwit
Copy link
Member Author

chanwit commented Nov 9, 2022

Next ticket: #2986

@chanwit chanwit merged commit 6bf4312 into main Nov 9, 2022
@chanwit chanwit deleted the kube-validate branch November 9, 2022 12:17
chanwit pushed a commit that referenced this pull request Nov 9, 2022
@ozamosi ozamosi added the type/enhancement New feature or request label Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gitops run client-side yaml validation
2 participants