dockerfile: rules check for EXPOSE instruction #6135
Conversation
feae557 to
52f3531
Compare
| } | ||
| } | ||
|
|
||
| func (ps *portSpecs) splitParts(rawport string) (hostIP, hostPort, containerPort string) { |
There was a problem hiding this comment.
Curious why you changed these all into methods, because the ps receiver is not used anywhere, so now it's required to first construct a (redundant portSpecs` before being able to use these functions.
There was a problem hiding this comment.
It is used in parsePort method to handle the build check and as other functions are only useful for port specs I was thinking it would be better to encapsulate them.
| return parts[0], parts[1], parts[2] | ||
| default: | ||
| n := len(parts) | ||
| return strings.Join(parts[:n-2], ":"), parts[n-2], parts[n-1] |
There was a problem hiding this comment.
This code should ultimately go away, as it's only for handling the hostIP:hostPort-hostPortEnd:containerPort-containerPortEnd/Proto
Ultimately, all that remains is;
Split <port>[-<endport>][/proto] (which would be a split on /, then splitting on -. Which may be a 5 line-function 😂
There was a problem hiding this comment.
Yes I'm looking to handle this as build check in follow-up for IP address case. In future releases we could just error out if an IP is detected.
Yes I think it makes sense to have a separate warning for IP address case to first warn users of removal in the future. |
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
52f3531 to
7f64931
Compare
|
Pushed extra commit to add a new rule for IP address and host-port mapping |
7f64931 to
d6c82c6
Compare
| in a Dockerfile is used to indicate which ports the container listens on at | ||
| runtime. It should not include an IP address or host-port mapping, as this is | ||
| not the intended use of the `EXPOSE` instruction. Instead, it should only | ||
| specify the port number and optionally the protocol (TCP or UDP). |
There was a problem hiding this comment.
Not sure if we do already, but we should document somewhere that tcp is the default if omitted (also in the OCI spec); https://github.com/opencontainers/image-spec/blob/2daaaaf0e7c16a6a147be91fde277f38573be672/config.md?plain=1#L144
Its keys can be in the format of: `port/tcp`, `port/udp`, `port` with the default protocol being `tcp` if not specified.Perhaps we should also recommend users to always include a proto. @akerouanton may have ideas on that.
follow-up #6128 (comment)
fixes #2173
Adds two rules for
EXPOSEinstruction:ExposeProtoCasing: Protocol in EXPOSE instruction should be lowercase.ExposeInvalidFormat: IP address and host-port mapping should not be used in EXPOSE instruction. This will become an error in a future release.