Skip to content

Commit f9b9b36

Browse files
tonistiigitonistiigi
authored
Merge pull request #6135 from crazy-max/lint-expose-proto-casing
dockerfile: rules check for EXPOSE instruction
2 parents ec7a152 + d6c82c6 commit f9b9b36

File tree

10 files changed

+374
-72
lines changed

10 files changed

+374
-72
lines changed

‎frontend/dockerfile/dockerfile2llb/convert.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -971,7 +971,7 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error {
971971
case *instructions.HealthCheckCommand:
972972
err = dispatchHealthcheck(d, c, opt.lint)
973973
case *instructions.ExposeCommand:
974-
err = dispatchExpose(d, c, opt.shlex)
974+
err = dispatchExpose(d, c, &opt)
975975
case *instructions.UserCommand:
976976
err = dispatchUser(d, c, true)
977977
case *instructions.VolumeCommand:

‎frontend/dockerfile/dockerfile2llb/convert_expose.go

Lines changed: 97 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -7,93 +7,100 @@ import (
77
"strings"
88

99
"github.com/moby/buildkit/frontend/dockerfile/instructions"
10-
"github.com/moby/buildkit/frontend/dockerfile/shell"
10+
"github.com/moby/buildkit/frontend/dockerfile/linter"
11+
"github.com/moby/buildkit/frontend/dockerfile/parser"
1112
"github.com/pkg/errors"
1213
)
1314

14-
func dispatchExpose(d *dispatchState, c *instructions.ExposeCommand, shlex *shell.Lex) error {
15+
func dispatchExpose(d *dispatchState, c *instructions.ExposeCommand, opt *dispatchOpt) error {
1516
ports := []string{}
1617
env := getEnv(d.state)
1718
for _, p := range c.Ports {
18-
ps, err := shlex.ProcessWords(p, env)
19+
ps, err := opt.shlex.ProcessWords(p, env)
1920
if err != nil {
2021
return err
2122
}
2223
ports = append(ports, ps...)
2324
}
2425
c.Ports = ports
2526

26-
ps, err := parsePortSpecs(c.Ports)
27+
ps := newPortSpecs(
28+
withLocation(c.Location()),
29+
withLint(opt.lint),
30+
)
31+
32+
psp, err := ps.parsePorts(c.Ports)
2733
if err != nil {
2834
return err
2935
}
3036

3137
if d.image.Config.ExposedPorts == nil {
3238
d.image.Config.ExposedPorts = make(map[string]struct{})
3339
}
34-
for _, p := range ps {
40+
for _, p := range psp {
3541
d.image.Config.ExposedPorts[p] = struct{}{}
3642
}
3743

3844
return commitToHistory(&d.image, fmt.Sprintf("EXPOSE %v", ps), false, nil, d.epoch)
3945
}
4046

41-
// parsePortSpecs receives port specs in the format of [ip:]public:private/proto
42-
// and returns them as a list of "port/proto".
43-
func parsePortSpecs(ports []string) (exposedPorts []string, _ error) {
44-
for _, p := range ports {
45-
portProtos, err := parsePortSpec(p)
46-
if err != nil {
47-
return nil, err
48-
}
49-
exposedPorts = append(exposedPorts, portProtos...)
50-
}
51-
return exposedPorts, nil
47+
type portSpecs struct {
48+
location []parser.Range
49+
lint *linter.Linter
5250
}
5351

54-
// splitProtoPort splits a port(range) and protocol, formatted as "<portnum>/[<proto>]"
55-
// "<startport-endport>/[<proto>]". It returns an error if no port(range) or
56-
// an invalid proto is provided. If no protocol is provided, the default ("tcp")
57-
// protocol is returned.
58-
func splitProtoPort(rawPort string) (proto string, port string, _ error) {
59-
port, proto, _ = strings.Cut(rawPort, "/")
60-
if port == "" {
61-
return "", "", errors.New("no port specified")
52+
type portSpecsOption func(ps *portSpecs)
53+
54+
func withLocation(location []parser.Range) portSpecsOption {
55+
return func(ps *portSpecs) {
56+
ps.location = location
6257
}
63-
proto = strings.ToLower(proto)
64-
switch proto {
65-
case "":
66-
return "tcp", port, nil
67-
case "tcp", "udp", "sctp":
68-
return proto, port, nil
69-
default:
70-
return "", "", errors.New("invalid proto: " + proto)
58+
}
59+
60+
func withLint(lint *linter.Linter) portSpecsOption {
61+
return func(ps *portSpecs) {
62+
ps.lint = lint
7163
}
7264
}
7365

74-
func splitParts(rawport string) (hostIP, hostPort, containerPort string) {
75-
parts := strings.Split(rawport, ":")
66+
func newPortSpecs(opts ...portSpecsOption) *portSpecs {
67+
ps := &portSpecs{}
68+
for _, opt := range opts {
69+
opt(ps)
70+
}
71+
return ps
72+
}
7673

77-
switch len(parts) {
78-
case 1:
79-
return "", "", parts[0]
80-
case 2:
81-
return "", parts[0], parts[1]
82-
case 3:
83-
return parts[0], parts[1], parts[2]
84-
default:
85-
n := len(parts)
86-
return strings.Join(parts[:n-2], ":"), parts[n-2], parts[n-1]
74+
// parsePorts receives port specs in the format of [ip:]public:private/proto
75+
// and returns them as a list of "port/proto".
76+
func (ps *portSpecs) parsePorts(ports []string) (exposedPorts []string, _ error) {
77+
for _, p := range ports {
78+
portProtos, err := ps.parsePort(p)
79+
if err != nil {
80+
return nil, err
81+
}
82+
exposedPorts = append(exposedPorts, portProtos...)
8783
}
84+
return exposedPorts, nil
8885
}
8986

90-
// parsePortSpec parses a port specification string into a slice of "<portnum>/[<proto>]"
91-
func parsePortSpec(rawPort string) (portProto []string, _ error) {
92-
ip, hostPort, containerPort := splitParts(rawPort)
93-
proto, containerPort, err := splitProtoPort(containerPort)
87+
// parsePort parses a port specification string into a slice of "<portnum>/[<proto>]"
88+
func (ps *portSpecs) parsePort(rawPort string) (portProto []string, _ error) {
89+
ip, hostPort, containerPort := ps.splitParts(rawPort)
90+
proto, containerPort, err := ps.splitProtoPort(containerPort)
9491
if err != nil {
9592
return nil, errors.Wrapf(err, "invalid port: %q", rawPort)
9693
}
94+
if ps.lint != nil {
95+
if proto != strings.ToLower(proto) {
96+
msg := linter.RuleExposeProtoCasing.Format(rawPort)
97+
ps.lint.Run(&linter.RuleExposeProtoCasing, ps.location, msg)
98+
}
99+
if ip != "" || hostPort != "" {
100+
msg := linter.RuleExposeInvalidFormat.Format(rawPort)
101+
ps.lint.Run(&linter.RuleExposeInvalidFormat, ps.location, msg)
102+
}
103+
}
97104

98105
// TODO(thaJeztah): mapping IP-addresses should not be allowed for EXPOSE; see https://github.com/moby/buildkit/issues/2173
99106
if ip != "" && ip[0] == '[' {
@@ -108,14 +115,14 @@ func parsePortSpec(rawPort string) (portProto []string, _ error) {
108115
return nil, errors.New("invalid IP address: " + ip)
109116
}
110117

111-
startPort, endPort, err := parsePortRange(containerPort)
118+
startPort, endPort, err := ps.parsePortRange(containerPort)
112119
if err != nil {
113120
return nil, errors.New("invalid containerPort: " + containerPort)
114121
}
115122

116123
// TODO(thaJeztah): mapping host-ports should not be allowed for EXPOSE; see https://github.com/moby/buildkit/issues/2173
117124
if hostPort != "" {
118-
startHostPort, endHostPort, err := parsePortRange(hostPort)
125+
startHostPort, endHostPort, err := ps.parsePortRange(hostPort)
119126
if err != nil {
120127
return nil, errors.New("invalid hostPort: " + hostPort)
121128
}
@@ -133,27 +140,27 @@ func parsePortSpec(rawPort string) (portProto []string, _ error) {
133140
ports := make([]string, 0, count)
134141

135142
for i := range count {
136-
ports = append(ports, strconv.Itoa(startPort+i)+"/"+proto)
143+
ports = append(ports, strconv.Itoa(startPort+i)+"/"+strings.ToLower(proto))
137144
}
138145
return ports, nil
139146
}
140147

141148
// parsePortRange parses and validates the specified string as a port range (e.g., "8000-9000").
142-
func parsePortRange(ports string) (startPort, endPort int, _ error) {
149+
func (ps *portSpecs) parsePortRange(ports string) (startPort, endPort int, _ error) {
143150
if ports == "" {
144151
return 0, 0, errors.New("empty string specified for ports")
145152
}
146153
start, end, ok := strings.Cut(ports, "-")
147154

148-
startPort, err := parsePortNumber(start)
155+
startPort, err := ps.parsePortNumber(start)
149156
if err != nil {
150157
return 0, 0, errors.Wrapf(err, "invalid start port '%s'", start)
151158
}
152159
if !ok || start == end {
153160
return startPort, startPort, nil
154161
}
155162

156-
endPort, err = parsePortNumber(end)
163+
endPort, err = ps.parsePortNumber(end)
157164
if err != nil {
158165
return 0, 0, errors.Wrapf(err, "invalid end port '%s'", end)
159166
}
@@ -165,7 +172,7 @@ func parsePortRange(ports string) (startPort, endPort int, _ error) {
165172

166173
// parsePortNumber parses rawPort into an int, unwrapping strconv errors
167174
// and returning a single "out of range" error for any value outside 0–65535.
168-
func parsePortNumber(rawPort string) (int, error) {
175+
func (ps *portSpecs) parsePortNumber(rawPort string) (int, error) {
169176
if rawPort == "" {
170177
return 0, errors.New("value is empty")
171178
}
@@ -183,3 +190,38 @@ func parsePortNumber(rawPort string) (int, error) {
183190

184191
return int(port), nil
185192
}
193+
194+
// splitProtoPort splits a port(range) and protocol, formatted as "<portnum>/[<proto>]"
195+
// "<startport-endport>/[<proto>]". It returns an error if no port(range) or
196+
// an invalid proto is provided. If no protocol is provided, the default ("tcp")
197+
// protocol is returned.
198+
func (ps *portSpecs) splitProtoPort(rawPort string) (proto string, port string, _ error) {
199+
port, proto, _ = strings.Cut(rawPort, "/")
200+
if port == "" {
201+
return "", "", errors.New("no port specified")
202+
}
203+
switch strings.ToLower(proto) {
204+
case "":
205+
return "tcp", port, nil
206+
case "tcp", "udp", "sctp":
207+
return proto, port, nil
208+
default:
209+
return "", "", errors.New("invalid proto: " + proto)
210+
}
211+
}
212+
213+
func (ps *portSpecs) splitParts(rawport string) (hostIP, hostPort, containerPort string) {
214+
parts := strings.Split(rawport, ":")
215+
216+
switch len(parts) {
217+
case 1:
218+
return "", "", parts[0]
219+
case 2:
220+
return "", parts[0], parts[1]
221+
case 3:
222+
return parts[0], parts[1], parts[2]
223+
default:
224+
n := len(parts)
225+
return strings.Join(parts[:n-2], ":"), parts[n-2], parts[n-1]
226+
}
227+
}

‎frontend/dockerfile/dockerfile2llb/convert_expose_test.go

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ import (
77
"github.com/stretchr/testify/require"
88
)
99

10-
func TestParsePortSpecEmptyContainerPort(t *testing.T) {
10+
var ps = newPortSpecs()
11+
12+
func TestParsePortEmptyContainerPort(t *testing.T) {
1113
tests := []struct {
1214
name string
1315
spec string
@@ -31,7 +33,7 @@ func TestParsePortSpecEmptyContainerPort(t *testing.T) {
3133
}
3234
for _, tc := range tests {
3335
t.Run(tc.name, func(t *testing.T) {
34-
_, err := parsePortSpec(tc.spec)
36+
_, err := ps.parsePort(tc.spec)
3537
if tc.expError != "" {
3638
assert.EqualError(t, err, tc.expError)
3739
} else {
@@ -41,13 +43,13 @@ func TestParsePortSpecEmptyContainerPort(t *testing.T) {
4143
}
4244
}
4345

44-
func TestParsePortSpecFull(t *testing.T) {
45-
exposedPorts, err := parsePortSpec("0.0.0.0:1234-1235:3333-3334/tcp")
46+
func TestParsePortFull(t *testing.T) {
47+
exposedPorts, err := ps.parsePort("0.0.0.0:1234-1235:3333-3334/tcp")
4648
require.NoError(t, err)
4749
assert.Equal(t, []string{"3333/tcp", "3334/tcp"}, exposedPorts)
4850
}
4951

50-
func TestPartPortSpecIPV6(t *testing.T) {
52+
func TestPartPortIPV6(t *testing.T) {
5153
type test struct {
5254
name string
5355
spec string
@@ -82,43 +84,43 @@ func TestPartPortSpecIPV6(t *testing.T) {
8284
}
8385
for _, c := range cases {
8486
t.Run(c.name, func(t *testing.T) {
85-
exposedPorts, err := parsePortSpec(c.spec)
87+
exposedPorts, err := ps.parsePort(c.spec)
8688
require.NoError(t, err)
8789
assert.Equal(t, c.expected, exposedPorts)
8890
})
8991
}
9092
}
9193

92-
func TestParsePortSpecs(t *testing.T) {
93-
exposedPorts, err := parsePortSpecs([]string{"1234/tcp", "2345/udp", "3456/sctp"})
94+
func TestParsePorts(t *testing.T) {
95+
exposedPorts, err := ps.parsePorts([]string{"1234/tcp", "2345/udp", "3456/sctp"})
9496
require.NoError(t, err)
9597
assert.Equal(t, []string{"1234/tcp", "2345/udp", "3456/sctp"}, exposedPorts)
9698

97-
exposedPorts, err = parsePortSpecs([]string{"1234:1234/tcp", "2345:2345/udp", "3456:3456/sctp"})
99+
exposedPorts, err = ps.parsePorts([]string{"1234:1234/tcp", "2345:2345/udp", "3456:3456/sctp"})
98100
require.NoError(t, err)
99101
assert.Equal(t, []string{"1234/tcp", "2345/udp", "3456/sctp"}, exposedPorts)
100102

101-
exposedPorts, err = parsePortSpecs([]string{"0.0.0.0:1234:1234/tcp", "0.0.0.0:2345:2345/udp", "0.0.0.0:3456:3456/sctp"})
103+
exposedPorts, err = ps.parsePorts([]string{"0.0.0.0:1234:1234/tcp", "0.0.0.0:2345:2345/udp", "0.0.0.0:3456:3456/sctp"})
102104
require.NoError(t, err)
103105
assert.Equal(t, []string{"1234/tcp", "2345/udp", "3456/sctp"}, exposedPorts)
104106

105-
_, err = parsePortSpecs([]string{"localhost:1234:1234/tcp"})
107+
_, err = ps.parsePorts([]string{"localhost:1234:1234/tcp"})
106108
assert.Error(t, err, "Received no error while trying to parse a hostname instead of ip")
107109
}
108110

109-
func TestParsePortSpecsWithRange(t *testing.T) {
110-
exposedPorts, err := parsePortSpecs([]string{"1234-1236/tcp", "2345-2347/udp", "3456-3458/sctp"})
111+
func TestParsePortsWithRange(t *testing.T) {
112+
exposedPorts, err := ps.parsePorts([]string{"1234-1236/tcp", "2345-2347/udp", "3456-3458/sctp"})
111113
require.NoError(t, err)
112114
assert.Equal(t, []string{"1234/tcp", "1235/tcp", "1236/tcp", "2345/udp", "2346/udp", "2347/udp", "3456/sctp", "3457/sctp", "3458/sctp"}, exposedPorts)
113115

114-
exposedPorts, err = parsePortSpecs([]string{"1234-1236:1234-1236/tcp", "2345-2347:2345-2347/udp", "3456-3458:3456-3458/sctp"})
116+
exposedPorts, err = ps.parsePorts([]string{"1234-1236:1234-1236/tcp", "2345-2347:2345-2347/udp", "3456-3458:3456-3458/sctp"})
115117
require.NoError(t, err)
116118
assert.Equal(t, []string{"1234/tcp", "1235/tcp", "1236/tcp", "2345/udp", "2346/udp", "2347/udp", "3456/sctp", "3457/sctp", "3458/sctp"}, exposedPorts)
117119

118-
exposedPorts, err = parsePortSpecs([]string{"0.0.0.0:1234-1236:1234-1236/tcp", "0.0.0.0:2345-2347:2345-2347/udp", "0.0.0.0:3456-3458:3456-3458/sctp"})
120+
exposedPorts, err = ps.parsePorts([]string{"0.0.0.0:1234-1236:1234-1236/tcp", "0.0.0.0:2345-2347:2345-2347/udp", "0.0.0.0:3456-3458:3456-3458/sctp"})
119121
require.NoError(t, err)
120122
assert.Equal(t, []string{"1234/tcp", "1235/tcp", "1236/tcp", "2345/udp", "2346/udp", "2347/udp", "3456/sctp", "3457/sctp", "3458/sctp"}, exposedPorts)
121123

122-
_, err = parsePortSpecs([]string{"localhost:1234-1236:1234-1236/tcp"})
124+
_, err = ps.parsePorts([]string{"localhost:1234-1236:1234-1236/tcp"})
123125
assert.Error(t, err, "Received no error while trying to parse a hostname instead of ip")
124126
}

0 commit comments

Comments
 (0)