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

feat: add new authentication method for GitLab >= 16 #876

Merged
merged 27 commits into from
Nov 3, 2023

Conversation

Kadeux
Copy link
Contributor

@Kadeux Kadeux commented Jun 9, 2023

Description

GitLab released a new authentication architecture for their runners since the version 16.0.0. This MR handle this new architecture while maintaining backward compatibility.

Migrations required

Highly recommended as the old GitLab Runner registration method will be removed with GitLab 17.

Migration steps:

  1. create a group access token for your GitLab group (needs the owner role and the api scope). To be refreshed manually once a year.
  2. store the token in a SSM parameter and set the variable runner_gitlab.access_token_secure_parameter_store_name to this SSM parameter
  3. remove runner_gitlab_registration_config.registration_token. No longer needed.
  4. add type = project or group, group_id = (for group runners) orproject_id = (for project runners) to therunner_gitlab_registration_config` section.

Verification

  • running the old authentication method on GitLab 16: success
  • running the new authentication method on GitLab 16: success

Kadeux and others added 4 commits June 7, 2023 19:09
Signed-off-by: François Bibron <francois.bibron@polyconseil.fr>
Signed-off-by: François Bibron <francois.bibron@polyconseil.fr>
Signed-off-by: François Bibron <francois.bibron@polyconseil.fr>
@github-actions
Copy link
Contributor

github-actions bot commented Jun 9, 2023

Hey @Kadeux! 👋

Thank you for your contribution to the project. Please refer to the contribution rules for a quick overview of the process.

Make sure that this PR clearly explains:

  • the problem being solved
  • the best way a reviewer and you can test your changes

With submitting this PR you confirm that you hold the rights of the code added and agree that it will published under this LICENSE.

The following ChatOps commands are supported:

  • /help: notifies a maintainer to help you out

Simply add a comment with the command in the first line. If you need to pass more information, separate it with a blank line from the command.

This message was generated automatically. You are welcome to improve it.

@kayman-mk kayman-mk changed the title Handling new authentication method for GitLab >= 16 feat: add new authentication method for GitLab >= 16 Jun 10, 2023
Copy link
Collaborator

@kayman-mk kayman-mk left a comment

Choose a reason for hiding this comment

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

Thanks for solving this issue, @Kadeux

I have some questions and suggestions which came into my mind. Especially because everyone needs to adjust his config at the moment.

Could you please also update the default example and describe how to configure the new authentication method?

variables.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
template/gitlab-runner.tftpl Outdated Show resolved Hide resolved
variables.tf Outdated Show resolved Hide resolved
template/gitlab-runner.tftpl Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
Signed-off-by: François Bibron <francois.bibron@polyconseil.fr>
Signed-off-by: François Bibron <francois.bibron@polyconseil.fr>
@Kadeux Kadeux requested a review from kayman-mk June 15, 2023 14:26
Kadeux and others added 3 commits July 13, 2023 17:00
Signed-off-by: François Bibron <francois.bibron@polyconseil.fr>
@kayman-mk
Copy link
Collaborator

kayman-mk commented Aug 31, 2023

Linter still complains about an unused variable use_new_runner_authentication_gitlab_16. Is it still work in progress?

Kadeux and others added 3 commits August 31, 2023 21:00
@Kadeux
Copy link
Contributor Author

Kadeux commented Aug 31, 2023

Linter still complains about an unused variable use_new_runner_authentication_gitlab_16. Is it still work in progress?

Sorry I didn't see it, I still don't know why did I put this variable here :)

Signed-off-by: François Bibron <francois.bibron@polyconseil.fr>
Signed-off-by: François Bibron <francois.bibron@polyconseil.fr>
@Kadeux Kadeux reopened this Sep 19, 2023
Signed-off-by: François Bibron <francois.bibron@polyconseil.fr>
Signed-off-by: François Bibron <francois.bibron@polyconseil.fr>
@Tiduster
Copy link

@kayman-mk : Any additional remarks on this change?

Do not hesitate to ask for more changes if required.

Best regards,

@kayman-mk
Copy link
Collaborator

Environment for testing:
GitLab 16.0.8
GitLab Runner 16.0.2

Upgraded my runner to this version and verify that jobs are still processed without changing my configuration: success

@kayman-mk
Copy link
Collaborator

kayman-mk commented Sep 30, 2023

Tried to use the new authentication. But it didn't work.

  runner_gitlab_access_token_secure_parameter_store_name = aws_ssm_parameter.gitlab_runner_registration_token_new.name

  runner_gitlab_registration_config = {
    # registration_token = var.gitlab_runner_registration_token.value
    tag_list           = "test"
    description        = "runner for testing new version (${each.value.availability_zone})"
    locked_to_project  = "false"
    run_untagged       = "false"
    maximum_timeout    = "10800"

    type = "group"
    group_id = 123
  }

It still uses the old registration method. The new glrt- registration token wasn't used.

EDIT: I used the runner before and there was a runner token present, which is handled by the module internally. Thus we have to remove this token in order to upgrade to the new authentication method. ❗

EDIT: type = "group" is wrong. Use group_type instead. List/Validate the correct types. Script should throw an error in case the type is wrong. ❗

EDIT: fetching the token does not work. GitLab returns 401 not authorized.

EDIT: It seems that we need a personal access token now to access the user/runners API. Thus runner_gitlab_access_token_secure_parameter_store_name is not the glrt- token for the runner and there is no need to pre-register a runner manually on the UI. The registration is done by the script here. This seems to be a problem as this token becomes invalid in case the user account is disabled. Seems to be better to pre-register the runner as it is a one time effort only.

@kayman-mk
Copy link
Collaborator

Regarding the registration process of the runners: The module needs a PAT to access GitLab and register the runner.

I think in an enterprise environment we shouldn't give a PAT to our module. I suggest that users register the runner manually and we use the glrt- token then. What do you think @Kadeux @npalm @Tiduster

@kayman-mk
Copy link
Collaborator

Seems that my question is obsolete as there are access tokens on group level available. https://about.gitlab.com/blog/2023/07/06/how-to-automate-creation-of-runners/

variables.tf Outdated Show resolved Hide resolved
variables.tf Show resolved Hide resolved
Copy link
Collaborator

@kayman-mk kayman-mk left a comment

Choose a reason for hiding this comment

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

Nice feature you implemented! Appreciated to be able to use the module with GitLab 17+

README.md Outdated Show resolved Hide resolved
template/gitlab-runner.tftpl Outdated Show resolved Hide resolved
template/gitlab-runner.tftpl Outdated Show resolved Hide resolved
template/gitlab-runner.tftpl Outdated Show resolved Hide resolved
template/gitlab-runner.tftpl Outdated Show resolved Hide resolved
variables.tf Outdated Show resolved Hide resolved
@kayman-mk
Copy link
Collaborator

Looks good. Needs a final test on my side.

@kayman-mk kayman-mk self-requested a review October 26, 2023 11:21
Kadeux and others added 5 commits October 31, 2023 15:51
Signed-off-by: François Bibron <francois.bibron@polyconseil.fr>
Signed-off-by: François Bibron <francois.bibron@polyconseil.fr>
…up`. It doesn't work with `optional(string, "")` as the variable is always filled.
…the correct `if` to avoid unnecessary errors in the logs
@kayman-mk
Copy link
Collaborator

Successfully tested the new authentication schema. Had some problems as I added the new variables into the wrong blocks and got no error. Guess this is one of the downsides using maps as variables.

…the empty string to make sure that the user set something
@kayman-mk
Copy link
Collaborator

Tested both scenarios with success. @Kadeux Could you please have a look at my (minor) changes, please?

Copy link
Collaborator

@kayman-mk kayman-mk left a comment

Choose a reason for hiding this comment

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

Great! Let's go with the new authentication method. Thanks for your work.

EDIT: Waiting for feedback from @Kadeux I am good to go.

@Kadeux
Copy link
Contributor Author

Kadeux commented Nov 2, 2023

Great! Let's go with the new authentication method. Thanks for your work.

EDIT: Waiting for feedback from @Kadeux I am good to go.

Looks good to me ! Thanks.

@kayman-mk kayman-mk merged commit c870745 into cattle-ops:main Nov 3, 2023
17 checks passed
kayman-mk pushed a commit that referenced this pull request Nov 7, 2023
🤖 I have created a release *beep* *boop*
---


##
[7.2.0](7.1.1...7.2.0)
(2023-11-07)


### Features

* add new authentication method for GitLab &gt;= 16
([#876](#876))
([c870745](c870745))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Signed-off-by: Niek Palm <dev.npalm@gmail.com>
Co-authored-by: cattle-ops-releaser-2[bot] <134548870+cattle-ops-releaser-2[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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.

None yet

3 participants