(PUP-8107) Support specifying the source-address for agent #6371
Conversation
|
@jerearista if you want this in 4.10.x you'll need to re-target the PR to that branch so that it can be merged up from there. |
|
Also, please squash the commits :) |
|
CLA signed by all contributors. |
jerearista
force-pushed
the
source-ip
branch
from
05fb9d4
to
7d3c540
Compare
jerearista
force-pushed
the
source-ip
branch
3 times, most recently
from
c579288
to
f5adb3a
Compare
|
Thanks, @shermdog. I rebased to 4.10.x and squashed commits. The Travis failure is due to Net::HTTP local_host method not existing before Ruby 2.0. |
lib/puppet/network/http/factory.rb
Outdated
| @@ -38,6 +38,11 @@ def create_connection(site) | |||
| http.read_timeout = Puppet[:http_read_timeout] | |||
| http.open_timeout = Puppet[:http_connect_timeout] | |||
|
|
|||
| if Puppet[:sourceaddress] | |||
| Puppet.debug("Using source IP #{Puppet[:sourceaddress]}") | |||
| http.local_host = Puppet[:sourceaddress] | |||
There was a problem hiding this comment.
Puppet still needs to support MRI 1.9.3 because it is embedded in JRuby 1.7. I would add a check here to raise an exception when running on < 2.0, something like
if RUBY_VERSION =~ /1\./
raise ...
else
http.local_host = ...
endOr just check if http.responds_to?(:local_host=)
|
Thanks for the background. I will get that added.
…On Fri, Nov 17, 2017 at 2:05 PM, Josh Cooper ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In lib/puppet/network/http/factory.rb
<#6371 (comment)>:
> @@ -38,6 +38,11 @@ def create_connection(site)
http.read_timeout = Puppet[:http_read_timeout]
http.open_timeout = Puppet[:http_connect_timeout]
+ if Puppet[:sourceaddress]
+ Puppet.debug("Using source IP #{Puppet[:sourceaddress]}")
+ http.local_host = Puppet[:sourceaddress]
Puppet still needs to support MRI 1.9.3 because it is embedded in JRuby
1.7. I would add a check here to raise an exception when running on < 2.0,
something like
if RUBY_VERSION =~ /1\./
raise ...else
http.local_host = ...end
Or just check if http.responds_to?(:local_host=)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6371 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHedgL9apx6hwtaPEA_ale5YvbGeVrB_ks5s3djugaJpZM4QhIkd>
.
--
Jere Julian | Extensibility Engineer
Arista Networks <http://arista.com> | EOS+ Consulting Services
Mobile: 919-250-8217 | Office: 408-547-8418 | jere@arista.com
|
jerearista
force-pushed
the
source-ip
branch
from
f5adb3a
to
6cf1c90
Compare
lib/puppet/network/http/factory.rb
Outdated
| if Puppet[:sourceaddress] && http.respond_to?(:local_host) | ||
| Puppet.debug("Using source IP #{Puppet[:sourceaddress]}") | ||
| http.local_host = Puppet[:sourceaddress] | ||
| end |
There was a problem hiding this comment.
After thinking some more about this, I think we want to raise if it doesn't respond to the method, otherwise, we might wonder why it's not working as intended.
There was a problem hiding this comment.
Added an exception and testcase.
jerearista
force-pushed
the
source-ip
branch
from
6cf1c90
to
ef27e00
Compare
lib/puppet/network/http/factory.rb
Outdated
| @@ -38,6 +38,13 @@ def create_connection(site) | |||
| http.read_timeout = Puppet[:http_read_timeout] | |||
| http.open_timeout = Puppet[:http_connect_timeout] | |||
|
|
|||
| if Puppet[:sourceaddress] | |||
| msg = 'Setting source address is unsupported by this version of Net::HTTP.' | |||
| raise Net::HTTPError.new(msg, 400) unless http.respond_to?(:local_host) | |||
There was a problem hiding this comment.
How about raising an ArgumentError instead of HTTP 400?
There was a problem hiding this comment.
Update made. Yes, that's a much better choice for the user instead of just indicating that the library doesn't support the option.
In an environment with multiple IP addresses on a system, there are cases where the default system-selected source IP address may not be appropriate for reaching the Puppet server due to access-lists, policy, etc. This adds the `--sourceaddress <ADDRESS>` option to the CLI allowing the user to supply the desired source IP address or hostname for outbound connections. Add check if Net::HTTP supports local_host (Ruby 2.x) and raise an ArgumentError when unsupported.
jerearista
force-pushed
the
source-ip
branch
from
ef27e00
to
c500335
Compare
|
Thanks @jerearista! I made one additional change, and will merge it in PR #6393. |
In an environment with multiple IP addresses on a system, there are cases where the default system-selected source IP address may not be appropriate for reaching the Puppet server due to access-lists, policy, etc. This adds the
--sourceaddress <ADDRESS>option to the CLI andpuppet.confallowing the user to supply the desired source IP address or hostname for outbound connections.