k8s:optional and k8s:required should be lowercase

[JoelSpeed]

These got uppercased through a hallucination this morning (I thought I'd checked against the existing implementation in validation-gen).

Thanks to @liggitt and @thockin for noticing

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoelSpeed

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [JoelSpeed]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[thockin]

/lgtm

One thing to think about, especially as it related to codegen -- where should we check for different classes of "error"?

1. Some constructions can cause code-generation to fail completely, producing code which is syntactically invalid. For those cases, I think we want to fail as obviously as possible, so tools like validation-gen will explicitly check them and fail. E.g. we know pointer-to-slice is not something we use, so no effort has been put into making it work. Best to fail with "pointers to slices are not supported". Should we add things like that to lint, too?

2. Finding +k8s:format on anything other than a string will generate Go code which is syntactically valid but does not compile. Validation-gen hard fails with an error message. Should we add things like that to lint, too?

For other cases it won't fail codegen, even if it doesn't make any sense. A couple different examples:

3a) Being tagged optional AND required will generate valid Go code, but it's still nonsense. It's not universally easy to cross-check tags, because each validator is self-contained, but tags like optional and required come from the same unit. Should validation-gen fail-hard or let it go and get caught by lint?

3b) Finding +k8s:required on a pointer will generate valid code (in fact we have test cases for it) but it's probably wrong. It's not nonsense, but I think we would be best to say "not supported". Should validation-gen fail-hard or let it go and get caught by lint?

Thoughts?

@jpbetz @yongruilin

[JoelSpeed]

Should we add things like that to lint, too?

I view linters as necessary, but not sufficient generally. Linters are quick, can be integrated into IDEs, and IMO provide an early feedback. Where generation is generally something done later, and takes longer.

With that as my baseline, my view on this is that yes, "pointers to slices" and "k8s:format" should also be covered in lint, to give an early warning, but the generation would be a backstop.

I figure pointers to slices is also likely not great in the CRD world, so adding that rule to KAL extends the usefulness of this knowledge. For the format validation, there's a kubebuilder equivalent and I had intended to check that one in KAL already, so checking the k8s version which appears to have the same semantics (must be on string) should be easy.
In that case, controller-gen will also fail when it sees the kubebuilder format marker on a non-string.

Should validation-gen fail-hard or let it go and get caught by lint?

These second two are harder.

For me this comes down to, if it works, someone is probably using it already.

I suspect, with these as linter rules, we offer the advice, but it can be ignored.

If this is a hard fail in validation-gen, then anyone doing this already has no get out of jail free card, right?