Merge pull request #76 from JoelSpeed/declarative-gen-optional-required

k8s-ci-robot · web-flow · commit 1901c31973f7 · 2025-05-01T04:21:54.000-07:00
Add declarative validation linting to optionalorrequired
diff --git 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
@@ -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
@@ -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
@@ -18,10 +18,18 @@ 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"
 
 // +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
@@ -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"
