Skip to content

Commit 75fb1ab

Browse files
idsulikglours
idsulik
authored and
glours
committed
feat(volume): Enhance volume parsing to support variable default values
Signed-off-by: Suleiman Dibirov <[email protected]>
1 parent df0892c commit 75fb1ab

File tree

2 files changed

+260
-2
lines changed

2 files changed

+260
-2
lines changed

‎format/volume.go‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,26 @@ func ParseVolume(spec string) (types.ServiceVolumeConfig, error) {
4242
}
4343

4444
var buffer []rune
45-
for _, char := range spec + string(endOfSpec) {
45+
var inVarSubstitution int // Track nesting depth of ${...}
46+
for i, char := range spec + string(endOfSpec) {
47+
// Check if we're entering a variable substitution
48+
if char == '$' && i+1 < len(spec) && rune(spec[i+1]) == '{' {
49+
inVarSubstitution++
50+
buffer = append(buffer, char)
51+
continue
52+
}
53+
54+
// Check if we're exiting a variable substitution
55+
if char == '}' && inVarSubstitution > 0 {
56+
inVarSubstitution--
57+
buffer = append(buffer, char)
58+
continue
59+
}
60+
4661
switch {
4762
case isWindowsDrive(buffer, char):
4863
buffer = append(buffer, char)
49-
case char == ':' || char == endOfSpec:
64+
case (char == ':' || char == endOfSpec) && inVarSubstitution == 0:
5065
if err := populateFieldFromBuffer(char, buffer, &volume); err != nil {
5166
populateType(&volume)
5267
return volume, fmt.Errorf("invalid spec: %s: %w", spec, err)

‎format/volume_test.go‎

Lines changed: 243 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,3 +295,246 @@ func TestVolumeStringer(t *testing.T) {
295295
}
296296
assert.Equal(t, v.String(), "/src:/target:rw,z,shared")
297297
}
298+
299+
// TestParseVolumeWithVariableDefaultValue tests that volume parsing
300+
// correctly handles variables with default values (${VAR:-DEFAULT})
301+
func TestParseVolumeWithVariableDefaultValue(t *testing.T) {
302+
testCases := []struct {
303+
name string
304+
input string
305+
expected types.ServiceVolumeConfig
306+
}{
307+
{
308+
name: "variable with default value in target path",
309+
input: "/tmp:/tmp/${BUG_HERE:-DEFAULT}/path",
310+
expected: types.ServiceVolumeConfig{
311+
Type: "bind",
312+
Source: "/tmp",
313+
Target: "/tmp/${BUG_HERE:-DEFAULT}/path",
314+
Bind: &types.ServiceVolumeBind{CreateHostPath: true},
315+
},
316+
},
317+
{
318+
name: "variable with default value in source path",
319+
input: "/tmp/${VAR:-default}:/target",
320+
expected: types.ServiceVolumeConfig{
321+
Type: "bind",
322+
Source: "/tmp/${VAR:-default}",
323+
Target: "/target",
324+
Bind: &types.ServiceVolumeBind{CreateHostPath: true},
325+
},
326+
},
327+
{
328+
name: "variable with default value in both source and target",
329+
input: "/src/${SRC:-default}:/dst/${DST:-value}",
330+
expected: types.ServiceVolumeConfig{
331+
Type: "bind",
332+
Source: "/src/${SRC:-default}",
333+
Target: "/dst/${DST:-value}",
334+
Bind: &types.ServiceVolumeBind{CreateHostPath: true},
335+
},
336+
},
337+
{
338+
name: "variable with empty default value",
339+
input: "/tmp:/tmp/${VAR:-}/path",
340+
expected: types.ServiceVolumeConfig{
341+
Type: "bind",
342+
Source: "/tmp",
343+
Target: "/tmp/${VAR:-}/path",
344+
Bind: &types.ServiceVolumeBind{CreateHostPath: true},
345+
},
346+
},
347+
{
348+
name: "variable with complex default value containing slashes",
349+
input: "/tmp:/tmp/${VAR:-default/path/value}/file",
350+
expected: types.ServiceVolumeConfig{
351+
Type: "bind",
352+
Source: "/tmp",
353+
Target: "/tmp/${VAR:-default/path/value}/file",
354+
Bind: &types.ServiceVolumeBind{CreateHostPath: true},
355+
},
356+
},
357+
{
358+
name: "multiple variables with default values in target",
359+
input: "/tmp:/tmp/${VAR1:-val1}/${VAR2:-val2}/path",
360+
expected: types.ServiceVolumeConfig{
361+
Type: "bind",
362+
Source: "/tmp",
363+
Target: "/tmp/${VAR1:-val1}/${VAR2:-val2}/path",
364+
Bind: &types.ServiceVolumeBind{CreateHostPath: true},
365+
},
366+
},
367+
{
368+
name: "variable with default value and read-only option",
369+
input: "/tmp:/tmp/${VAR:-default}/path:ro",
370+
expected: types.ServiceVolumeConfig{
371+
Type: "bind",
372+
Source: "/tmp",
373+
Target: "/tmp/${VAR:-default}/path",
374+
ReadOnly: true,
375+
Bind: &types.ServiceVolumeBind{CreateHostPath: true},
376+
},
377+
},
378+
{
379+
name: "variable without default value (normal variable)",
380+
input: "/tmp:/tmp/${WORKS_AS_EXPECTED}/path",
381+
expected: types.ServiceVolumeConfig{
382+
Type: "bind",
383+
Source: "/tmp",
384+
Target: "/tmp/${WORKS_AS_EXPECTED}/path",
385+
Bind: &types.ServiceVolumeBind{CreateHostPath: true},
386+
},
387+
},
388+
{
389+
name: "mixed variables with and without defaults",
390+
input: "/tmp:/tmp/${VAR1:-default}/${VAR2}/path",
391+
expected: types.ServiceVolumeConfig{
392+
Type: "bind",
393+
Source: "/tmp",
394+
Target: "/tmp/${VAR1:-default}/${VAR2}/path",
395+
Bind: &types.ServiceVolumeBind{CreateHostPath: true},
396+
},
397+
},
398+
{
399+
name: "variable with default containing colon",
400+
input: "/tmp:/tmp/${TIME:-12:30:45}/path",
401+
expected: types.ServiceVolumeConfig{
402+
Type: "bind",
403+
Source: "/tmp",
404+
Target: "/tmp/${TIME:-12:30:45}/path",
405+
Bind: &types.ServiceVolumeBind{CreateHostPath: true},
406+
},
407+
},
408+
{
409+
name: "named volume with variable default in target",
410+
input: "myvolume:/data/${VAR:-default}/path",
411+
expected: types.ServiceVolumeConfig{
412+
Type: "volume",
413+
Source: "myvolume",
414+
Target: "/data/${VAR:-default}/path",
415+
Volume: &types.ServiceVolumeVolume{},
416+
},
417+
},
418+
{
419+
name: "variable with default value and bind options",
420+
input: "/src:/target/${VAR:-default}/path:slave,ro",
421+
expected: types.ServiceVolumeConfig{
422+
Type: "bind",
423+
Source: "/src",
424+
Target: "/target/${VAR:-default}/path",
425+
ReadOnly: true,
426+
Bind: &types.ServiceVolumeBind{
427+
CreateHostPath: true,
428+
Propagation: "slave",
429+
},
430+
},
431+
},
432+
}
433+
434+
for _, tc := range testCases {
435+
t.Run(tc.name, func(t *testing.T) {
436+
volume, err := ParseVolume(tc.input)
437+
assert.NilError(t, err, fmt.Sprintf("Failed to parse: %s", tc.input))
438+
assert.Check(t, is.DeepEqual(tc.expected, volume))
439+
})
440+
}
441+
}
442+
443+
// TestParseVolumeWithVariableDefaultValueWindows tests Windows-specific
444+
// volume parsing with variables containing default values
445+
func TestParseVolumeWithVariableDefaultValueWindows(t *testing.T) {
446+
testCases := []struct {
447+
name string
448+
input string
449+
expected types.ServiceVolumeConfig
450+
}{
451+
{
452+
name: "Windows path with variable default in target",
453+
input: "C:\\source:D:\\target\\${VAR:-default}\\path",
454+
expected: types.ServiceVolumeConfig{
455+
Type: "bind",
456+
Source: "C:\\source",
457+
Target: "D:\\target\\${VAR:-default}\\path",
458+
Bind: &types.ServiceVolumeBind{CreateHostPath: true},
459+
},
460+
},
461+
{
462+
name: "Windows path with variable default in source",
463+
input: "C:\\src\\${VAR:-default}:D:\\target",
464+
expected: types.ServiceVolumeConfig{
465+
Type: "bind",
466+
Source: "C:\\src\\${VAR:-default}",
467+
Target: "D:\\target",
468+
Bind: &types.ServiceVolumeBind{CreateHostPath: true},
469+
},
470+
},
471+
{
472+
name: "Windows path with variable default and options",
473+
input: "C:\\source:D:\\${VAR:-default}\\path:ro",
474+
expected: types.ServiceVolumeConfig{
475+
Type: "bind",
476+
Source: "C:\\source",
477+
Target: "D:\\${VAR:-default}\\path",
478+
ReadOnly: true,
479+
Bind: &types.ServiceVolumeBind{CreateHostPath: true},
480+
},
481+
},
482+
}
483+
484+
for _, tc := range testCases {
485+
t.Run(tc.name, func(t *testing.T) {
486+
volume, err := ParseVolume(tc.input)
487+
assert.NilError(t, err, fmt.Sprintf("Failed to parse: %s", tc.input))
488+
assert.Check(t, is.DeepEqual(tc.expected, volume))
489+
})
490+
}
491+
}
492+
493+
// TestParseVolumeWithNestedBraces tests edge cases with nested or
494+
// complex brace patterns
495+
func TestParseVolumeWithNestedBraces(t *testing.T) {
496+
testCases := []struct {
497+
name string
498+
input string
499+
expected types.ServiceVolumeConfig
500+
}{
501+
{
502+
name: "variable with default containing braces",
503+
input: "/tmp:/tmp/${VAR:-{default}}/path",
504+
expected: types.ServiceVolumeConfig{
505+
Type: "bind",
506+
Source: "/tmp",
507+
Target: "/tmp/${VAR:-{default}}/path",
508+
Bind: &types.ServiceVolumeBind{CreateHostPath: true},
509+
},
510+
},
511+
{
512+
name: "variable with default at end of path",
513+
input: "/tmp:/tmp/path/${VAR:-default}",
514+
expected: types.ServiceVolumeConfig{
515+
Type: "bind",
516+
Source: "/tmp",
517+
Target: "/tmp/path/${VAR:-default}",
518+
Bind: &types.ServiceVolumeBind{CreateHostPath: true},
519+
},
520+
},
521+
{
522+
name: "variable with default at start of path",
523+
input: "/tmp:/${VAR:-default}/path",
524+
expected: types.ServiceVolumeConfig{
525+
Type: "bind",
526+
Source: "/tmp",
527+
Target: "/${VAR:-default}/path",
528+
Bind: &types.ServiceVolumeBind{CreateHostPath: true},
529+
},
530+
},
531+
}
532+
533+
for _, tc := range testCases {
534+
t.Run(tc.name, func(t *testing.T) {
535+
volume, err := ParseVolume(tc.input)
536+
assert.NilError(t, err, fmt.Sprintf("Failed to parse: %s", tc.input))
537+
assert.Check(t, is.DeepEqual(tc.expected, volume))
538+
})
539+
}
540+
}

0 commit comments

Comments
 (0)