fix: always add the cache policy

[kayman-mk]

Description

As described in #298 the cache module has to be applied separately before creating the Runners. Terraform complains about

Error: Invalid count argument

  on ..\..\..\terraform-aws-gitlab-runner\main.tf line 323, in resource "aws_iam_role_policy_attachment" "docker_machine_cache_instance":
 323:   count      = var.cache_bucket["create"] || local.bucket_policy != "" ? 1 : 0

The "count" value depends on resource attributes that cannot be determined
until apply, so Terraform cannot predict how many instances will be created.
To work around this, use the -target argument to first apply only the
resources that the count depends on.

This PR always adds the policy to get rid of the error message during apply. As far as we know there is no use cases deploying this module without a cache. Thus we can always add the cache policy to the role.

Closes #298

Migrations required

No

Verification

• create a runner with a cache created separately (using the cache module). No error is shown during apply
• create a runner and let the module create the cache. apply shows no error.
• destroy everything without error

[semantic-releaser]

🎉 This PR is included in version 5.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

[Neki]

I think this MR introduces an issue with this configuration:

  cache_bucket = {
    "create" : false,
    "bucket" : aws_s3_bucket.build_cache.id
  }

(use case: S3 cache created outside of this module, because it's shared with other runner types)

When applying this, I get the error:

│ Error: Error attaching policy  to IAM Role spot-runners-instance: InvalidParameter: 1 validation error(s) found.
│ - minimum field size of 20, AttachRolePolicyInput.PolicyArn.
│
│
│   with module.ec2_runner.aws_iam_role_policy_attachment.docker_machine_cache_instance,
│   on .terraform/modules/ec2_runner/main.tf line 392, in resource "aws_iam_role_policy_attachment" "docker_machine_cache_instance":
│  392: resource "aws_iam_role_policy_attachment" "docker_machine_cache_instance" {
│

since the policy_arn is set to the empty string.

[kayman-mk]

Had the same problem an hour ago, but no time to create the issue.

100% correct.

Do you have a use case for a Runner without a cache?

[Neki]

We do use a cache, but it's created outside of this module (I don't wish this module to create it for me).

That's because we run several Gitlab runner types (Kubernetes and docker-machine), and they should all share the same cache. So it's simpler to create the S3 resources in a separate module, then pass them to the various modules that deploy the runners.

So

• I don't have a use case for a runner without a cache
• but I have a use case for a runner where the cache is not created by this module 😄

[kayman-mk]

@Neki It would be wonderful if you could have a quick look at my PR mentioned above. Just did the the terraform apply without creating a cache and adding a policy and it worked like a charme.