remove `systemd::escape` usage for `timer_wrapper`

[TheMeier]

Pull Request (PR) description

Don't treat unit names with systemd::escape. If you have systemd::timer_wrapper instances with names that contain special characters like slashes and umlauts this will result in a validation error. In such cases you can produce the previous behaviour by using systemd::escape yourself for example:

systemd::timer_wrapper {systemd::escape( 'my_timer'):
  ensure        => 'present',
  command       => '/usr/bin/echo "Hello World"',
  on_calendar   => '*:0/5',
}

It is possible that this change will create duplicate resources in case the resource name was escaped but the unescaped name is also valid. In those cases make sure to remove the duplicate, e.g. like so:

systemd::timer_wrapper { systemd::escape("someTitle"):
  ensure => absent,
}

This Pull Request (PR) fixes the following issues

Fixes #451

[TheMeier]

Question is, is this considered breaking?

[bastelfreak]

mhm this is probably a breaking change to some users. Should we remove it or should we document that some characters can cause strange unit names?

[saz]

I'd still vote for getting rid of the systemd::escape usage, as it's completely unexpected behavior.

Another possible way: add a new parameter to force a specific name without this behavior?

[TheMeier]

The 7.0.0 major release is still pending https://github.com/voxpupuli/puppet-systemd/pull/447 so we could get it in there .... I am also not really happy with the escape

[smortex]

I think this make sense. As a backwards-incompatible change we should indicate explicitly how users can check if they are affected and what they need to do in the first comment of the PR

[webcompas]

I'd still vote for getting rid of the systemd::escape usage, as it's completely unexpected behavior.

I'd also agree that there shouldn't be any automatic escaping. It's very confusing and it makes it hard for other tools which rely on predictable unit names.

[saz]

If we're cleaning up unit files created with the escaped name and adding a new one, will this still count as a breaking change? Somebody might depend on systemd::manage_unit directly, but I'd rather not expect that, as you will stumble upon the unexpected name.

Something like:

$unit_name_escaped = systemd::escape($title)
systemd::manage_unit { "${unit_name_escaped}.service":
  ensure => absent,
}
systemd::manage_unit { "${unit_name_escaped}.timer":
  ensure => absent,
}

systemd::manage_unit { "${title}.service":
...
}
systemd::manage_unit { "${title}.timer":
...
}

Edit: maybe this needs some condition, e.g. $unit_name_escaped != $title to avoid duplicate resources on explicitly escaped unit names.

[TheMeier]

As this feature is quite new, I don't expect a wide usage of it yet. I would like to avoid more complexity here and keep it as is. We have a major release with multiple breaking changes lined up anyway.

[saz]

@TheMeier Maybe we should give an example on how to clean it up in the changelog?

Something like this should be working:

systemd::timer_wrapper { systemd::escape("someTitle"):
  ensure => absent,
}

[TheMeier]

I have updated the MR description

[TheMeier]

@traylenator can you include that in https://github.com/voxpupuli/puppet-systemd/pull/447? I can also take care of that if you like.