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.

Sign up for GitHub

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

Manage SSH key via 'debops.authorized_keys' role #15

Merged
merged 7 commits into from
Jul 23, 2016
Merged

Manage SSH key via 'debops.authorized_keys' role #15

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e734114
Manage SSH key via 'debops.authorized_keys' role
ganto Jul 18, 2016
e2a8f3b
Fix inventory group name
ganto Jul 19, 2016
89f86e1
Fix typo
ganto Jul 19, 2016
5e1c71c
Move host fact lookup to key definition
ganto Jul 20, 2016
3146772
Only query host facts if server group and host is found
ganto Jul 22, 2016
cac0bfd
Fix typo in 'authorized_keys__dependent_list' definition
ganto Jul 23, 2016
ed18b97
Don't try to create root user
ganto Jul 23, 2016
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
10 changes: 10 additions & 0 deletions defaults/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,16 @@ checkmk_agent__user_home: '/var/lib/nagios'
# SSH Public key for logging in with SSH user.
checkmk_agent__user_key: ''


# .. envvar:: checkmk_agent__authorized_keys__dependent_list
#
# Authorized key configuration for the ``debops.authorized_keys`` role.
checkmk_agent__authorized_keys__dependent_list:
- name: '{{ checkmk_agent__ssh_user }}'
ssh_keys: [ '{{ checkmk_agent__user_key }} if {{ checkmk_agent__user_key|d() }} else {{ hostvars[groups["debops_service_checkmk_server"][0]].ansible_local.checkmk_server.ssh_publickey|d("") }}' ]
Copy link
Member Author

Choose a reason for hiding this comment

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

@drybjed: what do you think of this? Is there a way how you implemented something like this more elegant?

somehow the hostvars[groups["debops_service_checkmk_server"]] expression will trigger a deprecation warning in case the group is not defined in the inventory.

Copy link

Choose a reason for hiding this comment

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

It's sshkeys. I don't think that you need to lookup keys in local facts. If you specify empty sshkeys, role should just skip management of the keys. So, defining only checkmk_agent__user_key variable in inventory should be enough.

And here, just set it like this:

- name: '{{ checkmk_agent__ssh_user }}'
  sshkeys: '{{ checkmk_agent__ssh_key }}'

That should do what you expect, although I would test it first to be sure.

Copy link
Member Author

Choose a reason for hiding this comment

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

The point is, that they key is generated during the setup of the monitoring site. If the user has to specify this manually, it would abort the playbook run, then the user has to lookup they key manually. Or he need to generate a keypair in advance and configure them before running the server setup.

I wish to find a solution where the setup just works. See also debops-contrib/ansible-checkmk_server#15

Copy link

@drybjed drybjed Jul 19, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

For this you will probably want to split the role into normal and "env" role (from "environment"). It would work like this:

- role: `checkmk_server/env'
- role: `debops.authorized_keys'
- role: `checkmk_server`

That way you can prepare data for other roles that need it, hand it off to the other roles and return back to managing your application. For an example, check the debops.php role and its example playbook.

Copy link
Member Author

Choose a reason for hiding this comment

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

For this you will probably want to split the role into normal and "env" role (from "environment").

@drybjed I don't see what problem this is going to solve.

Is this bad practice to lookup a service provider in the host facts? This code is functional and behaves as intended, just that it emits a deprecation warning, in case no debops_service_checkmk_service group has been defined.

I will move it to the variable definition of checkmk_agent__user_key then the code will become a bit easier to read.

Copy link

Choose a reason for hiding this comment

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

What deprecated warning, about Undefined variable? This will be error in the future, you need to fix that.

Moving the lookup code to the variable is good, then users can modify it if they wish without the need to modify the role itself.

As for the 'role/env' solution - this comes from a pretty specific design principles of DebOps roles - they are supposed to be read-only, and you should pass configuration of other roles through default variables. In specific situations, like generating data on a host by your role, or using lookup templates, you might find that Ansible does not have the data it needs at this specific point in the playbook, or it does the lookup in the wrong role.

In the given debops.php example, the role checks what PHP package versions are available and then acts on that information, however another role, debops.logrotate is also using this information to generate the correct logrotate configuration for the php-fpm service. The information about PHP version is stored in a variable, however only debops.php sets this variable. Since debops.logrotate is supposed to run before debops.php, it would not know beforehand what PHP version it should use to generate the config file. Basically a casuality violation. :-)

To resolve this, I decided to split the debops.php role essentially into two - first one, debops.php/env contains minimum amount of tasks to "set the environment" for other roles - it checks what PHP versions are available, and even configures external APT repositories. When its tasks are done, other roles like debops.logrotate can act on variables passed through debops.php defaults since the required facts are set in the memory. After that, debops.php continues as usual to finish the configuration.

As I said, this is a very situational solution and you won't need this with the most roles. However, I suggested it here since you mentioned that the SSH keys on the specific hosts are generated by the role. To avioid the need to run the playbook twice to complete the configuration, you can do it like this:

  • checkmk_server/env generates the SSH keys and slurps them into memory
  • debops.authorized_keys configures the SSH keys using data from checkmk_server defaults + SSH keys
  • checkmk_server finishes the configuration

This should keep things clean and consistent.

options: '{{ authorized_keys__options_map.strict }}'
key_options: 'command="{{ "/usr/bin/sudo " if not checkmk_agent__ssh_user == "root" else "" }}{{ checkmk_agent__exec }}"'

# .. )))

# .. Agent plugins options (((
Expand Down
7 changes: 7 additions & 0 deletions docs/playbooks/checkmk_agent.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@
- '{{ checkmk_agent__ferm__dependent_rules }}'
when: (checkmk_agent|d() and 'xinetd' in checkmk_agent)

- role: debops.authorized_keys
tags: [ 'depend::authorized_keys', 'depend::authorized_keys:checkmk_agent',
'depend-of::checkmk_agent', 'type::dependency' ]
authorized_keys__dependent_list:
- '{{ checkmk_agent__authorized_keys__dependent_list }}'
when: (checkmk_agent|d() and 'ssh' in checkmk_agent)

- role: debops.mariadb
tags: [ 'depend::mariadb', 'depend::mariadb:checkmk_agent',
'depend-of::checkmk_agent', 'type::dependency' ]
Expand Down
16 changes: 0 additions & 16 deletions tasks/ssh_user.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,6 @@
home: '{{ checkmk_agent__user_home }}'
createhome: 'yes'

- name: Create SSH authorized_keys directory
file:
name: '/etc/ssh/authorized_keys'
owner: 'root'
group: 'root'
mode: '755'
state: 'directory'

- name: Set SSH key for public key login
template:
src: '{{ lookup("template_src", "etc/ssh/authorized_keys/nagios.j2") }}'
dest: '/etc/ssh/authorized_keys/{{ checkmk_agent__ssh_user }}'
owner: 'root'
group: 'root'
mode: '644'

- name: Enable sudo rule for Check_MK SSH user
template:
src: '{{ lookup("template_src", "etc/sudoers.d/check-mk-agent.j2") }}'
Expand Down
3 changes: 0 additions & 3 deletions templates/etc/ssh/authorized_keys/nagios.j2
View file
Open in desktop

This file was deleted.