Add puppetserver configs #343

jyaworski · 2016-01-26T23:05:59Z

No description provided.

ekohl · 2016-02-03T13:42:46Z

@jyaworski Need any help finishing this?

ekohl · 2016-02-03T13:45:05Z

manifests/server/puppetserver.pp

+    notify => Class['puppet::server::service'],
+  }
+
+  $puppetserver_config_dir = versioncmp($::puppetversion, '4.0.0') >= 0 ? {


This line is causing the tests to fail, possibly because the fact is not defined in our tests and is causing the tests to fail.

Any ideas for a solution?

$puppetserver_config_dir = num2bool(versioncmp($::puppetversion, '4.0.0') >= 0) ? {

That fixed the test for me.

@brandonweeks that reads like it shouldn't work. I think it's not needed in params.pp because there it's an if statement and not a selector. In any case, I think we should move this to params.pp and possibly reuse $aio_package.

jyaworski · 2016-02-03T16:55:16Z

@ekohl reviews would be good. I'm still progressing on this but it has been on the backburner to some other PRs I'm working on. I still very much need to do this, though.

brandonweeks · 2016-02-03T19:59:53Z

@jyaworski this was on my todo list as well, so I'm very ready to test it. :)

ekohl · 2016-02-03T20:51:57Z

I was poked by @zipkid at a meeting between puppet users, puppetlabs and foreman devs about puppet 4 that this is on his wishlist as well, so this is in demand.

jyaworski · 2016-02-03T21:43:47Z

I'll see if I can work on this by the end of this week. I'll ping when it's comment-ready again.

ekohl · 2016-02-03T22:07:09Z

@jyaworski awesome!

zipkid · 2016-02-04T05:46:53Z

Hi @ekohl, @jyaworski,

Thanks for poking me on this.
As mentioned yesterday i also had to add management of conf.d/webserver.conf to set

ssl-cert
ssl-key
ssl-ca-cert
which i don't find in the PR.

Also, in bootstrap.cfg, when '::puppet::server_ca' evaluates to false the following line should be added.
'puppetlabs.services.ca.certificate-authority-disabled-service/certificate-authority-disabled-service'

Regards,

Stefan.

jyaworski · 2016-02-04T19:23:19Z

@zipkid thanks for the feedback. I'll make those changes. @ekohl and @brandonweeks I've already addressed some of your comments.

jyaworski · 2016-02-05T04:11:37Z

@zipkid made a few changes. How's it look?

zipkid · 2016-02-05T05:12:43Z

That is indeed what i meant.
Thank you.

jyaworski · 2016-02-05T06:56:39Z

Great. I believe this PR is mostly done, except that I have to make the gem-home configurable:

https://github.com/theforeman/puppet-puppet/pull/343/files#diff-133e2a0f1f52427763456aee8fba2a7aR5

I'm unaware of a way to get the puppetserver gem home without shelling out (is it worth a puppetserver_gem_home fact? I seriously hope not). If anyone has any ideas while I'm thinking on it, that would be great.

brandonweeks · 2016-02-05T07:42:40Z

manifests/init.pp

-#                                     configuration section. This is necessary to
-#                                     disable in case CA is delegated to a separate instance
-#                                     type:boolean
+# $server_ca_authorization_required::    Should the puppetserver ruby profiler be enabled?


This is missing from puppet::params and the description is wrong.

Good find. I did way too much copy-pasting during that part.

mmoll · 2016-02-05T16:01:53Z

@jyaworski thanks for working on this, that's awesome 🌟
tests are failing at the moment...

brandonweeks · 2016-02-05T22:39:49Z

templates/server/puppetserver/conf.d/webserver.conf.erb

@@ -0,0 +1,15 @@
+webserver: {
+    access-log-config = /etc/puppetlabs/puppetserver/request-logging.xml


This shouldn't be hardcoded.

access-log-config = <%= scope.lookupvar('puppet::server_puppetserver_dir') %>/request-logging.xml

brandonweeks · 2016-02-05T23:17:20Z

On Ubuntu 14.04 I'm getting /usr/local/lib/site_ruby/1.9.1 for $::rubysitedir, which isn't a working entry for ruby-load-path.

jyaworski · 2016-02-05T23:20:06Z

Do you have an idea? I don't have the facility to test Ubuntu at the moment?

brandonweeks · 2016-02-05T23:30:44Z

templates/server/puppetserver/conf.d/puppetserver.conf.erb

+puppet-admin: {
+    client-whitelist: [
+    <%- scope.lookupvar('::puppet::server_admin_api_whitelist').each do |certname| -%>
+        <%= certname %>,


Exception in thread "main" com.typesafe.config.ConfigException$Parse: /etc/puppetserver/conf.d/puppetserver.conf: 58: in value for key 'client-whitelist': List should have had new element after a comma, instead had token: ':' (if you want the comma or ':' to be part of a string value, then double-quote it)

I get this error when ::1 is in the list with puppetserver 1.1.3.

Weird. Fixed.

9af8dae fixed it for me, puppetserver 1.x expects quotes in the conf file itself.

brandonweeks · 2016-02-06T00:11:23Z

@jyaworski I confirmed it with a clean install of Ubuntu, I don't think you'll be able to rely on $::rubysitedir for Ubuntu. 😢 @mmoll do you have any insight into why this fact points to an empty directory on Ubuntu?

brandonweeks · 2016-02-06T00:28:41Z

templates/server/puppetserver/conf.d/web-routes.conf.erb

@@ -0,0 +1,10 @@
+web-router-service: {


https://gist.github.com/brandonweeks/bc72082ab65c17b11396

Replacement template with puppetserver 1.x and 2.x support :)

mmoll · 2016-02-06T01:44:28Z

@brandonweeks no idea, but since facter 2.x is probably unmaintained now, this won't get resolved.

jyaworski · 2016-02-06T03:18:57Z

@mmoll facter 2.4.x is in maintenance mode, just like 3.8.x is for puppet. If there's no fact we can rely on, that's unfortunate. If we add a dependency on puppetlabs-ruby, we can get access to the $::gemhome fact, which appears to be equivalent enough at least on a RHEL6 system:

[root@dagon ~]# facter -p gemhome rubysitedir
gemhome => /usr/lib/ruby/gems/1.8
rubysitedir => /usr/lib/ruby/site_ruby/1.8

We could always do directory magic or gsub...

jyaworski · 2016-02-06T03:44:19Z

Actually, looking into it, os-settings should essentially never be changed. I'm suggesting we just let whatever the packager puts in there exist, and we don't manage the file. I've removed that part of the code. I can't find references to it in the docs for newer versions.

https://docs.puppetlabs.com/puppetserver/2.2/configuration.html
http://docs.puppetlabs.com/puppetserver/2.1/configuration.html
http://docs.puppetlabs.com/puppetserver/1.1/configuration.html

jyaworski · 2016-02-06T03:47:43Z

Oh, sorry for the spam.... but it looks like I spoke too soon:

jruby-puppet: {
    # Where the puppet-agent dependency places puppet, facter, etc...
    # Puppet server expects to load Puppet from this location
    ruby-load-path: [/opt/puppetlabs/puppet/lib/ruby/vendor_ruby]

    # This setting determines where JRuby will look for gems.  It is also
    # used by the `puppetserver gem` command line tool.
    gem-home: /opt/puppetlabs/server/data/puppetserver/jruby-gems

In 2.x they moved it into puppetserver.conf. I think we can let that value stand (if we can get a puppetserver_version working), and leave os-settings.conf alone. I'm also willing to set gem-home on the same principle.

jyaworski · 2016-02-06T04:17:02Z

OK, I think I have a working solution now (without rspec tests). I'll add the tests once I get confirmation this works on a wide variety of setups.

As an aside, I saw puppetlabs-hocon if we ever decide to move away from templates for this kind of thing.

daenney · 2016-02-06T12:26:21Z

manifests/params.pp

+  $server_connect_timeout_milliseconds = 120000
+  $server_enable_ruby_profiler         = false
+  $server_ca_authorization_required    = true
+  $server_admin_api_whitelist          = [ "127.0.0.1", "::1", $::ipaddress ] # lint:ignore:double_quoted_strings


Why not simply switch to single quotes and remove the lint exception?

mmoll · 2016-02-19T18:03:57Z

in the spec tests, it's still 4G ;)

mmoll · 2016-02-19T19:42:13Z

BTW, also puppetserver 1.1.3 has -XX:MaxPermSize=256m in JAVA_ARGS per default, so that should IMHO get added here, too.

brandonweeks · 2016-02-19T22:28:38Z

I ran into the two things you already found, looks good otherwise so far.

mmoll · 2016-02-19T22:54:33Z

manifests/init.pp

 #
 # $manage_packages::                  Should this module install packages or not.
 #                                     Can also install only server packages with value
 #                                     of 'server' or only agent packages with 'agent'.
 #                                     Defaults to true
+#                                     type:boolean


this is a "stroolean", so it has to be string

jyaworski · 2016-02-20T03:35:07Z

Implemented all recommendations.

mmoll · 2016-02-20T14:51:04Z

@jyaworski thanks, please feel free to squash in mmoll/puppet-puppet@d6c921f to get the tests green by setting the JAVA options back to 2G also in the tests.

Furthermore I couldn't get this to work on a fresh system with puppet apply. Further investigation and a look onto cryptocamp-puppetserver shows that the augeas lens has to be deployed before using it, I have a POC there: mmoll/puppet-puppet@a6f9044

However, I think now that it's probably better to have the addition of the trapperkeeper lens in a seperate PR, because this is getting a bit too big here. 💣

So the way to go would be:

get Allow simple entries as standalones voxpupuli/puppet-puppetserver#25 merged
open a PR to add the trapperkeeper lens into theforeman/puppet-puppet - ideally @raphink and @domcleal could do this as augeas experts, otherwise (depending on how much is to do regarding testing in addition to just copying it in) I can draft the change and give colab permissions to my puppet-puppet fork to whoever wants to work on this as I'm a bit short on time)
rebase this PR onto theforeman/puppet-puppet master after the trapperkeeper lens went in
get this PR merged 😃

Opinions on this?

jyaworski · 2016-02-20T22:17:33Z

@mmoll the augeas lens should work if pluginsync is enabled.

http://docs.puppetlabs.com/guides/plugins_in_modules.html#module-structure

Are you seeing something else?

mmoll · 2016-02-20T22:39:06Z

@jyaworski did you test this with puppet apply on a fresh box also?

jyaworski · 2016-02-20T22:41:30Z

My mistake; you're talking about running locally with an apply, and I was testing in a client/server setup so pluginsync was working for me.

If that's the case, we'll have to ship this the way that @raphink originally did it (and yell at Puppetlabs to do this better)?

mmoll · 2016-02-20T22:45:05Z

Yes, the thing is for foreman-installer this needs to work locally.

Even if PL would change the behaviour, we would need to wait for a new Puppet version and depend on specific versions, that wouldn't end well. :)

For my POC I took the original cryptocamp code and transformed it back to a way that IMHO works without requiring cryptocamp-augeas.

jyaworski · 2016-02-20T22:56:54Z

Proposed solution: since we're just managing one file with augeas and not the entirety of trapperkeeper, we can bypass voxpupuli/puppet-puppetserver#25 until we figure out a way to somehow get them to talk. For now, we only care about its behavior with bootstrap.cfg. I've incorporated your changes.

mmoll · 2016-02-20T23:07:45Z

My changes were unpolished, at least mmoll/puppet-puppet@645c8e5 is needed, also the added lines in manifests/params.pp should be better aligned.

The lens test is not needed for now, as we don't test anything, which is not desirable, that's what I meant with "depending on how much is to do regarding testing in addition to just copying it in".

I think this PR is fine for now and we need to wait for other people to have a look onto the lens topic. 🔍.

jyaworski · 2016-02-21T13:02:05Z

Sure. This is good to merge minus looking at the lens later.

brandonweeks · 2016-02-23T23:55:37Z

https://gist.github.com/brandonweeks/6ca65417399650c4f8b8

Running into this.

jyaworski · 2016-02-24T18:07:56Z

@brandonweeks what's your environment? I'm not running into that.

brandonweeks · 2016-02-25T00:12:57Z

Oh, I'm an idiot. 😅 This is working perfectly for me with puppetserver 1.x. 👍

domcleal · 2016-02-25T13:11:57Z

The lens test is not needed for now, as we don't test anything, which is not desirable, that's what I meant with "depending on how much is to do regarding testing in addition to just copying it in".

I'd prefer to keep the lens test in the tree because it's customised from the camptocamp version. It's very important with Augeas lenses to typecheck them to prevent bizarre runtime behaviour, and this lets us do it (manually with augparse, if nothing else). Perhaps just remove/comment the broken test for now so the file passes.

jyaworski · 2016-02-25T13:30:15Z

@domcleal the failing test is the one that says that '/' can be a part of keys. I commented that out; the last few lines in the test.

mmoll · 2016-02-25T13:50:14Z

Regarding the lens see voxpupuli/puppet-puppetserver#25 (comment), we'll just pull that in as-is now.

mmoll · 2016-02-25T13:52:25Z

Merged, dziękujemy bardzo @jyaworski!

jyaworski · 2016-02-25T13:58:00Z

@mmoll proszę bardzo!

ekohl reviewed Feb 3, 2016
View reviewed changes

brandonweeks reviewed Feb 5, 2016
View reviewed changes

brandonweeks mentioned this pull request Feb 5, 2016

Incomplete SSL Configuration #275

Closed

brandonweeks reviewed Feb 6, 2016
View reviewed changes

daenney reviewed Feb 6, 2016
View reviewed changes

mmoll reviewed Feb 19, 2016
View reviewed changes

Add puppetserver configs

df7fcb2

This was referenced Feb 23, 2016

relay passenger_max_pool to apache::mod::passenger class #294

Closed

Fixing Passenger max_pool_size, adding more Passenger variables #310

Closed

Full interface for apache::mod::passenger #333

Closed

mmoll merged commit df7fcb2 into theforeman:master Feb 25, 2016

jyaworski deleted the puppetserver_config branch February 25, 2016 13:56

mmoll mentioned this pull request Mar 23, 2016

Upgrade to puppetserver 2.3 fails on EL #373

Closed

		@@ -0,0 +1,15 @@
		webserver: {
		access-log-config = /etc/puppetlabs/puppetserver/request-logging.xml

Add puppetserver configs #343

Add puppetserver configs #343

Conversation

jyaworski commented Jan 26, 2016

ekohl commented Feb 3, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jyaworski commented Feb 3, 2016

brandonweeks commented Feb 3, 2016

ekohl commented Feb 3, 2016

jyaworski commented Feb 3, 2016

ekohl commented Feb 3, 2016

zipkid commented Feb 4, 2016

jyaworski commented Feb 4, 2016

jyaworski commented Feb 5, 2016

zipkid commented Feb 5, 2016

jyaworski commented Feb 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmoll commented Feb 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brandonweeks commented Feb 5, 2016

jyaworski commented Feb 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brandonweeks commented Feb 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmoll commented Feb 6, 2016

jyaworski commented Feb 6, 2016

jyaworski commented Feb 6, 2016

jyaworski commented Feb 6, 2016

jyaworski commented Feb 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmoll commented Feb 19, 2016

mmoll commented Feb 19, 2016

brandonweeks commented Feb 19, 2016

Choose a reason for hiding this comment

jyaworski commented Feb 20, 2016

mmoll commented Feb 20, 2016

jyaworski commented Feb 20, 2016

mmoll commented Feb 20, 2016

jyaworski commented Feb 20, 2016

mmoll commented Feb 20, 2016

jyaworski commented Feb 20, 2016

mmoll commented Feb 20, 2016

jyaworski commented Feb 21, 2016

brandonweeks commented Feb 23, 2016

jyaworski commented Feb 24, 2016

brandonweeks commented Feb 25, 2016

domcleal commented Feb 25, 2016

jyaworski commented Feb 25, 2016

mmoll commented Feb 25, 2016

mmoll commented Feb 25, 2016

jyaworski commented Feb 25, 2016