fix tests for feature gates

Author: wazery

docs/book/src/reference/markers/feature-gates.md

@@ -226,12 +226,28 @@ type CronJob struct {`+`
 	 */`)
 	hackutils.CheckError("fixing schema for cronjob_types.go", err)
 
-	// fix lint
+	// fix lint - handle the pattern with or without feature-gated fields
 	err = pluginutil.ReplaceInFile(
 		filepath.Join(sp.ctx.Dir, "api/v1/cronjob_types.go"),
 		`/
 	
+
+	// Example of a feature-gated field:
+	// Bar is an experimental field that requires the "experimental-bar" feature gate to be enabled
+	// TODO: When controller-tools supports feature gates (issue #1238), use:
+	// +kubebuilder:feature-gate=experimental-bar
+	// +feature-gate experimental-bar
+	// +optional
+	Bar *string `+"`"+`json:"bar,omitempty"`+"`"+`
+}`, "/")
+	if err != nil {
+		// Fallback to the original pattern if the feature-gated field is not present
+		err = pluginutil.ReplaceInFile(
+			filepath.Join(sp.ctx.Dir, "api/v1/cronjob_types.go"),
+			`/
+	
 }`, "/")
+	}
 	hackutils.CheckError("fixing cronjob_types.go end of status", err)
 }
 

hack/docs/internal/cronjob-tutorial/generate_cronjob.go

@@ -735,11 +735,25 @@ We'll leave our spec largely unchanged, except to change the schedule field to a
 
 	err = pluginutil.ReplaceInFile(
 		filepath.Join(sp.ctx.Dir, path),
-		`// foo is an example field of CronJob. Edit cronjob_types.go to remove/update
+		`// Example of a feature-gated field:
+	// Bar is an experimental field that requires the "experimental-bar" feature gate to be enabled
+	// TODO: When controller-tools supports feature gates (issue #1238), use:
+	// +kubebuilder:feature-gate=experimental-bar
+	// +feature-gate experimental-bar
 	// +optional
-	Foo *string `+"`json:\"foo,omitempty\"`",
+	Bar *string `+"`"+`json:"bar,omitempty"`+"`",
 		cronjobSpecMore,
 	)
+	if err != nil {
+		// Fallback to the original pattern if the feature-gated field is not present
+		err = pluginutil.ReplaceInFile(
+			filepath.Join(sp.ctx.Dir, path),
+			`// foo is an example field of CronJob. Edit cronjob_types.go to remove/update
+	// +optional
+	Foo *string `+"`json:\"foo,omitempty\"`",
+			cronjobSpecMore,
+		)
+	}
 	hackutils.CheckError("replace Foo with cronjob spec fields", err)
 
 	err = pluginutil.ReplaceInFile(

hack/docs/internal/multiversion-tutorial/generate_multiversion.go

@@ -18,9 +18,14 @@ package machinery
 
 import (
 	"fmt"
+	"regexp"
 	"strings"
 )
 
+// featureGateNameRegex validates feature gate names
+// Names should follow Kubernetes conventions: start with lowercase letter, followed by lowercase alphanumeric with hyphens
+var featureGateNameRegex = regexp.MustCompile(`^[a-z][a-z0-9]*(-[a-z0-9]+)*$`)
+
 // FeatureGate represents a feature gate with its name and enabled state
 type FeatureGate struct {
 	Name    string
@@ -30,9 +35,23 @@ type FeatureGate struct {
 // FeatureGates represents a collection of feature gates
 type FeatureGates map[string]bool
 
+// validateFeatureGateName validates that a feature gate name follows conventions
+func validateFeatureGateName(name string) error {
+	if name == "" {
+		return fmt.Errorf("empty feature gate name")
+	}
+
+	if !featureGateNameRegex.MatchString(name) {
+		return fmt.Errorf("invalid feature gate name '%s': must be lowercase alphanumeric with hyphens (e.g., 'experimental-feature')", name)
+	}
+
+	return nil
+}
+
 // ParseFeatureGates parses a comma-separated string of feature gates
-// Format: "feature1=true,feature2=false,feature3"
-// If no value is specified, the feature gate defaults to enabled
+// Format: "feature1=true,feature2=false,feature3=true"
+// Feature gate names must be lowercase alphanumeric with hyphens
+// Values must be 'true' or 'false' (case insensitive)
 func ParseFeatureGates(featureGates string) (FeatureGates, error) {
 	if featureGates == "" {
 		return make(FeatureGates), nil
@@ -41,13 +60,10 @@ func ParseFeatureGates(featureGates string) (FeatureGates, error) {
 	gates := make(FeatureGates)
 	parts := strings.Split(featureGates, ",")
 
-	for i, part := range parts {
+	for _, part := range parts {
 		part = strings.TrimSpace(part)
 		if part == "" {
-			// If this is the first part and it's empty, it's an error
-			if i == 0 {
-				return nil, fmt.Errorf("empty feature gate name")
-			}
+			// Skip empty parts (e.g., trailing commas)
 			continue
 		}
 
@@ -63,20 +79,26 @@ func ParseFeatureGates(featureGates string) (FeatureGates, error) {
 			name = strings.TrimSpace(kv[0])
 			value := strings.TrimSpace(kv[1])
 
+			if value == "" {
+				return nil, fmt.Errorf("empty value for feature gate '%s'", name)
+			}
+
 			switch strings.ToLower(value) {
-			case "true", "1", "yes", "on":
+			case "true":
 				enabled = true
-			case "false", "0", "no", "off":
+			case "false":
 				enabled = false
 			default:
-				return nil, fmt.Errorf("invalid feature gate value for %s: %s", name, value)
+				return nil, fmt.Errorf("invalid feature gate value for %s: %s (must be 'true' or 'false')", name, value)
 			}
 		} else {
+			// No '=' found, treat entire part as name with default value true
 			name = part
 		}
 
-		if name == "" {
-			return nil, fmt.Errorf("empty feature gate name")
+		// Validate the feature gate name
+		if err := validateFeatureGateName(name); err != nil {
+			return nil, err
 		}
 
 		gates[name] = enabled

pkg/machinery/featuregate.go

@@ -20,6 +20,7 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"os"
 	"path/filepath"
 
 	"github.com/spf13/afero"
@@ -97,9 +98,18 @@ func (s *apiScaffolder) Scaffold() error {
 		return fmt.Errorf("error updating resource: %w", err)
 	}
 
+	// If using --force, discover existing feature gates before overwriting files
+	var existingGates []string
+	if s.force && doAPI {
+		existingGates = s.discoverFeatureGates()
+	}
+
 	if doAPI {
 		if err := scaffold.Execute(
-			&api.Types{Force: s.force},
+			&api.Types{
+				Force:                     s.force,
+				IncludeFeatureGateExample: false, // Don't include by default - keep it simple
+			},
 			&api.Group{},
 		); err != nil {
 			return fmt.Errorf("error scaffolding APIs: %w", err)
@@ -116,13 +126,40 @@ func (s *apiScaffolder) Scaffold() error {
 		}
 	}
 
-	// Discover feature gates from API types
-	availableGates := s.discoverFeatureGates()
+	// Discover feature gates from newly scaffolded API types
+	newGates := s.discoverFeatureGates()
+	var availableGates []string
+
+	// Merge existing gates with newly discovered ones if we used --force
+	if len(existingGates) > 0 {
+		gateMap := make(map[string]bool)
+
+		// Add existing gates
+		for _, gate := range existingGates {
+			gateMap[gate] = true
+		}
+
+		// Add newly discovered gates (from template)
+		for _, gate := range newGates {
+			gateMap[gate] = true
+		}
+
+		// Create merged list
+		var mergedGates []string
+		for gate := range gateMap {
+			mergedGates = append(mergedGates, gate)
+		}
+
+		availableGates = mergedGates
+	} else {
+		availableGates = newGates
+	}
 
 	// Generate feature gates file
-	if err := scaffold.Execute(
-		&cmd.FeatureGates{AvailableGates: availableGates},
-	); err != nil {
+	featureGatesTemplate := &cmd.FeatureGates{}
+	featureGatesTemplate.AvailableGates = availableGates
+	featureGatesTemplate.IfExistsAction = machinery.OverwriteFile
+	if err := scaffold.Execute(featureGatesTemplate); err != nil {
 		return fmt.Errorf("error scaffolding feature gates: %w", err)
 	}
 
@@ -137,29 +174,62 @@ func (s *apiScaffolder) Scaffold() error {
 
 // discoverFeatureGates scans the API directory for feature gate markers
 func (s *apiScaffolder) discoverFeatureGates() []string {
-	parser := machinery.NewFeatureGateMarkerParser()
+	mgParser := machinery.NewFeatureGateMarkerParser()
 
 	// Try to parse the API directory
 	apiDir := "api"
 	if s.config.IsMultiGroup() && s.resource.Group != "" {
 		apiDir = filepath.Join("api", s.resource.Group)
 	}
 
+	// Debug: Get current working directory
+	if wd, err := os.Getwd(); err == nil {
+		slog.Debug("Feature gate discovery", "workingDir", wd, "apiDir", apiDir)
+	}
+
 	// Check if the directory exists before trying to parse it
-	if _, err := s.fs.FS.Stat(apiDir); err != nil {
-		slog.Debug("API directory does not exist yet, skipping feature gate discovery", "apiDir", apiDir)
+	if _, err := os.Stat(apiDir); err != nil {
+		slog.Debug("API directory does not exist yet, skipping feature gate discovery", "apiDir", apiDir, "error", err)
 		return []string{}
 	}
 
-	markers, err := parser.ParseDirectory(apiDir)
+	var allMarkers []machinery.FeatureGateMarker
+
+	// Walk through each version directory and parse for feature gates
+	entries, err := os.ReadDir(apiDir)
 	if err != nil {
-		slog.Debug("Failed to parse feature gates from directory", "apiDir", apiDir, "error", err)
+		slog.Debug("Error reading API directory", "error", err, "apiDir", apiDir)
 		return []string{}
 	}
 
-	featureGates := machinery.ExtractFeatureGates(markers)
+	slog.Debug("API directory contents", "apiDir", apiDir, "fileCount", len(entries))
+
+	for _, entry := range entries {
+		slog.Debug("API directory file", "name", entry.Name(), "isDir", entry.IsDir())
+		if entry.IsDir() {
+			versionDir := filepath.Join(apiDir, entry.Name())
+
+			// Use the existing parser to parse the version directory
+			markers, err := mgParser.ParseDirectory(versionDir)
+			if err != nil {
+				slog.Debug("Error parsing version directory for feature gates", "error", err, "versionDir", versionDir)
+				continue
+			}
+
+			slog.Debug("Parsed markers from directory", "versionDir", versionDir, "markerCount", len(markers))
+
+			// Debug: Print all discovered markers
+			for _, marker := range markers {
+				slog.Debug("Found feature gate marker", "gateName", marker.GateName, "line", marker.Line, "file", marker.File)
+			}
+
+			allMarkers = append(allMarkers, markers...)
+		}
+	}
+
+	featureGates := machinery.ExtractFeatureGates(allMarkers)
 	if len(featureGates) > 0 {
-		slog.Info("Discovered feature gates", "featureGates", featureGates)
+		slog.Debug("Discovered feature gates", "featureGates", featureGates)
 	} else {
 		slog.Debug("No feature gates found in directory", "apiDir", apiDir)
 	}

pkg/plugins/golang/v4/scaffolds/api.go

@@ -185,7 +185,23 @@ func TestAPIScaffolder_discoverFeatureGates_Testdata(t *testing.T) {
 	apiScaffolder.fs = machinery.Filesystem{FS: afero.NewOsFs()}
 
 	featureGates := apiScaffolder.discoverFeatureGates()
-	if len(featureGates) > 0 {
-		t.Errorf("Expected no feature gates from testdata, but found %d: %v", len(featureGates), featureGates)
+
+	// The testdata contains a feature gate marker for "experimental-bar"
+	expectedGates := []string{"experimental-bar"}
+	if len(featureGates) != len(expectedGates) {
+		t.Errorf("Expected %d feature gates from testdata, but found %d: %v", len(expectedGates), len(featureGates), featureGates)
+	}
+
+	for _, expectedGate := range expectedGates {
+		found := false
+		for _, gate := range featureGates {
+			if gate == expectedGate {
+				found = true
+				break
+			}
+		}
+		if !found {
+			t.Errorf("Expected to find feature gate %s, but it was not discovered", expectedGate)
+		}
 	}
 }

pkg/plugins/golang/v4/scaffolds/api_test.go

@@ -35,6 +35,8 @@ type Types struct {
 	machinery.ResourceMixin
 
 	Force bool
+	// IncludeFeatureGateExample determines whether to include feature gate example in the scaffolded API
+	IncludeFeatureGateExample bool
 }
 
 // SetTemplateDefaults implements machinery.Template
@@ -83,14 +85,15 @@ type {{ .Resource.Kind }}Spec struct {
 	// foo is an example field of {{ .Resource.Kind }}. Edit {{ lower .Resource.Kind }}_types.go to remove/update
 	// +optional	
 	Foo *string ` + "`" + `json:"foo,omitempty"` + "`" + `
+{{- if .IncludeFeatureGateExample }}
 
 	// Example of a feature-gated field:
 	// Bar is an experimental field that requires the "experimental-bar" feature gate to be enabled
-	// TODO: When controller-tools supports feature gates (issue #1238), use:
 	// +kubebuilder:feature-gate=experimental-bar
 	// +feature-gate experimental-bar
 	// +optional
-	// Bar *string ` + "`" + `json:"bar,omitempty"` + "`" + `
+	Bar *string ` + "`" + `json:"bar,omitempty"` + "`" + `
+{{- end }}
 }
 
 // {{ .Resource.Kind }}Status defines the observed state of {{ .Resource.Kind }}.

pkg/plugins/golang/v4/scaffolds/internal/templates/api/types.go

@@ -293,6 +293,7 @@ func main() {
 	flag.BoolVar(&enableHTTP2, "enable-http2", false,
 		"If set, HTTP/2 will be enabled for the metrics and webhook servers")
 	flag.StringVar(&featureGates, "feature-gates", "", "A set of key=value pairs that describe feature gates for alpha/experimental features. " +
+		"Example: --feature-gates \"gate1=true,gate2=false\". " +
 		"Options are: "+featuregates.GetFeatureGatesHelpText())
 	opts := zap.Options{
 		Development: true,

pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/main.go

@@ -119,8 +119,6 @@ help: ## Display this help.
 .PHONY: manifests
 manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects.
 	$(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
-	# TODO: When controller-tools supports feature gates (issue #1238), add:
-	# $(CONTROLLER_GEN) --feature-gates=$(FEATURE_GATES) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
 
 .PHONY: generate
 generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.