chore: correct port validation

[vzhukovs]

What does this PR do?

The following changes request provides better port validation process for Run Images dialog.

Screenshot / video of UI

What issues does this PR fix or reference?

#6390

How to test this PR?

Try to create two different containers with same exposed port to check that new port is proposed in Run Image dialog and validation of input works proper.

• Tests are covering the bug fix or the new feature

[axel7083]

I am not a big fan of this library as it has no support, not been updated since Aug 11, 2021, it fetched two other library the is-admin and is-root which have the same problem.

The is-admin is running the following on windows fsutil dirty query C: and the is-root is simply checking the user uid..

I don't know what is our policy for adding packages cc @benoitf

[benoitf]

I would not proceed with this library

[benoitf]

I mean, the library is OK from maintenance POV as they're managed by sindresorhus so there is support

but I'm not sure why we would need that library, I mean, in which case people run Podman Desktop with admin privileges ?

I would not allow ports < 1024 in all cases (Windows, macOS or Linux)

also Podman Desktop could be run in admin mode but the gvproxy in user mode so it would fail anyway

[vzhukovs]

I would not allow ports < 1024 in all cases (Windows, macOS or Linux)

This simplifies the whole checks, will going to update the PR by removing this check.

I just rely on that scenario, that all ports, that are below 1024 by default requires administrative permissions to expose.

[benoitf]

@vzhukovs yes but I think it's simpler/easier/more robust to not allow that anyway using Podman Desktop

thanks for updating the PR

[vzhukovs]

@benoitf new changes are in place

[benoitf]

I'm wondering why we need to have an object being returned that provides the value and an optional error message vs the method returning the value or throw an error ?

I mean, why we can't rely on catching an error vs returning an object ?

[vzhukovs]

I'm wondering why we need to have an object being returned that provides the value and an optional error message vs the method returning the value or throw an error ?

I mean, why we can't rely on catching an error vs returning an object ?

Because, we will have situation, when the method will return true when port is available and false when some check was not passed and not caused by any exception, but here we have to know what was the reason for that. Also, then we can catch some system exception (like EACCESS or EADDRINUSE or some else) and we will have the situation, when this scenario will be the same as return false but on the client side we have to catch this exception.

It's just to simplify the processing the final checking result. Because when the port is not available, there should be a reason why. WDYT?

[deboer-tim]

I'd say the same thing as @benoitf: there doesn't seem to be a need for a new type to return both a boolean and a message: either the port is free and we need no message or it's not and we need a message. We don't need to expose any underlying errors directly - we can catch them and throw new Error('Port must be...') to give our own more useful messages where the code currently does.

[vzhukovs]

@deboer-tim @axel7083 new changes are in place

[axel7083]

Nice having custom message 👍

[axel7083]

LGTM

[deboer-tim]

Approved.

Side note: I wonder if we should use NumberInput here (in another PR), would block non-numerical entry and provide +/- for small changes.

[vzhukovs]

Approved.

Side note: I wonder if we should use NumberInput here (in another PR), would block non-numerical entry and provide +/- for small changes.

Submitted an issue for this proposal: #7198