Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 58 additions & 14 deletions internal/controller/broker_router.go
Original file line number Diff line number Diff line change
Expand Up @@ -534,33 +534,77 @@ 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 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function name flagName collides with the struct field commandEntry.flagName — both live in the same file and share the identifier. Consider renaming to extractFlagName or parseFlagName to make them distinct at a glance.

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
}

// 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)
Expand Down
38 changes: 38 additions & 0 deletions internal/controller/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
Loading