From 5114e71ff6c7fe0f022e802690d7dcdcaa2be58a Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Tue, 2 Jul 2019 08:37:47 -0500 Subject: [PATCH 1/2] update webhook to set value from configuration rather than whats returned from the api --- github/resource_github_organization_webhook.go | 14 ++++++++++++++ github/resource_github_repository_webhook.go | 14 ++++++++++++++ github/resource_github_repository_webhook_test.go | 9 +++++---- github/schema_webhook_configuration.go | 9 --------- website/docs/r/repository_webhook.html.markdown | 2 ++ 5 files changed, 35 insertions(+), 13 deletions(-) diff --git a/github/resource_github_organization_webhook.go b/github/resource_github_organization_webhook.go index 835d1f84dd..ed2e366bda 100644 --- a/github/resource_github_organization_webhook.go +++ b/github/resource_github_organization_webhook.go @@ -92,6 +92,11 @@ func resourceGithubOrganizationWebhookCreate(d *schema.ResourceData, meta interf } d.SetId(strconv.FormatInt(*hook.ID, 10)) + if hook.Config["secret"] != nil { + hook.Config["secret"] = webhookObj.Config["secret"] + } + d.Set("configuration", []interface{}{hook.Config}) + return resourceGithubOrganizationWebhookRead(d, meta) } @@ -134,6 +139,15 @@ func resourceGithubOrganizationWebhookRead(d *schema.ResourceData, meta interfac d.Set("url", hook.URL) d.Set("active", hook.Active) d.Set("events", hook.Events) + + 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 diff --git a/github/resource_github_repository_webhook.go b/github/resource_github_repository_webhook.go index d45b9a632c..6ab3db5174 100644 --- a/github/resource_github_repository_webhook.go +++ b/github/resource_github_repository_webhook.go @@ -111,6 +111,11 @@ func resourceGithubRepositoryWebhookCreate(d *schema.ResourceData, meta interfac } d.SetId(strconv.FormatInt(*hook.ID, 10)) + if hook.Config["secret"] != nil { + hook.Config["secret"] = hk.Config["secret"] + } + d.Set("configuration", []interface{}{hook.Config}) + return resourceGithubRepositoryWebhookRead(d, meta) } @@ -152,6 +157,15 @@ func resourceGithubRepositoryWebhookRead(d *schema.ResourceData, meta interface{ d.Set("url", hook.URL) d.Set("active", hook.Active) d.Set("events", hook.Events) + + 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 diff --git a/github/resource_github_repository_webhook_test.go b/github/resource_github_repository_webhook_test.go index 387e3eb269..0b33c5fe4d 100644 --- a/github/resource_github_repository_webhook_test.go +++ b/github/resource_github_repository_webhook_test.go @@ -119,10 +119,11 @@ func TestAccGithubRepositoryWebhook_importSecret(t *testing.T) { Config: testAccGithubRepositoryWebhookConfig_secret(randString), }, { - 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 }, }, }) diff --git a/github/schema_webhook_configuration.go b/github/schema_webhook_configuration.go index 98c602609c..bfe37d310c 100644 --- a/github/schema_webhook_configuration.go +++ b/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, diff --git a/website/docs/r/repository_webhook.html.markdown b/website/docs/r/repository_webhook.html.markdown index fbada58a6f..57aa1c27b1 100644 --- a/website/docs/r/repository_webhook.html.markdown +++ b/website/docs/r/repository_webhook.html.markdown @@ -68,3 +68,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 "********". \ No newline at end of file From ed9e44bbaa887132b3a4938c65ed9115c8613bc1 Mon Sep 17 00:00:00 2001 From: Megan Bang Date: Tue, 9 Jul 2019 15:31:36 -0500 Subject: [PATCH 2/2] review comments, add comments, add test to compare to state --- .../resource_github_organization_webhook.go | 7 +++++ ...source_github_organization_webhook_test.go | 28 +++++++++++-------- github/resource_github_repository_webhook.go | 7 +++++ ...resource_github_repository_webhook_test.go | 18 ++++++++++++ 4 files changed, 48 insertions(+), 12 deletions(-) diff --git a/github/resource_github_organization_webhook.go b/github/resource_github_organization_webhook.go index ed2e366bda..802fb09676 100644 --- a/github/resource_github_organization_webhook.go +++ b/github/resource_github_organization_webhook.go @@ -92,6 +92,9 @@ 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"] } @@ -140,6 +143,10 @@ func resourceGithubOrganizationWebhookRead(d *schema.ResourceData, meta interfac 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"] diff --git a/github/resource_github_organization_webhook_test.go b/github/resource_github_organization_webhook_test.go index 5803a0bd0e..fc55105e52 100644 --- a/github/resource_github_organization_webhook_test.go +++ b/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 diff --git a/github/resource_github_repository_webhook.go b/github/resource_github_repository_webhook.go index 6ab3db5174..cbdfe298ca 100644 --- a/github/resource_github_repository_webhook.go +++ b/github/resource_github_repository_webhook.go @@ -111,6 +111,9 @@ 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"] } @@ -158,6 +161,10 @@ func resourceGithubRepositoryWebhookRead(d *schema.ResourceData, meta interface{ 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"] diff --git a/github/resource_github_repository_webhook_test.go b/github/resource_github_repository_webhook_test.go index 0b33c5fe4d..c44110d971 100644 --- a/github/resource_github_repository_webhook_test.go +++ b/github/resource_github_repository_webhook_test.go @@ -117,6 +117,9 @@ 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", @@ -181,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