Implement resource gitlab_project_runner_enablement to attach specific runners to projects

[Dutchy-]

Description

This new resource gitlab_project_runner attaches a specific runner to a project.

This modification has been in use by us for years and today I updated it to apply to the latest master. I would like to know if this has the potential to be merged.

PR Checklist Acknowledgement

• I acknowledge that all of the following items are true, where applicable:
  • Resource attributes match 1:1 the names and structure of the API resource in the GitLab API documentation.
  • Examples are updated with:
    • A *.tf file for the resource/s with at least one usage example
    • A *.sh file for the resource/s with an import example (if applicable)
  • New resources have at minimum a basic test with three steps:
    • Create the resource
    • Update the attributes
    • Import the resource
  • No new //lintignore comments were copied from existing code. (Linter rules are meant to be enforced on new code.)
• Address feedback
• make reviewable

I have read the above checklist and I will add the missing parts if the resource in the current form is deemed good enough.

[github-actions]

Welcome @Dutchy- 👋

It looks like this is your first submission to the Terraform GitLab Provider! If you haven’t already done so, please make sure you have checked out our CONTRIBUTING.md guide to make sure your contribution has all the necessary elements in place for a successful approval.

Thanks again, and welcome to the community! 😃

[PatrickRice-KSC]

Hey @Dutchy- ! 👋 thanks for opening your first PR; Please feel free to check out our CONTRIBUTING.md for tips on helping develop the provider, and let me know if you have any questions!

I'm imagining that the workflow for this resource looks like:

• Register a new GitLab runner (not available through TF, but I'm actually working on a PR for that right now) and link it either to an instance or to a group
• Instead of using the group-level runner to share the runner explicitly to all child projects, use this gitlab_project_runner resource to explicitly link the runner to individual projects

Do I have the use-case correct? It makes sense to merge this as a potential new resource, I could certainly see managing project:runner linkage as a very valuable job for terraform where you want to control runners at a more granular level than groups.

A couple general notes on the PR:

• We will want to see a basic acceptance test written, there is some info on that in the CONTRIBUTING.md page linked above and you can look at something like Group Access Token Test file as an example
• You'll want to make sure you run make reviewable to generate your documentation and run linting 🙂 .
• Is this resource impacted by the instance runner -> project runner deprecation? https://docs.gitlab.com/ee/update/deprecations.html#converting-an-instance-shared-runner-to-a-project-specific-runner

I've also left a couple comments in your file as well with some targeted suggestions.

Thanks for the hard work!

[Dutchy-]

Amazing feedback guys, I'll have a look at it.

[Dutchy-]

Thanks guys, I've pushed a new version. I've also included a context and diagnostics on the errors.

Instead of using the group-level runner to share the runner explicitly to all child projects, use this gitlab_project_runner resource to explicitly link the runner to individual projects

Yes, this is indeed the use case

Is this resource impacted by the instance runner -> project runner deprecation? https://docs.gitlab.com/ee/update/deprecations.html#converting-an-instance-shared-runner-to-a-project-specific-runner

I would say no because this is already intended to be used with specific runners. However, I don't understand the full effect of the deprecation.
Looks like it might be affected, at least for tests. See my newer comment.

I'm not yet able to write tests because I am not yet able to start the test environment

$ make testacc-up
docker-compose up -d gitlab-ce
Creating network "terraform-provider-gitlab_default" with the default driver
Creating volume "terraform-provider-gitlab_config-ce" with default driver
Creating volume "terraform-provider-gitlab_logs-ce" with default driver
Creating volume "terraform-provider-gitlab_data-ce" with default driver
Creating volume "terraform-provider-gitlab_config-ee" with default driver
Creating volume "terraform-provider-gitlab_logs-ee" with default driver
Creating volume "terraform-provider-gitlab_data-ee" with default driver
Pulling gitlab-ce (gitlab/gitlab-ce:)...
latest: Pulling from gitlab/gitlab-ce
4d32b49e2995: Pull complete
a25ec19848b1: Pull complete
7741690a7b2c: Pull complete
6218cf34dfa7: Pull complete
6d4ffb564776: Pull complete
83a0bf8d83e9: Pull complete
a5c4bdd9e52f: Pull complete
fb8f3c333b7d: Pull complete
Digest: sha256:b987cc1bbff2d2b70ab5dbac272be47c1df86c745451ce7b03678139101ddbe9
Status: Downloaded newer image for gitlab/gitlab-ce:latest
Creating terraform-provider-gitlab_gitlab-ce_1 ... done
./scripts/await-healthy.sh
Waiting for GitLab container to become healthy............................................................................................................................................

I see this and it hangs forever on the health check. I also can't connect to localhost:8080.

1. Is there a troubleshooting guide for the test environment?
2. Does the test environment include a (specific) runner?

[floh96]

@Dutchy- maybe you are facing #912?
You could try using Gitpod to develop the provider.

[Dutchy-]

Thanks @floh96 I've managed to get it running using gitpod 👍 .

Since the test environment indeed does not include a runner, would it make sense to wait for your new resource to register a runner @PatrickRice-KSC ?

I'll have a look at registering a runner for the tests through the API directly, but I'm not sure yet how feasible that is.

[Dutchy-]

I'm currently a bit confused on how to approach this, especially in light of the announced deprecation https://docs.gitlab.com/ee/update/deprecations.html#converting-an-instance-shared-runner-to-a-project-specific-runner

The current API seems to only support instance level registrations with a registration token I can predict or get programmatically. This can be set using the GITLAB_SHARED_RUNNERS_REGISTRATION_TOKEN environment variable. For project or group level runners I don't know how to get this token for a test case.

Setting up the shared instance runner through that environment variable seems to be the functionality that will be deprecated in 15.0 so if there's a better way I would like to know. What's also unclear to me is if these project or group level runners can be shared by multiple projects, e.g. across the group boundary - that's how we are currently using it in our self-hosted setup.

I think I can conclude that this is indeed affected by the mentioned deprecation.

@PatrickRice-KSC we're in almost opposite timezones, but if you are available on a chatplatform somewhere I'd love to have a chat about this.

[PatrickRice-KSC]

Amazing feedback guys, I'll have a look at it.

No problem, glad we could help!

we're in almost opposite timezones, but if you are available on a chatplatform somewhere I'd love to have a chat about this.

Thanks @Dutchy- ! I'll take another review through this here in a couple hours 🙂 . I'm on the Discord Server that's listed in the contributing.md, and happy to chat there! If you drop me a note there we can figure out a time to chat.

I'm currently a bit confused on how to approach this, especially in light of the announced deprecation

I think for the current register runner API (I think, I haven't tested yet since I'm waiting on #1007 to merge before I pick up work on it again) that it will register runners based on the registration token you provide it. If you pass in a registration token for the instance, you'll get an instance runner and be subject to the deprecation. If you register using a registration token for a group, you'd get a group runner and you wouldn't be subject to the deprecation. So that means we would likely want to have a note on this resource in the provider stating that it should only be used with group_runner setups. Essentially, you may want a top level group (or child of that TLG) that holds all your runners, then you can link from there to individual projects.

Since the test environment indeed does not include a runner, would it make sense to wait for your new resource to register a runner

We've been trying to keep acceptance tests to just the resource being tested, and using the go-gitlab client to register dependent resources. That way when tests fail, it's more clear what resource is actually causing the failure as opposed to just failing by proxy. You can see an example of this here: https://github.com/gitlabhq/terraform-provider-gitlab/blob/main/internal/provider/resource_gitlab_group_project_file_template_test.go#L15

I'd suggest you follow a similar pattern here and create the group, runner, and project in the header of the test, then test your resource individually. That way you're not blocked by my PR as well 🙂

Hopefully that helps!

[Shocktrooper]

@PatrickRice-KSC Since a runner/runner container is not included in the setup scripts from what I can tell, should that be added to accommodate this resource + any future resources/data sources?

[PatrickRice-KSC]

Since a runner/runner container is not included in the setup scripts from what I can tell, should that be added to accommodate this resource + any future resources/data sources?

@Shocktrooper - Do you think we need one? We don't need to actually run any jobs, we just need the ability to register the resource and bind it to a project. I don't think that requires a runner per se, but maybe I'm missing something 🙂

[Shocktrooper]

Per a conversation earlier with @PatrickRice-KSC we do not need a runner registered to test this resource as it does not do any registering. Another note is that the resource itself should probably be renamed to something like gitlab_runner_project_link or something like that as this resource only enables and disables runners for specified projects that have already been registered previously.

[Dutchy-]

Another note is that the resource itself should probably be renamed to something like gitlab_runner_project_link or something like that as this resource only enables and disables runners for specified projects that have already been registered previously.

I'd like to support this but I am currently using this resource. Can you give me some tips on what I can do to support this rename in my current setup?

Had a chat on discord about this and I'll figure it out how to migrate. So only to decide on the name now

• gitlab_project_runner_link
• gitlab_project_runner_attachment (known verbiage in the ecosystem)

[PatrickRice-KSC]

So only to decide on the name now

The more I think about this, the more I like the gitlab_project_runner_enablement verbiage. While the verbiage does insinuate that it's enabling something that's otherwise disabled (which it's not), it better matches the terraform provider design principle of aligning to the underlying API conventions. I was reviewing the "big 3" tf providers, and there isn't consistency there in the verbiage they use to approach this issue:

• AWS largely uses "attachment"
• GCP doesn't seem to use anything
• Azure largely uses "assignment"

So we don't really benefit from any recognizable "prior art". I'd suggest we have some good description text around what it does, then align to the underlying API.

I'm interested in thoughts from others though - Any thoughts @Shocktrooper , @armsnyder , @timofurrer ?

[Dutchy-]

I've renamed the resource per your suggestion, pending consensus. I'll happily rename it again if you decide on something different.

[Shocktrooper]

@PatrickRice-KSC I guess enablement might be the best even though its a misnomer for what it actually does. If I were looking to attach a runner with the terraform provider I would probably look at either the GitLab API docs first, see its enable/disable then check the provider to see if there is a matching resource or the other way around and see if the provider docs have something before checking the GitLab API to then go back to the provider

[Dutchy-]

I've processed all the feedback - if there is no further feedback I can squash the commits and the PR is completely ready :)

[timofurrer]

LGTM 🎉

[PatrickRice-KSC]

Looks good to me too, migrating to the GetRunnerDetails API for the read method addresses my concerns about the locking and pagination race condition that's present on the GetProjectRunners API as well, since the runner details API returns all projects. If someone has a runner linked to a ton of projects, this refresh may go a bit more slowly, but importantly it'll be error free.

I like it, let's ship it! Nice work @Dutchy- 🎉

[github-actions]

This functionality has been released in v3.14.0 of the Terraform GitLab Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue. Thank you!