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

added function to returns a file system path as escaped by systemd #95

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions lib/puppet/parser/functions/systemd_escape.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
require 'puppet/util/execution'

module Puppet::Parser::Functions
newfunction(:systemd_escape, :type => :rvalue, :doc => <<-EOS
Returns a file system path as escaped by systemd.
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this was copied from the validate_cmd function code. It should be adapted to reflect the name of the function.

path = args[0]
cmd = "/bin/systemd-escape --path #{path}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguably, Linux OSes without systemd are getting more and more rare these days…

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably ok then (so long as they all put that binary under /bin)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ 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
  }
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Systemd: the unique layer to standardize everything on Linux OSes 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually need to provide an absolute path? I thought puppet is able to figure this out by itself?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use /usr/bin/env?

escaped = Puppet::Util::Execution.execute(cmd)
escaped.strip
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add the missing newline here.