Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using a space in the letsencrypt::certonly title creates a cron script that will never run. #91

Closed
andrewwippler opened this issue May 31, 2017 · 1 comment
Labels
bug Something isn't working

Comments

@andrewwippler
Copy link

Affected Puppet, Ruby, OS and module versions/distributions

  • Puppet: 4.10.0
  • Ruby: ruby 2.1.9p490
  • Distribution: Ubuntu 16.04.2 LTS (Xenial Xerus)
  • Module version: commit e72470c (May 9th - latest commit)

How to reproduce (e.g Puppet code you use)

    letsencrypt::certonly { "LE ${server}":
      domains                           => [$server,],
      manage_cron                   => true,
      plugin                               => 'webroot',
      webroot_paths                 => [$webroot],
      cron_success_command => '/bin/systemctl reload nginx.service',
    }

What are you seeing

Note with the space in the name above, the module improperly creates a file with a space in the name. When running the command from cron, it would fail due to the space not being escaped.

What behaviour did you expect instead

The cron script substituting a character for space.

Any additional information you'd like to impart

Obviously this is an edge case and I fixed my issue by substituting the space with a -.

siebrand added a commit to siebrand/puppet-letsencrypt that referenced this issue Feb 5, 2018
Issue voxpupuli#91 reports that creating a letsencrypt::certonly resource with a
title containing a space will create a cron entry that will never run.
That's because it will create a cron entry like this:

    10 22 * * * /cron_scripts_path/renew-title with space.sh

That executes /cron_scripts_path/renew-title that obviously does not
exist. When the full cron command is put in quotes, it will run as
expected.
siebrand added a commit to siebrand/puppet-letsencrypt that referenced this issue Feb 7, 2018
Issue voxpupuli#91 reports that creating a letsencrypt::certonly resource with a
title containing a space will create a cron entry that will never run.
That's because it will create a cron entry like this:

    10 22 * * * /cron_scripts_path/renew-title with space.sh

That executes /cron_scripts_path/renew-title that obviously does not
exist. When the full cron command is put in quotes, it will run as
expected.
siebrand added a commit to siebrand/puppet-letsencrypt that referenced this issue Dec 27, 2018
Issue voxpupuli#91 reports that creating a letsencrypt::certonly resource with a
title containing a space will create a cron entry that will never run.
That's because it will create a cron entry like this:

    10 22 * * * /cron_scripts_path/renew-title with space.sh

That executes /cron_scripts_path/renew-title that obviously does not
exist. When the full cron command is put in quotes, it will run as
expected.
@juniorsysadmin juniorsysadmin added the bug Something isn't working label Dec 27, 2018
juniorsysadmin added a commit that referenced this issue Dec 27, 2018
#91: Cron should also run with space in certonly title
@juniorsysadmin
Copy link
Member

Closed by #113

minorOffense pushed a commit to coldfrontlabs/puppet-letsencrypt that referenced this issue May 17, 2019
Issue voxpupuli#91 reports that creating a letsencrypt::certonly resource with a
title containing a space will create a cron entry that will never run.
That's because it will create a cron entry like this:

    10 22 * * * /cron_scripts_path/renew-title with space.sh

That executes /cron_scripts_path/renew-title that obviously does not
exist. When the full cron command is put in quotes, it will run as
expected.
minorOffense added a commit to coldfrontlabs/puppet-letsencrypt that referenced this issue May 17, 2019
author Mathew Winstone <mathew.winstone@gmail.com> 1516893540 -0500
committer Mathew Winstone <mwinstone@coldfrontlabs.ca> 1558122751 -0400

parent 80d8513
author Mathew Winstone <mathew.winstone@gmail.com> 1516893540 -0500
committer Mathew Winstone <mwinstone@coldfrontlabs.ca> 1558122719 -0400

refac(fact) deprecated in favour of letsencrypt_directory

feat(lookup) add custom lookup function

refac(folders) reorganize to proper locations

fix coding standards

fix coding standards

fix coding standards

fix(spec)

refac(condition) use normal unless

Favor a normal unless-statement

fix again

fix(typo)

bump inifile dependency to allow 2.x

bump to lowest inifile version that supports puppet5

bump to lowest vcsrepo version that supports puppet5

modulesync 1.6.0

release 2.2.0

[blacksmith] Bump version to 2.2.1-rc0

OpenBSD support

rake lint fixes

use selector to determine group, via classifc fact_name

modulesync 1.8.0

release 2.3.0

[blacksmith] Bump version to 2.3.1-rc0

Fixes voxpupuli#81 added -n flag to certbot to run in unattended mode.

Changed the flag `-n` to `--non-interactive`

Allow custom config dir; FreeBSD support

* Adds a parameter `config_dir` to provide an absolute path to the
configuration directory.
    * Defaults to '/etc/letsencrypt' except on FreeBSD, where it defaults to
'/usr/local/etc/letsencrypt', as FreeBSD standard and expected by
`py27-certbot`.

modulesync 1.9.0

modulesync 1.9.2

drop EOL OSs; fix puppet version range

Remove docker nodesets

support dns-route53 plugin

release 2.4.0

[blacksmith] Bump version to 2.4.1-rc0

modulesync 1.9.3

modulesync 1.9.4

modulesync 1.9.6

Fix cron environment leaking

This commit moves the environment variable definition from the cron
resource environment parameter to the renewal script. This ensure that
environment variables aren't conflicting with other cronjobs.

Fixes voxpupuliGH-125.

allow puppetlabs/stdlib 5.x

Add support for dns-google plugin support

Migrate letsencrypt::config::ini helper to native Puppet 4

modulesync 2.0.0

Remove inline_template in favour of native Puppet code

* Migrates all inline_template calls to Puppet native code, as this is
recommended best practice
* Changes the default value/data type of webroot_paths and
additional_args to an Array instead of Optional[Array]
* Adds a data type for the certificate plugins for better readability

Allow additional domains to be added later

This commit adds support for expansion/shrinking of an already existing
certificate by defining a unique identifier (--cert-name) which maps to
the Puppet resource title. By doing this, we can ensure that always all
requested domains are contained in the certificate.

Fixes voxpupuliGH-94.

modulesync 2.2.0

delete unneeded coverage_spec.rb

purge obsolete symlink setting in fixtures

allow puppet 6.x

add yumrepo_core module to fixtures

add cron_core module to fixtures

release 2.5.0

[blacksmith] Bump version to 2.5.1-rc0

Changed certonly::domains datatype to Array[String[1]] to accommodate wildcard domains

Added *.example.com to spec test

make the module compatible with FreeBSD

As FreeBSD also has no `root` group, the only change I noticed would be necessary to successfully run this module on FreeBSD is the same like for OpenBSD.

Added support for dns-cloudflare CertBot plugin.

Add cron day of month parameter

Use non-optional and strict data type for monthday param

Specify min/max Integer value for cron_monthday param

modulesync 2.3.1

Enable user to define cron time attributes instead of defaulting to random time

modulesync 2.4.0

Cron should also run with space in certonly title

Issue voxpupuli#91 reports that creating a letsencrypt::certonly resource with a
title containing a space will create a cron entry that will never run.
That's because it will create a cron entry like this:

    10 22 * * * /cron_scripts_path/renew-title with space.sh

That executes /cron_scripts_path/renew-title that obviously does not
exist. When the full cron command is put in quotes, it will run as
expected.

Improve performance of unless

Turns out, that calling certbot for every certificate resources takes
ages on systems with a lot of certificates. Some comparison:

certbot: ~0.7s
openssl: ~0.01s

use certbot repository

declare ubuntu16.04 as supported OS

add basics acceptance tests

add ubuntu 18.04 as supported OS

modulesync 2.5.0

bump lower puppet version from 4.10.0 to 5.5.8

fix fact scope; migrate tests to rspec-puppet-facts

facts called with $facts

fix rubocop

migrate tests to rspec-puppet-facts

increase spec coverage

Move fact definitions to inside the context

migrate FreeBSD tests to rspec-puppet-facts

fix tests for correct FS paths

cleanup spec files

vcs method, bump version of certbot

release 3.0.0

[blacksmith] Bump version to 3.0.1-rc0

Certonly: Ensure cron is absent if manage_cron is false

Certonly: Removing cron script file if manage_cron is false

Spec: Fixing certonly tests to manage cron script

Certonly: Refactor manage_cron Boolean to ensure_cron Enum

remove certonly acceptance test

Fedora doesn't use EPEL for certbot/letsencrypt.

Requested changes to simplify and follow preferred facts.

modulesync 2.5.1

Fix exec onlyif logic

This commit fixes the improved check logic introduces in
460a7b7. It ensure that the exec is
actually triggered if the cert file doesn't exist.

Move to script for domain validation

Add Fedora to tested systems list.

Add ability to control keysize. Default to 4096.

Add missing rspec value

Clean up some old Puppet 3 syntax

Add support for plugin dns-rfc2136

Add support for wildcard certs

Upgrade vcs version to 0.30.2

0.30.0 had a bug in combination with Pip 19.0.1 which made the
installation fail. certbot/certbot#6692

Fixes voxpupuli#172.

Fix for $live_path variable

Certbot uses the "--cert-name" parameter as part of the path, not the
first domain. This means the computed $live_path would be wrong if
$domain[0] differs from $title.

Rework letsencrypt::certonly to allow deprovisioning

+ Added $ensure attribute
+ Renamed $ensure_cron back to $manage_cron
+ Returned $manage_cron back to a Boolean value
+ Made execute of "letsencrypt certonly ${title}" conditional to $ensure == 'present'
+ Added cleanup of directory for domain certs when $ensure == 'absent'
+ Used global $ensure to signal desired state of cronjobs

Tiny style tweak

Add missing var doc in dns-rfc2136

Add support for certbot hooks

Add single quotes around all domains names in shell commands

Fixes voxpupuli#178

surfacing package_ensure in README

modulesync 2.6.0

release 4.0.0

[blacksmith] Bump version to 4.0.1-rc0

modulesync 2.7.0

fix

fix(docs): add basic documentation about facts and function

Add sample output from facts
Add notes on function usage

Bump dependency, because of new major release

fix(os): confine to *nix like systems

fix(format): update rubocop violations

fix again

fix(typo)

bump inifile dependency to allow 2.x

bump to lowest inifile version that supports puppet5

bump to lowest vcsrepo version that supports puppet5

modulesync 1.6.0

release 2.2.0

[blacksmith] Bump version to 2.2.1-rc0

OpenBSD support

rake lint fixes

use selector to determine group, via classifc fact_name

modulesync 1.8.0

release 2.3.0

[blacksmith] Bump version to 2.3.1-rc0

Fixes voxpupuli#81 added -n flag to certbot to run in unattended mode.

Changed the flag `-n` to `--non-interactive`

modulesync 1.9.2

drop EOL OSs; fix puppet version range

Remove docker nodesets

support dns-route53 plugin

release 2.4.0

[blacksmith] Bump version to 2.4.1-rc0

modulesync 1.9.3

modulesync 1.9.4

modulesync 1.9.6

Fix cron environment leaking

This commit moves the environment variable definition from the cron
resource environment parameter to the renewal script. This ensure that
environment variables aren't conflicting with other cronjobs.

Fixes voxpupuliGH-125.

allow puppetlabs/stdlib 5.x

Add support for dns-google plugin support

Migrate letsencrypt::config::ini helper to native Puppet 4

modulesync 2.0.0

Remove inline_template in favour of native Puppet code

* Migrates all inline_template calls to Puppet native code, as this is
recommended best practice
* Changes the default value/data type of webroot_paths and
additional_args to an Array instead of Optional[Array]
* Adds a data type for the certificate plugins for better readability

Allow additional domains to be added later

This commit adds support for expansion/shrinking of an already existing
certificate by defining a unique identifier (--cert-name) which maps to
the Puppet resource title. By doing this, we can ensure that always all
requested domains are contained in the certificate.

Fixes voxpupuliGH-94.

delete unneeded coverage_spec.rb

purge obsolete symlink setting in fixtures

allow puppet 6.x

add yumrepo_core module to fixtures

add cron_core module to fixtures

release 2.5.0

[blacksmith] Bump version to 2.5.1-rc0

Changed certonly::domains datatype to Array[String[1]] to accommodate wildcard domains

Added *.example.com to spec test

make the module compatible with FreeBSD

As FreeBSD also has no `root` group, the only change I noticed would be necessary to successfully run this module on FreeBSD is the same like for OpenBSD.

Added support for dns-cloudflare CertBot plugin.

Add cron day of month parameter

Use non-optional and strict data type for monthday param

Specify min/max Integer value for cron_monthday param

modulesync 2.3.1

Enable user to define cron time attributes instead of defaulting to random time

modulesync 2.4.0

Cron should also run with space in certonly title

Issue voxpupuli#91 reports that creating a letsencrypt::certonly resource with a
title containing a space will create a cron entry that will never run.
That's because it will create a cron entry like this:

    10 22 * * * /cron_scripts_path/renew-title with space.sh

That executes /cron_scripts_path/renew-title that obviously does not
exist. When the full cron command is put in quotes, it will run as
expected.

Improve performance of unless

Turns out, that calling certbot for every certificate resources takes
ages on systems with a lot of certificates. Some comparison:

certbot: ~0.7s
openssl: ~0.01s

use certbot repository

declare ubuntu16.04 as supported OS

add basics acceptance tests

add ubuntu 18.04 as supported OS

modulesync 2.5.0

bump lower puppet version from 4.10.0 to 5.5.8

fix fact scope; migrate tests to rspec-puppet-facts

facts called with $facts

fix rubocop

migrate tests to rspec-puppet-facts

increase spec coverage

Move fact definitions to inside the context

migrate FreeBSD tests to rspec-puppet-facts

fix tests for correct FS paths

cleanup spec files

vcs method, bump version of certbot

release 3.0.0

[blacksmith] Bump version to 3.0.1-rc0

Certonly: Ensure cron is absent if manage_cron is false

Certonly: Removing cron script file if manage_cron is false

Spec: Fixing certonly tests to manage cron script

Certonly: Refactor manage_cron Boolean to ensure_cron Enum

remove certonly acceptance test

Fedora doesn't use EPEL for certbot/letsencrypt.

Requested changes to simplify and follow preferred facts.

modulesync 2.5.1

Fix exec onlyif logic

This commit fixes the improved check logic introduces in
460a7b7. It ensure that the exec is
actually triggered if the cert file doesn't exist.

Move to script for domain validation

Add Fedora to tested systems list.

Add ability to control keysize. Default to 4096.

Add missing rspec value

Clean up some old Puppet 3 syntax

Add support for plugin dns-rfc2136

Add support for wildcard certs

Upgrade vcs version to 0.30.2

0.30.0 had a bug in combination with Pip 19.0.1 which made the
installation fail. certbot/certbot#6692

Fixes voxpupuli#172.

Fix for $live_path variable

Certbot uses the "--cert-name" parameter as part of the path, not the
first domain. This means the computed $live_path would be wrong if
$domain[0] differs from $title.

Rework letsencrypt::certonly to allow deprovisioning

+ Added $ensure attribute
+ Renamed $ensure_cron back to $manage_cron
+ Returned $manage_cron back to a Boolean value
+ Made execute of "letsencrypt certonly ${title}" conditional to $ensure == 'present'
+ Added cleanup of directory for domain certs when $ensure == 'absent'
+ Used global $ensure to signal desired state of cronjobs

Tiny style tweak

Add missing var doc in dns-rfc2136

Add support for certbot hooks

Add single quotes around all domains names in shell commands

Fixes voxpupuli#178

surfacing package_ensure in README

modulesync 2.6.0

release 4.0.0

[blacksmith] Bump version to 4.0.1-rc0

modulesync 2.7.0

fix

fix(docs): add basic documentation about facts and function

Add sample output from facts
Add notes on function usage

Bump dependency, because of new major release

fix(os): confine to *nix like systems

fix(format): update rubocop violations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants