expose port range #173
expose port range #173
Conversation
fahedouch
force-pushed
the
fix-port-range
branch
from
51cb9c3
to
541c290
Compare
fahedouch
force-pushed
the
fix-port-range
branch
2 times, most recently
from
36c4472
to
8d6648f
Compare
pkg/portutil/portutil.go
Outdated
| } | ||
|
|
||
| return multi_res, nil |
There was a problem hiding this comment.
underscore is discouraged in Go
pkg/portutil/portutil.go
Outdated
|
|
||
| if hostPort != "" && (endPort-startPort) != (endHostPort-startHostPort) { | ||
| if endPort != startPort { | ||
| return nil, errors.Errorf("Invalid ranges specified for container and host Ports: %s and %s", containerPort, hostPort) |
There was a problem hiding this comment.
Go discourages using an error string that start with a lower character
| hostPort: "7000-7005", | ||
| containerPort: "80", | ||
| connectURLPort: 7005, | ||
| err: "connection refused", | ||
| }, |
There was a problem hiding this comment.
Can we also test an invalid range such as -p 7000-7005:79-85
| "github.com/pkg/errors" | ||
| ) | ||
|
|
||
| func ParseFlagP(s string) (*gocni.PortMapping, error) { | ||
| func splitParts(rawport string) (string, string, string) { |
There was a problem hiding this comment.
needs comment lines to explain the ret values
| case 3: | ||
| return parts[0], parts[1], containerport | ||
| default: | ||
| return strings.Join(parts[:n-2], ":"), parts[n-2], containerport |
There was a problem hiding this comment.
The switch is hard to read. Is this default case for IPv6?
There was a problem hiding this comment.
Nope, I targeting Ipv4 here
look a this two example :
./nerdctl --snapshotter native run -d -p 1515:127.0.0.1:80:80 alpine sleep 100
parts = [1515 127.0.0.1 80 80]
parts[n-1] -> 80
parts[n-2] -> 80
parts[:n-2] -> [ 1515 127.0.0.1] So strings.Join(parts[:n-2], ":") => 1515:127.0.0.1 (original input)
./nerdctl --snapshotter native run -d -p 1515-127.0.0.1:80:80 alpine sleep 100
parts = [1515-127.0.0.1 80 80]
parts[n-1] -> 80
parts[n-2] -> 80
parts[:n-2] -> [1515-127.0.0.1] So strings.Join(parts[:n-2], ":") => 1515-127.0.0.1
it is ok for ipv6 too.
There was a problem hiding this comment.
I don't understand what 1515-127.0.0.1:80:80 means. Is it officially supported by Docker?
There was a problem hiding this comment.
@AkihiroSuda it is a wrong IP that a user may try (just an example)
the purpose is to get the IP block and check it.
pkg/portutil/portutil.go
Outdated
| ip, hostPort, containerPort := splitParts(splitBySlash[0]) | ||
|
|
||
| if containerPort == "" { | ||
| return nil, errors.Errorf("No port specified: %s<empty>", splitBySlash[0]) |
fahedouch
force-pushed
the
fix-port-range
branch
2 times, most recently
from
3a19562
to
0624f8f
Compare
fahedouch
force-pushed
the
fix-port-range
branch
2 times, most recently
from
d2dd140
to
5806333
Compare
|
@AkihiroSuda can you re-review plz. |
Signed-off-by: fahed dorgaa <fahed.dorgaa@gmail.com> fix tests Signed-off-by: fahed dorgaa <fahed.dorgaa@gmail.com>
fahedouch
force-pushed
the
fix-port-range
branch
from
8edbc32
to
459eba6
Compare
Refacto
portutil&& fixing #163Signed-off-by: fahed dorgaa fahed.dorgaa@gmail.com