dockerfile: rule check for protocol casing in EXPOSE instruction

Signed-off-by: CrazyMax <[email protected]>

Author: crazy-max

frontend/dockerfile/dockerfile2llb/convert.go

@@ -971,7 +971,7 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error {
 	case *instructions.HealthCheckCommand:
 		err = dispatchHealthcheck(d, c, opt.lint)
 	case *instructions.ExposeCommand:
-		err = dispatchExpose(d, c, opt.shlex)
+		err = dispatchExpose(d, c, &opt)
 	case *instructions.UserCommand:
 		err = dispatchUser(d, c, true)
 	case *instructions.VolumeCommand:

frontend/dockerfile/dockerfile2llb/convert_expose.go

@@ -7,93 +7,95 @@ import (
 	"strings"
 
 	"github.com/moby/buildkit/frontend/dockerfile/instructions"
-	"github.com/moby/buildkit/frontend/dockerfile/shell"
+	"github.com/moby/buildkit/frontend/dockerfile/linter"
+	"github.com/moby/buildkit/frontend/dockerfile/parser"
 	"github.com/pkg/errors"
 )
 
-func dispatchExpose(d *dispatchState, c *instructions.ExposeCommand, shlex *shell.Lex) error {
+func dispatchExpose(d *dispatchState, c *instructions.ExposeCommand, opt *dispatchOpt) error {
 	ports := []string{}
 	env := getEnv(d.state)
 	for _, p := range c.Ports {
-		ps, err := shlex.ProcessWords(p, env)
+		ps, err := opt.shlex.ProcessWords(p, env)
 		if err != nil {
 			return err
 		}
 		ports = append(ports, ps...)
 	}
 	c.Ports = ports
 
-	ps, err := parsePortSpecs(c.Ports)
+	ps := newPortSpecs(
+		withLocation(c.Location()),
+		withLint(opt.lint),
+	)
+
+	psp, err := ps.parsePorts(c.Ports)
 	if err != nil {
 		return err
 	}
 
 	if d.image.Config.ExposedPorts == nil {
 		d.image.Config.ExposedPorts = make(map[string]struct{})
 	}
-	for _, p := range ps {
+	for _, p := range psp {
 		d.image.Config.ExposedPorts[p] = struct{}{}
 	}
 
 	return commitToHistory(&d.image, fmt.Sprintf("EXPOSE %v", ps), false, nil, d.epoch)
 }
 
-// parsePortSpecs receives port specs in the format of [ip:]public:private/proto
-// and returns them as a list of "port/proto".
-func parsePortSpecs(ports []string) (exposedPorts []string, _ error) {
-	for _, p := range ports {
-		portProtos, err := parsePortSpec(p)
-		if err != nil {
-			return nil, err
-		}
-		exposedPorts = append(exposedPorts, portProtos...)
-	}
-	return exposedPorts, nil
+type portSpecs struct {
+	location []parser.Range
+	lint     *linter.Linter
 }
 
-// splitProtoPort splits a port(range) and protocol, formatted as "<portnum>/[<proto>]"
-// "<startport-endport>/[<proto>]". It returns an error if no port(range) or
-// an invalid proto is provided. If no protocol is provided, the default ("tcp")
-// protocol is returned.
-func splitProtoPort(rawPort string) (proto string, port string, _ error) {
-	port, proto, _ = strings.Cut(rawPort, "/")
-	if port == "" {
-		return "", "", errors.New("no port specified")
+type portSpecsOption func(ps *portSpecs)
+
+func withLocation(location []parser.Range) portSpecsOption {
+	return func(ps *portSpecs) {
+		ps.location = location
 	}
-	proto = strings.ToLower(proto)
-	switch proto {
-	case "":
-		return "tcp", port, nil
-	case "tcp", "udp", "sctp":
-		return proto, port, nil
-	default:
-		return "", "", errors.New("invalid proto: " + proto)
+}
+
+func withLint(lint *linter.Linter) portSpecsOption {
+	return func(ps *portSpecs) {
+		ps.lint = lint
 	}
 }
 
-func splitParts(rawport string) (hostIP, hostPort, containerPort string) {
-	parts := strings.Split(rawport, ":")
+func newPortSpecs(opts ...portSpecsOption) *portSpecs {
+	ps := &portSpecs{}
+	for _, opt := range opts {
+		opt(ps)
+	}
+	return ps
+}
 
-	switch len(parts) {
-	case 1:
-		return "", "", parts[0]
-	case 2:
-		return "", parts[0], parts[1]
-	case 3:
-		return parts[0], parts[1], parts[2]
-	default:
-		n := len(parts)
-		return strings.Join(parts[:n-2], ":"), parts[n-2], parts[n-1]
+// parsePorts receives port specs in the format of [ip:]public:private/proto
+// and returns them as a list of "port/proto".
+func (ps *portSpecs) parsePorts(ports []string) (exposedPorts []string, _ error) {
+	for _, p := range ports {
+		portProtos, err := ps.parsePort(p)
+		if err != nil {
+			return nil, err
+		}
+		exposedPorts = append(exposedPorts, portProtos...)
 	}
+	return exposedPorts, nil
 }
 
-// parsePortSpec parses a port specification string into a slice of "<portnum>/[<proto>]"
-func parsePortSpec(rawPort string) (portProto []string, _ error) {
-	ip, hostPort, containerPort := splitParts(rawPort)
-	proto, containerPort, err := splitProtoPort(containerPort)
+// parsePort parses a port specification string into a slice of "<portnum>/[<proto>]"
+func (ps *portSpecs) parsePort(rawPort string) (portProto []string, _ error) {
+	ip, hostPort, containerPort := ps.splitParts(rawPort)
+	proto, containerPort, err := ps.splitProtoPort(containerPort)
 	if err != nil {
 		return nil, errors.Wrapf(err, "invalid port: %q", rawPort)
 	}
+	if lcproto := strings.ToLower(proto); proto != lcproto && ps.lint != nil {
+		msg := linter.RuleExposeProtoCasing.Format(rawPort)
+		ps.lint.Run(&linter.RuleExposeProtoCasing, ps.location, msg)
+		proto = lcproto
+	}
 
 	// TODO(thaJeztah): mapping IP-addresses should not be allowed for EXPOSE; see https://github.com/moby/buildkit/issues/2173
 	if ip != "" && ip[0] == '[' {
@@ -108,14 +110,14 @@ func parsePortSpec(rawPort string) (portProto []string, _ error) {
 		return nil, errors.New("invalid IP address: " + ip)
 	}
 
-	startPort, endPort, err := parsePortRange(containerPort)
+	startPort, endPort, err := ps.parsePortRange(containerPort)
 	if err != nil {
 		return nil, errors.New("invalid containerPort: " + containerPort)
 	}
 
 	// TODO(thaJeztah): mapping host-ports should not be allowed for EXPOSE; see https://github.com/moby/buildkit/issues/2173
 	if hostPort != "" {
-		startHostPort, endHostPort, err := parsePortRange(hostPort)
+		startHostPort, endHostPort, err := ps.parsePortRange(hostPort)
 		if err != nil {
 			return nil, errors.New("invalid hostPort: " + hostPort)
 		}
@@ -139,21 +141,21 @@ func parsePortSpec(rawPort string) (portProto []string, _ error) {
 }
 
 // parsePortRange parses and validates the specified string as a port range (e.g., "8000-9000").
-func parsePortRange(ports string) (startPort, endPort int, _ error) {
+func (ps *portSpecs) parsePortRange(ports string) (startPort, endPort int, _ error) {
 	if ports == "" {
 		return 0, 0, errors.New("empty string specified for ports")
 	}
 	start, end, ok := strings.Cut(ports, "-")
 
-	startPort, err := parsePortNumber(start)
+	startPort, err := ps.parsePortNumber(start)
 	if err != nil {
 		return 0, 0, errors.Wrapf(err, "invalid start port '%s'", start)
 	}
 	if !ok || start == end {
 		return startPort, startPort, nil
 	}
 
-	endPort, err = parsePortNumber(end)
+	endPort, err = ps.parsePortNumber(end)
 	if err != nil {
 		return 0, 0, errors.Wrapf(err, "invalid end port '%s'", end)
 	}
@@ -165,7 +167,7 @@ func parsePortRange(ports string) (startPort, endPort int, _ error) {
 
 // parsePortNumber parses rawPort into an int, unwrapping strconv errors
 // and returning a single "out of range" error for any value outside 0–65535.
-func parsePortNumber(rawPort string) (int, error) {
+func (ps *portSpecs) parsePortNumber(rawPort string) (int, error) {
 	if rawPort == "" {
 		return 0, errors.New("value is empty")
 	}
@@ -183,3 +185,38 @@ func parsePortNumber(rawPort string) (int, error) {
 
 	return int(port), nil
 }
+
+// splitProtoPort splits a port(range) and protocol, formatted as "<portnum>/[<proto>]"
+// "<startport-endport>/[<proto>]". It returns an error if no port(range) or
+// an invalid proto is provided. If no protocol is provided, the default ("tcp")
+// protocol is returned.
+func (ps *portSpecs) splitProtoPort(rawPort string) (proto string, port string, _ error) {
+	port, proto, _ = strings.Cut(rawPort, "/")
+	if port == "" {
+		return "", "", errors.New("no port specified")
+	}
+	switch strings.ToLower(proto) {
+	case "":
+		return "tcp", port, nil
+	case "tcp", "udp", "sctp":
+		return proto, port, nil
+	default:
+		return "", "", errors.New("invalid proto: " + proto)
+	}
+}
+
+func (ps *portSpecs) splitParts(rawport string) (hostIP, hostPort, containerPort string) {
+	parts := strings.Split(rawport, ":")
+
+	switch len(parts) {
+	case 1:
+		return "", "", parts[0]
+	case 2:
+		return "", parts[0], parts[1]
+	case 3:
+		return parts[0], parts[1], parts[2]
+	default:
+		n := len(parts)
+		return strings.Join(parts[:n-2], ":"), parts[n-2], parts[n-1]
+	}
+}

frontend/dockerfile/dockerfile2llb/convert_expose_test.go

@@ -7,7 +7,9 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestParsePortSpecEmptyContainerPort(t *testing.T) {
+var ps = newPortSpecs()
+
+func TestParsePortEmptyContainerPort(t *testing.T) {
 	tests := []struct {
 		name     string
 		spec     string
@@ -31,7 +33,7 @@ func TestParsePortSpecEmptyContainerPort(t *testing.T) {
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			_, err := parsePortSpec(tc.spec)
+			_, err := ps.parsePort(tc.spec)
 			if tc.expError != "" {
 				assert.EqualError(t, err, tc.expError)
 			} else {
@@ -41,13 +43,13 @@ func TestParsePortSpecEmptyContainerPort(t *testing.T) {
 	}
 }
 
-func TestParsePortSpecFull(t *testing.T) {
-	exposedPorts, err := parsePortSpec("0.0.0.0:1234-1235:3333-3334/tcp")
+func TestParsePortFull(t *testing.T) {
+	exposedPorts, err := ps.parsePort("0.0.0.0:1234-1235:3333-3334/tcp")
 	require.NoError(t, err)
 	assert.Equal(t, []string{"3333/tcp", "3334/tcp"}, exposedPorts)
 }
 
-func TestPartPortSpecIPV6(t *testing.T) {
+func TestPartPortIPV6(t *testing.T) {
 	type test struct {
 		name     string
 		spec     string
@@ -82,43 +84,43 @@ func TestPartPortSpecIPV6(t *testing.T) {
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
-			exposedPorts, err := parsePortSpec(c.spec)
+			exposedPorts, err := ps.parsePort(c.spec)
 			require.NoError(t, err)
 			assert.Equal(t, c.expected, exposedPorts)
 		})
 	}
 }
 
-func TestParsePortSpecs(t *testing.T) {
-	exposedPorts, err := parsePortSpecs([]string{"1234/tcp", "2345/udp", "3456/sctp"})
+func TestParsePorts(t *testing.T) {
+	exposedPorts, err := ps.parsePorts([]string{"1234/tcp", "2345/udp", "3456/sctp"})
 	require.NoError(t, err)
 	assert.Equal(t, []string{"1234/tcp", "2345/udp", "3456/sctp"}, exposedPorts)
 
-	exposedPorts, err = parsePortSpecs([]string{"1234:1234/tcp", "2345:2345/udp", "3456:3456/sctp"})
+	exposedPorts, err = ps.parsePorts([]string{"1234:1234/tcp", "2345:2345/udp", "3456:3456/sctp"})
 	require.NoError(t, err)
 	assert.Equal(t, []string{"1234/tcp", "2345/udp", "3456/sctp"}, exposedPorts)
 
-	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"})
+	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"})
 	require.NoError(t, err)
 	assert.Equal(t, []string{"1234/tcp", "2345/udp", "3456/sctp"}, exposedPorts)
 
-	_, err = parsePortSpecs([]string{"localhost:1234:1234/tcp"})
+	_, err = ps.parsePorts([]string{"localhost:1234:1234/tcp"})
 	assert.Error(t, err, "Received no error while trying to parse a hostname instead of ip")
 }
 
-func TestParsePortSpecsWithRange(t *testing.T) {
-	exposedPorts, err := parsePortSpecs([]string{"1234-1236/tcp", "2345-2347/udp", "3456-3458/sctp"})
+func TestParsePortsWithRange(t *testing.T) {
+	exposedPorts, err := ps.parsePorts([]string{"1234-1236/tcp", "2345-2347/udp", "3456-3458/sctp"})
 	require.NoError(t, err)
 	assert.Equal(t, []string{"1234/tcp", "1235/tcp", "1236/tcp", "2345/udp", "2346/udp", "2347/udp", "3456/sctp", "3457/sctp", "3458/sctp"}, exposedPorts)
 
-	exposedPorts, err = parsePortSpecs([]string{"1234-1236:1234-1236/tcp", "2345-2347:2345-2347/udp", "3456-3458:3456-3458/sctp"})
+	exposedPorts, err = ps.parsePorts([]string{"1234-1236:1234-1236/tcp", "2345-2347:2345-2347/udp", "3456-3458:3456-3458/sctp"})
 	require.NoError(t, err)
 	assert.Equal(t, []string{"1234/tcp", "1235/tcp", "1236/tcp", "2345/udp", "2346/udp", "2347/udp", "3456/sctp", "3457/sctp", "3458/sctp"}, exposedPorts)
 
-	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"})
+	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"})
 	require.NoError(t, err)
 	assert.Equal(t, []string{"1234/tcp", "1235/tcp", "1236/tcp", "2345/udp", "2346/udp", "2347/udp", "3456/sctp", "3457/sctp", "3458/sctp"}, exposedPorts)
 
-	_, err = parsePortSpecs([]string{"localhost:1234-1236:1234-1236/tcp"})
+	_, err = ps.parsePorts([]string{"localhost:1234-1236:1234-1236/tcp"})
 	assert.Error(t, err, "Received no error while trying to parse a hostname instead of ip")
 }

frontend/dockerfile/dockerfile_lint_test.go

@@ -49,6 +49,7 @@ var lintTests = integration.TestFuncs(
 	testFromPlatformFlagConstDisallowed,
 	testCopyIgnoredFiles,
 	testDefinitionDescription,
+	testExposeProtoCasing,
 )
 
 func testDefinitionDescription(t *testing.T, sb integration.Sandbox) {
@@ -1435,6 +1436,34 @@ FROM --platform=linux/amd64 scratch AS linux
 	})
 }
 
+func testExposeProtoCasing(t *testing.T, sb integration.Sandbox) {
+	dockerfile := []byte(`
+FROM busybox
+EXPOSE 80/TcP 8080/TCP 8080/udp
+`)
+	checkLinterWarnings(t, sb, &lintTestParams{
+		Dockerfile: dockerfile,
+		Warnings: []expectedLintWarning{
+			{
+				RuleName:    "ExposeProtoCasing",
+				Description: "Protocol in EXPOSE instruction should be lowercase",
+				URL:         "https://docs.docker.com/go/dockerfile/rule/expose-proto-casing/",
+				Detail:      "Defined protocol '80/TcP' in EXPOSE instruction should be lowercase",
+				Level:       1,
+				Line:        3,
+			},
+			{
+				RuleName:    "ExposeProtoCasing",
+				Description: "Protocol in EXPOSE instruction should be lowercase",
+				URL:         "https://docs.docker.com/go/dockerfile/rule/expose-proto-casing/",
+				Detail:      "Defined protocol '8080/TCP' in EXPOSE instruction should be lowercase",
+				Level:       1,
+				Line:        3,
+			},
+		},
+	})
+}
+
 func checkUnmarshal(t *testing.T, sb integration.Sandbox, lintTest *lintTestParams) {
 	destDir, err := os.MkdirTemp("", "buildkit")
 	require.NoError(t, err)

frontend/dockerfile/docs/rules/_index.md

@@ -107,5 +107,9 @@ To learn more about how to use build checks, see
       <td><a href="./invalid-definition-description/">InvalidDefinitionDescription (experimental)</a></td>
       <td>Comment for build stage or argument should follow the format: `# <arg/stage name> <description>`. If this is not intended to be a description comment, add an empty line or comment between the instruction and the comment.</td>
     </tr>
+    <tr>
+      <td><a href="./expose-proto-casing/">ExposeProtoCasing</a></td>
+      <td>Protocol in EXPOSE instruction should be lowercase</td>
+    </tr>
   </tbody>
 </table>