From 9afcb161262f61cd07df0e772c577bdb17670f69 Mon Sep 17 00:00:00 2001 From: Eli Skeggs <1348991+skeggse@users.noreply.github.com> Date: Wed, 18 Oct 2023 15:32:23 -0700 Subject: [PATCH 1/7] fix(tfe_policy): update query if changed --- internal/provider/resource_tfe_policy.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/provider/resource_tfe_policy.go b/internal/provider/resource_tfe_policy.go index d69173329..692347901 100644 --- a/internal/provider/resource_tfe_policy.go +++ b/internal/provider/resource_tfe_policy.go @@ -264,7 +264,7 @@ func resourceTFEPolicyUpdate(d *schema.ResourceData, meta interface{}) error { config := meta.(ConfiguredClient) // nolint:nestif - if d.HasChange("description") || d.HasChange("enforce_mode") { + if d.HasChange("description") || d.HasChange("enforce_mode") || d.HasChange("query") { // Create a new options struct. options := tfe.PolicyUpdateOptions{} @@ -288,6 +288,10 @@ func resourceTFEPolicyUpdate(d *schema.ResourceData, meta interface{}) error { } } + if query, ok := d.HasChange("query"); ok { + options.Query = tfe.String(query.(string)) + } + log.Printf("[DEBUG] Update configuration for %s policy: %s", vKind, d.Id()) _, err := config.Client.Policies.Update(ctx, d.Id(), options) if err != nil { From 2df51e1db1cc1a56f03a8eef7b3e11d851d31f40 Mon Sep 17 00:00:00 2001 From: Eli Skeggs <1348991+skeggse@users.noreply.github.com> Date: Wed, 18 Oct 2023 15:35:40 -0700 Subject: [PATCH 2/7] docs: update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 466eb3b52..5ffb3d8d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ +BUG FIXES: +* `r/tfe_policy`: Fix the provider ignoring updates to the `query` field, by @skeggse [1108](https://github.com/hashicorp/terraform-provider-tfe/pull/1108) + FEATURES: * `d/tfe_registry_module`: Add `vcs_repo.tags` and `vcs_repo.branch` attributes to allow configuration of `publishing_mechanism`. Add `test_config` to support running tests on `branch`-based registry modules, by @hashimoon [1096](https://github.com/hashicorp/terraform-provider-tfe/pull/1096) From 6eb8f5e0c9011f3dd61c1dac86fadd437f6904d8 Mon Sep 17 00:00:00 2001 From: Eli Skeggs <1348991+skeggse@users.noreply.github.com> Date: Wed, 18 Oct 2023 15:38:20 -0700 Subject: [PATCH 3/7] fix deref --- internal/provider/resource_tfe_policy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/provider/resource_tfe_policy.go b/internal/provider/resource_tfe_policy.go index 692347901..b5b772ab3 100644 --- a/internal/provider/resource_tfe_policy.go +++ b/internal/provider/resource_tfe_policy.go @@ -288,7 +288,7 @@ func resourceTFEPolicyUpdate(d *schema.ResourceData, meta interface{}) error { } } - if query, ok := d.HasChange("query"); ok { + if query, ok := d.GetOk("query"); ok { options.Query = tfe.String(query.(string)) } From c01b923ffd09695ca385cd9622a0fe539fe4bdb7 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Mon, 27 Nov 2023 11:53:04 -0700 Subject: [PATCH 4/7] Remove skipUnlessBeta from tfe_policy tests --- internal/provider/resource_tfe_policy_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/internal/provider/resource_tfe_policy_test.go b/internal/provider/resource_tfe_policy_test.go index 7bb0f4418..cd144f4ed 100644 --- a/internal/provider/resource_tfe_policy_test.go +++ b/internal/provider/resource_tfe_policy_test.go @@ -13,7 +13,6 @@ import ( ) func TestAccTFEPolicy_basic(t *testing.T) { - skipUnlessBeta(t) tfeClient, err := getClientUsingEnv() if err != nil { t.Fatal(err) @@ -52,7 +51,6 @@ func TestAccTFEPolicy_basic(t *testing.T) { } func TestAccTFEPolicy_basicWithDefaults(t *testing.T) { - skipUnlessBeta(t) tfeClient, err := getClientUsingEnv() if err != nil { t.Fatal(err) @@ -91,7 +89,6 @@ func TestAccTFEPolicy_basicWithDefaults(t *testing.T) { } func TestAccTFEPolicyOPA_basic(t *testing.T) { - skipUnlessBeta(t) tfeClient, err := getClientUsingEnv() if err != nil { t.Fatal(err) @@ -132,7 +129,6 @@ func TestAccTFEPolicyOPA_basic(t *testing.T) { } func TestAccTFEPolicy_update(t *testing.T) { - skipUnlessBeta(t) tfeClient, err := getClientUsingEnv() if err != nil { t.Fatal(err) @@ -244,7 +240,6 @@ func TestAccTFEPolicy_unsetEnforce(t *testing.T) { } func TestAccTFEPolicyOPA_update(t *testing.T) { - skipUnlessBeta(t) tfeClient, err := getClientUsingEnv() if err != nil { t.Fatal(err) @@ -304,7 +299,6 @@ func TestAccTFEPolicyOPA_update(t *testing.T) { } func TestAccTFEPolicy_import(t *testing.T) { - skipUnlessBeta(t) tfeClient, err := getClientUsingEnv() if err != nil { t.Fatal(err) From 9eab95ba1dbde365e5d9f14d4f4d2d09cf04c303 Mon Sep 17 00:00:00 2001 From: Nick Fagerlund Date: Mon, 27 Nov 2023 11:16:29 -0800 Subject: [PATCH 5/7] r/tfe_policy test: Include OPA query in the "real" attributes check Failing to update this was a real provider bug. --- internal/provider/resource_tfe_policy_test.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/internal/provider/resource_tfe_policy_test.go b/internal/provider/resource_tfe_policy_test.go index cd144f4ed..3f8876a05 100644 --- a/internal/provider/resource_tfe_policy_test.go +++ b/internal/provider/resource_tfe_policy_test.go @@ -287,9 +287,9 @@ func TestAccTFEPolicyOPA_update(t *testing.T) { resource.TestCheckResourceAttr( "tfe_policy.foobar", "description", "An updated test policy"), resource.TestCheckResourceAttr( - "tfe_policy.foobar", "policy", "package example rule[\"not allowed\"] { true }"), + "tfe_policy.foobar", "policy", "package example ruler[\"not allowed\"] { true }"), resource.TestCheckResourceAttr( - "tfe_policy.foobar", "query", "data.example.rule"), + "tfe_policy.foobar", "query", "data.example.ruler"), resource.TestCheckResourceAttr( "tfe_policy.foobar", "enforce_mode", "advisory"), ), @@ -382,6 +382,10 @@ func testAccCheckTFEOPAPolicyAttributes( return fmt.Errorf("Bad enforce mode: %s", policy.Enforce[0].Mode) } + if *policy.Query != "data.example.rule" { + return fmt.Errorf("Bad OPA query string: %s", *policy.Query) + } + return nil } } @@ -432,6 +436,10 @@ func testAccCheckTFEOPAPolicyAttributesUpdated( return fmt.Errorf("Bad enforce mode: %s", policy.Enforce[0].Mode) } + if *policy.Query != "data.example.ruler" { + return fmt.Errorf("Bad OPA query string: %s", *policy.Query) + } + return nil } } @@ -519,8 +527,8 @@ resource "tfe_policy" "foobar" { description = "An updated test policy" organization = "%s" kind = "opa" - policy = "package example rule[\"not allowed\"] { true }" - query = "data.example.rule" + policy = "package example ruler[\"not allowed\"] { true }" + query = "data.example.ruler" enforce_mode = "advisory" }`, organization) } From 2724aeb06b1476625e8e6b7f11abc80255622af0 Mon Sep 17 00:00:00 2001 From: Nick Fagerlund Date: Mon, 27 Nov 2023 11:18:05 -0800 Subject: [PATCH 6/7] r/tfe_policy test: check downloaded policy content too Another spot where these tests weren't sufficiently paranoid, although in this case we aren't aware of a bug in the wild. --- internal/provider/resource_tfe_policy_test.go | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/internal/provider/resource_tfe_policy_test.go b/internal/provider/resource_tfe_policy_test.go index 3f8876a05..254c14a1b 100644 --- a/internal/provider/resource_tfe_policy_test.go +++ b/internal/provider/resource_tfe_policy_test.go @@ -150,6 +150,7 @@ func TestAccTFEPolicy_update(t *testing.T) { testAccCheckTFEPolicyExists( "tfe_policy.foobar", policy), testAccCheckTFEPolicyAttributes(policy), + testAccCheckTFEPolicyContent(policy, "main = rule { true }"), resource.TestCheckResourceAttr( "tfe_policy.foobar", "name", "policy-test"), resource.TestCheckResourceAttr( @@ -169,6 +170,7 @@ func TestAccTFEPolicy_update(t *testing.T) { testAccCheckTFEPolicyExists( "tfe_policy.foobar", policy), testAccCheckTFEPolicyAttributesUpdated(policy), + testAccCheckTFEPolicyContent(policy, "main = rule { false }"), resource.TestCheckResourceAttr( "tfe_policy.foobar", "name", "policy-test"), resource.TestCheckResourceAttr( @@ -261,6 +263,7 @@ func TestAccTFEPolicyOPA_update(t *testing.T) { testAccCheckTFEPolicyExists( "tfe_policy.foobar", policy), testAccCheckTFEOPAPolicyAttributes(policy), + testAccCheckTFEPolicyContent(policy, "package example rule[\"not allowed\"] { false }"), resource.TestCheckResourceAttr( "tfe_policy.foobar", "name", "policy-test"), resource.TestCheckResourceAttr( @@ -282,6 +285,7 @@ func TestAccTFEPolicyOPA_update(t *testing.T) { testAccCheckTFEPolicyExists( "tfe_policy.foobar", policy), testAccCheckTFEOPAPolicyAttributesUpdated(policy), + testAccCheckTFEPolicyContent(policy, "package example ruler[\"not allowed\"] { true }"), resource.TestCheckResourceAttr( "tfe_policy.foobar", "name", "policy-test"), resource.TestCheckResourceAttr( @@ -356,6 +360,22 @@ func testAccCheckTFEPolicyExists( } } +func testAccCheckTFEPolicyContent(policy *tfe.Policy, content string) resource.TestCheckFunc { + return func(_ *terraform.State) error { + config := testAccProvider.Meta().(ConfiguredClient) + + b, err := config.Client.Policies.Download(ctx, policy.ID) + if err != nil { + return err + } + s := string(b) + if s != content { + return fmt.Errorf("Policy content didn't match. Expected: %q; got: %q", content, s) + } + return nil + } +} + func testAccCheckTFEPolicyAttributes( policy *tfe.Policy) resource.TestCheckFunc { return func(s *terraform.State) error { From 6aa39dac893de20ddd7436411babd385107d8633 Mon Sep 17 00:00:00 2001 From: Nick Fagerlund Date: Mon, 27 Nov 2023 12:24:51 -0800 Subject: [PATCH 7/7] Lint: Wrap foreign error before returning it --- internal/provider/resource_tfe_policy_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/provider/resource_tfe_policy_test.go b/internal/provider/resource_tfe_policy_test.go index 254c14a1b..4cae5c23f 100644 --- a/internal/provider/resource_tfe_policy_test.go +++ b/internal/provider/resource_tfe_policy_test.go @@ -366,7 +366,7 @@ func testAccCheckTFEPolicyContent(policy *tfe.Policy, content string) resource.T b, err := config.Client.Policies.Download(ctx, policy.ID) if err != nil { - return err + return fmt.Errorf("Problem downloading policy content: %w", err) } s := string(b) if s != content {