Seconded - let's leave the hostname that comes from the Proxy URL alone. In the future, we could extend this further, perhaps by read the dnsaltnames from the masters certificate, but for now, this is an easy fix for some unexpected behaviour.

Or maybe we can just validation that the string is "puppet" and not "puppet*"

Sam Kottler wrote:

Or maybe we can just validate that the string is "puppet" and not "puppet*"

Just triaging this, but can we remove it?

Personally I would change the behavior to not do any thing funky with the puppetmaster names, (which should be clear since that is what I propose in this ticket.) However, since there may be legacy installations that use this, and we probably don't want to force people to go redo all their certs, perhaps the answer is to just change the default behavior as I proposed and to create a new setting to revert to the legacy behavior. (But again leave the default "principal of least surprise.")

The other option is to poll -users, and see if anyone is relying on the legacy behavior?

-Brian

Ohad Levy wrote:

I think the main reason is that from puppet certname point of view:

puppet is in the cert
fqdn is in the cert
puppet.domain is not (unless your server name is puppet).

puppet.domain is in the list of alt names for a puppet master. I think this was always the case, but may have been introduced when they moved from certdnsnames to dns_alt_names (due to a CVE).

Dominic Cleal wrote:

puppet.domain is in the list of alt names for a puppet master. I think this was always the case, but may have been introduced when they moved from certdnsnames to dns_alt_names (due to a CVE).

That doesn't mean it's in your site DNS. Messing with what a user specifies in the proxy name just causes confusion. I'm happy with stripping this behaviour.

Brian has a point. However, I'm not sure how likely it is that people rely on this behaviour (they might use it accidentally, but how difficult is it to add a DNS record?). It seems like a lot of work to support both methods via a Setting - do we want to do that?

I don't believe it's simply a matter of adding a DNS record. The behavior the original fix worked around is that the certificate needs to match the name as called. At least in some versions of puppet, the puppetmaster certname that got generated included its $fqdn and an unqualified "puppet" string.. so if your puppetmaster's hostname was something like server1.mydomain.com and you had a DNS cname puppet.mydomain.com pointing at it, clients would need to reference it by an unqualified 'puppet' or it's actual hostname.

IE: 'server1.mydomain.com' and 'puppet' would work, but 'puppet.mydomain.com' would not.

It sounds like (from Dominic's comment) this isn't still the default behavior when creating the puppetmaster's cert, but it was definitely the behavior for many versions of puppet.

Ohad definitely implemented this for a reason (to resolve certain users' issues), so we should be careful to change this behavior without providing people a quick way to go into "legacy mode" that doesn't require generating new puppetmaster certs. (As much as I really want this change to go through, I think we do need to support a setting.)

I agree also that should be supported with a setting.

ok, so the code change was pretty trivial. I have it done, however I have 2 questions:

1) how do I make this setting available to an existing install? The code checking for the setting was being used properly, but going to the settings page it didn't show up. It only started showing up after I deleted the database and started from scratch with a rake db:migrate

2) how should we handle upgrades vs fresh installs? Dominic says the default should be no change in the hostname, and I would agree with that, but that's a change in the default behavior and a pretty quiet one which could be really frustrating at 2am :)

Thanks, and I should be able to have a PR after this is taken care of!

Jeremy Kitchen wrote:

ok, so the code change was pretty trivial. I have it done, however I have 2 questions:

1) how do I make this setting available to an existing install? The code checking for the setting was being used properly, but going to the settings page it didn't show up. It only started showing up after I deleted the database and started from scratch with a rake db:migrate

Adding a new entry to app/models/setting/*.rb will caused it to be added if the setting doesn't already exist when the application starts up. It won't need a reinstallation/migration.

2) how should we handle upgrades vs fresh installs? Dominic says the default should be no change in the hostname, and I would agree with that, but that's a change in the default behavior and a pretty quiet one which could be really frustrating at 2am :)

I'm in favour of changing the default behaviour, and ensuring it's in the upgrade notes for 1.3, though appreciate your point.

The only other idea I have is to change the default value of the setting based on whether there are any smart proxies currently registered, but since setting default values are re-evaluated and changed on each Foreman startup, this would affect new installs as well as old ones, so that's a no-go.

Thanks, and I should be able to have a PR after this is taken care of!

Thanks!

PR #852