terraform-provider-gitlab icon indicating copy to clipboard operation
terraform-provider-gitlab copied to clipboard

Merge request approval rule `any_approver` fails during apply if present in GitLab project

Open petrmvala opened this issue 2 years ago • 9 comments

GitLab Provider version

3.12.0

GitLab version

Gitlab 14.8 Edition Ultimate

Terraform version

1.1

Relevant Terraform Configuration

resource "gitlab_project_approval_rule" "codeowners" {
  count = local.gitlab_protect_count == 1 && local.try_codeowners_file ? 1 : 0

  project            = var.gitlab_project
  name               = "Any approver"
  rule_type          = "any_approver"
  approvals_required = 1
}

Relevant log output

│ Error: POST https://gitlab.i-am-not-telling-you.com/api/v4/projects/some-id/approval_rules: 400 {message: {name: [has already been taken]}, {rule_type: [any-approver for the project already exists]}}
│ 
│   with module.my_project.gitlab_project_approval_rule.codeowners[0],
│   on gcp_project/gitlab_protect.tf line 55, in resource "gitlab_project_approval_rule" "codeowners":
│   55: resource "gitlab_project_approval_rule" "codeowners" {

Description

The any_approver approval rule can be present only once in the GitLab project. From UI it gets created automatically when someone increases the number of approvers from 0 to a different number. Even when decreased back to 0, the rule still exists and is not deleted. The rule can only be deleted via API. The Terraform plan does not detect the presence of this rule and therefore produces a valid plan which then fails to apply because API rejects the creation of a new rule.

petrmvala avatar Mar 11 '22 14:03 petrmvala

It sounds like a change made outside of Terraform for something that Terraform wasn't tracking. Have you tried importing the approval rule into state?

# GitLab project approval rules can be imported using a key composed of `<project-id>:<rule-id>`, e.g.
terraform import gitlab_project_approval_rule.codeowners[0] "12345:6"

Edit: I see the conversation that occurred in Discord now. So it seems like the issue is that you'd want the plan to look up all the approval rules and fail if there is already an existing one with rule_type "any_approver"?

jtymes avatar Mar 11 '22 16:03 jtymes

I'm still trying to understand what exactly is going on here. I wonder why it tries to re-create the approval rule. I assume that you've first run your terraform, then changed it out of bound, then re-run terraform, right?

In any case, looking at the code I found a potential bug when having a lot of approval rules in the same project. Do you have many? How many? (see https://github.com/gitlabhq/terraform-provider-gitlab/pull/950)

The proper behavior would probably be that if the resource is owned by terraform already, it should just update the rule ...

timofurrer avatar Mar 12 '22 08:03 timofurrer

This is not a problem of many rules. I have just one in there. It is either a regular rule (in most of the projects). Or, if some other conditions are met (e.g. CODEOWNERS file present in the repository), the any_approver rule.

The problem is not in re-creating. The terraform rule hasn't been there before. Respectively, not the any_approver one. If the any_approver rule was created by TF before, then yes, it should be just updated. But in this case, it was created manually and only then added to terraform. I would expect this plan to fail, because it is actually not a valid plan.

petrmvala avatar Mar 12 '22 19:03 petrmvala

The Terraform plan does not detect the presence of this rule and therefore produces a valid plan which then fails to apply because API rejects the creation of a new rule.

Terraform doesn't know what the rule ID is since it's not in the state. Terraform is seeing a new resource in the configuration to add to state, and will propose a plan to create it.

The rule already exists in the API, so it should be imported for Terraform to manage.

jtymes avatar Mar 12 '22 20:03 jtymes

Yes, I think I agree there with @jtymes. You need to import first and the read (during plan) isn't aware of the id conflict there ...

timofurrer avatar Mar 13 '22 11:03 timofurrer

Well, that solves just one part of the problem, I think. Agree on importing is the best approach.

On the other hand, there is no smart way (that I know of) of importing every resource and one just can not reasonably expect that anyone would be going through all possible TF resources to make sure he imported everything.

In addition to this, it is not true that TF does not know the rule ID. Since there can be only one any_approver rule, we can extract it pretty easily and check against it.

petrmvala avatar Mar 14 '22 14:03 petrmvala

On the other hand, there is no smart way (that I know of) of importing every resource and one just can not reasonably expect that anyone would be going through all possible TF resources to make sure he imported everything.

For bringing them under the management of Terraform, that is the expectation. It's tedious, but generally you can script out import calls if needed.

In addition to this, it is not true that TF does not know the rule ID. Since there can be only one any_approver rule, we can extract it pretty easily and check against it.

The extraction itself wouldn't be difficult, it's when the provider would even be able to perform the extraction. If you have an example from another provider that does this, that would be helpful.

jtymes avatar Mar 14 '22 15:03 jtymes

@petrmvala Following up here, do you have any examples of this behavior? I'm unsure of the usefulness of this since either way the resource is not created, it's just a difference between finding out in a plan vs. an apply.

jtymes avatar Apr 13 '22 19:04 jtymes

Hey @jtymes, sorry for such a long radio silence. I have been totally overloaded in the meantime.

The first point The example goes like this:

  1. You create a Gitlab repo. Via Terraform or manually, that's not important. You do not set approval rules via terraform though.
  2. You go to the repo and manually set Eligible approvers to 1. (Settings -> General -> Merge request approvals -> Eligible users). This will create the any_approver rule in repository. Setting back to 0 does not delete the rule, just sets the number of approvers to 0.
  3. You run terraform to set the any_approver type rule and observe the failure.

Why is this a good example? Consider you manage GitLab repositories (hundreds or thousands). Some of them might be new and created via Terraform, but some have been created manually at random points in time. What you want to do is to impose some standards in these repositories, and manage these standards in Terraform. One of these standards is using CODEOWNERS. For that, you need to be able to set the any_approver rule in every repo where you match some conditions (such as how CODEOWNERS looks like). However, your pipeline fails on every repository where someone enabled Eligible users at any point in time. The result is that your pipeline fails on all repositories where this was done.

To the second point. It is a fundamental difference between knowing during plan that something will fail on apply, or knowing it only during apply. For example, you can do terraform plan on the merge request and terraform apply only after it has been merged into master. When the apply fails, you need to revert changes and potentially kill some newly deployed infrastructure. In principle, you will need to spend unnecessary and unexpected time on that, which is just toil. While you can probably argue away that in the case of GitLab approver rules it is not such a big deal, having Terraform modules that behave in an unpredictable way is time-consuming and wasteful.

petrmvala avatar Jul 08 '22 18:07 petrmvala