From 87f6087c6d564eaa0cdfdfd86d3cf5ec44411aa2 Mon Sep 17 00:00:00 2001 From: Steve Hipwell Date: Thu, 17 Oct 2024 12:50:23 +0100 Subject: [PATCH] fix: Fixed issue labels adoption Signed-off-by: Steve Hipwell --- github/data_source_github_issue_labels.go | 50 +------ github/resource_github_issue_labels.go | 155 +++++++------------- github/resource_github_issue_labels_test.go | 73 ++++++--- github/util_labels.go | 52 +++++++ 4 files changed, 166 insertions(+), 164 deletions(-) create mode 100644 github/util_labels.go diff --git a/github/data_source_github_issue_labels.go b/github/data_source_github_issue_labels.go index 1535ceb38d..2f3c1b3713 100644 --- a/github/data_source_github_issue_labels.go +++ b/github/data_source_github_issue_labels.go @@ -2,9 +2,7 @@ package github import ( "context" - "fmt" - "github.com/google/go-github/v65/github" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) @@ -49,59 +47,19 @@ func dataSourceGithubIssueLabelsRead(d *schema.ResourceData, meta interface{}) e client := meta.(*Owner).v3client owner := meta.(*Owner).name repository := d.Get("repository").(string) - ctx := context.Background() - opts := &github.ListOptions{ - PerPage: maxPerPage, - } d.SetId(repository) - allLabels := make([]interface{}, 0) - for { - labels, resp, err := client.Issues.ListLabels(ctx, owner, repository, opts) - if err != nil { - return err - } - - result, err := flattenLabels(labels) - if err != nil { - return fmt.Errorf("unable to flatten GitHub Labels (Owner: %q/Repository: %q) : %+v", owner, repository, err) - } - - allLabels = append(allLabels, result...) - - if resp.NextPage == 0 { - break - } - opts.Page = resp.NextPage + labels, err := listLabels(client, ctx, owner, repository) + if err != nil { + return err } - err := d.Set("labels", allLabels) + err = d.Set("labels", flattenLabels(labels)) if err != nil { return err } return nil } - -func flattenLabels(labels []*github.Label) ([]interface{}, error) { - if labels == nil { - return make([]interface{}, 0), nil - } - - results := make([]interface{}, 0) - - for _, l := range labels { - result := make(map[string]interface{}) - - result["name"] = l.GetName() - result["color"] = l.GetColor() - result["description"] = l.GetDescription() - result["url"] = l.GetURL() - - results = append(results, result) - } - - return results, nil -} diff --git a/github/resource_github_issue_labels.go b/github/resource_github_issue_labels.go index 93d2919652..9ba5b29ac4 100644 --- a/github/resource_github_issue_labels.go +++ b/github/resource_github_issue_labels.go @@ -2,8 +2,8 @@ package github import ( "context" + "fmt" "log" - "strings" "github.com/google/go-github/v65/github" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -61,49 +61,23 @@ func resourceGithubIssueLabels() *schema.Resource { func resourceGithubIssueLabelsRead(d *schema.ResourceData, meta interface{}) error { client := meta.(*Owner).v3client - owner := meta.(*Owner).name repository := d.Id() - - log.Printf("[DEBUG] Reading GitHub issue labels for %s/%s", owner, repository) - ctx := context.WithValue(context.Background(), ctxId, repository) - options := &github.ListOptions{ - PerPage: maxPerPage, - } - - labels := make([]map[string]interface{}, 0) - - for { - ls, resp, err := client.Issues.ListLabels(ctx, owner, repository, options) - if err != nil { - return err - } - for _, l := range ls { - labels = append(labels, map[string]interface{}{ - "name": l.GetName(), - "color": l.GetColor(), - "description": l.GetDescription(), - "url": l.GetURL(), - }) - } + log.Printf("[DEBUG] Reading GitHub issue labels for %s/%s", owner, repository) - if resp.NextPage == 0 { - break - } - options.Page = resp.NextPage + labels, err := listLabels(client, ctx, owner, repository) + if err != nil { + return err } - log.Printf("[DEBUG] Found %d GitHub issue labels for %s/%s", len(labels), owner, repository) - log.Printf("[DEBUG] Labels: %v", labels) - - err := d.Set("repository", repository) + err = d.Set("repository", repository) if err != nil { return err } - err = d.Set("label", labels) + err = d.Set("label", flattenLabels(labels)) if err != nil { return err } @@ -113,97 +87,82 @@ func resourceGithubIssueLabelsRead(d *schema.ResourceData, meta interface{}) err func resourceGithubIssueLabelsCreateOrUpdate(d *schema.ResourceData, meta interface{}) error { client := meta.(*Owner).v3client - owner := meta.(*Owner).name repository := d.Get("repository").(string) ctx := context.WithValue(context.Background(), ctxId, repository) - o, n := d.GetChange("label") + wantLabels := d.Get("label").(*schema.Set).List() - log.Printf("[DEBUG] Updating GitHub issue labels for %s/%s", owner, repository) - log.Printf("[DEBUG] Old labels: %v", o) - log.Printf("[DEBUG] New labels: %v", n) - - oMap := make(map[string]map[string]interface{}) - nMap := make(map[string]map[string]interface{}) - for _, raw := range o.(*schema.Set).List() { - m := raw.(map[string]interface{}) - name := strings.ToLower(m["name"].(string)) - oMap[name] = m + wantLabelsMap := make(map[string]interface{}, len(wantLabels)) + for _, label := range wantLabels { + name := label.(map[string]interface{})["name"].(string) + if _, found := wantLabelsMap[name]; found { + return fmt.Errorf("duplicate set label: %s", name) + } + wantLabelsMap[name] = label } - for _, raw := range n.(*schema.Set).List() { - m := raw.(map[string]interface{}) - name := strings.ToLower(m["name"].(string)) - nMap[name] = m + + hasLabels, err := listLabels(client, ctx, owner, repository) + if err != nil { + return err } - labels := make([]map[string]interface{}, 0) + log.Printf("[DEBUG] Updating GitHub issue labels for %s/%s", owner, repository) - // create - for name, n := range nMap { - if _, ok := oMap[name]; !ok { - log.Printf("[DEBUG] Creating GitHub issue label %s/%s/%s", owner, repository, name) + hasLabelsMap := make(map[string]struct{}, len(hasLabels)) + for _, hasLabel := range hasLabels { + name := hasLabel.GetName() + wantLabel, found := wantLabelsMap[name] + if found { + labelData := wantLabel.(map[string]interface{}) + description := labelData["description"].(string) + color := labelData["color"].(string) + if hasLabel.GetDescription() != description || hasLabel.GetColor() != color { + log.Printf("[DEBUG] Updating GitHub issue label %s/%s/%s", owner, repository, name) - label, _, err := client.Issues.CreateLabel(ctx, owner, repository, &github.Label{ - Name: github.String(n["name"].(string)), - Color: github.String(n["color"].(string)), - Description: github.String(n["description"].(string)), - }) - if err != nil { - return err + _, _, err := client.Issues.EditLabel(ctx, owner, repository, name, &github.Label{ + Name: github.String(name), + Description: github.String(description), + Color: github.String(color), + }) + if err != nil { + return err + } } - - labels = append(labels, map[string]interface{}{ - "name": label.GetName(), - "color": label.GetColor(), - "description": label.GetDescription(), - "url": label.GetURL(), - }) - } - } - - // delete - for name, o := range oMap { - if _, ok := nMap[name]; !ok { + } else { log.Printf("[DEBUG] Deleting GitHub issue label %s/%s/%s", owner, repository, name) - _, err := client.Issues.DeleteLabel(ctx, owner, repository, o["name"].(string)) + _, err := client.Issues.DeleteLabel(ctx, owner, repository, name) if err != nil { return err } } + + hasLabelsMap[name] = struct{}{} } - // update - for name, n := range nMap { - if o, ok := oMap[name]; ok { - if o["name"] != n["name"] || o["color"] != n["color"] || o["description"] != n["description"] { - log.Printf("[DEBUG] Updating GitHub issue label %s/%s/%s", owner, repository, name) + for _, l := range wantLabels { + labelData := l.(map[string]interface{}) + name := labelData["name"].(string) - label, _, err := client.Issues.EditLabel(ctx, owner, repository, name, &github.Label{ - Name: github.String(n["name"].(string)), - Color: github.String(n["color"].(string)), - Description: github.String(n["description"].(string)), - }) - if err != nil { - return err - } + _, found := hasLabelsMap[name] + if !found { + log.Printf("[DEBUG] Creating GitHub issue label %s/%s/%s", owner, repository, name) - labels = append(labels, map[string]interface{}{ - "name": label.GetName(), - "color": label.GetColor(), - "description": label.GetDescription(), - "url": label.GetURL(), - }) - } else { - labels = append(labels, o) + _, _, err := client.Issues.CreateLabel(ctx, owner, repository, &github.Label{ + Name: github.String(name), + Description: github.String(labelData["description"].(string)), + Color: github.String(labelData["color"].(string)), + }) + if err != nil { + return err } } } d.SetId(repository) - err := d.Set("label", labels) + err = d.Set("label", wantLabels) if err != nil { return err } @@ -213,7 +172,6 @@ func resourceGithubIssueLabelsCreateOrUpdate(d *schema.ResourceData, meta interf func resourceGithubIssueLabelsDelete(d *schema.ResourceData, meta interface{}) error { client := meta.(*Owner).v3client - owner := meta.(*Owner).name repository := d.Get("repository").(string) ctx := context.WithValue(context.Background(), ctxId, repository) @@ -221,7 +179,6 @@ func resourceGithubIssueLabelsDelete(d *schema.ResourceData, meta interface{}) e labels := d.Get("label").(*schema.Set).List() log.Printf("[DEBUG] Deleting GitHub issue labels for %s/%s", owner, repository) - log.Printf("[DEBUG] Labels: %v", labels) // delete for _, raw := range labels { diff --git a/github/resource_github_issue_labels_test.go b/github/resource_github_issue_labels_test.go index d37efd7395..251ff951e0 100644 --- a/github/resource_github_issue_labels_test.go +++ b/github/resource_github_issue_labels_test.go @@ -1,16 +1,19 @@ package github import ( + "context" "fmt" "testing" + "github.com/google/go-github/v65/github" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" ) func TestAccGithubIssueLabels(t *testing.T) { t.Run("authoritatively overtakes existing labels", func(t *testing.T) { - randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + repoName := fmt.Sprintf("tf-acc-test-%s", acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum)) + existingLabelName := fmt.Sprintf("label-%s", acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum)) empty := []map[string]interface{}{} testCase := func(t *testing.T, mode string) { @@ -18,37 +21,58 @@ func TestAccGithubIssueLabels(t *testing.T) { PreCheck: func() { skipUnlessMode(t, mode) }, Providers: testAccProviders, Steps: []resource.TestStep{ - // 0. Check if some labels already exist (indicated by non-empty plan) + // 0. Create the repository { - Config: testAccGithubIssueLabelsConfig(randomID, empty), + Config: testAccGithubIssueLabelsConfig(repoName, nil), ExpectNonEmptyPlan: true, }, - // 1. Check if all the labels are destroyed when the resource is added + // 1. Check if some labels already exist (indicated by non-empty plan) { - Config: testAccGithubIssueLabelsConfig(randomID, empty), + PreConfig: func() { + err := testAccGithubIssueLabelsAddLabel(repoName, existingLabelName) + if err != nil { + t.Fatalf("failed to add label: %s", existingLabelName) + } + }, + Config: testAccGithubIssueLabelsConfig(repoName, empty), + PlanOnly: true, + ExpectNonEmptyPlan: true, + }, + // 2. Check if existing labels can be adopted + { + Config: testAccGithubIssueLabelsConfig(repoName, append(empty, map[string]interface{}{ + "name": existingLabelName, + "color": "000000", + "description": "Test label", + })), + Check: resource.TestCheckResourceAttr("github_issue_labels.test", "label.#", "1"), + }, + // 3. Check if all the labels are destroyed when the resource has no labels + { + Config: testAccGithubIssueLabelsConfig(repoName, empty), Check: resource.TestCheckResourceAttr("github_issue_labels.test", "label.#", "0"), }, - // 2. Check if a label can be created + // 4. Check if a new label can be created { - Config: testAccGithubIssueLabelsConfig(randomID, append(empty, map[string]interface{}{ + Config: testAccGithubIssueLabelsConfig(repoName, append(empty, map[string]interface{}{ "name": "foo", "color": "000000", "description": "foo", })), Check: resource.TestCheckResourceAttr("github_issue_labels.test", "label.#", "1"), }, - // 3. Check if a label can be recreated + // 5. Check if a label can be recreated { - Config: testAccGithubIssueLabelsConfig(randomID, append(empty, map[string]interface{}{ + Config: testAccGithubIssueLabelsConfig(repoName, append(empty, map[string]interface{}{ "name": "Foo", "color": "000000", "description": "foo", })), Check: resource.TestCheckResourceAttr("github_issue_labels.test", "label.#", "1"), }, - // 4. Check if multiple labels can be created + // 6. Check if multiple labels can be created { - Config: testAccGithubIssueLabelsConfig(randomID, append(empty, + Config: testAccGithubIssueLabelsConfig(repoName, append(empty, map[string]interface{}{ "name": "Foo", "color": "000000", @@ -65,14 +89,16 @@ func TestAccGithubIssueLabels(t *testing.T) { })), Check: resource.TestCheckResourceAttr("github_issue_labels.test", "label.#", "3"), }, - // 5. Check if labels can be destroyed + // 7. Check if labels can be destroyed { - Config: testAccGithubIssueLabelsConfig(randomID, nil), + Config: testAccGithubIssueLabelsConfig(repoName, nil), + ExpectNonEmptyPlan: true, }, - // 6. Check if labels were actually destroyed + // 8. Check if labels were actually destroyed { - Config: testAccGithubIssueLabelsConfig(randomID, empty), - Check: resource.TestCheckResourceAttr("github_issue_labels.test", "label.#", "0"), + Config: testAccGithubIssueLabelsConfig(repoName, empty), + PlanOnly: true, + ExpectNonEmptyPlan: true, }, }, }) @@ -92,7 +118,7 @@ func TestAccGithubIssueLabels(t *testing.T) { }) } -func testAccGithubIssueLabelsConfig(randomId string, labels []map[string]interface{}) string { +func testAccGithubIssueLabelsConfig(repoName string, labels []map[string]interface{}) string { resource := "" if labels != nil { dynamic := "" @@ -117,10 +143,19 @@ func testAccGithubIssueLabelsConfig(randomId string, labels []map[string]interfa return fmt.Sprintf(` resource "github_repository" "test" { - name = "tf-acc-test-%s" + name = "%s" auto_init = true } %s - `, randomId, resource) + `, repoName, resource) +} + +func testAccGithubIssueLabelsAddLabel(repository, label string) error { + client := testAccProvider.Meta().(*Owner).v3client + orgName := testAccProvider.Meta().(*Owner).name + ctx := context.TODO() + + _, _, err := client.Issues.CreateLabel(ctx, orgName, repository, &github.Label{Name: github.String(label)}) + return err } diff --git a/github/util_labels.go b/github/util_labels.go new file mode 100644 index 0000000000..2755aef7c5 --- /dev/null +++ b/github/util_labels.go @@ -0,0 +1,52 @@ +package github + +import ( + "context" + + "github.com/google/go-github/v65/github" +) + +func flattenLabels(labels []*github.Label) []interface{} { + if labels == nil { + return make([]interface{}, 0) + } + + results := make([]interface{}, 0) + + for _, l := range labels { + result := make(map[string]interface{}) + + result["name"] = l.GetName() + result["color"] = l.GetColor() + result["description"] = l.GetDescription() + result["url"] = l.GetURL() + + results = append(results, result) + } + + return results +} + +func listLabels(client *github.Client, ctx context.Context, owner, repository string) ([]*github.Label, error) { + options := &github.ListOptions{ + PerPage: maxPerPage, + } + + labels := make([]*github.Label, 0) + + for { + ls, resp, err := client.Issues.ListLabels(ctx, owner, repository, options) + if err != nil { + return nil, err + } + + labels = append(labels, ls...) + + if resp.NextPage == 0 { + break + } + options.Page = resp.NextPage + } + + return labels, nil +}