Merge pull request #251 from terraform-providers/b-webhook-secret

Bug Fix: Webhook secret stored incorrectly in state

github/resource_github_organization_webhook.go

@@ -92,6 +92,14 @@ func resourceGithubOrganizationWebhookCreate(d *schema.ResourceData, meta interf
 	}
 	d.SetId(strconv.FormatInt(*hook.ID, 10))
 
+	// GitHub returns the secret as a string of 8 astrisks "********"
+	// We would prefer to store the real secret in state, so we'll
+	// write the configuration secret in state from our request to GitHub
+	if hook.Config["secret"] != nil {
+		hook.Config["secret"] = webhookObj.Config["secret"]
+	}
+	d.Set("configuration", []interface{}{hook.Config})
+
 	return resourceGithubOrganizationWebhookRead(d, meta)
 }
 
@@ -134,6 +142,19 @@ func resourceGithubOrganizationWebhookRead(d *schema.ResourceData, meta interfac
 	d.Set("url", hook.URL)
 	d.Set("active", hook.Active)
 	d.Set("events", hook.Events)
+
+	// GitHub returns the secret as a string of 8 astrisks "********"
+	// We would prefer to store the real secret in state, so we'll
+	// write the configuration secret in state from what we get from
+	// ResourceData
+	if len(d.Get("configuration").([]interface{})) > 0 {
+		currentSecret := d.Get("configuration").([]interface{})[0].(map[string]interface{})["secret"]
+
+		if hook.Config["secret"] != nil {
+			hook.Config["secret"] = currentSecret
+		}
+	}
+
 	d.Set("configuration", []interface{}{hook.Config})
 
 	return nil

github/resource_github_organization_webhook_test.go

@@ -56,7 +56,6 @@ func TestAccGithubOrganizationWebhook_basic(t *testing.T) {
 }
 
 func TestAccGithubOrganizationWebhook_secret(t *testing.T) {
-	var hook github.Hook
 
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -66,17 +65,7 @@ func TestAccGithubOrganizationWebhook_secret(t *testing.T) {
 			{
 				Config: testAccGithubOrganizationWebhookConfig_secret,
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckGithubOrganizationWebhookExists("github_organization_webhook.foo", &hook),
-					testAccCheckGithubOrganizationWebhookAttributes(&hook, &testAccGithubOrganizationWebhookExpectedAttributes{
-						Events: []string{"pull_request"},
-						Configuration: map[string]interface{}{
-							"url":          "https://www.terraform.io/webhook",
-							"content_type": "json",
-							"secret":       "********",
-							"insecure_ssl": "false",
-						},
-						Active: true,
-					}),
+					testAccCheckGithubOrganizationWebhookSecret("github_organization_webhook.foo", "VerySecret"),
 				),
 			},
 		},
@@ -135,6 +124,21 @@ func testAccCheckGithubOrganizationWebhookAttributes(hook *github.Hook, want *te
 	}
 }
 
+func testAccCheckGithubOrganizationWebhookSecret(r, secret string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		rs, ok := s.RootModule().Resources[r]
+		if !ok {
+			return fmt.Errorf("Not Found: %s", r)
+		}
+
+		if rs.Primary.Attributes["configuration.0.secret"] != secret {
+			return fmt.Errorf("Configured secret in %s does not match secret in state.  (Expected: %s, Actual: %s)", r, secret, rs.Primary.Attributes["configuration.0.secret"])
+		}
+
+		return nil
+	}
+}
+
 func testAccCheckGithubOrganizationWebhookDestroy(s *terraform.State) error {
 	conn := testAccProvider.Meta().(*Organization).client
 	orgName := testAccProvider.Meta().(*Organization).name

github/resource_github_repository_webhook.go

@@ -111,6 +111,14 @@ func resourceGithubRepositoryWebhookCreate(d *schema.ResourceData, meta interfac
 	}
 	d.SetId(strconv.FormatInt(*hook.ID, 10))
 
+	// GitHub returns the secret as a string of 8 astrisks "********"
+	// We would prefer to store the real secret in state, so we'll
+	// write the configuration secret in state from our request to GitHub
+	if hook.Config["secret"] != nil {
+		hook.Config["secret"] = hk.Config["secret"]
+	}
+	d.Set("configuration", []interface{}{hook.Config})
+
 	return resourceGithubRepositoryWebhookRead(d, meta)
 }
 
@@ -152,6 +160,19 @@ func resourceGithubRepositoryWebhookRead(d *schema.ResourceData, meta interface{
 	d.Set("url", hook.URL)
 	d.Set("active", hook.Active)
 	d.Set("events", hook.Events)
+
+	// GitHub returns the secret as a string of 8 astrisks "********"
+	// We would prefer to store the real secret in state, so we'll
+	// write the configuration secret in state from what we get from
+	// ResourceData
+	if len(d.Get("configuration").([]interface{})) > 0 {
+		currentSecret := d.Get("configuration").([]interface{})[0].(map[string]interface{})["secret"]
+
+		if hook.Config["secret"] != nil {
+			hook.Config["secret"] = currentSecret
+		}
+	}
+
 	d.Set("configuration", []interface{}{hook.Config})
 
 	return nil

github/resource_github_repository_webhook_test.go

@@ -117,12 +117,16 @@ func TestAccGithubRepositoryWebhook_importSecret(t *testing.T) {
 		Steps: []resource.TestStep{
 			{
 				Config: testAccGithubRepositoryWebhookConfig_secret(randString),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckGithubRepositoryWebhookSecret("github_repository_webhook.foo", "RandomSecretString"),
+				),
 			},
 			{
-				ResourceName:        "github_repository_webhook.foo",
-				ImportState:         true,
-				ImportStateVerify:   true,
-				ImportStateIdPrefix: fmt.Sprintf("foo-%s/", randString),
+				ResourceName:            "github_repository_webhook.foo",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateIdPrefix:     fmt.Sprintf("foo-%s/", randString),
+				ImportStateVerifyIgnore: []string{"configuration.0.secret"}, // github does not allow a read of the actual secret
 			},
 		},
 	})
@@ -180,6 +184,21 @@ func testAccCheckGithubRepositoryWebhookAttributes(hook *github.Hook, want *test
 	}
 }
 
+func testAccCheckGithubRepositoryWebhookSecret(r, secret string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		rs, ok := s.RootModule().Resources[r]
+		if !ok {
+			return fmt.Errorf("Not Found: %s", r)
+		}
+
+		if rs.Primary.Attributes["configuration.0.secret"] != secret {
+			return fmt.Errorf("Configured secret in %s does not match secret in state.  (Expected: %s, Actual: %s)", r, secret, rs.Primary.Attributes["configuration.0.secret"])
+		}
+
+		return nil
+	}
+}
+
 func testAccCheckGithubRepositoryWebhookDestroy(s *terraform.State) error {
 	conn := testAccProvider.Meta().(*Organization).client
 	orgName := testAccProvider.Meta().(*Organization).name

github/schema_webhook_configuration.go

@@ -23,15 +23,6 @@ func webhookConfigurationSchema() *schema.Schema {
 					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,

website/docs/r/repository_webhook.html.markdown

@@ -72,3 +72,5 @@ Importing uses the name of the repository, as well as the ID of the webhook, e.g
 ```
 $ terraform import github_repository_webhook.terraform terraform/11235813
 ```
+
+If secret is populated in the webhook's configuration, the value will be imported as "********".