Skip to content

Commit 8f51679

Browse files
jtaylor-csjtaylor-cs
authored
Fixed handling of zero values in retention attributes. (#162)
* Fixed handling of zero values in retention attributes. * Refined solution based on PR feedback.
1 parent aebcdd4 commit 8f51679

File tree

3 files changed

+86
-37
lines changed

3 files changed

+86
-37
lines changed

‎cloudsmith/resource_repository_retention_rule.go‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,27 +27,28 @@ func resourceRepoRetentionRuleUpdate(d *schema.ResourceData, meta interface{}) e
2727
repo := requiredString(d, "repository")
2828

2929
req := pc.APIClient.ReposApi.RepoRetentionPartialUpdate(pc.Auth, namespace, repo)
30+
31+
// For integer fields with defaults, we need to always send the value to handle
32+
// the case where users explicitly set them to 0 (which would otherwise be
33+
// indistinguishable from "not set" using GetOk)
34+
retentionCountLimit := int64(d.Get("retention_count_limit").(int))
35+
retentionDaysLimit := int64(d.Get("retention_days_limit").(int))
36+
3037
updateData := cloudsmith.RepositoryRetentionRulesRequestPatch{
3138
RetentionEnabled: optionalBool(d, "retention_enabled"),
3239
RetentionGroupByName: optionalBool(d, "retention_group_by_name"),
3340
RetentionGroupByFormat: optionalBool(d, "retention_group_by_format"),
3441
RetentionGroupByPackageType: optionalBool(d, "retention_group_by_package_type"),
3542
RetentionPackageQueryString: nullableString(d, "retention_package_query_string"),
43+
RetentionCountLimit: &retentionCountLimit,
44+
RetentionDaysLimit: &retentionDaysLimit,
3645
}
3746

38-
// Explicitly set these values, even if they're zero
39-
if d.HasChange("retention_count_limit") {
40-
value := int64(d.Get("retention_count_limit").(int))
41-
updateData.RetentionCountLimit = &value
42-
}
43-
if d.HasChange("retention_days_limit") {
44-
value := int64(d.Get("retention_days_limit").(int))
45-
updateData.RetentionDaysLimit = &value
46-
}
47-
if d.HasChange("retention_size_limit") {
48-
value := int64(d.Get("retention_size_limit").(int))
49-
updateData.RetentionSizeLimit = &value
50-
}
47+
// For retention_size_limit, we need to always send the value to handle
48+
// the case where users explicitly set it to 0 (which would otherwise be
49+
// indistinguishable from "not set" using GetOk)
50+
retentionSizeLimit := int64(d.Get("retention_size_limit").(int))
51+
updateData.RetentionSizeLimit = &retentionSizeLimit
5152

5253
req = req.Data(updateData)
5354

‎cloudsmith/resource_repository_retention_rule_test.go‎

Lines changed: 70 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,27 @@ import (
77
"testing"
88

99
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
10+
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
1011
)
1112

13+
// testCheckResourceAttrWithMessage enhances output for attribute checks
14+
func testCheckResourceAttrWithMessage(resourceName, attrName, expected string) resource.TestCheckFunc {
15+
return func(s *terraform.State) error {
16+
rs, ok := s.RootModule().Resources[resourceName]
17+
if !ok {
18+
return fmt.Errorf("Not found: %s", resourceName)
19+
}
20+
actual, ok := rs.Primary.Attributes[attrName]
21+
if !ok {
22+
return fmt.Errorf("Attribute '%s' not found in resource '%s'. State: %#v", attrName, resourceName, rs.Primary.Attributes)
23+
}
24+
if actual != expected {
25+
return fmt.Errorf("Attribute '%s' in resource '%s' expected '%s', got '%s'. Full state: %#v", attrName, resourceName, expected, actual, rs.Primary.Attributes)
26+
}
27+
return nil
28+
}
29+
}
30+
1231
func TestAccRepositoryRetentionRule_basic(t *testing.T) {
1332
t.Parallel()
1433

@@ -26,14 +45,23 @@ func TestAccRepositoryRetentionRule_basic(t *testing.T) {
2645
Config: testAccRepositoryRetentionRuleConfigBasic,
2746
Check: resource.ComposeTestCheckFunc(
2847
testAccRepositoryCheckExists("cloudsmith_repository.test-retention"),
29-
resource.TestCheckResourceAttr("cloudsmith_repository_retention_rule.test", "retention_count_limit", "100"),
30-
resource.TestCheckResourceAttr("cloudsmith_repository_retention_rule.test", "retention_days_limit", "28"),
31-
resource.TestCheckResourceAttr("cloudsmith_repository_retention_rule.test", "retention_enabled", "false"),
32-
resource.TestCheckResourceAttr("cloudsmith_repository_retention_rule.test", "retention_group_by_name", "false"),
33-
resource.TestCheckResourceAttr("cloudsmith_repository_retention_rule.test", "retention_group_by_format", "false"),
34-
resource.TestCheckResourceAttr("cloudsmith_repository_retention_rule.test", "retention_group_by_package_type", "false"),
35-
resource.TestCheckResourceAttr("cloudsmith_repository_retention_rule.test", "retention_size_limit", "0"),
36-
resource.TestCheckResourceAttr("cloudsmith_repository_retention_rule.test", "retention_package_query_string", "name:test"),
48+
testCheckResourceAttrWithMessage("cloudsmith_repository_retention_rule.test", "retention_count_limit", "100"),
49+
testCheckResourceAttrWithMessage("cloudsmith_repository_retention_rule.test", "retention_days_limit", "28"),
50+
testCheckResourceAttrWithMessage("cloudsmith_repository_retention_rule.test", "retention_enabled", "false"),
51+
testCheckResourceAttrWithMessage("cloudsmith_repository_retention_rule.test", "retention_group_by_name", "false"),
52+
testCheckResourceAttrWithMessage("cloudsmith_repository_retention_rule.test", "retention_group_by_format", "false"),
53+
testCheckResourceAttrWithMessage("cloudsmith_repository_retention_rule.test", "retention_group_by_package_type", "false"),
54+
testCheckResourceAttrWithMessage("cloudsmith_repository_retention_rule.test", "retention_size_limit", "0"),
55+
testCheckResourceAttrWithMessage("cloudsmith_repository_retention_rule.test", "retention_package_query_string", "name:test"),
56+
),
57+
},
58+
{
59+
Config: testAccRepositoryRetentionRuleConfigZero,
60+
Check: resource.ComposeTestCheckFunc(
61+
testAccRepositoryCheckExists("cloudsmith_repository.test-retention"),
62+
testCheckResourceAttrWithMessage("cloudsmith_repository_retention_rule.test", "retention_count_limit", "0"),
63+
testCheckResourceAttrWithMessage("cloudsmith_repository_retention_rule.test", "retention_days_limit", "0"),
64+
testCheckResourceAttrWithMessage("cloudsmith_repository_retention_rule.test", "retention_size_limit", "0"),
3765
),
3866
},
3967
},
@@ -43,27 +71,47 @@ func TestAccRepositoryRetentionRule_basic(t *testing.T) {
4371

4472
var testAccRepositoryConfig = fmt.Sprintf(`
4573
resource "cloudsmith_repository" "test-retention" {
46-
name = "terraform-acc-repo-retention-rule"
47-
namespace = "%s"
74+
name = "terraform-acc-repo-retention-rule"
75+
namespace = "%s"
4876
}
4977
`, os.Getenv("CLOUDSMITH_NAMESPACE"))
5078

5179
var testAccRepositoryRetentionRuleConfigBasic = fmt.Sprintf(`
5280
resource "cloudsmith_repository" "test-retention" {
53-
name = "terraform-acc-repo-retention-rule"
54-
namespace = "%s"
81+
name = "terraform-acc-repo-retention-rule"
82+
namespace = "%s"
83+
}
84+
85+
resource "cloudsmith_repository_retention_rule" "test" {
86+
namespace = "%s"
87+
repository = cloudsmith_repository.test-retention.name
88+
retention_enabled = false
89+
retention_count_limit = 100
90+
retention_days_limit = 28
91+
retention_group_by_name = false
92+
retention_group_by_format = false
93+
retention_group_by_package_type = false
94+
retention_size_limit = 0
95+
retention_package_query_string = "name:test"
96+
}
97+
`, os.Getenv("CLOUDSMITH_NAMESPACE"), os.Getenv("CLOUDSMITH_NAMESPACE"))
98+
99+
var testAccRepositoryRetentionRuleConfigZero = fmt.Sprintf(`
100+
resource "cloudsmith_repository" "test-retention" {
101+
name = "terraform-acc-repo-retention-rule"
102+
namespace = "%s"
55103
}
56104
57105
resource "cloudsmith_repository_retention_rule" "test" {
58-
namespace = "%s"
59-
repository = cloudsmith_repository.test-retention.name
60-
retention_enabled = false
61-
retention_count_limit = 100
62-
retention_days_limit = 28
63-
retention_group_by_name = false
64-
retention_group_by_format = false
65-
retention_group_by_package_type = false
66-
retention_size_limit = 0
67-
retention_package_query_string = "name:test"
106+
namespace = "%s"
107+
repository = cloudsmith_repository.test-retention.name
108+
retention_enabled = false
109+
retention_count_limit = 0
110+
retention_days_limit = 0
111+
retention_group_by_name = false
112+
retention_group_by_format = false
113+
retention_group_by_package_type = false
114+
retention_size_limit = 0
115+
retention_package_query_string = "name:test"
68116
}
69117
`, os.Getenv("CLOUDSMITH_NAMESPACE"), os.Getenv("CLOUDSMITH_NAMESPACE"))

‎docs/resources/repository_retention.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,12 @@ The following arguments are supported:
4444
* `namespace` - (Required) The namespace of the repository.
4545
* `repository` - (Required) If true, the retention rules will be activated for the repository and settings will be updated.
4646
* `retention_enabled` - (Required) If true, the retention rules will be activated for the repository and settings will be updated.
47-
* `retention_count_limit` - (Optional) The maximum number of packages to retain. Must be between 0 and 10000.
47+
* `retention_count_limit` - (Optional) The maximum number of packages to retain. Must be between `0` and `10000`. Default set to 100 packages as part of repository creation.
4848
* `retention_days_limit` - (Optional) The number of days of packages to retain. Must be between `0` and `180`. Default set to 28 days as part of repository creation.
4949
* `retention_group_by_name` - (Optional) If true, retention will apply to groups of packages by name rather than all packages.
5050
* `retention_group_by_format` - (Optional) If true, retention will apply to packages by package formats rather than across all package formats.
5151
* `retention_group_by_package_type` - (Optional) If true, retention will apply to packages by package type rather than across all package types for one or more formats.
52-
* `retention_size_limit` - (Optional) The maximum total size (in bytes) of packages to retain. Must be between `0` and `21474836480` (21.47 GB / 21474.83 MB).
52+
* `retention_size_limit` - (Optional) The maximum total size (in bytes) of packages to retain. Must be between `0` and `20000000000` up to the maximum size of 20 GB (20,000,000,000 bytes).
5353
* `retention_package_query_string` - (Optional) A package search expression which, if provided, filters the packages to be deleted. For example, `name:foo` will only delete packages called 'foo'.
5454

5555
## Import

0 commit comments

Comments
 (0)