fix service port number lookup to use protocol #1023
Conversation
firewall is a typethat may have no external impact to Forge modules. This module is declared in 105 of 578 indexed public
|
|
Hrm, looks like the test case "'040 partial invert" needs adjustment? It now bombs since "http/udp" does not exist in the test harness (it does exist on my Fedora!). This error was hidden earlier since it looked up "http/tcp". |
|
This PR has been marked as stale because it has been open for a while and has had no recent activity. If this PR is still important to you please drop a comment below and we will add this to our backlog to complete. Otherwise, it will be closed in 7 days. |
|
This bug is still relevant, and the patch still applies cleanly. |
|
Hi @kjetilho, thanks for letting us know. We are using the stale-bot as a tool to sort our current PRs and figure out which ones are relevant and which ones might be outdated. We will be putting your PR into the active column and, hopefully, we will be able to review it soon. Thanks for your patience. |
|
Closing and opening to re-kick automated testing. |
|
Hi @kjetilho, there seems to be a spec test that is failing currently on your PR. The log is pointing at the following file:
Can you investigate this issue and (if related to your PR) make the necessary changes for the test to pass? |
|
@kjetilho Any movement on this? |
kjetilho
force-pushed
the
fix_string_to_port
branch
from
beea16b to
a2cb174
Compare
|
yep, I can reproduce when testing locally. will fix soon. thanks! |
kjetilho
force-pushed
the
fix_string_to_port
branch
from
a2cb174 to
3208d52
Compare
|
@kjetilho Thanks for this - can you rebase with the current main so we can proceed. |
The existing code passes `:proto`, which `string_to_port` casts to a string, gets "proto", compares that to the possibilities "udp" or "tcp", and when neither, falls back to using "tcp". This patch passes the actual proto value to the function, in case there is a UDP specific service in your /etc/services (uncommon, but it happens). It looks like Puppet will evaluate the properties in declared order, so I had to move `newproperty(:proto)` up so `@resource[:proto]` was available in the code for `sport`, `dport` and `port`.
Switch to "talk" (port 517), since that is an UDP only service in the default Debian and RedHat /etc/services.
kjetilho
force-pushed
the
fix_string_to_port
branch
from
3208d52 to
ae64598
Compare
|
not sure if you are notified when I push a rebase, so adding a comment: "sure!" :) |
The existing code passes
:proto, whichstring_to_portcasts to astring, gets "proto", compares that to the possibilities "udp" or "tcp",
and when neither, falls back to using "tcp".
This patch passes the actual proto value to the function, in case there is
a UDP specific service in your /etc/services (uncommon, but it happens).
It looks like Puppet will evaluate the properties in declared order,
so I had to move
newproperty(:proto)up so@resource[:proto]wasavailable in the code for
sport,dportandport.