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

Unable to import existing resources to gitlab_branch resource

Open genei09 opened this issue 1 year ago • 3 comments

GitLab Provider version

3.16.1

GitLab version

15.3.0 SaaS

Terraform version

Terraform v0.14.6

Relevant Terraform Configuration

resource "gitlab_branch" "production" {
  for_each = gitlab_project.this
  project  = each.value.id
  name     = "production"
  ref      = "main"

  lifecycle {
    prevent_destroy = true
  }
}

Relevant log output

- '  # gitlab_branch.production["gitlab-project-settings-test"] must be replaced'
    - -/+ resource "gitlab_branch" "production" {
    - '      ~ can_push            = true -> (known after apply)'
    - '      ~ commit              = ['
    - '          - {'
    - '              - author_email    = "redacted"'
    - '              - author_name     = "Aaron Sua"'
    - '              - authored_date   = "2022-07-31T21:45:24Z"'
    - '              - committed_date  = "2022-07-31T21:45:24Z"'
    - '              - committer_email = "redacted"'
    - '              - committer_name  = "Aaron Sua"'
    - '              - id              = "redacted"'
    - '              - message         = "Add README.md"'
    - '              - parent_ids      = []'
    - '              - short_id        = "39332ccb"'
    - '              - title           = "Add README.md"'
    - '            },'
    - '        ] -> (known after apply)'
    - '      ~ default             = false -> (known after apply)'
    - '      ~ developer_can_merge = false -> (known after apply)'
    - '      ~ developer_can_push  = false -> (known after apply)'
    - '      ~ id                  = "38232732:production" -> (known after apply)'
    - '      ~ merged              = false -> (known after apply)'
    - '        name                = "production"'
    - '      ~ protected           = true -> (known after apply)'
    - '      + ref                 = "main" # forces replacement'
    - '      ~ web_url             = "https://gitlab.com/redacted/demo_repos/demos/gitlab-project-settings-test/-/tree/production" -> (known after apply)'
    - '        # (1 unchanged attribute hidden)'
    - '    }'

Description

after importing an existing branch the required parameter of "ref" causes the branch to be destroyed which conflicts with a lifecycle aiming to keep the branches protected from deletion.

it took some time to figure this out. At least documenting this case would be good, but really I think that import should be able to record ref and assume that the code is correct. If I didn't care about it being destroyed why would I run import? I'd just go delete the branch and let it be recreated.

genei09 avatar Aug 03 '22 19:08 genei09

Thanks for open this @genei09

The reason for this behavior is that the upstream GitLab API doesn't return the ref the user provided when the branch was created. Therefore, when this was implemented it was decided that ref is only being set to state during create - which isn't hit during an import. The documentation is clearly missing for that.

However, I believe there must be a better solution. At the moment the GitLab API returns a commit object which contains the commit SHA. Therefore, we may be able to use a suppress function to suppress a diff when the ref matches the returned commit SHA.

I'll experiment a little to see if I can work something out.

EDIT: a diff suppress func doesn't work, because we don't have access to the provider config in those hooks. A CustomizeDiff function would do, though. I was thinking about something along the lines of this:

CustomizeDiff: customdiff.ForceNewIf("ref", func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) bool {
	if !d.HasChange("ref") {
		return false
	}

	project := d.Get("project").(string)
	old, new := d.GetChange("ref")
	oldRef := old.(string)
	newRef := new.(string)
	client := meta.(*gitlab.Client)

	branch, _, err := client.Branches.GetBranch(project, oldRef, gitlab.WithContext(ctx))
	if err != nil {
		return true
	}

	return branch.Commit.ID != newRef && branch.Commit.ShortID != newRef
}),

... but I'm not yet sure if that would work in all circumstances and I'm still trying to figure out where and when ref should / must be set ...

timofurrer avatar Aug 05 '22 13:08 timofurrer

as a workaround the lifecycle option can be included to ignore changes to ref, which is what I've done for now.

I can't think of a use case when I would want to destroy a branch and recreate it from a different ref. That could all be managed within git itself.

So other than being necessary for create, it's not really helpful in the ongoing management. The branches will naturally diverge and the origination of the branch becomes unimportant.

If terraform allows to ignore that, it would make sense to me to do so.

Perhaps a default, setting it to the earliest commit hash would allow it to not be required, or at least to default use the default branch.

You'd still have a problem if you specify a ref and do an import, but I think it would help and keep the hcl a little more DRY on implementation.

genei09 avatar Aug 05 '22 14:08 genei09

@genei09 Yes, I agree.

So other than being necessary for create, it's not really helpful in the ongoing management. The branches will naturally diverge and the origination of the branch becomes unimportant.

I think the only case it becomes important is if someone wants to "update" a branch or move it along with another branch somehow (which would be a suspicious workflow - but there may be some use case for it ...)

If terraform allows to ignore that, it would make sense to me to do so.

We could just add a suppress function for that particular field and basically ignore all changes from within the resource schema. However, I think it might be better to advise users to use the lifecycle ignore in the hcl instead.

Perhaps a default, setting it to the earliest commit hash would allow it to not be required, or at least to default use the default branch.

.. AFAIK, there is no way to figure out the commit id the branch was started from - thus, this wouldn't be an option and wouldn't work for imports properly, too.

At this point and with the APIs at hand I'm not sure if there is a "proper fits-all" solution.

What do you think about just adding documentation to advise the user to use lifecycle for ref to avoid problems on imports and diverging branches (which is a pretty normal thing) ?

timofurrer avatar Aug 10 '22 13:08 timofurrer