Skip to content

Commit d77ed66

Browse files
lionellolionello
authored
Fix port warnings (#645)
1 parent f649184 commit d77ed66

File tree

9 files changed

+216
-20
lines changed

9 files changed

+216
-20
lines changed

‎src/pkg/cli/compose/convert.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -310,21 +310,21 @@ func convertPort(port compose.ServicePortConfig) *defangv1.Port {
310310
switch port.Mode {
311311
case "":
312312
// TODO: This never happens now as compose-go set default to "ingress"
313-
term.Warnf("No port 'mode' was specified; defaulting to 'ingress' (add 'mode: ingress' to silence)")
313+
term.Warnf("port %d: no 'mode' was specified; defaulting to 'ingress' (add 'mode: ingress' to silence)", port.Target)
314314
fallthrough
315315
case "ingress":
316316
// This code is unnecessarily complex because compose-go silently converts short port: syntax to ingress+tcp
317317
if port.Protocol != "udp" {
318318
if port.Published != "" {
319-
term.Warnf("Published ports are ignored in ingress mode")
319+
term.Debugf("port %d: ignoring 'published: %s' in 'ingress' mode", port.Target, port.Published)
320320
}
321321
pbPort.Mode = defangv1.Mode_INGRESS
322322
if pbPort.Protocol == defangv1.Protocol_TCP {
323323
pbPort.Protocol = defangv1.Protocol_HTTP
324324
}
325325
break
326326
}
327-
term.Warnf("UDP ports default to 'host' mode (add 'mode: host' to silence)")
327+
term.Warnf("port %d: UDP ports default to 'host' mode (add 'mode: host' to silence)", port.Target)
328328
fallthrough
329329
case "host":
330330
pbPort.Mode = defangv1.Mode_HOST

‎src/pkg/cli/compose/convert_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func TestConvertPort(t *testing.T) {
2222
{
2323
name: "No target port xfail",
2424
input: types.ServicePortConfig{},
25-
wantErr: "port 'target' must be an integer between 1 and 32767",
25+
wantErr: "port 0: 'target' must be an integer between 1 and 32767",
2626
},
2727
{
2828
name: "Undefined mode and protocol, target only",
@@ -72,12 +72,12 @@ func TestConvertPort(t *testing.T) {
7272
{
7373
name: "Host mode and protocol, published range xfail",
7474
input: types.ServicePortConfig{Mode: "host", Target: 1234, Published: "1511-2222"},
75-
wantErr: "port 'published' range must include 'target': 1511-2222",
75+
wantErr: "port 1234: 'published' range must include 'target': 1511-2222",
7676
},
7777
{
7878
name: "Host mode and protocol, published range xfail",
7979
input: types.ServicePortConfig{Mode: "host", Target: 1234, Published: "22222"},
80-
wantErr: "port 'published' must be empty or equal to 'target': 22222",
80+
wantErr: "port 1234: 'published' must be empty or equal to 'target': 22222",
8181
},
8282
{
8383
name: "Host mode and protocol, target in published range",
@@ -107,7 +107,7 @@ func TestConvertPort(t *testing.T) {
107107
{
108108
name: "Localhost IP, unsupported mode and protocol xfail",
109109
input: types.ServicePortConfig{Mode: "ingress", HostIP: "127.0.0.1", Protocol: "tcp", Published: "1234", Target: 1234},
110-
wantErr: "port 'host_ip' is not supported",
110+
wantErr: "port 1234: 'host_ip' is not supported",
111111
},
112112
{
113113
name: "Ingress mode without host IP, single target, published range xfail", // - 1511-2223:1234

‎src/pkg/cli/compose/validation.go‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@ import (
55
"fmt"
66
"os"
77
"path/filepath"
8+
"sort"
89
"strconv"
910
"strings"
1011

1112
"github.com/DefangLabs/defang/src/pkg"
1213
"github.com/DefangLabs/defang/src/pkg/term"
14+
"github.com/compose-spec/compose-go/v2/types"
1315
compose "github.com/compose-spec/compose-go/v2/types"
1416
)
1517

@@ -19,7 +21,15 @@ func ValidateProject(project *compose.Project) error {
1921
if project == nil {
2022
return errors.New("no project found")
2123
}
24+
// Copy the services map into a slice so we can sort them and have consistent output
25+
var services []types.ServiceConfig
2226
for _, svccfg := range project.Services {
27+
services = append(services, svccfg)
28+
}
29+
sort.Slice(services, func(i, j int) bool {
30+
return services[i].Name < services[j].Name
31+
})
32+
for _, svccfg := range services {
2333
normalized := NormalizeServiceName(svccfg.Name)
2434
if !pkg.IsValidServiceName(normalized) {
2535
// FIXME: this is too strict; we should allow more characters like underscores and dots
@@ -304,37 +314,37 @@ var validModes = map[string]bool{"": true, "host": true, "ingress": true}
304314

305315
func validatePort(port compose.ServicePortConfig) error {
306316
if port.Target < 1 || port.Target > 32767 {
307-
return fmt.Errorf("port 'target' must be an integer between 1 and 32767: %v", port.Target)
317+
return fmt.Errorf("port %d: 'target' must be an integer between 1 and 32767", port.Target)
308318
}
309319
if port.HostIP != "" {
310-
return errors.New("port 'host_ip' is not supported")
320+
return fmt.Errorf("port %d: 'host_ip' is not supported", port.Target)
311321
}
312322
if !validProtocols[port.Protocol] {
313-
return fmt.Errorf("port 'protocol' not one of [tcp udp http http2 grpc]: %v", port.Protocol)
323+
return fmt.Errorf("port %d: 'protocol' not one of [tcp udp http http2 grpc]: %v", port.Target, port.Protocol)
314324
}
315325
if !validModes[port.Mode] {
316-
return fmt.Errorf("port 'mode' not one of [host ingress]: %v", port.Mode)
326+
return fmt.Errorf("port %d: 'mode' not one of [host ingress]: %v", port.Target, port.Mode)
317327
}
318328
if port.Published != "" && (port.Mode == "host" || port.Protocol == "udp") {
319329
portRange := strings.SplitN(port.Published, "-", 2)
320330
start, err := strconv.ParseUint(portRange[0], 10, 16)
321331
if err != nil {
322-
return fmt.Errorf("port 'published' start must be an integer: %v", portRange[0])
332+
return fmt.Errorf("port %d: 'published' start must be an integer: %v", port.Target, portRange[0])
323333
}
324334
if len(portRange) == 2 {
325335
end, err := strconv.ParseUint(portRange[1], 10, 16)
326336
if err != nil {
327-
return fmt.Errorf("port 'published' end must be an integer: %v", portRange[1])
337+
return fmt.Errorf("port %d: 'published' end must be an integer: %v", port.Target, portRange[1])
328338
}
329339
if start > end {
330-
return fmt.Errorf("port 'published' start must be less than end: %v", port.Published)
340+
return fmt.Errorf("port %d: 'published' start must be less than end: %v", port.Target, port.Published)
331341
}
332342
if port.Target < uint32(start) || port.Target > uint32(end) {
333-
return fmt.Errorf("port 'published' range must include 'target': %v", port.Published)
343+
return fmt.Errorf("port %d: 'published' range must include 'target': %v", port.Target, port.Published)
334344
}
335345
} else {
336346
if start != uint64(port.Target) {
337-
return fmt.Errorf("port 'published' must be empty or equal to 'target': %v", port.Published)
347+
return fmt.Errorf("port %d: 'published' must be empty or equal to 'target': %v", port.Target, port.Published)
338348
}
339349
}
340350
}

‎src/pkg/cli/tail.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ func tail(ctx context.Context, client client.Client, params TailOptions) error {
352352
prefixLen += l
353353
}
354354
} else {
355-
io.WriteString(buf, strings.Repeat(" ", prefixLen))
355+
buf.WriteString(strings.Repeat(" ", prefixLen))
356356
}
357357
if term.StdoutCanColor() {
358358
if !strings.Contains(line, "\033[") {
@@ -361,15 +361,16 @@ func tail(ctx context.Context, client client.Client, params TailOptions) error {
361361
} else {
362362
line = term.StripAnsi(line)
363363
}
364-
io.WriteString(buf, line)
364+
buf.WriteString(line)
365+
buf.WriteRune('\n')
365366

366367
// Detect end logging event
367368
if params.EndEventDetectFunc != nil && params.EndEventDetectFunc([]string{service}, host, line) {
368369
cancel() // TODO: stuck on defer Close() if we don't do this
369370
return nil
370371
}
371372
}
372-
term.Println(buf.String())
373+
term.Print(buf.String())
373374
}
374375
}
375376
}

‎src/tests/ports/compose.yaml‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
services:
2+
short:
3+
image: bogus
4+
ports:
5+
- 80
6+
short-published:
7+
image: bogus
8+
ports:
9+
- "8081:81"
10+
long:
11+
image: bogus
12+
ports:
13+
- target: 82
14+
long-published:
15+
image: bogus
16+
ports:
17+
- target: 83
18+
published: 8083
19+
short-udp:
20+
image: bogus
21+
ports:
22+
- "84/udp"
23+
short-udp-published:
24+
image: bogus
25+
ports:
26+
- "8085:85/udp"

‎src/tests/ports/compose.yaml.convert‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
[
2+
{
3+
"name": "long",
4+
"image": "bogus",
5+
"platform": 2,
6+
"internal": true,
7+
"ports": [
8+
{
9+
"target": 82,
10+
"protocol": 3,
11+
"mode": 1
12+
}
13+
],
14+
"networks": 1
15+
},
16+
{
17+
"name": "long-published",
18+
"image": "bogus",
19+
"platform": 2,
20+
"internal": true,
21+
"ports": [
22+
{
23+
"target": 83,
24+
"protocol": 3,
25+
"mode": 1
26+
}
27+
],
28+
"networks": 1
29+
},
30+
{
31+
"name": "short",
32+
"image": "bogus",
33+
"platform": 2,
34+
"internal": true,
35+
"ports": [
36+
{
37+
"target": 80,
38+
"protocol": 3,
39+
"mode": 1
40+
}
41+
],
42+
"networks": 1
43+
},
44+
{
45+
"name": "short-published",
46+
"image": "bogus",
47+
"platform": 2,
48+
"internal": true,
49+
"ports": [
50+
{
51+
"target": 81,
52+
"protocol": 3,
53+
"mode": 1
54+
}
55+
],
56+
"networks": 1
57+
},
58+
{
59+
"name": "short-udp",
60+
"image": "bogus",
61+
"platform": 2,
62+
"internal": true,
63+
"ports": [
64+
{
65+
"target": 84,
66+
"protocol": 1
67+
}
68+
],
69+
"networks": 1
70+
},
71+
{
72+
"name": "short-udp-published",
73+
"image": "bogus",
74+
"platform": 2,
75+
"internal": true,
76+
"ports": [
77+
{
78+
"target": 85,
79+
"protocol": 1
80+
}
81+
],
82+
"networks": 1
83+
}
84+
]

‎src/tests/ports/compose.yaml.golden‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
name: ports
2+
services:
3+
long:
4+
image: bogus
5+
networks:
6+
default: null
7+
ports:
8+
- mode: ingress
9+
target: 82
10+
protocol: tcp
11+
long-published:
12+
image: bogus
13+
networks:
14+
default: null
15+
ports:
16+
- mode: ingress
17+
target: 83
18+
published: "8083"
19+
protocol: tcp
20+
short:
21+
image: bogus
22+
networks:
23+
default: null
24+
ports:
25+
- mode: ingress
26+
target: 80
27+
protocol: tcp
28+
short-published:
29+
image: bogus
30+
networks:
31+
default: null
32+
ports:
33+
- mode: ingress
34+
target: 81
35+
published: "8081"
36+
protocol: tcp
37+
short-udp:
38+
image: bogus
39+
networks:
40+
default: null
41+
ports:
42+
- mode: ingress
43+
target: 84
44+
protocol: udp
45+
short-udp-published:
46+
image: bogus
47+
networks:
48+
default: null
49+
ports:
50+
- mode: ingress
51+
target: 85
52+
published: "8085"
53+
protocol: udp
54+
networks:
55+
default:
56+
name: ports_default

‎src/tests/ports/compose.yaml.warnings‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
! port 84: UDP ports default to 'host' mode (add 'mode: host' to silence)
2+
! port 85: UDP ports default to 'host' mode (add 'mode: host' to silence)
3+
! service "long": ingress port without healthcheck defaults to GET / HTTP/1.1
4+
! service "long": missing compose directive: restart; assuming 'unless-stopped' (add 'restart' to silence)
5+
! service "long": missing memory reservation; using provider-specific defaults. Specify deploy.resources.reservations.memory to avoid out-of-memory errors
6+
! service "long-published": ingress port without healthcheck defaults to GET / HTTP/1.1
7+
! service "long-published": missing compose directive: restart; assuming 'unless-stopped' (add 'restart' to silence)
8+
! service "long-published": missing memory reservation; using provider-specific defaults. Specify deploy.resources.reservations.memory to avoid out-of-memory errors
9+
! service "short": ingress port without healthcheck defaults to GET / HTTP/1.1
10+
! service "short": missing compose directive: restart; assuming 'unless-stopped' (add 'restart' to silence)
11+
! service "short": missing memory reservation; using provider-specific defaults. Specify deploy.resources.reservations.memory to avoid out-of-memory errors
12+
! service "short-published": ingress port without healthcheck defaults to GET / HTTP/1.1
13+
! service "short-published": missing compose directive: restart; assuming 'unless-stopped' (add 'restart' to silence)
14+
! service "short-published": missing memory reservation; using provider-specific defaults. Specify deploy.resources.reservations.memory to avoid out-of-memory errors
15+
! service "short-udp": ingress port without healthcheck defaults to GET / HTTP/1.1
16+
! service "short-udp": missing compose directive: restart; assuming 'unless-stopped' (add 'restart' to silence)
17+
! service "short-udp": missing memory reservation; using provider-specific defaults. Specify deploy.resources.reservations.memory to avoid out-of-memory errors
18+
! service "short-udp-published": missing compose directive: restart; assuming 'unless-stopped' (add 'restart' to silence)
19+
port 85: 'published' must be empty or equal to 'target': 8085

‎src/tests/testproj/compose.yaml.warnings‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
! UDP ports default to 'host' mode (add 'mode: host' to silence)
1+
! port 4567: UDP ports default to 'host' mode (add 'mode: host' to silence)
22
! service "dfnx": secrets will be exposed as environment variables, not files (use 'environment' instead)
33
! unsupported compose extension: "x-unsupported"
44
* Compressing build context for dfnx at .

0 commit comments

Comments
 (0)