diff --git a/docs/resources/lifecycle.md b/docs/resources/lifecycle.md index 4777c939..edbd4d4c 100644 --- a/docs/resources/lifecycle.md +++ b/docs/resources/lifecycle.md @@ -95,9 +95,9 @@ 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. @@ -105,9 +105,9 @@ 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. @@ -116,9 +116,9 @@ 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. @@ -126,9 +126,9 @@ 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. ## Import diff --git a/octopusdeploy_framework/resource_lifecycle_retention_test.go b/octopusdeploy_framework/resource_lifecycle_retention_test.go index 5c44ffb1..513ec4df 100644 --- a/octopusdeploy_framework/resource_lifecycle_retention_test.go +++ b/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`), }, }, }) diff --git a/octopusdeploy_framework/resource_lifecycle_test.go b/octopusdeploy_framework/resource_lifecycle_test.go index bfddb241..4d5a73f0 100644 --- a/octopusdeploy_framework/resource_lifecycle_test.go +++ b/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) diff --git a/octopusdeploy_framework/schemas/lifecycle.go b/octopusdeploy_framework/schemas/lifecycle.go index c38f929c..85db0747 100644 --- a/octopusdeploy_framework/schemas/lifecycle.go +++ b/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", ) } }