refactor lifecycle retention policies around quantity_to_keep

docs/resources/lifecycle.md

@@ -95,19 +95,19 @@ Optional:
 
 Optional:
 
-- `quantity_to_keep` (Number) The number of days/releases to keep. The default value is 30. If 0 then all are kept.
-- `should_keep_forever` (Boolean) Indicates if items should never be deleted. The default value is false.
-- `unit` (String) The unit of quantity to keep. Valid units are Days or Items. The default value is Days.
+- `quantity_to_keep` (Number) The number of days/releases to keep. If 0 then all are kept.
+- `should_keep_forever` (Boolean) Indicates if items should never be deleted.
+- `unit` (String) The unit of quantity to keep. Valid units are Days or Items.
 
 
 <a id="nestedblock--phase--tentacle_retention_policy"></a>
 ### Nested Schema for `phase.tentacle_retention_policy`
 
 Optional:
 
-- `quantity_to_keep` (Number) The number of days/releases to keep. The default value is 30. If 0 then all are kept.
-- `should_keep_forever` (Boolean) Indicates if items should never be deleted. The default value is false.
-- `unit` (String) The unit of quantity to keep. Valid units are Days or Items. The default value is Days.
+- `quantity_to_keep` (Number) The number of days/releases to keep. If 0 then all are kept.
+- `should_keep_forever` (Boolean) Indicates if items should never be deleted.
+- `unit` (String) The unit of quantity to keep. Valid units are Days or Items.
 
 
 
@@ -116,19 +116,19 @@ Optional:
 
 Optional:
 
-- `quantity_to_keep` (Number) The number of days/releases to keep. The default value is 30. If 0 then all are kept.
-- `should_keep_forever` (Boolean) Indicates if items should never be deleted. The default value is false.
-- `unit` (String) The unit of quantity to keep. Valid units are Days or Items. The default value is Days.
+- `quantity_to_keep` (Number) The number of days/releases to keep. If 0 then all are kept.
+- `should_keep_forever` (Boolean) Indicates if items should never be deleted.
+- `unit` (String) The unit of quantity to keep. Valid units are Days or Items.
 
 
 <a id="nestedblock--tentacle_retention_policy"></a>
 ### Nested Schema for `tentacle_retention_policy`
 
 Optional:
 
-- `quantity_to_keep` (Number) The number of days/releases to keep. The default value is 30. If 0 then all are kept.
-- `should_keep_forever` (Boolean) Indicates if items should never be deleted. The default value is false.
-- `unit` (String) The unit of quantity to keep. Valid units are Days or Items. The default value is Days.
+- `quantity_to_keep` (Number) The number of days/releases to keep. If 0 then all are kept.
+- `should_keep_forever` (Boolean) Indicates if items should never be deleted.
+- `unit` (String) The unit of quantity to keep. Valid units are Days or Items.
 
 ## Import
 

octopusdeploy_framework/resource_lifecycle_retention_test.go

@@ -117,33 +117,40 @@ func TestAccRetentionAttributeValidation(t *testing.T) {
 			{
 				Config:      lifecycleGivenRetentionAttributes(lifecycleName, "1", "Items", "true"),
 				PlanOnly:    true,
-				ExpectError: regexp.MustCompile(`should_keep_forever must be false when quantity_to_keep is not 0`),
+				ExpectError: regexp.MustCompile(`should_keep_forever must be false when quantity_to_keep is greater than 0`),
 			},
 			{
 				Config:      lifecycleGivenRetentionAttributes(lifecycleName, "1", "", "true"),
 				PlanOnly:    true,
-				ExpectError: regexp.MustCompile(`should_keep_forever must be false when quantity_to_keep is not 0`),
+				ExpectError: regexp.MustCompile(`should_keep_forever must be false when quantity_to_keep is greater than 0`),
 			},
 			// when quantity_to_keep is 0, should_keep_forever shouldn't be false
 			{
 				Config:      lifecycleGivenRetentionAttributes(lifecycleName, "0", "", "false"),
 				PlanOnly:    true,
-				ExpectError: regexp.MustCompile(`should_keep_forever must be true when quantity_to_keep is 0`),
+				ExpectError: regexp.MustCompile(`should_keep_forever must be true when quantity_to_keep is zero or missing`),
 			},
 			{
 				Config:      lifecycleGivenRetentionAttributes(lifecycleName, "", "", "false"),
 				PlanOnly:    true,
-				ExpectError: regexp.MustCompile(`The non-refresh plan was not empty`),
+				ExpectError: regexp.MustCompile(`should_keep_forever must be true when quantity_to_keep is zero or missing`),
 			},
 			{
 				Config:      lifecycleGivenRetentionAttributes(lifecycleName, "", "Items", "false"),
 				PlanOnly:    true,
-				ExpectError: regexp.MustCompile(`The non-refresh plan was not empty`),
+				ExpectError: regexp.MustCompile(`should_keep_forever must be true when quantity_to_keep is zero or missing`),
 			},
+			//should thow error when something other than days or items is submitted for units
+			{
+				Config:      lifecycleGivenRetentionAttributes(lifecycleName, "4", "Months", "false"),
+				PlanOnly:    true,
+				ExpectError: regexp.MustCompile(`Invalid Attribute Value Match`),
+			},
+			//should throw error when empty block is given
 			{
 				Config:      lifecycleGivenRetentionAttributes(lifecycleName, "", "", ""),
 				PlanOnly:    true,
-				ExpectError: regexp.MustCompile(`The non-refresh plan was not empty`),
+				ExpectError: regexp.MustCompile(`please either add retention policy attributes or remove the entire block`),
 			},
 		},
 	})

octopusdeploy_framework/resource_lifecycle_test.go

@@ -254,25 +254,6 @@ func TestAccLifecycleWithUpdate(t *testing.T) {
 				),
 				Config: testAccLifecycle(localName, name),
 			},
-			// update lifecycle add retention policy
-			{
-				Check: resource.ComposeTestCheckFunc(
-					testAccCheckLifecycleExists(resourceName),
-					resource.TestCheckResourceAttrSet(resourceName, "id"),
-					resource.TestCheckResourceAttr(resourceName, "name", name),
-					resource.TestCheckResourceAttr(resourceName, "description", description),
-					resource.TestCheckResourceAttr(resourceName, "release_retention_policy.#", "1"),
-					resource.TestCheckResourceAttr(resourceName, "release_retention_policy.0.quantity_to_keep", "60"),
-					resource.TestCheckResourceAttr(resourceName, "release_retention_policy.0.should_keep_forever", "false"),
-					resource.TestCheckResourceAttr(resourceName, "release_retention_policy.0.unit", "Days"),
-					resource.TestCheckResourceAttrSet(resourceName, "space_id"),
-					resource.TestCheckResourceAttr(resourceName, "tentacle_retention_policy.#", "1"),
-					resource.TestCheckResourceAttr(resourceName, "tentacle_retention_policy.0.quantity_to_keep", "0"),
-					resource.TestCheckResourceAttr(resourceName, "tentacle_retention_policy.0.should_keep_forever", "true"),
-					resource.TestCheckResourceAttr(resourceName, "tentacle_retention_policy.0.unit", "Items"),
-				),
-				Config: testAccLifecycleWithRetentionPolicy(localName, name, description),
-			},
 		},
 	})
 }
@@ -324,24 +305,6 @@ func testAccLifecycleWithDescription(localName string, name string, description
     }`, localName, name, description)
 }
 
-func testAccLifecycleWithRetentionPolicy(localName string, name string, description string) string {
-	return fmt.Sprintf(`resource "octopusdeploy_lifecycle" "%s" {
-       name        = "%s"
-       description = "%s"
-		release_retention_policy {
-			unit             = "Days"
-			quantity_to_keep = 60
-			should_keep_forever = false
-		}
-
-		tentacle_retention_policy {
-			unit             = "Items"
-			quantity_to_keep = 0
-			should_keep_forever = true
-		}
-    }`, localName, name, description)
-}
-
 func testAccLifecycleComplex(localName string, name string) string {
 	environment1LocalName := acctest.RandStringFromCharSet(20, acctest.CharSetAlpha)
 	environment1Name := acctest.RandStringFromCharSet(20, acctest.CharSetAlpha)

octopusdeploy_framework/schemas/lifecycle.go

@@ -2,6 +2,7 @@ package schemas
 
 import (
 	"context"
+	"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
 	"strings"
 
 	"github.com/OctopusDeploy/terraform-provider-octopusdeploy/octopusdeploy_framework/util"
@@ -13,7 +14,6 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64default"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/listplanmodifier"
-	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/schema/validator"
 	"github.com/hashicorp/terraform-plugin-framework/tfsdk"
@@ -111,19 +111,16 @@ func getResourceRetentionPolicyBlockSchema() resourceSchema.ListNestedBlock {
 			Attributes: map[string]resourceSchema.Attribute{
 				"quantity_to_keep": util.ResourceInt64().
 					Optional().Computed().
-					Default(int64default.StaticInt64(30)).
 					Validators(int64validator.AtLeast(0)).
-					Description("The number of days/releases to keep. The default value is 30. If 0 then all are kept.").
+					Description("The number of days/releases to keep. If 0 then all are kept.").
 					Build(),
 				"should_keep_forever": util.ResourceBool().
 					Optional().Computed().
-					Default(booldefault.StaticBool(false)).
-					Description("Indicates if items should never be deleted. The default value is false.").
+					Description("Indicates if items should never be deleted.").
 					Build(),
 				"unit": util.ResourceString().
-					Optional().Computed().
-					Default(stringdefault.StaticString("Days")).
-					Description("The unit of quantity to keep. Valid units are Days or Items. The default value is Days.").
+					Optional().Computed().Validators(stringvalidator.OneOfCaseInsensitive("Days", "Items")).
+					Description("The unit of quantity to keep. Valid units are Days or Items.").
 					Build(),
 			},
 			Validators: []validator.Object{
@@ -206,32 +203,56 @@ func (v retentionPolicyValidator) ValidateObject(ctx context.Context, req valida
 		return
 	}
 
-	if !retentionPolicy.QuantityToKeep.IsNull() && !retentionPolicy.QuantityToKeep.IsUnknown() && !retentionPolicy.ShouldKeepForever.IsNull() && !retentionPolicy.ShouldKeepForever.IsUnknown() {
-		quantityToKeep := retentionPolicy.QuantityToKeep.ValueInt64()
-		shouldKeepForever := retentionPolicy.ShouldKeepForever.ValueBool()
+	unitPresent := !retentionPolicy.Unit.IsNull() && !retentionPolicy.Unit.IsUnknown()
+	quantityToKeepPresent := !retentionPolicy.QuantityToKeep.IsNull() && !retentionPolicy.QuantityToKeep.IsUnknown()
+	shouldKeepForeverPresent := !retentionPolicy.ShouldKeepForever.IsNull() && !retentionPolicy.ShouldKeepForever.IsUnknown()
+	shouldKeepForever := shouldKeepForeverPresent && retentionPolicy.ShouldKeepForever.ValueBool()
+	quantityToKeepIsMoreThanZero := quantityToKeepPresent && retentionPolicy.QuantityToKeep.ValueInt64() > 0
 
-		if quantityToKeep == 0 && !shouldKeepForever {
+	if !unitPresent && !quantityToKeepPresent && !shouldKeepForeverPresent {
+		resp.Diagnostics.AddAttributeError(
+			req.Path.AtName("strategy"),
+			"Invalid retention policy configuration",
+			"please either add retention policy attributes or remove the entire block",
+		)
+	}
+
+	// count strategy validations
+	if quantityToKeepIsMoreThanZero {
+		if shouldKeepForever {
 			resp.Diagnostics.AddAttributeError(
 				req.Path.AtName("should_keep_forever"),
 				"Invalid retention policy configuration",
-				"should_keep_forever must be true when quantity_to_keep is 0",
+				"should_keep_forever must be false when quantity_to_keep is greater than 0",
 			)
-		} else if quantityToKeep != 0 && shouldKeepForever {
+		}
+		if !unitPresent {
 			resp.Diagnostics.AddAttributeError(
-				req.Path.AtName("should_keep_forever"),
+				req.Path.AtName("unit"),
 				"Invalid retention policy configuration",
-				"should_keep_forever must be false when quantity_to_keep is not 0",
+				"unit is required when quantity_to_keep is greater than 0",
 			)
 		}
 	}
 
-	if !retentionPolicy.Unit.IsNull() && !retentionPolicy.Unit.IsUnknown() {
-		unit := retentionPolicy.Unit.ValueString()
-		if !strings.EqualFold(unit, "Days") && !strings.EqualFold(unit, "Items") {
+	// keep forever strategy validation
+	if !quantityToKeepIsMoreThanZero && !shouldKeepForever {
+		resp.Diagnostics.AddAttributeError(
+			req.Path.AtName("should_keep_forever"),
+			"Invalid retention policy configuration",
+			"should_keep_forever must be true when quantity_to_keep is zero or missing",
+		)
+	}
+
+	if unitPresent && !quantityToKeepIsMoreThanZero {
+		if strings.EqualFold(retentionPolicy.Unit.ValueString(), "Items") {
+			// do not throw an error for backwards compatability.
+		} else {
 			resp.Diagnostics.AddAttributeError(
+				// replaces a confusing state change to "unit = Items" error at the api
 				req.Path.AtName("unit"),
-				"Invalid retention policy unit",
-				"Unit must be either 'Days' or 'Items' (case insensitive)",
+				"Invalid retention policy configuration",
+				"unit is only used when quantity_to_keep is greater than 0",
 			)
 		}
 	}