diff --git a/docs/linters.md b/docs/linters.md index 594f8407..58ed0775 100644 --- a/docs/linters.md +++ b/docs/linters.md @@ -6,6 +6,7 @@ - [ConflictingMarkers](#conflictingmarkers) - Detects mutually exclusive markers on the same field - [DefaultOrRequired](#defaultorrequired) - Ensures fields marked as required do not have default values - [DuplicateMarkers](#duplicatemarkers) - Checks for exact duplicates of markers +- [Enums](#enums) - Enforces proper usage of enumerated fields with type aliases and +enum marker - [DependentTags](#dependenttags) - Enforces dependencies between markers - [ForbiddenMarkers](#forbiddenmarkers) - Checks that no forbidden markers are present on types/fields. - [Integers](#integers) - Validates usage of supported integer types @@ -235,13 +236,45 @@ will not. The `duplicatemarkers` linter can automatically fix all markers that are exact match to another markers. If there are duplicates across fields and their underlying type, the marker on the type will be preferred and the marker on the field will be removed. +## Enums + +The `enums` linter enforces that string type aliases used for enumerated values have proper enum markers. + +**Enum Marker Types:** +- **CRD Validation Marker** (`+kubebuilder:validation:Enum`): Used for CRD validation in projects with CustomResourceDefinitions. Processed by controller-gen to generate OpenAPI schema validation. +- **Declarative Validation Marker** (`+k8s:enum`): Used in Kubernetes core API types for declarative validation. + +**Enum Marker Modes:** +- **Auto-discovery** (`+k8s:enum` or `+kubebuilder:validation:Enum`): Validates that constants follow PascalCase +- **Explicit values** (`+kubebuilder:validation:Enum=Value1;Value2`): Skips constant validation + +**PascalCase allows:** "Pending", "PhaseRunning", "HTTP", "IPv4" (acronyms and digits). +**PascalCase rejects:** "pending", "phase_pending", "Phase-Failed". + +**Note:** The linter only flags string type aliases that have associated constants (indicating enum usage), avoiding false positives for generic string types. + +By default, `enums` is enabled. + +### Configuration + +```yaml +linterConfig: + enums: + # Values exempt from PascalCase validation + allowlist: + - kubectl + - docker + # When true, flags plain string fields (default: false) + requireTypeAliasForEnums: false +``` + ## ForbiddenMarkers The `forbiddenmarkers` linter ensures that types and fields do not contain any markers that are forbidden. By default, `forbiddenmarkers` is not enabled. -### Configuation +### Configuration It can be configured with a list of marker identifiers and optionally their attributes and values that are forbidden. diff --git a/pkg/analysis/enums/analyzer.go b/pkg/analysis/enums/analyzer.go new file mode 100644 index 00000000..c9d9d58e --- /dev/null +++ b/pkg/analysis/enums/analyzer.go @@ -0,0 +1,430 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package enums + +import ( + "fmt" + "go/ast" + "go/types" + "slices" + "strings" + "unicode" + + "golang.org/x/tools/go/analysis" + kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors" + "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags" + "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector" + markershelper "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers" + "sigs.k8s.io/kube-api-linter/pkg/analysis/utils" + "sigs.k8s.io/kube-api-linter/pkg/markers" +) + +const name = "enums" + +func init() { + // Register enum markers so they can be parsed + markershelper.DefaultRegistry().Register( + markers.KubebuilderEnumMarker, + markers.K8sEnumMarker, + ) +} + +type analyzer struct { + config *Config +} + +func newAnalyzer(cfg *Config) *analysis.Analyzer { + a := &analyzer{config: cfg} + + return &analysis.Analyzer{ + Name: name, + Doc: "Enforces that string type aliases with constants have enum markers (+kubebuilder:validation:Enum for CRDs, +k8s:enum for core APIs) and that enum values follow PascalCase conventions", + Run: a.run, + Requires: []*analysis.Analyzer{inspector.Analyzer}, + } +} + +func (a *analyzer) run(pass *analysis.Pass) (any, error) { + inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector) + if !ok { + return nil, kalerrors.ErrCouldNotGetInspector + } + + // Check struct fields for proper enum usage + inspect.InspectFields(func(field *ast.Field, _ extractjsontags.FieldTagInfo, markersAccess markershelper.Markers, qualifiedFieldName string) { + a.checkField(pass, field, markersAccess, qualifiedFieldName) + }) + + // Check type declarations for enum markers + inspect.InspectTypeSpec(func(typeSpec *ast.TypeSpec, markersAccess markershelper.Markers) { + a.checkTypeSpec(pass, typeSpec, markersAccess) + }) + + a.checkConstValues(pass) + + return nil, nil //nolint:nilnil +} + +func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelper.Markers, qualifiedFieldName string) { + if qualifiedFieldName == "" { + return + } + + fieldType, isArray := unwrapTypeWithArrayTracking(field.Type) + ident, ok := fieldType.(*ast.Ident) + + if !ok { + return + } + + prefix := buildFieldPrefix(qualifiedFieldName, isArray) + + if ident.Name == "string" && utils.IsBasicType(pass, ident) { + if a.config != nil && a.config.RequireTypeAliasForEnums { + a.checkPlainStringField(pass, field, markersAccess, prefix) + } + + return + } + + a.checkTypeAliasField(pass, field, ident, markersAccess, prefix) +} + +func (a *analyzer) checkPlainStringField(pass *analysis.Pass, field *ast.Field, markersAccess markershelper.Markers, prefix string) { + if !hasEnumMarker(markersAccess.FieldMarkers(field)) { + pass.Reportf(field.Pos(), + "%s uses plain string type. Consider using a type alias with an enum marker (+kubebuilder:validation:Enum or +k8s:enum)", + prefix) + } +} + +func (a *analyzer) checkTypeAliasField(pass *analysis.Pass, field *ast.Field, ident *ast.Ident, markersAccess markershelper.Markers, prefix string) { + if utils.IsBasicType(pass, ident) { + return + } + + typeSpec, ok := utils.LookupTypeSpec(pass, ident) + if !ok || !isStringTypeAlias(pass, typeSpec) { + return + } + // Only check if this type has constants (indicating enum usage) + if !typeHasConstants(pass, typeSpec.Name.Name) { + return + } + // Check for enum markers (CRD validation or declarative validation) + if !hasEnumMarker(markersAccess.TypeMarkers(typeSpec)) { + pass.Reportf(field.Pos(), + "%s uses type %s which appears to be an enum but is missing an enum marker (+kubebuilder:validation:Enum or +k8s:enum)", + prefix, typeSpec.Name.Name) + } +} + +func (a *analyzer) checkTypeSpec(pass *analysis.Pass, typeSpec *ast.TypeSpec, markersAccess markershelper.Markers) { + if typeSpec.Name == nil { + return + } + + typeMarkers := markersAccess.TypeMarkers(typeSpec) + + if !hasEnumMarker(typeMarkers) { + return + } + + if !isStringTypeAlias(pass, typeSpec) { + pass.Reportf(typeSpec.Pos(), + "type %s has enum marker but underlying type is not string", + typeSpec.Name.Name) + } +} + +func (a *analyzer) checkConstValues(pass *analysis.Pass) { + for _, file := range pass.Files { + for _, decl := range file.Decls { + genDecl, ok := decl.(*ast.GenDecl) + if !ok || genDecl.Tok.String() != "const" { + continue + } + + for _, spec := range genDecl.Specs { + if valueSpec, ok := spec.(*ast.ValueSpec); ok { + a.checkConstSpec(pass, valueSpec) + } + } + } + } +} + +func (a *analyzer) checkConstSpec(pass *analysis.Pass, valueSpec *ast.ValueSpec) { + for i, name := range valueSpec.Names { + a.validateEnumConstant(pass, name, valueSpec, i) + } +} + +func (a *analyzer) validateEnumConstant(pass *analysis.Pass, name *ast.Ident, valueSpec *ast.ValueSpec, index int) { + if name == nil || index >= len(valueSpec.Values) { + return + } + + typeSpec := a.getEnumTypeSpec(pass, name) + if typeSpec == nil { + return + } + + // Extract and validate the enum value + basicLit, ok := valueSpec.Values[index].(*ast.BasicLit) + if !ok { + return + } + + strValue := strings.Trim(basicLit.Value, `"`) + + if !a.isInAllowlist(strValue) && !isPascalCase(strValue) { + pass.Reportf(basicLit.Pos(), + "enum value %q should be PascalCase (e.g., \"PhasePending\", \"StateRunning\")", + strValue) + } +} + +func (a *analyzer) getEnumTypeSpec(pass *analysis.Pass, name *ast.Ident) *ast.TypeSpec { + constObj, ok := pass.TypesInfo.ObjectOf(name).(*types.Const) + if !ok { + return nil + } + + namedType, ok := constObj.Type().(*types.Named) + + if !ok || namedType.Obj().Pkg() == nil || namedType.Obj().Pkg() != pass.Pkg { + return nil + } + + typeSpec := findTypeSpecByName(pass, namedType.Obj().Name()) + if typeSpec == nil || !usesAutoDiscovery(pass, typeSpec) { + return nil + } + + return typeSpec +} + +func (a *analyzer) isInAllowlist(value string) bool { + if a.config == nil { + return false + } + + return slices.Contains(a.config.Allowlist, value) +} + +// Helper functions below this line. + +// buildFieldPrefix constructs a human-readable prefix for error messages. +func buildFieldPrefix(fieldName string, isArray bool) string { + if isArray { + return fmt.Sprintf("field %s array element", fieldName) + } + + return fmt.Sprintf("field %s", fieldName) +} + +// unwrapType removes pointer and array wrappers to get the underlying type. +func unwrapType(expr ast.Expr) ast.Expr { + switch t := expr.(type) { + case *ast.StarExpr: + return unwrapType(t.X) + case *ast.ArrayType: + return unwrapType(t.Elt) + default: + return expr + } +} + +// unwrapTypeWithArrayTracking removes pointer and array wrappers to get the underlying type +// and tracks whether an array was encountered during unwrapping. +func unwrapTypeWithArrayTracking(expr ast.Expr) (ast.Expr, bool) { + isArray := false + + for { + switch t := expr.(type) { + case *ast.StarExpr: + expr = t.X + case *ast.ArrayType: + expr = t.Elt + isArray = true + default: + return expr, isArray + } + } +} + +// isStringTypeAlias checks if a type spec is a string type alias. +func isStringTypeAlias(pass *analysis.Pass, typeSpec *ast.TypeSpec) bool { + underlyingType := unwrapType(typeSpec.Type) + ident, ok := underlyingType.(*ast.Ident) + + if !ok { + return false + } + + // Both checks are needed: name check is fast, IsBasicType handles edge cases + // where 'string' might be redefined (rare but possible) + return ident.Name == "string" && utils.IsBasicType(pass, ident) +} + +// hasEnumMarker checks if a marker set contains enum markers +// (either CRD validation marker or declarative validation marker). +func hasEnumMarker(markerSet markershelper.MarkerSet) bool { + return markerSet.Has(markers.KubebuilderEnumMarker) || + markerSet.Has(markers.K8sEnumMarker) +} + +// usesAutoDiscovery checks if a type uses auto-discovery mode for enum values. +// Auto-discovery mode validates that constants follow PascalCase conventions. +// +// Returns true for: +// - +k8s:enum (declarative validation marker, always auto-discovers) +// - +kubebuilder:validation:Enum without explicit values (CRD validation marker in auto-discovery mode) +// +// Returns false for: +// - +kubebuilder:validation:Enum=value1;value2 (CRD validation marker with explicit values) +func usesAutoDiscovery(pass *analysis.Pass, typeSpec *ast.TypeSpec) bool { + for _, file := range pass.Files { + genDecl := findGenDeclForSpec(file, typeSpec) + if genDecl == nil || genDecl.Doc == nil { + continue + } + + for _, comment := range genDecl.Doc.List { + text := comment.Text + // Must be an actual marker (starts with "// +") + if !strings.HasPrefix(text, "// +") { + continue + } + + markerContent := strings.TrimPrefix(text, "// +") + + // +k8s:enum always uses auto-discovery + if strings.HasPrefix(markerContent, markers.K8sEnumMarker) { + return true + } + // +kubebuilder:validation:Enum without explicit values uses auto-discovery + if strings.HasPrefix(markerContent, markers.KubebuilderEnumMarker) { + afterMarker := markerContent[len(markers.KubebuilderEnumMarker):] + // If there's an "=" or ":=" immediately after, it has explicit values + trimmed := strings.TrimSpace(afterMarker) + if strings.HasPrefix(trimmed, "=") || strings.HasPrefix(trimmed, ":=") { + return false // Explicit values mode + } + + return true // Auto-discovery mode + } + } + } + + return false +} + +func findGenDeclForSpec(file *ast.File, typeSpec *ast.TypeSpec) *ast.GenDecl { + for _, decl := range file.Decls { + genDecl, ok := decl.(*ast.GenDecl) + if !ok { + continue + } + + for _, spec := range genDecl.Specs { + if spec == typeSpec { + return genDecl + } + } + } + + return nil +} + +// findTypeSpecByName finds a type spec by its name in the pass's files. +func findTypeSpecByName(pass *analysis.Pass, typeName string) *ast.TypeSpec { + for _, file := range pass.Files { + for _, decl := range file.Decls { + genDecl, ok := decl.(*ast.GenDecl) + if !ok { + continue + } + + for _, spec := range genDecl.Specs { + typeSpec, ok := spec.(*ast.TypeSpec) + if !ok { + continue + } + + if typeSpec.Name != nil && typeSpec.Name.Name == typeName { + return typeSpec + } + } + } + } + + return nil +} + +// typeHasConstants checks if any constants are defined for the given type name. +func typeHasConstants(pass *analysis.Pass, typeName string) bool { + for _, file := range pass.Files { + for _, decl := range file.Decls { + genDecl, ok := decl.(*ast.GenDecl) + + if !ok || genDecl.Tok.String() != "const" { + continue + } + + for _, spec := range genDecl.Specs { + valueSpec, ok := spec.(*ast.ValueSpec) + if !ok || valueSpec.Type == nil { + continue + } + // Check if the const has this type + if ident, ok := valueSpec.Type.(*ast.Ident); ok && ident.Name == typeName { + return true + } + } + } + } + + return false +} + +// isPascalCase checks if a string follows PascalCase naming convention. +// Allows: PascalCase (Running), all-uppercase acronyms (HTTP, API), single letters (A). +// Rejects: lowercase start (running), snake_case (phase_pending), kebab-case (phase-pending). +func isPascalCase(s string) bool { + if len(s) == 0 { + return false + } + // Must start with uppercase + if !unicode.IsUpper(rune(s[0])) { + return false + } + + // No underscores or hyphens allowed (these indicate snake_case/kebab-case) + for _, r := range s { + if r == '_' || r == '-' { + return false + } + // Allow letters, digits, and "+" (for signal names like SIGRTMIN+1) + if !unicode.IsLetter(r) && !unicode.IsDigit(r) && r != '+' { + return false + } + } + // Valid: starts with uppercase, no underscores/hyphens + // Accepts: PascalCase, HTTP, HTTPS, SIGRTMIN+1, etc. + return true +} diff --git a/pkg/analysis/enums/analyzer_test.go b/pkg/analysis/enums/analyzer_test.go new file mode 100644 index 00000000..6150cfbc --- /dev/null +++ b/pkg/analysis/enums/analyzer_test.go @@ -0,0 +1,55 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package enums_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "sigs.k8s.io/kube-api-linter/pkg/analysis/enums" +) + +func TestAnalyzer(t *testing.T) { + testdata := analysistest.TestData() + + // Test with default config (plain strings allowed) + config := &enums.Config{ + RequireTypeAliasForEnums: false, + } + analyzer, err := enums.Initializer().Init(config) + + if err != nil { + t.Fatalf("initializing enums linter: %v", err) + } + + analysistest.Run(t, testdata, analyzer, "a") +} + +func TestAnalyzerWithAllowlist(t *testing.T) { + testdata := analysistest.TestData() + + // Test with allowlist + config := &enums.Config{ + Allowlist: []string{"kubectl", "docker", "helm"}, + } + + analyzer, err := enums.Initializer().Init(config) + if err != nil { + t.Fatalf("initializing enums linter: %v", err) + } + + analysistest.Run(t, testdata, analyzer, "b") +} diff --git a/pkg/analysis/enums/config.go b/pkg/analysis/enums/config.go new file mode 100644 index 00000000..cb38407e --- /dev/null +++ b/pkg/analysis/enums/config.go @@ -0,0 +1,28 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package enums + +// Config is the configuration for the enums linter. +type Config struct { + // Allowlist contains values that are exempt from PascalCase validation. + // This is useful for command-line executable names like "kubectl", "docker", etc. + Allowlist []string `yaml:"allowlist" json:"allowlist"` + + // RequireTypeAliasForEnums when true, enforces that string fields representing enums + // must use type aliases instead of plain string types. + // Default: false (plain strings are allowed) + RequireTypeAliasForEnums bool `yaml:"requireTypeAliasForEnums" json:"requireTypeAliasForEnums"` +} diff --git a/pkg/analysis/enums/doc.go b/pkg/analysis/enums/doc.go new file mode 100644 index 00000000..72bc8770 --- /dev/null +++ b/pkg/analysis/enums/doc.go @@ -0,0 +1,132 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +/* +enums is an analyzer that enforces proper usage of enumerated fields in Kubernetes APIs. + +Enumerated fields provide better API evolution and self-documentation compared to plain strings. +By using type aliases with explicit enum markers, the API schema can include valid values and +provide better validation at the schema level. + +# Rules + +1. String type aliases that have associated constants must be annotated with an enum marker: + - +kubebuilder:validation:Enum for CRD validation (used in projects with CustomResourceDefinitions) + - +k8s:enum for declarative validation (used in Kubernetes core API types) + +2. Enum constant values must follow PascalCase when using auto-discovery mode. +Valid: "Pending", "Running", "HTTP", "HTTPS" (acronyms allowed). +Invalid: "pending", "phase_pending", "Phase-Failed" (snake_case/kebab-case). + +3. (Optional) When RequireTypeAliasForEnums is true: String fields without enum markers +should use type aliases instead of plain string types. + +The linter only flags type aliases that have constants defined, avoiding false positives +for generic string wrapper types. + +# Enum Marker Types + +CRD Validation Marker (+kubebuilder:validation:Enum): +- Used in projects with CustomResourceDefinitions +- Processed by controller-gen to generate OpenAPI schema validation +- Supports two modes: + - Auto-discovery: +kubebuilder:validation:Enum (validates constants as PascalCase) + - Explicit values: +kubebuilder:validation:Enum=Pending;Running;Failed (skips constant validation) + +Declarative Validation Marker (+k8s:enum): +- Used in Kubernetes core API types (in-tree APIs) +- Part of the k8s declarative validation system +- Always uses auto-discovery mode (validates constants as PascalCase) + +Examples: + +Auto-discovery (validates constants): + + // +k8s:enum ← Always auto-discovers constants + // +kubebuilder:validation:Enum ← Auto-discovers when no explicit values + type Phase string + const ( + PhasePending Phase = "Pending" ← These must be PascalCase + ) + +Explicit values (constants not validated): + + // +kubebuilder:validation:Enum=Pending;Running;Failed ← Uses these specific values + type Phase string + const ( + helper Phase = "helper" ← Not validated, marker values are used + ) + +# Examples + +Good: + + // +kubebuilder:validation:Enum + type Phase string + const ( + PhasePending Phase = "Pending" + PhaseRunning Phase = "Running" + ) + type MySpec struct { + Phase Phase + } + +Bad: + + // String type alias with constants but missing enum marker + type Phase string + const ( + phase_pending Phase = "pending" // Should be "Pending" + phase_running Phase = "phase_running" // Should be "PhaseRunning" + Phase_Failed Phase = "Phase-Failed" // Should be "PhaseFailed" + ) + +Note: Acronyms (HTTP, HTTPS, API) are allowed. The linter only flags type aliases with +constants, not all string types. + +# Configuration + +Configuration options: + + linterConfig: + enums: + # Values exempt from PascalCase validation + allowlist: + - kubectl + - docker + # Require type aliases for enum fields (default: false) + requireTypeAliasForEnums: false + +# Rationale + +Using type aliases with enum markers instead of raw strings provides several benefits: +- API schemas can explicitly list valid enum values +- Better validation at both the schema and runtime level +- Improved documentation and API evolution +- Stronger type safety in generated clients +- Clearer intent for API consumers + +The PascalCase convention for enum values aligns with Kubernetes API conventions and +improves readability and consistency across the ecosystem. + +The distinction between CRD validation markers and declarative validation markers allows +the linter to work correctly in both CRD-based projects (using controller-gen) and +Kubernetes core API development (using declarative validation). + +Note: This linter is enabled by default to ensure string types with constants follow enum +conventions. It only flags types that have associated constants, minimizing false positives. +*/ +package enums diff --git a/pkg/analysis/enums/initializer.go b/pkg/analysis/enums/initializer.go new file mode 100644 index 00000000..2bd0e428 --- /dev/null +++ b/pkg/analysis/enums/initializer.go @@ -0,0 +1,53 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package enums + +import ( + "golang.org/x/tools/go/analysis" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/kube-api-linter/pkg/analysis/initializer" + "sigs.k8s.io/kube-api-linter/pkg/analysis/registry" +) + +func init() { + registry.DefaultRegistry().RegisterLinter(Initializer()) +} + +// Initializer returns the AnalyzerInitializer for this +// Analyzer so that it can be added to the registry. +func Initializer() initializer.AnalyzerInitializer { + return initializer.NewConfigurableInitializer( + name, + initAnalyzer, + // Enabled by default: validates string type aliases with constants have enum markers + true, + validateConfig, + ) +} + +func initAnalyzer(cfg *Config) (*analysis.Analyzer, error) { + if cfg == nil { + cfg = &Config{} + } + + return newAnalyzer(cfg), nil +} + +// validateConfig implements validation of the enums linter config. +func validateConfig(cfg *Config, fldPath *field.Path) field.ErrorList { + // Config is optional, allowlist can be empty + return field.ErrorList{} +} diff --git a/pkg/analysis/enums/testdata/src/a/a.go b/pkg/analysis/enums/testdata/src/a/a.go new file mode 100644 index 00000000..cfc92584 --- /dev/null +++ b/pkg/analysis/enums/testdata/src/a/a.go @@ -0,0 +1,141 @@ +package a + +// Valid: +kubebuilder:validation:Enum without explicit values (auto-discovers constants) +// +kubebuilder:validation:Enum=Pending;Running;Succeeded;Failed +type Phase string + +const ( + PhasePending Phase = "Pending" // Valid PascalCase + PhaseRunning Phase = "Running" // Valid PascalCase + PhaseSucceeded Phase = "Succeeded" // Valid PascalCase + PhaseFailed Phase = "Failed" // Valid PascalCase +) + +// Valid: +k8s:enum always auto-discovers constants +// +k8s:enum +type State string + +const ( + StateActive State = "Active" // Valid PascalCase + StateInactive State = "Inactive" // Valid PascalCase +) + +// Valid: +kubebuilder:validation:Enum with explicit values (doesn't check constants) +// +kubebuilder:validation:Enum=Pending;helper +type ExplicitPhase string + +// These constants are helpers - not validated because marker has explicit values +const ( + ExplicitPending ExplicitPhase = "Pending" + explicit_helper ExplicitPhase = "helper" // Not flagged - explicit values mode +) + +// Invalid: Type alias without enum marker (will be flagged when used in fields) +type Status string + +const ( + StatusReady Status = "Ready" + StatusNotReady Status = "NotReady" +) + +// Invalid: Type with kubebuilder:validation:Enum marker but underlying type is not string +// +kubebuilder:validation:Enum +type InvalidEnumType int // want "type InvalidEnumType has enum marker but underlying type is not string" + +// Invalid enum values (not PascalCase) - using auto-discovery +// +kubebuilder:validation:Enum +type BadPhase string + +const ( + phase_pending BadPhase = "pending" // want "enum value \"pending\" should be PascalCase" + phase_failed BadPhase = "Phase-Failed" // want "enum value \"Phase-Failed\" should be PascalCase" +) + +// Valid: Acronyms and all-caps are allowed +// +kubebuilder:validation:Enum +type Protocol string + +const ( + ProtocolHTTP Protocol = "HTTP" // Valid: acronym + ProtocolHTTPS Protocol = "HTTPS" // Valid: acronym + ProtocolTCP Protocol = "TCP" // Valid: acronym +) + +// Test struct with fields +type MySpec struct { + // Valid: uses type alias with enum marker + Phase Phase + + // Valid: uses type alias with enum marker + State State + + // Valid: plain string (not required to be enum unless RequireTypeAliasForEnums is enabled) + PlainString string + + // Invalid: type alias without enum marker + Status Status // want "field MySpec.Status uses type Status which appears to be an enum but is missing an enum marker \\(\\+kubebuilder:validation:Enum or \\+k8s:enum\\)" + + // Valid: pointer to enum type + PhasePtr *Phase + + // Valid: slice of enum type + Phases []Phase + + // Valid: plain string slice (not required to be enum) + PlainStrings []string + + // Valid: explicit enum type + Explicit ExplicitPhase +} + +// Test pointer fields +type PointerSpec struct { + PhasePtr *Phase + StatusPtr *Status // want "field PointerSpec.StatusPtr uses type Status which appears to be an enum but is missing an enum marker \\(\\+kubebuilder:validation:Enum or \\+k8s:enum\\)" +} + +// Test array fields +type ArraySpec struct { + Phases []Phase + Statuses []Status // want "field ArraySpec.Statuses array element uses type Status which appears to be an enum but is missing an enum marker \\(\\+kubebuilder:validation:Enum or \\+k8s:enum\\)" +} + +// Embedded field test +type EmbeddedSpec struct { + MySpec + Phase Phase +} + +// Edge case: Field with enum marker directly on the field (allowed as exception) +type DirectMarkerSpec struct { + // +kubebuilder:validation:Enum + DirectEnum string // Valid: has enum marker directly on field +} + +// Edge case: Enum values with numbers (should be valid PascalCase) +// +kubebuilder:validation:Enum=Priority1;Priority2 +type Priority string + +const ( + Priority1 Priority = "Priority1" // Valid: PascalCase with number + Priority2 Priority = "Priority2" // Valid: PascalCase with number +) + +// Valid: Single letter and all-caps values +// +kubebuilder:validation:Enum=A;B;API +type Level string + +const ( + LevelA Level = "A" // Valid: single letter + LevelB Level = "B" // Valid: single letter + LevelAPI Level = "API" // Valid: acronym +) + +// Edge case: Map with enum types (maps are allowed, not enforced to use enum types) +type MapSpec struct { + // Valid: map with enum value type + PhaseMap map[string]Phase + + // Valid: map with plain string value (maps are not enforced to use enums) + PlainMap map[string]string +} diff --git a/pkg/analysis/enums/testdata/src/b/b.go b/pkg/analysis/enums/testdata/src/b/b.go new file mode 100644 index 00000000..9c6f7b06 --- /dev/null +++ b/pkg/analysis/enums/testdata/src/b/b.go @@ -0,0 +1,29 @@ +package b + +// Test with allowlist configuration + +// Valid enum type with proper marker +// +kubebuilder:validation:Enum +type ExecutableName string + +const ( + ExecKubectl ExecutableName = "kubectl" // Valid: in allowlist + ExecDocker ExecutableName = "docker" // Valid: in allowlist + ExecHelm ExecutableName = "helm" // Valid: in allowlist + ExecUnknown ExecutableName = "unknown" // want "enum value \"unknown\" should be PascalCase" +) + +// Regular enum with PascalCase (should still work) +// +kubebuilder:validation:Enum +type Status string + +const ( + StatusReady Status = "Ready" // Valid PascalCase + StatusNotReady Status = "NotReady" // Valid PascalCase +) + +// Test struct +type MyConfig struct { + Executable ExecutableName + Status Status +} diff --git a/pkg/registration/doc.go b/pkg/registration/doc.go index 439a7513..528a3bb9 100644 --- a/pkg/registration/doc.go +++ b/pkg/registration/doc.go @@ -30,6 +30,7 @@ import ( _ "sigs.k8s.io/kube-api-linter/pkg/analysis/defaultorrequired" _ "sigs.k8s.io/kube-api-linter/pkg/analysis/dependenttags" _ "sigs.k8s.io/kube-api-linter/pkg/analysis/duplicatemarkers" + _ "sigs.k8s.io/kube-api-linter/pkg/analysis/enums" _ "sigs.k8s.io/kube-api-linter/pkg/analysis/forbiddenmarkers" _ "sigs.k8s.io/kube-api-linter/pkg/analysis/integers" _ "sigs.k8s.io/kube-api-linter/pkg/analysis/jsontags" diff --git a/tests/integration/testdata/default_configurations/affinity.go b/tests/integration/testdata/default_configurations/affinity.go index 64eb5417..a0820d4c 100644 --- a/tests/integration/testdata/default_configurations/affinity.go +++ b/tests/integration/testdata/default_configurations/affinity.go @@ -215,7 +215,7 @@ type Taint struct { } -// +enum +// +kubebuilder:validation:Enum type TaintEffect string const ( @@ -269,7 +269,7 @@ type Toleration struct { } // A toleration operator is the set of operators that can be used in a toleration. -// +enum +// +kubebuilder:validation:Enum type TolerationOperator string const ( @@ -322,7 +322,7 @@ type NodeSelectorRequirement struct { // A node selector operator is the set of operators that can be used in // a node selector requirement. -// +enum +// +kubebuilder:validation:Enum type NodeSelectorOperator string const ( @@ -454,7 +454,7 @@ type TopologySpreadConstraint struct { MatchLabelKeys []string `json:"matchLabelKeys,omitempty" protobuf:"bytes,8,opt,name=matchLabelKeys"` } -// +enum +// +kubebuilder:validation:Enum type UnsatisfiableConstraintAction string const ( @@ -467,7 +467,7 @@ const ( ) // NodeInclusionPolicy defines the type of node inclusion policy -// +enum +// +kubebuilder:validation:Enum type NodeInclusionPolicy string const ( @@ -478,7 +478,7 @@ const ( ) // PreemptionPolicy describes a policy for if/when to preempt a pod. -// +enum +// +kubebuilder:validation:Enum type PreemptionPolicy string const ( diff --git a/tests/integration/testdata/default_configurations/container.go b/tests/integration/testdata/default_configurations/container.go index 6d48cd7f..b0c47ace 100644 --- a/tests/integration/testdata/default_configurations/container.go +++ b/tests/integration/testdata/default_configurations/container.go @@ -209,7 +209,7 @@ type Container struct { } // Protocol defines network protocols supported for things like container ports. -// +enum +// +kubebuilder:validation:Enum type Protocol string const ( @@ -453,6 +453,7 @@ type ResourceClaim struct { } // ResourceResizeRestartPolicy specifies how to handle container resource resize. +// +kubebuilder:validation:Enum=NotRequired;RestartContainer type ResourceResizeRestartPolicy string // These are the valid resource resize restart policy values: @@ -484,6 +485,7 @@ type ContainerResizePolicy struct { // ContainerRestartPolicy is the restart policy for a single container. // The only allowed values are "Always", "Never", and "OnFailure". +// +kubebuilder:validation:Enum=Always;Never;OnFailure type ContainerRestartPolicy string const ( @@ -498,7 +500,7 @@ type ContainerRestartRule struct { // are satisfied. The only possible value is "Restart" to restart the // container. // +required - Action ContainerRestartRuleAction `json:"action,omitempty" proto:"bytes,1,opt,name=action" protobuf:"bytes,1,opt,name=action,casttype=ContainerRestartRuleAction"` // want "requiredfields: field Action has a valid zero value \\(\\\"\\\"\\), but the validation is not complete \\(e.g. minimum length\\). The field should be a pointer to allow the zero value to be set. If the zero value is not a valid use case, complete the validation and remove the pointer." + Action ContainerRestartRuleAction `json:"action,omitempty" proto:"bytes,1,opt,name=action" protobuf:"bytes,1,opt,name=action,casttype=ContainerRestartRuleAction"` // Represents the exit codes to check on container exits. // want "commentstart: godoc for field ContainerRestartRule.ExitCodes should start with 'exitCodes ...'" // +optional @@ -508,6 +510,7 @@ type ContainerRestartRule struct { // ContainerRestartRuleAction describes the action to take when the // container exits. +// +kubebuilder:validation:Enum=Restart type ContainerRestartRuleAction string // The only valid action is Restart. @@ -525,7 +528,7 @@ type ContainerRestartRuleOnExitCodes struct { // - NotIn: the requirement is satisfied if the container exit code is // not in the set of specified values. // +required - Operator ContainerRestartRuleOnExitCodesOperator `json:"operator,omitempty" proto:"bytes,1,opt,name=operator" protobuf:"bytes,1,opt,name=operator,casttype=ContainerRestartRuleOnExitCodesOperator"` // want "requiredfields: field Operator has a valid zero value \\(\\\"\\\"\\), but the validation is not complete \\(e.g. minimum length\\). The field should be a pointer to allow the zero value to be set. If the zero value is not a valid use case, complete the validation and remove the pointer." + Operator ContainerRestartRuleOnExitCodesOperator `json:"operator,omitempty" proto:"bytes,1,opt,name=operator" protobuf:"bytes,1,opt,name=operator,casttype=ContainerRestartRuleOnExitCodesOperator"` // Specifies the set of values to check for container exit codes. // want "commentstart: godoc for field ContainerRestartRuleOnExitCodes.Values should start with 'values ...'" // At most 255 elements are allowed. @@ -536,6 +539,7 @@ type ContainerRestartRuleOnExitCodes struct { // ContainerRestartRuleOnExitCodesOperator describes the operator // to take for the exit codes. +// +kubebuilder:validation:Enum type ContainerRestartRuleOnExitCodesOperator string const ( @@ -634,7 +638,7 @@ type HTTPHeader struct { } // URIScheme identifies the scheme used for connection to a host for Get actions -// +enum +// +kubebuilder:validation:Enum type URIScheme string const ( @@ -689,7 +693,7 @@ type SleepAction struct { } // Signal defines the stop signal of containers -// +enum +// +kubebuilder:validation:Enum=SIGABRT;SIGALRM;SIGBUS;SIGCHLD;SIGCLD;SIGCONT;SIGFPE;SIGHUP;SIGILL;SIGINT;SIGIO;SIGIOT;SIGKILL;SIGPIPE;SIGPOLL;SIGPROF;SIGPWR;SIGQUIT;SIGSEGV;SIGSTKFLT;SIGSTOP;SIGSYS;SIGTERM;SIGTRAP;SIGTSTP;SIGTTIN;SIGTTOU;SIGURG;SIGUSR1;SIGUSR2;SIGVTALRM;SIGWINCH;SIGXCPU;SIGXFSZ;SIGRTMIN;SIGRTMIN+1;SIGRTMIN+2;SIGRTMIN+3;SIGRTMIN+4;SIGRTMIN+5;SIGRTMIN+6;SIGRTMIN+7;SIGRTMIN+8;SIGRTMIN+9;SIGRTMIN+10;SIGRTMIN+11;SIGRTMIN+12;SIGRTMIN+13;SIGRTMIN+14;SIGRTMIN+15;SIGRTMAX-14;SIGRTMAX-13;SIGRTMAX-12;SIGRTMAX-11;SIGRTMAX-10;SIGRTMAX-9;SIGRTMAX-8;SIGRTMAX-7;SIGRTMAX-6;SIGRTMAX-5;SIGRTMAX-4;SIGRTMAX-3;SIGRTMAX-2;SIGRTMAX-1;SIGRTMAX type Signal string const ( @@ -808,7 +812,7 @@ type LifecycleHandler struct { } // TerminationMessagePolicy describes how termination messages are retrieved from a container. -// +enum +// +kubebuilder:validation:Enum type TerminationMessagePolicy string const ( @@ -822,7 +826,7 @@ const ( ) // PullPolicy describes a policy for if/when to pull a container image -// +enum +// +kubebuilder:validation:Enum type PullPolicy string const ( @@ -918,7 +922,7 @@ type SecurityContext struct { AppArmorProfile *AppArmorProfile `json:"appArmorProfile,omitempty" protobuf:"bytes,12,opt,name=appArmorProfile"` } -// +enum +// +kubebuilder:validation:Enum type ProcMountType string const ( @@ -1012,7 +1016,7 @@ type SeccompProfile struct { } // SeccompProfileType defines the supported seccomp profile types. -// +enum +// +kubebuilder:validation:Enum type SeccompProfileType string const ( @@ -1044,7 +1048,7 @@ type AppArmorProfile struct { LocalhostProfile *string `json:"localhostProfile,omitempty" protobuf:"bytes,2,opt,name=localhostProfile"` } -// +enum +// +kubebuilder:validation:Enum type AppArmorProfileType string const ( @@ -1072,7 +1076,7 @@ type HostAlias struct { // Only one of the following restart policies may be specified. // If none of the following policies is specified, the default one // is RestartPolicyAlways. -// +enum +// +kubebuilder:validation:Enum type RestartPolicy string const ( @@ -1082,7 +1086,7 @@ const ( ) // DNSPolicy defines how a pod's DNS will be configured. -// +enum +// +kubebuilder:validation:Enum type DNSPolicy string const ( diff --git a/tests/integration/testdata/default_configurations/pod.go b/tests/integration/testdata/default_configurations/pod.go index f0bde5e5..08f43e4c 100644 --- a/tests/integration/testdata/default_configurations/pod.go +++ b/tests/integration/testdata/default_configurations/pod.go @@ -388,6 +388,7 @@ type PodReadinessGate struct { } // PodConditionType is a valid value for PodCondition.Type +// +kubebuilder:validation:Enum=ContainersReady;Initialized;Ready;PodScheduled;DisruptionTarget;PodReadyToStartContainers;PodResizePending;PodResizeInProgress type PodConditionType string // These are built-in conditions of pod. An application may use a custom condition not listed here. @@ -420,6 +421,7 @@ const ( PodResizeInProgress PodConditionType = "PodResizeInProgress" ) +// +kubebuilder:validation:Enum=linux;windows // OSName is the set of OS'es that can be used in OS. type OSName string diff --git a/tests/integration/testdata/default_configurations/security_context.go b/tests/integration/testdata/default_configurations/security_context.go index c54d07f5..73a8e8a4 100644 --- a/tests/integration/testdata/default_configurations/security_context.go +++ b/tests/integration/testdata/default_configurations/security_context.go @@ -128,7 +128,7 @@ type PodSecurityContext struct { // SupplementalGroupsPolicy defines how supplemental groups // of the first container processes are calculated. -// +enum +// +kubebuilder:validation:Enum=Merge;Strict type SupplementalGroupsPolicy string const ( @@ -153,7 +153,7 @@ type Sysctl struct { // PodFSGroupChangePolicy holds policies that will be used for applying fsGroup to a volume // when volume is mounted. -// +enum +// +kubebuilder:validation:Enum=OnRootMismatch;Always type PodFSGroupChangePolicy string const ( @@ -168,6 +168,7 @@ const ( FSGroupChangeAlways PodFSGroupChangePolicy = "Always" ) +// +kubebuilder:validation:Enum // PodSELinuxChangePolicy defines how the container's SELinux label is applied to all volumes used by the Pod. type PodSELinuxChangePolicy string diff --git a/tests/integration/testdata/default_configurations/volume.go b/tests/integration/testdata/default_configurations/volume.go index 4e63238e..9f4bb4dd 100644 --- a/tests/integration/testdata/default_configurations/volume.go +++ b/tests/integration/testdata/default_configurations/volume.go @@ -50,7 +50,7 @@ type VolumeMount struct { } // MountPropagationMode describes mount propagation. -// +enum +// +kubebuilder:validation:Enum=None;HostToContainer;Bidirectional type MountPropagationMode string const ( @@ -75,6 +75,7 @@ const ( MountPropagationBidirectional MountPropagationMode = "Bidirectional" ) +// +kubebuilder:validation:Enum // RecursiveReadOnlyMode describes recursive-readonly mode. type RecursiveReadOnlyMode string @@ -460,7 +461,7 @@ type HostPathVolumeSource struct { Type *HostPathType `json:"type,omitempty" protobuf:"bytes,2,opt,name=type"` } -// +enum +// +kubebuilder:validation:Enum="";DirectoryOrCreate;Directory;FileOrCreate;File;Socket;CharDevice;BlockDevice type HostPathType string const ( @@ -762,6 +763,7 @@ type FlockerVolumeSource struct { } // StorageMedium defines ways that storage can be allocated to a volume. +// +kubebuilder:validation:Enum=Memory;HugePages;HugePages- type StorageMedium string const ( @@ -1194,10 +1196,10 @@ type PhotonPersistentDiskVolumeSource struct { FSType string `json:"fsType,omitempty" protobuf:"bytes,2,opt,name=fsType"` // want "optionalorrequired: field PhotonPersistentDiskVolumeSource.FSType must be marked as optional or required" } -// +enum +// +kubebuilder:validation:Enum=None;ReadOnly;ReadWrite type AzureDataDiskCachingMode string -// +enum +// +kubebuilder:validation:Enum=Shared;Dedicated;Managed type AzureDataDiskKind string const ( @@ -1872,7 +1874,7 @@ type PersistentVolumeClaimSpec struct { VolumeAttributesClassName *string `json:"volumeAttributesClassName,omitempty" protobuf:"bytes,9,opt,name=volumeAttributesClassName"` } -// +enum +// +kubebuilder:validation:Enum type PersistentVolumeAccessMode string const ( @@ -1888,7 +1890,7 @@ const ( ) // PersistentVolumeMode describes how a volume is intended to be consumed, either Block or Filesystem. -// +enum +// +kubebuilder:validation:Enum=Block;Filesystem type PersistentVolumeMode string const (