Skip to content

Commit cc13c98

Browse files
anisaoshafianisaoshafi
committed
Improve parsing logic
1 parent f705762 commit cc13c98

2 files changed

Lines changed: 136 additions & 32 deletions

File tree

‎internal/snapshot/destination.go‎

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,37 +9,65 @@ import (
99
"time"
1010
)
1111

12-
var ErrCloudNotSupported = errors.New("cloud destinations are not yet supported — use a file path like ./my-snapshot.zip")
12+
var (
13+
// ErrRemoteNotSupported is returned for known remote schemes (s3://, oras://, cloud:).
14+
ErrRemoteNotSupported = errors.New("remote destinations are not yet supported — coming soon")
15+
// ErrUnknownScheme is returned for unrecognized URL schemes.
16+
ErrUnknownScheme = errors.New("unrecognized destination scheme")
17+
)
1318

14-
// ParseDestination resolves the user-supplied path to an absolute local path,
15-
// or returns an error for cloud/bare names. When dest is empty, a default name
16-
// based on now (UTC) is used, e.g. "snapshot-2026-05-11T21-04-32.zip".
19+
// ParseDestination resolves the user-supplied path to an absolute local path.
20+
// When dest is empty, a default name based on now (UTC) is used, e.g.
21+
// "snapshot-2026-05-11T21-04-32.zip", saved in the current working directory.
1722
// The returned path always has a .zip extension.
1823
func ParseDestination(dest string, now time.Time) (string, error) {
1924
if dest == "" {
2025
dest = "./" + now.UTC().Format("snapshot-2006-01-02T15-04-05")
21-
} else if strings.Contains(dest, "://") {
22-
return "", ErrCloudNotSupported
23-
} else if !strings.HasPrefix(dest, ".") && !strings.HasPrefix(dest, "~") && !filepath.IsAbs(dest) && filepath.Base(dest) == dest {
24-
// bare name with no path separators: reserved for future cloud pod names
25-
return "", ErrCloudNotSupported
26+
} else {
27+
lower := strings.ToLower(dest)
28+
switch {
29+
case strings.HasPrefix(lower, "s3://"),
30+
strings.HasPrefix(lower, "oras://"),
31+
strings.HasPrefix(lower, "cloud:"):
32+
return "", ErrRemoteNotSupported
33+
case strings.Contains(lower, "://"):
34+
scheme := dest[:strings.Index(lower, "://")+3]
35+
return "", fmt.Errorf("%w: %q", ErrUnknownScheme, scheme)
36+
}
2637
}
38+
2739
if dest == "~" || strings.HasPrefix(dest, "~/") || strings.HasPrefix(dest, `~\`) {
2840
home, err := os.UserHomeDir()
2941
if err != nil {
3042
return "", fmt.Errorf("resolve home directory: %w", err)
3143
}
3244
dest = filepath.Join(home, strings.TrimLeft(dest[1:], `/\`))
3345
}
46+
3447
abs, err := filepath.Abs(dest)
3548
if err != nil {
3649
return "", fmt.Errorf("resolve path: %w", err)
3750
}
51+
52+
parent := filepath.Dir(abs)
53+
parentInfo, err := os.Stat(parent)
54+
if err != nil {
55+
if os.IsNotExist(err) {
56+
return "", fmt.Errorf("parent directory %q does not exist — create it first", parent)
57+
}
58+
return "", fmt.Errorf("check parent directory: %w", err)
59+
}
60+
if !parentInfo.IsDir() {
61+
return "", fmt.Errorf("parent path %q is not a directory", parent)
62+
}
63+
3864
if info, err := os.Stat(abs); err == nil && info.IsDir() {
39-
return "", fmt.Errorf("%q is a directory — specify a file path like ./my-snapshot.zip", abs)
65+
return "", fmt.Errorf("%q is a directory — specify a file path like ./my-snapshot", abs)
4066
}
67+
4168
if !strings.EqualFold(filepath.Ext(abs), ".zip") {
4269
abs += ".zip"
4370
}
71+
4472
return abs, nil
4573
}

‎internal/snapshot/destination_test.go‎

Lines changed: 98 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"os"
55
"path/filepath"
66
"runtime"
7+
"strings"
78
"testing"
89
"time"
910

@@ -41,14 +42,22 @@ func TestParseDestination(t *testing.T) {
4142

4243
now := time.Date(2026, 5, 11, 21, 4, 32, 0, time.UTC)
4344

45+
// Set up dirs used in path-based cases below.
46+
existingDir := t.TempDir()
47+
subDir := filepath.Join(existingDir, "subdir")
48+
require.NoError(t, os.Mkdir(subDir, 0o755))
49+
4450
type testCase struct {
45-
input string
46-
wantPath string
47-
wantErr string
48-
wantCloudErr bool
51+
name string // optional; uses input when empty
52+
input string
53+
wantPath string
54+
wantErr string
55+
wantRemoteErr bool
56+
wantSchemeErr bool
4957
}
5058

5159
tests := []testCase{
60+
// --- local paths ---
5261
{
5362
input: "./my-state",
5463
wantPath: filepath.Join(wd, "my-state.zip"),
@@ -58,16 +67,23 @@ func TestParseDestination(t *testing.T) {
5867
wantPath: filepath.Join(os.TempDir(), "state.zip"),
5968
},
6069
{
61-
input: "~",
62-
wantErr: "is a directory",
70+
input: "~",
71+
wantErr: "is a directory",
72+
},
73+
{
74+
// parent (~/) always exists
75+
input: "~/my-state",
76+
wantPath: filepath.Join(home, "my-state.zip"),
6377
},
6478
{
65-
input: "~/snapshots/s",
66-
wantPath: filepath.Join(home, "snapshots", "s.zip"),
79+
name: "relative path with existing subdir",
80+
input: filepath.Join(subDir, "state"),
81+
wantPath: filepath.Join(subDir, "state.zip"),
6782
},
6883
{
69-
input: "subdir/state",
70-
wantPath: filepath.Join(wd, "subdir", "state.zip"),
84+
// bare name: treated as relative to CWD
85+
input: "my-pod",
86+
wantPath: filepath.Join(wd, "my-pod.zip"),
7187
},
7288
{
7389
input: "./checkpoint.zip",
@@ -77,39 +93,99 @@ func TestParseDestination(t *testing.T) {
7793
input: "./already.ZIP",
7894
wantPath: filepath.Join(wd, "already.ZIP"),
7995
},
96+
97+
// --- parent directory does not exist ---
98+
{
99+
name: "parent dir missing",
100+
input: filepath.Join(existingDir, "nonexistent", "state"),
101+
wantErr: "parent directory",
102+
},
103+
104+
// --- remote: s3 ---
105+
{
106+
input: "s3://bucket/key",
107+
wantRemoteErr: true,
108+
},
109+
{
110+
input: "S3://bucket/key",
111+
wantRemoteErr: true,
112+
},
113+
114+
// --- remote: oras ---
115+
{
116+
input: "oras://registry/image",
117+
wantRemoteErr: true,
118+
},
119+
{
120+
input: "ORAS://registry/image",
121+
wantRemoteErr: true,
122+
},
123+
124+
// --- remote: cloud ---
125+
{
126+
input: "cloud:my-pod",
127+
wantRemoteErr: true,
128+
},
129+
{
130+
input: "Cloud:my-pod",
131+
wantRemoteErr: true,
132+
},
80133
{
81-
input: "my-pod",
82-
wantCloudErr: true,
134+
// cloud: prefix also catches cloud://
135+
input: "cloud://my-pod",
136+
wantRemoteErr: true,
83137
},
138+
139+
// --- unknown schemes ---
84140
{
85-
input: "cloud://my-pod",
86-
wantCloudErr: true,
141+
input: "https://example.com/snap",
142+
wantSchemeErr: true,
87143
},
88144
{
89-
input: "s3://bucket/key",
90-
wantCloudErr: true,
145+
input: "gcs://bucket/key",
146+
wantSchemeErr: true,
91147
},
92148
}
93149

94150
if runtime.GOOS == "windows" {
151+
tmpParent := filepath.Clean(os.TempDir())
95152
tests = append(tests,
96-
testCase{input: `~\snapshots\s`, wantPath: filepath.Join(home, "snapshots", "s.zip")},
97-
testCase{input: `C:\Users\user\snap`, wantPath: `C:\Users\user\snap.zip`},
98-
testCase{input: `C:/Users/user/snap`, wantPath: `C:\Users\user\snap.zip`},
153+
testCase{
154+
input: `~\my-state`,
155+
wantPath: filepath.Join(home, "my-state.zip"),
156+
},
157+
testCase{
158+
name: "windows abs backslash",
159+
input: filepath.Join(tmpParent, "snap"),
160+
wantPath: filepath.Join(tmpParent, "snap.zip"),
161+
},
162+
testCase{
163+
name: "windows abs forward-slash",
164+
input: strings.ReplaceAll(filepath.Join(tmpParent, "snap"), `\`, `/`),
165+
wantPath: filepath.Join(tmpParent, "snap.zip"),
166+
},
99167
)
100168
}
101169

102170
for _, tc := range tests {
103-
t.Run(tc.input, func(t *testing.T) {
171+
name := tc.input
172+
if tc.name != "" {
173+
name = tc.name
174+
}
175+
t.Run(name, func(t *testing.T) {
104176
t.Parallel()
105177
got, err := snapshot.ParseDestination(tc.input, now)
106178
if tc.wantErr != "" {
107179
require.Error(t, err)
108180
assert.Contains(t, err.Error(), tc.wantErr)
109181
return
110182
}
111-
if tc.wantCloudErr {
112-
require.ErrorIs(t, err, snapshot.ErrCloudNotSupported)
183+
if tc.wantRemoteErr {
184+
require.ErrorIs(t, err, snapshot.ErrRemoteNotSupported)
185+
return
186+
}
187+
if tc.wantSchemeErr {
188+
require.ErrorIs(t, err, snapshot.ErrUnknownScheme)
113189
return
114190
}
115191
require.NoError(t, err)

0 commit comments

Comments
 (0)