From 419fcfebf45918541d5c48d0f91998637844cf21 Mon Sep 17 00:00:00 2001 From: Danielku15 Date: Mon, 19 Feb 2024 09:59:46 +0100 Subject: [PATCH] fix: Use desired state for determining actions config --- ...github_actions_organization_permissions.go | 26 ++++++--- ...b_actions_organization_permissions_test.go | 43 ++++++++++++++ ...e_github_actions_repository_permissions.go | 28 ++++++---- ...hub_actions_repository_permissions_test.go | 56 +++++++++++++++++++ 4 files changed, 135 insertions(+), 18 deletions(-) diff --git a/github/resource_github_actions_organization_permissions.go b/github/resource_github_actions_organization_permissions.go index e635c16724..c79320223d 100644 --- a/github/resource_github_actions_organization_permissions.go +++ b/github/resource_github_actions_organization_permissions.go @@ -3,6 +3,7 @@ package github import ( "context" "errors" + "log" "github.com/google/go-github/v57/github" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -106,8 +107,7 @@ func resourceGithubActionsOrganizationAllowedObject(d *schema.ResourceData) (*gi allowed.PatternsAllowed = patternsAllowed } else { - return &github.ActionsAllowed{}, - errors.New("the allowed_actions_config {} block must be specified if allowed_actions == 'selected'") + return nil, nil } return allowed, nil @@ -162,11 +162,16 @@ func resourceGithubActionsOrganizationPermissionsCreateOrUpdate(d *schema.Resour if err != nil { return err } - _, _, err = client.Actions.EditActionsAllowed(ctx, - orgName, - *actionsAllowedData) - if err != nil { - return err + if actionsAllowedData != nil { + log.Printf("[DEBUG] Allowed actions config is set") + _, _, err = client.Actions.EditActionsAllowed(ctx, + orgName, + *actionsAllowedData) + if err != nil { + return err + } + } else { + log.Printf("[DEBUG] Allowed actions config not set, skipping") } } @@ -201,7 +206,12 @@ func resourceGithubActionsOrganizationPermissionsRead(d *schema.ResourceData, me return err } - if actionsPermissions.GetAllowedActions() == "selected" { + // only load and fill allowed_actions_config if allowed_actions_config is also set + // in the TF code. (see #2105) + // on initial import there might not be any value in the state, then we have to import the data + allowedActions := d.Get("allowed_actions").(string) + allowedActionsConfig := d.Get("allowed_actions_config").([]interface{}) + if (allowedActions == "selected" && len(allowedActionsConfig) > 0) || allowedActions == "" { actionsAllowed, _, err := client.Actions.GetActionsAllowed(ctx, d.Id()) if err != nil { return err diff --git a/github/resource_github_actions_organization_permissions_test.go b/github/resource_github_actions_organization_permissions_test.go index 50681ffa53..97a01f1719 100644 --- a/github/resource_github_actions_organization_permissions_test.go +++ b/github/resource_github_actions_organization_permissions_test.go @@ -166,6 +166,49 @@ func TestAccGithubActionsOrganizationPermissions(t *testing.T) { }) }) + t.Run("test not setting of organization allowed actions without error", func(t *testing.T) { + + allowedActions := "selected" + enabledRepositories := "all" + + config := fmt.Sprintf(` + + resource "github_actions_organization_permissions" "test" { + allowed_actions = "%s" + enabled_repositories = "%s" + } + `, allowedActions, enabledRepositories) + + check := resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "github_actions_organization_permissions.test", "allowed_actions", allowedActions, + ), + resource.TestCheckResourceAttr( + "github_actions_organization_permissions.test", "enabled_repositories", enabledRepositories, + ), + resource.TestCheckResourceAttr( + "github_actions_organization_permissions.test", "allowed_actions_config.#", "0", + ), + ) + + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: config, + Check: check, + }, + }, + }) + } + + t.Run("with an organization account", func(t *testing.T) { + testCase(t, organization) + }) + }) + t.Run("test setting of organization enabled repositories", func(t *testing.T) { allowedActions := "all" diff --git a/github/resource_github_actions_repository_permissions.go b/github/resource_github_actions_repository_permissions.go index 2875873d77..94392bbe08 100644 --- a/github/resource_github_actions_repository_permissions.go +++ b/github/resource_github_actions_repository_permissions.go @@ -2,7 +2,6 @@ package github import ( "context" - "errors" "log" "github.com/google/go-github/v57/github" @@ -97,8 +96,7 @@ func resourceGithubActionsRepositoryAllowedObject(d *schema.ResourceData) (*gith allowed.PatternsAllowed = patternsAllowed } else { - return &github.ActionsAllowed{}, - errors.New("the allowed_actions_config {} block must be specified if allowed_actions == 'selected'") + return nil, nil } return allowed, nil @@ -141,12 +139,17 @@ func resourceGithubActionsRepositoryPermissionsCreateOrUpdate(d *schema.Resource if err != nil { return err } - _, _, err = client.Repositories.EditActionsAllowed(ctx, - owner, - repoName, - *actionsAllowedData) - if err != nil { - return err + if actionsAllowedData != nil { + log.Printf("[DEBUG] Allowed actions config is set") + _, _, err = client.Repositories.EditActionsAllowed(ctx, + owner, + repoName, + *actionsAllowedData) + if err != nil { + return err + } + } else { + log.Printf("[DEBUG] Allowed actions config not set, skipping") } } @@ -166,7 +169,12 @@ func resourceGithubActionsRepositoryPermissionsRead(d *schema.ResourceData, meta return err } - if actionsPermissions.GetAllowedActions() == "selected" { + // only load and fill allowed_actions_config if allowed_actions_config is also set + // in the TF code. (see #2105) + // on initial import there might not be any value in the state, then we have to import the data + allowedActions := d.Get("allowed_actions").(string) + allowedActionsConfig := d.Get("allowed_actions_config").([]interface{}) + if (allowedActions == "selected" && len(allowedActionsConfig) > 0) || allowedActions == "" { actionsAllowed, _, err := client.Repositories.GetActionsAllowed(ctx, owner, repoName) if err != nil { return err diff --git a/github/resource_github_actions_repository_permissions_test.go b/github/resource_github_actions_repository_permissions_test.go index 0b80569e03..5cb3a03188 100644 --- a/github/resource_github_actions_repository_permissions_test.go +++ b/github/resource_github_actions_repository_permissions_test.go @@ -188,6 +188,62 @@ func TestAccGithubActionsRepositoryPermissions(t *testing.T) { }) + t.Run("test not setting of repository allowed actions without error", func(t *testing.T) { + + allowedActions := "selected" + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + + config := fmt.Sprintf(` + resource "github_repository" "test" { + name = "tf-acc-test-topic-%[1]s" + description = "Terraform acceptance tests %[1]s" + topics = ["terraform", "testing"] + } + + resource "github_actions_repository_permissions" "test" { + allowed_actions = "%s" + repository = github_repository.test.name + } + `, randomID, allowedActions) + + check := resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "github_actions_repository_permissions.test", "allowed_actions", allowedActions, + ), + // Even if we do not set the allowed_actions_config, + // it will be set to the organization level settings + resource.TestCheckResourceAttr( + "github_actions_repository_permissions.test", "allowed_actions_config.#", "0", + ), + ) + + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: config, + Check: check, + }, + }, + }) + } + + t.Run("with an anonymous account", func(t *testing.T) { + t.Skip("anonymous account not supported for this operation") + }) + + t.Run("with an individual account", func(t *testing.T) { + testCase(t, individual) + }) + + t.Run("with an organization account", func(t *testing.T) { + testCase(t, organization) + }) + + }) + t.Run("test disabling actions on a repository", func(t *testing.T) { actionsEnabled := false