Merge pull request integrations#134 from terraform-providers/b-org-we…

…bhook-secret resource/github_organization_webhook: Avoid spurious diff
kfcampbell · Aug 16, 2018 · 0ef7a69 · 0ef7a69
2 parents 336a60e + df2d5d4
commit 0ef7a69
Show file tree

Hide file tree

Showing 6 changed files with 109 additions and 51 deletions.
diff --git a/github/migrate_github_repository_webhook.go b/github/migrate_github_repository_webhook.go
@@ -8,23 +8,23 @@ import (
 	"github.com/hashicorp/terraform/terraform"
 )
 
-func resourceGithubRepositoryWebhookMigrateState(v int, is *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) {
+func resourceGithubWebhookMigrateState(v int, is *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) {
 	switch v {
 	case 0:
-		log.Println("[INFO] Found GitHub Repository Webhook State v0; migrating to v1")
-		return migrateGithubRepositoryWebhookStateV0toV1(is)
+		log.Println("[INFO] Found GitHub Webhook State v0; migrating to v1")
+		return migrateGithubWebhookStateV0toV1(is)
 	default:
 		return is, fmt.Errorf("Unexpected schema version: %d", v)
 	}
 }
 
-func migrateGithubRepositoryWebhookStateV0toV1(is *terraform.InstanceState) (*terraform.InstanceState, error) {
+func migrateGithubWebhookStateV0toV1(is *terraform.InstanceState) (*terraform.InstanceState, error) {
 	if is.Empty() {
 		log.Println("[DEBUG] Empty InstanceState; nothing to migrate.")
 		return is, nil
 	}
 
-	log.Printf("[DEBUG] GitHub Repository Webhook Attributes before migration: %#v", is.Attributes)
+	log.Printf("[DEBUG] GitHub Webhook Attributes before migration: %#v", is.Attributes)
 
 	prefix := "configuration."
 
@@ -49,7 +49,7 @@ func migrateGithubRepositoryWebhookStateV0toV1(is *terraform.InstanceState) (*te
 
 	is.Attributes[prefix+"#"] = "1"
 
-	log.Printf("[DEBUG] GitHub Repository Webhook Attributes after State Migration: %#v", is.Attributes)
+	log.Printf("[DEBUG] GitHub Webhook Attributes after State Migration: %#v", is.Attributes)
 
 	return is, nil
 }
diff --git a/github/migrate_github_repository_webhook_test.go b/github/migrate_github_repository_webhook_test.go
@@ -7,7 +7,7 @@ import (
 	"github.com/hashicorp/terraform/terraform"
 )
 
-func TestMigrateGithubRepositoryWebhookStateV0toV1(t *testing.T) {
+func TestMigrateGithubWebhookStateV0toV1(t *testing.T) {
 	oldAttributes := map[string]string{
 		"configuration.%":            "4",
 		"configuration.content_type": "form",
@@ -16,7 +16,7 @@ func TestMigrateGithubRepositoryWebhookStateV0toV1(t *testing.T) {
 		"configuration.url":          "https://google.co.uk/",
 	}
 
-	newState, err := migrateGithubRepositoryWebhookStateV0toV1(&terraform.InstanceState{
+	newState, err := migrateGithubWebhookStateV0toV1(&terraform.InstanceState{
 		ID:         "nonempty",
 		Attributes: oldAttributes,
 	})

diff --git a/github/resource_github_organization_webhook.go b/github/resource_github_organization_webhook.go
@@ -18,6 +18,9 @@ func resourceGithubOrganizationWebhook() *schema.Resource {
 		Update: resourceGithubOrganizationWebhookUpdate,
 		Delete: resourceGithubOrganizationWebhookDelete,
 
+		SchemaVersion: 1,
+		MigrateState:  resourceGithubWebhookMigrateState,
+
 		Schema: map[string]*schema.Schema{
 			"name": {
 				Type:         schema.TypeString,
@@ -31,10 +34,7 @@ func resourceGithubOrganizationWebhook() *schema.Resource {
 				Elem:     &schema.Schema{Type: schema.TypeString},
 				Set:      schema.HashString,
 			},
-			"configuration": {
-				Type:     schema.TypeMap,
-				Optional: true,
-			},
+			"configuration": webhookConfigurationSchema(),
 			"url": {
 				Type:     schema.TypeString,
 				Computed: true,
@@ -62,13 +62,19 @@ func resourceGithubOrganizationWebhookObject(d *schema.ResourceData) *github.Hoo
 		events = append(events, v.(string))
 	}
 
-	return &github.Hook{
+	hook := &github.Hook{
 		Name:   github.String(d.Get("name").(string)),
 		URL:    github.String(d.Get("url").(string)),
 		Events: events,
 		Active: github.Bool(d.Get("active").(bool)),
-		Config: d.Get("configuration").(map[string]interface{}),
 	}
+
+	config := d.Get("configuration").([]interface{})
+	if len(config) > 0 {
+		hook.Config = config[0].(map[string]interface{})
+	}
+
+	return hook
 }
 
 func resourceGithubOrganizationWebhookCreate(d *schema.ResourceData, meta interface{}) error {
@@ -105,7 +111,7 @@ func resourceGithubOrganizationWebhookRead(d *schema.ResourceData, meta interfac
 	d.Set("url", hook.URL)
 	d.Set("active", hook.Active)
 	d.Set("events", hook.Events)
-	d.Set("configuration", hook.Config)
+	d.Set("configuration", []interface{}{hook.Config})
 
 	return nil
 }

diff --git a/github/resource_github_organization_webhook_test.go b/github/resource_github_organization_webhook_test.go
@@ -57,6 +57,35 @@ func TestAccGithubOrganizationWebhook_basic(t *testing.T) {
 	})
 }
 
+func TestAccGithubOrganizationWebhook_secret(t *testing.T) {
+	var hook github.Hook
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckGithubOrganizationWebhookDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccGithubOrganizationWebhookConfig_secret,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckGithubOrganizationWebhookExists("github_organization_webhook.foo", &hook),
+					testAccCheckGithubOrganizationWebhookAttributes(&hook, &testAccGithubOrganizationWebhookExpectedAttributes{
+						Name:   "web",
+						Events: []string{"pull_request"},
+						Configuration: map[string]interface{}{
+							"url":          "https://www.terraform.io/webhook",
+							"content_type": "json",
+							"secret":       "********",
+							"insecure_ssl": "0",
+						},
+						Active: true,
+					}),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckGithubOrganizationWebhookExists(n string, hook *github.Hook) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[n]
@@ -167,3 +196,17 @@ resource "github_organization_webhook" "foo" {
   events = ["issues"]
 }
 `
+
+const testAccGithubOrganizationWebhookConfig_secret = `
+resource "github_organization_webhook" "foo" {
+  name = "web"
+  configuration {
+    url          = "https://www.terraform.io/webhook"
+    content_type = "json"
+    secret       = "VerySecret"
+    insecure_ssl = false
+  }
+
+  events = ["pull_request"]
+}
+`
diff --git a/github/resource_github_repository_webhook.go b/github/resource_github_repository_webhook.go
@@ -29,7 +29,7 @@ func resourceGithubRepositoryWebhook() *schema.Resource {
 		},
 
 		SchemaVersion: 1,
-		MigrateState:  resourceGithubRepositoryWebhookMigrateState,
+		MigrateState:  resourceGithubWebhookMigrateState,
 
 		Schema: map[string]*schema.Schema{
 			"name": {
@@ -48,41 +48,7 @@ func resourceGithubRepositoryWebhook() *schema.Resource {
 				Elem:     &schema.Schema{Type: schema.TypeString},
 				Set:      schema.HashString,
 			},
-			"configuration": {
-				Type:     schema.TypeList,
-				MaxItems: 1,
-				Optional: true,
-				Elem: &schema.Resource{
-					Schema: map[string]*schema.Schema{
-						"url": {
-							Type:     schema.TypeString,
-							Required: true,
-						},
-						"content_type": {
-							Type:     schema.TypeString,
-							Optional: true,
-						},
-						"secret": {
-							Type:      schema.TypeString,
-							Optional:  true,
-							Sensitive: true,
-							DiffSuppressFunc: func(k, oldV, newV string, d *schema.ResourceData) bool {
-								// Undocumented GitHub feature where API returns 8 asterisks in place of the secret
-								maskedSecret := "********"
-								if oldV == maskedSecret {
-									return true
-								}
-
-								return oldV == newV
-							},
-						},
-						"insecure_ssl": {
-							Type:     schema.TypeString,
-							Optional: true,
-						},
-					},
-				},
-			},
+			"configuration": webhookConfigurationSchema(),
 			"url": {
 				Type:     schema.TypeString,
 				Computed: true,

diff --git a/github/schema_webhook_configuration.go b/github/schema_webhook_configuration.go
@@ -0,0 +1,43 @@
+package github
+
+import (
+	"github.com/hashicorp/terraform/helper/schema"
+)
+
+func webhookConfigurationSchema() *schema.Schema {
+	return &schema.Schema{
+		Type:     schema.TypeList,
+		MaxItems: 1,
+		Optional: true,
+		Elem: &schema.Resource{
+			Schema: map[string]*schema.Schema{
+				"url": {
+					Type:     schema.TypeString,
+					Required: true,
+				},
+				"content_type": {
+					Type:     schema.TypeString,
+					Optional: true,
+				},
+				"secret": {
+					Type:      schema.TypeString,
+					Optional:  true,
+					Sensitive: true,
+					DiffSuppressFunc: func(k, oldV, newV string, d *schema.ResourceData) bool {
+						// Undocumented GitHub feature where API returns 8 asterisks in place of the secret
+						maskedSecret := "********"
+						if oldV == maskedSecret {
+							return true
+						}
+
+						return oldV == newV
+					},
+				},
+				"insecure_ssl": {
+					Type:     schema.TypeString,
+					Optional: true,
+				},
+			},
+		},
+	}
+}