From 639bca9f19f5a4a18a053e64c55c124f2a1ebeb0 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Thu, 1 May 2025 11:49:16 +0100 Subject: [PATCH] Add declarative validation linting to optionalorrequired --- pkg/analysis/optionalorrequired/analyzer.go | 29 ++++++++++++- .../optionalorrequired/testdata/src/a/a.go | 41 +++++++++++++++++++ .../testdata/src/a/a.go.golden | 41 +++++++++++++++++++ .../optionalorrequired/testdata/src/c/c.go | 8 ++++ .../testdata/src/c/c.go.golden | 6 +++ 5 files changed, 124 insertions(+), 1 deletion(-) diff --git a/pkg/analysis/optionalorrequired/analyzer.go b/pkg/analysis/optionalorrequired/analyzer.go index 695f2e59..35492ef8 100644 --- a/pkg/analysis/optionalorrequired/analyzer.go +++ b/pkg/analysis/optionalorrequired/analyzer.go @@ -41,6 +41,12 @@ const ( // KubebuilderRequiredMarker is the marker that indicates that a field is required in kubebuilder. KubebuilderRequiredMarker = "kubebuilder:validation:Required" + + // K8sOptionalMarker is the marker that indicates that a field is optional in k8s declarative validation. + K8sOptionalMarker = "k8s:Optional" + + // K8sRequiredMarker is the marker that indicates that a field is required in k8s declarative validation. + K8sRequiredMarker = "k8s:Required" ) func init() { @@ -49,6 +55,8 @@ func init() { RequiredMarker, KubebuilderOptionalMarker, KubebuilderRequiredMarker, + K8sOptionalMarker, + K8sRequiredMarker, ) } @@ -135,6 +143,8 @@ func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field, fieldMarker hasBothOptional := hasPrimaryOptional && hasSecondaryOptional hasBothRequired := hasPrimaryRequired && hasSecondaryRequired + a.checkK8sMarkers(pass, field, fieldMarkers, prefix, hasEitherOptional, hasEitherRequired) + switch { case hasEitherOptional && hasEitherRequired: pass.Reportf(field.Pos(), "%s must not be marked as both optional and required", prefix) @@ -159,6 +169,23 @@ func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field, fieldMarker } } +func (a *analyzer) checkK8sMarkers(pass *analysis.Pass, field *ast.Field, fieldMarkers markers.MarkerSet, prefix string, hasEitherOptional, hasEitherRequired bool) { + hasK8sOptional := fieldMarkers.Has(K8sOptionalMarker) + hasK8sRequired := fieldMarkers.Has(K8sRequiredMarker) + + if hasK8sOptional && hasK8sRequired { + pass.Reportf(field.Pos(), "%s must not be marked as both %s and %s", prefix, K8sOptionalMarker, K8sRequiredMarker) + } + + if hasK8sOptional && hasEitherRequired { + pass.Reportf(field.Pos(), "%s must not be marked as both %s and %s", prefix, K8sOptionalMarker, RequiredMarker) + } + + if hasK8sRequired && hasEitherOptional { + pass.Reportf(field.Pos(), "%s must not be marked as both %s and %s", prefix, OptionalMarker, K8sRequiredMarker) + } +} + func reportShouldReplaceSecondaryMarker(field *ast.Field, marker []markers.Marker, primaryMarker, secondaryMarker, prefix string) analysis.Diagnostic { textEdits := make([]analysis.TextEdit, len(marker)) @@ -221,7 +248,7 @@ func (a *analyzer) checkTypeSpec(pass *analysis.Pass, typeSpec *ast.TypeSpec, ma for _, marker := range set.UnsortedList() { switch marker.Identifier { - case a.primaryOptionalMarker, a.secondaryOptionalMarker, a.primaryRequiredMarker, a.secondaryRequiredMarker: + case a.primaryOptionalMarker, a.secondaryOptionalMarker, a.primaryRequiredMarker, a.secondaryRequiredMarker, K8sOptionalMarker, K8sRequiredMarker: pass.Report(analysis.Diagnostic{ Pos: typeSpec.Pos(), Message: fmt.Sprintf("type %s should not be marked as %s", name, marker.String()), diff --git a/pkg/analysis/optionalorrequired/testdata/src/a/a.go b/pkg/analysis/optionalorrequired/testdata/src/a/a.go index 470c2be1..f9a7bbda 100644 --- a/pkg/analysis/optionalorrequired/testdata/src/a/a.go +++ b/pkg/analysis/optionalorrequired/testdata/src/a/a.go @@ -41,6 +41,47 @@ type OptionalOrRequiredTestStruct struct { // +optional MarkedWithKubeBuilderOptionalTwiceAndOptional string // want "field MarkedWithKubeBuilderOptionalTwiceAndOptional should use only the marker optional, kubebuilder:validation:Optional is not required" + // MarkedWithK8sRequiredAndKubeBuilderRequired is a field with both k8s and kubebuilder required markers. + // The k8s versions of the markers are currently in addition to other markers so this is accepted. + // +k8s:Required + // +kubebuilder:validation:Required + MarkedWithK8sRequiredAndKubeBuilderRequired string // want "field MarkedWithK8sRequiredAndKubeBuilderRequired should use marker required instead of kubebuilder:validation:Required" + + // MarkedWithK8sRequiredAndRequired is a field with both k8s and required markers. + // The k8s versions of the markers are currently in addition to other markers so this is accepted. + // +k8s:Required + // +required + MarkedWithK8sRequiredAndRequired string + + // MarkedWithK8sOptionalAndKubeBuilderOptional is a field with both k8s and kubebuilder optional markers. + // The k8s versions of the markers are currently in addition to other markers so this is accepted. + // +k8s:Optional + // +kubebuilder:validation:Optional + MarkedWithK8sOptionalAndKubeBuilderOptional string // want "field MarkedWithK8sOptionalAndKubeBuilderOptional should use marker optional instead of kubebuilder:validation:Optional" + + // MarkedWithK8sOptionalAndOptional is a field with both k8s and optional markers. + // The k8s versions of the markers are currently in addition to other markers so this is accepted. + // +k8s:Optional + // +optional + MarkedWithK8sOptionalAndOptional string + + // MarkedWithK8sOptionalAndRequired is a field with both k8s and required markers. + // The k8s versions of the markers are currently in addition to other markers, but they should match the semantics. + // +k8s:Optional + // +required + MarkedWithK8sOptionalAndRequired string // want "field MarkedWithK8sOptionalAndRequired must not be marked as both k8s:Optional and required" + + // MarkedWithK8sRequiredAndOptional is a field with both k8s and optional markers. + // The k8s versions of the markers are currently in addition to other markers, but they should match the semantics. + // +k8s:Required + // +optional + MarkedWithK8sRequiredAndOptional string // want "field MarkedWithK8sRequiredAndOptional must not be marked as both optional and k8s:Required" + + // MarkedWithK8sRequiredAndK8sOptional is a field with both k8s and kubebuilder optional markers. + // +k8s:Required + // +k8s:Optional + MarkedWithK8sRequiredAndK8sOptional string // want "field MarkedWithK8sRequiredAndK8sOptional must be marked as optional or required" "field MarkedWithK8sRequiredAndK8sOptional must not be marked as both k8s:Optional and k8s:Required" + A `json:",inline"` B `json:"b,omitempty"` // want "embedded field B must be marked as optional or required" diff --git a/pkg/analysis/optionalorrequired/testdata/src/a/a.go.golden b/pkg/analysis/optionalorrequired/testdata/src/a/a.go.golden index 5dad146a..ecaba319 100644 --- a/pkg/analysis/optionalorrequired/testdata/src/a/a.go.golden +++ b/pkg/analysis/optionalorrequired/testdata/src/a/a.go.golden @@ -36,6 +36,47 @@ type OptionalOrRequiredTestStruct struct { // +optional MarkedWithKubeBuilderOptionalTwiceAndOptional string // want "field MarkedWithKubeBuilderOptionalTwiceAndOptional should use only the marker optional, kubebuilder:validation:Optional is not required" + // MarkedWithK8sRequiredAndKubeBuilderRequired is a field with both k8s and kubebuilder required markers. + // The k8s versions of the markers are currently in addition to other markers so this is accepted. + // +k8s:Required + // +required + MarkedWithK8sRequiredAndKubeBuilderRequired string // want "field MarkedWithK8sRequiredAndKubeBuilderRequired should use marker required instead of kubebuilder:validation:Required" + + // MarkedWithK8sRequiredAndRequired is a field with both k8s and required markers. + // The k8s versions of the markers are currently in addition to other markers so this is accepted. + // +k8s:Required + // +required + MarkedWithK8sRequiredAndRequired string + + // MarkedWithK8sOptionalAndKubeBuilderOptional is a field with both k8s and kubebuilder optional markers. + // The k8s versions of the markers are currently in addition to other markers so this is accepted. + // +k8s:Optional + // +optional + MarkedWithK8sOptionalAndKubeBuilderOptional string // want "field MarkedWithK8sOptionalAndKubeBuilderOptional should use marker optional instead of kubebuilder:validation:Optional" + + // MarkedWithK8sOptionalAndOptional is a field with both k8s and optional markers. + // The k8s versions of the markers are currently in addition to other markers so this is accepted. + // +k8s:Optional + // +optional + MarkedWithK8sOptionalAndOptional string + + // MarkedWithK8sOptionalAndRequired is a field with both k8s and required markers. + // The k8s versions of the markers are currently in addition to other markers, but they should match the semantics. + // +k8s:Optional + // +required + MarkedWithK8sOptionalAndRequired string // want "field MarkedWithK8sOptionalAndRequired must not be marked as both k8s:Optional and required" + + // MarkedWithK8sRequiredAndOptional is a field with both k8s and optional markers. + // The k8s versions of the markers are currently in addition to other markers, but they should match the semantics. + // +k8s:Required + // +optional + MarkedWithK8sRequiredAndOptional string // want "field MarkedWithK8sRequiredAndOptional must not be marked as both optional and k8s:Required" + + // MarkedWithK8sRequiredAndK8sOptional is a field with both k8s and kubebuilder optional markers. + // +k8s:Required + // +k8s:Optional + MarkedWithK8sRequiredAndK8sOptional string // want "field MarkedWithK8sRequiredAndK8sOptional must be marked as optional or required" "field MarkedWithK8sRequiredAndK8sOptional must not be marked as both k8s:Optional and k8s:Required" + A `json:",inline"` B `json:"b,omitempty"` // want "embedded field B must be marked as optional or required" diff --git a/pkg/analysis/optionalorrequired/testdata/src/c/c.go b/pkg/analysis/optionalorrequired/testdata/src/c/c.go index 37238d45..a7691581 100644 --- a/pkg/analysis/optionalorrequired/testdata/src/c/c.go +++ b/pkg/analysis/optionalorrequired/testdata/src/c/c.go @@ -18,6 +18,10 @@ type RequiredEnum string // want "type RequiredEnum should not be marked as requ // +kubebuilder:validation:Enum=Foo;Bar;Baz type KubeBuilderRequiredEnum string // want "type KubeBuilderRequiredEnum should not be marked as kubebuilder:validation:Required" +// +k8s:Required +// +kubebuilder:validation:Enum=Foo;Bar;Baz +type K8sRequiredEnum string // want "type K8sRequiredEnum should not be marked as k8s:Required" + // +optional // +kubebuilder:validation:Enum=Foo;Bar;Baz type OptionalEnum string // want "type OptionalEnum should not be marked as optional" @@ -25,3 +29,7 @@ type OptionalEnum string // want "type OptionalEnum should not be marked as opti // +kubebuilder:validation:Optional // +kubebuilder:validation:Enum=Foo;Bar;Baz type KubeBuilderOptionalEnum string // want "type KubeBuilderOptionalEnum should not be marked as kubebuilder:validation:Optional" + +// +k8s:Optional +// +kubebuilder:validation:Enum=Foo;Bar;Baz +type K8sOptionalEnum string // want "type K8sOptionalEnum should not be marked as k8s:Optional" diff --git a/pkg/analysis/optionalorrequired/testdata/src/c/c.go.golden b/pkg/analysis/optionalorrequired/testdata/src/c/c.go.golden index 369df971..560f5f74 100644 --- a/pkg/analysis/optionalorrequired/testdata/src/c/c.go.golden +++ b/pkg/analysis/optionalorrequired/testdata/src/c/c.go.golden @@ -16,8 +16,14 @@ type RequiredEnum string // want "type RequiredEnum should not be marked as requ // +kubebuilder:validation:Enum=Foo;Bar;Baz type KubeBuilderRequiredEnum string // want "type KubeBuilderRequiredEnum should not be marked as kubebuilder:validation:Required" +// +kubebuilder:validation:Enum=Foo;Bar;Baz +type K8sRequiredEnum string // want "type K8sRequiredEnum should not be marked as k8s:Required" + // +kubebuilder:validation:Enum=Foo;Bar;Baz type OptionalEnum string // want "type OptionalEnum should not be marked as optional" // +kubebuilder:validation:Enum=Foo;Bar;Baz type KubeBuilderOptionalEnum string // want "type KubeBuilderOptionalEnum should not be marked as kubebuilder:validation:Optional" + +// +kubebuilder:validation:Enum=Foo;Bar;Baz +type K8sOptionalEnum string // want "type K8sOptionalEnum should not be marked as k8s:Optional"