added function to returns a file system path as escaped by systemd #95
Conversation
|
Hi @KoenDierckx, thanks for providing this PR. Can you please convert his to the modern functions API? Some docs are available at https://puppet.com/docs/puppet/4.9/functions_basics.html. Pretty helpful is also: https://github.com/puppetlabs/puppet-specifications/blob/master/language/func-api.md |
|
Use of the modern API is needed for correct ‘environment isolation’. With the new API, you can also namespace functions. This is recommended for all functions that aren’t built into puppet. |
There was a problem hiding this comment.
Use the modern Ruby function API mentioned by @bastelfreak and @alexjfisher
| ) do |args| | ||
| raise Puppet::ParseError, ("validate_cmd(): wrong number of arguments (#{args.length}; must be 1)") if args.length != 1 | ||
| path = args[0] | ||
| cmd = "/bin/systemd-escape --path #{path}" |
There was a problem hiding this comment.
The function will be run on the master, not the agents. Could this be a problem? (Perhaps the server isn't a systemd based system and doesn't have this command available).
There was a problem hiding this comment.
Arguably, Linux OSes without systemd are getting more and more rare these days…
There was a problem hiding this comment.
Probably ok then (so long as they all put that binary under /bin)
There was a problem hiding this comment.
$ which systemd-escape
/usr/bin/systemd-escape
$ facter os
{
architecture => "x86_64",
distro => {
codename => "n/a",
description => "Arch Linux",
id => "Arch",
release => {
full => "rolling",
major => "rolling"
},
specification => "1.4"
},
family => "Archlinux",
hardware => "x86_64",
name => "Archlinux",
release => {
full => "4.19.10-arch1-1-ARCH",
major => "4",
minor => "19"
},
selinux => {
enabled => false
}
}
There was a problem hiding this comment.
Systemd: the unique layer to standardize everything on Linux OSes 😄
There was a problem hiding this comment.
nitpick: It isn't the fault from the systemd team:
There was a problem hiding this comment.
Do we actually need to provide an absolute path? I thought puppet is able to figure this out by itself?
| https://www.freedesktop.org/software/systemd/man/systemd.unit.html#String%20Escaping%20for%20Inclusion%20in%20Unit%20Names | ||
| EOS | ||
| ) do |args| | ||
| raise Puppet::ParseError, ("validate_cmd(): wrong number of arguments (#{args.length}; must be 1)") if args.length != 1 |
There was a problem hiding this comment.
I'm guessing this was copied from the validate_cmd function code. It should be adapted to reflect the name of the function.
| escaped = Puppet::Util::Execution.execute(cmd) | ||
| escaped.strip | ||
| end | ||
| end |
There was a problem hiding this comment.
please add the missing newline here.
|
@KoenDierckx Hey there, could you finish up this PR? |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
New attempt #220 |
Fixes voxpupuli#95 Add EPEL GPG Key and logic to handle yum::gpgkeys
.rubocop.yml : update TrailingCommainLiteral
While creating a systemd mount and automount unit file, I needed a way to convert a mount point into a systemd unit name, as this requires some very specific conventions
https://www.freedesktop.org/software/systemd/man/systemd.unit.html#String%20Escaping%20for%20Inclusion%20in%20Unit%20Names
Instead of recreating the logic in puppet, I've created a custom function that calls the native systemd-escape binary.
This can be used like this:
$mount_name = systemd_escape($mountpoint)systemd::unit_file { "${mount_name}.mount": }