integrations / terraform-provider-github

Terraform GitHub provider
https://www.terraform.io/docs/providers/github/
MIT License
889 stars 732 forks source link

Can't import github_branch_protection using repository:branch id #597

Closed k24dizzle closed 1 year ago

k24dizzle commented 3 years ago

Terraform Version

v4.0.0

Affected Resource(s)

Terraform Configuration Files

provider "github" {
  organization = "kzhaotest"
}

resource "github_branch_protection" "test" {
  repository_id  = "branch_protection"
  pattern        = "main"
  enforce_admins = false

  required_pull_request_reviews {
    dismiss_stale_reviews           = true
    required_approving_review_count = 1
  }
}

Debug Output

https://gist.github.com/k24dizzle/db8134b8b00171f7b6dbd4c38d4886b4

I tried importing the branch protection, but the provider couldn't find the resource. But when I applied the changes, I ran into errors saying the branch protection rule for that branch already existed.

Expected Behavior

I expected terraform to import the state of the branch_protection of my main branch in this repo: https://github.com/kzhaotest/test as described in the documentation here: https://www.terraform.io/docs/providers/github/r/branch_protection.html#import

Actual Behavior

kzhao@kzhao-mbp151 ~/other/branch_protection (main) $ terraform import github_branch_protection.test test:main
github_branch_protection.test: Importing from ID "test:main"...
github_branch_protection.test: Import prepared!
  Prepared github_branch_protection for import
github_branch_protection.test: Refreshing state... [id=test:main]

Error: Cannot import non-existent remote object

While attempting to import an existing object to
github_branch_protection.test, the provider detected that no object exists
with the given id. Only pre-existing objects can be imported; check that the
id is correct and that it is associated with the provider's configured region
or endpoint, or use "terraform apply" to create a new remote object for this
resource.

Steps to Reproduce

terraform init terraform import github_branch_protection.test test:main

Notes

I did notice in other experiments that I was able to import a github_branch_protection resource by instead of using repo_name:branch_name using the graphql node id of the branch protection rule.

But I assume that this overall issue is related to https://github.com/terraform-providers/terraform-provider-github/pull/593

Interestingly enough, even though https://github.com/kzhaotest/test has a branch protection rule for the main branch, I wasn't able to get the id of the rule in the graphql explorer, so I wasn't able to test this assumption out here.

jcudit commented 3 years ago

This does look related to https://github.com/terraform-providers/terraform-provider-github/pull/593. 🤔 we will likely need to update the import function to handle the more flexible behaviour of providing a node ID or a repository name.

jcudit commented 3 years ago

Raised ☝️ with the import logic fixes. Take a look and let me know if the PR looks 👌 .

Tested well locally but am seeing some flakiness with the acceptance tests. Aiming to get this released alongside a couple of other fixes in the upcoming v4.0.1 release.

kchristensen commented 3 years ago

I think there's another issue with https://github.com/terraform-providers/terraform-provider-github/pull/593 when trying to apply branch_protection_rules where repository_id is set to a repositories node_id (as opposed to it's name). When trying to add a rule I get:

Error: Could not resolve to a Repository with the name 'myOrgNname/whatIPresumeIsTheNodeId'.

When I switch repository_id back to using name, it creates the rule as expected.

Creating branch_protection_rules in 3.1.0 with node_id works as expected.

jcudit commented 3 years ago

@kchristensen see this branch which creates a branch protection rule in v4.0.0 using node_id. I may be misunderstanding the issue though.

faultylee commented 3 years ago

We encountered an issue today on this, and our repo name was base64 decoeable, so it tricked the code to use the node_id path and fail with the same error. Turn out most 4 & 9 char string (a-z) are base64 decodeable. I'm sure there's more. The example repo name I tested was abcdefgh. Log: https://gist.github.com/faultylee/974596a4349bb7d38dba42eddcfb4f08

Maybe consider checking of the decoded output is valid ascii.

NOTE: we've updated to v4.2.0 which contain #610

We then tried importing using node_id and ran into a diff error, and have raised an issue here (#671)

brunobertoldi commented 3 years ago

Same here, the names wiki and identity are being interpreted as node_id instead of name:

Error: Could not resolve to a node with the global id of 'identity'

Version v4.3.1

k24dizzle commented 3 years ago

I went ahead and gave the above bug a shot here: https://github.com/integrations/terraform-provider-github/pull/684

Working on verifying that it works locally

bateller commented 3 years ago

Just to update, this appears to still fail for me:

Initializing provider plugins...
- Finding integrations/github versions matching "4.5.0"...
- Installing integrations/github v4.5.0...
- Installed integrations/github v4.5.0 (signed by a HashiCorp partner, key ID 38027F80D7FD5FB2)
$ terraform import github_branch_protection.some_repo some_repo:branch
github_branch_protection.some_repo: Importing from ID "some_repo:branch"...

Error: nil entry in ImportState results. This is always a bug with
the resource that is being imported. Please report this as
a bug to Terraform.
bateller commented 3 years ago

The failure was due to master not having branch protection setup.

It appears this issue (and https://github.com/integrations/terraform-provider-github/issues/671) are failing for me due to not checking if the imported resource exists on Github (which would be a nice check for those of us using Terraform to manage Github when we don't have direct Admin access)

Should be closed by https://github.com/integrations/terraform-provider-github/pull/721 Disregard, it looks like it doesn't fail nicely when branch protection doesn't exist https://github.com/integrations/terraform-provider-github/pull/684/files#r590391784

jcudit commented 3 years ago

those of us using Terraform to manage Github when we don't have direct Admin access

That call out is very much appreciated. Will have to factor this angle into how the project grows over time.

As for next steps to correct this behaviour, adding better error handling when branch protection does not exist makes sense.

martibosch commented 2 years ago

Hello!

I am still encountering this issue in v4.26.1 when the branch protection does not exist (but the branch does exist):

Error: Could not resolve to a node with the global id of 'martibosch'

with:

resource "github_repository" "repository" {
  name        = "foo"
  description = "bar"
  visibility  = "public"

  allow_merge_commit = false

  auto_init          = true
  gitignore_template = "Terraform"
  license_template   = "gpl-3.0"
}

data "github_user" "user" {
  username = "martibosch"
}

## Branch protection rules
resource "github_branch_protection" "repository" {
  repository_id           = github_repository.repository.node_id
  pattern                 = "develop"
  required_linear_history = true

  required_status_checks {
    strict   = true
    contexts = ["ci"]
  }

  required_pull_request_reviews {
    dismiss_stale_reviews  = true
    pull_request_bypassers = [data.github_user.user.username]
  }
}

what can I do to overcome this error?

Thank you. Best, Martí

github-actions[bot] commented 1 year ago

👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!