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

[WIP] Fix outdated systemd config parameter #91

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[WIP] Fix outdated systemd config parameter #91

wants to merge 2 commits into from

Conversation

zados
Copy link

@zados zados commented Oct 18, 2017

SUMMARY

Capabilities is succeeded by AmbientCapabilities in Systemd. Debian 9 on 4.9.0-4-amd64 running systemd 232-25+deb9u1 displays:

[/etc/systemd/system/vault.service:21] Support for option Capabilities= has been removed and it is ignored

Therefore it is not possible to start the Systemd service with mlock support enabled:
systemd[4092]: vault.service: Failed at step SECUREBITS spawning /usr/local/bin/vault: Operation not permitted vault.service: Main process exited, code=exited, status=213/SECUREBITS

TESTS/SPECS

This has change been tested on my local vault server. The service starts and Vault is confirming that mlock.
vault[18669]: Mlock: supported: true, enabled: true

@jsok
Copy link
Owner

jsok commented Oct 18, 2017

Thanks for picking this up.

Looking at the systemd changelog this rename occurred on release 229.

However we can't just blindly update the option name since some users may not be on a recent enough version of systemd!
This change will require either a manual flag or some more intelligent detection of systemd version, e.g. something like https://github.com/camptocamp/puppet-systemd/blob/master/lib/facter/systemd.rb#L40

As for testing, it will require actual puppet-rspec tests rather than a "works on my machine" kind of thing.

systemd 229

  • A new service setting AmbientCapabilities= has been added. It allows
    configuration of additional Linux process capabilities that are
    passed to the activated processes. This is only available on very
    recent kernels.

systemd 230

  • The Capabilities= unit file setting has been removed (it is ignored
    for backwards compatibility). AmbientCapabilities= and
    CapabilityBoundingSet= should be used instead.

@zados
Copy link
Author

zados commented Oct 19, 2017

However we can't just blindly update the option name since some users may not be on a recent enough version of systemd!

I agree.

I will see if I can write a something to mitigate this.

As for testing, it will require actual puppet-rspec tests rather than a "works on my machine" kind of thing.

This we will need then definitely!

context 'includes systemd init script' do
context 'includes systemd init script with systemd_version < 229' do
let(:facts) {{
:systemd_version => 228
Copy link
Owner

Choose a reason for hiding this comment

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

Some funky indentation in all the :facts blocks you've added, can you de-dent to match.

Copy link
Author

Choose a reason for hiding this comment

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

Will do.

@@ -23,7 +23,11 @@ CapabilityBoundingSet=CAP_SYSLOG
NoNewPrivileges=yes
<% else -%>
SecureBits=keep-caps
<% if scope['::systemd_version'] > 229 -%>
Copy link
Owner

Choose a reason for hiding this comment

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

There's something up here looking at the test failures on Travis.
I'm not sure if the confine is working as expected.

 error during compilation: Evaluation Error: Error while evaluating a Function Call, Failed to parse template vault/vault.systemd.erb:
 Filepath: /home/travis/build/jsok/puppet-vault/spec/fixtures/modules/vault/templates/vault.systemd.erb
 Line: 26
 Detail: undefined method `>' for nil:NilClass

https://travis-ci.org/jsok/puppet-vault/jobs/290455939

Copy link
Author

Choose a reason for hiding this comment

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

Yeah I am still on it. Just using Travis to test it.

@zados zados changed the title Fix outdated systemd config parameter [WIP] Fix outdated systemd config parameter Oct 23, 2017
@zados
Copy link
Author

zados commented Oct 23, 2017

I think the integration test is broken, as it only does apply on a single manifest. Therefore the fact systemd_version is never imported. I tried it on my live system and it worked. As I have literally no experience with Travis and rspec. Is there a way to include the fact?

@maartenschalekamp
Copy link

maartenschalekamp commented Dec 7, 2018

Any idea if this will be merged?

@jsok
Copy link
Owner

jsok commented Dec 7, 2018

@maartenschalekamp you're welcome to branch off it and make the necessary fixes.

@jsok jsok removed this from the 2.0.0 - Puppet 5 support milestone Mar 17, 2019
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 this pull request may close these issues.

3 participants