✨ Undeprecate plugin optionals

cmd/transform/optionals/optionals.go

@@ -47,9 +47,8 @@ func NewOptionalsCommand(f *flags.GlobalFlags) *cobra.Command {
 		cobraGlobalFlags: f,
 	}
 	cmd := &cobra.Command{
-		Use:        "optionals",
-		Short:      "Return a list of optional fields accepted by configured plugins",
-		Deprecated: "use custom stages with kustomization patches instead. Optional flags apply globally to all stages and will be removed in a future version.",
+		Use:   "optionals",
+		Short: "Return a list of optional fields accepted by configured plugins",
 		RunE: func(c *cobra.Command, args []string) error {
 			if err := o.Complete(c, args); err != nil {
 				return err

cmd/transform/optionals_test.go

@@ -149,7 +149,10 @@ func TestOptionalFlagsToLower(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			result := optionalFlagsToLower(tt.input)
+			result, err := optionalFlagsToLowerChecked(tt.input)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
 			assertMapEquals(t, tt.expected, result)
 		})
 	}

cmd/transform/transform.go

@@ -45,6 +45,7 @@ type Flags struct {
 	TransformDir      string   `mapstructure:"transform-dir"`
 	SkipPlugins       []string `mapstructure:"skip-plugins"`
 	OptionalFlags     string   `mapstructure:"optional-flags"`
+	StageOptionals    []string `mapstructure:"stage-optionals"`
 	Overwrite         bool     `mapstructure:"overwrite"`
 	// Kustomize arguments
 	KustomizeArgs string `mapstructure:"kustomize-args"`
@@ -159,9 +160,8 @@ func addFlagsForOptions(o *Flags, cmd *cobra.Command) {
 	cmd.Flags().StringVar(&o.InstructionsFile, "instructions-file", "", "Path to the transform instructions file")
 	cmd.Flags().BoolVar(&o.Overwrite, "overwrite", false, "Overwrite existing stage directories even if they contain user modifications")
 
-	// Deprecated: optional-flags will be removed in a future version
-	cmd.Flags().StringVar(&o.OptionalFlags, "optional-flags", "", "(DEPRECATED) JSON string holding flag value pairs to be passed to all plugins. Use custom stages with kustomization instead. (ie. '{\"foo-flag\": \"foo-a=/data,foo-b=/data\", \"bar-flag\": \"bar-value\"}')")
-	cmd.Flags().MarkDeprecated("optional-flags", "use custom stages with kustomization patches instead. This flag applies globally to all stages and will be removed in a future version.")
+	cmd.Flags().StringVar(&o.OptionalFlags, "optional-flags", "", "JSON string holding flag value pairs to be passed to all plugins (e.g. '{\"registry-replacement\": \"docker.io=quay.io\"}')")
+	cmd.Flags().StringArrayVar(&o.StageOptionals, "stage-optionals", nil, "Per-stage optional flags as StageName=JSON, repeatable (e.g. --stage-optionals 'KubernetesPlugin={\"registry-replacement\":\"docker.io=quay.io\"}')")
 
 	// Kustomize arguments
 	cmd.Flags().StringVar(&o.KustomizeArgs, "kustomize-args", "", "Additional arguments for kustomize (e.g., '--enable-helm --helm-command=helm3')")
@@ -190,10 +190,15 @@ func (o *Options) run() error {
 		return err
 	}
 
-	if o.InstructionsFile != "" && len(o.RequestedStages) > 0 { // instructions file and positional args are mutually exclusive
+	if o.InstructionsFile != "" && len(o.RequestedStages) > 0 {
 		return fmt.Errorf("use either --instructions-file or positional stage arguments, not both")
 	}
+	if o.InstructionsFile != "" && len(o.StageOptionals) > 0 {
+		return fmt.Errorf("use either --instructions-file or --stage-optionals, not both")
+	}
+
 	var instructionStages []string
+	var instructionStageOptionals map[string]map[string]string
 	if o.InstructionsFile != "" {
 		instructionsFilePath, err := filepath.Abs(o.InstructionsFile)
 		if err != nil {
@@ -203,7 +208,11 @@ func (o *Options) run() error {
 		if err != nil {
 			return err
 		}
-		instructionStages = internalTransform.GenerateStageDirNames(cfg.Stages)
+		instructionStages = internalTransform.GenerateStageDirNames(cfg.StageNames())
+		instructionStageOptionals, err = cfg.StageOptionals()
+		if err != nil {
+			return fmt.Errorf("invalid instructions file %q: %w", instructionsFilePath, err)
+		}
 	}
 	// Parse optional flags
 	var optionalFlags map[string]string
@@ -212,7 +221,24 @@ func (o *Options) run() error {
 		if err != nil {
 			return err
 		}
-		optionalFlags = optionalFlagsToLower(optionalFlags)
+		optionalFlags, err = optionalFlagsToLowerChecked(optionalFlags)
+		if err != nil {
+			return fmt.Errorf("invalid --optional-flags: %w", err)
+		}
+	}
+
+	// Parse per-stage optional flags from CLI
+	var stageOptionalFlags map[string]map[string]string
+	if len(o.StageOptionals) > 0 {
+		stageOptionalFlags, err = parseStageOptionals(o.StageOptionals)
+		if err != nil {
+			return err
+		}
+	}
+
+	// Use instruction file per-stage optionals if present, otherwise CLI
+	if instructionStageOptionals != nil {
+		stageOptionalFlags = instructionStageOptionals
 	}
 
 	// Parse and validate kustomize arguments
@@ -229,6 +255,7 @@ func (o *Options) run() error {
 		PluginDir:          pluginDir,
 		SkipPlugins:        o.SkipPlugins,
 		OptionalFlags:      optionalFlags,
+		StageOptionalFlags: stageOptionalFlags,
 		Overwrite:          o.Overwrite,
 		CraneVersion:       "v1.0.0", // TODO: Get from build version
 		NewlyCreatedStages: make(map[string]bool),
@@ -321,14 +348,51 @@ func (o *Options) run() error {
 	return orchestrator.RunMultiStage(selector)
 }
 
+// parseStageOptionals parses --stage-optionals values from "StageName=JSON" format
+// into a map of stage name to optional flags.
+func parseStageOptionals(values []string) (map[string]map[string]string, error) {
+	result := make(map[string]map[string]string, len(values))
+	for _, v := range values {
+		stageName, jsonStr, found := strings.Cut(v, "=")
+		if !found {
+			return nil, fmt.Errorf("invalid --stage-optionals value %q: expected format StageName=JSON", v)
+		}
+
+		if stageName == "" {
+			return nil, fmt.Errorf("invalid --stage-optionals value %q: stage name is empty", v)
+		}
+		if _, exists := result[stageName]; exists {
+			return nil, fmt.Errorf("duplicate --stage-optionals for stage %q", stageName)
+		}
+
+		var flags map[string]string
+		if err := json.Unmarshal([]byte(jsonStr), &flags); err != nil {
+			return nil, fmt.Errorf("invalid JSON in --stage-optionals for stage %q: %w", stageName, err)
+		}
+		if flags == nil {
+			return nil, fmt.Errorf("invalid JSON in --stage-optionals for stage %q: expected a JSON object, got null", stageName)
+		}
+		lower, err := optionalFlagsToLowerChecked(flags)
+		if err != nil {
+			return nil, fmt.Errorf("invalid --stage-optionals for stage %q: %w", stageName, err)
+		}
+		result[stageName] = lower
+	}
+	return result, nil
+}
+
 // Returns an extras map with lowercased keys, since any keys coming from the config file
 // are lower-cased by viper
-func optionalFlagsToLower(inFlags map[string]string) map[string]string {
-	lowerMap := make(map[string]string)
+func optionalFlagsToLowerChecked(inFlags map[string]string) (map[string]string, error) {
+	lowerMap := make(map[string]string, len(inFlags))
 	for key, val := range inFlags {
-		lowerMap[strings.ToLower(key)] = val
+		lk := strings.ToLower(key)
+		if _, exists := lowerMap[lk]; exists {
+			return nil, fmt.Errorf("duplicate optional key %q (case-insensitive collision)", lk)
+		}
+		lowerMap[lk] = val
 	}
-	return lowerMap
+	return lowerMap, nil
 }
 
 // runStageWithCleanup runs a single stage and optionally cleans up on error.

cmd/transform/transform_test.go

@@ -1088,6 +1088,166 @@ func TestValidate_ExportDir(t *testing.T) {
 	}
 }
 
+func TestParseStageOptionals(t *testing.T) {
+	tests := []struct {
+		name      string
+		values    []string
+		wantErr   bool
+		errMsg    string
+		expected  map[string]map[string]string
+	}{
+		{
+			name:   "single stage",
+			values: []string{`KubernetesPlugin={"registry-replacement": "docker.io=quay.io"}`},
+			expected: map[string]map[string]string{
+				"KubernetesPlugin": {"registry-replacement": "docker.io=quay.io"},
+			},
+		},
+		{
+			name: "multiple stages",
+			values: []string{
+				`KubernetesPlugin={"registry-replacement": "docker.io=quay.io"}`,
+				`RegistryPlugin={"registry-replacement": "quay.io=ghcr.io"}`,
+			},
+			expected: map[string]map[string]string{
+				"KubernetesPlugin": {"registry-replacement": "docker.io=quay.io"},
+				"RegistryPlugin":   {"registry-replacement": "quay.io=ghcr.io"},
+			},
+		},
+		{
+			name:    "missing equals sign",
+			values:  []string{"KubernetesPlugin"},
+			wantErr: true,
+			errMsg:  "expected format StageName=JSON",
+		},
+		{
+			name:    "empty stage name",
+			values:  []string{`={"key": "value"}`},
+			wantErr: true,
+			errMsg:  "stage name is empty",
+		},
+		{
+			name:    "malformed JSON",
+			values:  []string{`KubernetesPlugin=not-json`},
+			wantErr: true,
+			errMsg:  "invalid JSON",
+		},
+		{
+			name: "duplicate stage name",
+			values: []string{
+				`KubernetesPlugin={"key": "val1"}`,
+				`KubernetesPlugin={"key": "val2"}`,
+			},
+			wantErr: true,
+			errMsg:  "duplicate",
+		},
+		{
+			name:   "keys are lowercased",
+			values: []string{`MyPlugin={"Registry-Replacement": "docker.io=quay.io"}`},
+			expected: map[string]map[string]string{
+				"MyPlugin": {"registry-replacement": "docker.io=quay.io"},
+			},
+		},
+		{
+			name:    "null JSON value",
+			values:  []string{`KubernetesPlugin=null`},
+			wantErr: true,
+			errMsg:  "expected a JSON object",
+		},
+		{
+			name:   "JSON value containing equals sign",
+			values: []string{`MyPlugin={"registry-replacement": "docker.io=quay.io,gcr.io=ghcr.io"}`},
+			expected: map[string]map[string]string{
+				"MyPlugin": {"registry-replacement": "docker.io=quay.io,gcr.io=ghcr.io"},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result, err := parseStageOptionals(tt.values)
+			if tt.wantErr {
+				if err == nil {
+					t.Fatalf("expected error, got nil")
+				}
+				if !strings.Contains(err.Error(), tt.errMsg) {
+					t.Fatalf("expected error containing %q, got %v", tt.errMsg, err)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if len(result) != len(tt.expected) {
+				t.Fatalf("expected %d stages, got %d", len(tt.expected), len(result))
+			}
+			for stage, expectedFlags := range tt.expected {
+				actualFlags, ok := result[stage]
+				if !ok {
+					t.Errorf("missing stage %q", stage)
+					continue
+				}
+				for k, v := range expectedFlags {
+					if actualFlags[k] != v {
+						t.Errorf("stage %q key %q: expected %q, got %q", stage, k, v, actualFlags[k])
+					}
+				}
+			}
+		})
+	}
+}
+
+// Reject case-insensitive duplicate keys in --stage-optionals JSON.
+func TestParseStageOptionals_CaseInsensitiveDuplicateKey(t *testing.T) {
+	values := []string{
+		`MyPlugin={"Registry-Replacement": "docker.io=quay.io", "registry-replacement": "gcr.io=ghcr.io"}`,
+	}
+	_, err := parseStageOptionals(values)
+	if err == nil {
+		t.Fatalf("expected error for case-insensitive duplicate key, got nil")
+	}
+	if !strings.Contains(err.Error(), "duplicate optional key") {
+		t.Fatalf("expected duplicate key error, got %v", err)
+	}
+}
+
+// Regression: multi-field JSON with commas must not be split by the flag parser.
+// StringArrayVar preserves each value as-is; StringSliceVar would split on commas.
+func TestParseStageOptionals_MultiFieldJSON(t *testing.T) {
+	values := []string{
+		`KubernetesPlugin={"registry-replacement": "docker.io=quay.io", "strip-default-rbac": "false"}`,
+	}
+	result, err := parseStageOptionals(values)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	got := result["KubernetesPlugin"]
+	if got["registry-replacement"] != "docker.io=quay.io" {
+		t.Errorf("registry-replacement: expected %q, got %q", "docker.io=quay.io", got["registry-replacement"])
+	}
+	if got["strip-default-rbac"] != "false" {
+		t.Errorf("strip-default-rbac: expected %q, got %q", "false", got["strip-default-rbac"])
+	}
+}
+
+func TestRun_InstructionsFileAndStageOptionalsConflict(t *testing.T) {
+	o := &Options{
+		globalFlags: &flags.GlobalFlags{},
+		Flags: Flags{
+			InstructionsFile: "instructions.yaml",
+			StageOptionals:   []string{`KubernetesPlugin={"key": "val"}`},
+		},
+	}
+
+	err := o.run()
+	if err == nil {
+		t.Fatalf("expected conflict error, got nil")
+	}
+	if !strings.Contains(err.Error(), "use either --instructions-file or --stage-optionals, not both") {
+		t.Fatalf("unexpected error message: %v", err)
+	}
+}
+
 func TestValidate_MissingExportDir_FailsBeforeRun(t *testing.T) {
 	tmpDir := t.TempDir()
 	transformDir := filepath.Join(tmpDir, "transform")

e2e-tests/framework/crane.go

@@ -49,6 +49,7 @@ type TransformOptions struct {
 	PluginDir         string
 	SkipPlugins       []string
 	OptionalFlags     string
+	StageOptionals    []string
 	Overwrite         bool
 	KustomizeArgs     string
 	InstructionsFile  string
@@ -122,6 +123,9 @@ func (c CraneRunner) Transform(opts TransformOptions) error {
 	if opts.OptionalFlags != "" {
 		args = append(args, "--optional-flags", opts.OptionalFlags)
 	}
+	for _, so := range opts.StageOptionals {
+		args = append(args, "--stage-optionals", so)
+	}
 	if opts.Overwrite {
 		args = append(args, "--overwrite")
 	}