Add setting to disable tcp/udp server (inbound)

[Steve-Mcl]

fixes #3902

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

Proposed changes

As per issue #3902

TODO 2022/12/04

• ensure it is clear to user why inbound connections are not possible but dont show a the red validation triangle
• add settings to default settings.js file
• update node-red-admin settings.js file

Notes and concerns... (updated 2022/12/04)

• Introduces two new settings flags tcpInAllowInboundConnections and udpInAllowInboundConnections
• These settings are exposed to the client side via the in built mechanism for exposing setting to editor (RED.nodes.registerType("tcp in",TCPin, { settings: {...} }). However this forces the name of the setting to match the node e.g. tcpInSomething or tcpOutSomething. For now, tcpInAllowInboundConnections was chosen but since it is used for the TCP In and TCP Out it looks a little odd asking users to set tcpInAllowInboundConnections for the TCP In and Out nodes?
  • Alternative: map a nicer setting name through to editor programmatically
  • Use 2 variables tcpInAllowInboundConnections / tcpOutAllowInboundConnections
• UI elements are deliberately still shown and still accessible (see next point)
• It may be better to NOT indicate a design time error since a specific installation may permit these nodes for "offline flow building" (for later export to a remote device)
  • Alternatively, additional flags of tcpInValidateAllowInboundConnections and udpInValidateAllowInboundConnections could be employed to turn on/off design time validation
• New settings, for now, have deliberately NOT been included in the default settings file (merits to discuss)
• Currently, this PR does not introduce the feature(s) via ENV settings since they would need mapping through to the editor (for the requested frontend validation) - until above points are ratified and we speak about these points, I did not want to spend too much time heading in (potentially) the wrong direction :) .

Screenshots (Updated 2022/12/04)

TCP

NOTE: The help comment is improved to read: NOTE: If your node shows "inbound connections are disabled", then it has been disabled in the Node-RED settings file.

UDP

NOTE: The help comment is improved to read: NOTE: If your node shows "inbound connections are disabled", then it has been disabled in the Node-RED settings file.

Screenshots (OUTDATED 2022/12/04)

Flow status

TCP In node

TCP Out node

UDP In node

Checklist

• I have read the contribution guidelines
• For non-bugfix PRs, I have discussed this change on the forum/slack team.
• I have run grunt to verify the unit tests pass
• I have added suitable unit tests to cover the new/changed functionality

[coveralls]

Coverage decreased (-0.02%) to 68.178% when pulling b5fae8b on Steve-Mcl:tcp-udp-inbound-disable into c065d25 on node-red:dev.

[hardillb]

First thoughts:

Do we want a flag for all possible options (tcp, udp, ....), or just one overall flag that can be easily shared across any nodes. E.g. we may want to disable the SMTP server option in the node-red-node-email at the same time.

[Steve-Mcl]

First thoughts:

Do we want a flag for all possible options (tcp, udp, ....), or just one overall flag that can be easily shared across any nodes. E.g. we may want to disable the SMTP server option in the node-red-node-email at the same time.

I would (personally) say no in that regard Ben. It can be valid for a provider/implementor to prohibit SNMP/SMTP but allow TCP/UDP for example.

That said, if it was an overall/overriding flag (e.g "InhibitAllIncomingConnections") that we could later iterate to have additional (more scoped) flags then maybe - BUT - I do fear the insinuation would be someone sees "InhibitAllIncomingConnections" and thinks they are covered for all nodes (even contrib nodes). So I would probably edge towards specific node options.

[knolleary]

In principle, I'm mostly happy with these proposed changes, but would like @dceejay's view on it.

To summarise:

• tcpInAllowInboundConnections & updInAllowInboundConnections added as new settings that default to true, but if set to false will prevent the nodes from accepting inbound connections
• The nodes will still allow that to be configured - but with a warning in the UI.
  • I think we could make it a bit more obvious than just the error highlighting on the select box
• If deployed with an inbound configuration, the node will set its status to an appropriate warning that inbound is disabled

[dceejay]

Only thought so far... does it need to show red triangle error ? Is it really misconfigured if it is disabled ? - the status should be enough I think. (and of course that then begs the question from the user... "so how do I enable it ?" - so maybe the config panel needs some Tip text as well)

[Steve-Mcl]

@dceejay I removed validation red triangle. I will revisit this today, will check to see if I included the suggested info and convert draft to ready.

[dceejay]

Looks good to me

[dceejay]

All good here