fix: Use default_branch_protection_defaults to decide on protection wait

Hamza Hassanainrequested to merge
gitlab-community/gitlab-org/terraform-provider-gitlab:6895-branch-protection-defaults into main

What does this MR do?

In some groups, gitlab_project create waited 10 minutes for a branch protection that GitLab never adds, then failed. The project was left tainted, so the next apply deleted and created it again. Now the provider only waits when GitLab will really protect the branch.

The provider decided whether to wait from the deprecated default_branch_protection number. GitLab itself decides from default_branch_protection_defaults, and the two can disagree. Changing the number also updates the defaults, but changing the defaults leaves the number as it was. The reporter's group returns this (from the issue):

"default_branch_protection": 2,
"default_branch_protection_defaults": {
  "allowed_to_push": [], "allow_force_push": false, "allowed_to_merge": [],
  "developer_can_initial_push": false, "code_owner_approval_required": false
}

Now the provider reads default_branch_protection_defaults and applies GitLab's rule: the branch is protected unless force push is allowed and every push and merge entry is Developer. For a project outside a group, it reads the instance settings, as before (admin tokens only). If GitLab doesn't return the defaults, the provider still uses the deprecated number.

The reporter's case (empty push and merge lists) needs one more rule. GitLab then falls back to the Admin access level. Self-managed accepts that and protects the branch, so the provider still waits there. GitLab.com rejects it and never protects the branch, so I skip the wait for this case only on gitlab.com.

The GitLab code this follows:

Related issues

Closes #6895

How I tested

I couldn't reproduce this by hand locally, because creating a project needs a running GitLab instance.

So I added an acceptance test for it. Its group allows force push with Developer push and merge, so the defaults mean "no protection" while the deprecated number stays 2. It creates a project there with a 2 minute create timeout and checks that main isn't protected. It passed in the pipeline on the current commit, on GitLab CE 19.2.6, 19.3.2 and 19.4.0. I didn't check that it times out without the fix. Job logs:

I added unit tests for the decision, with a fake GitLab server for the group and instance cases. They are the only tests for the gitlab.com rule, because the pipeline runs self-managed GitLab. To make sure they catch the bug, I put the old decision back in a scratch copy, and the group and instance tests failed there. Locally I ran the unit tests of the changed package:

$ go test -count=1 ./internal/provider/sdk/
ok  	gitlab.com/gitlab-org/terraform-provider-gitlab/internal/provider/sdk	37.044s

gofmt, go build ./... and go vet (with and without the acceptance tag) printed nothing. The lint jobs passed in the pipeline.

One question:

  1. For empty push and merge lists, should I keep skipping the wait only on gitlab.com? Or skip it on every instance? That's a one-line change, but on self-managed apply could then finish before the branch is protected.

Implemented with AI assistance; I reviewed and tested the changes.

Edited by Hamza Hassanain

Merge request reports