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

systemd::escape function is does not escape a lot of other characters #242

Closed
jkroepke opened this issue Jan 23, 2022 · 9 comments · Fixed by #243
Closed

systemd::escape function is does not escape a lot of other characters #242

jkroepke opened this issue Jan 23, 2022 · 9 comments · Fixed by #243

Comments

@jkroepke
Copy link
Contributor

jkroepke commented Jan 23, 2022

The function systemd::escape introduced in #220 supports only a minimalistic subset of escape sequences.

For mount units, the - needs to be escaped and replaced by \x2d (hex representation).

In general, numbers, letters and some special chars are allowed and others needs to be replaced by a hex presentation.

Before someone ask 'Can you create a PR?'.

First I want to know if it worth to reimplement the systemd-escape logic into the puppet DSL or should the puppet function replaced by a function it's simply calling systemd-escape? In combination with Defferred functions, it should NOT add the dependency having systemd installed on the puppet master.

In case the current function should extend, I need to know how I can convert strings/chars to octal or hex decimal and do comparisons like if "x".ord > 127?

@traylenator
Copy link
Contributor

Problems with calling systemd-escape on master is that different versions of systemd do different things so you never quite no what you are getting. Server might even be running windows.

Indeed the - is missed from test cases.

@jkroepke
Copy link
Contributor Author

Problems with calling systemd-escape on master is that different versions of systemd do different things so you never quite no what you are getting. Server might even be running windows.

Or running inside kubernetes ...

Thats correct and thats the reason why this functions should be always called deferred. This avoids any dependency against the master since systemd-escape is executed on the agent only.

Indeed the - is missed from test cases.

And any other special chars, too. Mount points can go wild (looking at samba servers...)

@traylenator
Copy link
Contributor

Deferreed is pretty limited - e.g cannot be called in a template.

Hex is handled

When I wrote function I wrote from the docs - clearly not enough.

@jkroepke
Copy link
Contributor Author

jkroepke commented Jan 24, 2022

cannot be called in a template.

It's documented, how you can use Deferred function in epp templates, for example by Defer the epp template call, too. https://puppet.com/docs/puppet/6/deferring_functions.html#deferring_functions-deferred-function-example

Hex is handled

I saw this. Converting Hex to String is simple. Can you figure out, how I can convert character to hex?

$ puppet apply -e 'notify{"${sprintf("%x", "x")}": }'
Error: Evaluation Error: Error while evaluating a Function Call, invalid value for Integer(): "x" (line: 1, column: 11) on node 

@traylenator
Copy link
Contributor

Indeed. Needs an ord function in extlib ?

@jkroepke
Copy link
Contributor Author

And now, it begins to be complex.....

Also see the note from @bastelfreak .. #243 (comment)

@ekohl
Copy link
Member

ekohl commented Jan 24, 2022

Indeed. Needs an ord function in extlib ?

Why extlib and not stdlib?

@jkroepke
Copy link
Contributor Author

And why not puppet core?

I guess extlib would be the fastest way to get such a function. The lifecycle of puppet or stdlib maybe takes longer... depends on the puppet development team ...

@ekohl
Copy link
Member

ekohl commented Jan 24, 2022

That's indeed what I thought. It could even be first submitted to stdlib and once it's in core, the stdlib function could delegate.

weaselp pushed a commit to weaselp/puppet-systemd that referenced this issue Feb 8, 2022
We also need to escape dashes in unit names, for instance for .swap
services on device nodes with dashes in them.

systemd-escape escapes dashes:
  # systemd-escape -p /dev/disk/by-id/scsi-0QEMU_QEMU_HARDDISK_drive-ua-swap
  dev-disk-by\x2did-scsi\x2d0QEMU_QEMU_HARDDISK_drive\x2dua\x2dswap
but this function does not (yet).

In the long term, we may want to more closely match the behaviour of
systemd's unit-name.c, but this is all I need for now.

(Also cf. voxpupuli#242.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants