Closed iamkirkbater closed 2 months ago
This looks great, thanks for talking through the changes with me. :shippit:
/lgtm /approve
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: clcollins, iamkirkbater
The full list of commands accepted by this bot can be found here.
The pull request process is described here
During some testing we realized that by using the
--publish-all
functionality we inadvertently open up the exposed port to all other hosts on our local networks, posing a potential security risk.This changes that functionality so that by default it binds the console port to the local interface, so that you can only access the service running in ocm-container from your local machine and not other hosts on the network.
Effectively changes:
I ssh'd into another machine and curl'd my macbook and it served the contents running on port 43407 to my other machine. After changing the binding to the bottom example, my macbook replied with
curl: (7) Failed to connect to 192.168.1.212 port 35023: Connection refused
As an architecture decision, I figured that it would be better to add a LocalPorts map to the ContainerRef so that we could optionally further expand the availability of ports in the future. For example, in some recent testing it would have been nice to have the ability to have a console running as well as the ability to port forward for PromLens; though that can come in a future PR.