fix(bundler): re-enable aws-ebs-csi-driver by default and support --set disable (#397)

Signed-off-by: Yuan Chen <yuanchen97@gmail.com>
Co-authored-by: Brian Lockwood <lockwobr@gmail.com>
Co-authored-by: Mark Chmarny <mchmarny@users.noreply.github.com>

Author: 3 people

docs/user/cli-reference.md

@@ -632,7 +632,7 @@ aicr bundle [flags]
 | `--output` | `-o` | string | Output directory (default: current dir) |
 | `--deployer` | | string | Deployment method: helm (default), argocd |
 | `--repo` | | string | Git repository URL for ArgoCD applications (only used with `--deployer argocd`) |
-| `--set` | | string[] | Override values in bundle files (repeatable) |
+| `--set` | | string[] | Override values in bundle files (repeatable). Use `enabled` key to include/exclude components (e.g., `--set awsebscsidriver:enabled=false`) |
 | `--data` | | string | External data directory to overlay on embedded data (see [External Data](#external-data-directory)) |
 | `--system-node-selector` | | string[] | Node selector for system components (format: key=value, repeatable) |
 | `--system-node-toleration` | | string[] | Toleration for system components (format: key=value:effect, repeatable) |
@@ -717,6 +717,7 @@ Override any value in the generated bundle files using dot notation:
 - **Duplicate keys**: When the same `bundler:path` is specified multiple times, the **last value wins**
 - **Array values**: Individual array elements cannot be overridden (no `[0]` index syntax). Arrays can only be replaced entirely via recipe overrides, not via `--set` flags. Use recipe-level overrides in `componentRefs[].overrides` if you need to replace an entire array.
 - **Type conversion**: String values are automatically converted to appropriate types (`true`/`false` → bool, numeric strings → numbers)
+- **Component enable/disable**: The special `enabled` key controls whether a component is included in the bundle. `--set <component>:enabled=false` excludes the component; `--set <component>:enabled=true` re-enables a recipe-disabled component. The `enabled` key is consumed by the bundler and not passed to Helm chart values.
 
 **Examples:**
 ```shell
@@ -748,6 +749,11 @@ aicr bundle -r recipe.yaml \
   --set skyhook-operator:manager.resources.memory.limit=256Mi \
   -o ./bundles
 
+# Disable a component at bundle time (e.g., EBS CSI already installed as EKS addon)
+aicr bundle -r recipe.yaml \
+  --set awsebscsidriver:enabled=false \
+  -o ./bundles
+
 # Schedule system components on specific node pool
 aicr bundle -r recipe.yaml \
   --system-node-selector nodeGroup=system-pool \

pkg/bundler/bundler.go

@@ -188,11 +188,21 @@ func (b *DefaultBundler) Make(ctx context.Context, input recipe.RecipeInput, dir
 			"recipe must contain at least one component reference")
 	}
 
-	// Filter out disabled components (overrides.enabled: false)
+	// Filter out disabled components.
+	// Check order: --set overrides take precedence over recipe overrides.
+	// This allows users to enable/disable components at bundle time:
+	//   --set awsebscsidriver:enabled=false  (disable)
+	//   --set awsebscsidriver:enabled=true   (re-enable)
 	enabledRefs := make([]recipe.ComponentRef, 0, len(recipeResult.ComponentRefs))
 	enabledSet := make(map[string]struct{})
 	for _, ref := range recipeResult.ComponentRefs {
-		if !ref.IsEnabled() {
+		if setEnabled, ok := b.getSetEnabledOverride(ref.Name); ok {
+			if !setEnabled {
+				slog.Info("skipping component disabled via --set", "component", ref.Name)
+				continue
+			}
+			// --set enabled=true overrides recipe-level disabled
+		} else if !ref.IsEnabled() {
 			slog.Info("skipping disabled component", "component", ref.Name)
 			continue
 		}
@@ -441,8 +451,19 @@ func (b *DefaultBundler) extractComponentValues(ctx context.Context, recipeResul
 			values = make(map[string]any)
 		}
 
-		// Apply user value overrides from --set flags
+		// Apply user value overrides from --set flags.
+		// Strip "enabled" key — it controls component inclusion, not Helm chart values.
 		if overrides := b.getValueOverridesForComponent(ref.Name); len(overrides) > 0 {
+			if _, has := overrides["enabled"]; has {
+				filtered := make(map[string]string, len(overrides)-1)
+				for k, v := range overrides {
+					if k == "enabled" {
+						continue
+					}
+					filtered[k] = v
+				}
+				overrides = filtered
+			}
 			if applyErr := component.ApplyMapOverrides(values, overrides); applyErr != nil {
 				slog.Warn("failed to apply some value overrides",
 					"component", ref.Name,
@@ -506,6 +527,27 @@ func (b *DefaultBundler) getValueOverridesForComponent(componentName string) map
 	return nil
 }
 
+// getSetEnabledOverride checks if --set overrides contain an "enabled" key
+// for the given component. Returns (value, true) if found, (false, false) otherwise.
+// This allows --set awsebscsidriver:enabled=false to disable a component at bundle time.
+func (b *DefaultBundler) getSetEnabledOverride(componentName string) (bool, bool) {
+	overrides := b.getValueOverridesForComponent(componentName)
+	if overrides == nil {
+		return false, false
+	}
+	val, ok := overrides["enabled"]
+	if !ok {
+		return false, false
+	}
+	parsed, parseErr := strconv.ParseBool(val)
+	if parseErr != nil {
+		slog.Warn("invalid --set enabled value, ignoring override",
+			"component", componentName, "value", val, "error", parseErr)
+		return false, false
+	}
+	return parsed, true
+}
+
 // applyNodeSchedulingOverrides applies node selectors and tolerations to component values.
 // Uses the component registry to determine the correct paths for each component.
 func (b *DefaultBundler) applyNodeSchedulingOverrides(componentName string, values map[string]any) {

pkg/bundler/bundler_test.go

@@ -335,6 +335,150 @@ func TestMake_DisabledComponentsFiltered(t *testing.T) {
 	}
 }
 
+func TestMake_SetEnabledOverridesPrecedence(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name           string
+		recipeEnabled  *bool // nil = no override, true/false = overrides.enabled
+		setEnabled     string
+		expectIncluded bool
+	}{
+		{
+			name:           "recipe disabled + --set enabled=true => included",
+			recipeEnabled:  boolPtr(false),
+			setEnabled:     "true",
+			expectIncluded: true,
+		},
+		{
+			name:           "recipe enabled + --set enabled=false => excluded",
+			recipeEnabled:  nil,
+			setEnabled:     "false",
+			expectIncluded: false,
+		},
+		{
+			name:           "recipe disabled + no --set => excluded",
+			recipeEnabled:  boolPtr(false),
+			setEnabled:     "",
+			expectIncluded: false,
+		},
+		{
+			name:           "recipe enabled (default) + no --set => included",
+			recipeEnabled:  nil,
+			setEnabled:     "",
+			expectIncluded: true,
+		},
+		{
+			name:           "invalid --set value ignored => falls back to recipe",
+			recipeEnabled:  boolPtr(false),
+			setEnabled:     "ture",
+			expectIncluded: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			var bundlerOpts []Option
+			if tt.setEnabled != "" {
+				cfg := config.NewConfig(
+					config.WithValueOverrides(map[string]map[string]string{
+						"awsebscsidriver": {"enabled": tt.setEnabled},
+					}),
+				)
+				bundlerOpts = append(bundlerOpts, WithConfig(cfg))
+			}
+
+			bundler, err := New(bundlerOpts...)
+			if err != nil {
+				t.Fatalf("New() error = %v", err)
+			}
+
+			overrides := map[string]any{}
+			if tt.recipeEnabled != nil {
+				overrides["enabled"] = *tt.recipeEnabled
+			}
+
+			recipeResult := &recipe.RecipeResult{
+				APIVersion: "aicr.nvidia.com/v1alpha1",
+				Kind:       "Recipe",
+				Criteria:   &recipe.Criteria{Service: "eks", Accelerator: "h100", Intent: "training"},
+				ComponentRefs: []recipe.ComponentRef{
+					{Name: "gpu-operator", Version: "v25.3.3", Type: "helm", Source: "https://helm.ngc.nvidia.com/nvidia"},
+					{Name: "aws-ebs-csi-driver", Version: "2.55.0", Type: "helm", Source: "https://kubernetes-sigs.github.io/aws-ebs-csi-driver", Overrides: overrides},
+				},
+				DeploymentOrder: []string{"gpu-operator", "aws-ebs-csi-driver"},
+			}
+
+			ctx := context.Background()
+			tmpDir := t.TempDir()
+			_, makeErr := bundler.Make(ctx, recipeResult, tmpDir)
+			if makeErr != nil {
+				t.Fatalf("Make() error = %v", makeErr)
+			}
+
+			_, statErr := os.Stat(filepath.Join(tmpDir, "aws-ebs-csi-driver"))
+			included := !os.IsNotExist(statErr)
+
+			if included != tt.expectIncluded {
+				t.Errorf("aws-ebs-csi-driver included=%v, want %v", included, tt.expectIncluded)
+			}
+		})
+	}
+}
+
+func TestMake_SetEnabledNotLeakedToHelmValues(t *testing.T) {
+	t.Parallel()
+
+	cfg := config.NewConfig(
+		config.WithValueOverrides(map[string]map[string]string{
+			"awsebscsidriver": {"enabled": "true", "controller.replicaCount": "2"},
+		}),
+	)
+	bundler, err := New(WithConfig(cfg))
+	if err != nil {
+		t.Fatalf("New() error = %v", err)
+	}
+
+	recipeResult := &recipe.RecipeResult{
+		APIVersion: "aicr.nvidia.com/v1alpha1",
+		Kind:       "Recipe",
+		Criteria:   &recipe.Criteria{Service: "eks", Accelerator: "h100", Intent: "training"},
+		ComponentRefs: []recipe.ComponentRef{
+			{Name: "gpu-operator", Version: "v25.3.3", Type: "helm", Source: "https://helm.ngc.nvidia.com/nvidia"},
+			{Name: "aws-ebs-csi-driver", Version: "2.55.0", Type: "helm", Source: "https://kubernetes-sigs.github.io/aws-ebs-csi-driver"},
+		},
+		DeploymentOrder: []string{"gpu-operator", "aws-ebs-csi-driver"},
+	}
+
+	ctx := context.Background()
+	tmpDir := t.TempDir()
+	_, makeErr := bundler.Make(ctx, recipeResult, tmpDir)
+	if makeErr != nil {
+		t.Fatalf("Make() error = %v", makeErr)
+	}
+
+	valuesPath := filepath.Join(tmpDir, "aws-ebs-csi-driver", "values.yaml")
+	valuesData, readErr := os.ReadFile(valuesPath)
+	if readErr != nil {
+		t.Fatalf("failed to read values.yaml: %v", readErr)
+	}
+
+	// "enabled" must not appear as a top-level key in the values file
+	valuesStr := string(valuesData)
+	if strings.Contains(valuesStr, "enabled: true") {
+		t.Errorf("enabled key leaked into Helm values:\n%s", valuesStr)
+	}
+
+	// Other overrides should still be applied
+	if !strings.Contains(valuesStr, "replicaCount") {
+		t.Errorf("expected controller.replicaCount override in values, got:\n%s", valuesStr)
+	}
+}
+
+func boolPtr(b bool) *bool { return &b }
+
 func TestMake_WithValueOverrides(t *testing.T) {
 	cfg := config.NewConfig(
 		config.WithValueOverrides(map[string]map[string]string{

pkg/cli/bundle.go

@@ -258,7 +258,9 @@ Package with explicit tag (overrides CLI version):
 			&cli.StringSliceFlag{
 				Name: "set",
 				Usage: `Override values in generated bundle files
-	(format: bundler:path.to.field=value, e.g., --set gpuoperator:gds.enabled=true)`,
+	(format: component:path.to.field=value, e.g., --set gpuoperator:gds.enabled=true).
+	Use the special 'enabled' key to include/exclude components at bundle time
+	(e.g., --set awsebscsidriver:enabled=false to skip aws-ebs-csi-driver)`,
 				Category: "Deployment",
 			},
 			&cli.StringSliceFlag{

recipes/overlays/eks.yaml

@@ -32,17 +32,14 @@ spec:
 
   # EKS-specific components
   componentRefs:
-    # Disabled by default: EKS clusters typically have EBS CSI driver
-    # installed as a managed addon. Deploying it again causes ClusterRole
-    # conflicts ("managed-by: EKS" vs "managed-by: Helm").
-    # Users can re-enable with: --set aws-ebs-csi-driver.enabled=true
+    # EBS CSI driver for persistent volume provisioning on EKS.
+    # Enabled by default since EKS does not include it as a managed addon.
+    # Disable with: --set awsebscsidriver:enabled=false (if installed as EKS addon)
     - name: aws-ebs-csi-driver
       type: Helm
       source: https://kubernetes-sigs.github.io/aws-ebs-csi-driver
       version: 2.55.0
       valuesFile: components/aws-ebs-csi-driver/values.yaml
-      overrides:
-        enabled: false
 
     # Enable Prometheus persistent storage for EKS (requires EBS CSI driver)
     - name: kube-prometheus-stack