Prepare 2.4.1 release (#82)

CHANGELOG.md

@@ -1,3 +1,11 @@
+## [2.4.1] (January 21, 2022)
+
+BUG FIXES:
+
+- Fixed a [bug](https://app.shortcut.com/launchdarkly/story/138913/terraform-provider-panics-when-trying-to-create-triggers-that-are-enabled) preventing `launchdarkly_flag_trigger`s from being created in the `enabled` state.
+
+- Fixed a [bug](https://github.com/launchdarkly/terraform-provider-launchdarkly/issues/79) introduced in v2.2.0 where `launchdarkly_segments` with `rule` blocks not containing a `weight` were defaulting to a `weight` of 0.
+
 ## [2.4.0] (January 19, 2022)
 
 FEATURES:

launchdarkly/resource_launchdarkly_flag_trigger.go

@@ -38,15 +38,19 @@ func resourceFlagTriggerCreate(ctx context.Context, d *schema.ResourceData, meta
 	triggerBody := ldapi.NewTriggerPost(integrationKey)
 	triggerBody.Instructions = &instructions
 
-	preUpdateTrigger, _, err := client.ld.FlagTriggersApi.CreateTriggerWorkflow(client.ctx, projectKey, envKey, flagKey).TriggerPost(*triggerBody).Execute()
+	createdTrigger, _, err := client.ld.FlagTriggersApi.CreateTriggerWorkflow(client.ctx, projectKey, envKey, flagKey).TriggerPost(*triggerBody).Execute()
 	if err != nil {
 		return diag.Errorf("failed to create %s trigger for proj/env/flag %s/%s/%s: %s", integrationKey, projectKey, envKey, flagKey, err.Error())
 	}
-	_ = d.Set(TRIGGER_URL, preUpdateTrigger.TriggerURL)
+	_ = d.Set(TRIGGER_URL, createdTrigger.TriggerURL)
+
+	if createdTrigger.Id == nil {
+		return diag.Errorf("received a nil trigger ID when creating %s trigger for proj/env/flag %s/%s/%s: %s", integrationKey, projectKey, envKey, flagKey, err.Error())
+	}
+	d.SetId(*createdTrigger.Id)
 
 	// if enabled is false upon creation, we need to do a patch since the create endpoint
 	// does not accept multiple instructions
-	var postUpdateTrigger ldapi.TriggerWorkflowRep
 	if !enabled {
 		instructions = []map[string]interface{}{{
 			KIND: "disableTrigger",
@@ -55,13 +59,11 @@ func resourceFlagTriggerCreate(ctx context.Context, d *schema.ResourceData, meta
 			Instructions: &instructions,
 		}
 
-		postUpdateTrigger, _, err = client.ld.FlagTriggersApi.PatchTriggerWorkflow(client.ctx, projectKey, envKey, flagKey, *preUpdateTrigger.Id).FlagTriggerInput(input).Execute()
+		_, _, err = client.ld.FlagTriggersApi.PatchTriggerWorkflow(client.ctx, projectKey, envKey, flagKey, *createdTrigger.Id).FlagTriggerInput(input).Execute()
 		if err != nil {
 			return diag.Errorf("failed to update %s trigger for proj/env/flag %s/%s/%s: %s", integrationKey, projectKey, envKey, flagKey, err.Error())
 		}
 	}
-
-	d.SetId(*postUpdateTrigger.Id)
 	return resourceFlagTriggerRead(ctx, d, metaRaw)
 }
 

launchdarkly/resource_launchdarkly_flag_trigger_test.go

@@ -22,6 +22,20 @@ resource "launchdarkly_flag_trigger" "basic" {
 	enabled = false
 }
 `
+
+	testAccFlagTriggerCreateEnabled = `
+resource "launchdarkly_flag_trigger" "basic" {
+	project_key = launchdarkly_project.test.key
+	env_key = "test"
+	flag_key = launchdarkly_feature_flag.trigger_flag.key
+	integration_key = "generic-trigger"
+	instructions {
+		kind = "turnFlagOff"
+	}
+	enabled = true
+}
+`
+
 	testAccFlagTriggerUpdate = `
 resource "launchdarkly_flag_trigger" "basic" {
 	project_key = launchdarkly_project.test.key
@@ -127,6 +141,42 @@ func TestAccFlagTrigger_CreateUpdate(t *testing.T) {
 	})
 }
 
+func TestAccFlagTrigger_CreateEnabled(t *testing.T) {
+	projectKey := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
+	flagKey := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
+	resourceName := "launchdarkly_flag_trigger.basic"
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck: func() {
+			testAccPreCheck(t)
+		},
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: withRandomProject(projectKey, withRandomFlag(flagKey, testAccFlagTriggerCreateEnabled)),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckProjectExists("launchdarkly_project.test"),
+					testAccCheckFlagExists(projectKey, "launchdarkly_feature_flag.trigger_flag"),
+					resource.TestCheckResourceAttr(resourceName, PROJECT_KEY, projectKey),
+					resource.TestCheckResourceAttr(resourceName, ENV_KEY, "test"),
+					resource.TestCheckResourceAttr(resourceName, FLAG_KEY, flagKey),
+					resource.TestCheckResourceAttr(resourceName, INTEGRATION_KEY, "generic-trigger"),
+					resource.TestCheckResourceAttr(resourceName, "instructions.0.kind", "turnFlagOff"),
+					resource.TestCheckResourceAttr(resourceName, ENABLED, "true"),
+					resource.TestCheckResourceAttrSet(resourceName, TRIGGER_URL),
+					resource.TestCheckResourceAttrSet(resourceName, MAINTAINER_ID),
+				),
+			},
+			{
+				ResourceName:            resourceName,
+				ImportState:             true,
+				ImportStateIdPrefix:     fmt.Sprintf("%s/test/%s/", projectKey, flagKey),
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{TRIGGER_URL},
+			},
+		},
+	})
+}
+
 func testAccCheckFlagExists(projectKey, resourceName string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[resourceName]

launchdarkly/segment_rule_helper.go

@@ -17,8 +17,8 @@ func segmentRulesSchema() *schema.Schema {
 					Type:             schema.TypeInt,
 					Elem:             &schema.Schema{Type: schema.TypeInt},
 					Optional:         true,
-					ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(0, 100000)),
-					Description:      "The integer weight of the rule (between 0 and 100000).",
+					ValidateDiagFunc: validation.ToDiagFunc(validation.IntBetween(1, 100000)),
+					Description:      "The integer weight of the rule (between 1 and 100000).",
 				},
 				BUCKET_BY: {
 					Type:        schema.TypeString,
@@ -47,21 +47,31 @@ func segmentRulesFromResourceData(d *schema.ResourceData, metaRaw interface{}) (
 
 func segmentRuleFromResourceData(val interface{}) (ldapi.UserSegmentRule, error) {
 	ruleMap := val.(map[string]interface{})
-	weight := int32(ruleMap[WEIGHT].(int))
-	bucketBy := ruleMap[BUCKET_BY].(string)
-	r := ldapi.UserSegmentRule{
-		Weight:   &weight,
-		BucketBy: &bucketBy,
-	}
-	for _, c := range ruleMap[CLAUSES].([]interface{}) {
+	rawClauses := ruleMap[CLAUSES].([]interface{})
+	clauses := make([]ldapi.Clause, 0, len(rawClauses))
+	for _, c := range rawClauses {
 		clause, err := clauseFromResourceData(c)
 		if err != nil {
-			return r, err
+			return ldapi.UserSegmentRule{}, err
 		}
-		r.Clauses = append(r.Clauses, clause)
+		clauses = append(clauses, clause)
+	}
+
+	r := ldapi.NewUserSegmentRule(clauses)
+
+	bucketBy := ruleMap[BUCKET_BY].(string)
+	if bucketBy != "" {
+		r.SetBucketBy(bucketBy)
+	}
+
+	// weight == 0 when omitted from the config. In this case we do not want to include a weight in the PATCH
+	// request because the results will be counterintuitive. See: https://github.com/launchdarkly/terraform-provider-launchdarkly/issues/79
+	weight := int32(ruleMap[WEIGHT].(int))
+	if weight > 0 {
+		r.SetWeight(weight)
 	}
 
-	return r, nil
+	return *r, nil
 }
 
 func segmentRulesToResourceData(rules []ldapi.UserSegmentRule) (interface{}, error) {

launchdarkly/segment_rule_helper_test.go

@@ -0,0 +1,64 @@
+package launchdarkly
+
+import (
+	"testing"
+
+	ldapi "github.com/launchdarkly/api-client-go/v7"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestSegmentRuleFromResourceData(t *testing.T) {
+	cases := []struct {
+		name     string
+		input    map[string]interface{}
+		expected ldapi.UserSegmentRule
+	}{
+		{
+			// https://github.com/launchdarkly/terraform-provider-launchdarkly/issues/79
+			name: "Zero case should not create a UserSegmentRule with 0 weight",
+			input: map[string]interface{}{
+				WEIGHT:    0,
+				BUCKET_BY: "",
+				CLAUSES:   []interface{}{},
+			},
+			expected: ldapi.UserSegmentRule{
+				Clauses: []ldapi.Clause{},
+			},
+		},
+		{
+			name: "Clauses only - most typical case",
+			input: map[string]interface{}{
+				WEIGHT:    0,
+				BUCKET_BY: "",
+				CLAUSES: []interface{}{
+					map[string]interface{}{
+						ATTRIBUTE:  "country",
+						OP:         "in",
+						NEGATE:     false,
+						VALUES:     []interface{}{"us", "gb"},
+						VALUE_TYPE: "string",
+					},
+				},
+			},
+			expected: ldapi.UserSegmentRule{
+				Clauses: []ldapi.Clause{
+					{
+						Attribute: "country",
+						Op:        "in",
+						Negate:    false,
+						Values:    []interface{}{"us", "gb"},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			actual, err := segmentRuleFromResourceData(tc.input)
+			require.NoError(t, err)
+			assert.Equal(t, tc.expected, actual)
+		})
+	}
+}

website/docs/r/segment.html.markdown

@@ -59,7 +59,7 @@ resource "launchdarkly_segment" "example" {
 
 Nested `rules` blocks have the following structure:
 
-- `weight` - (Optional) The integer weight of the rule (between 0 and 100000).
+- `weight` - (Optional) The integer weight of the rule (between 1 and 100000).
 
 - `bucket_by` - (Optional) The attribute by which to group users together.