Skip to content
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

[BUG]: Since 6.0.1 the github_actions_repository_permissions resource errors on read when it's disabled #2182

Closed
1 task done
stevehipwell opened this issue Mar 5, 2024 · 7 comments · Fixed by #2186
Closed
1 task done
Labels
Status: Up for grabs Issues that are ready to be worked on by anyone Type: Bug Something isn't working as documented

Comments

@stevehipwell
Copy link
Contributor

Expected Behavior

When a github_actions_repository_permissions resource is set to disabled it should be able to be read correctly.

Actual Behavior

We're getting the following error when we have a github_actions_repository_permissions resource set to disabled.


│ Error: GET https://api.github.com/repos/my-org/test/actions/permissions/selected-actions: 409 []

│ with module.repo["test"].github_actions_repository_permissions.default,
│ on modules/repo/main.tf line 72, in resource "github_actions_repository_permissions" "default":
│ 72: resource "github_actions_repository_permissions" "default" {

I suspect that this has been caused by the changes in 97d5113.

Terraform Version

Terraform v1.5.7
on linux_amd64

  • provider registry.terraform.io/hashicorp/random v3.6.0
  • provider registry.terraform.io/integrations/github v6.0.1

Affected Resource(s)

  • github_actions_repository_permissions

Terraform Configuration Files

locals {
  actions_enabled = false
}

resource "github_actions_repository_permissions" "default" {
  repository      = "my-repo"
  enabled         = local.actions_enabled
  allowed_actions = local.actions_enabled ? "all" : null
}

Steps to Reproduce

No response

Debug Output

No response

Panic Output

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@stevehipwell stevehipwell added Status: Triage This is being looked at and prioritized Type: Bug Something isn't working as documented labels Mar 5, 2024
@github-project-automation github-project-automation bot moved this to 🆕 Triage in 🧰 Octokit Active Mar 5, 2024
@kfcampbell kfcampbell added Status: Up for grabs Issues that are ready to be worked on by anyone and removed Status: Triage This is being looked at and prioritized labels Mar 5, 2024
@kfcampbell kfcampbell moved this from 🆕 Triage to 🔥 Backlog in 🧰 Octokit Active Mar 5, 2024
@kfcampbell
Copy link
Member

Oh, that's not ideal. Do you think this is severe enough to warrant a rollback of #2114? @Danielku15, do you have thoughts on how we might go about resolving this?

@stevehipwell
Copy link
Contributor Author

@kfcampbell from our side it wasted a couple of hours and then required a rollback to v6.0.0. I assume it could be fixed forward but if not it'd probably warrant a rollback. I think it'd be worth getting a regression test in either way.

@Danielku15
Copy link
Contributor

Danielku15 commented Mar 5, 2024

I'll try to have a look tomorrow (its already 11pm here) 😅 From my direct changes I cannot directly infer what would trigger this. My new conditions mainly target the allowed_actions="selected". I assume they trigger some unintended else-path. Should be easy to fix once I narrowed down the exact path.

Edit: The TestAccGithubActionsRepositoryPermissions/test_disabling_actions_on_a_repository suite also fails already on my PC with this error. Seems tests are not fully running on GitHub Actions / PullRequests?

@kfcampbell Would it make sense to setup there a more continuous acceptance testing to catch those errors? https://github.com/integrations/terraform-provider-github/actions/runs/8144672944/job/22259269917#step:8:7

Found the error and will work on a fix. The problematic code is the resource import / initial read logic where terraform does not provide access to the desired state defined in code and we try to load the actions always:

if (allowedActions == "selected" && len(allowedActionsConfig) > 0) || allowedActions == "" {
actionsAllowed, _, err := client.Repositories.GetActionsAllowed(ctx, owner, repoName)
if err != nil {

Edit 2: Fix is ready at #2186

@kfcampbell
Copy link
Member

@Danielku15 thanks for the attention here! You're correct that integration tests are not automatically running on PRs and merges; I've opened #1414 to track this but we haven't gotten the time or priority to work on it unfortunately. We've struggled with the expense, time, and hassle of automatically testing resources and we're certainly in an unhealthy position now. In the meantime, I've been running relevant tests manually, which is not ideal.

I appreciate you opening the PR, I'll look shortly!

@github-project-automation github-project-automation bot moved this from 🔥 Backlog to ✅ Done in 🧰 Octokit Active Mar 8, 2024
@stevehipwell
Copy link
Contributor Author

@kfcampbell it looks like the v6.1.0 release hasn't made it to the registry yet? Is this expected?

@kfcampbell
Copy link
Member

It's definitely not. Let's keep future discussion about that issue on #2191.

@stevehipwell
Copy link
Contributor Author

#2191 wasn't created when I added this comment, but I saw it was opened and am following it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Up for grabs Issues that are ready to be worked on by anyone Type: Bug Something isn't working as documented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants