From 357b71b17fdf7a44a35287df9283c95ed1a55922 Mon Sep 17 00:00:00 2001 From: rogueslasher Date: Sat, 13 Jun 2026 17:23:13 +0530 Subject: [PATCH] fix: preserve split-form (--flag value) user-managed broker-router command flags Signed-off-by: rogueslasher --- internal/controller/broker_router.go | 72 +++++++++++++++++++++----- internal/controller/deployment_test.go | 38 ++++++++++++++ 2 files changed, 96 insertions(+), 14 deletions(-) diff --git a/internal/controller/broker_router.go b/internal/controller/broker_router.go index d4f36b3a8..29e26238f 100644 --- a/internal/controller/broker_router.go +++ b/internal/controller/broker_router.go @@ -534,14 +534,60 @@ func deploymentNeedsUpdate(desired, existing *appsv1.Deployment) (bool, string) return false, "" } -// filterManagedFlags returns only the binary name and flags the controller manages. +// commandEntry represents a single logical entry in a container command: +// either the binary name, a "--flag=value" token, or a "--flag value" pair +// spanning two tokens. +type commandEntry struct { + tokens []string + flagName string +} + +// extractFlagName extracts the flag name from a "--flag" or "--flag=value" token. +func extractFlagName(arg string) string { + if idx := strings.Index(arg, "="); idx != -1 { + return arg[:idx] + } + return arg +} + +// isManagedFlag reports whether name (e.g. "--log-level") is a controller-managed flag. +func isManagedFlag(name string) bool { + return slices.Contains(managedCommandFlags, name) +} + +// parseCommandEntries splits a container command into logical entries, +// grouping "--flag value" split-form flags into a single entry with their +// value token. +func parseCommandEntries(command []string) []commandEntry { + var entries []commandEntry + for i := 0; i < len(command); i++ { + arg := command[i] + if !strings.HasPrefix(arg, "--") { + entries = append(entries, commandEntry{tokens: []string{arg}}) + continue + } + name := extractFlagName(arg) + if strings.Contains(arg, "=") { + entries = append(entries, commandEntry{tokens: []string{arg}, flagName: name}) + continue + } + if i+1 < len(command) && !strings.HasPrefix(command[i+1], "--") { + entries = append(entries, commandEntry{tokens: []string{arg, command[i+1]}, flagName: name}) + i++ + continue + } + entries = append(entries, commandEntry{tokens: []string{arg}, flagName: name}) + } + return entries +} + +// filterManagedFlags returns the binary name and the controller-managed flags +// (with their values) as a flat token slice. func filterManagedFlags(command []string) []string { var out []string - for _, arg := range command { - if !strings.HasPrefix(arg, "--") || slices.ContainsFunc(managedCommandFlags, func(flag string) bool { - return strings.HasPrefix(arg, flag) - }) { - out = append(out, arg) + for _, entry := range parseCommandEntries(command) { + if entry.flagName == "" || isManagedFlag(entry.flagName) { + out = append(out, entry.tokens...) } } return out @@ -549,18 +595,16 @@ func filterManagedFlags(command []string) []string { // mergeCommand takes the desired command from the controller and the existing // command from the deployment. It returns a merged command that preserves any -// user-added flags while updating controller-managed flags. +// user-added flags (including split "--flag value" entries) while updating +// controller-managed flags. func mergeCommand(desired, existing []string) []string { - // start with all user flags from the existing command var userFlags []string - for _, arg := range existing { - if !strings.HasPrefix(arg, "--") { + for _, entry := range parseCommandEntries(existing) { + if entry.flagName == "" { continue } - if !slices.ContainsFunc(managedCommandFlags, func(flag string) bool { - return strings.HasPrefix(arg, flag) - }) { - userFlags = append(userFlags, arg) + if !isManagedFlag(entry.flagName) { + userFlags = append(userFlags, entry.tokens...) } } return slices.Concat(desired, userFlags) diff --git a/internal/controller/deployment_test.go b/internal/controller/deployment_test.go index 885615297..008c957d8 100644 --- a/internal/controller/deployment_test.go +++ b/internal/controller/deployment_test.go @@ -190,6 +190,16 @@ func TestDeploymentNeedsUpdate(t *testing.T) { }, expected: false, }, + { + name: "user-managed split-form flag does not trigger update", + modify: func(d *appsv1.Deployment) { + d.Spec.Template.Spec.Containers[0].Command = append( + d.Spec.Template.Spec.Containers[0].Command, + "--session-length", "3600", + ) + }, + expected: false, + }, { name: "env var added", modify: func(d *appsv1.Deployment) { @@ -1431,6 +1441,16 @@ func TestFilterManagedFlags(t *testing.T) { command: []string{}, want: nil, }, + { + name: "user split-form flag with value stripped", + command: []string{"./mcp_gateway", "--mcp-broker-public-address=0.0.0.0:8080", "--session-length", "3600"}, + want: []string{"./mcp_gateway", "--mcp-broker-public-address=0.0.0.0:8080"}, + }, + { + name: "managed split-form flag with value kept as a unit", + command: []string{"./mcp_gateway", "--mcp-broker-public-address", "0.0.0.0:8080", "--log-level=-4"}, + want: []string{"./mcp_gateway", "--mcp-broker-public-address", "0.0.0.0:8080", "--log-level=-4"}, + }, } for _, tt := range tests { @@ -1497,6 +1517,24 @@ func TestMergeCommand(t *testing.T) { existing: []string{"./mcp_gateway"}, want: []string{"./mcp_gateway", "--mcp-broker-public-address=0.0.0.0:8080"}, }, + { + name: "preserves user split-form flag with value", + desired: []string{"./mcp_gateway", "--mcp-broker-public-address=0.0.0.0:8080"}, + existing: []string{"./mcp_gateway", "--mcp-broker-public-address=0.0.0.0:8080", "--session-length", "3600"}, + want: []string{"./mcp_gateway", "--mcp-broker-public-address=0.0.0.0:8080", "--session-length", "3600"}, + }, + { + name: "preserves mixed managed equals-form and user split-form flags", + desired: []string{"./mcp_gateway", "--mcp-broker-public-address=0.0.0.0:8080", "--log-level=-4"}, + existing: []string{"./mcp_gateway", "--mcp-broker-public-address=0.0.0.0:8080", "--log-level=0", "--session-length", "3600"}, + want: []string{"./mcp_gateway", "--mcp-broker-public-address=0.0.0.0:8080", "--log-level=-4", "--session-length", "3600"}, + }, + { + name: "managed split-form flag replaced by desired equals-form value", + desired: []string{"./mcp_gateway", "--mcp-broker-public-address=0.0.0.0:9090"}, + existing: []string{"./mcp_gateway", "--mcp-broker-public-address", "0.0.0.0:8080", "--session-length", "3600"}, + want: []string{"./mcp_gateway", "--mcp-broker-public-address=0.0.0.0:9090", "--session-length", "3600"}, + }, } for _, tt := range tests {