Validate Kubernetes objects using kubeval

[bastjan]

This PR adds a validate target for both the Makefile and the Github test action. The action is used to validate the compiled Kubernetes config.

We already use kubeval in some components, this PR aims at creating a standardised way to use it:
https://github.com/projectsyn/component-lieutenant/blob/efa4020cf71a761de81a601da781ae0af459c396/tests/unit/defaults_test.go#L72

See projectsyn/component-kyverno#16 for a sample of the changes.

Checklist

• Keep pull requests small so they can be easily reviewed.
• Categorize the PR by setting a good title and adding one of the labels:
  bug, enhancement, documentation, change, breaking, dependency
  as they show up in the changelog

[ccremer]

In https://syn.tools/syn/SDDs/0026-commodore-component-testing.html we have documented why we didn't use Kubeval back then.

Is that document obsolete? For what reasons?

[bastjan]

In https://syn.tools/syn/SDDs/0026-commodore-component-testing.html we have documented why we didn't use Kubeval back then.

Is that document obsolete? For what reasons?

I was not aware of this document. I saw we're using kubeval and wanted to standardise the process. I'll read the SDD and come back to you.

[bastjan]

In https://syn.tools/syn/SDDs/0026-commodore-component-testing.html we have documented why we didn't use Kubeval back then.

Is that document obsolete? For what reasons?

I would argue the SDD and kubeval can coexist:

• As far as I understand configtest does not validate kubernetes object.
• Go tests are still not widely used and can be verbose to write. Most of them already use kubeval with ~30-40 lines of code copied between them.
• Having some form of basic validation can catch some (most) errors before applying objects to a cluster.
• I do understand kubeval more as a linter than a test.

IMO We have many (too many?) components and not enough manpower to thoroughly unit test them all. Even for the tests we have, they are more or less "structure" tests where using kubeval and golden tests would be mostly sufficient.

[ccremer]

The problem I have with Kubeval is that it gives easily a false sense of security. For example, it doesn't tell whether you should write env or environment in your Deployment config. It just parses it and then checks for syntax. But if the wrong YAML key is provided, then it gets completely ignored in the YAML deserialization, missing an obvious error. With Go unit tests, this can be avoided.
At least that's from earlier Kubeval versions, I don't know if that's improved by now.
And as for CRDs, one has to know that those aren't validated, so it's useless there, but we make extensively use of them (e.g. Prometheus, K8up, cert-manager etc).

In the end, a green check in GH says little to nothing whether my artifacts are valid as expected. And for linting purposes we have Yamllint already? So I'd rather have no test than a bad one, at least I'd know it's not tested.

If a component absolutely needs Kubeval they are free to do so, as you've read in the SDD. But I have my doubts rolling that out for every component.

[bastjan]

The problem I have with Kubeval is that it gives easily a false sense of security. For example, it doesn't tell whether you should write env or environment in your Deployment config. [...] At least that's from earlier Kubeval versions, I don't know if that's improved by now.

I did enable --strict mode which solves this problem.

And as for CRDs, one has to know that those aren't validated, so it's useless there, but we make extensively use of them (e.g. Prometheus, K8up, cert-manager etc).

ACK. This PR won't solve CRDs.

In the end, a green check in GH says little to nothing whether my artifacts are valid as expected.

A green checkmark never says a software is ok as in valid or bug free. This PR does not aim at formal verification of a component. It does however improve testing slightly where a component is only compiled as a test.

And for linting purposes we have Yamllint already? So I'd rather have no test than a bad one, at least I'd know it's not tested.

Yamllint does not lint the compiled output. Commodore does have different ideas how compiled yaml should look than our yamllint config.

If a component absolutely needs Kubeval they are free to do so, as you've read in the SDD. But I have my doubts rolling that out for every component.

I still prefer to allow people to enable it with one line in sync.yml than copying many lines of Go code. It is not enabled by default.

[ccremer]

A green checkmark never says a software is ok as in valid or bug free. This PR does not aim at formal verification of a component.

Agreed, but then I woudn't call it validate either. It'll be just linting in that case. For me, "validation" verifies semantic, whereas linting only verifies syntax. A generic Kubeval check would then only be linting as it has no idea if the spec is semantically correct.

copying many lines of Go code.

This is only the current case where we don't have a helper library/repository yet. It was an experiment started in one component, but not rolled out to every component. So maybe we should actually do it now?

I'm still not a fan of rolling out Kubeval as I see no value, but if the majority of contributors think it's useful, I won't stand in the way.

Please also update the SDD page if this gets merged.

[ccremer]

resulting from the discussion, I would not call it validate since that's not a formal validation but rather just K8s-specific linting, thus name it lint_kubeval or so.

Suggested change

	<%- if @configs['feature_kubevalValidate'] -%>
	.PHONY: validate
	validate: commodore_args = -f tests/$(instance).yml --search-paths ./dependencies
	validate: .compile ## Validate the resulting Kubernetes objects using kubeval.
	@echo
	@echo
	$(KUBEVAL_DOCKER) $(KUBEVAL_ARGS) -d compiled/
	<%- end -%>
	<%- if @configs['feature_kubevalLint'] -%>
	.PHONY: lint_kubeval
	lint_kubeval: commodore_args = -f tests/$(instance).yml --search-paths ./dependencies
	lint_kubeval: .compile ## Lint the resulting Kubernetes objects using kubeval (ignores CRDs!)
	@echo
	@echo
	$(KUBEVAL_DOCKER) $(KUBEVAL_ARGS) -d compiled/
	<%- end -%>

[ccremer]

Suggested change

	feature_kubevalValidate: false
	feature_kubevalLint: false

[corvus-ch]

Settle on wheter to use "validate" or "lint" (I am fine with both. Then ensure you use the chosen term consistently.

[simu]

Closing this since we're migrating away from modulesync, I created an issue to track this suggestion on the component template repo: projectsyn/commodore-component-template#11