Skip to content

Commit a7b5d2b

Browse files
sfc-gh-gnugmanovsfc-gh-gnugmanov
authored
SNOW-2147639: fix fdb conf command when key string has a column symbol (#574)
* SNOW-2147639: fix fdb conf file when key string has a column symbol
1 parent b2930de commit a7b5d2b

File tree

3 files changed

+218
-12
lines changed

3 files changed

+218
-12
lines changed

‎services/fdb/server/conf.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,10 @@ var (
4747
)
4848

4949
func (s *cserver) Read(_ context.Context, req *pb.ReadRequest) (*pb.FdbConfResponse, error) {
50-
cfg, err := ini.Load(req.Location.File)
50+
cfg, err := ini.LoadSources(ini.LoadOptions{
51+
KeyValueDelimiters: "=",
52+
PreserveSurroundedQuote: true,
53+
}, req.Location.File)
5154
if err != nil {
5255
return nil, status.Errorf(codes.Internal, "could not load config file %s: %v", req.Location.File, err)
5356
}
@@ -75,7 +78,10 @@ func (s *cserver) Write(_ context.Context, req *pb.WriteRequest) (*emptypb.Empty
7578
return nil, status.Error(codes.InvalidArgument, "key value can not be empty")
7679
}
7780

78-
cfg, err := ini.Load(req.Location.File)
81+
cfg, err := ini.LoadSources(ini.LoadOptions{
82+
KeyValueDelimiters: "=",
83+
PreserveSurroundedQuote: true,
84+
}, req.Location.File)
7985
if err != nil {
8086
return nil, status.Errorf(codes.Internal, "could not load config file %s: %v", req.Location.File, err)
8187
}
@@ -95,7 +101,10 @@ func (s *cserver) Delete(_ context.Context, req *pb.DeleteRequest) (*emptypb.Emp
95101
return nil, status.Error(codes.InvalidArgument, "section name can not be empty")
96102
}
97103

98-
cfg, err := ini.Load(req.Location.File)
104+
cfg, err := ini.LoadSources(ini.LoadOptions{
105+
KeyValueDelimiters: "=",
106+
PreserveSurroundedQuote: true,
107+
}, req.Location.File)
99108
if err != nil {
100109
return nil, status.Errorf(codes.Internal, "could not load config file %s: %v", req.Location.File, err)
101110
}

‎services/fdb/server/conf_test.go‎

Lines changed: 205 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,17 @@ func TestRead(t *testing.T) {
6363
},
6464
},
6565
},
66+
{
67+
name: "read key with colon in name",
68+
respValue: "test_value",
69+
req: &pb.ReadRequest{
70+
Location: &pb.Location{
71+
File: path,
72+
Section: "backup_agent",
73+
Key: "the:key:with:colon",
74+
},
75+
},
76+
},
6677
} {
6778
tc := tc
6879
t.Run(tc.name, func(t *testing.T) {
@@ -124,9 +135,11 @@ cluster_file = /etc/foundatindb/fdb.cluster`)
124135

125136
client := pb.NewConfClient(conn)
126137
for _, tc := range []struct {
127-
name string
128-
req *pb.WriteRequest
129-
expected string
138+
name string
139+
req *pb.WriteRequest
140+
expected string
141+
expectErr bool
142+
expectErrString string
130143
}{
131144
{
132145
name: "write cluster_file to general",
@@ -157,10 +170,124 @@ test = badcoffee`,
157170
Value: "badcoffee",
158171
},
159172
},
173+
{
174+
name: "write key with colon in key name",
175+
expected: `[general]
176+
cluster_file = /tmp/fdb.cluster
177+
178+
[backup.1]
179+
test = badcoffee
180+
181+
[backup_agent]
182+
the:key:with:colon = test_value`,
183+
req: &pb.WriteRequest{
184+
Location: &pb.Location{
185+
File: name,
186+
Section: "backup_agent",
187+
Key: "the:key:with:colon",
188+
},
189+
Value: "test_value",
190+
},
191+
},
192+
{
193+
name: "write key containing colon in value",
194+
expected: `[general]
195+
cluster_file = /tmp/fdb.cluster
196+
197+
[backup.1]
198+
test = badcoffee
199+
200+
[backup_agent]
201+
the:key:with:colon = test_value
202+
203+
[network]
204+
listen_address = 127.0.0.1:4500`,
205+
req: &pb.WriteRequest{
206+
Location: &pb.Location{
207+
File: name,
208+
Section: "network",
209+
Key: "listen_address",
210+
},
211+
Value: "127.0.0.1:4500",
212+
},
213+
},
214+
{
215+
name: "write key with multiple colons in key name",
216+
expected: `[general]
217+
cluster_file = /tmp/fdb.cluster
218+
219+
[backup.1]
220+
test = badcoffee
221+
222+
[backup_agent]
223+
the:key:with:colon = test_value
224+
225+
[network]
226+
listen_address = 127.0.0.1:4500
227+
228+
[cluster]
229+
database:connection:string:key = "connection://host:port/db"`,
230+
req: &pb.WriteRequest{
231+
Location: &pb.Location{
232+
File: name,
233+
Section: "cluster",
234+
Key: "database:connection:string:key",
235+
},
236+
Value: `"connection://host:port/db"`,
237+
},
238+
},
239+
{
240+
name: "empty section name",
241+
req: &pb.WriteRequest{
242+
Location: &pb.Location{
243+
File: name,
244+
Section: "",
245+
Key: "test_key",
246+
},
247+
Value: "test_value",
248+
},
249+
expectErr: true,
250+
expectErrString: "section name can not be empty",
251+
},
252+
{
253+
name: "empty key name",
254+
req: &pb.WriteRequest{
255+
Location: &pb.Location{
256+
File: name,
257+
Section: "test_section",
258+
Key: "",
259+
},
260+
Value: "test_value",
261+
},
262+
expectErr: true,
263+
expectErrString: "key name can not be empty",
264+
},
265+
{
266+
name: "empty key value",
267+
req: &pb.WriteRequest{
268+
Location: &pb.Location{
269+
File: name,
270+
Section: "test_section",
271+
Key: "test_key",
272+
},
273+
Value: "",
274+
},
275+
expectErr: true,
276+
expectErrString: "key value can not be empty",
277+
},
160278
} {
161279
tc := tc
162280
t.Run(tc.name, func(t *testing.T) {
163281
resp, err := client.Write(ctx, tc.req)
282+
if tc.expectErr {
283+
if err == nil {
284+
t.Fatal("expected error but got none")
285+
}
286+
if !strings.Contains(err.Error(), tc.expectErrString) {
287+
t.Errorf("expected error to contain %q, got %q", tc.expectErrString, err.Error())
288+
}
289+
return
290+
}
164291
testutil.FatalOnErr(fmt.Sprintf("%v - resp %v", tc.name, resp), err, t)
165292
got, err := os.ReadFile(name)
166293
testutil.FatalOnErr("failed reading config file", err, t)
@@ -193,9 +320,13 @@ cluster_file = /etc/foundatindb/fdb.cluster
193320
194321
[foo.1]
195322
bar = baz
323+
the:key:with:colon = test_value
196324
197325
[foo.2]
198-
bar = baz`)
326+
bar = baz
327+
328+
[backup_agent]
329+
database:connection:string:key = "connection://host:port/db"`)
199330
testutil.FatalOnErr("WriteString", err, t)
200331
name := f1.Name()
201332
err = f1.Close()
@@ -223,9 +354,13 @@ bar = baz`)
223354
cluster_file = /etc/foundatindb/fdb.cluster
224355
225356
[foo.1]
226-
bar = baz
357+
bar = baz
358+
the:key:with:colon = test_value
359+
360+
[foo.2]
227361
228-
[foo.2]`,
362+
[backup_agent]
363+
database:connection:string:key = "connection://host:port/db"`,
229364
},
230365
{
231366
name: "delete empty section",
@@ -236,7 +371,11 @@ bar = baz
236371
cluster_file = /etc/foundatindb/fdb.cluster
237372
238373
[foo.1]
239-
bar = baz`,
374+
bar = baz
375+
the:key:with:colon = test_value
376+
377+
[backup_agent]
378+
database:connection:string:key = "connection://host:port/db"`,
240379
},
241380
{
242381
name: "delete section that doesnt exist",
@@ -247,17 +386,74 @@ bar = baz`,
247386
cluster_file = /etc/foundatindb/fdb.cluster
248387
249388
[foo.1]
250-
bar = baz`,
389+
bar = baz
390+
the:key:with:colon = test_value
391+
392+
[backup_agent]
393+
database:connection:string:key = "connection://host:port/db"`,
251394
expectErr: true,
252395
expectErrString: "section foo.42 does not exist",
253396
},
397+
{
398+
name: "delete key with colon in key name",
399+
req: &pb.DeleteRequest{
400+
Location: &pb.Location{File: name, Section: "foo.1", Key: "the:key:with:colon"},
401+
},
402+
expected: `[general]
403+
cluster_file = /etc/foundatindb/fdb.cluster
404+
405+
[foo.1]
406+
bar = baz
407+
408+
[backup_agent]
409+
database:connection:string:key = "connection://host:port/db"`,
410+
},
411+
{
412+
name: "delete key with multiple colons in key name",
413+
req: &pb.DeleteRequest{
414+
Location: &pb.Location{File: name, Section: "backup_agent", Key: "database:connection:string:key"},
415+
},
416+
expected: `[general]
417+
cluster_file = /etc/foundatindb/fdb.cluster
418+
419+
[foo.1]
420+
bar = baz
421+
422+
[backup_agent]`,
423+
},
254424
{
255425
name: "delete whole section with keys",
256426
req: &pb.DeleteRequest{
257427
Location: &pb.Location{File: name, Section: "foo.1", Key: ""},
258428
},
259429
expected: `[general]
260-
cluster_file = /etc/foundatindb/fdb.cluster`,
430+
cluster_file = /etc/foundatindb/fdb.cluster
431+
432+
[backup_agent]`,
433+
},
434+
{
435+
name: "empty section name",
436+
req: &pb.DeleteRequest{
437+
Location: &pb.Location{
438+
File: name,
439+
Section: "",
440+
Key: "test_key",
441+
},
442+
},
443+
expectErr: true,
444+
expectErrString: "section name can not be empty",
445+
},
446+
{
447+
name: "non-existent section",
448+
req: &pb.DeleteRequest{
449+
Location: &pb.Location{
450+
File: name,
451+
Section: "nonexistent",
452+
Key: "test_key",
453+
},
454+
},
455+
expectErr: true,
456+
expectErrString: "section nonexistent does not exist",
261457
},
262458
} {
263459
tc := tc

‎services/fdb/server/testdata/foundationdb.conf‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,6 @@ restart_delay = 60
2222

2323
[backup_agent]
2424
command = /usr/lib/foundationdb/backup_agent/backup_agent
25+
the:key:with:colon = test_value
2526

2627
[backup_agent.1]

0 commit comments

Comments
 (0)