-
Notifications
You must be signed in to change notification settings - Fork 763
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
github_branch_default *always* returns 422 Visibility is already private. [] on deletion #620
Labels
Comments
dee-kryvenko
added a commit
to dee-kryvenko/terraform-provider-github
that referenced
this issue
Jan 21, 2021
jcudit
pushed a commit
that referenced
this issue
Jan 22, 2021
#666) * github_branch_default: send only fields that changed. Fixes #625 #620. * fix failing test and update docs Co-authored-by: Jeremy Udit <[email protected]>
just hit the same issue on a terratest run:
|
hi @jcudit @kiddom-kq, just verified v4.3.1 fixes it. if somebody else also wants to confirm that the fix was sufficient, feel free to do so. here's the destroy output from the same run I posted prior:
|
k24dizzle
added a commit
to lyft/terraform-provider-github
that referenced
this issue
Feb 22, 2021
* v4.1.0 * Fix unable to resolve node id for branch_protection (integrations#610) * Don't check node id for length * Check if node id is valid base 64 Co-authored-by: Willem Gillis <[email protected]> * temporarily disable PR acceptance testing these jobs all fail and are confusing to contributors when launched from a PR raised by a fork. there are ways to get around this, but will defer until the repository is transferred. disabling for now. * remove `ForceNew` on `template*` as they are concerns only at creation time (integrations#609) There are a number of resources that have been marked as `ForceNew: true` out of a desire in "correctness" by those that do not actually understand how these resources are used in the real world and damage that can be done. No one wants to blow up a repository to change something like this, if they need to there is a mechanism built into terraform called [taint](https://www.terraform.io/docs/commands/taint.html). While there are some things that make sense for using `ForceNew` a repository for source control on properties that the API will ignore outside of creation is not one of them. Signed-off-by: Ben Abrams <[email protected]> * Add diff suppression function to the branch protection resource (integrations#614) Adding a diff suppression function to the branch protection resource to ignore the strict status check field if no contexts have been specified. This resolves the issue with the GraphQL API returning a strict status check value of "true" by default, regardless of contexts being set or not. * Add Apps to actor types in branch protection (integrations#615) Adding Github Apps to actor types in the branch protection resource. NOTE: Apps as an actor type is only available in push restrictions. * Added `allowsDeletions`and `allowsForcePushes`settings (integrations#623) * Added `allowsDeletions`and `allowsForcePushes`settings https://developer.github.com/v4/changelog/2020-11-13-schema-changes/ (#1) * complete documentation * update module github.com/shurcooL/githubv4 with `go get github.com/shurcooL/githubv4` * vendor latest githubv4 * add test for deletions and force pushes Co-authored-by: Jeremy Udit <[email protected]> * Fix syntax error * Conditionally Run GHES Test Suite * Run gofmt (integrations#645) Signed-off-by: Stephen Hoekstra <[email protected]> * Allow dependabot to check github actions (integrations#643) * Typo: s/visiblity/visibility (integrations#629) Small typo in the docs. * github_repository_webhook: describe content_type options (integrations#510) * change private to visibility (integrations#635) * Use commit SHA to lookup commit info in github_repository_file resource (integrations#644) * Fix references to "master" Signed-off-by: Stephen Hoekstra <[email protected]> * Use commit SHA to lookup commit info Currently the provider loops through commits in a repo until it finds the most recent commit containing the managed file. This is inefficient and could lead to you being rate limited if managing a few files that were updated a long time ago. This commit fixes that by storing the commit SHA when updating the file and using that SHA to lookup the commit info instead of looping through all commits. Signed-off-by: Stephen Hoekstra <[email protected]> * add release automation for terraform registry /cc https://www.terraform.io/docs/registry/providers/publishing.html * Add v4.2.0 Release (integrations#641) * add v4.1.1 release items * correct semver version * Document Additional Breaking Change For v3.0.0 * add `github_repository_file` bugfix * move to correct release * add goreleaser configuration to enable release automation * resource/repository: add support for enabling github pages (integrations#490) * add support for enabling github pages * update resource comments * add additional comments in expand methods * add formatting fixes Co-authored-by: Jeremy Udit <[email protected]> * Add `github_branch_protection_v3` Resource (integrations#642) * Add `branch_protection_v3` Resource - add new resource to `website/github.erb` - add new resource to `website/docs/r/<resource>.html.markdown` - add new resource to `github/provider.go` - add tests for resource in `github/resource_<resource>_test.go` - implement new resource in `github/resource_<resource>.go` * fixup! gofmt fixes * add changelog entries for v4.3.0 release (integrations#658) * Remove github.com/hashicorp/terraform from dependencies (integrations#628) * remove github.com/hashicorp/terraform from dependencies * go mod tidy * refactor: execute fmt * Allow dependabot to check go dependencies (integrations#653) * Fix link to Milestones page (integrations#663) * github_branch_default: send only fields that changed. Fixes integrations#625 integrations#620. (integrations#666) * github_branch_default: send only fields that changed. Fixes integrations#625 integrations#620. * fix failing test and update docs Co-authored-by: Jeremy Udit <[email protected]> * Fix error handling (integrations#668) Do not silently proceed further on receiving an error response. Signed-off-by: rustyclock <[email protected]> * Update CHANGELOG.md * Remove Obsolete Test My understanding is our use of Terraform Registry makes this failing test unnecessary. * Update GitHub organization references (integrations#672) Following the project transfer from `terraform-providers` organization to `integrations`. * Add Example For `github_team_repository` (integrations#676) * Add Example For `github_team_repository` also documents a limitation with `for_each` in this scenario * fixup! add newline * changelog: manually fix links to issues (integrations#673) They were pointing to the old archived repo, which doesn't have newer issues. * Handle base64 decodable repo names (integrations#684) * github/config: Fix detection of individual, non-org accounts (integrations#685) - Previously, whenever an individual user tried to interact with their repos, the provider would return an error, rendering v4.3.1 unusable _for individuals_: ``` ➜ terraform plan Error: GET https://api.github.com/orgs/issyl0: 404 Not Found [] on /Users/issyl0/repos/terraform/github.tf line 1, in provider "github": ``` - `ConfigureOwner` works such that if the `owner.name` is not blank (ie, the user had specified `owner = <username>` in their Terraform file), the code progresses to check if the `owner.name` is an org. Importantly, that check (prior to this change) returned an error if `owner.name` was not an org. That final error meant it was impossible to run if not using an organisation account. - Reproduction steps: https://gist.github.com/issyl0/cd61e4cb59de2c2e1e8f45e3cf7c12f5 * add changelog entry for v4.3.2 * Add `create_default_maintainer` Option To `github_team` (integrations#661) * Add `create_default_maintainer` option to `github_team` This adds a possible fix to the following issues by providing users with an option to remove the automatic addition of a default maintainer to a team during creation. /cc integrations#527 /cc integrations#104 /cc integrations#130 * Refresh CONTRIBUTING Documentation (integrations#682) * refresh contributing docs * add quick instructions * add updates for newer versions of terraform * Add Diff Suppression Option To `repository_collaborator` (integrations#683) * add diff suppression option to `repository_collaborator` * fixup! remove comment * remove hardcoded username from test * Update CHANGELOG for v4.4.0 release * Add repo context to error message when branch is not found (integrations#691) * Add repo context to error message when branch is not found * fix failing `TestAccGithubRepositoryFile` test access committer data through `Commit` struct Co-authored-by: Jeremy Udit <[email protected]> * fix based on linter (integrations#694) * add v4.4.1 release notes * Modify github_team_repository to accept slug as a valid team_id as well (integrations#693) * First attempt * Add comment * Attempt to modify unit tests * Edit docs to reflect change * Make sure team_id is set appropriately * fixing lint * add passing tests for `github_team_repository` Co-authored-by: Jeremy Udit <[email protected]> * add v4.5.0 release notes * temporarily ignore `darwin/arm64` to unblock releases /cc integrations#695 * update release notes for v4.5.0 Co-authored-by: tf-release-bot <[email protected]> Co-authored-by: Polygens <[email protected]> Co-authored-by: Willem Gillis <[email protected]> Co-authored-by: Jeremy Udit <[email protected]> Co-authored-by: Ben Abrams <[email protected]> Co-authored-by: Patrick Marabeas <[email protected]> Co-authored-by: Francois BAYART <[email protected]> Co-authored-by: Stephen Hoekstra <[email protected]> Co-authored-by: John Losito <[email protected]> Co-authored-by: Bret <[email protected]> Co-authored-by: Jakub Holy <[email protected]> Co-authored-by: Ichinose Shogo <[email protected]> Co-authored-by: angie pinilla <[email protected]> Co-authored-by: Shu Kutsuzawa <[email protected]> Co-authored-by: Oleksandr Dievri <[email protected]> Co-authored-by: Dee Kryvenko <[email protected]> Co-authored-by: Ravi <[email protected]> Co-authored-by: Alexis Gauthiez <[email protected]> Co-authored-by: Christian Höltje <[email protected]> Co-authored-by: Issy Long <[email protected]> Co-authored-by: Michael Barany <[email protected]>
kfcampbell
pushed a commit
to kfcampbell/terraform-provider-github
that referenced
this issue
Jul 26, 2022
…ons#625 integrations#620. (integrations#666) * github_branch_default: send only fields that changed. Fixes integrations#625 integrations#620. * fix failing test and update docs Co-authored-by: Jeremy Udit <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I've found an issue that seems to crop up on repo deletion.
The issue appears to be with private repos that have a `` resource set.
Terraform Version
This issue is reproducible on the latest versions. (as of today; 2020-12-04)
Affected Resource(s)
Terraform Configuration Files
Here is a simple test-case that is identical to the test case I used to find/document this bug. I have stripped out org/personal information and substituted generic tokens in place:
Debug Output
Please provide a link to a GitHub Gist containing the complete debug output: https://www.terraform.io/docs/internals/debugging.html. Please do NOT paste the debug output in the issue; just paste a link to the Gist.
Here is a short version:
Note: I get the same result if I comment out only the
github_branch_default
resource and then run a normalterraform apply
. I just used--target
to hone which resource caused the error.I am more than happy to provide the full
TF_LOG=TRACE
output, but it's 3K lines long with the--target
flag. I've included the relevant bits below:Panic Output
No panic produced.
Expected Behavior
I would expect the default branch configuration to be deleted w/o the
422
API error.Actual Behavior
I get this error returned from GH API:
Steps to Reproduce
note2
below) comment out thegithub_branch_default
resource in the above 'test case' configuration.tf apply
against the config and observe that 4 resources are created, one of which is the repo.github_branch_default
resource in the above 'test-case' config.git checkout -b && git push
to make the branch calledMyNonMainDefaultBranch
in the new repotf apply
against the config and observe that one new resource will be createdtf apply
against the config and observe that 5 resources will be destroyednote1
below). Manually remove thegithub_branch_default
resource from the state filetf apply
against the config and observe that the remaining resource (thegithub_repository.myOrg_testing
) will be destroyedNote1:: If, instead of manually removing the
github_branch_default
resource from the state file as in step 10, you leave ONLY thegithub_repository
resource un-commented, terraform will still only destroy thegithub_branch_default.myOrg_testing
resource. This tells me that the dependency resolution is logically correct but inefficient/un-optimized; the deletion of thegithub_branch_default
does not matter if thegithub_repository
is to be deleted. Deleting the repo necessarily deletes the default branch config.Note2: Failure to comment out the
github_branch_default
resource in step 1 will result in an error:Important Factoids
I can reproduce this behavior with the token that was allocated specifically for our automated Terraform ci/cd pipeline and with my personal token which has 'Owner' level permissions attached to it.
The workaround is to:
comment out / delete the
github_branch_default
resource from the*.tf
file(s).manually remove the
github_branch_default
resource from the state file:terraform state rm github_branch_default.myOrg_testing
:terraform apply
and let it observe that thegithub_repository
is missing and thus should be deleted. Deleting the repo necessarily deletes the repos' default branch config.References
These issues seem related:
https://github.com/terraform-providers/terraform-provider-github/issues/580
The text was updated successfully, but these errors were encountered: