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

provider wants to endlessly change the `branches` attribute of `github_repository` resource when repo is archived

Open dimisjim opened this issue 2 years ago • 12 comments

Hey,

What the title says basically. The provider keeps trying to change the branches attribute of the github_repository resource, when the repo is archived. Even after applying, which outputs no error, the drift persists.

I presume this is related to the fact that even via the UI, when a repo is archived, it is read-only and thus branches config cannot change. So when visiting https://github.com/$org/$repo$/settings/branches one gets this message: image

Terraform Version

1.1.2

Affected Resource(s)

  • github_repository

Terraform Configuration Files

resource "github_repository" "r" {
  name                   = var.name
  description            = var.description
  visibility             = var.visibility
  homepage_url           = var.homepage_url
  allow_merge_commit     = var.allow_merge_commit
  allow_squash_merge     = var.allow_squash_merge
  allow_rebase_merge     = var.allow_rebase_merge
  vulnerability_alerts   = var.vul_alerts
  has_downloads          = var.has_downloads
  has_issues             = var.has_issues
  archived               = var.archived
  archive_on_destroy     = var.archive_on_destroy
  topics                 = var.topics
  is_template            = var.is_template
  delete_branch_on_merge = var.delete_br_on_merge
  auto_init              = true
  allow_auto_merge       = var.allow_auto_merge
  
  dynamic "template" {
    for_each = length(var.template_name) > 0 ? [""] : []
    
    content {
      owner      = "$redacted"
      repository = var.template_name
    }
  }

  lifecycle {
    ignore_changes = [auto_init]
  }
}

Expected Behavior

No drift to be outputted

Actual Behavior

  # module.repo_test-repo.github_repository.r will be updated in-place
  ~ resource "github_repository" "r" {
      + branches               = (known after apply)
        id                     = "$redacted"
        name                   = "$redacted"
        # (28 unchanged attributes hidden)
    }

Steps to Reproduce

Please list the steps required to reproduce the issue, for example:

  1. import an archived repo (or archive it after creating it via terraform)
  2. plan
  3. see drift being outputted
  4. issue is mitigated when branches is included in the ignore_changes array, but this could lead to other issues too

dimisjim avatar Jan 14 '22 06:01 dimisjim

I'm seeing this on normal, unarchived repositories too.

The first pass I did terraform wanted to add "branches" attribute to 191 repos and subsequent passes it wants to add "branches" to 92 repos.

Terraform v1.1.3 on darwin_arm64

  • provider registry.terraform.io/hashicorp/aws v3.72.0
  • provider registry.terraform.io/hashicorp/github v4.19.1
  • provider registry.terraform.io/integrations/github v4.19.1

Looking at a diff between a repository terraform repeatedly wants to add "branches" to vs. one it actually put "branches" into, the one it added it to has attribute "auto_init" and it's set to "false". The repo it keeps asking to add "branches" to does not have attribute "auto_init". -- This seems to be a red herring as I found another problematic repo that does have "auto_init" attribute set and no "branches" set.

wagnerone avatar Jan 18 '22 22:01 wagnerone

The repo it keeps asking to add "branches" to does not have attribute "auto_init"

Interesting. The archived repos in our case, have indeed the auto_init attribute set to true, so it could be correlated

dimisjim avatar Jan 18 '22 23:01 dimisjim

After upgrading provider to 4.19.1 we are seeing the same issue with normal, un-archived repositories.

Workaround we have for now is adding :

  lifecycle {
    ignore_changes = [
      branches,
    ]
  }

in resource "github_repository" "repos" {...}

Terraform v1.0.11 on linux_amd64

aliculPix4D avatar Jan 20 '22 08:01 aliculPix4D

I'm guessing this was introduced in 4.19.0 given the changelog mentions Export branches attribute of github_repository resource.

Sure enough attempting to upgrade from 4.16.0 -> 4.19.0 results in the same behaviour others are seeing (with both archived and normal repos). Given we have some 400 repos we've decided not to update all their resources and have instead pinned to 4.18.2 until this gets resolved.

shoddyguard avatar Feb 01 '22 17:02 shoddyguard

After upgrading provider to 4.19.1 we are seeing the same issue with normal, un-archived repositories.

Workaround we have for now is adding :

  lifecycle {
    ignore_changes = [
      branches,
    ]
  }

in resource "github_repository" "repos" {...}

Terraform v1.0.11 on linux_amd64

Sorry - I had to downvote this answer as it is a hack. We really need this fixed as we have 500+ repositories managed and adding this 500+ times and then removing once the bug is fixed is a non-starter.

jensenbox avatar Mar 09 '22 17:03 jensenbox

https://github.com/integrations/terraform-provider-github/issues/1010

Related issue ^

k24dizzle avatar Mar 22 '22 23:03 k24dizzle

I have migrated from the deprecated hashicorp/github provider to integrations/github and this started happening for me too.

MrBlaise avatar Mar 31 '22 14:03 MrBlaise

I gave a shot to fix this issue here: https://github.com/integrations/terraform-provider-github/pull/1117

Would appreciate any feedback on the best ways to potentially roll this change out.

k24dizzle avatar Apr 18 '22 23:04 k24dizzle

Adding on additional information to the sentiment that the ignore_changes workaround is insufficient. Thanks to https://github.com/hashicorp/terraform/pull/30517, Terraform now slaps the user's hand for ignoring a computed attribute (branches in this case).

╷ │ Warning: Redundant ignore_changes element │ │ on < redacted > line 1, in resource "github_repository" "this": │ 1: resource "github_repository" "this" { │ │ Adding an attribute name to ignore_changes tells Terraform to ignore future changes to the argument in configuration after the object has been created, retaining the │ value originally configured. │ │ The attribute branches is decided by the provider alone and therefore there can be no configured value to compare with. Including this attribute in ignore_changes has │ no effect. Remove the attribute from ignore_changes to quiet this warning.

StephenWithPH avatar Jul 01 '22 21:07 StephenWithPH

We still have sort of an issue with ignoring branches (version 1.2.5). In part of our code, we use a loop to bulk create repositories

resource "github_repository" "repository" {
  for_each           = toset(concat(var.repositories["group1"], var.repositories["group2"])
  name               = each.key
  archive_on_destroy = true
  visibility         = "private"
}

In such case, running terraform plan results in plan containing changes in branches. Update to following form:

resource "github_repository" "repository" {
  for_each           = toset(concat(var.repositories["group1"], var.repositories["group2"])
  name               = each.key
  archive_on_destroy = true
  visibility         = "private"
  lifecycle {
    ignore_changes = [
      branches,
    ]
  }
}

generates warning

Warning: Redundant ignore_changes element

It looks like changes in branches are not properly ignored.

PrzemyslawZieja avatar Jul 19 '22 08:07 PrzemyslawZieja

Any ETA when this will be fixed? The Warning: Redundant ignore_changes element is really annoying

gastaldi avatar Aug 17 '22 14:08 gastaldi

Does 5.0 version close this issue by eliminating branches?

maciejp-ro avatar Sep 15 '22 14:09 maciejp-ro

@dimisjim , @k24dizzle , @kfcampbell it seems after merging https://github.com/integrations/terraform-provider-github/pull/1117 fixes this issue, could you confirm? I see this issue still open, so I'm not sure on how it is going.

Esysc avatar Oct 04 '22 07:10 Esysc

@Esysc thanks for the poke; I had neglected to close this issue. You're correct that #1117 should resolve this behavior, and any further issues can be opened separately.

kfcampbell avatar Oct 10 '22 22:10 kfcampbell