Alex/validation tag issues #729
Conversation
github-actions
bot
added
area/controller
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #729 +/- ##
==========================================
- Coverage 48.99% 48.73% -0.27%
==========================================
Files 95 95
Lines 10594 10594
==========================================
- Hits 5191 5163 -28
- Misses 5044 5066 +22
- Partials 359 365 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| // Metadata is a subset of metav1.ObjectMeta that is added to the Service | ||
| // +kube:validation:Optional | ||
| // +kubebuilder:validation:Optional |
There was a problem hiding this comment.
Even though it was invalid, it looks like this had no effect on the generated output.
| INVALID_PATTERNS=( | ||
| '+kube:validation:' # Should be +kubebuilder:validation: | ||
| '+kube:default:' # Should be +kubebuilder:default: | ||
| '+kube:object:' # Should be +kubebuilder:object: | ||
| '+kube:subresource:' # Should be +kubebuilder:subresource: | ||
| '+kube:printcolumn:' # Should be +kubebuilder:printcolumn: | ||
| '+kube:resource:' # Should be +kubebuilder:resource: | ||
| '+kube:rbac:' # Should be +kubebuilder:rbac: | ||
| '+kube:webhook:' # Should be +kubebuilder:webhook: | ||
| '+kube:skip' # Should be +kubebuilder:skip | ||
| '+kube:storageversion' # Should be +kubebuilder:storageversion | ||
| ) |
There was a problem hiding this comment.
This looks like it will only catch +kube making the marker invalid. It looks like we aren't handling the case of +kuebuilder:object or any other misspellings. Still feel like an improvement, just noting where it might bite us.
I think long term if we want something more robust, maybe we add a linter to golangci-lint for this? I'm sure others might be interested too.
|
|
||
|
|
||
| .PHONY: lint-markers | ||
| lint-markers: ## Check for invalid kubebuilder marker prefixes |
There was a problem hiding this comment.
Should we add this as a dependency for the lint target so that it gets run in CI automatically?
What
We noticed that some of these kubebuilder annotations were incorrect so this fixes them.
We also called out in the issue that we should try to lint for these going forward.
How
Fixing the actual issue is very simple (just updating the annotation name)
Linting for this has proven more difficult. I did not find any well supported libraries that could lint for these values. I worked with amp and it came up with this soluation of hardcoding valid annotations but i'm not a big fan of them.
I'm inclined to just not lint for this.
Breaking Changes
No