Skip to content

fix(config): Fix flag parsing with space-separated value #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adamwg merged 2 commits into from
May 8, 2026
+87 −1
Merged

fix(config): Fix flag parsing with space-separated value #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f44f2dc
fix(config): Fix flag parsing with space-separated value
tampakrap May 8, 2026
9de9449
fix(config): reject empty --config= value, add unittest for configFlag
tampakrap May 8, 2026
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
5 changes: 4 additions & 1 deletion cmd/crossplane/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,10 @@ func configFlag(args []string) (string, error) {
continue
}

if v := strings.TrimPrefix(a, "--config="); v != "" {
if v, ok := strings.CutPrefix(a, "--config="); ok {
if v == "" {
return "", errors.New("flag --config requires a value")
}
Comment on lines +144 to +147
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 8, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Please confirm breaking-change labeling for --config= behavior change

Thanks for adding explicit validation. Since this removes prior behavior (--config= no longer falls back), can you confirm whether this PR should carry a breaking-change label (or document why this is exempt)?

As per coding guidelines Do not remove behavior in apis/** or cmd/** without labeling as 'breaking-change'.

Also applies to: 155-155

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/crossplane/main.go` around lines 144 - 147, This change makes the flag
parsing for "--config=" (the strings.CutPrefix(a, "--config=") branch that
checks v == "") strictly require a value and removes the previous empty-fallback
behavior; update the PR to either add the 'breaking-change' label to reflect the
user-visible behavior change or add a clear note in the release notes/changelog
and cmd/* documentation explaining why the previous fallback was intentionally
removed and why this is not considered a breaking change (also confirm the
identical change at the other occurrence handling "--config=").

return v, nil
}

Expand Down
83 changes: 83 additions & 0 deletions cmd/crossplane/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
Copyright 2026 The Crossplane 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 main

import (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
)

func TestConfigFlag(t *testing.T) {
type want struct {
path string
err error
}

cases := map[string]struct {
reason string
args []string
want want
}{
"Empty": {
reason: "No args should yield no path and no error.",
args: nil,
},
"NoConfigFlag": {
reason: "Argv without --config should yield no path and no error.",
args: []string{"version"},
},
"EqualsForm": {
reason: "--config=PATH should return PATH.",
args: []string{"--config=/tmp/config.yaml"},
want: want{path: "/tmp/config.yaml"},
},
"SpaceForm": {
reason: "--config PATH should return PATH from the next argv.",
args: []string{"--config", "/tmp/config.yaml"},
want: want{path: "/tmp/config.yaml"},
},
"EmptyEquals": {
reason: "--config= is an explicitly empty value and should return an error.",
args: []string{"--config="},
want: want{err: cmpopts.AnyError},
},
"MissingValue": {
reason: "Trailing --config with no following argv should return an error.",
args: []string{"--config"},
want: want{err: cmpopts.AnyError},
},
"FirstWins": {
reason: "If --config appears more than once, the first occurrence wins.",
args: []string{"--config=/first.yaml", "--config=/second.yaml"},
want: want{path: "/first.yaml"},
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
got, err := configFlag(tc.args)
if diff := cmp.Diff(tc.want.err, err, cmpopts.EquateErrors()); diff != "" {
t.Errorf("\n%s\nconfigFlag(%v): -want err, +got err:\n%s", tc.reason, tc.args, diff)
}
if diff := cmp.Diff(tc.want.path, got); diff != "" {
t.Errorf("\n%s\nconfigFlag(%v): -want, +got:\n%s", tc.reason, tc.args, diff)
}
})
}
}
Loading