Using certonly in webroot mode fails with "can't convert Enumerable::Enumerator into Array"

[akomakom]

Puppet: 3.8.5
Letsencrypt: 0.5.0 via GIT (0.1.0, which is default, doesn't like Centos 6 with Ruby 1.8.7, and adding epel doesn't seem to get me any packages, thus I'm using git.)
Ruby: 2.1.2p95 on the master, 1.8.7 on the node.

 letsencrypt::certonly{"My letsencrypt":
    domains => ['www.my.org', 'my.org'],
    plugin => 'webroot',
    webroot_paths => ['/var/www/html/something/public']  # I also tried specifying it twice but saw use of cycle() 
  }

Error:

Error: Could not retrieve catalog from remote server: Error 400 on SERVER: Failed to parse inline template: can't convert Enumerable::Enumerator into Array at /etc/puppet/modules/letsencrypt/manifests/certonly.pp:47

Now, I'm not sure what exactly is Enumerable here, so I tried it on command line and it works fine:

irb(main):001:0> webroot_paths = ['/some/path']
=> ["/some/path"]
irb(main):002:0> domains = ['www.my.org','my.org']
=> ["www.my.org", "my.org"]
irb(main):003:0> domains.zip(webroot_paths.cycle).map { |domain| "--webroot-path #{domain[1]} -d #{domain[0]}"}.join(" ")
=> "--webroot-path /some/path -d www.my.org --webroot-path /some/path -d my.org"

Obviously I had to drop the '@' signs.

I'm not that familiar with rake and couldn't get the tests to work, so this remains unsolved on my end.

[akomakom]

removing ".cycle" from letsencrypt certonly.pp and using a matching-size array of web roots makes this work... not sure why cycle() doesn't work the same way in puppet as it does in irb.

[tomgillett]

I can +1 this issue with .cycle.

Master Puppet 3.7.5, Ruby 1.8.7
Agent Puppet 3.4.3, Ruby 2.1.5p273

[danzilio]

@akomakom just want to confirm: you're running 1.8.7 on the agent but 2.1.2 on the master? It would make sense with 1.8.7 on the master. This module isn't tested against 1.8.7 and, to be quite honest, I have no desire to support such an old Ruby version.

[akomakom]

@danzilio You are right, the master is using 1.8.7, even though I have 2.1.2 installed also, my mistake. Most of our infrastructure is running on Centos 6 and that's the ruby you get, so I'll stick with my hack until we can test newer ruby.

Thanks!

[ways]

@akomakom Can you make a fork with that workaround?

[akomakom]

@ways I made a fork: https://github.com/akomakom/puppet-letsencrypt , but it reduces functionality (The two arrays must always be the same size).

[ways]

@akomakom Ok. I made one at https://github.com/copyleft/puppet-letsencrypt/ with a new plugin called cli, which just allows you to specify everything via additional_args.

[danzilio]

@ways interesting. that's something I would consider merging. although I'd rather change the name from cli to something like none or manual to differentiate it from an actual letsencrypt plugin.

[ways]

@danzilio Cool. I'll be happy to change it. I started out calling it 'manual', but that can be confused with letsencrypts manual verification. 'none', 'noop' or 'ways_rulez!' sounds OK to me. Want a pull-request for one of those?

[danzilio]

@ways I'm starting to wonder if we have too much indirection/abstraction around the command that gets run. Thoughts?

[ways]

@danzilio Hm. Not sure what you mean. After inserting this hack? I don't see any problem with it. As long as the documentation is solid. I think it's a short-lived solution for us stuck with old setups, and most people can ignore it.

[danzilio]

@ways i'm just wondering if the more durable solution is to give the user more access to the command that gets run... Let me mull this over a bit!

[ways]

@danzilio Ah, understood. No rush.

[ways]

@danzilio Any progress?

[domcleal]

I opened #40 with a compatible equivalent of the existing #cycle call, though for my personal use of this module I'd also be happy removing cycle entirely and having a fixed domains -> webroot_paths list (per akomakom@bf99c26 which I'm currently using).

I like the existing abstraction over the webroot command, it makes writing wrappers much easier.

[danzilio]

Hmm...I feel like @akomakom's solution is cleaner. Is this the more intuitive solution?

[domcleal]

I agree having the array lengths match is the cleaner interface - the zipping behaviour might be useful for some, but I think it's a minority feature. The downside is that it's an API change, but since the module's 0.x, perhaps you're OK accepting it?

[domcleal]

but since the module's 0.x, perhaps you're OK accepting it?

Correction, it's 1.x now, so it'd probably be considered a major version change.

[danzilio]

I'm fine bumping the major version. There are some big changes coming in the next release, the major version bump will raise some awareness.

[pgassmann]

The webroot_paths and domain array should not need to match. it's possible to specify one webroot path and multiple domains on the cli. This standard case should be reflected.

at least one-for-all and matching numbers should be supported. I don't know how mismatched lengths should be handled best. Just append the remaining domains with no webroot specified, perhaps? -w 1 -d 1 -w 2 -d 2 -d 3 -d 4

[domcleal]

@pgassmann ah nice, you're right - the CLI arguments don't need to match.

I don't know how mismatched lengths should be handled best. Just append the remaining domains with no webroot specified, perhaps? -w 1 -d 1 -w 2 -d 2 -d 3 -d 4

The docs show this format with mismatched lengths is supported, any additional domains use the last webroot path. The current behaviour of this module with the cycle is to repeat from the first webroot path, so we ought to just remove that and allow the regular certbot behaviour.

[domcleal]

master...domcleal:28-no-cycle for that last suggested change.

[danzilio]

@domcleal looks good to me! i'll pull this in