1032133 - Proxy update: need direct connection for 'puppet', and 'repos'

Status: RESOLVED FIXED

(Infrastructure & Operations :: RelOps: Puppet, task, P1)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Last Friday I tried to get the Java installation working on OS X, but it failed because we have a proxy set. So when installing the DMG via the pkgdmg manifest, it cannot grab the referenced installer from /repos. As you can see below, the use of a proxy server is enforced:

Debug: Executing '/usr/bin/curl -o /var/folders/zz/zyxvpxvq6csfxvn_n0000000000000/T/d20140627-78104-1fzhkr9/java-7.0.600.dmg.dmg -C - -k -L -s --fail --url http://puppet/repos/DMGs/java-7.0.600.dmg --proxy proxy.dmz.scl3.mozilla.com:3128'
Debug: curl -o /var/folders/zz/zyxvpxvq6csfxvn_n0000000000000/T/d20140627-78104-1fzhkr9/java-7.0.600.dmg.dmg -C - -k -L -s --fail --url http://puppet/repos/DMGs/java-7.0.600.dmg --proxy proxy.dmz.scl3.mozilla.com:3128 did not transfer [java-7.0.600.dmg].  Falling back to local file.
Error: Could not set 'present' on ensure: No such file or directory - http://puppet/repos/DMGs/java-7.0.600.dmg at 30:/etc/puppet/environments/hskupin/env/modules/packages/manifests/pkgdmg.pp

Dustin, what's the best option you could think of here?

IMHO we have two options:

1. Setup ACL for puppetmaster, so the proxy will never be used.
2. Stop using a proxy on puppetmaster

I'm not sure if step 2 is possible given that we want to sync external repositores eg. for Linux distributions.

Comment 1

[Dustin J. Mitchell [:dustin] (he/him)]

We should probably add a proxy exception for un-qualified names 'puppet' and 'repos'.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Alright. So moving this over to NetOps. Here the requirement...

Any access to 'puppet' and 'repos' should resolve directly and not via the squid proxy to our puppetmaster1.qa.scl3.mozilla.com. Reason is that puppetagain is forcing the usage of the proxy, and requesting pages/files from the internal network fails. We cannot change that on our side given that puppet does not make use of the no_proxy environment variable, which actually has this set.

puppet -> puppetmaster1.qa.scl3.mozilla.com *
repos -> puppetmaster1.qa.scl3.mozilla.com *

Comment 3

[Dustin J. Mitchell [:dustin] (he/him)]

There's nothing netops can do here -- the flows are within the same VLAN.  Sorry I misled you!

What needs to happen is that the puppet agent needs to *not* use the proxy for access to 'puppet' and 'repos', at least in this specific case.  If puppet doesn't support no_proxy, then we may need to rethink having puppet use a proxy at all.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Dustin J. Mitchell [:dustin] from comment #3)
> There's nothing netops can do here -- the flows are within the same VLAN. 

Right, but NetOps can indeed update the proxy.pac for squid here, to force a direct connection if a request to this host has been initiated. But if you think that it should be done inside of puppet, we have to find a way doing so.

> What needs to happen is that the puppet agent needs to *not* use the proxy
> for access to 'puppet' and 'repos', at least in this specific case.  If
> puppet doesn't support no_proxy, then we may need to rethink having puppet
> use a proxy at all.

That would mean we use a proxy or not. It will make it harder for us to upgrade packages by downloading them, but we could put them in /data/repos via scp.

Comment 5

[Dustin J. Mitchell [:dustin] (he/him)]

'puppet' only points to the QA puppetmasters from the QA VLAN.  I don't know if squid is flexible enough to look at the source IP when making such a decision, but that should probably be a last option.

As I understand it, puppet ignores the systemwide proxy settings. What would you lose if you just remove the bit that configures *puppet's* proxy settings?  Then you can still do rsyncs, etc. via command line, but fetches of 'http://puppet/foo' will not try to go via proxy.  I think I'm missing a key piece of information here..

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Dustin J. Mitchell [:dustin] from comment #5)
> As I understand it, puppet ignores the systemwide proxy settings. What would

No, it only obeys http_proxy but fails to check noproxy. So each request puppet is doing, is actually forced to go through the proxy.

> you lose if you just remove the bit that configures *puppet's* proxy
> settings?

How and where to do this? When I see this failure when running puppet agent, I assume we should remove the http_proxy env variable when calling the puppet master process? It's not working when I do it for the puppet agent call. Also where would it have to be set?

Comment 7

[Dustin J. Mitchell [:dustin] (he/him)]

I could swear that in another bug, you put proxy settings in puppet.conf.  I was suggesting removing those.  Am I mis-remembering?

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Yes, that was /etc/profile.puppet.d/proxy.sh. That one gets included via base.pp, which also server and puppetmaster node types are based off.

Comment 9

[Dustin J. Mitchell [:dustin] (he/him)]

So yes, I was totally mis-remembering!  Sorry!

So puppet is relaying the settings from the environment variables, and passing those through as explicit arguments to curl
  https://github.com/puppetlabs/puppet/blob/master/lib/puppet/provider/package/pkgdmg.rb#L85
but doesn't include anything from noproxy, as you said in comment 6.

The proxy gets the hostname 'puppet', and rejects it with a 403 response (presumably because it's unqualified).  Using other hostnames, such as puppetmaster1.qa.scl3.mozilla.com, doesn't work either -- the proxy host has no access to the puppetmaster (as well it shouldn't!).

Looking at
  https://github.com/puppetlabs/puppet/blob/master/lib/puppet/util/http_proxy.rb
the environment variables take higher precedence than the puppet settings, so even adding http_proxy_{host,port} to puppet.conf wouldn't help.

The `curl` invocation takes place on the client, not on the puppetmaster, so disabling proxies host-wide on the puppetmaster won't help.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Ok, good news here! Unsetting both http_proxy and HTTP_PROXY environment variables fixed the problem, and puppet agent can successfully install the Java package. So we might want to update the crontab job for puppet agent to unset the proxy. And indeed the agent never would have to access any host in the internet!

Comment 11

[Dustin J. Mitchell [:dustin] (he/him)]

That would need updating both in the crontab and in the startup script (on all OS's).

That's not a great long-term solution, since it means running 'puppet agent --test' on the command line will fail, but it will work for now!

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Dustin, a question for OS X here. In the file puppet-atboot.plist we have a reference to '/usr/local/bin/run-puppet.sh'. Sadly I don't find any of such a file in the repository. Also no reference to copy it from somewhere. So from where do we get this file?

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Ups. I was searching for run_puppet and not run-puppet. All clear here.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I was remembering that I tried to use 'env -i' when Dustin mentinoed it to me on IRC. It was working at that time, but now I only get failures from puppet like:

Error: Could not create resources for managing Puppet's files and directories in sections [:main, :agent, :ssl]: undefined method `exists?' for nil:NilClass
Error: Could not prepare for execution: Could not create resources for managing Puppet's files and directories in sections [:main, :agent, :ssl]: undefined method `exists?' for nil:NilClass
undefined method `exists?' for nil:NilClass

Not sure what this new failure is now. I may have to check, but it can be that we would have to unset each individual proxy environment variable.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Ok, so the above failure is visible when the TERM and PATH environment variables are getting removed. Maybe other combinations could also be involved.

The stack I get is:

Error: Could not create resources for managing Puppet's files and directories in sections [:main, :agent, :ssl]: undefined method `exists?' for nil:NilClass
/Library/Ruby/Site/puppet/type/user.rb:381:in `exists?'
/Library/Ruby/Site/puppet/settings.rb:768:in `service_user_available?'
/Library/Ruby/Site/puppet/settings/file_setting.rb:52:in `safe_to_use_settings_value?'
/Library/Ruby/Site/puppet/settings/file_setting.rb:43:in `value'
/Library/Ruby/Site/puppet/settings/file_setting.rb:106:in `owner'
/Library/Ruby/Site/puppet/settings/file_setting.rb:160:in `to_resource'
/Library/Ruby/Site/puppet/settings.rb:882:in `block in to_catalog'
/Library/Ruby/Site/puppet/settings.rb:879:in `each'
/Library/Ruby/Site/puppet/settings.rb:879:in `to_catalog'
/Library/Ruby/Site/puppet/settings.rb:940:in `use'
/Library/Ruby/Site/puppet/application/agent.rb:403:in `setup'
/Library/Ruby/Site/puppet/application.rb:369:in `block (2 levels) in run'
/Library/Ruby/Site/puppet/application.rb:477:in `plugin_hook'
/Library/Ruby/Site/puppet/application.rb:369:in `block in run'
/Library/Ruby/Site/puppet/util.rb:479:in `exit_on_fail'
/Library/Ruby/Site/puppet/application.rb:369:in `run'
/Library/Ruby/Site/puppet/util/command_line.rb:137:in `run'
/Library/Ruby/Site/puppet/util/command_line.rb:91:in `execute'
/usr/bin/puppet:4:in `<main>'

So maybe it's best to only remove the HTTP_PROXY and http_proxy environment variables and leave all the others intact. Dustin, what do you think?

Comment 16

[Dustin J. Mitchell [:dustin] (he/him)]

I agree -- removing all env vars is too much.  It works for curl, but not for puppet.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

As talked with Dustin on IRC today, a separate script which contains all the unset commands for every kind of proxy setting might help here. That way we could even use it for testing changes via environments in the shell.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: no proxy for agent v1

This patch fixes the proxy issue by adding a new script, which can be used by every command that requires no proxy set. Not sure if that is the best location, or if it would even be good to have as method in bash.rc. Please let me know.

Otherwise I also removed the code for resetting the environmnet file on CentOS. Meanwhile this should have been distributed to all the machines.

I tested the atboot behavior and it works fine. Java has been gotten installed successfully.

Comment 19

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8452239 [details] [diff] [review]
no proxy for agent v1

Review of attachment 8452239 [details] [diff] [review]:
-----------------------------------------------------------------

::: modules/web_proxy/manifests/environment.pp
@@ +19,5 @@
> +                    Darwin,
> +                    CentOS,
> +                    Ubuntu: {
> +                        shellprofile::file { "proxy":
> +                            ensure => absent

This isn't actually necessary -- if the 'proxy' file isn't included in the catalog, then the purge => true in modules/shellprofile/manifests/base.pp will delete it.  I missed this when you added $ensure to shellprofile::file.

No need to change this now -- just commenting since I noticed it.

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Oh, I see. So we can completely get rid of the else case then? I would include this now, otherwise not sure when the follow-up will happen.

Comment 21

[Dustin J. Mitchell [:dustin] (he/him)]

Yes, that sounds right.

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Correct, I tested this and it works fine. Updated patch is coming.

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: no proxy for agent v2

Comment 24

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8452309 [details] [diff] [review]
no proxy for agent v2

Review of attachment 8452309 [details] [diff] [review]:
-----------------------------------------------------------------

If you can revert the order of the "case" and "if", that will help markco when he tries to land his ginormous windows patch.

::: modules/web_proxy/manifests/environment.pp
@@ +7,5 @@
> +    case $operatingsystem {
> +        Darwin,
> +        CentOS,
> +        Ubuntu: {
> +            if $web_proxy::host != "" {

Won't this reversal cause puppet runs on other OS's to fail, even if no proxy is desired?  What's the benefit?

(sorry for not bringing this up the first time -- I blame morning-review syndrome)

Comment 25

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Dustin J. Mitchell [:dustin] from comment #24)
> If you can revert the order of the "case" and "if", that will help markco
> when he tries to land his ginormous windows patch.

With the else case removed that makes it simpler now. When this was included I had to add another if condition at the highest level. So yes, I will revert that.

Btw, we get Windows support??? :)

> ::: modules/web_proxy/manifests/environment.pp
> @@ +7,5 @@
> > +    case $operatingsystem {
> > +        Darwin,
> > +        CentOS,
> > +        Ubuntu: {
> > +            if $web_proxy::host != "" {
> 
> Won't this reversal cause puppet runs on other OS's to fail, even if no
> proxy is desired?  What's the benefit?

Ups, that's correct. Totally missed that. So good thing to revert this now. :)

Comment 26

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: no proxy for agent v3

Comment 27

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8452368 [details] [diff] [review]
no proxy for agent v3

Yeah, Mark is working on Windows support, but in a limited fashion -- just for setting up new machines before capturing a snapshot of them.  That will probably be useful for you, but we'll see!

Comment 28

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Dustin J. Mitchell [:dustin] from comment #27)
> Yeah, Mark is working on Windows support, but in a limited fashion -- just
> for setting up new machines before capturing a snapshot of them.  That will
> probably be useful for you, but we'll see!

Indeed. I will follow the work then.

Pushed this patch as:
https://hg.mozilla.org/qa/puppet/rev/578e31801655 (default)
https://hg.mozilla.org/qa/puppet/rev/2869859a81e9 (production)

Dustin, maybe we should merge with build/puppet soon, to not cause merge conflicts for Mark's work?

Comment 29

[Dustin J. Mitchell [:dustin] (he/him)]

Yeah, I'll put a merge on my backlog.