From 7e6038685a78fabd8787d809e65a5fe43c5b987f Mon Sep 17 00:00:00 2001 From: wazery Date: Mon, 28 Jul 2025 21:00:11 +0200 Subject: [PATCH 01/11] feat(feature-gates) implement feature gates for #849 --- FEATURE_GATES.md | 191 ++++++++++++++ go.mod | 9 + pkg/machinery/featuregate.go | 131 ++++++++++ pkg/machinery/featuregate_marker.go | 239 ++++++++++++++++++ pkg/machinery/featuregate_marker_test.go | 210 +++++++++++++++ pkg/machinery/featuregate_test.go | 166 ++++++++++++ pkg/plugins/golang/v4/scaffolds/api.go | 42 +++ pkg/plugins/golang/v4/scaffolds/api_test.go | 183 ++++++++++++++ pkg/plugins/golang/v4/scaffolds/init.go | 1 + .../scaffolds/internal/templates/api/types.go | 8 + .../internal/templates/cmd/featuregates.go | 177 +++++++++++++ .../scaffolds/internal/templates/cmd/main.go | 22 +- .../scaffolds/internal/templates/makefile.go | 2 + 13 files changed, 1379 insertions(+), 2 deletions(-) create mode 100644 FEATURE_GATES.md create mode 100644 pkg/machinery/featuregate.go create mode 100644 pkg/machinery/featuregate_marker.go create mode 100644 pkg/machinery/featuregate_marker_test.go create mode 100644 pkg/machinery/featuregate_test.go create mode 100644 pkg/plugins/golang/v4/scaffolds/api_test.go create mode 100644 pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/featuregates.go diff --git a/FEATURE_GATES.md b/FEATURE_GATES.md new file mode 100644 index 00000000000..0ded08f73df --- /dev/null +++ b/FEATURE_GATES.md @@ -0,0 +1,191 @@ +# Feature Gates for Kubebuilder + +This document describes the feature gates functionality implemented in Kubebuilder, which allows developers to mark fields in their API types as belonging to specific feature gates. + +## Overview + +Feature gates provide a way to enable or disable experimental features in your CRDs, similar to how Kubernetes core types handle feature gates. This allows for: + +- Gradual rollout of new features +- A/B testing of experimental functionality +- Safe deprecation of features +- Better control over API stability + +## Usage + +### 1. Marking Fields with Feature Gates + +In your API types, you can mark fields with feature gate annotations: + +```go +// MyResourceSpec defines the desired state of MyResource +type MyResourceSpec struct { + // Standard field + Name string `json:"name"` + + // Experimental field that requires the "experimental-feature" gate + // +feature-gate experimental-feature + ExperimentalField string `json:"experimentalField,omitempty"` + + // Another experimental field + // +feature-gate another-feature + AnotherField int `json:"anotherField,omitempty"` +} +``` + +### 2. Running with Feature Gates + +When you run your controller, you can enable or disable feature gates: + +```bash +# Enable all feature gates +./manager --feature-gates=experimental-feature,another-feature + +# Enable some gates and disable others +./manager --feature-gates=experimental-feature=true,another-feature=false + +# Disable a specific gate +./manager --feature-gates=experimental-feature=false +``` + +### 3. Feature Gate Discovery + +Kubebuilder automatically discovers feature gates from your API types and generates validation code. The available feature gates are listed in the help text: + +```bash +./manager --help +``` + +You'll see output like: +``` +--feature-gates string A set of key=value pairs that describe feature gates for alpha/experimental features. Options are: experimental-feature, another-feature +``` + +## Implementation Details + +### Feature Gate Parsing + +The feature gate string follows this format: +- `feature1` - enables feature1 (default) +- `feature1=true` - explicitly enables feature1 +- `feature1=false` - explicitly disables feature1 +- `feature1,feature2=false,feature3` - mixed enabled/disabled gates + +### Marker Parsing + +Kubebuilder scans your API types for markers in the format: +``` +// +feature-gate gate-name +``` + +These markers are found in: +- Struct field comments +- Function comments +- Type comments + +### Validation + +The controller validates that: +1. All specified feature gates are valid (exist in the codebase) +2. Feature gate values are properly formatted +3. No duplicate or conflicting gate specifications + +## Examples + +### Basic Example + +```go +// api/v1/myresource_types.go +type MyResourceSpec struct { + // Standard field + Name string `json:"name"` + + // Experimental field + // +feature-gate alpha-feature + AlphaField string `json:"alphaField,omitempty"` +} +``` + +### Advanced Example + +```go +// api/v1/complexresource_types.go +type ComplexResourceSpec struct { + // Standard fields + Name string `json:"name"` + Replicas int32 `json:"replicas"` + + // Beta feature + // +feature-gate beta-feature + BetaField string `json:"betaField,omitempty"` + + // Alpha feature + // +feature-gate alpha-feature + AlphaField string `json:"alphaField,omitempty"` + + // Experimental feature + // +feature-gate experimental-feature + ExperimentalField string `json:"experimentalField,omitempty"` +} +``` + +Running with different configurations: + +```bash +# Enable all features +./manager --feature-gates=alpha-feature,beta-feature,experimental-feature + +# Enable only beta features +./manager --feature-gates=beta-feature + +# Disable experimental features +./manager --feature-gates=alpha-feature,beta-feature,experimental-feature=false +``` + +## Best Practices + +1. **Use descriptive gate names**: Choose names that clearly indicate what the feature does +2. **Document your gates**: Add comments explaining what each feature gate enables +3. **Gradual rollout**: Start with alpha features, then beta, then stable +4. **Consistent naming**: Use kebab-case for feature gate names +5. **Validation**: Always validate feature gate inputs in your controllers + +## Migration Guide + +### From No Feature Gates + +1. Add feature gate markers to your experimental fields +2. Rebuild your project: `make manifests` +3. Update your deployment to include the `--feature-gates` flag +4. Test with different gate combinations + +### To Stable Features + +When a feature is ready for stable release: +1. Remove the feature gate marker +2. Update documentation +3. Consider the field stable and always available + +## Troubleshooting + +### Common Issues + +1. **Unknown feature gate**: Make sure the gate name matches exactly in your code +2. **Invalid format**: Check that your feature gate string follows the correct format +3. **Missing validation**: Ensure your controller validates feature gates before use + +### Debugging + +Enable verbose logging to see feature gate processing: + +```bash +./manager --feature-gates=your-gate --v=2 +``` + +## Future Enhancements + +Planned improvements include: +- CRD schema modification based on enabled gates +- Automatic documentation generation +- Integration with controller-runtime feature gates +- Webhook validation based on feature gates \ No newline at end of file diff --git a/go.mod b/go.mod index 18274ccc4f4..99403ba2862 100644 --- a/go.mod +++ b/go.mod @@ -10,15 +10,23 @@ require ( github.com/spf13/afero v1.14.0 github.com/spf13/cobra v1.9.1 github.com/spf13/pflag v1.0.7 +<<<<<<< HEAD golang.org/x/mod v0.27.0 golang.org/x/text v0.28.0 golang.org/x/tools v0.36.0 +======= + github.com/stretchr/testify v1.10.0 + golang.org/x/mod v0.26.0 + golang.org/x/text v0.27.0 + golang.org/x/tools v0.35.0 +>>>>>>> ba64bd710 (feat(feature-gates) implement feature gates for #849) helm.sh/helm/v3 v3.18.4 sigs.k8s.io/yaml v1.6.0 ) require ( github.com/Masterminds/semver/v3 v3.3.0 // indirect + github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/go-logr/logr v1.4.2 // indirect github.com/go-task/slim-sprig/v3 v3.0.0 // indirect github.com/google/go-cmp v0.7.0 // indirect @@ -27,6 +35,7 @@ require ( github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/kr/pretty v0.3.1 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/rogpeppe/go-internal v1.14.1 // indirect go.uber.org/automaxprocs v1.6.0 // indirect go.yaml.in/yaml/v2 v2.4.2 // indirect diff --git a/pkg/machinery/featuregate.go b/pkg/machinery/featuregate.go new file mode 100644 index 00000000000..b8468c5c1fa --- /dev/null +++ b/pkg/machinery/featuregate.go @@ -0,0 +1,131 @@ +/* +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 machinery + +import ( + "fmt" + "strings" +) + +// FeatureGate represents a feature gate with its name and enabled state +type FeatureGate struct { + Name string + Enabled bool +} + +// FeatureGates represents a collection of feature gates +type FeatureGates map[string]bool + +// 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 +func ParseFeatureGates(featureGates string) (FeatureGates, error) { + if featureGates == "" { + return make(FeatureGates), nil + } + + gates := make(FeatureGates) + parts := strings.Split(featureGates, ",") + + for i, 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") + } + continue + } + + // Parse the feature gate name and value + var name string + var enabled bool = true // Default to enabled + + if strings.Contains(part, "=") { + kv := strings.SplitN(part, "=", 2) + if len(kv) != 2 { + return nil, fmt.Errorf("invalid feature gate format: %s", part) + } + name = strings.TrimSpace(kv[0]) + value := strings.TrimSpace(kv[1]) + + switch strings.ToLower(value) { + case "true", "1", "yes", "on": + enabled = true + case "false", "0", "no", "off": + enabled = false + default: + return nil, fmt.Errorf("invalid feature gate value for %s: %s", name, value) + } + } else { + name = part + } + + if name == "" { + return nil, fmt.Errorf("empty feature gate name") + } + + gates[name] = enabled + } + + return gates, nil +} + +// IsEnabled checks if a specific feature gate is enabled +func (fg FeatureGates) IsEnabled(name string) bool { + enabled, exists := fg[name] + return exists && enabled +} + +// GetEnabledGates returns a list of enabled feature gate names +func (fg FeatureGates) GetEnabledGates() []string { + var enabled []string + for name, isEnabled := range fg { + if isEnabled { + enabled = append(enabled, name) + } + } + return enabled +} + +// GetDisabledGates returns a list of disabled feature gate names +func (fg FeatureGates) GetDisabledGates() []string { + var disabled []string + for name, enabled := range fg { + if !enabled { + disabled = append(disabled, name) + } + } + return disabled +} + +// String returns a string representation of the feature gates +func (fg FeatureGates) String() string { + if len(fg) == 0 { + return "" + } + + var parts []string + for name, enabled := range fg { + if enabled { + parts = append(parts, name) + } else { + parts = append(parts, fmt.Sprintf("%s=false", name)) + } + } + return strings.Join(parts, ",") +} diff --git a/pkg/machinery/featuregate_marker.go b/pkg/machinery/featuregate_marker.go new file mode 100644 index 00000000000..1c8cdba2122 --- /dev/null +++ b/pkg/machinery/featuregate_marker.go @@ -0,0 +1,239 @@ +/* +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 machinery + +import ( + "bufio" + "fmt" + "go/ast" + "go/parser" + "go/token" + "os" + "regexp" + "strings" +) + +// FeatureGateMarker represents a feature gate marker found in source code +type FeatureGateMarker struct { + GateName string + Line int + File string +} + +// FeatureGateMarkerParser parses Go source files for feature gate markers +type FeatureGateMarkerParser struct { + // Regex to match feature gate markers + // Matches: // +feature-gate gate-name + markerRegex *regexp.Regexp +} + +// NewFeatureGateMarkerParser creates a new feature gate marker parser +func NewFeatureGateMarkerParser() *FeatureGateMarkerParser { + return &FeatureGateMarkerParser{ + markerRegex: regexp.MustCompile(`^\s*//\s*\+feature-gate\s+([a-zA-Z0-9_-]+)\s*$`), + } +} + +// ParseFile parses a Go source file for feature gate markers +func (p *FeatureGateMarkerParser) ParseFile(filePath string) ([]FeatureGateMarker, error) { + file, err := os.Open(filePath) + if err != nil { + return nil, fmt.Errorf("failed to open file %s: %w", filePath, err) + } + defer file.Close() + + var markers []FeatureGateMarker + scanner := bufio.NewScanner(file) + lineNum := 0 + var lines []string + + // First pass: collect all lines + for scanner.Scan() { + lines = append(lines, scanner.Text()) + } + + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("error reading file %s: %w", filePath, err) + } + + // Second pass: analyze lines to find feature gate markers associated with real code + for i, line := range lines { + lineNum = i + 1 + + matches := p.markerRegex.FindStringSubmatch(line) + if len(matches) == 2 { + // Check if this marker is associated with actual code (not just comments) + if p.isMarkerAssociatedWithCode(lines, i) { + markers = append(markers, FeatureGateMarker{ + GateName: matches[1], + Line: lineNum, + File: filePath, + }) + } + } + } + + return markers, nil +} + +// isMarkerAssociatedWithCode checks if a feature gate marker is associated with actual code +// rather than just commented code +func (p *FeatureGateMarkerParser) isMarkerAssociatedWithCode(lines []string, markerLineIndex int) bool { + // Look ahead for actual code (not comments) within the next few lines + // Feature gate markers should be followed by actual field declarations + for i := markerLineIndex + 1; i < len(lines) && i <= markerLineIndex+5; i++ { + line := lines[i] + trimmed := strings.TrimSpace(line) + + // Skip empty lines + if trimmed == "" { + continue + } + + // If we hit another marker, stop looking + if strings.Contains(trimmed, "+feature-gate") { + break + } + + // If we hit end of struct, stop looking + if strings.Contains(trimmed, "}") { + break + } + + // Skip pure comment lines + if strings.HasPrefix(trimmed, "//") { + continue + } + + // Check if this line contains actual Go code (field declaration, struct, etc.) + // Look for patterns like: FieldName Type `json:"fieldName,omitempty"` + if strings.Contains(trimmed, "`json:") || + strings.Contains(trimmed, "string") || + strings.Contains(trimmed, "int") || + strings.Contains(trimmed, "bool") || + strings.Contains(trimmed, "*") { + return true + } + } + + return false +} + +// ParseDirectory parses all Go files in a directory for feature gate markers +func (p *FeatureGateMarkerParser) ParseDirectory(dirPath string) ([]FeatureGateMarker, error) { + fset := token.NewFileSet() + pkgs, err := parser.ParseDir(fset, dirPath, nil, parser.ParseComments) + if err != nil { + return nil, fmt.Errorf("failed to parse directory %s: %w", dirPath, err) + } + + var allMarkers []FeatureGateMarker + + for _, pkg := range pkgs { + for fileName, file := range pkg.Files { + markers := p.parseASTFile(fileName, file, fset) + allMarkers = append(allMarkers, markers...) + } + } + + return allMarkers, nil +} + +// parseASTFile parses an AST file for feature gate markers +func (p *FeatureGateMarkerParser) parseASTFile(fileName string, file *ast.File, fset *token.FileSet) []FeatureGateMarker { + var markers []FeatureGateMarker + + // Parse comments for feature gate markers + for _, commentGroup := range file.Comments { + for _, comment := range commentGroup.List { + matches := p.markerRegex.FindStringSubmatch(comment.Text) + if len(matches) == 2 { + // Check if this marker is associated with actual code (not just comments) + if p.isASTMarkerAssociatedWithCode(file, fset, comment) { + markers = append(markers, FeatureGateMarker{ + GateName: matches[1], + Line: fset.Position(comment.Pos()).Line, + File: fileName, + }) + } + } + } + } + + return markers +} + +// isASTMarkerAssociatedWithCode checks if a feature gate marker in AST is associated with actual code +// rather than just commented code +func (p *FeatureGateMarkerParser) isASTMarkerAssociatedWithCode(file *ast.File, fset *token.FileSet, comment *ast.Comment) bool { + commentPos := fset.Position(comment.Pos()) + + // Look for AST nodes that come after this comment + for _, decl := range file.Decls { + declPos := fset.Position(decl.Pos()) + + // Look for field declarations within structs + if genDecl, ok := decl.(*ast.GenDecl); ok { + for _, spec := range genDecl.Specs { + if typeSpec, ok := spec.(*ast.TypeSpec); ok { + if structType, ok := typeSpec.Type.(*ast.StructType); ok { + for _, field := range structType.Fields.List { + fieldPos := fset.Position(field.Pos()) + // Check if this field is within a reasonable distance from the comment (before or after) + if fieldPos.Line >= commentPos.Line-2 && fieldPos.Line <= commentPos.Line+5 { + // This field is close to the comment, so the marker is likely associated with it + return true + } + } + } + } + } + } + + // If we've looked too far ahead, stop + if declPos.Line > commentPos.Line+10 { + break + } + } + + return false +} + +// ExtractFeatureGates extracts all unique feature gate names from markers +func ExtractFeatureGates(markers []FeatureGateMarker) []string { + gateMap := make(map[string]bool) + for _, marker := range markers { + gateMap[marker.GateName] = true + } + + var gates []string + for gate := range gateMap { + gates = append(gates, gate) + } + return gates +} + +// ValidateFeatureGates validates that all required feature gates are enabled +func ValidateFeatureGates(requiredGates []string, enabledGates FeatureGates) []string { + var missing []string + for _, gate := range requiredGates { + if !enabledGates.IsEnabled(gate) { + missing = append(missing, gate) + } + } + return missing +} diff --git a/pkg/machinery/featuregate_marker_test.go b/pkg/machinery/featuregate_marker_test.go new file mode 100644 index 00000000000..4903c16398b --- /dev/null +++ b/pkg/machinery/featuregate_marker_test.go @@ -0,0 +1,210 @@ +/* +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 machinery + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestFeatureGateMarkerParser_ParseFile(t *testing.T) { + parser := NewFeatureGateMarkerParser() + + // Create a temporary test file + tempDir := t.TempDir() + testFile := filepath.Join(tempDir, "test.go") + + content := `package test + +import "fmt" + +// MyStruct represents a test struct +type MyStruct struct { + // Regular field + Name string ` + "`" + `json:"name"` + "`" + ` + + // Feature gated field + // +feature-gate experimental-feature + ExperimentalField string ` + "`" + `json:"experimentalField,omitempty"` + "`" + ` + + // Another feature gated field + // +feature-gate another-feature + AnotherField int ` + "`" + `json:"anotherField,omitempty"` + "`" + ` + + // Regular comment + // This is not a feature gate marker + RegularField bool ` + "`" + `json:"regularField"` + "`" + ` +} + +// +feature-gate function-feature +func (m *MyStruct) ExperimentalMethod() { + fmt.Println("This is experimental") +} +` + + err := os.WriteFile(testFile, []byte(content), 0644) + assert.NoError(t, err) + + markers, err := parser.ParseFile(testFile) + assert.NoError(t, err) + assert.Len(t, markers, 3) + + // Check that we found the expected feature gates + expectedGates := map[string]bool{ + "experimental-feature": false, + "another-feature": false, + "function-feature": false, + } + + for _, marker := range markers { + assert.Contains(t, expectedGates, marker.GateName) + assert.Equal(t, testFile, marker.File) + assert.Greater(t, marker.Line, 0) + } +} + +func TestExtractFeatureGates(t *testing.T) { + markers := []FeatureGateMarker{ + {GateName: "feature1", Line: 1, File: "test1.go"}, + {GateName: "feature2", Line: 2, File: "test1.go"}, + {GateName: "feature1", Line: 3, File: "test2.go"}, // Duplicate + {GateName: "feature3", Line: 4, File: "test2.go"}, + } + + gates := ExtractFeatureGates(markers) + assert.ElementsMatch(t, []string{"feature1", "feature2", "feature3"}, gates) +} + +func TestValidateFeatureGates(t *testing.T) { + requiredGates := []string{"feature1", "feature2", "feature3"} + + // All required gates enabled + enabledGates := FeatureGates{ + "feature1": true, + "feature2": true, + "feature3": true, + } + + missing := ValidateFeatureGates(requiredGates, enabledGates) + assert.Empty(t, missing) + + // Some gates disabled + enabledGates = FeatureGates{ + "feature1": true, + "feature2": false, + "feature3": true, + } + + missing = ValidateFeatureGates(requiredGates, enabledGates) + assert.ElementsMatch(t, []string{"feature2"}, missing) + + // Missing gates + enabledGates = FeatureGates{ + "feature1": true, + // feature2 missing + "feature3": true, + } + + missing = ValidateFeatureGates(requiredGates, enabledGates) + assert.ElementsMatch(t, []string{"feature2"}, missing) +} + +func TestFeatureGateMarkerParser_CommentedMarkers(t *testing.T) { + parser := NewFeatureGateMarkerParser() + + // Create a temporary file with commented feature gate markers + tempDir := t.TempDir() + testFile := filepath.Join(tempDir, "test.go") + + content := `package test + +// 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"` + "`" + ` + +type TestSpec struct { + // foo is an example field + // +optional + Foo *string ` + "`" + `json:"foo,omitempty"` + "`" + ` +} +` + + err := os.WriteFile(testFile, []byte(content), 0644) + if err != nil { + t.Fatalf("Failed to write test file: %v", err) + } + + // Parse the directory + markers, err := parser.ParseDirectory(tempDir) + if err != nil { + t.Fatalf("Failed to parse directory: %v", err) + } + + // Should not find any feature gate markers since they're commented out + if len(markers) > 0 { + t.Errorf("Expected no feature gate markers, but found %d: %v", len(markers), markers) + } +} + +func TestFeatureGateMarkerParser_ActiveMarkers(t *testing.T) { + parser := NewFeatureGateMarkerParser() + + // Create a temporary file with active feature gate markers + tempDir := t.TempDir() + testFile := filepath.Join(tempDir, "test.go") + + content := `package test + +type TestSpec struct { + // foo is an example field + // +optional + Foo *string ` + "`" + `json:"foo,omitempty"` + "`" + ` + + // Bar is an experimental field that requires the "experimental-bar" feature gate to be enabled + // +feature-gate experimental-bar + // +optional + Bar *string ` + "`" + `json:"bar,omitempty"` + "`" + ` +} +` + + err := os.WriteFile(testFile, []byte(content), 0644) + if err != nil { + t.Fatalf("Failed to write test file: %v", err) + } + + // Parse the directory + markers, err := parser.ParseDirectory(tempDir) + if err != nil { + t.Fatalf("Failed to parse directory: %v", err) + } + + // Should find the active feature gate marker + if len(markers) != 1 { + t.Errorf("Expected 1 feature gate marker, but found %d: %v", len(markers), markers) + } + + if markers[0].GateName != "experimental-bar" { + t.Errorf("Expected gate name 'experimental-bar', but got '%s'", markers[0].GateName) + } +} diff --git a/pkg/machinery/featuregate_test.go b/pkg/machinery/featuregate_test.go new file mode 100644 index 00000000000..d7be0a4f6ed --- /dev/null +++ b/pkg/machinery/featuregate_test.go @@ -0,0 +1,166 @@ +/* +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 machinery + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestParseFeatureGates(t *testing.T) { + tests := []struct { + name string + input string + expected FeatureGates + expectError bool + }{ + { + name: "empty string", + input: "", + expected: FeatureGates{}, + }, + { + name: "single feature gate", + input: "feature1", + expected: FeatureGates{"feature1": true}, + }, + { + name: "single feature gate with explicit true", + input: "feature1=true", + expected: FeatureGates{"feature1": true}, + }, + { + name: "single feature gate with explicit false", + input: "feature1=false", + expected: FeatureGates{"feature1": false}, + }, + { + name: "multiple feature gates", + input: "feature1,feature2", + expected: FeatureGates{"feature1": true, "feature2": true}, + }, + { + name: "mixed enabled and disabled", + input: "feature1=true,feature2=false,feature3", + expected: FeatureGates{"feature1": true, "feature2": false, "feature3": true}, + }, + { + name: "with whitespace", + input: " feature1 , feature2=false , feature3 ", + expected: FeatureGates{"feature1": true, "feature2": false, "feature3": true}, + }, + { + name: "invalid format", + input: "feature1=invalid", + expectError: true, + }, + { + name: "empty feature name", + input: "=true", + expectError: true, + }, + { + name: "empty feature name with comma", + input: ",feature1", + expectError: true, + }, + { + name: "empty feature name with comma and space", + input: " ,feature1", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := ParseFeatureGates(tt.input) + if tt.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.expected, result) + } + }) + } +} + +func TestFeatureGates_IsEnabled(t *testing.T) { + gates := FeatureGates{ + "enabled": true, + "disabled": false, + } + + assert.True(t, gates.IsEnabled("enabled")) + assert.False(t, gates.IsEnabled("disabled")) + assert.False(t, gates.IsEnabled("nonexistent")) +} + +func TestFeatureGates_GetEnabledGates(t *testing.T) { + gates := FeatureGates{ + "enabled1": true, + "enabled2": true, + "disabled1": false, + "disabled2": false, + } + + enabled := gates.GetEnabledGates() + assert.ElementsMatch(t, []string{"enabled1", "enabled2"}, enabled) +} + +func TestFeatureGates_GetDisabledGates(t *testing.T) { + gates := FeatureGates{ + "enabled1": true, + "enabled2": true, + "disabled1": false, + "disabled2": false, + } + + disabled := gates.GetDisabledGates() + assert.ElementsMatch(t, []string{"disabled1", "disabled2"}, disabled) +} + +func TestFeatureGates_String(t *testing.T) { + tests := []struct { + name string + gates FeatureGates + expected string + }{ + { + name: "empty gates", + gates: FeatureGates{}, + expected: "", + }, + { + name: "only enabled gates", + gates: FeatureGates{"feature1": true, "feature2": true}, + expected: "feature1,feature2", + }, + { + name: "mixed gates", + gates: FeatureGates{"feature1": true, "feature2": false}, + expected: "feature1,feature2=false", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.gates.String() + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/pkg/plugins/golang/v4/scaffolds/api.go b/pkg/plugins/golang/v4/scaffolds/api.go index aa8c708417a..863738e92cd 100644 --- a/pkg/plugins/golang/v4/scaffolds/api.go +++ b/pkg/plugins/golang/v4/scaffolds/api.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" log "log/slog" + "path/filepath" "github.com/spf13/afero" @@ -115,6 +116,16 @@ func (s *apiScaffolder) Scaffold() error { } } + // Discover feature gates from API types + availableGates := s.discoverFeatureGates() + + // Generate feature gates file + if err := scaffold.Execute( + &cmd.FeatureGates{AvailableGates: availableGates}, + ); err != nil { + return fmt.Errorf("error scaffolding feature gates: %w", err) + } + if err := scaffold.Execute( &cmd.MainUpdater{WireResource: doAPI, WireController: doController}, ); err != nil { @@ -123,3 +134,34 @@ func (s *apiScaffolder) Scaffold() error { return nil } + +// discoverFeatureGates scans the API directory for feature gate markers +func (s *apiScaffolder) discoverFeatureGates() []string { + parser := machinery.NewFeatureGateMarkerParser() + + // Try to parse the API directory + apiDir := "api" + if s.config.IsMultiGroup() && s.resource.Group != "" { + apiDir = filepath.Join("api", s.resource.Group) + + // Check if the directory exists before trying to parse it + if _, err := s.fs.FS.Stat(apiDir); err != nil { + log.Debugf("API directory %s does not exist yet, skipping feature gate discovery", apiDir) + return []string{} + } + + markers, err := parser.ParseDirectory(apiDir) + if err != nil { + log.Debugf("Failed to parse feature gates from %s: %v", apiDir, err) + return []string{} + } + + featureGates := machinery.ExtractFeatureGates(markers) + if len(featureGates) > 0 { + log.Infof("Discovered feature gates: %v", featureGates) + } else { + log.Debugf("No feature gates found in %s", apiDir) + } + + return featureGates +} diff --git a/pkg/plugins/golang/v4/scaffolds/api_test.go b/pkg/plugins/golang/v4/scaffolds/api_test.go new file mode 100644 index 00000000000..6f2273c1a25 --- /dev/null +++ b/pkg/plugins/golang/v4/scaffolds/api_test.go @@ -0,0 +1,183 @@ +/* +Copyright 2022 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 scaffolds + +import ( + "os" + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "sigs.k8s.io/kubebuilder/v4/pkg/config" + v3 "sigs.k8s.io/kubebuilder/v4/pkg/config/v3" + "sigs.k8s.io/kubebuilder/v4/pkg/machinery" + "sigs.k8s.io/kubebuilder/v4/pkg/model/resource" +) + +// mockConfig implements config.Config for testing +type mockConfig struct { + multiGroup bool +} + +func (m *mockConfig) GetVersion() config.Version { return config.Version{} } +func (m *mockConfig) GetCliVersion() string { return "" } +func (m *mockConfig) SetCliVersion(version string) error { return nil } +func (m *mockConfig) GetDomain() string { return "example.com" } +func (m *mockConfig) SetDomain(domain string) error { return nil } +func (m *mockConfig) GetRepository() string { return "" } +func (m *mockConfig) SetRepository(repository string) error { return nil } +func (m *mockConfig) GetProjectName() string { return "" } +func (m *mockConfig) SetProjectName(name string) error { return nil } +func (m *mockConfig) GetPluginChain() []string { return nil } +func (m *mockConfig) SetPluginChain(pluginChain []string) error { return nil } +func (m *mockConfig) IsMultiGroup() bool { return m.multiGroup } +func (m *mockConfig) SetMultiGroup() error { return nil } +func (m *mockConfig) ClearMultiGroup() error { return nil } +func (m *mockConfig) ResourcesLength() int { return 0 } +func (m *mockConfig) HasResource(gvk resource.GVK) bool { return false } +func (m *mockConfig) GetResource(gvk resource.GVK) (resource.Resource, error) { + return resource.Resource{}, nil +} +func (m *mockConfig) GetResources() ([]resource.Resource, error) { return nil, nil } +func (m *mockConfig) AddResource(res resource.Resource) error { return nil } +func (m *mockConfig) UpdateResource(res resource.Resource) error { return nil } +func (m *mockConfig) HasGroup(group string) bool { return false } +func (m *mockConfig) ListCRDVersions() []string { return nil } +func (m *mockConfig) ListWebhookVersions() []string { return nil } +func (m *mockConfig) DecodePluginConfig(key string, configObj interface{}) error { + return nil +} +func (m *mockConfig) EncodePluginConfig(key string, configObj interface{}) error { + return nil +} +func (m *mockConfig) MarshalYAML() ([]byte, error) { return nil, nil } +func (m *mockConfig) UnmarshalYAML([]byte) error { return nil } + +func TestAPIScaffolder_discoverFeatureGates(t *testing.T) { + tests := []struct { + name string + config config.Config + resource resource.Resource + expectedGates []string + }{ + { + name: "single group project", + config: &mockConfig{ + multiGroup: false, + }, + resource: resource.Resource{ + GVK: resource.GVK{ + Group: "example.com", + Version: "v1", + Kind: "MyResource", + }, + }, + expectedGates: []string{}, + }, + { + name: "multi group project", + config: &mockConfig{ + multiGroup: true, + }, + resource: resource.Resource{ + GVK: resource.GVK{ + Group: "example.com", + Version: "v1", + Kind: "MyResource", + }, + }, + expectedGates: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a mock filesystem + fs := afero.NewMemMapFs() + + scaffolder := &apiScaffolder{ + config: tt.config, + resource: tt.resource, + fs: machinery.Filesystem{FS: fs}, + } + + gates := scaffolder.discoverFeatureGates() + + // In a real test environment, we'd create test files with feature gates + // For now, we just verify the function doesn't panic and returns a slice + assert.NotNil(t, gates, "Expected gates slice to be returned") + assert.IsType(t, []string{}, gates, "Expected gates to be a string slice") + }) + } +} + +func TestAPIScaffolder_NewAPIScaffolder(t *testing.T) { + cfg := &mockConfig{} + res := resource.Resource{ + GVK: resource.GVK{ + Group: "example.com", + Version: "v1", + Kind: "MyResource", + }, + } + + scaffolder := NewAPIScaffolder(cfg, res, false) + require.NotNil(t, scaffolder, "Expected scaffolder to be created") + + apiScaffolder, ok := scaffolder.(*apiScaffolder) + require.True(t, ok, "Expected apiScaffolder type") + + assert.Equal(t, cfg, apiScaffolder.config) + assert.Equal(t, res, apiScaffolder.resource) + assert.False(t, apiScaffolder.force) +} + +func TestAPIScaffolder_discoverFeatureGates_Testdata(t *testing.T) { + // Test with actual testdata to ensure commented markers are ignored + scaffolder := &apiScaffolder{ + config: v3.New(), + resource: resource.Resource{ + GVK: resource.GVK{ + Group: "crew", + }, + }, + fs: machinery.Filesystem{ + FS: afero.NewOsFs(), + }, + } + + // Change to the testdata directory + originalDir, err := os.Getwd() + if err != nil { + t.Fatalf("Failed to get current directory: %v", err) + } + defer os.Chdir(originalDir) + + if err := os.Chdir("testdata/project-v4"); err != nil { + t.Skipf("Skipping test - testdata directory not available: %v", err) + } + + // Discover feature gates from the testdata + featureGates := scaffolder.discoverFeatureGates() + + // Should not find any feature gates since they're all commented out + if len(featureGates) > 0 { + t.Errorf("Expected no feature gates from testdata, but found %d: %v", len(featureGates), featureGates) + } +} diff --git a/pkg/plugins/golang/v4/scaffolds/init.go b/pkg/plugins/golang/v4/scaffolds/init.go index 5f6bd14218a..3f5dc3c7989 100644 --- a/pkg/plugins/golang/v4/scaffolds/init.go +++ b/pkg/plugins/golang/v4/scaffolds/init.go @@ -159,6 +159,7 @@ func (s *initScaffolder) Scaffold() error { &cmd.Main{ ControllerRuntimeVersion: ControllerRuntimeVersion, }, + &cmd.FeatureGates{AvailableGates: []string{}}, &templates.GoMod{ ControllerRuntimeVersion: ControllerRuntimeVersion, }, diff --git a/pkg/plugins/golang/v4/scaffolds/internal/templates/api/types.go b/pkg/plugins/golang/v4/scaffolds/internal/templates/api/types.go index 429b68192d6..2f045aeb199 100644 --- a/pkg/plugins/golang/v4/scaffolds/internal/templates/api/types.go +++ b/pkg/plugins/golang/v4/scaffolds/internal/templates/api/types.go @@ -83,6 +83,14 @@ 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"` + "`" + ` + + // 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"` + "`" + ` } // {{ .Resource.Kind }}Status defines the observed state of {{ .Resource.Kind }}. diff --git a/pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/featuregates.go b/pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/featuregates.go new file mode 100644 index 00000000000..fadeab5cac6 --- /dev/null +++ b/pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/featuregates.go @@ -0,0 +1,177 @@ +/* +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 cmd + +import ( + "sigs.k8s.io/kubebuilder/v4/pkg/machinery" +) + +var _ machinery.Template = &FeatureGates{} + +// FeatureGates scaffolds a feature gates package +type FeatureGates struct { + machinery.TemplateMixin + machinery.BoilerplateMixin + machinery.RepositoryMixin + + // Available feature gates discovered from the project + AvailableGates []string +} + +// SetTemplateDefaults implements machinery.Template +func (f *FeatureGates) SetTemplateDefaults() error { + if f.Path == "" { + f.Path = "internal/featuregates/featuregates.go" + } + + f.TemplateBody = featureGatesTemplate + + return nil +} + +const featureGatesTemplate = `{{ .Boilerplate }} + +package featuregates + +import ( + "fmt" + "strings" +) + +// FeatureGates represents a map of feature gate names to their enabled state +type FeatureGates map[string]bool + +// GetAvailableFeatureGates returns a list of available feature gates for this project +func GetAvailableFeatureGates() []string { + return []string{ +{{- range .AvailableGates }} + "{{ . }}", +{{- end }} + } +} + +// GetFeatureGatesHelpText returns the help text for the feature-gates flag +func GetFeatureGatesHelpText() string { + gates := GetAvailableFeatureGates() + if len(gates) == 0 { + return "No feature gates available" + } + return strings.Join(gates, ", ") +} + +// ValidateFeatureGates validates that all specified feature gates are available +func ValidateFeatureGates(specifiedGates FeatureGates) error { + availableGates := GetAvailableFeatureGates() + availableMap := make(map[string]bool) + + for _, gate := range availableGates { + availableMap[gate] = true + } + + var invalidGates []string + for gate := range specifiedGates { + if !availableMap[gate] { + invalidGates = append(invalidGates, gate) + } + } + + if len(invalidGates) > 0 { + return fmt.Errorf("invalid feature gates: %s. Available gates: %s", + strings.Join(invalidGates, ", "), + strings.Join(availableGates, ", ")) + } + + return nil +} + +// ParseFeatureGates parses the feature gates string into a map +func ParseFeatureGates(featureGates string) (FeatureGates, error) { + gates := make(FeatureGates) + + if featureGates == "" { + return gates, nil + } + + parts := strings.Split(featureGates, ",") + for _, part := range parts { + part = strings.TrimSpace(part) + if part == "" { + continue + } + + if strings.Contains(part, "=") { + kv := strings.SplitN(part, "=", 2) + if len(kv) == 2 { + gateName := strings.TrimSpace(kv[0]) + if gateName == "" { + return nil, fmt.Errorf("empty feature gate name") + } + gates[gateName] = strings.TrimSpace(kv[1]) == "true" + } else { + return nil, fmt.Errorf("invalid feature gate format: %s", part) + } + } else { + if part == "" { + return nil, fmt.Errorf("empty feature gate name") + } + gates[part] = true + } + } + + return gates, nil +} + +// IsEnabled checks if a specific feature gate is enabled +func (fg FeatureGates) IsEnabled(gateName string) bool { + return fg[gateName] +} + +// GetEnabledGates returns a list of enabled feature gates +func (fg FeatureGates) GetEnabledGates() []string { + var enabled []string + for gate, isEnabled := range fg { + if isEnabled { + enabled = append(enabled, gate) + } + } + return enabled +} + +// GetDisabledGates returns a list of disabled feature gates +func (fg FeatureGates) GetDisabledGates() []string { + var disabled []string + for gate, isEnabled := range fg { + if !isEnabled { + disabled = append(disabled, gate) + } + } + return disabled +} + +// String returns a string representation of the feature gates +func (fg FeatureGates) String() string { + var parts []string + for gate, enabled := range fg { + if enabled { + parts = append(parts, gate) + } else { + parts = append(parts, gate+"=false") + } + } + return strings.Join(parts, ",") +} +` diff --git a/pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/main.go b/pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/main.go index 52cf3886a8f..6dc40a90b49 100644 --- a/pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/main.go +++ b/pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/main.go @@ -248,6 +248,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/metrics/filters" "sigs.k8s.io/controller-runtime/pkg/webhook" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + + featuregates "{{ .Repo }}/internal/featuregates" %s ) @@ -267,17 +269,18 @@ func main() { var metricsAddr string var metricsCertPath, metricsCertName, metricsCertKey string var webhookCertPath, webhookCertName, webhookCertKey string - var enableLeaderElection bool + var enableLeaderElection bool var probeAddr string var secureMetrics bool var enableHTTP2 bool + var featureGates string var tlsOpts []func(*tls.Config) flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address the metrics endpoint binds to. " + "Use :8443 for HTTPS or :8080 for HTTP, or leave as 0 to disable the metrics service.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") flag.BoolVar(&enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. " + - "Enabling this will ensure there is only one active controller manager.") + "Enabling this will ensure there is only one active controller manager.") flag.BoolVar(&secureMetrics, "metrics-secure", true, "If set, the metrics endpoint is served securely via HTTPS. Use --metrics-secure=false to use HTTP instead.") flag.StringVar(&webhookCertPath, "webhook-cert-path", "", "The directory that contains the webhook certificate.") @@ -289,12 +292,27 @@ func main() { flag.StringVar(&metricsCertKey, "metrics-cert-key", "tls.key", "The name of the metrics server key file.") 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. " + + "Options are: "+featuregates.GetFeatureGatesHelpText()) opts := zap.Options{ Development: true, } opts.BindFlags(flag.CommandLine) flag.Parse() + // Parse feature gates + parsedFeatureGates, err := featuregates.ParseFeatureGates(featureGates) + if err != nil { + setupLog.Error(err, "failed to parse feature gates") + os.Exit(1) + } + + // Validate feature gates + if err := featuregates.ValidateFeatureGates(parsedFeatureGates); err != nil { + setupLog.Error(err, "invalid feature gates") + os.Exit(1) + } + ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) // if the enable-http2 flag is false (the default), http/2 should be disabled diff --git a/pkg/plugins/golang/v4/scaffolds/internal/templates/makefile.go b/pkg/plugins/golang/v4/scaffolds/internal/templates/makefile.go index fe36e1b34f5..3f795dd5beb 100644 --- a/pkg/plugins/golang/v4/scaffolds/internal/templates/makefile.go +++ b/pkg/plugins/golang/v4/scaffolds/internal/templates/makefile.go @@ -119,6 +119,8 @@ 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. From 66094625e23d424f7811bc4edccf6cdc681c4934 Mon Sep 17 00:00:00 2001 From: wazery Date: Tue, 29 Jul 2025 13:41:40 +0200 Subject: [PATCH 02/11] Add feature gate discovery tests and improve parser --- FEATURE_GATES.md | 191 ----------------- docs/book/src/SUMMARY.md | 1 + docs/book/src/reference/markers.md | 4 +- .../src/reference/markers/feature-gates.md | 136 ++++++++++++ test/e2e/v4/featuregates_discovery_test.go | 152 ++++++++++++++ test/e2e/v4/featuregates_test.go | 198 ++++++++++++++++++ testdata/project-v4/api/v1/captain_types.go | 5 + 7 files changed, 494 insertions(+), 193 deletions(-) delete mode 100644 FEATURE_GATES.md create mode 100644 docs/book/src/reference/markers/feature-gates.md create mode 100644 test/e2e/v4/featuregates_discovery_test.go create mode 100644 test/e2e/v4/featuregates_test.go diff --git a/FEATURE_GATES.md b/FEATURE_GATES.md deleted file mode 100644 index 0ded08f73df..00000000000 --- a/FEATURE_GATES.md +++ /dev/null @@ -1,191 +0,0 @@ -# Feature Gates for Kubebuilder - -This document describes the feature gates functionality implemented in Kubebuilder, which allows developers to mark fields in their API types as belonging to specific feature gates. - -## Overview - -Feature gates provide a way to enable or disable experimental features in your CRDs, similar to how Kubernetes core types handle feature gates. This allows for: - -- Gradual rollout of new features -- A/B testing of experimental functionality -- Safe deprecation of features -- Better control over API stability - -## Usage - -### 1. Marking Fields with Feature Gates - -In your API types, you can mark fields with feature gate annotations: - -```go -// MyResourceSpec defines the desired state of MyResource -type MyResourceSpec struct { - // Standard field - Name string `json:"name"` - - // Experimental field that requires the "experimental-feature" gate - // +feature-gate experimental-feature - ExperimentalField string `json:"experimentalField,omitempty"` - - // Another experimental field - // +feature-gate another-feature - AnotherField int `json:"anotherField,omitempty"` -} -``` - -### 2. Running with Feature Gates - -When you run your controller, you can enable or disable feature gates: - -```bash -# Enable all feature gates -./manager --feature-gates=experimental-feature,another-feature - -# Enable some gates and disable others -./manager --feature-gates=experimental-feature=true,another-feature=false - -# Disable a specific gate -./manager --feature-gates=experimental-feature=false -``` - -### 3. Feature Gate Discovery - -Kubebuilder automatically discovers feature gates from your API types and generates validation code. The available feature gates are listed in the help text: - -```bash -./manager --help -``` - -You'll see output like: -``` ---feature-gates string A set of key=value pairs that describe feature gates for alpha/experimental features. Options are: experimental-feature, another-feature -``` - -## Implementation Details - -### Feature Gate Parsing - -The feature gate string follows this format: -- `feature1` - enables feature1 (default) -- `feature1=true` - explicitly enables feature1 -- `feature1=false` - explicitly disables feature1 -- `feature1,feature2=false,feature3` - mixed enabled/disabled gates - -### Marker Parsing - -Kubebuilder scans your API types for markers in the format: -``` -// +feature-gate gate-name -``` - -These markers are found in: -- Struct field comments -- Function comments -- Type comments - -### Validation - -The controller validates that: -1. All specified feature gates are valid (exist in the codebase) -2. Feature gate values are properly formatted -3. No duplicate or conflicting gate specifications - -## Examples - -### Basic Example - -```go -// api/v1/myresource_types.go -type MyResourceSpec struct { - // Standard field - Name string `json:"name"` - - // Experimental field - // +feature-gate alpha-feature - AlphaField string `json:"alphaField,omitempty"` -} -``` - -### Advanced Example - -```go -// api/v1/complexresource_types.go -type ComplexResourceSpec struct { - // Standard fields - Name string `json:"name"` - Replicas int32 `json:"replicas"` - - // Beta feature - // +feature-gate beta-feature - BetaField string `json:"betaField,omitempty"` - - // Alpha feature - // +feature-gate alpha-feature - AlphaField string `json:"alphaField,omitempty"` - - // Experimental feature - // +feature-gate experimental-feature - ExperimentalField string `json:"experimentalField,omitempty"` -} -``` - -Running with different configurations: - -```bash -# Enable all features -./manager --feature-gates=alpha-feature,beta-feature,experimental-feature - -# Enable only beta features -./manager --feature-gates=beta-feature - -# Disable experimental features -./manager --feature-gates=alpha-feature,beta-feature,experimental-feature=false -``` - -## Best Practices - -1. **Use descriptive gate names**: Choose names that clearly indicate what the feature does -2. **Document your gates**: Add comments explaining what each feature gate enables -3. **Gradual rollout**: Start with alpha features, then beta, then stable -4. **Consistent naming**: Use kebab-case for feature gate names -5. **Validation**: Always validate feature gate inputs in your controllers - -## Migration Guide - -### From No Feature Gates - -1. Add feature gate markers to your experimental fields -2. Rebuild your project: `make manifests` -3. Update your deployment to include the `--feature-gates` flag -4. Test with different gate combinations - -### To Stable Features - -When a feature is ready for stable release: -1. Remove the feature gate marker -2. Update documentation -3. Consider the field stable and always available - -## Troubleshooting - -### Common Issues - -1. **Unknown feature gate**: Make sure the gate name matches exactly in your code -2. **Invalid format**: Check that your feature gate string follows the correct format -3. **Missing validation**: Ensure your controller validates feature gates before use - -### Debugging - -Enable verbose logging to see feature gate processing: - -```bash -./manager --feature-gates=your-gate --v=2 -``` - -## Future Enhancements - -Planned improvements include: -- CRD schema modification based on enabled gates -- Automatic documentation generation -- Integration with controller-runtime feature gates -- Webhook validation based on feature gates \ No newline at end of file diff --git a/docs/book/src/SUMMARY.md b/docs/book/src/SUMMARY.md index aa4d36bd6f1..a53bc5324ca 100644 --- a/docs/book/src/SUMMARY.md +++ b/docs/book/src/SUMMARY.md @@ -95,6 +95,7 @@ - [Object/DeepCopy](./reference/markers/object.md) - [RBAC](./reference/markers/rbac.md) - [Scaffold](./reference/markers/scaffold.md) + - [Feature Gates](./reference/markers/feature-gates.md) - [controller-gen CLI](./reference/controller-gen.md) - [completion](./reference/completion.md) diff --git a/docs/book/src/reference/markers.md b/docs/book/src/reference/markers.md index 0520717b0f2..37f5637363a 100644 --- a/docs/book/src/reference/markers.md +++ b/docs/book/src/reference/markers.md @@ -38,8 +38,8 @@ controller-gen: - `make manifests` generates Kubernetes object YAML, like [CustomResourceDefinitions](./markers/crd.md), - [WebhookConfigurations](./markers/webhook.md), and [RBAC - roles](./markers/rbac.md). + [WebhookConfigurations](./markers/webhook.md), [RBAC + roles](./markers/rbac.md), and [Feature Gates](./markers/feature-gates.md). - `make generate` generates code, like [runtime.Object/DeepCopy implementations](./markers/object.md). diff --git a/docs/book/src/reference/markers/feature-gates.md b/docs/book/src/reference/markers/feature-gates.md new file mode 100644 index 00000000000..8646d1571c9 --- /dev/null +++ b/docs/book/src/reference/markers/feature-gates.md @@ -0,0 +1,136 @@ +# Feature Gates + +Feature gates allow you to enable or disable experimental features in your Kubebuilder controllers. This is similar to how Kubernetes core uses feature gates to manage experimental functionality. + +## Overview + +Feature gates provide a mechanism to: +- Control the availability of experimental features +- Enable gradual rollout of new functionality +- Maintain backward compatibility during API evolution + +## Usage + +### Marking Fields with Feature Gates + +Use the `+feature-gate` marker to mark experimental fields in your API types: + +```go +type MyResourceSpec struct { + // Standard field + Name string `json:"name"` + + // Experimental field that requires the "experimental-bar" feature gate + // +feature-gate experimental-bar + Bar *string `json:"bar,omitempty"` +} +``` + +### Running with Feature Gates + +Enable feature gates when running your controller: + +```bash +# Enable a single feature gate +./manager --feature-gates=experimental-bar + +# Enable multiple feature gates +./manager --feature-gates=experimental-bar,advanced-features + +# Mixed enabled/disabled states +./manager --feature-gates=experimental-bar=true,advanced-features=false +``` + +### Feature Gate Formats + +The `--feature-gates` flag accepts: +- `feature1` - Enables feature1 (defaults to true) +- `feature1=true` - Explicitly enables feature1 +- `feature1=false` - Explicitly disables feature1 +- `feature1,feature2` - Enables both features +- `feature1=true,feature2=false` - Mixed states + +## Best Practices + +### Naming Conventions + +Use descriptive, lowercase names with hyphens: +- ✅ `experimental-bar` +- ✅ `advanced-features` +- ❌ `ExperimentalBar` +- ❌ `advanced_features` + +### Documentation + +Always document feature-gated fields: + +```go +// Bar is an experimental field that requires the "experimental-bar" feature gate +// +feature-gate experimental-bar +// +optional +Bar *string `json:"bar,omitempty"` +``` + +### Gradual Rollout Strategy + +1. **Alpha**: Feature behind feature gate +2. **Beta**: Feature enabled by default, gate for opt-out +3. **Stable**: Remove feature gate, feature always available + +## Future Integration + +When [controller-tools supports feature gates](https://github.com/kubernetes-sigs/controller-tools/issues/1238), you'll be able to use: + +```go +// +kubebuilder:feature-gate=experimental-bar +// +optional +Bar *string `json:"bar,omitempty"` +``` + +This will automatically exclude the field from CRD schemas when the feature gate is disabled. + +## Examples + +### Basic Example + +```go +type CronJobSpec struct { + // Standard fields + Schedule string `json:"schedule"` + + // Experimental timezone support + // +feature-gate timezone-support + Timezone *string `json:"timezone,omitempty"` +} +``` + +### Controller Logic + +```go +func (r *CronJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + // Check if timezone feature is enabled + if featureGates.IsEnabled("timezone-support") { + // Use timezone-aware scheduling + return r.reconcileWithTimezone(ctx, req) + } + + // Fall back to standard scheduling + return r.reconcileStandard(ctx, req) +} +``` + +## Troubleshooting + +### Common Issues + +1. **Feature gate not discovered**: Ensure the `+feature-gate` marker is on the line before the field +2. **Invalid feature gate name**: Use lowercase with hyphens only +3. **Validation errors**: Check that all specified gates are available + +### Debugging + +Enable debug logging to see feature gate discovery: + +```bash +./manager --feature-gates=experimental-bar --zap-log-level=debug +``` \ No newline at end of file diff --git a/test/e2e/v4/featuregates_discovery_test.go b/test/e2e/v4/featuregates_discovery_test.go new file mode 100644 index 00000000000..bcc9bf919e7 --- /dev/null +++ b/test/e2e/v4/featuregates_discovery_test.go @@ -0,0 +1,152 @@ +/* +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 v4 + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + + "sigs.k8s.io/kubebuilder/v4/pkg/machinery" +) + +func TestFeatureGateDiscoveryInTestdata(t *testing.T) { + // Test feature gate discovery in our testdata project + parser := machinery.NewFeatureGateMarkerParser() + + // Parse the testdata project API types + markers, err := parser.ParseFile("../../../testdata/project-v4/api/v1/captain_types.go") + assert.NoError(t, err) + assert.NotEmpty(t, markers, "Should discover feature gate markers") + + // Extract feature gate names + featureGates := machinery.ExtractFeatureGates(markers) + + // Verify we found the expected feature gates + expectedGates := []string{ + "experimental-bar", + } + + for _, expectedGate := range expectedGates { + assert.Contains(t, featureGates, expectedGate, + "Should discover feature gate: %s", expectedGate) + } + + t.Logf("Discovered feature gates: %v", featureGates) +} + +func TestFeatureGateDiscoveryIgnoresCommentedFields(t *testing.T) { + // Test that commented fields with feature gate markers are not discovered + parser := machinery.NewFeatureGateMarkerParser() + + // Create a temporary file with commented feature gate markers + tempContent := `package test + +type TestStruct struct { + // This field is commented out and should not be discovered + // +feature-gate commented-feature + // Field *string ` + "`" + `json:"field,omitempty"` + "`" + ` + + // This field is active and should be discovered + // +feature-gate active-feature + ActiveField *string ` + "`" + `json:"activeField,omitempty"` + "`" + ` +} +` + + // Write to temporary file + tmpFile, err := os.CreateTemp("", "featuregate_test_*.go") + assert.NoError(t, err) + defer os.Remove(tmpFile.Name()) + + _, err = tmpFile.WriteString(tempContent) + assert.NoError(t, err) + tmpFile.Close() + + // Parse the temporary file + markers, err := parser.ParseFile(tmpFile.Name()) + assert.NoError(t, err) + + // Extract feature gate names + featureGates := machinery.ExtractFeatureGates(markers) + + // Should only discover the active feature gate, not the commented one + assert.Contains(t, featureGates, "active-feature", "Should discover active feature gate") + assert.NotContains(t, featureGates, "commented-feature", "Should NOT discover commented feature gate") + + t.Logf("Discovered feature gates: %v", featureGates) +} + +func TestFeatureGateParsing(t *testing.T) { + // Test various feature gate parsing scenarios + testCases := []struct { + name string + input string + expected map[string]bool + hasError bool + }{ + { + name: "empty string", + input: "", + expected: map[string]bool{}, + hasError: false, + }, + { + name: "single feature gate", + input: "experimental-bar", + expected: map[string]bool{"experimental-bar": true}, + hasError: false, + }, + { + name: "multiple feature gates", + input: "experimental-bar,advanced-features", + expected: map[string]bool{ + "experimental-bar": true, + "advanced-features": true, + }, + hasError: false, + }, + { + name: "mixed enabled and disabled", + input: "experimental-bar=true,advanced-features=false", + expected: map[string]bool{ + "experimental-bar": true, + "advanced-features": false, + }, + hasError: false, + }, + { + name: "invalid format", + input: "experimental-bar=invalid", + expected: nil, + hasError: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result, err := machinery.ParseFeatureGates(tc.input) + + if tc.hasError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, machinery.FeatureGates(tc.expected), result) + } + }) + } +} diff --git a/test/e2e/v4/featuregates_test.go b/test/e2e/v4/featuregates_test.go new file mode 100644 index 00000000000..f0b128f57c5 --- /dev/null +++ b/test/e2e/v4/featuregates_test.go @@ -0,0 +1,198 @@ +/* +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 v4 + +import ( + "os" + "path/filepath" + "strings" + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "sigs.k8s.io/kubebuilder/v4/test/e2e/utils" +) + +func TestFeatureGates(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Feature Gates Suite") +} + +var _ = Describe("Feature Gates", func() { + Context("project with feature gates", func() { + var ( + ctx *utils.TestContext + ) + + BeforeEach(func() { + var err error + ctx, err = utils.NewTestContext("go") + Expect(err).NotTo(HaveOccurred()) + Expect(ctx).NotTo(BeNil()) + + // Set up test context + ctx.Domain = "example.com" + ctx.Group = "crew" + ctx.Version = "v1" + ctx.Kind = "Captain" + ctx.Resources = "captains" + + // Prepare the test environment + err = ctx.Prepare() + Expect(err).NotTo(HaveOccurred()) + }) + + AfterEach(func() { + ctx.Destroy() + }) + + It("should scaffold a project with feature gates", func() { + // Initialize the project + err := ctx.Init() + Expect(err).NotTo(HaveOccurred()) + + // Create API with feature gates + err = ctx.CreateAPI( + "--group", ctx.Group, + "--version", ctx.Version, + "--kind", ctx.Kind, + "--resource", "--controller", + ) + Expect(err).NotTo(HaveOccurred()) + + // Verify feature gates file was generated + featureGatesFile := filepath.Join(ctx.Dir, "internal", "featuregates", "featuregates.go") + Expect(featureGatesFile).To(BeAnExistingFile()) + + // Verify main.go has feature gates flag + mainFile := filepath.Join(ctx.Dir, "cmd", "main.go") + Expect(mainFile).To(BeAnExistingFile()) + mainContent, err := os.ReadFile(mainFile) + Expect(err).NotTo(HaveOccurred()) + Expect(string(mainContent)).To(ContainSubstring("--feature-gates")) + Expect(string(mainContent)).To(ContainSubstring("featuregates")) + + // Verify API types have feature gate example + typesFile := filepath.Join(ctx.Dir, "api", ctx.Version, ctx.Kind+"_types.go") + Expect(typesFile).To(BeAnExistingFile()) + typesContent, err := os.ReadFile(typesFile) + Expect(err).NotTo(HaveOccurred()) + Expect(string(typesContent)).To(ContainSubstring("+feature-gate")) + }) + + It("should discover feature gates from API types", func() { + // Initialize the project + err := ctx.Init() + Expect(err).NotTo(HaveOccurred()) + + // Create API + err = ctx.CreateAPI( + "--group", ctx.Group, + "--version", ctx.Version, + "--kind", ctx.Kind, + "--resource", "--controller", + ) + Expect(err).NotTo(HaveOccurred()) + + // Add custom feature gates to the API types + typesFile := filepath.Join(ctx.Dir, "api", ctx.Version, ctx.Kind+"_types.go") + typesContent, err := os.ReadFile(typesFile) + Expect(err).NotTo(HaveOccurred()) + + // Add a new field with feature gate + newField := ` + // Experimental field that requires the "experimental-field" feature gate + // +feature-gate experimental-field + // +optional + ExperimentalField *string ` + "`" + `json:"experimentalField,omitempty"` + "`" + ` +` + // Insert the new field in the Spec struct + updatedContent := strings.Replace(string(typesContent), + "// Bar *string ` + \"`\" + `json:\"bar,omitempty\"` + \"`\" + `", + "// Bar *string ` + \"`\" + `json:\"bar,omitempty\"` + \"`\" + `\n"+newField, + 1) + + err = os.WriteFile(typesFile, []byte(updatedContent), 0644) + Expect(err).NotTo(HaveOccurred()) + + // Regenerate the project to discover new feature gates + err = ctx.CreateAPI( + "--group", ctx.Group, + "--version", ctx.Version, + "--kind", ctx.Kind, + "--resource", "--controller", + "--force", + ) + Expect(err).NotTo(HaveOccurred()) + + // Verify the new feature gate was discovered + featureGatesFile := filepath.Join(ctx.Dir, "internal", "featuregates", "featuregates.go") + Expect(featureGatesFile).To(BeAnExistingFile()) + featureGatesContent, err := os.ReadFile(featureGatesFile) + Expect(err).NotTo(HaveOccurred()) + Expect(string(featureGatesContent)).To(ContainSubstring("experimental-field")) + }) + + It("should build project with feature gates", func() { + // Initialize the project + err := ctx.Init() + Expect(err).NotTo(HaveOccurred()) + + // Create API + err = ctx.CreateAPI( + "--group", ctx.Group, + "--version", ctx.Version, + "--kind", ctx.Kind, + "--resource", "--controller", + ) + Expect(err).NotTo(HaveOccurred()) + + // Build the project - this should succeed with feature gates + err = ctx.Make("build") + Expect(err).NotTo(HaveOccurred()) + + // Verify the binary was created + binaryPath := filepath.Join(ctx.Dir, "bin", "manager") + Expect(binaryPath).To(BeAnExistingFile()) + }) + + It("should generate manifests with feature gates", func() { + // Initialize the project + err := ctx.Init() + Expect(err).NotTo(HaveOccurred()) + + // Create API + err = ctx.CreateAPI( + "--group", ctx.Group, + "--version", ctx.Version, + "--kind", ctx.Kind, + "--resource", "--controller", + ) + Expect(err).NotTo(HaveOccurred()) + + // Generate manifests + err = ctx.Make("manifests") + Expect(err).NotTo(HaveOccurred()) + + // Verify CRDs were generated + crdFile := filepath.Join(ctx.Dir, "config", "crd", "bases", + strings.ToLower(ctx.Group)+"_"+ctx.Version+"_"+strings.ToLower(ctx.Kind)+".yaml") + Expect(crdFile).To(BeAnExistingFile()) + }) + }) +}) diff --git a/testdata/project-v4/api/v1/captain_types.go b/testdata/project-v4/api/v1/captain_types.go index c7377c347c9..f0a51cdf871 100644 --- a/testdata/project-v4/api/v1/captain_types.go +++ b/testdata/project-v4/api/v1/captain_types.go @@ -33,6 +33,11 @@ type CaptainSpec struct { // foo is an example field of Captain. Edit captain_types.go to remove/update // +optional Foo *string `json:"foo,omitempty"` + + // Bar is an experimental field that requires the "experimental-bar" feature gate to be enabled + // +feature-gate experimental-bar + // +optional + Bar *string `json:"bar,omitempty"` } // CaptainStatus defines the observed state of Captain. From 302b27dab0ee8efa3301a4bfdd9e2da671aca491 Mon Sep 17 00:00:00 2001 From: wazery Date: Tue, 29 Jul 2025 13:45:15 +0200 Subject: [PATCH 03/11] Enhance docs book integration with feature gates documentation --- .../src/reference/markers/feature-gates.md | 60 +++++++++++++++++-- pkg/machinery/featuregate.go | 2 +- .../scaffolds/internal/templates/cmd/main.go | 4 +- 3 files changed, 58 insertions(+), 8 deletions(-) diff --git a/docs/book/src/reference/markers/feature-gates.md b/docs/book/src/reference/markers/feature-gates.md index 8646d1571c9..ed824949717 100644 --- a/docs/book/src/reference/markers/feature-gates.md +++ b/docs/book/src/reference/markers/feature-gates.md @@ -2,6 +2,34 @@ Feature gates allow you to enable or disable experimental features in your Kubebuilder controllers. This is similar to how Kubernetes core uses feature gates to manage experimental functionality. +## Quick Start + +### Marking Fields + +```go +type MyResourceSpec struct { + // Standard field + Name string `json:"name"` + + // Experimental field + // +feature-gate experimental-bar + Bar *string `json:"bar,omitempty"` +} +``` + +### Running with Feature Gates + +```bash +# Enable feature gates +./manager --feature-gates=experimental-bar + +# Multiple gates +./manager --feature-gates=experimental-bar,advanced-features + +# Mixed states +./manager --feature-gates=experimental-bar=true,advanced-features=false +``` + ## Overview Feature gates provide a mechanism to: @@ -55,10 +83,10 @@ The `--feature-gates` flag accepts: ### Naming Conventions Use descriptive, lowercase names with hyphens: -- ✅ `experimental-bar` -- ✅ `advanced-features` -- ❌ `ExperimentalBar` -- ❌ `advanced_features` +- `experimental-bar` +- `advanced-features` +- `ExperimentalBar` +- `advanced_features` ### Documentation @@ -133,4 +161,26 @@ Enable debug logging to see feature gate discovery: ```bash ./manager --feature-gates=experimental-bar --zap-log-level=debug -``` \ No newline at end of file +``` + +## Implementation Status + +### Production Ready + +- Feature gate discovery and validation +- Controller integration with `--feature-gates` flag +- Scaffolding integration for new projects + +### Future Enhancement + +- CRD schema modification (requires [controller-tools support](https://github.com/kubernetes-sigs/controller-tools/issues/1238)) + +When [controller-tools supports feature gates](https://github.com/kubernetes-sigs/controller-tools/issues/1238), you'll be able to use: + +```go +// +kubebuilder:feature-gate=experimental-bar +// +optional +Bar *string `json:"bar,omitempty"` +``` + +This will automatically exclude the field from CRD schemas when the feature gate is disabled. \ No newline at end of file diff --git a/pkg/machinery/featuregate.go b/pkg/machinery/featuregate.go index b8468c5c1fa..f5875e166af 100644 --- a/pkg/machinery/featuregate.go +++ b/pkg/machinery/featuregate.go @@ -5,7 +5,7 @@ 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 + 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, diff --git a/pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/main.go b/pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/main.go index 6dc40a90b49..4232b2dff60 100644 --- a/pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/main.go +++ b/pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/main.go @@ -269,7 +269,7 @@ func main() { var metricsAddr string var metricsCertPath, metricsCertName, metricsCertKey string var webhookCertPath, webhookCertName, webhookCertKey string - var enableLeaderElection bool + var enableLeaderElection bool var probeAddr string var secureMetrics bool var enableHTTP2 bool @@ -280,7 +280,7 @@ func main() { flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") flag.BoolVar(&enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. " + - "Enabling this will ensure there is only one active controller manager.") + "Enabling this will ensure there is only one active controller manager.") flag.BoolVar(&secureMetrics, "metrics-secure", true, "If set, the metrics endpoint is served securely via HTTPS. Use --metrics-secure=false to use HTTP instead.") flag.StringVar(&webhookCertPath, "webhook-cert-path", "", "The directory that contains the webhook certificate.") From 7b325d51ea32bdea3ec9d6ba5171981bc6350eff Mon Sep 17 00:00:00 2001 From: wazery Date: Wed, 6 Aug 2025 13:07:59 +0200 Subject: [PATCH 04/11] change the tests to use gomega to adhere to project standard --- pkg/plugins/golang/v4/scaffolds/api_test.go | 78 ++++++++++----------- 1 file changed, 37 insertions(+), 41 deletions(-) diff --git a/pkg/plugins/golang/v4/scaffolds/api_test.go b/pkg/plugins/golang/v4/scaffolds/api_test.go index 6f2273c1a25..2e4e914c5c7 100644 --- a/pkg/plugins/golang/v4/scaffolds/api_test.go +++ b/pkg/plugins/golang/v4/scaffolds/api_test.go @@ -18,15 +18,13 @@ package scaffolds import ( "os" + "path/filepath" "testing" - "github.com/spf13/afero" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" + . "github.com/onsi/gomega" "sigs.k8s.io/kubebuilder/v4/pkg/config" v3 "sigs.k8s.io/kubebuilder/v4/pkg/config/v3" - "sigs.k8s.io/kubebuilder/v4/pkg/machinery" "sigs.k8s.io/kubebuilder/v4/pkg/model/resource" ) @@ -108,27 +106,24 @@ func TestAPIScaffolder_discoverFeatureGates(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Create a mock filesystem - fs := afero.NewMemMapFs() - - scaffolder := &apiScaffolder{ - config: tt.config, - resource: tt.resource, - fs: machinery.Filesystem{FS: fs}, + scaffolder := NewAPIScaffolder(tt.config, tt.resource, false) + apiScaffolder, ok := scaffolder.(*apiScaffolder) + if !ok { + t.Fatal("Expected apiScaffolder type") } - gates := scaffolder.discoverFeatureGates() + gates := apiScaffolder.discoverFeatureGates() // In a real test environment, we'd create test files with feature gates // For now, we just verify the function doesn't panic and returns a slice - assert.NotNil(t, gates, "Expected gates slice to be returned") - assert.IsType(t, []string{}, gates, "Expected gates to be a string slice") + Expect(gates).NotTo(BeNil()) + Expect(gates).To(BeAssignableToTypeOf([]string{})) }) } } func TestAPIScaffolder_NewAPIScaffolder(t *testing.T) { - cfg := &mockConfig{} + cfg := v3.New() res := resource.Resource{ GVK: resource.GVK{ Group: "example.com", @@ -138,45 +133,46 @@ func TestAPIScaffolder_NewAPIScaffolder(t *testing.T) { } scaffolder := NewAPIScaffolder(cfg, res, false) - require.NotNil(t, scaffolder, "Expected scaffolder to be created") + Expect(scaffolder).NotTo(BeNil()) apiScaffolder, ok := scaffolder.(*apiScaffolder) - require.True(t, ok, "Expected apiScaffolder type") + Expect(ok).To(BeTrue()) - assert.Equal(t, cfg, apiScaffolder.config) - assert.Equal(t, res, apiScaffolder.resource) - assert.False(t, apiScaffolder.force) + Expect(apiScaffolder.config).To(Equal(cfg)) + Expect(apiScaffolder.resource).To(Equal(res)) + Expect(apiScaffolder.force).To(BeFalse()) } func TestAPIScaffolder_discoverFeatureGates_Testdata(t *testing.T) { - // Test with actual testdata to ensure commented markers are ignored - scaffolder := &apiScaffolder{ - config: v3.New(), - resource: resource.Resource{ - GVK: resource.GVK{ - Group: "crew", - }, - }, - fs: machinery.Filesystem{ - FS: afero.NewOsFs(), - }, - } - - // Change to the testdata directory - originalDir, err := os.Getwd() + // Change to testdata directory + oldDir, err := os.Getwd() if err != nil { t.Fatalf("Failed to get current directory: %v", err) } - defer os.Chdir(originalDir) + defer os.Chdir(oldDir) - if err := os.Chdir("testdata/project-v4"); err != nil { - t.Skipf("Skipping test - testdata directory not available: %v", err) + // Change to testdata/project-v4 directory + testdataDir := filepath.Join("testdata", "project-v4") + if err := os.Chdir(testdataDir); err != nil { + t.Fatalf("Failed to change to testdata directory: %v", err) } - // Discover feature gates from the testdata - featureGates := scaffolder.discoverFeatureGates() + cfg := v3.New() + res := resource.Resource{ + GVK: resource.GVK{ + Group: "crew.example.com", + Version: "v1", + Kind: "Captain", + }, + } + + scaffolder := NewAPIScaffolder(cfg, res, false) + apiScaffolder, ok := scaffolder.(*apiScaffolder) + if !ok { + t.Fatal("Expected apiScaffolder type") + } - // Should not find any feature gates since they're all commented out + featureGates := apiScaffolder.discoverFeatureGates() if len(featureGates) > 0 { t.Errorf("Expected no feature gates from testdata, but found %d: %v", len(featureGates), featureGates) } From 76f14a0c05880be09ea338f18f31482abba69d7a Mon Sep 17 00:00:00 2001 From: wazery Date: Wed, 6 Aug 2025 13:11:08 +0200 Subject: [PATCH 05/11] rewrite the e2e test to adhere to project style --- pkg/plugins/golang/v4/scaffolds/api.go | 15 +- pkg/plugins/golang/v4/scaffolds/api_test.go | 16 +- test/e2e/v4/featuregates_discovery_test.go | 282 ++++++++++++-------- 3 files changed, 200 insertions(+), 113 deletions(-) diff --git a/pkg/plugins/golang/v4/scaffolds/api.go b/pkg/plugins/golang/v4/scaffolds/api.go index 863738e92cd..2507d9ebd81 100644 --- a/pkg/plugins/golang/v4/scaffolds/api.go +++ b/pkg/plugins/golang/v4/scaffolds/api.go @@ -19,7 +19,7 @@ package scaffolds import ( "errors" "fmt" - log "log/slog" + "log/slog" "path/filepath" "github.com/spf13/afero" @@ -65,13 +65,13 @@ func (s *apiScaffolder) InjectFS(fs machinery.Filesystem) { // Scaffold implements cmdutil.Scaffolder func (s *apiScaffolder) Scaffold() error { - log.Info("Writing scaffold for you to edit...") + slog.Info("Writing scaffold for you to edit...") // Load the boilerplate boilerplate, err := afero.ReadFile(s.fs.FS, hack.DefaultBoilerplatePath) if err != nil { if errors.Is(err, afero.ErrFileNotFound) { - log.Warn("Unable to find boilerplate file."+ + slog.Warn("Unable to find boilerplate file."+ "This file is used to generate the license header in the project.\n"+ "Note that controller-gen will also use this. Therefore, ensure that you "+ "add the license file or configure your project accordingly.", @@ -143,24 +143,25 @@ func (s *apiScaffolder) discoverFeatureGates() []string { apiDir := "api" if s.config.IsMultiGroup() && s.resource.Group != "" { apiDir = filepath.Join("api", s.resource.Group) + } // Check if the directory exists before trying to parse it if _, err := s.fs.FS.Stat(apiDir); err != nil { - log.Debugf("API directory %s does not exist yet, skipping feature gate discovery", apiDir) + slog.Debug("API directory does not exist yet, skipping feature gate discovery", "apiDir", apiDir) return []string{} } markers, err := parser.ParseDirectory(apiDir) if err != nil { - log.Debugf("Failed to parse feature gates from %s: %v", apiDir, err) + slog.Debug("Failed to parse feature gates from directory", "apiDir", apiDir, "error", err) return []string{} } featureGates := machinery.ExtractFeatureGates(markers) if len(featureGates) > 0 { - log.Infof("Discovered feature gates: %v", featureGates) + slog.Info("Discovered feature gates", "featureGates", featureGates) } else { - log.Debugf("No feature gates found in %s", apiDir) + slog.Debug("No feature gates found in directory", "apiDir", apiDir) } return featureGates diff --git a/pkg/plugins/golang/v4/scaffolds/api_test.go b/pkg/plugins/golang/v4/scaffolds/api_test.go index 2e4e914c5c7..3f2b0f94452 100644 --- a/pkg/plugins/golang/v4/scaffolds/api_test.go +++ b/pkg/plugins/golang/v4/scaffolds/api_test.go @@ -23,8 +23,10 @@ import ( . "github.com/onsi/gomega" + "github.com/spf13/afero" "sigs.k8s.io/kubebuilder/v4/pkg/config" v3 "sigs.k8s.io/kubebuilder/v4/pkg/config/v3" + "sigs.k8s.io/kubebuilder/v4/pkg/machinery" "sigs.k8s.io/kubebuilder/v4/pkg/model/resource" ) @@ -68,6 +70,8 @@ func (m *mockConfig) MarshalYAML() ([]byte, error) { return nil, nil } func (m *mockConfig) UnmarshalYAML([]byte) error { return nil } func TestAPIScaffolder_discoverFeatureGates(t *testing.T) { + RegisterTestingT(t) + tests := []struct { name string config config.Config @@ -112,6 +116,9 @@ func TestAPIScaffolder_discoverFeatureGates(t *testing.T) { t.Fatal("Expected apiScaffolder type") } + // Initialize filesystem to prevent nil pointer dereference + apiScaffolder.fs = machinery.Filesystem{FS: afero.NewMemMapFs()} + gates := apiScaffolder.discoverFeatureGates() // In a real test environment, we'd create test files with feature gates @@ -123,6 +130,8 @@ func TestAPIScaffolder_discoverFeatureGates(t *testing.T) { } func TestAPIScaffolder_NewAPIScaffolder(t *testing.T) { + RegisterTestingT(t) + cfg := v3.New() res := resource.Resource{ GVK: resource.GVK{ @@ -151,8 +160,8 @@ func TestAPIScaffolder_discoverFeatureGates_Testdata(t *testing.T) { } defer os.Chdir(oldDir) - // Change to testdata/project-v4 directory - testdataDir := filepath.Join("testdata", "project-v4") + // Change to testdata/project-v4 directory (from pkg/plugins/golang/v4/scaffolds/ to ../../../../../testdata/project-v4) + testdataDir := filepath.Join("../../../../../testdata", "project-v4") if err := os.Chdir(testdataDir); err != nil { t.Fatalf("Failed to change to testdata directory: %v", err) } @@ -172,6 +181,9 @@ func TestAPIScaffolder_discoverFeatureGates_Testdata(t *testing.T) { t.Fatal("Expected apiScaffolder type") } + // Initialize filesystem to prevent nil pointer dereference + 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) diff --git a/test/e2e/v4/featuregates_discovery_test.go b/test/e2e/v4/featuregates_discovery_test.go index bcc9bf919e7..8a3c17d6984 100644 --- a/test/e2e/v4/featuregates_discovery_test.go +++ b/test/e2e/v4/featuregates_discovery_test.go @@ -18,44 +18,61 @@ package v4 import ( "os" - "testing" + "path/filepath" + "strings" - "github.com/stretchr/testify/assert" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" "sigs.k8s.io/kubebuilder/v4/pkg/machinery" + "sigs.k8s.io/kubebuilder/v4/pkg/plugin/util" + "sigs.k8s.io/kubebuilder/v4/test/e2e/utils" ) -func TestFeatureGateDiscoveryInTestdata(t *testing.T) { - // Test feature gate discovery in our testdata project - parser := machinery.NewFeatureGateMarkerParser() - - // Parse the testdata project API types - markers, err := parser.ParseFile("../../../testdata/project-v4/api/v1/captain_types.go") - assert.NoError(t, err) - assert.NotEmpty(t, markers, "Should discover feature gate markers") - - // Extract feature gate names - featureGates := machinery.ExtractFeatureGates(markers) - - // Verify we found the expected feature gates - expectedGates := []string{ - "experimental-bar", - } - - for _, expectedGate := range expectedGates { - assert.Contains(t, featureGates, expectedGate, - "Should discover feature gate: %s", expectedGate) - } +var _ = Describe("Feature Gates Discovery", func() { + var kbc *utils.TestContext + + BeforeEach(func() { + var err error + kbc, err = utils.NewTestContext(util.KubebuilderBinName, "GO111MODULE=on") + Expect(err).NotTo(HaveOccurred()) + Expect(kbc.Prepare()).To(Succeed()) + }) + + AfterEach(func() { + By("cleaning up test context") + kbc.Destroy() + }) + + Context("when parsing testdata project", func() { + It("should discover feature gate markers from captain_types.go", func() { + By("parsing the testdata project API types") + parser := machinery.NewFeatureGateMarkerParser() + markers, err := parser.ParseFile("../../../testdata/project-v4/api/v1/captain_types.go") + Expect(err).NotTo(HaveOccurred()) + Expect(markers).NotTo(BeEmpty(), "Should discover feature gate markers") + + By("extracting feature gate names") + featureGates := machinery.ExtractFeatureGates(markers) + + By("verifying expected feature gates are found") + expectedGates := []string{ + "experimental-bar", + } - t.Logf("Discovered feature gates: %v", featureGates) -} + for _, expectedGate := range expectedGates { + Expect(featureGates).To(ContainElement(expectedGate), + "Should discover feature gate: %s", expectedGate) + } -func TestFeatureGateDiscoveryIgnoresCommentedFields(t *testing.T) { - // Test that commented fields with feature gate markers are not discovered - parser := machinery.NewFeatureGateMarkerParser() + GinkgoWriter.Printf("Discovered feature gates: %v\n", featureGates) + }) + }) - // Create a temporary file with commented feature gate markers - tempContent := `package test + Context("when parsing files with commented fields", func() { + It("should ignore commented feature gate markers", func() { + By("creating a temporary file with commented feature gate markers") + tempContent := `package test type TestStruct struct { // This field is commented out and should not be discovered @@ -68,85 +85,142 @@ type TestStruct struct { } ` - // Write to temporary file - tmpFile, err := os.CreateTemp("", "featuregate_test_*.go") - assert.NoError(t, err) - defer os.Remove(tmpFile.Name()) + By("writing temporary file") + tmpFile, err := os.CreateTemp("", "featuregate_test_*.go") + Expect(err).NotTo(HaveOccurred()) + defer os.Remove(tmpFile.Name()) - _, err = tmpFile.WriteString(tempContent) - assert.NoError(t, err) - tmpFile.Close() + _, err = tmpFile.WriteString(tempContent) + Expect(err).NotTo(HaveOccurred()) + tmpFile.Close() - // Parse the temporary file - markers, err := parser.ParseFile(tmpFile.Name()) - assert.NoError(t, err) + By("parsing the temporary file") + parser := machinery.NewFeatureGateMarkerParser() + markers, err := parser.ParseFile(tmpFile.Name()) + Expect(err).NotTo(HaveOccurred()) - // Extract feature gate names - featureGates := machinery.ExtractFeatureGates(markers) + By("extracting feature gate names") + featureGates := machinery.ExtractFeatureGates(markers) - // Should only discover the active feature gate, not the commented one - assert.Contains(t, featureGates, "active-feature", "Should discover active feature gate") - assert.NotContains(t, featureGates, "commented-feature", "Should NOT discover commented feature gate") + By("verifying only active feature gates are discovered") + Expect(featureGates).To(ContainElement("active-feature"), "Should discover active feature gate") + Expect(featureGates).NotTo(ContainElement("commented-feature"), "Should NOT discover commented feature gate") - t.Logf("Discovered feature gates: %v", featureGates) -} + GinkgoWriter.Printf("Discovered feature gates: %v\n", featureGates) + }) + }) + + Context("when parsing feature gate strings", func() { + It("should handle various feature gate parsing scenarios", func() { + testCases := []struct { + name string + input string + expected map[string]bool + hasError bool + }{ + { + name: "empty string", + input: "", + expected: map[string]bool{}, + hasError: false, + }, + { + name: "single feature gate", + input: "experimental-bar", + expected: map[string]bool{"experimental-bar": true}, + hasError: false, + }, + { + name: "multiple feature gates", + input: "experimental-bar,advanced-features", + expected: map[string]bool{ + "experimental-bar": true, + "advanced-features": true, + }, + hasError: false, + }, + { + name: "mixed enabled and disabled", + input: "experimental-bar=true,advanced-features=false", + expected: map[string]bool{ + "experimental-bar": true, + "advanced-features": false, + }, + hasError: false, + }, + { + name: "invalid format", + input: "experimental-bar=invalid", + expected: nil, + hasError: true, + }, + } -func TestFeatureGateParsing(t *testing.T) { - // Test various feature gate parsing scenarios - testCases := []struct { - name string - input string - expected map[string]bool - hasError bool - }{ - { - name: "empty string", - input: "", - expected: map[string]bool{}, - hasError: false, - }, - { - name: "single feature gate", - input: "experimental-bar", - expected: map[string]bool{"experimental-bar": true}, - hasError: false, - }, - { - name: "multiple feature gates", - input: "experimental-bar,advanced-features", - expected: map[string]bool{ - "experimental-bar": true, - "advanced-features": true, - }, - hasError: false, - }, - { - name: "mixed enabled and disabled", - input: "experimental-bar=true,advanced-features=false", - expected: map[string]bool{ - "experimental-bar": true, - "advanced-features": false, - }, - hasError: false, - }, - { - name: "invalid format", - input: "experimental-bar=invalid", - expected: nil, - hasError: true, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - result, err := machinery.ParseFeatureGates(tc.input) - - if tc.hasError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - assert.Equal(t, machinery.FeatureGates(tc.expected), result) + for _, tc := range testCases { + By("testing scenario: " + tc.name) + result, err := machinery.ParseFeatureGates(tc.input) + + if tc.hasError { + Expect(err).To(HaveOccurred()) + } else { + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(machinery.FeatureGates(tc.expected))) + } } }) - } -} + }) + + Context("when scaffolding a project with feature gates", func() { + It("should generate feature gates file correctly", func() { + By("initializing a project") + err := kbc.Init( + "--plugins", "go/v4", + "--project-version", "3", + "--domain", kbc.Domain, + ) + Expect(err).NotTo(HaveOccurred(), "Failed to initialize project") + + By("creating API with feature gate markers") + err = kbc.CreateAPI( + "--group", kbc.Group, + "--version", kbc.Version, + "--kind", kbc.Kind, + "--make=false", + "--manifests=false", + ) + Expect(err).NotTo(HaveOccurred(), "Failed to create API definition") + + By("adding feature gate markers to the API types") + apiTypesFile := filepath.Join(kbc.Dir, "api", kbc.Version, strings.ToLower(kbc.Kind)+"_types.go") + + // Add a feature gate marker to the spec + err = util.InsertCode( + apiTypesFile, + "//+kubebuilder:validation:Optional", + "\n\t// +feature-gate experimental-feature", + ) + Expect(err).NotTo(HaveOccurred(), "Failed to add feature gate marker") + + // Add a field after the marker + err = util.InsertCode( + apiTypesFile, + "// +feature-gate experimental-feature", + "\n\tExperimentalField *string `json:\"experimentalField,omitempty\"`", + ) + Expect(err).NotTo(HaveOccurred(), "Failed to add experimental field") + + By("regenerating the project to discover feature gates") + err = kbc.Regenerate() + Expect(err).NotTo(HaveOccurred(), "Failed to regenerate project") + + By("verifying feature gates file was generated") + featureGatesFile := filepath.Join(kbc.Dir, "internal", "featuregates", "featuregates.go") + Expect(featureGatesFile).To(BeAnExistingFile()) + + By("verifying feature gates file contains the discovered feature gate") + hasContent, err := util.HasFileContentWith(featureGatesFile, "experimental-feature") + Expect(err).NotTo(HaveOccurred()) + Expect(hasContent).To(BeTrue(), "Feature gates file should contain the discovered feature gate") + }) + }) +}) From ec261d7a560919779a4f52d82b7e48664537092d Mon Sep 17 00:00:00 2001 From: wazery Date: Thu, 7 Aug 2025 16:14:07 +0200 Subject: [PATCH 06/11] fix tests for feature gates --- .../src/reference/markers/feature-gates.md | 332 ++++++++++++++---- .../cronjob-tutorial/generate_cronjob.go | 18 +- .../generate_multiversion.go | 18 +- pkg/machinery/featuregate.go | 46 ++- pkg/plugins/golang/v4/scaffolds/api.go | 96 ++++- pkg/plugins/golang/v4/scaffolds/api_test.go | 20 +- .../scaffolds/internal/templates/api/types.go | 7 +- .../scaffolds/internal/templates/cmd/main.go | 1 + .../scaffolds/internal/templates/makefile.go | 2 - test/e2e/featuregates/featuregates_test.go | 217 ++++++++++++ test/e2e/v4/featuregates_discovery_test.go | 15 +- test/e2e/v4/featuregates_test.go | 172 ++++----- test/features.sh | 3 + 13 files changed, 751 insertions(+), 196 deletions(-) create mode 100644 test/e2e/featuregates/featuregates_test.go diff --git a/docs/book/src/reference/markers/feature-gates.md b/docs/book/src/reference/markers/feature-gates.md index ed824949717..6d4db06759c 100644 --- a/docs/book/src/reference/markers/feature-gates.md +++ b/docs/book/src/reference/markers/feature-gates.md @@ -13,6 +13,7 @@ type MyResourceSpec struct { // Experimental field // +feature-gate experimental-bar + // +optional Bar *string `json:"bar,omitempty"` } ``` @@ -21,13 +22,13 @@ type MyResourceSpec struct { ```bash # Enable feature gates -./manager --feature-gates=experimental-bar +./manager --feature-gates=experimental-bar=true # Multiple gates -./manager --feature-gates=experimental-bar,advanced-features - -# Mixed states ./manager --feature-gates=experimental-bar=true,advanced-features=false + +# All gates enabled (useful for testing) +./manager --feature-gates=experimental-bar=true,advanced-features=true ``` ## Overview @@ -36,6 +37,7 @@ Feature gates provide a mechanism to: - Control the availability of experimental features - Enable gradual rollout of new functionality - Maintain backward compatibility during API evolution +- Test experimental functionality safely in production environments ## Usage @@ -45,55 +47,91 @@ Use the `+feature-gate` marker to mark experimental fields in your API types: ```go type MyResourceSpec struct { - // Standard field + // Standard field - always available Name string `json:"name"` // Experimental field that requires the "experimental-bar" feature gate // +feature-gate experimental-bar + // +optional Bar *string `json:"bar,omitempty"` + + // Another experimental field with different gate + // +feature-gate advanced-features + // This field enables advanced processing capabilities + // +optional + AdvancedConfig *AdvancedConfig `json:"advancedConfig,omitempty"` } ``` -### Running with Feature Gates +### Feature Gate Validation -Enable feature gates when running your controller: +Feature gates are validated for proper format: +- **Valid**: `experimental-bar=true,advanced-features=false` +- **Valid**: `experimental-bar=true` (short form) +- **Invalid**: `experimental-bar=maybe` (only 'true' and 'false' are allowed) +- **Invalid**: `ExperimentalBar=true` (should use kebab-case) -```bash -# Enable a single feature gate -./manager --feature-gates=experimental-bar +### Automated Discovery -# Enable multiple feature gates -./manager --feature-gates=experimental-bar,advanced-features +Kubebuilder automatically discovers feature gates from your API types during scaffolding: -# Mixed enabled/disabled states -./manager --feature-gates=experimental-bar=true,advanced-features=false +1. **During `kubebuilder create api`**: Scans for existing `+feature-gate` markers +2. **Generates `internal/featuregates/featuregates.go`**: Contains all discovered gates +3. **Updates `cmd/main.go`**: Adds `--feature-gates` flag support + +```go +// Generated in internal/featuregates/featuregates.go +var availableFeatureGates = map[string]bool{ + "experimental-bar": false, // Default disabled + "advanced-features": false, // Default disabled +} ``` -### Feature Gate Formats +### Controller Integration + +Access feature gate state in your controller logic: -The `--feature-gates` flag accepts: -- `feature1` - Enables feature1 (defaults to true) -- `feature1=true` - Explicitly enables feature1 -- `feature1=false` - Explicitly disables feature1 -- `feature1,feature2` - Enables both features -- `feature1=true,feature2=false` - Mixed states +```go +func (r *MyResourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := log.FromContext(ctx) + + // Get your resource + var myResource myapiv1.MyResource + if err := r.Get(ctx, req.NamespacedName, &myResource); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + + // Check if experimental feature is enabled + if featureGates.IsEnabled("experimental-bar") && myResource.Spec.Bar != nil { + log.Info("Using experimental bar feature", "value", *myResource.Spec.Bar) + // Implement experimental functionality + return r.handleExperimentalBar(ctx, &myResource) + } + + // Standard reconciliation logic + return r.handleStandard(ctx, &myResource) +} +``` ## Best Practices ### Naming Conventions Use descriptive, lowercase names with hyphens: -- `experimental-bar` -- `advanced-features` -- `ExperimentalBar` -- `advanced_features` +- ✅ `experimental-bar` +- ✅ `advanced-features` +- ✅ `timezone-support` +- ❌ `ExperimentalBar` (should be kebab-case) +- ❌ `advanced_features` (use hyphens, not underscores) ### Documentation Always document feature-gated fields: ```go -// Bar is an experimental field that requires the "experimental-bar" feature gate +// Bar is an experimental field that provides enhanced functionality. +// It requires the "experimental-bar" feature gate to be enabled. +// When disabled, this field is ignored during reconciliation. // +feature-gate experimental-bar // +optional Bar *string `json:"bar,omitempty"` @@ -101,81 +139,161 @@ Bar *string `json:"bar,omitempty"` ### Gradual Rollout Strategy -1. **Alpha**: Feature behind feature gate -2. **Beta**: Feature enabled by default, gate for opt-out +1. **Alpha**: Feature behind feature gate (disabled by default) + ```go + // +feature-gate experimental-feature + ExperimentalField *string `json:"experimentalField,omitempty"` + ``` + +2. **Beta**: Feature available but can be disabled + ```go + // +feature-gate beta-feature + BetaField *string `json:"betaField,omitempty"` + ``` + 3. **Stable**: Remove feature gate, feature always available + ```go + // No feature gate marker - always available + StableField string `json:"stableField"` + ``` -## Future Integration +### Testing -When [controller-tools supports feature gates](https://github.com/kubernetes-sigs/controller-tools/issues/1238), you'll be able to use: +Test your controller with different feature gate configurations: -```go -// +kubebuilder:feature-gate=experimental-bar -// +optional -Bar *string `json:"bar,omitempty"` -``` - -This will automatically exclude the field from CRD schemas when the feature gate is disabled. +```bash +# Test with all features disabled (default) +make test -## Examples +# Test with specific features enabled +FEATURE_GATES="experimental-bar=true" make test -### Basic Example +# Test with all features enabled +FEATURE_GATES="experimental-bar=true,advanced-features=true" make test +``` -```go -type CronJobSpec struct { - // Standard fields - Schedule string `json:"schedule"` - - // Experimental timezone support - // +feature-gate timezone-support - Timezone *string `json:"timezone,omitempty"` -} +## Deployment Configurations + +### Development Environment + +```yaml +# config/manager/manager.yaml +apiVersion: apps/v1 +kind: Deployment +metadata: + name: controller-manager +spec: + template: + spec: + containers: + - name: manager + args: + - --leader-elect + - --feature-gates=experimental-bar=true,advanced-features=true ``` -### Controller Logic +### Production Environment + +```yaml +# Production - only stable features +apiVersion: apps/v1 +kind: Deployment +metadata: + name: controller-manager +spec: + template: + spec: + containers: + - name: manager + args: + - --leader-elect + - --feature-gates=experimental-bar=false,advanced-features=false +``` -```go -func (r *CronJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - // Check if timezone feature is enabled - if featureGates.IsEnabled("timezone-support") { - // Use timezone-aware scheduling - return r.reconcileWithTimezone(ctx, req) - } - - // Fall back to standard scheduling - return r.reconcileStandard(ctx, req) -} +### Canary Deployment + +```yaml +# Canary deployment with experimental features +apiVersion: apps/v1 +kind: Deployment +metadata: + name: controller-manager-canary +spec: + replicas: 1 + template: + spec: + containers: + - name: manager + args: + - --leader-elect + - --feature-gates=experimental-bar=true ``` ## Troubleshooting ### Common Issues -1. **Feature gate not discovered**: Ensure the `+feature-gate` marker is on the line before the field -2. **Invalid feature gate name**: Use lowercase with hyphens only -3. **Validation errors**: Check that all specified gates are available +1. **Feature gate not discovered** + - Ensure the `+feature-gate` marker is on the line immediately before the field + - Re-run `kubebuilder create api` to regenerate feature gate files + - Check that the marker follows the correct format: `// +feature-gate gate-name` + +2. **Invalid feature gate name** + - Use only lowercase letters, numbers, and hyphens + - Examples: `experimental-bar`, `alpha-feature-v2` + +3. **Validation errors** + - Check that all specified gates are discovered in your API types + - Verify syntax: `gate1=true,gate2=false` (no spaces around `=`) + +4. **Controller not recognizing feature gates** + - Ensure `cmd/main.go` includes the generated feature gate initialization + - Verify that `internal/featuregates/featuregates.go` is properly generated ### Debugging -Enable debug logging to see feature gate discovery: +Enable debug logging to see feature gate discovery and validation: ```bash -./manager --feature-gates=experimental-bar --zap-log-level=debug +# See feature gate initialization +./manager --feature-gates=experimental-bar=true --zap-log-level=debug + +# Check available gates +./manager --help | grep feature-gates +``` + +### Verification + +Verify your feature gates are working: + +```bash +# List all available feature gates +grep -r "+feature-gate" api/ + +# Check generated feature gate file +cat internal/featuregates/featuregates.go + +# Verify controller initialization +grep -A 5 -B 5 "feature-gates" cmd/main.go ``` ## Implementation Status -### Production Ready +### ✅ Production Ready -- Feature gate discovery and validation -- Controller integration with `--feature-gates` flag -- Scaffolding integration for new projects +- Feature gate discovery from API type markers +- CLI flag `--feature-gates` for runtime control +- Automatic scaffolding integration +- Validation and error handling +- Controller runtime integration -### Future Enhancement +### 🚧 Future Enhancement -- CRD schema modification (requires [controller-tools support](https://github.com/kubernetes-sigs/controller-tools/issues/1238)) +- **CRD schema modification**: Requires [controller-tools support](https://github.com/kubernetes-sigs/controller-tools/issues/1238) +- **Helm chart integration**: Dynamic feature gate configuration in Helm values +- **Operator lifecycle management**: Feature gate coordination across multiple controllers -When [controller-tools supports feature gates](https://github.com/kubernetes-sigs/controller-tools/issues/1238), you'll be able to use: +When controller-tools gains feature gate support, you'll be able to use: ```go // +kubebuilder:feature-gate=experimental-bar @@ -183,4 +301,72 @@ When [controller-tools supports feature gates](https://github.com/kubernetes-sig Bar *string `json:"bar,omitempty"` ``` -This will automatically exclude the field from CRD schemas when the feature gate is disabled. \ No newline at end of file +This will automatically exclude the field from CRD schemas when the feature gate is disabled, providing true schema-level gating. + +## Examples + +### Complete Working Example + +Here's a complete example showing feature gates in action: + +```go +// api/v1/webapp_types.go +type WebAppSpec struct { + // Core functionality - always available + Image string `json:"image"` + Replicas int32 `json:"replicas"` + + // Experimental auto-scaling + // +feature-gate auto-scaling + // +optional + AutoScaling *AutoScalingConfig `json:"autoScaling,omitempty"` + + // Beta feature - advanced routing + // +feature-gate advanced-routing + // +optional + AdvancedRouting *RoutingConfig `json:"advancedRouting,omitempty"` +} + +type AutoScalingConfig struct { + MinReplicas int32 `json:"minReplicas"` + MaxReplicas int32 `json:"maxReplicas"` + TargetCPU int32 `json:"targetCPU"` +} + +type RoutingConfig struct { + Rules []RoutingRule `json:"rules"` +} +``` + +```go +// controllers/webapp_controller.go +func (r *WebAppReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := log.FromContext(ctx) + + var webapp webappv1.WebApp + if err := r.Get(ctx, req.NamespacedName, &webapp); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + + // Handle auto-scaling if enabled + if featureGates.IsEnabled("auto-scaling") && webapp.Spec.AutoScaling != nil { + log.Info("Configuring auto-scaling", "min", webapp.Spec.AutoScaling.MinReplicas) + if err := r.reconcileAutoScaling(ctx, &webapp); err != nil { + return ctrl.Result{}, err + } + } + + // Handle advanced routing if enabled + if featureGates.IsEnabled("advanced-routing") && webapp.Spec.AdvancedRouting != nil { + log.Info("Configuring advanced routing") + if err := r.reconcileAdvancedRouting(ctx, &webapp); err != nil { + return ctrl.Result{}, err + } + } + + // Core reconciliation logic + return r.reconcileCore(ctx, &webapp) +} +``` + +This example demonstrates how feature gates enable experimental functionality while maintaining backward compatibility. \ No newline at end of file diff --git a/hack/docs/internal/cronjob-tutorial/generate_cronjob.go b/hack/docs/internal/cronjob-tutorial/generate_cronjob.go index 4e5023f3de0..de2726cb0ca 100644 --- a/hack/docs/internal/cronjob-tutorial/generate_cronjob.go +++ b/hack/docs/internal/cronjob-tutorial/generate_cronjob.go @@ -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) } diff --git a/hack/docs/internal/multiversion-tutorial/generate_multiversion.go b/hack/docs/internal/multiversion-tutorial/generate_multiversion.go index 29e5e5f710f..d506b7deac0 100644 --- a/hack/docs/internal/multiversion-tutorial/generate_multiversion.go +++ b/hack/docs/internal/multiversion-tutorial/generate_multiversion.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( diff --git a/pkg/machinery/featuregate.go b/pkg/machinery/featuregate.go index f5875e166af..674f4dea218 100644 --- a/pkg/machinery/featuregate.go +++ b/pkg/machinery/featuregate.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 diff --git a/pkg/plugins/golang/v4/scaffolds/api.go b/pkg/plugins/golang/v4/scaffolds/api.go index 2507d9ebd81..fb78c14574f 100644 --- a/pkg/plugins/golang/v4/scaffolds/api.go +++ b/pkg/plugins/golang/v4/scaffolds/api.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,7 +174,7 @@ 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" @@ -145,21 +182,54 @@ func (s *apiScaffolder) discoverFeatureGates() []string { 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) } diff --git a/pkg/plugins/golang/v4/scaffolds/api_test.go b/pkg/plugins/golang/v4/scaffolds/api_test.go index 3f2b0f94452..e94daad4d02 100644 --- a/pkg/plugins/golang/v4/scaffolds/api_test.go +++ b/pkg/plugins/golang/v4/scaffolds/api_test.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) + } } } diff --git a/pkg/plugins/golang/v4/scaffolds/internal/templates/api/types.go b/pkg/plugins/golang/v4/scaffolds/internal/templates/api/types.go index 2f045aeb199..538694fb5d5 100644 --- a/pkg/plugins/golang/v4/scaffolds/internal/templates/api/types.go +++ b/pkg/plugins/golang/v4/scaffolds/internal/templates/api/types.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 }}. diff --git a/pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/main.go b/pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/main.go index 4232b2dff60..68f6c69ac1c 100644 --- a/pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/main.go +++ b/pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/main.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, diff --git a/pkg/plugins/golang/v4/scaffolds/internal/templates/makefile.go b/pkg/plugins/golang/v4/scaffolds/internal/templates/makefile.go index 3f795dd5beb..fe36e1b34f5 100644 --- a/pkg/plugins/golang/v4/scaffolds/internal/templates/makefile.go +++ b/pkg/plugins/golang/v4/scaffolds/internal/templates/makefile.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. diff --git a/test/e2e/featuregates/featuregates_test.go b/test/e2e/featuregates/featuregates_test.go new file mode 100644 index 00000000000..bf9319ae554 --- /dev/null +++ b/test/e2e/featuregates/featuregates_test.go @@ -0,0 +1,217 @@ +/* +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 featuregates + +import ( + "os" + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "sigs.k8s.io/kubebuilder/v4/pkg/machinery" +) + +func TestFeatureGates(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Feature Gates Unit Tests Suite") +} + +var _ = Describe("Feature Gate Parsing", func() { + It("should parse valid boolean feature gate values", func() { + gates, err := machinery.ParseFeatureGates("test-gate=true,another-gate=false") + Expect(err).NotTo(HaveOccurred()) + Expect(gates).To(HaveKeyWithValue("test-gate", true)) + Expect(gates).To(HaveKeyWithValue("another-gate", false)) + }) + + It("should reject invalid feature gate values", func() { + _, err := machinery.ParseFeatureGates("test-gate=maybe") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("must be 'true' or 'false'")) + }) + + It("should handle empty input", func() { + gates, err := machinery.ParseFeatureGates("") + Expect(err).NotTo(HaveOccurred()) + Expect(gates).To(BeEmpty()) + }) + + It("should reject malformed input", func() { + // Test various malformed inputs + invalidInputs := []string{ + "InvalidFormat", // CamelCase not allowed + "invalid_underscore", // Underscores not allowed + "invalid.dot", // Dots not allowed + "invalid space", // Spaces not allowed + "=true", // Empty name + "feature=", // Empty value + "feature=maybe", // Invalid boolean value + } + + for _, input := range invalidInputs { + _, err := machinery.ParseFeatureGates(input) + Expect(err).To(HaveOccurred(), "Expected error for input: %s", input) + } + }) +}) + +var _ = Describe("Feature Gate Marker Discovery", func() { + var parser *machinery.FeatureGateMarkerParser + + BeforeEach(func() { + parser = machinery.NewFeatureGateMarkerParser() + }) + + It("should discover feature gate markers in Go source files", func() { + // Create a temporary directory and file for testing + tempDir := GinkgoT().TempDir() + testFile := tempDir + "/test_types.go" + + source := `package v1 + +type TestSpec struct { + // Regular field without feature gate + Name string ` + "`json:\"name\"`" + ` + + // +feature-gate experimental-field + // +optional + ExperimentalField *string ` + "`json:\"experimentalField,omitempty\"`" + ` + + // +feature-gate another-experimental + // This field requires the another-experimental gate to be enabled + // +optional + AnotherField *int ` + "`json:\"anotherField,omitempty\"`" + ` +} +` + err := os.WriteFile(testFile, []byte(source), 0644) + Expect(err).NotTo(HaveOccurred()) + + markers, err := parser.ParseFile(testFile) + Expect(err).NotTo(HaveOccurred()) + Expect(markers).To(HaveLen(2)) + + gates := machinery.ExtractFeatureGates(markers) + Expect(gates).To(ContainElements("experimental-field", "another-experimental")) + }) + + It("should handle source code without feature gates", func() { + // Create a temporary directory and file for testing + tempDir := GinkgoT().TempDir() + testFile := tempDir + "/test_types.go" + + source := `package v1 + +type TestSpec struct { + Name string ` + "`json:\"name\"`" + ` + Count int ` + "`json:\"count\"`" + ` +} +` + err := os.WriteFile(testFile, []byte(source), 0644) + Expect(err).NotTo(HaveOccurred()) + + markers, err := parser.ParseFile(testFile) + Expect(err).NotTo(HaveOccurred()) + Expect(markers).To(BeEmpty()) + + gates := machinery.ExtractFeatureGates(markers) + Expect(gates).To(BeEmpty()) + }) + + It("should ignore commented out feature gate markers", func() { + // Create a temporary directory and file for testing + tempDir := GinkgoT().TempDir() + testFile := tempDir + "/test_types.go" + + source := `package v1 + +type TestSpec struct { + // This is a comment about feature gates but not a marker + // +feature-gate experimental-field + ActiveField string ` + "`json:\"activeField\"`" + ` + + // // +feature-gate commented-out-gate + // This feature gate marker is commented out + InactiveField string ` + "`json:\"inactiveField\"`" + ` +} +` + err := os.WriteFile(testFile, []byte(source), 0644) + Expect(err).NotTo(HaveOccurred()) + + markers, err := parser.ParseFile(testFile) + Expect(err).NotTo(HaveOccurred()) + Expect(markers).To(HaveLen(1)) + + gates := machinery.ExtractFeatureGates(markers) + Expect(gates).To(ContainElement("experimental-field")) + Expect(gates).NotTo(ContainElement("commented-out-gate")) + }) +}) + +var _ = Describe("Feature Gate Validation", func() { + It("should validate feature gate names", func() { + validNames := []string{ + "valid-name", + "another-valid-name", + "alpha-feature", + "beta-api", + "feature1", + "a", + "experimental-feature-v2", + } + + for _, name := range validNames { + gates, err := machinery.ParseFeatureGates(name + "=true") + Expect(err).NotTo(HaveOccurred(), "Expected %s to be valid", name) + Expect(gates).To(HaveKeyWithValue(name, true)) + } + }) + + It("should reject invalid feature gate names", func() { + invalidNames := []string{ + "InvalidCamelCase", // CamelCase not allowed + "invalid_underscore", // Underscores not allowed + "invalid.dot", // Dots not allowed + "invalid space", // Spaces not allowed + "UPPERCASE", // Uppercase not allowed + "feature-", // Cannot end with hyphen + "-feature", // Cannot start with hyphen + "feature--name", // Double hyphens not allowed + "123-feature", // Cannot start with number + } + + for _, name := range invalidNames { + _, err := machinery.ParseFeatureGates(name + "=true") + Expect(err).To(HaveOccurred(), "Expected error for name: %s", name) + Expect(err.Error()).To(ContainSubstring("invalid feature gate name")) + } + }) + + It("should reject truly invalid formats", func() { + invalidInputs := []string{ + "=true", // Empty name + "feature=maybe", // Invalid boolean value + "feature=", // Empty value + "valid-feature=TRUE,invalid_name=true", // Mix of valid and invalid + } + + for _, input := range invalidInputs { + _, err := machinery.ParseFeatureGates(input) + Expect(err).To(HaveOccurred(), "Expected error for input: %s", input) + } + }) +}) diff --git a/test/e2e/v4/featuregates_discovery_test.go b/test/e2e/v4/featuregates_discovery_test.go index 8a3c17d6984..1c06eac11bb 100644 --- a/test/e2e/v4/featuregates_discovery_test.go +++ b/test/e2e/v4/featuregates_discovery_test.go @@ -185,8 +185,8 @@ type TestStruct struct { "--group", kbc.Group, "--version", kbc.Version, "--kind", kbc.Kind, + "--resource", "--controller", "--make=false", - "--manifests=false", ) Expect(err).NotTo(HaveOccurred(), "Failed to create API definition") @@ -196,8 +196,8 @@ type TestStruct struct { // Add a feature gate marker to the spec err = util.InsertCode( apiTypesFile, - "//+kubebuilder:validation:Optional", - "\n\t// +feature-gate experimental-feature", + "// +optional\n\tFoo *string `json:\"foo,omitempty\"`", + "\n\n\t// +feature-gate experimental-feature", ) Expect(err).NotTo(HaveOccurred(), "Failed to add feature gate marker") @@ -210,7 +210,14 @@ type TestStruct struct { Expect(err).NotTo(HaveOccurred(), "Failed to add experimental field") By("regenerating the project to discover feature gates") - err = kbc.Regenerate() + err = kbc.CreateAPI( + "--group", kbc.Group, + "--version", kbc.Version, + "--kind", kbc.Kind, + "--resource", "--controller", + "--make=false", + "--force", + ) Expect(err).NotTo(HaveOccurred(), "Failed to regenerate project") By("verifying feature gates file was generated") diff --git a/test/e2e/v4/featuregates_test.go b/test/e2e/v4/featuregates_test.go index f0b128f57c5..8a45e97a01b 100644 --- a/test/e2e/v4/featuregates_test.go +++ b/test/e2e/v4/featuregates_test.go @@ -20,75 +20,71 @@ import ( "os" "path/filepath" "strings" - "testing" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + pluginutil "sigs.k8s.io/kubebuilder/v4/pkg/plugin/util" "sigs.k8s.io/kubebuilder/v4/test/e2e/utils" ) -func TestFeatureGates(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "Feature Gates Suite") -} - var _ = Describe("Feature Gates", func() { Context("project with feature gates", func() { var ( - ctx *utils.TestContext + kbc *utils.TestContext ) BeforeEach(func() { var err error - ctx, err = utils.NewTestContext("go") + kbc, err = utils.NewTestContext(pluginutil.KubebuilderBinName, "GO111MODULE=on") Expect(err).NotTo(HaveOccurred()) - Expect(ctx).NotTo(BeNil()) + Expect(kbc).NotTo(BeNil()) - // Set up test context - ctx.Domain = "example.com" - ctx.Group = "crew" - ctx.Version = "v1" - ctx.Kind = "Captain" - ctx.Resources = "captains" + kbc.Domain = "example.com" + kbc.Group = "crew" + kbc.Version = "v1" + kbc.Kind = "Captain" - // Prepare the test environment - err = ctx.Prepare() - Expect(err).NotTo(HaveOccurred()) + By("preparing the test environment") + Expect(kbc.Prepare()).To(Succeed()) }) AfterEach(func() { - ctx.Destroy() + By("destroying directory") + kbc.Destroy() }) It("should scaffold a project with feature gates", func() { - // Initialize the project - err := ctx.Init() + By("initializing a project") + err := kbc.Init( + "--domain", kbc.Domain, + ) Expect(err).NotTo(HaveOccurred()) - // Create API with feature gates - err = ctx.CreateAPI( - "--group", ctx.Group, - "--version", ctx.Version, - "--kind", ctx.Kind, + By("creating API with feature gates") + err = kbc.CreateAPI( + "--group", kbc.Group, + "--version", kbc.Version, + "--kind", kbc.Kind, "--resource", "--controller", + "--make=false", ) Expect(err).NotTo(HaveOccurred()) - // Verify feature gates file was generated - featureGatesFile := filepath.Join(ctx.Dir, "internal", "featuregates", "featuregates.go") + By("verifying feature gates file was generated") + featureGatesFile := filepath.Join(kbc.Dir, "internal", "featuregates", "featuregates.go") Expect(featureGatesFile).To(BeAnExistingFile()) - // Verify main.go has feature gates flag - mainFile := filepath.Join(ctx.Dir, "cmd", "main.go") + By("verifying main.go has feature gates flag") + mainFile := filepath.Join(kbc.Dir, "cmd", "main.go") Expect(mainFile).To(BeAnExistingFile()) mainContent, err := os.ReadFile(mainFile) Expect(err).NotTo(HaveOccurred()) Expect(string(mainContent)).To(ContainSubstring("--feature-gates")) Expect(string(mainContent)).To(ContainSubstring("featuregates")) - // Verify API types have feature gate example - typesFile := filepath.Join(ctx.Dir, "api", ctx.Version, ctx.Kind+"_types.go") + By("verifying API types have feature gate example") + typesFile := filepath.Join(kbc.Dir, "api", kbc.Version, strings.ToLower(kbc.Kind)+"_types.go") Expect(typesFile).To(BeAnExistingFile()) typesContent, err := os.ReadFile(typesFile) Expect(err).NotTo(HaveOccurred()) @@ -96,52 +92,52 @@ var _ = Describe("Feature Gates", func() { }) It("should discover feature gates from API types", func() { - // Initialize the project - err := ctx.Init() + By("initializing a project") + err := kbc.Init( + "--domain", kbc.Domain, + ) Expect(err).NotTo(HaveOccurred()) - // Create API - err = ctx.CreateAPI( - "--group", ctx.Group, - "--version", ctx.Version, - "--kind", ctx.Kind, + By("creating API") + err = kbc.CreateAPI( + "--group", kbc.Group, + "--version", kbc.Version, + "--kind", kbc.Kind, "--resource", "--controller", + "--make=false", ) Expect(err).NotTo(HaveOccurred()) - // Add custom feature gates to the API types - typesFile := filepath.Join(ctx.Dir, "api", ctx.Version, ctx.Kind+"_types.go") - typesContent, err := os.ReadFile(typesFile) - Expect(err).NotTo(HaveOccurred()) + By("adding custom feature gates to the API types") + typesFile := filepath.Join(kbc.Dir, "api", kbc.Version, strings.ToLower(kbc.Kind)+"_types.go") - // Add a new field with feature gate + // Add a new field with feature gate using the utility function newField := ` // Experimental field that requires the "experimental-field" feature gate // +feature-gate experimental-field // +optional - ExperimentalField *string ` + "`" + `json:"experimentalField,omitempty"` + "`" + ` -` - // Insert the new field in the Spec struct - updatedContent := strings.Replace(string(typesContent), - "// Bar *string ` + \"`\" + `json:\"bar,omitempty\"` + \"`\" + `", - "// Bar *string ` + \"`\" + `json:\"bar,omitempty\"` + \"`\" + `\n"+newField, - 1) - - err = os.WriteFile(typesFile, []byte(updatedContent), 0644) - Expect(err).NotTo(HaveOccurred()) - - // Regenerate the project to discover new feature gates - err = ctx.CreateAPI( - "--group", ctx.Group, - "--version", ctx.Version, - "--kind", ctx.Kind, + ExperimentalField *string ` + "`" + `json:"experimentalField,omitempty"` + "`" + + err = pluginutil.InsertCode( + typesFile, + "Bar *string `json:\"bar,omitempty\"`", + newField, + ) + Expect(err).NotTo(HaveOccurred()) + + By("regenerating the project to discover new feature gates") + err = kbc.CreateAPI( + "--group", kbc.Group, + "--version", kbc.Version, + "--kind", kbc.Kind, "--resource", "--controller", "--force", + "--make=false", ) Expect(err).NotTo(HaveOccurred()) - // Verify the new feature gate was discovered - featureGatesFile := filepath.Join(ctx.Dir, "internal", "featuregates", "featuregates.go") + By("verifying the new feature gate was discovered") + featureGatesFile := filepath.Join(kbc.Dir, "internal", "featuregates", "featuregates.go") Expect(featureGatesFile).To(BeAnExistingFile()) featureGatesContent, err := os.ReadFile(featureGatesFile) Expect(err).NotTo(HaveOccurred()) @@ -149,49 +145,55 @@ var _ = Describe("Feature Gates", func() { }) It("should build project with feature gates", func() { - // Initialize the project - err := ctx.Init() + By("initializing a project") + err := kbc.Init( + "--domain", kbc.Domain, + ) Expect(err).NotTo(HaveOccurred()) - // Create API - err = ctx.CreateAPI( - "--group", ctx.Group, - "--version", ctx.Version, - "--kind", ctx.Kind, + By("creating API") + err = kbc.CreateAPI( + "--group", kbc.Group, + "--version", kbc.Version, + "--kind", kbc.Kind, "--resource", "--controller", + "--make=false", ) Expect(err).NotTo(HaveOccurred()) - // Build the project - this should succeed with feature gates - err = ctx.Make("build") + By("building the project - this should succeed with feature gates") + err = kbc.Make("build") Expect(err).NotTo(HaveOccurred()) - // Verify the binary was created - binaryPath := filepath.Join(ctx.Dir, "bin", "manager") + By("verifying the binary was created") + binaryPath := filepath.Join(kbc.Dir, "bin", "manager") Expect(binaryPath).To(BeAnExistingFile()) }) It("should generate manifests with feature gates", func() { - // Initialize the project - err := ctx.Init() + By("initializing a project") + err := kbc.Init( + "--domain", kbc.Domain, + ) Expect(err).NotTo(HaveOccurred()) - // Create API - err = ctx.CreateAPI( - "--group", ctx.Group, - "--version", ctx.Version, - "--kind", ctx.Kind, + By("creating API") + err = kbc.CreateAPI( + "--group", kbc.Group, + "--version", kbc.Version, + "--kind", kbc.Kind, "--resource", "--controller", + "--make=false", ) Expect(err).NotTo(HaveOccurred()) - // Generate manifests - err = ctx.Make("manifests") + By("generating manifests") + err = kbc.Make("manifests") Expect(err).NotTo(HaveOccurred()) - // Verify CRDs were generated - crdFile := filepath.Join(ctx.Dir, "config", "crd", "bases", - strings.ToLower(ctx.Group)+"_"+ctx.Version+"_"+strings.ToLower(ctx.Kind)+".yaml") + By("verifying CRDs were generated") + crdFile := filepath.Join(kbc.Dir, "config", "crd", "bases", + strings.ToLower(kbc.Group)+"."+kbc.Domain+"_captains.yaml") Expect(crdFile).To(BeAnExistingFile()) }) }) diff --git a/test/features.sh b/test/features.sh index f5d48d9a611..420c992c2f3 100755 --- a/test/features.sh +++ b/test/features.sh @@ -37,4 +37,7 @@ go test "$(dirname "$0")/e2e/alphagenerate" ${flags:-} -timeout 30m header_text "Running Alpha Update Command E2E tests" go test "$(dirname "$0")/e2e/alphaupdate" ${flags:-} -timeout 30m +header_text "Running Feature Gates Unit tests" +go test "$(dirname "$0")/e2e/featuregates" ${flags:-} -timeout 30m + popd >/dev/null From 1105b60f13519da77f1fa3a3a6a06106b629da9e Mon Sep 17 00:00:00 2001 From: wazery Date: Mon, 11 Aug 2025 13:40:16 +0200 Subject: [PATCH 07/11] feat: make feature gate infrastructure optional This change improves the UX by making feature gate infrastructure opt-in rather than always generated. Key changes: - Added --with-feature-gates flag to init and create api commands - Made feature gate scaffolding conditional based on flag or auto-detection - Updated main.go template to conditionally include feature gate code - Added auto-detection for existing feature gate infrastructure - Created FeatureGateScaffolder interface for proper type handling The infrastructure is now only generated when: 1. Explicitly requested via --with-feature-gates flag, or 2. Auto-detected when existing feature gate infrastructure is found This addresses PR feedback suggesting that feature gate infrastructure should be optional to improve user experience. --- pkg/plugins/golang/v4/api.go | 22 +++++++++++++ pkg/plugins/golang/v4/init.go | 9 +++++ pkg/plugins/golang/v4/scaffolds/api.go | 33 +++++++++++++++---- pkg/plugins/golang/v4/scaffolds/init.go | 31 ++++++++++++----- pkg/plugins/golang/v4/scaffolds/interfaces.go | 6 ++++ .../scaffolds/internal/templates/cmd/main.go | 9 +++++ 6 files changed, 94 insertions(+), 16 deletions(-) create mode 100644 pkg/plugins/golang/v4/scaffolds/interfaces.go diff --git a/pkg/plugins/golang/v4/api.go b/pkg/plugins/golang/v4/api.go index 3747dc9de7d..94ef6521bc9 100644 --- a/pkg/plugins/golang/v4/api.go +++ b/pkg/plugins/golang/v4/api.go @@ -50,11 +50,17 @@ type createAPISubcommand struct { resourceFlag *pflag.Flag controllerFlag *pflag.Flag + // featureGateFlag holds the flag for feature gates + featureGateFlag *pflag.Flag + // force indicates that the resource should be created even if it already exists force bool // runMake indicates whether to run make or not after scaffolding APIs runMake bool + + // withFeatureGates indicates whether to include feature gate support + withFeatureGates bool } func (p *createAPISubcommand) UpdateMetadata(cliMeta plugin.CLIMetadata, subcmdMeta *plugin.SubcommandMetadata) { @@ -96,6 +102,10 @@ func (p *createAPISubcommand) BindFlags(fs *pflag.FlagSet) { fs.BoolVar(&p.force, "force", false, "attempt to create resource even if it already exists") + fs.BoolVar(&p.withFeatureGates, "with-feature-gates", false, + "if specified, include feature gate support in scaffolded files") + p.featureGateFlag = fs.Lookup("with-feature-gates") + p.options = &goPlugin.Options{} fs.StringVar(&p.options.Plural, "plural", "", "resource irregular plural form") @@ -147,6 +157,13 @@ func (p *createAPISubcommand) InjectResource(res *resource.Resource) error { p.options.UpdateResource(p.resource, p.config) + // Auto-detect feature gates if not explicitly set and infrastructure exists + if !p.featureGateFlag.Changed { + if _, err := os.Stat("internal/featuregates/featuregates.go"); err == nil { + p.withFeatureGates = true + } + } + if err := p.resource.Validate(); err != nil { return fmt.Errorf("error validating resource: %w", err) } @@ -180,6 +197,11 @@ func (p *createAPISubcommand) PreScaffold(machinery.Filesystem) error { func (p *createAPISubcommand) Scaffold(fs machinery.Filesystem) error { scaffolder := scaffolds.NewAPIScaffolder(p.config, *p.resource, p.force) scaffolder.InjectFS(fs) + + // Set feature gates flag if the scaffolder supports it + if fgScaffolder, ok := scaffolder.(scaffolds.FeatureGateScaffolder); ok { + fgScaffolder.SetWithFeatureGates(p.withFeatureGates) + } if err := scaffolder.Scaffold(); err != nil { return fmt.Errorf("error scaffolding API: %w", err) } diff --git a/pkg/plugins/golang/v4/init.go b/pkg/plugins/golang/v4/init.go index 278f5f3bdf3..8611cc47db6 100644 --- a/pkg/plugins/golang/v4/init.go +++ b/pkg/plugins/golang/v4/init.go @@ -57,6 +57,7 @@ type initSubcommand struct { // flags fetchDeps bool skipGoVersionCheck bool + withFeatureGates bool } func (p *initSubcommand) UpdateMetadata(cliMeta plugin.CLIMetadata, subcmdMeta *plugin.SubcommandMetadata) { @@ -84,6 +85,10 @@ func (p *initSubcommand) BindFlags(fs *pflag.FlagSet) { // dependency args fs.BoolVar(&p.fetchDeps, "fetch-deps", true, "ensure dependencies are downloaded") + // feature gate args + fs.BoolVar(&p.withFeatureGates, "with-feature-gates", false, + "if specified, scaffold feature gate infrastructure for experimental functionality") + // boilerplate args fs.StringVar(&p.license, "license", "apache2", "license to use to boilerplate, may be one of 'apache2', 'none'") @@ -128,6 +133,10 @@ func (p *initSubcommand) PreScaffold(machinery.Filesystem) error { func (p *initSubcommand) Scaffold(fs machinery.Filesystem) error { scaffolder := scaffolds.NewInitScaffolder(p.config, p.license, p.owner, p.commandName) scaffolder.InjectFS(fs) + // Set feature gates flag if the scaffolder supports it + if fgScaffolder, ok := scaffolder.(scaffolds.FeatureGateScaffolder); ok { + fgScaffolder.SetWithFeatureGates(p.withFeatureGates) + } if err := scaffolder.Scaffold(); err != nil { return fmt.Errorf("error scaffolding init plugin: %w", err) } diff --git a/pkg/plugins/golang/v4/scaffolds/api.go b/pkg/plugins/golang/v4/scaffolds/api.go index fb78c14574f..ac1d69d331d 100644 --- a/pkg/plugins/golang/v4/scaffolds/api.go +++ b/pkg/plugins/golang/v4/scaffolds/api.go @@ -48,6 +48,9 @@ type apiScaffolder struct { // force indicates whether to scaffold controller files even if it exists or not force bool + + // withFeatureGates indicates whether to include feature gate support + withFeatureGates bool } // NewAPIScaffolder returns a new Scaffolder for API/controller creation operations @@ -64,6 +67,11 @@ func (s *apiScaffolder) InjectFS(fs machinery.Filesystem) { s.fs = fs } +// SetWithFeatureGates sets whether to include feature gate support +func (s *apiScaffolder) SetWithFeatureGates(withFeatureGates bool) { + s.withFeatureGates = withFeatureGates +} + // Scaffold implements cmdutil.Scaffolder func (s *apiScaffolder) Scaffold() error { slog.Info("Writing scaffold for you to edit...") @@ -98,6 +106,14 @@ func (s *apiScaffolder) Scaffold() error { return fmt.Errorf("error updating resource: %w", err) } + // Check if feature gates infrastructure already exists + existingFeatureGatesFile := filepath.Join("internal", "featuregates", "featuregates.go") + if _, err := os.Stat(existingFeatureGatesFile); err == nil { + // Feature gates infrastructure already exists, enable support + s.withFeatureGates = true + slog.Debug("Detected existing feature gates infrastructure, enabling support") + } + // If using --force, discover existing feature gates before overwriting files var existingGates []string if s.force && doAPI { @@ -108,7 +124,7 @@ func (s *apiScaffolder) Scaffold() error { if err := scaffold.Execute( &api.Types{ Force: s.force, - IncludeFeatureGateExample: false, // Don't include by default - keep it simple + IncludeFeatureGateExample: s.withFeatureGates, }, &api.Group{}, ); err != nil { @@ -155,12 +171,15 @@ func (s *apiScaffolder) Scaffold() error { availableGates = newGates } - // Generate feature gates file - 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) + // Only generate feature gates infrastructure if requested or if gates were discovered + if s.withFeatureGates || len(availableGates) > 0 { + // Generate feature gates file + 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) + } } if err := scaffold.Execute( diff --git a/pkg/plugins/golang/v4/scaffolds/init.go b/pkg/plugins/golang/v4/scaffolds/init.go index 3f5dc3c7989..5dd557a3af7 100644 --- a/pkg/plugins/golang/v4/scaffolds/init.go +++ b/pkg/plugins/golang/v4/scaffolds/init.go @@ -53,11 +53,12 @@ var _ plugins.Scaffolder = &initScaffolder{} var kustomizeVersion string type initScaffolder struct { - config config.Config - boilerplatePath string - license string - owner string - commandName string + config config.Config + boilerplatePath string + license string + owner string + commandName string + withFeatureGates bool // fs is the filesystem that will be used by the scaffolder fs machinery.Filesystem @@ -79,6 +80,11 @@ func (s *initScaffolder) InjectFS(fs machinery.Filesystem) { s.fs = fs } +// SetWithFeatureGates sets whether to scaffold feature gate infrastructure +func (s *initScaffolder) SetWithFeatureGates(withFeatureGates bool) { + s.withFeatureGates = withFeatureGates +} + // getControllerRuntimeReleaseBranch converts the ControllerRuntime semantic versioning string to a // release branch string. Example input: "v0.17.0" -> Output: "release-0.17" func getControllerRuntimeReleaseBranch() string { @@ -155,11 +161,12 @@ func (s *initScaffolder) Scaffold() error { } } - err := scaffold.Execute( + // Build the list of templates to scaffold + scaffoldFiles := []machinery.Builder{ &cmd.Main{ ControllerRuntimeVersion: ControllerRuntimeVersion, + WithFeatureGates: s.withFeatureGates, }, - &cmd.FeatureGates{AvailableGates: []string{}}, &templates.GoMod{ ControllerRuntimeVersion: ControllerRuntimeVersion, }, @@ -178,7 +185,6 @@ func (s *initScaffolder) Scaffold() error { &templates.Readme{CommandName: s.commandName}, &templates.Golangci{}, &e2e.Test{}, - &e2e.WebhookTestUpdater{WireWebhook: false}, &e2e.SuiteTest{}, &github.E2eTestCi{}, &github.TestCi{}, @@ -188,7 +194,14 @@ func (s *initScaffolder) Scaffold() error { &utils.Utils{}, &templates.DevContainer{}, &templates.DevContainerPostInstallScript{}, - ) + } + + // Only add feature gates template if requested + if s.withFeatureGates { + scaffoldFiles = append(scaffoldFiles, &cmd.FeatureGates{AvailableGates: []string{}}) + } + + err := scaffold.Execute(scaffoldFiles...) if err != nil { return fmt.Errorf("failed to execute init scaffold: %w", err) } diff --git a/pkg/plugins/golang/v4/scaffolds/interfaces.go b/pkg/plugins/golang/v4/scaffolds/interfaces.go new file mode 100644 index 00000000000..3673b480fcc --- /dev/null +++ b/pkg/plugins/golang/v4/scaffolds/interfaces.go @@ -0,0 +1,6 @@ +package scaffolds + +// FeatureGateScaffolder interface for scaffolders that support feature gates +type FeatureGateScaffolder interface { + SetWithFeatureGates(bool) +} diff --git a/pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/main.go b/pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/main.go index 68f6c69ac1c..fb5a96547ba 100644 --- a/pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/main.go +++ b/pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/main.go @@ -35,6 +35,7 @@ type Main struct { machinery.RepositoryMixin ControllerRuntimeVersion string + WithFeatureGates bool } // SetTemplateDefaults implements machinery.Template @@ -248,8 +249,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/metrics/filters" "sigs.k8s.io/controller-runtime/pkg/webhook" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + {{- if .WithFeatureGates }} featuregates "{{ .Repo }}/internal/featuregates" + {{- end }} %s ) @@ -273,7 +276,9 @@ func main() { var probeAddr string var secureMetrics bool var enableHTTP2 bool + {{- if .WithFeatureGates }} var featureGates string + {{- end }} var tlsOpts []func(*tls.Config) flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address the metrics endpoint binds to. " + "Use :8443 for HTTPS or :8080 for HTTP, or leave as 0 to disable the metrics service.") @@ -292,15 +297,18 @@ func main() { flag.StringVar(&metricsCertKey, "metrics-cert-key", "tls.key", "The name of the metrics server key file.") flag.BoolVar(&enableHTTP2, "enable-http2", false, "If set, HTTP/2 will be enabled for the metrics and webhook servers") + {{- if .WithFeatureGates }} 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()) + {{- end }} opts := zap.Options{ Development: true, } opts.BindFlags(flag.CommandLine) flag.Parse() + {{- if .WithFeatureGates }} // Parse feature gates parsedFeatureGates, err := featuregates.ParseFeatureGates(featureGates) if err != nil { @@ -313,6 +321,7 @@ func main() { setupLog.Error(err, "invalid feature gates") os.Exit(1) } + {{- end }} ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) From cc5623267f2409704342d586fa1250c5f4310594 Mon Sep 17 00:00:00 2001 From: wazery Date: Mon, 11 Aug 2025 13:45:19 +0200 Subject: [PATCH 08/11] docs: update documentation for optional feature gate infrastructure Updated documentation to reflect the new optional feature gate infrastructure: - Updated feature-gates.md to explain optional nature and new CLI flags - Added note about --with-feature-gates flag in quick-start.md - Added examples to CLI help text for both init and create api commands - Clarified auto-detection behavior and when infrastructure is generated - Updated troubleshooting section with new flag usage The documentation now clearly explains: 1. Feature gates are opt-in rather than always generated 2. Three ways to enable: explicit flags, auto-detection, or marker discovery 3. Better UX through conditional scaffolding --- docs/book/src/quick-start.md | 17 +++++++ .../src/reference/markers/feature-gates.md | 50 ++++++++++++++++--- pkg/plugins/golang/v4/api.go | 3 ++ pkg/plugins/golang/v4/init.go | 3 ++ 4 files changed, 65 insertions(+), 8 deletions(-) diff --git a/docs/book/src/quick-start.md b/docs/book/src/quick-start.md index bdedbf7377c..58672f8c5f1 100644 --- a/docs/book/src/quick-start.md +++ b/docs/book/src/quick-start.md @@ -74,6 +74,23 @@ Run the following command to create a new API (group/version) as `webapp/v1` and kubebuilder create api --group webapp --version v1 --kind Guestbook ``` + +