[libvirt-jenkins-ci PATCH 0/5] Introduce the new 'gitlab' flavor
Erik Skultety (5): guests: templates: Introduce a gitlab-runner systemd service template guests: templates: Introduce a gitlab-runner RC init service template guests: Introduce the new 'gitlab' flavor playbooks: gitlab: Force a random password for the root account guests: lcitool: Enable the new 'gitlab' flavor in the lcitool script guests/group_vars/all/main.yml | 3 + guests/lcitool | 29 ++++++-- guests/playbooks/update/main.yml | 5 ++ guests/playbooks/update/tasks/gitlab.yml | 68 +++++++++++++++++++ .../update/templates/gitlab-runner.j2 | 32 +++++++++ .../update/templates/gitlab-runner.service.j2 | 12 ++++ 6 files changed, 144 insertions(+), 5 deletions(-) create mode 100644 guests/playbooks/update/tasks/gitlab.yml create mode 100644 guests/playbooks/update/templates/gitlab-runner.j2 create mode 100644 guests/playbooks/update/templates/gitlab-runner.service.j2 -- 2.25.1
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- .../update/templates/gitlab-runner.service.j2 | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 guests/playbooks/update/templates/gitlab-runner.service.j2 diff --git a/guests/playbooks/update/templates/gitlab-runner.service.j2 b/guests/playbooks/update/templates/gitlab-runner.service.j2 new file mode 100644 index 0000000..f7a70dc --- /dev/null +++ b/guests/playbooks/update/templates/gitlab-runner.service.j2 @@ -0,0 +1,12 @@ +[Unit] +Description=GitLab Runner +After=network.target +ConditionFileIsExecutable=/usr/local/bin/gitlab-runner + +[Service] +ExecStart=/usr/local/bin/gitlab-runner run --user gitlab --working-directory /home/gitlab --config /home/gitlab/.gitlab-runner/config.toml +Restart=always + +[Install] +WantedBy=multi-user.target + -- 2.25.1
On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
+++ b/guests/playbooks/update/templates/gitlab-runner.service.j2 @@ -0,0 +1,12 @@ +[Unit] +Description=GitLab Runner +After=network.target +ConditionFileIsExecutable=/usr/local/bin/gitlab-runner + +[Service] +ExecStart=/usr/local/bin/gitlab-runner run --user gitlab --working-directory /home/gitlab --config /home/gitlab/.gitlab-runner/config.toml
Since we're going to use /home/gitlab as the base directory for all things gitlab-runner, I would suggest we store the gitlab-runner binary itself inside that directory too, and since we're pointing the application to the configuration file explicitly we could shorten that a bit and use /home/gitlab/config.toml as the path. The only setting that concerns me a bit is --working-directory: is that the base directory for gitlab-runner's operation, or is that the one where it will store source code and temporary files? Because in the latter case I would feel more comfortable if we used a subdirectory of /home/gitlab, rather than the home directory itself, as path. -- Andrea Bolognani / Red Hat / Virtualization
On Tue, Mar 31, 2020 at 04:32:15PM +0200, Andrea Bolognani wrote:
On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
+++ b/guests/playbooks/update/templates/gitlab-runner.service.j2 @@ -0,0 +1,12 @@ +[Unit] +Description=GitLab Runner +After=network.target +ConditionFileIsExecutable=/usr/local/bin/gitlab-runner + +[Service] +ExecStart=/usr/local/bin/gitlab-runner run --user gitlab --working-directory /home/gitlab --config /home/gitlab/.gitlab-runner/config.toml
Since we're going to use /home/gitlab as the base directory for all things gitlab-runner, I would suggest we store the gitlab-runner binary itself inside that directory too, and since we're pointing the application to the configuration file explicitly we could shorten that a bit and use /home/gitlab/config.toml as the path.
The only setting that concerns me a bit is --working-directory: is that the base directory for gitlab-runner's operation, or is that the one where it will store source code and temporary files? Because in the latter case I would feel more comfortable if we used a subdirectory of /home/gitlab, rather than the home directory itself, as path.
IIUC it uses it for both, but it creates a very long subdirectory structure by default anyway, so there's nothing here to be concerned about really. -- Erik Skultety
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- .../update/templates/gitlab-runner.j2 | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 guests/playbooks/update/templates/gitlab-runner.j2 diff --git a/guests/playbooks/update/templates/gitlab-runner.j2 b/guests/playbooks/update/templates/gitlab-runner.j2 new file mode 100644 index 0000000..5063a53 --- /dev/null +++ b/guests/playbooks/update/templates/gitlab-runner.j2 @@ -0,0 +1,32 @@ +#!/bin/sh +# PROVIDE: gitlab_runner +# REQUIRE: DAEMON NETWORKING +# BEFORE: +# KEYWORD: + +. /etc/rc.subr + +name="gitlab_runner" +rcvar="gitlab_runner_enable" + +user="{{ flavor }}" +user_home="/home/{{ flavor }}" +command="/usr/local/bin/gitlab-runner" +command_args="run --user ${user} --working-directory ${user_home} --config ${user_home}/.gitlab-runner/config.toml" +pidfile="/var/run/${name}.pid" + +start_cmd="gitlab_runner_start" + +gitlab_runner_start() +{ + export USER=${user} + export HOME=${user_home} + export PATH=${PATH}:/usr/local/bin/:/usr/local/sbin/ + if checkyesno ${rcvar}; then + cd ${user_home} + /usr/sbin/daemon -p ${pidfile} ${command} ${command_args} > /var/log/gitlab-runner.log 2>&1 + fi +} + +load_rc_config $name +run_rc_command $1 -- 2.25.1
On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
guests: templates: Introduce a gitlab-runner RC init service template
"RC init" is sort of a loaded term, I'd just call out FreeBSD directly.
+++ b/guests/playbooks/update/templates/gitlab-runner.j2 @@ -0,0 +1,32 @@ +#!/bin/sh +# PROVIDE: gitlab_runner +# REQUIRE: DAEMON NETWORKING +# BEFORE: +# KEYWORD:
This seems to be heavily based on [1], so maybe include a reference to that URL somewhere.
+user="{{ flavor }}" +user_home="/home/{{ flavor }}"
Either use substitution for {{ flavor }} both here and in the systemd service, or in neither. Personally I'd go for the latter, since it's not really buying us much.
+gitlab_runner_start() +{ + export USER=${user} + export HOME=${user_home} + export PATH=${PATH}:/usr/local/bin/:/usr/local/sbin/ + if checkyesno ${rcvar}; then + cd ${user_home} + /usr/sbin/daemon -p ${pidfile} ${command} ${command_args} > /var/log/gitlab-runner.log 2>&1
The version in the official documentation does this a little differently... I guess the difference is that in their case the gitlab-runner application is running as the gitlab user, wereas in ours the daemon is running as root but is instructed to execute workloads as the gitlab user. The latter seems fine, as that's what happens on Linux as well, but have you fully considered the security implications? [1] https://docs.gitlab.com/runner/install/freebsd.html -- Andrea Bolognani / Red Hat / Virtualization
On Tue, Mar 31, 2020 at 04:42:10PM +0200, Andrea Bolognani wrote:
On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
guests: templates: Introduce a gitlab-runner RC init service template
"RC init" is sort of a loaded term, I'd just call out FreeBSD directly.
+++ b/guests/playbooks/update/templates/gitlab-runner.j2 @@ -0,0 +1,32 @@ +#!/bin/sh +# PROVIDE: gitlab_runner +# REQUIRE: DAEMON NETWORKING +# BEFORE: +# KEYWORD:
This seems to be heavily based on [1], so maybe include a reference to that URL somewhere.
Since it's not 1:1, I'll put it into the commit message.
+user="{{ flavor }}" +user_home="/home/{{ flavor }}"
Either use substitution for {{ flavor }} both here and in the systemd service, or in neither. Personally I'd go for the latter, since it's not really buying us much.
True, it can be hardcoded.
+gitlab_runner_start() +{ + export USER=${user} + export HOME=${user_home} + export PATH=${PATH}:/usr/local/bin/:/usr/local/sbin/ + if checkyesno ${rcvar}; then + cd ${user_home} + /usr/sbin/daemon -p ${pidfile} ${command} ${command_args} > /var/log/gitlab-runner.log 2>&1
The version in the official documentation does this a little differently... I guess the difference is that in their case the gitlab-runner application is running as the gitlab user, wereas in ours the daemon is running as root but is instructed to execute workloads as the gitlab user. The latter seems fine, as that's what happens on Linux as well, but have you fully considered the security implications?
It is different because I wanted a unified behaviour on Linux and FreeBSD. What security implications are you talking about, can you be more specific? The machines are going to run behind a NAT, the daemon executing the service should be trusted by default (otherwise, engage the tin foil hat mode), the gitlab user doesn't have sudo permissions (and we should not trust this user), and in later patches I setup a random root password, so that only access via an SSH pub key to the root account is allowed. Alternatively, we could set up another service user which will have sudo (not passwordless) access and will not run any services, so that root isn't accessible over the network, would consider that as suitable precaution measures?
-- Erik Skultety
On Fri, 2020-04-03 at 09:21 +0200, Erik Skultety wrote:
On Tue, Mar 31, 2020 at 04:42:10PM +0200, Andrea Bolognani wrote:
On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
+gitlab_runner_start() +{ + export USER=${user} + export HOME=${user_home} + export PATH=${PATH}:/usr/local/bin/:/usr/local/sbin/ + if checkyesno ${rcvar}; then + cd ${user_home} + /usr/sbin/daemon -p ${pidfile} ${command} ${command_args} > /var/log/gitlab-runner.log 2>&1
The version in the official documentation does this a little differently... I guess the difference is that in their case the gitlab-runner application is running as the gitlab user, wereas in ours the daemon is running as root but is instructed to execute workloads as the gitlab user. The latter seems fine, as that's what happens on Linux as well, but have you fully considered the security implications?
It is different because I wanted a unified behaviour on Linux and FreeBSD. What security implications are you talking about, can you be more specific? The machines are going to run behind a NAT, the daemon executing the service should be trusted by default (otherwise, engage the tin foil hat mode), the gitlab user doesn't have sudo permissions (and we should not trust this user), and in later patches I setup a random root password, so that only access via an SSH pub key to the root account is allowed. Alternatively, we could set up another service user which will have sudo (not passwordless) access and will not run any services, so that root isn't accessible over the network, would consider that as suitable precaution measures?
I trust gitlab-runner in the sense that I don't expect it to contain intentional backdoor, but not necessarily in the sense that I expect it to be entirely bug-free and impossible for an attacker to abuse as a compromise vector. With that in mind, running it as an unprivileged user right off the bat is obviously strictly safer than running it as root and delegating the privilege dropping part to it. Having the same behavior for both Linux and FreeBSD is certainly something that we should strive for, but can we make that behavior the safest one of the two? I have tested this, though not extensively, on Linux and adding User=gitlab to the service file seems to be basically all that's needed to make it work; for FreeBSD this setup is the one described in the official documentation, so I'm going to assume it's not going to cause any issues either. If we find that running gitlab-runner as an unprivileged user gets in the way we can certainly go back on this decision, but I would like to try and see if we can get the safest option to work first. -- Andrea Bolognani / Red Hat / Virtualization
On Fri, Apr 03, 2020 at 03:50:21PM +0200, Andrea Bolognani wrote:
On Fri, 2020-04-03 at 09:21 +0200, Erik Skultety wrote:
On Tue, Mar 31, 2020 at 04:42:10PM +0200, Andrea Bolognani wrote:
On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
+gitlab_runner_start() +{ + export USER=${user} + export HOME=${user_home} + export PATH=${PATH}:/usr/local/bin/:/usr/local/sbin/ + if checkyesno ${rcvar}; then + cd ${user_home} + /usr/sbin/daemon -p ${pidfile} ${command} ${command_args} > /var/log/gitlab-runner.log 2>&1
The version in the official documentation does this a little differently... I guess the difference is that in their case the gitlab-runner application is running as the gitlab user, wereas in ours the daemon is running as root but is instructed to execute workloads as the gitlab user. The latter seems fine, as that's what happens on Linux as well, but have you fully considered the security implications?
It is different because I wanted a unified behaviour on Linux and FreeBSD. What security implications are you talking about, can you be more specific? The machines are going to run behind a NAT, the daemon executing the service should be trusted by default (otherwise, engage the tin foil hat mode), the gitlab user doesn't have sudo permissions (and we should not trust this user), and in later patches I setup a random root password, so that only access via an SSH pub key to the root account is allowed. Alternatively, we could set up another service user which will have sudo (not passwordless) access and will not run any services, so that root isn't accessible over the network, would consider that as suitable precaution measures?
I trust gitlab-runner in the sense that I don't expect it to contain intentional backdoor, but not necessarily in the sense that I expect it to be entirely bug-free and impossible for an attacker to abuse as a compromise vector. With that in mind, running it as an unprivileged user right off the bat is obviously strictly safer than running it as root and delegating the privilege dropping part to it.
Having the same behavior for both Linux and FreeBSD is certainly something that we should strive for, but can we make that behavior the safest one of the two?
I have tested this, though not extensively, on Linux and adding User=gitlab to the service file seems to be basically all that's
Did ^this actually work? I recall having some issues on Linux when I used the User= directive and I could not get the agent pull a job from the server, however I used it in combination with WorkingDir (or what the proper name is) so it may have also been that one, but I definitely tried what you describe and didn't work for me, hence the patch looks like the way it does, but now I have to go verify your claim and if indeed it works then we can go with what you suggest for sure. I admit that I was playing with a handful of different runner setups, both containerized and direct executions, so a tiny mistake may have slipped in despite the fact I was restoring the VM state from a snapshot.
needed to make it work; for FreeBSD this setup is the one described in the official documentation, so I'm going to assume it's not going to cause any issues either.
If we find that running gitlab-runner as an unprivileged user gets in the way we can certainly go back on this decision, but I would like to try and see if we can get the safest option to work first.
Let me try again and I'll get back to you. -- Erik Skultety
On Fri, 2020-04-03 at 16:04 +0200, Erik Skultety wrote:
On Fri, Apr 03, 2020 at 03:50:21PM +0200, Andrea Bolognani wrote:
On Fri, 2020-04-03 at 09:21 +0200, Erik Skultety wrote:
On Tue, Mar 31, 2020 at 04:42:10PM +0200, Andrea Bolognani wrote:
On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
+gitlab_runner_start() +{ + export USER=${user} + export HOME=${user_home} + export PATH=${PATH}:/usr/local/bin/:/usr/local/sbin/ + if checkyesno ${rcvar}; then + cd ${user_home} + /usr/sbin/daemon -p ${pidfile} ${command} ${command_args} > /var/log/gitlab-runner.log 2>&1
The version in the official documentation does this a little differently... I guess the difference is that in their case the gitlab-runner application is running as the gitlab user, wereas in ours the daemon is running as root but is instructed to execute workloads as the gitlab user. The latter seems fine, as that's what happens on Linux as well, but have you fully considered the security implications?
It is different because I wanted a unified behaviour on Linux and FreeBSD. What security implications are you talking about, can you be more specific? The machines are going to run behind a NAT, the daemon executing the service should be trusted by default (otherwise, engage the tin foil hat mode), the gitlab user doesn't have sudo permissions (and we should not trust this user), and in later patches I setup a random root password, so that only access via an SSH pub key to the root account is allowed. Alternatively, we could set up another service user which will have sudo (not passwordless) access and will not run any services, so that root isn't accessible over the network, would consider that as suitable precaution measures?
I trust gitlab-runner in the sense that I don't expect it to contain intentional backdoor, but not necessarily in the sense that I expect it to be entirely bug-free and impossible for an attacker to abuse as a compromise vector. With that in mind, running it as an unprivileged user right off the bat is obviously strictly safer than running it as root and delegating the privilege dropping part to it.
Having the same behavior for both Linux and FreeBSD is certainly something that we should strive for, but can we make that behavior the safest one of the two?
I have tested this, though not extensively, on Linux and adding User=gitlab to the service file seems to be basically all that's
Did ^this actually work? I recall having some issues on Linux when I used the User= directive and I could not get the agent pull a job from the server, however I used it in combination with WorkingDir (or what the proper name is) so it may have also been that one, but I definitely tried what you describe and didn't work for me, hence the patch looks like the way it does, but now I have to go verify your claim and if indeed it works then we can go with what you suggest for sure. I admit that I was playing with a handful of different runner setups, both containerized and direct executions, so a tiny mistake may have slipped in despite the fact I was restoring the VM state from a snapshot.
needed to make it work; for FreeBSD this setup is the one described in the official documentation, so I'm going to assume it's not going to cause any issues either.
If we find that running gitlab-runner as an unprivileged user gets in the way we can certainly go back on this decision, but I would like to try and see if we can get the safest option to work first.
Let me try again and I'll get back to you.
-- Erik Skultety -- Andrea Bolognani / Red Hat / Virtualization
Looks like I somehow sent an empty reply by mistake the first time around. Let's try again... On Fri, 2020-04-03 at 16:04 +0200, Erik Skultety wrote:
On Fri, Apr 03, 2020 at 03:50:21PM +0200, Andrea Bolognani wrote:
I have tested this, though not extensively, on Linux and adding User=gitlab to the service file seems to be basically all that's
Did ^this actually work? I recall having some issues on Linux when I used the User= directive and I could not get the agent pull a job from the server,
It would seem that way: https://gitlab.com/abologna/libvirt/pipelines/132661098 Pay no attention to the failures in the second round of jobs, the Docker daemon seems to be having some trouble getting in touch with quay.io right now. It managed to pull the two images necessary for the prebuild jobs before that, however. Of course for that to work I had to add the gitlab user to the docker group, which is another potential attack venue... The alternative is running everything as root, however, so it would still seem preferable to that. Hopefully at some point gitlab-runner will grow a Podman executor :) -- Andrea Bolognani / Red Hat / Virtualization
On Fri, Apr 03, 2020 at 06:26:23PM +0200, Andrea Bolognani wrote:
Looks like I somehow sent an empty reply by mistake the first time around. Let's try again...
On Fri, 2020-04-03 at 16:04 +0200, Erik Skultety wrote:
On Fri, Apr 03, 2020 at 03:50:21PM +0200, Andrea Bolognani wrote:
I have tested this, though not extensively, on Linux and adding User=gitlab to the service file seems to be basically all that's
Did ^this actually work? I recall having some issues on Linux when I used the User= directive and I could not get the agent pull a job from the server,
It would seem that way:
Sorry, what exactly am I looking at ^here? Those are all containers, whereas these patches are targeting VMs mainly. Anyhow, now I remember why I didn't go with User=gitlab systemd service directive and opted for dropping the privileges by gitlab-runner itself, the build fails on debian-9: https://gitlab.com/eskultety/libvirt/-/jobs/498107944 ..so far, I haven't been able to identify the problem on debian. In fact, if I create the directory forcefully, I get: https://gitlab.com/eskultety/libvirt/-/jobs/499941944 ...which is even weirder because it doesn't tell me anything and not even debug messages on gitlab-runner reveal what the issue is, it just "fails the job" Centos-7 (identical setup): https://gitlab.com/eskultety/libvirt/-/jobs/498107857 FreeBSD: https://gitlab.com/eskultety/libvirt/-/jobs/498107686 -- Erik Skultety
On Mon, Apr 06, 2020 at 12:09:50PM +0200, Erik Skultety wrote:
On Fri, Apr 03, 2020 at 06:26:23PM +0200, Andrea Bolognani wrote:
Looks like I somehow sent an empty reply by mistake the first time around. Let's try again...
On Fri, 2020-04-03 at 16:04 +0200, Erik Skultety wrote:
On Fri, Apr 03, 2020 at 03:50:21PM +0200, Andrea Bolognani wrote:
I have tested this, though not extensively, on Linux and adding User=gitlab to the service file seems to be basically all that's
Did ^this actually work? I recall having some issues on Linux when I used the User= directive and I could not get the agent pull a job from the server,
It would seem that way:
Sorry, what exactly am I looking at ^here? Those are all containers, whereas these patches are targeting VMs mainly.
Anyhow, now I remember why I didn't go with User=gitlab systemd service directive and opted for dropping the privileges by gitlab-runner itself, the build fails on debian-9: https://gitlab.com/eskultety/libvirt/-/jobs/498107944
..so far, I haven't been able to identify the problem on debian. In fact, if I create the directory forcefully, I get: https://gitlab.com/eskultety/libvirt/-/jobs/499941944
Found it [1],[2] [1] https://gitlab.com/gitlab-org/gitlab-runner/issues/4449 [2] https://gitlab.com/gitlab-org/gitlab-runner/issues/1379 If I haven't missed anything in the discussion, gitlab is going with the following "fix": https://gitlab.com/ubarbaxor/gitlab-runner/-/merge_requests/1/diffs?commit_i... Unfortunately for us, that "fix" is irrelevant to our use case as it's only available for packaged versions of gitlab-runner and by the time the package would be installed and created the user without the skeleton files we'd had already created the user profile including them, because we need .bashrc anyway. However, after wiping the contents of .bash_logout, the build passes: https://gitlab.com/eskultety/libvirt/-/jobs/499969944 So, I suggest I add an additional ansible task that removes .bash_logout (it's empty on RH-like distros and isn't even created on FreeBSD) and adjust the service file as you suggested in your original patch review, that way, we run the gitlab-runner with the right privileges since the very begining. -- Erik Skultety
On Mon, 2020-04-06 at 12:52 +0200, Erik Skultety wrote:
On Mon, Apr 06, 2020 at 12:09:50PM +0200, Erik Skultety wrote:
Anyhow, now I remember why I didn't go with User=gitlab systemd service directive and opted for dropping the privileges by gitlab-runner itself, the build fails on debian-9: https://gitlab.com/eskultety/libvirt/-/jobs/498107944
..so far, I haven't been able to identify the problem on debian. In fact, if I create the directory forcefully, I get: https://gitlab.com/eskultety/libvirt/-/jobs/499941944
Found it [1],[2] [1] https://gitlab.com/gitlab-org/gitlab-runner/issues/4449 [2] https://gitlab.com/gitlab-org/gitlab-runner/issues/1379
If I haven't missed anything in the discussion, gitlab is going with the following "fix": https://gitlab.com/ubarbaxor/gitlab-runner/-/merge_requests/1/diffs?commit_i...
Unfortunately for us, that "fix" is irrelevant to our use case as it's only available for packaged versions of gitlab-runner and by the time the package would be installed and created the user without the skeleton files we'd had already created the user profile including them, because we need .bashrc anyway.
However, after wiping the contents of .bash_logout, the build passes: https://gitlab.com/eskultety/libvirt/-/jobs/499969944
Oh yeah, I had encountered the same issue during my tests, and after searching around I had also landed on the same workaround.
So, I suggest I add an additional ansible task that removes .bash_logout (it's empty on RH-like distros and isn't even created on FreeBSD) and adjust the service file as you suggested in your original patch review, that way, we run the gitlab-runner with the right privileges since the very begining.
Sounds good to me. We can remove it unconditionally no matter the flavor, it's not like it does much anyway since even developers who create VMs locally will access them via ssh. -- Andrea Bolognani / Red Hat / Virtualization
With the recent efforts in upstream libvirt to centralize our CI on gitlab, let's add a new gitlab-specific flavor along with related playbook tasks. This flavour revolves around installing and configuring the gitlab-runner agent binary which requires the per-project registration token to be specified in order for the runner to be successfully registered with the gitlab server. Note that as part of the registration process each runner acquires a new unique access token. This means that we must ensure that the registration is run only on the first update, otherwise a new runner with a new access token is registered with the gitlab project. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- guests/group_vars/all/main.yml | 3 ++ guests/playbooks/update/main.yml | 5 ++ guests/playbooks/update/tasks/gitlab.yml | 64 ++++++++++++++++++++++++ 3 files changed, 72 insertions(+) create mode 100644 guests/playbooks/update/tasks/gitlab.yml diff --git a/guests/group_vars/all/main.yml b/guests/group_vars/all/main.yml index b73795e..9d9a413 100644 --- a/guests/group_vars/all/main.yml +++ b/guests/group_vars/all/main.yml @@ -5,3 +5,6 @@ ansible_ssh_pass: root jenkins_url: https://ci.centos.org/computer/{{ inventory_hostname }}/slave-agent.jnlp + +# In our case, ansible_system is either Linux or FreeBSD +gitlab_runner_url: https://gitlab-runner-downloads.s3.amazonaws.com/latest/binaries/gitlab-runner-{{ ansible_system|lower }}-amd64 diff --git a/guests/playbooks/update/main.yml b/guests/playbooks/update/main.yml index e82055b..9e63391 100644 --- a/guests/playbooks/update/main.yml +++ b/guests/playbooks/update/main.yml @@ -58,3 +58,8 @@ - include: '{{ playbook_base }}/tasks/jenkins.yml' when: - flavor == 'jenkins' + + # Install the Gitlab runner agent + - include: '{{ playbook_base }}/tasks/gitlab.yml' + when: + - flavor == 'gitlab' diff --git a/guests/playbooks/update/tasks/gitlab.yml b/guests/playbooks/update/tasks/gitlab.yml new file mode 100644 index 0000000..9a30140 --- /dev/null +++ b/guests/playbooks/update/tasks/gitlab.yml @@ -0,0 +1,64 @@ +--- +- name: Look up Gitlab runner secret + set_fact: + gitlab_runner_secret: '{{ lookup("file", gitlab_runner_token_file) }}' + gitlab_runner_config_path: '/home/gitlab/.gitlab-runner/config.toml' + +- name: Download gitlab-runner agent + get_url: + url: '{{ gitlab_runner_url }}' + dest: /usr/local/bin/gitlab-runner + mode: '0755' + force: yes + +- name: Make sure the gitlab-runner config dir exists exists + file: + path: '{{ gitlab_runner_config_path | dirname }}' + owner: gitlab + group: gitlab + state: directory + register: rc_gitlab_runner_config_dir + +- name: Create and empty gitlab-runner config + file: + path: '{{ gitlab_runner_config_path }}' + owner: gitlab + group: gitlab + state: touch + when: rc_gitlab_runner_config_dir.changed + +# To ensure idempotency, we must run the registration only when we first +# created the config dir, otherwise we'll register a new runner on every +# update +- name: Register the gitlab-runner agent + shell: 'gitlab-runner register --non-interactive --config /home/gitlab/.gitlab-runner/config.toml --registration-token {{ gitlab_runner_secret }} --url https://gitlab.com --executor shell --tag-list {{ inventory_hostname }}' + when: rc_gitlab_runner_config_dir.changed + +- block: + - name: Install the gitlab-runner service unit + template: + src: '{{ playbook_base }}/templates/gitlab-runner.service.j2' + dest: /etc/systemd/system/gitlab-runner.service + + - name: Enable the gitlab-runner service + systemd: + name: gitlab-runner + state: started + enabled: yes + daemon_reload: yes + when: ansible_service_mgr == 'systemd' + +- block: + - name: Install the gitlab_runner rc service script + template: + src: '{{ playbook_base }}/templates/gitlab-runner.j2' + dest: '/usr/local/etc/rc.d/gitlab_runner' + mode: '0755' + + - name: Enable the gitlab-runner rc service + service: + name: gitlab_runner + state: started + enabled: yes + when: ansible_service_mgr != 'systemd' + -- 2.25.1
On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
+++ b/guests/group_vars/all/main.yml @@ -5,3 +5,6 @@ ansible_ssh_pass: root
jenkins_url: https://ci.centos.org/computer/{{ inventory_hostname }}/slave-agent.jnlp + +# In our case, ansible_system is either Linux or FreeBSD +gitlab_runner_url: https://gitlab-runner-downloads.s3.amazonaws.com/latest/binaries/gitlab-runner-{{ ansible_system|lower }}-amd64
Note that these two URLs are semantically quite different: * jenkins_url is the endpoint to which the Jenkins agent will connect to; * gitlab_runner_url is the location from where the gitlab-runner binary can be downloaded. To keep things consistent, we could have jenkins_url: https://ci.centos.org/computer/{{ inventory_hostname }}/slave-agent.jnlp jenkins_agent_url: https://ci.centos.org/jnlpJars/slave.jar gitlab_url: https://gitlab.com gitlab_runner_url: https://gitlab-runner-downloads.s3.amazonaws.com/latest/binaries/gitlab-runner-{{ ansible_system|lower }}-amd64 although I'm starting to question why these URLs should even be part of the inventory when they could just as easily be hardcoded into the corresponding playbook...
+++ b/guests/playbooks/update/tasks/gitlab.yml @@ -0,0 +1,64 @@ +--- +- name: Look up Gitlab runner secret + set_fact: + gitlab_runner_secret: '{{ lookup("file", gitlab_runner_token_file) }}'
The GitLab token should be extracted from the vault, not from a regular file. [...]
+- name: Make sure the gitlab-runner config dir exists exists + file: + path: '{{ gitlab_runner_config_path | dirname }}' + owner: gitlab + group: gitlab + state: directory + register: rc_gitlab_runner_config_dir + +- name: Create and empty gitlab-runner config + file: + path: '{{ gitlab_runner_config_path }}' + owner: gitlab + group: gitlab + state: touch + when: rc_gitlab_runner_config_dir.changed
Running 'gitlab-runner register' will create the configuration file, so touching it in advance is unnecessary and...
+# To ensure idempotency, we must run the registration only when we first +# created the config dir, otherwise we'll register a new runner on every +# update +- name: Register the gitlab-runner agent + shell: 'gitlab-runner register --non-interactive --config /home/gitlab/.gitlab-runner/config.toml --registration-token {{ gitlab_runner_secret }} --url https://gitlab.com --executor shell --tag-list {{ inventory_hostname }}' + when: rc_gitlab_runner_config_dir.changed
... this check could be replaced with creates: '{{ gitlab_runner_config_path }}' Additional comments on this command: * you have defined gitlab_runner_config_path earlier, might as well use it here; * the 'shell' executor is I think our only option when it comes to FreeBSD (and we should make sure we fully understand the security implications of using it before exposing any of this on the internet), but for Linux we should be able to use the 'docker' executor instead, which should provide additional isolation; * I haven't played around with tags much, but from what I think I have understood we will want to have something more generic here, such as "freebsd" and "linux", so that we can for example tag some of the jobs as "freebsd" and be sure they will run on the expected OS; * I think we'll want to have the Linux runners configured to accept untagged jobs, but the FreeBSD ones only accepting tagged ones (again, if I have understood how job tagging works). [...]
+- block: + - name: Install the gitlab_runner rc service script + template: + src: '{{ playbook_base }}/templates/gitlab-runner.j2' + dest: '/usr/local/etc/rc.d/gitlab_runner' + mode: '0755' + + - name: Enable the gitlab-runner rc service
s/gitlab-runner/gitlab_runner/ though I wonder if we can't just use the same name on both Linux and FreeBSD? On my FreeBSD machine, while the use of underscore in service names is clearly the default, there's at least one service (ftp-proxy) which doesn't follow that convention with (I assume) no ill effect. -- Andrea Bolognani / Red Hat / Virtualization
On Tue, Mar 31, 2020 at 05:23:32PM +0200, Andrea Bolognani wrote:
On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
+++ b/guests/group_vars/all/main.yml @@ -5,3 +5,6 @@ ansible_ssh_pass: root
jenkins_url: https://ci.centos.org/computer/{{ inventory_hostname }}/slave-agent.jnlp + +# In our case, ansible_system is either Linux or FreeBSD +gitlab_runner_url: https://gitlab-runner-downloads.s3.amazonaws.com/latest/binaries/gitlab-runner-{{ ansible_system|lower }}-amd64
Note that these two URLs are semantically quite different:
* jenkins_url is the endpoint to which the Jenkins agent will connect to;
* gitlab_runner_url is the location from where the gitlab-runner binary can be downloaded.
To keep things consistent, we could have
jenkins_url: https://ci.centos.org/computer/{{ inventory_hostname }}/slave-agent.jnlp jenkins_agent_url: https://ci.centos.org/jnlpJars/slave.jar
gitlab_url: https://gitlab.com gitlab_runner_url: https://gitlab-runner-downloads.s3.amazonaws.com/latest/binaries/gitlab-runner-{{ ansible_system|lower }}-amd64
although I'm starting to question why these URLs should even be part of the inventory when they could just as easily be hardcoded into the corresponding playbook...
+++ b/guests/playbooks/update/tasks/gitlab.yml @@ -0,0 +1,64 @@ +--- +- name: Look up Gitlab runner secret + set_fact: + gitlab_runner_secret: '{{ lookup("file", gitlab_runner_token_file) }}'
The GitLab token should be extracted from the vault, not from a regular file.
This was a deliberate decision and I didn't want to do it the jenkins way, let me expand on why. Ultimately, we want other community members to contribute their resources to our gitlab so that the test results are centralized, but what if they want to setup the same machine for their internal infrastructure as well to stay unified with the setup upstream libvirt "mandates". With the secret in the vault, we always create machines that can only be plugged into *our* gitlab, but why should be that restrictive? Give the consumers the leeway to actually have a possibility to plug it into their own instance by supplying the token the same way as we supply the vault and root passwords, so I'd rather not be tying this to a single use case just like we did with jenkins.
[...]
+- name: Make sure the gitlab-runner config dir exists exists + file: + path: '{{ gitlab_runner_config_path | dirname }}' + owner: gitlab + group: gitlab + state: directory + register: rc_gitlab_runner_config_dir + +- name: Create and empty gitlab-runner config + file: + path: '{{ gitlab_runner_config_path }}' + owner: gitlab + group: gitlab + state: touch + when: rc_gitlab_runner_config_dir.changed
Running 'gitlab-runner register' will create the configuration file, so touching it in advance is unnecessary and...
+# To ensure idempotency, we must run the registration only when we first +# created the config dir, otherwise we'll register a new runner on every +# update +- name: Register the gitlab-runner agent + shell: 'gitlab-runner register --non-interactive --config /home/gitlab/.gitlab-runner/config.toml --registration-token {{ gitlab_runner_secret }} --url https://gitlab.com --executor shell --tag-list {{ inventory_hostname }}' + when: rc_gitlab_runner_config_dir.changed
... this check could be replaced with
creates: '{{ gitlab_runner_config_path }}'
Like I said in the comment above, the reason why I created those 2 tasks to create the config file was to ensure idempotency, the registration runs every single time you execute the playbook and guess what, it registers a new runner which you can see in the gitlab runner pool - we don't want that. I'm not sure "creates" actually prevents that behaviour.
Additional comments on this command:
* you have defined gitlab_runner_config_path earlier, might as well use it here;
* the 'shell' executor is I think our only option when it comes to FreeBSD (and we should make sure we fully understand the security implications of using it before exposing any of this on the internet), but for Linux we should be able to use the 'docker' executor instead, which should provide additional isolation;
I get the isolation part, but why exactly is the additional isolation by the container deemed so necessary compared to achieving consistency in terms of maintenance? I mentioned it earlier, these machines run in a secured environment not accessible from the outside. Moreover, these are test machines that are designed to come and go, so if the worst comes, they can be torn down and replaced with new copies, that's the whole point of CI environments, so aren't we trying to be a bit too paranoid?
* I haven't played around with tags much, but from what I think I have understood we will want to have something more generic here, such as "freebsd" and "linux", so that we can for example tag some of the jobs as "freebsd" and be sure they will run on the expected OS;
we can go that way as well...
* I think we'll want to have the Linux runners configured to accept untagged jobs, but the FreeBSD ones only accepting tagged ones (again, if I have understood how job tagging works).
Untagged jobs have a problem - you cannot ensure parallelism, so you may end up with job queued on a single machine, e.g. we tried to play with different job settings with ptoscano and even though there were 3 nodes available and we scheduled 3 jobs, 2 jobs were queued on a single machine, so tagged jobs is probably what we do want, unless we don't care about parallel execution (but we certainly do care about distro coverage).
[...]
+- block: + - name: Install the gitlab_runner rc service script + template: + src: '{{ playbook_base }}/templates/gitlab-runner.j2' + dest: '/usr/local/etc/rc.d/gitlab_runner' + mode: '0755' + + - name: Enable the gitlab-runner rc service
s/gitlab-runner/gitlab_runner/
though I wonder if we can't just use the same name on both Linux and FreeBSD? On my FreeBSD machine, while the use of underscore in service names is clearly the default, there's at least one service (ftp-proxy) which doesn't follow that convention with (I assume) no ill effect.
Have you managed to get it work? I spent like 2 days trying different combinations in the name of the service and I just could not make it work, so I stopped trying to rub it against the grain and simply followed the FreeBSD convention, but if you can make it work, I'm happy to switch, as I really wanted to be 99.9% (maybe a little bit less :) ) consistent. -- Erik Skultety
On Fri, 2020-04-03 at 09:38 +0200, Erik Skultety wrote:
On Tue, Mar 31, 2020 at 05:23:32PM +0200, Andrea Bolognani wrote:
On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
+++ b/guests/playbooks/update/tasks/gitlab.yml @@ -0,0 +1,64 @@ +--- +- name: Look up Gitlab runner secret + set_fact: + gitlab_runner_secret: '{{ lookup("file", gitlab_runner_token_file) }}'
The GitLab token should be extracted from the vault, not from a regular file.
This was a deliberate decision and I didn't want to do it the jenkins way, let me expand on why. Ultimately, we want other community members to contribute their resources to our gitlab so that the test results are centralized, but what if they want to setup the same machine for their internal infrastructure as well to stay unified with the setup upstream libvirt "mandates". With the secret in the vault, we always create machines that can only be plugged into *our* gitlab, but why should be that restrictive? Give the consumers the leeway to actually have a possibility to plug it into their own instance by supplying the token the same way as we supply the vault and root passwords, so I'd rather not be tying this to a single use case just like we did with jenkins.
Thanks for spelling out your intentions, it really helps. I agree that reusing the recipe to build runners that connect to a local / private GitLab instance is a valid use case that we should accomodate. One point that immediately leads to, and which was unintentionally already addressed above, is that in that case you need not only the token to be configurable, but also the connection URL. Other than that, storing the token in ~/.config/lcitool is fine I guess. The number of files in that directory is getting fairly ridiculous, though, so Someone™ should really look into throwing away the current, extremely crude configuration system that's been inherited from the original shell implementation and replace it with something more sensible like a config.toml.
+# To ensure idempotency, we must run the registration only when we first +# created the config dir, otherwise we'll register a new runner on every +# update +- name: Register the gitlab-runner agent + shell: 'gitlab-runner register --non-interactive --config /home/gitlab/.gitlab-runner/config.toml --registration-token {{ gitlab_runner_secret }} --url https://gitlab.com --executor shell --tag-list {{ inventory_hostname }}' + when: rc_gitlab_runner_config_dir.changed
... this check could be replaced with
creates: '{{ gitlab_runner_config_path }}'
Like I said in the comment above, the reason why I created those 2 tasks to create the config file was to ensure idempotency, the registration runs every single time you execute the playbook and guess what, it registers a new runner which you can see in the gitlab runner pool - we don't want that. I'm not sure "creates" actually prevents that behaviour.
That's *exactly* what it does: creates A filename, when it already exists, this step (path) will *not* be run. https://docs.ansible.com/ansible/latest/modules/shell_module.html#parameter-...
* the 'shell' executor is I think our only option when it comes to FreeBSD (and we should make sure we fully understand the security implications of using it before exposing any of this on the internet), but for Linux we should be able to use the 'docker' executor instead, which should provide additional isolation;
I get the isolation part, but why exactly is the additional isolation by the container deemed so necessary compared to achieving consistency in terms of maintenance? I mentioned it earlier, these machines run in a secured environment not accessible from the outside. Moreover, these are test machines that are designed to come and go, so if the worst comes, they can be torn down and replaced with new copies, that's the whole point of CI environments, so aren't we trying to be a bit too paranoid?
Consistency is a worthy goal, but it's not the only one. Security is another very important one. Anyway, it's simpler than that: all of our Linux jobs currently expect to run in container images, and we can only have that using the Docker executor. We could, I guess, change our configuration so that, instead of having each job use a specific container image, they would be tagged with a specific label that would ensure they are scheduled on the expected target OS, eg. instead of dco: script: - ... image: quay.io/libvirt/buildenv-libvirt-fedora-31:latest we could have dco: script: - ... tags: - fedora-31 However, due to the restrictions in how GitLab will schedule jobs for forks, that configuration would only run for libvirt/libvirt branches and would not run for forks and merge requests, the latter of which would basically make it pointless. What I think we should have instead, in addition to the FreeBSD runner(s) that we need to match Jenkins' coverage, is Linux runner(s) using the Docker executor that are tagged appropriately and can speed up post-merge and periodic build tasks by complementing the shared runners.
* I think we'll want to have the Linux runners configured to accept untagged jobs, but the FreeBSD ones only accepting tagged ones (again, if I have understood how job tagging works).
Untagged jobs have a problem - you cannot ensure parallelism, so you may end up with job queued on a single machine, e.g. we tried to play with different job settings with ptoscano and even though there were 3 nodes available and we scheduled 3 jobs, 2 jobs were queued on a single machine, so tagged jobs is probably what we do want, unless we don't care about parallel execution (but we certainly do care about distro coverage).
Were all those runners configured to accept untagged jobs? The default is not to accept them.
[...]
+- block: + - name: Install the gitlab_runner rc service script + template: + src: '{{ playbook_base }}/templates/gitlab-runner.j2' + dest: '/usr/local/etc/rc.d/gitlab_runner' + mode: '0755' + + - name: Enable the gitlab-runner rc service
s/gitlab-runner/gitlab_runner/
though I wonder if we can't just use the same name on both Linux and FreeBSD? On my FreeBSD machine, while the use of underscore in service names is clearly the default, there's at least one service (ftp-proxy) which doesn't follow that convention with (I assume) no ill effect.
Have you managed to get it work? I spent like 2 days trying different combinations in the name of the service and I just could not make it work, so I stopped trying to rub it against the grain and simply followed the FreeBSD convention, but if you can make it work, I'm happy to switch, as I really wanted to be 99.9% (maybe a little bit less :) ) consistent.
I haven't even tried O:-) If you've already spent that much time on it without success, let's forget it for the time being. We can always go back and change it at a later time if we find a way to make it work with the prettier name :) -- Andrea Bolognani / Red Hat / Virtualization
On Fri, Apr 03, 2020 at 05:08:59PM +0200, Andrea Bolognani wrote:
On Fri, 2020-04-03 at 09:38 +0200, Erik Skultety wrote:
On Tue, Mar 31, 2020 at 05:23:32PM +0200, Andrea Bolognani wrote:
On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
+++ b/guests/playbooks/update/tasks/gitlab.yml @@ -0,0 +1,64 @@ +--- +- name: Look up Gitlab runner secret + set_fact: + gitlab_runner_secret: '{{ lookup("file", gitlab_runner_token_file) }}'
The GitLab token should be extracted from the vault, not from a regular file.
This was a deliberate decision and I didn't want to do it the jenkins way, let me expand on why. Ultimately, we want other community members to contribute their resources to our gitlab so that the test results are centralized, but what if they want to setup the same machine for their internal infrastructure as well to stay unified with the setup upstream libvirt "mandates". With the secret in the vault, we always create machines that can only be plugged into *our* gitlab, but why should be that restrictive? Give the consumers the leeway to actually have a possibility to plug it into their own instance by supplying the token the same way as we supply the vault and root passwords, so I'd rather not be tying this to a single use case just like we did with jenkins.
Thanks for spelling out your intentions, it really helps.
I agree that reusing the recipe to build runners that connect to a local / private GitLab instance is a valid use case that we should accomodate.
One point that immediately leads to, and which was unintentionally already addressed above, is that in that case you need not only the token to be configurable, but also the connection URL.
Doh! How could I have forgot that.
Other than that, storing the token in ~/.config/lcitool is fine I guess. The number of files in that directory is getting fairly ridiculous, though, so Someone™ should really look into throwing away the current, extremely crude configuration system that's been inherited from the original shell implementation and replace it with something more sensible like a config.toml.
Let me spend some time on that improvement :). Erik
+# To ensure idempotency, we must run the registration only when we first +# created the config dir, otherwise we'll register a new runner on every +# update +- name: Register the gitlab-runner agent + shell: 'gitlab-runner register --non-interactive --config /home/gitlab/.gitlab-runner/config.toml --registration-token {{ gitlab_runner_secret }} --url https://gitlab.com --executor shell --tag-list {{ inventory_hostname }}' + when: rc_gitlab_runner_config_dir.changed
... this check could be replaced with
creates: '{{ gitlab_runner_config_path }}'
Like I said in the comment above, the reason why I created those 2 tasks to create the config file was to ensure idempotency, the registration runs every single time you execute the playbook and guess what, it registers a new runner which you can see in the gitlab runner pool - we don't want that. I'm not sure "creates" actually prevents that behaviour.
That's *exactly* what it does:
creates A filename, when it already exists, this step (path) will *not* be run.
Ok, I will incorporate this.
https://docs.ansible.com/ansible/latest/modules/shell_module.html#parameter-...
* the 'shell' executor is I think our only option when it comes to FreeBSD (and we should make sure we fully understand the security implications of using it before exposing any of this on the internet), but for Linux we should be able to use the 'docker' executor instead, which should provide additional isolation;
I get the isolation part, but why exactly is the additional isolation by the container deemed so necessary compared to achieving consistency in terms of maintenance? I mentioned it earlier, these machines run in a secured environment not accessible from the outside. Moreover, these are test machines that are designed to come and go, so if the worst comes, they can be torn down and replaced with new copies, that's the whole point of CI environments, so aren't we trying to be a bit too paranoid?
Consistency is a worthy goal, but it's not the only one. Security is another very important one.
Anyway, it's simpler than that: all of our Linux jobs currently expect to run in container images, and we can only have that using the Docker executor.
With the functional tests in mind, the Docker executor doesn't really solve anything, for builds, there are already a bunch of containers taking care of that, now we need to cover cases where container technology cannot be utilized as well as environments for functional tests - of course, for that, these patches are only sort of a prequel. -- Erik Skultety
On Mon, 2020-04-06 at 13:28 +0200, Erik Skultety wrote:
On Fri, Apr 03, 2020 at 05:08:59PM +0200, Andrea Bolognani wrote:
Other than that, storing the token in ~/.config/lcitool is fine I guess. The number of files in that directory is getting fairly ridiculous, though, so Someone™ should really look into throwing away the current, extremely crude configuration system that's been inherited from the original shell implementation and replace it with something more sensible like a config.toml.
Let me spend some time on that improvement :).
It was not my intention to volunteer you for this effort, but okay, I'll take it :)
Anyway, it's simpler than that: all of our Linux jobs currently expect to run in container images, and we can only have that using the Docker executor.
With the functional tests in mind, the Docker executor doesn't really solve anything, for builds, there are already a bunch of containers taking care of that, now we need to cover cases where container technology cannot be utilized as well as environments for functional tests - of course, for that, these patches are only sort of a prequel.
Okay, so the target for the VMs built this way is * functional tests, for all of them; * unit tests, for the FreeBSD ones. Is that correct? Because I was under the impression that, aside of filling the FreeBSD-sized gap in our GitLab CI coverage, we were aiming at increasing the capacity of the existing container-based unit tests beyond what the shared runners provide. If that's not what we're aiming for, then of course using the Docker executor makes little sense and would probably only get in the way. -- Andrea Bolognani / Red Hat / Virtualization
Unlike with the 'test' flavour, where the 'test' user has sudo permissions on the system, with machines set up with the 'gitlab' flavour which are intended to contact the outside world which, we don't want that. More importantly though, we must not use the default root password which is set by the install script on such machines. Therefore, set the root password to a random one as part of the gitlab flavour task, thus only allowing SSH pubkey authentication for the root account. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- guests/playbooks/update/tasks/gitlab.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/guests/playbooks/update/tasks/gitlab.yml b/guests/playbooks/update/tasks/gitlab.yml index 9a30140..db27966 100644 --- a/guests/playbooks/update/tasks/gitlab.yml +++ b/guests/playbooks/update/tasks/gitlab.yml @@ -62,3 +62,7 @@ enabled: yes when: ansible_service_mgr != 'systemd' +- name: Set random root password for security reasons + user: + name: root + password: '{{ lookup("password","/dev/null encrypt=sha512_crypt") }}' -- 2.25.1
On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
Unlike with the 'test' flavour, where the 'test' user has sudo permissions on the system, with machines set up with the 'gitlab' flavour which are intended to contact the outside world which, we don't want that. More importantly though, we must not use the default root password which is set by the install script on such machines. Therefore, set the root password to a random one as part of the gitlab flavour task, thus only allowing SSH pubkey authentication for the root account.
I'm confused by this. If we want the root account to only be accessible via SSH with a pubkey, then we can configure sshd accordingly: setting a random password which is not stored anywhere prevents access not only via SSH, but also via local access (eg. serial console), which I don't think is desirable. Moreover, the root password that is set in the first place is taken from a mandatory user-provided configuration file, and I'm not sure we should be condescending towards users by basically saying "we know you didn't choose a secure password, so we're going to generate a new one ourselves". What am I missing? -- Andrea Bolognani / Red Hat / Virtualization
On Tue, Mar 31, 2020 at 05:39:45PM +0200, Andrea Bolognani wrote:
On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
Unlike with the 'test' flavour, where the 'test' user has sudo permissions on the system, with machines set up with the 'gitlab' flavour which are intended to contact the outside world which, we don't want that. More importantly though, we must not use the default root password which is set by the install script on such machines. Therefore, set the root password to a random one as part of the gitlab flavour task, thus only allowing SSH pubkey authentication for the root account.
I'm confused by this.
If we want the root account to only be accessible via SSH with a pubkey, then we can configure sshd accordingly: setting a random password which is not stored anywhere prevents access not only via SSH, but also via local access (eg. serial console), which I don't think is desirable.
I answered this in one of the former patches, so I don't want to repeat it here too.
Moreover, the root password that is set in the first place is taken from a mandatory user-provided configuration file, and I'm not sure we should be condescending towards users by basically saying "we know you didn't choose a secure password, so we're going to generate a new one ourselves".
Like I said, with these machines, we need to design them in a way where they can come and go easily. Once you accept that, you don't care about the root password as long as you have SSH access via a secure manner (at least I never cared with the machines I created with virt-builder, or provisioned in beaker). For personal machines, yes, this is inconvenient, but the sole purpose of these executors is to live somewhere in the cloud and do 1 job and 1 job only. I'm planning on proceeding with creating a cloud config for OpenStack for these machines which is another explanation for the password - for cloud machines, the root password will always be set by the cloud init script and that one can either be static, or random (and I have a hunch that the latter is actually true in production environments where other mechanism are put in use to be able to get root access, like SSH or a service account with sudo perms). -- Erik Skultety
On Fri, 2020-04-03 at 09:43 +0200, Erik Skultety wrote:
On Tue, Mar 31, 2020 at 05:39:45PM +0200, Andrea Bolognani wrote:
On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
Unlike with the 'test' flavour, where the 'test' user has sudo permissions on the system, with machines set up with the 'gitlab' flavour which are intended to contact the outside world which, we don't want that. More importantly though, we must not use the default root password which is set by the install script on such machines. Therefore, set the root password to a random one as part of the gitlab flavour task, thus only allowing SSH pubkey authentication for the root account.
I'm confused by this.
If we want the root account to only be accessible via SSH with a pubkey, then we can configure sshd accordingly: setting a random password which is not stored anywhere prevents access not only via SSH, but also via local access (eg. serial console), which I don't think is desirable.
Turns out we already do this: # guests/playbooks/update/tasks/users.yml - name: 'root: Disable ssh password authentication' lineinfile: path: /etc/ssh/sshd_config regexp: '^#*\s*PermitRootLogin\s*.*$' line: 'PermitRootLogin without-password' state: present So it's already the case that the only way you can ssh into the builders as root is by using pubkey authentication.
Moreover, the root password that is set in the first place is taken from a mandatory user-provided configuration file, and I'm not sure we should be condescending towards users by basically saying "we know you didn't choose a secure password, so we're going to generate a new one ourselves".
Like I said, with these machines, we need to design them in a way where they can come and go easily. Once you accept that, you don't care about the root password as long as you have SSH access via a secure manner (at least I never cared with the machines I created with virt-builder, or provisioned in beaker). For personal machines, yes, this is inconvenient, but the sole purpose of these executors is to live somewhere in the cloud and do 1 job and 1 job only. I'm planning on proceeding with creating a cloud config for OpenStack for these machines which is another explanation for the password - for cloud machines, the root password will always be set by the cloud init script and that one can either be static, or random (and I have a hunch that the latter is actually true in production environments where other mechanism are put in use to be able to get root access, like SSH or a service account with sudo perms).
Sorry, I still don't get it. If the password doesn't matter because you're going to set a new one using cloud-init, why bother replacing the one that was set earlier? I think we are conflating different scenarios: * if you want to build your runner locally using 'lcitool install' and then connect it to whatever GitLab instance, then the root password that was configured by the user should be preserved, because it's an explicit setting from the user; * if you want to build qcow2 images for distribution, such that other users can later spin them up on their cloud of choice, then the root password must be set at instantiation time using cloud-init or other similar mechanisms. In neither scenario generating a random password at install time is necessary, or helpful. For the latter one, in particular, you'd need to ensure cloud-init is present and enabled in the image, which is currently not the case, so it will simply not work. You'll also want to strip out the ssh key of the user who generated the image from authorized_keys, but I assume something like virt-sysprep is going to be one of the steps of the image generation pipeline and will take care of that. -- Andrea Bolognani / Red Hat / Virtualization
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- guests/lcitool | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/guests/lcitool b/guests/lcitool index 209380a..dfb1010 100755 --- a/guests/lcitool +++ b/guests/lcitool @@ -175,7 +175,7 @@ class Config: ) ) - if flavor not in ["test", "jenkins"]: + if flavor not in ["test", "jenkins", "gitlab"]: raise Exception("Invalid flavor '{}'".format(flavor)) return flavor @@ -185,7 +185,7 @@ class Config: # The vault password is only needed for the jenkins flavor, but in # that case we want to make sure there's *something* in there - if self.get_flavor() != "test": + if self.get_flavor() == "jenkins": vault_pass_file = self._get_config_file("vault-password") try: @@ -217,6 +217,21 @@ class Config: return root_pass_file + def get_gitlab_runner_token_file(self): + gitlab_runner_token_file = self._get_config_file("gitlab-runner-token") + + try: + with open(gitlab_runner_token_file, "r") as infile: + if not infile.readline().strip(): + raise ValueError + except Exception as ex: + raise Exception( + "Missing or invalid gitlab runner token file ({}): {}".format( + gitlab_runner_token_file, ex + ) + ) + + return gitlab_runner_token_file class Inventory: @@ -449,6 +464,7 @@ class Application: flavor = self._config.get_flavor() vault_pass_file = self._config.get_vault_password_file() root_pass_file = self._config.get_root_password_file() + gitlab_runner_token_file = self._config.get_gitlab_runner_token_file() ansible_hosts = ",".join(self._inventory.expand_pattern(hosts)) selected_projects = self._projects.expand_pattern(projects) @@ -469,7 +485,7 @@ class Application: playbook_base = os.path.join(base, "playbooks", playbook) playbook_path = os.path.join(playbook_base, "main.yml") - extra_vars = json.dumps({ + extra_vars_d = { "base": base, "playbook_base": playbook_base, "root_password_file": root_pass_file, @@ -477,7 +493,10 @@ class Application: "selected_projects": selected_projects, "git_remote": git_remote, "git_branch": git_branch, - }) + } + + if flavor == "gitlab": + extra_vars_d["gitlab_runner_token_file"] = gitlab_runner_token_file ansible_playbook = distutils.spawn.find_executable("ansible-playbook") if ansible_playbook is None: @@ -486,7 +505,7 @@ class Application: cmd = [ ansible_playbook, "--limit", ansible_hosts, - "--extra-vars", extra_vars, + "--extra-vars", json.dumps(extra_vars_d), ] # Provide the vault password if available -- 2.25.1
On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
+++ b/guests/lcitool + def get_gitlab_runner_token_file(self): + gitlab_runner_token_file = self._get_config_file("gitlab-runner-token") + + try: + with open(gitlab_runner_token_file, "r") as infile: + if not infile.readline().strip(): + raise ValueError + except Exception as ex: + raise Exception( + "Missing or invalid gitlab runner token file ({}): {}".format( + gitlab_runner_token_file, ex + ) + ) + + return gitlab_runner_token_file
As per my comments to patch 3/5, we want the token to be stored in the vault instead that on the disk, which will make most of this commit not necessary. -- Andrea Bolognani / Red Hat / Virtualization
On Tue, Mar 31, 2020 at 05:43:37PM +0200, Andrea Bolognani wrote:
On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
+++ b/guests/lcitool + def get_gitlab_runner_token_file(self): + gitlab_runner_token_file = self._get_config_file("gitlab-runner-token") + + try: + with open(gitlab_runner_token_file, "r") as infile: + if not infile.readline().strip(): + raise ValueError + except Exception as ex: + raise Exception( + "Missing or invalid gitlab runner token file ({}): {}".format( + gitlab_runner_token_file, ex + ) + ) + + return gitlab_runner_token_file
As per my comments to patch 3/5, we want the token to be stored in the vault instead that on the disk, which will make most of this commit not necessary.
I already responded to this one, TL;DR I don't think we want to tie it to libvirt's git only, what if someone has their own testing gitlab instance deployed and they want to deploy the machine there first and only then submit the runner to libvirt gitlab CI? -- Erik Skultety
participants (2)
-
Andrea Bolognani -
Erik Skultety