Skip to content

Commit bab4a01

Browse files
committed
cmdline: improve robustness of parseCmdline and tests
- Use strings.Fields instead of strings.Split for proper whitespace handling - Use errors.Is for wrapped error comparison (context.DeadlineExceeded) - Rename local url variable to parsedURL to avoid shadowing net/url import - Check f.Close() error in test helper - Check url.Parse error in test assertion Signed-off-by: Andrew Dodds <andrew.dodds@sap.com>
1 parent 79fc035 commit bab4a01

2 files changed

Lines changed: 15 additions & 25 deletions

File tree

internal/providers/cmdline/cmdline.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ package cmdline
2020

2121
import (
2222
"context"
23+
"errors"
2324
"fmt"
2425
"net/url"
2526
"os"
@@ -28,7 +29,7 @@ import (
2829
"strings"
2930
"time"
3031

31-
"github.com/coreos/ignition/v2/config/shared/errors"
32+
configErrors "github.com/coreos/ignition/v2/config/shared/errors"
3233
"github.com/coreos/ignition/v2/config/v3_7_experimental/types"
3334
"github.com/coreos/ignition/v2/internal/distro"
3435
"github.com/coreos/ignition/v2/internal/log"
@@ -101,7 +102,7 @@ func parseCmdline(logger *log.Logger, path string) (*cmdlineOpts, error) {
101102

102103
opts := &cmdlineOpts{}
103104

104-
for _, arg := range strings.Split(string(cmdline), " ") {
105+
for _, arg := range strings.Fields(string(cmdline)) {
105106
parts := strings.SplitN(strings.TrimSpace(arg), "=", 2)
106107
if len(parts) != 2 {
107108
continue
@@ -117,12 +118,12 @@ func parseCmdline(logger *log.Logger, path string) (*cmdlineOpts, error) {
117118
continue
118119
}
119120

120-
url, err := url.Parse(value)
121+
parsedURL, err := url.Parse(value)
121122
if err != nil {
122123
logger.Err("failed to parse url: %v", err)
123124
continue
124125
}
125-
opts.Url = url
126+
opts.Url = parsedURL
126127
case flagDeviceLabel:
127128
if value == "" {
128129
logger.Info("device label flag found but no value provided")
@@ -146,16 +147,16 @@ func fetchConfigFromDevice(logger *log.Logger, opts *cmdlineOpts) (types.Config,
146147
defer cancel()
147148

148149
data, err := tryMounting(logger, ctx, opts)
149-
if err == context.DeadlineExceeded {
150+
if errors.Is(err, context.DeadlineExceeded) {
150151
logger.Info("disk was not available in time. Continuing without a config...")
151-
return types.Config{}, report.Report{}, errors.ErrEmpty
152+
return types.Config{}, report.Report{}, configErrors.ErrEmpty
152153
}
153154
if err != nil {
154155
return types.Config{}, report.Report{}, err
155156
}
156157
if data == nil {
157158
logger.Info("config file %q not found on device. Continuing without config...", opts.UserDataPath)
158-
return types.Config{}, report.Report{}, errors.ErrEmpty
159+
return types.Config{}, report.Report{}, configErrors.ErrEmpty
159160
}
160161

161162
return util.ParseConfig(logger, data)

internal/providers/cmdline/cmdline_test.go

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ package cmdline
1717
import (
1818
"net/url"
1919
"os"
20-
"path/filepath"
2120
"testing"
2221

2322
"github.com/coreos/ignition/v2/internal/log"
@@ -39,7 +38,9 @@ func writeCmdline(t *testing.T, content string) string {
3938
if _, err := f.WriteString(content); err != nil {
4039
t.Fatal(err)
4140
}
42-
f.Close()
41+
if err := f.Close(); err != nil {
42+
t.Fatal(err)
43+
}
4344
return f.Name()
4445
}
4546

@@ -50,7 +51,10 @@ func TestParseCmdlineURL(t *testing.T) {
5051
if err != nil {
5152
t.Fatalf("unexpected error: %v", err)
5253
}
53-
expected, _ := url.Parse("https://example.com/config.ign")
54+
expected, err := url.Parse("https://example.com/config.ign")
55+
if err != nil {
56+
t.Fatalf("failed to parse expected URL: %v", err)
57+
}
5458
if opts.Url == nil || opts.Url.String() != expected.String() {
5559
t.Errorf("expected URL %q, got %v", expected, opts.Url)
5660
}
@@ -172,18 +176,3 @@ func TestParseCmdlineURLTakesPrecedence(t *testing.T) {
172176
t.Errorf("expected UserDataPath %q, got %q", "/config.ign", opts.UserDataPath)
173177
}
174178
}
175-
176-
func TestFileExists(t *testing.T) {
177-
dir := t.TempDir()
178-
existing := filepath.Join(dir, "file.txt")
179-
if err := os.WriteFile(existing, []byte("x"), 0600); err != nil {
180-
t.Fatal(err)
181-
}
182-
183-
if !fileExists(existing) {
184-
t.Errorf("expected fileExists(%q) = true", existing)
185-
}
186-
if fileExists(filepath.Join(dir, "nonexistent.txt")) {
187-
t.Errorf("expected fileExists on missing file = false")
188-
}
189-
}

0 commit comments

Comments
 (0)