From 5d96a37bbcdd25f19079f61dff1d7807053be4cb Mon Sep 17 00:00:00 2001 From: Henry Barrow Date: Tue, 8 Sep 2020 12:32:43 +0100 Subject: [PATCH] Release 1.4.1 (#39) - Fixed a bug where omitted optional `launchdarkly_feature_flag_environment` parameters where making unwanted changes to the resource upon creation. [#38](https://github.com/launchdarkly/terraform-provider-launchdarkly/issues/38) --- CHANGELOG.md | 13 +- launchdarkly/fallthrough_helper.go | 3 +- launchdarkly/prerequisite_helper.go | 14 ++- ...e_launchdarkly_feature_flag_environment.go | 116 +++++++++++------- ...nchdarkly_feature_flag_environment_test.go | 98 ++++++++++++++- launchdarkly/rule_helper.go | 3 +- launchdarkly/target_helper.go | 1 + 7 files changed, 191 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77735fc7..ae85c2bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,15 @@ -## [Unreleased] +## [1.4.1] (September 8, 2020) + +FEATURES: + +- Fixed a bug where omitted optional `launchdarkly_feature_flag_environment` parameters where making unwanted changes + to the resource upon creation. [#38](https://github.com/launchdarkly/terraform-provider-launchdarkly/issues/38) + +## [1.4.0] (August 21, 2020) + +FEATURES: + +- Added the `launchdarkly_access_token` resource. FEATURES: diff --git a/launchdarkly/fallthrough_helper.go b/launchdarkly/fallthrough_helper.go index 0a94c6a5..2c2759db 100644 --- a/launchdarkly/fallthrough_helper.go +++ b/launchdarkly/fallthrough_helper.go @@ -13,6 +13,7 @@ func fallthroughSchema() *schema.Schema { return &schema.Schema{ Type: schema.TypeList, Optional: true, + Computed: true, MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ @@ -21,7 +22,7 @@ func fallthroughSchema() *schema.Schema { Type: schema.TypeString, Optional: true, }, - VARIATION: &schema.Schema{ + VARIATION: { Type: schema.TypeInt, Optional: true, ValidateFunc: validation.IntAtLeast(0), diff --git a/launchdarkly/prerequisite_helper.go b/launchdarkly/prerequisite_helper.go index dcd66f32..c64b4647 100644 --- a/launchdarkly/prerequisite_helper.go +++ b/launchdarkly/prerequisite_helper.go @@ -12,6 +12,7 @@ func prerequisitesSchema() *schema.Schema { return &schema.Schema{ Type: schema.TypeList, Optional: true, + Computed: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ FLAG_KEY: { @@ -19,7 +20,7 @@ func prerequisitesSchema() *schema.Schema { Required: true, ValidateFunc: validateKey(), }, - VARIATION: &schema.Schema{ + VARIATION: { Type: schema.TypeInt, Elem: &schema.Schema{Type: schema.TypeInt}, Required: true, @@ -51,3 +52,14 @@ func prerequisiteFromResourceData(val interface{}) ldapi.Prerequisite { log.Printf("[DEBUG] %+v\n", p) return p } + +func prerequisitesToResourceData(prerequisites []ldapi.Prerequisite) interface{} { + transformed := make([]interface{}, 0, len(prerequisites)) + for _, prereq := range prerequisites { + transformed = append(transformed, map[string]interface{}{ + FLAG_KEY: prereq.Key, + VARIATION: prereq.Variation, + }) + } + return transformed +} diff --git a/launchdarkly/resource_launchdarkly_feature_flag_environment.go b/launchdarkly/resource_launchdarkly_feature_flag_environment.go index 417f99cb..562b7f2e 100644 --- a/launchdarkly/resource_launchdarkly_feature_flag_environment.go +++ b/launchdarkly/resource_launchdarkly_feature_flag_environment.go @@ -37,6 +37,7 @@ func resourceFeatureFlagEnvironment() *schema.Resource { TARGETING_ENABLED: { Type: schema.TypeBool, Optional: true, + Computed: true, }, USER_TARGETS: targetsSchema(), RULES: rulesSchema(), @@ -45,10 +46,12 @@ func resourceFeatureFlagEnvironment() *schema.Resource { TRACK_EVENTS: { Type: schema.TypeBool, Optional: true, + Computed: true, }, OFF_VARIATION: { Type: schema.TypeInt, Optional: true, + Computed: true, ValidateFunc: validation.IntAtLeast(0), }, }, @@ -93,42 +96,68 @@ func resourceFeatureFlagEnvironmentCreate(d *schema.ResourceData, metaRaw interf return fmt.Errorf("failed to find environment with key %q", envKey) } - enabled := d.Get(TARGETING_ENABLED).(bool) - rules, err := rulesFromResourceData(d) - if err != nil { - return err + patches := make([]ldapi.PatchOperation, 0) + + enabled, ok := d.GetOk(TARGETING_ENABLED) + if ok { + patches = append(patches, patchReplace(patchFlagEnvPath(d, "on"), enabled.(bool))) } - trackEvents := d.Get(TRACK_EVENTS).(bool) - prerequisites := prerequisitesFromResourceData(d, PREREQUISITES) - offVariation := d.Get(OFF_VARIATION).(int) - targets := targetsFromResourceData(d, USER_TARGETS) - fall, err := fallthroughFromResourceData(d) - if err != nil { - return err + offVariation, ok := d.GetOk(OFF_VARIATION) + if ok { + patches = append(patches, patchReplace(patchFlagEnvPath(d, "offVariation"), offVariation.(int))) } - patch := ldapi.PatchComment{ - Comment: "Terraform", - Patch: []ldapi.PatchOperation{ - patchReplace(patchFlagEnvPath(d, "on"), enabled), - patchReplace(patchFlagEnvPath(d, "rules"), rules), - patchReplace(patchFlagEnvPath(d, "trackEvents"), trackEvents), - patchReplace(patchFlagEnvPath(d, "prerequisites"), prerequisites), - patchReplace(patchFlagEnvPath(d, "offVariation"), offVariation), - patchReplace(patchFlagEnvPath(d, "targets"), targets), - patchReplace(patchFlagEnvPath(d, "fallthrough"), fall), - }} + trackEvents, ok := d.GetOk(TRACK_EVENTS) + if ok { + patches = append(patches, patchReplace(patchFlagEnvPath(d, "trackEvents"), trackEvents.(bool))) + } - log.Printf("[DEBUG] %+v\n", patch) + _, ok = d.GetOk(RULES) + if ok { + rules, err := rulesFromResourceData(d) + if err != nil { + return err + } + patches = append(patches, patchReplace(patchFlagEnvPath(d, "rules"), rules)) + } - _, _, err = handleRateLimit(func() (interface{}, *http.Response, error) { - return handleNoConflict(func() (interface{}, *http.Response, error) { - return client.ld.FeatureFlagsApi.PatchFeatureFlag(client.ctx, projectKey, flagKey, patch) + _, ok = d.GetOk(PREREQUISITES) + if ok { + prerequisites := prerequisitesFromResourceData(d, PREREQUISITES) + patches = append(patches, patchReplace(patchFlagEnvPath(d, "prerequisites"), prerequisites)) + } + + _, ok = d.GetOk(USER_TARGETS) + if ok { + targets := targetsFromResourceData(d, USER_TARGETS) + patches = append(patches, patchReplace(patchFlagEnvPath(d, "targets"), targets)) + } + + _, ok = d.GetOk(FLAG_FALLTHROUGH) + if ok { + fall, err := fallthroughFromResourceData(d) + if err != nil { + return err + } + patches = append(patches, patchReplace(patchFlagEnvPath(d, "fallthrough"), fall)) + } + + if len(patches) > 0 { + patch := ldapi.PatchComment{ + Comment: "Terraform", + Patch: patches, + } + log.Printf("[DEBUG] %+v\n", patch) + + _, _, err = handleRateLimit(func() (interface{}, *http.Response, error) { + return handleNoConflict(func() (interface{}, *http.Response, error) { + return client.ld.FeatureFlagsApi.PatchFeatureFlag(client.ctx, projectKey, flagKey, patch) + }) }) - }) - if err != nil { - return fmt.Errorf("failed to update flag %q in project %q: %s", flagKey, projectKey, handleLdapiErr(err)) + if err != nil { + return fmt.Errorf("failed to update flag %q in project %q: %s", flagKey, projectKey, handleLdapiErr(err)) + } } d.SetId(projectKey + "/" + envKey + "/" + flagKey) @@ -166,34 +195,27 @@ func resourceFeatureFlagEnvironmentRead(d *schema.ResourceData, metaRaw interfac } _ = d.Set(KEY, flag.Key) + + // Computed values are set even if they do not exist on the config _ = d.Set(TARGETING_ENABLED, environment.On) + _ = d.Set(OFF_VARIATION, environment.OffVariation) + _ = d.Set(TRACK_EVENTS, environment.TrackEvents) + _ = d.Set(PREREQUISITES, prerequisitesToResourceData(environment.Prerequisites)) err = d.Set(RULES, rulesToResourceData(environment.Rules)) if err != nil { return fmt.Errorf("failed to set rules on flag with key %q: %v", flagKey, err) } - if _, ok := d.GetOk(USER_TARGETS); ok { - err = d.Set(USER_TARGETS, targetsToResourceData(environment.Targets)) - if err != nil { - return fmt.Errorf("failed to set targets on flag with key %q: %v", flagKey, err) - } - } - - if _, ok := d.GetOk(FLAG_FALLTHROUGH); ok { - err = d.Set(FLAG_FALLTHROUGH, fallthroughToResourceData(environment.Fallthrough_)) - if err != nil { - return fmt.Errorf("failed to set flag fallthrough on flag with key %q: %v", flagKey, err) - } + err = d.Set(USER_TARGETS, targetsToResourceData(environment.Targets)) + if err != nil { + return fmt.Errorf("failed to set targets on flag with key %q: %v", flagKey, err) } - if _, ok := d.GetOk(RULES); ok { - err = d.Set(RULES, rulesToResourceData(environment.Rules)) - if err != nil { - return fmt.Errorf("failed to set targeting rules on flag with key %q: %v", flagKey, err) - } + err = d.Set(FLAG_FALLTHROUGH, fallthroughToResourceData(environment.Fallthrough_)) + if err != nil { + return fmt.Errorf("failed to set flag fallthrough on flag with key %q: %v", flagKey, err) } - return nil } diff --git a/launchdarkly/resource_launchdarkly_feature_flag_environment_test.go b/launchdarkly/resource_launchdarkly_feature_flag_environment_test.go index ea2cd6a0..fa186fcb 100644 --- a/launchdarkly/resource_launchdarkly_feature_flag_environment_test.go +++ b/launchdarkly/resource_launchdarkly_feature_flag_environment_test.go @@ -39,6 +39,29 @@ resource "launchdarkly_feature_flag_environment" "basic" { variation = 1 } } +` + + testAccFeatureFlagEnvironmentEmpty = ` +resource "launchdarkly_feature_flag" "basic" { + project_key = launchdarkly_project.test.key + key = "basic-flag" + name = "Basic feature flag" + variation_type = "number" + variations { + value = 10 + } + variations { + value = 20 + } + variations { + value = 30 + } +} + +resource "launchdarkly_feature_flag_environment" "basic" { + flag_id = launchdarkly_feature_flag.basic.id + env_key = "test" +} ` testAccFeatureFlagEnvironmentUpdate = ` @@ -225,6 +248,40 @@ func TestAccFeatureFlagEnvironment_Basic(t *testing.T) { }) } +func TestAccFeatureFlagEnvironment_Empty(t *testing.T) { + projectKey := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) + resourceName := "launchdarkly_feature_flag_environment.basic" + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: withRandomProject(projectKey, testAccFeatureFlagEnvironmentEmpty), + Check: resource.ComposeTestCheckFunc( + testAccCheckFeatureFlagEnvironmentExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "targeting_enabled", "false"), + resource.TestCheckResourceAttr(resourceName, "off_variation", "2"), + resource.TestCheckResourceAttr(resourceName, "track_events", "false"), + resource.TestCheckNoResourceAttr(resourceName, "rules"), + resource.TestCheckNoResourceAttr(resourceName, "rules.#"), + resource.TestCheckNoResourceAttr(resourceName, "prerequisites"), + resource.TestCheckNoResourceAttr(resourceName, "prerequisites.#"), + resource.TestCheckResourceAttr(resourceName, "flag_fallthrough.#", "1"), + resource.TestCheckResourceAttr(resourceName, "flag_fallthrough.0.variation", "0"), + resource.TestCheckNoResourceAttr(resourceName, "user_targets"), + resource.TestCheckNoResourceAttr(resourceName, "user_targets.#"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + }, + }, + }) +} + func TestAccFeatureFlagEnvironment_Update(t *testing.T) { projectKey := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) resourceName := "launchdarkly_feature_flag_environment.basic" @@ -286,16 +343,45 @@ func TestAccFeatureFlagEnvironment_Update(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "rules.1.clauses.0.negate", "false"), ), }, + // After changes have been made to the resource, removing optional values should not change them. { - Config: withRandomProject(projectKey, testAccFeatureFlagEnvironmentBasic), + Config: withRandomProject(projectKey, testAccFeatureFlagEnvironmentEmpty), Check: resource.ComposeTestCheckFunc( testAccCheckFeatureFlagEnvironmentExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "targeting_enabled", "false"), + resource.TestCheckResourceAttr(resourceName, "targeting_enabled", "true"), + resource.TestCheckResourceAttr(resourceName, "track_events", "true"), resource.TestCheckResourceAttr(resourceName, "flag_fallthrough.#", "1"), - resource.TestCheckResourceAttr(resourceName, "flag_fallthrough.0.variation", "1"), - resource.TestCheckResourceAttr(resourceName, "flag_fallthrough.0.rollout.#", "0"), - resource.TestCheckResourceAttr(resourceName, "user_targets.#", "0"), - resource.TestCheckResourceAttr(resourceName, "rules.#", "0"), + resource.TestCheckResourceAttr(resourceName, "flag_fallthrough.0.variation", "0"), + resource.TestCheckResourceAttr(resourceName, "flag_fallthrough.0.rollout_weights.#", "3"), + resource.TestCheckResourceAttr(resourceName, "flag_fallthrough.0.rollout_weights.0", "60000"), + resource.TestCheckResourceAttr(resourceName, "flag_fallthrough.0.rollout_weights.1", "40000"), + resource.TestCheckResourceAttr(resourceName, "flag_fallthrough.0.rollout_weights.2", "0"), + resource.TestCheckResourceAttr(resourceName, "flag_fallthrough.0.bucket_by", "email"), + resource.TestCheckResourceAttr(resourceName, "user_targets.#", "3"), + resource.TestCheckResourceAttr(resourceName, "user_targets.0.values.#", "0"), + resource.TestCheckResourceAttr(resourceName, "user_targets.1.values.#", "2"), + resource.TestCheckResourceAttr(resourceName, "user_targets.1.values.0", "user1"), + resource.TestCheckResourceAttr(resourceName, "user_targets.1.values.1", "user2"), + resource.TestCheckResourceAttr(resourceName, "user_targets.2.values.#", "0"), + resource.TestCheckResourceAttr(resourceName, "rules.#", "2"), + resource.TestCheckResourceAttr(resourceName, "rules.0.variation", "0"), + resource.TestCheckResourceAttr(resourceName, "rules.0.clauses.#", "1"), + resource.TestCheckResourceAttr(resourceName, "rules.0.clauses.0.attribute", "country"), + resource.TestCheckResourceAttr(resourceName, "rules.0.clauses.0.op", "startsWith"), + resource.TestCheckResourceAttr(resourceName, "rules.0.clauses.0.values.#", "2"), + resource.TestCheckResourceAttr(resourceName, "rules.0.clauses.0.values.0", "great"), + resource.TestCheckResourceAttr(resourceName, "rules.0.clauses.0.values.1", "amazing"), + resource.TestCheckResourceAttr(resourceName, "rules.0.clauses.0.negate", "false"), + resource.TestCheckResourceAttr(resourceName, "rules.1.rollout_weights.#", "3"), + resource.TestCheckResourceAttr(resourceName, "rules.1.rollout_weights.0", "90000"), + resource.TestCheckResourceAttr(resourceName, "rules.1.rollout_weights.1", "10000"), + resource.TestCheckResourceAttr(resourceName, "rules.1.rollout_weights.2", "0"), + resource.TestCheckResourceAttr(resourceName, "rules.1.bucket_by", "email"), + resource.TestCheckResourceAttr(resourceName, "rules.1.clauses.0.attribute", "name"), + resource.TestCheckResourceAttr(resourceName, "rules.1.clauses.0.op", "startsWith"), + resource.TestCheckResourceAttr(resourceName, "rules.1.clauses.0.values.#", "1"), + resource.TestCheckResourceAttr(resourceName, "rules.1.clauses.0.values.0", "h"), + resource.TestCheckResourceAttr(resourceName, "rules.1.clauses.0.negate", "false"), ), }, }, diff --git a/launchdarkly/rule_helper.go b/launchdarkly/rule_helper.go index 2a819063..7b9d4602 100644 --- a/launchdarkly/rule_helper.go +++ b/launchdarkly/rule_helper.go @@ -13,10 +13,11 @@ func rulesSchema() *schema.Schema { return &schema.Schema{ Type: schema.TypeList, Optional: true, + Computed: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ CLAUSES: clauseSchema(), - VARIATION: &schema.Schema{ + VARIATION: { Type: schema.TypeInt, Elem: &schema.Schema{Type: schema.TypeInt}, Optional: true, diff --git a/launchdarkly/target_helper.go b/launchdarkly/target_helper.go index 5d5488ff..6f5d69bc 100644 --- a/launchdarkly/target_helper.go +++ b/launchdarkly/target_helper.go @@ -12,6 +12,7 @@ func targetsSchema() *schema.Schema { return &schema.Schema{ Type: schema.TypeList, Optional: true, + Computed: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "values": {