On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote: 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

Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- .../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 Tue, Mar 31, 2020 at 04:42:10PM +0200, Andrea Bolognani wrote: Since it's not 1:1, I'll put it into the commit message. True, it can be hardcoded. 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

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(a)redhat.com&gt; --- 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-r... 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 Tue, Mar 31, 2020 at 05:23:32PM +0200, Andrea Bolognani wrote: 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. 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. 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? we can go that way as well... 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). 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, Apr 03, 2020 at 05:08:59PM +0200, Andrea Bolognani wrote: Doh! How could I have forgot that. Let me spend some time on that improvement :). Erik Ok, I will incorporate this. 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

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(a)redhat.com&gt; --- 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

Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- 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