artifactregistry: accept all valid durations (#12667)

Co-authored-by: Chris Hawk <[email protected]> [upstream:94499923e80f3168760b9a75e0b3d3a5c07e67e3] Signed-off-by: Modular Magician <[email protected]>
hashicorp · Jan 13, 2025 · 13bfaa9 · 13bfaa9
1 parent 4246cef
commit 13bfaa9
Show file tree

Hide file tree

Showing 4 changed files with 149 additions and 6 deletions.
diff --git a/.changelog/12667.txt b/.changelog/12667.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+artifactregistry: fix `artifact_registry_repository` not accepting durations with 'm', 'h' or 'd'
+```
diff --git a/google-beta/services/artifactregistry/resource_artifact_registry_repository.go b/google-beta/services/artifactregistry/resource_artifact_registry_repository.go
@@ -22,6 +22,7 @@ import (
 	"log"
 	"net/http"
 	"reflect"
+	"strconv"
 	"strings"
 	"time"
 
@@ -67,6 +68,58 @@ func upstreamPoliciesDiffSuppress(k, old, new string, d *schema.ResourceData) bo
 	return oldSet.Equal(newSet)
 }
 
+func parseDurationAsSeconds(v string) (int, bool) {
+	if len(v) == 0 {
+		return 0, false
+	}
+	n, err := strconv.Atoi(v[:len(v)-1])
+	if err != nil {
+		return 0, false
+	}
+	switch v[len(v)-1] {
+	case 's':
+		return n, true
+	case 'm':
+		return n * 60, true
+	case 'h':
+		return n * 3600, true
+	case 'd':
+		return n * 86400, true
+	default:
+		return 0, false
+	}
+}
+
+// Like tpgresource.DurationDiffSuppress, but supports 'd'
+func durationDiffSuppress(k, oldr, newr string, d *schema.ResourceData) bool {
+	o, n := d.GetChange(k)
+	old, ok := o.(string)
+	if !ok {
+		return false
+	}
+	new, ok := n.(string)
+	if !ok {
+		return false
+	}
+	if old == new {
+		return true
+	}
+	oldSeconds, ok := parseDurationAsSeconds(old)
+	if !ok {
+		return false
+	}
+	newSeconds, ok := parseDurationAsSeconds(new)
+	if !ok {
+		return false
+	}
+	return oldSeconds == newSeconds
+}
+
+func mapHashID(v any) int {
+	obj := v.(map[string]any)
+	return schema.HashString(obj["id"])
+}
+
 func ResourceArtifactRegistryRepository() *schema.Resource {
 	return &schema.Resource{
 		Create: resourceArtifactRegistryRepositoryCreate,
@@ -136,13 +189,13 @@ unique within a repository and be under 128 characters in length.`,
 									"newer_than": {
 										Type:             schema.TypeString,
 										Optional:         true,
-										DiffSuppressFunc: tpgresource.DurationDiffSuppress,
+										DiffSuppressFunc: durationDiffSuppress,
 										Description:      `Match versions newer than a duration.`,
 									},
 									"older_than": {
 										Type:             schema.TypeString,
 										Optional:         true,
-										DiffSuppressFunc: tpgresource.DurationDiffSuppress,
+										DiffSuppressFunc: durationDiffSuppress,
 										Description:      `Match versions older than a duration.`,
 									},
 									"package_name_prefixes": {
@@ -205,6 +258,7 @@ specified with a Keep action.`,
 						},
 					},
 				},
+				Set: mapHashID,
 			},
 			"cleanup_policy_dry_run": {
 				Type:     schema.TypeBool,
@@ -2095,11 +2149,67 @@ func expandArtifactRegistryRepositoryCleanupPoliciesConditionPackageNamePrefixes
 }
 
 func expandArtifactRegistryRepositoryCleanupPoliciesConditionOlderThan(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
-	return v, nil
+	if v == nil {
+		return nil, nil
+	}
+	val, ok := v.(string)
+	if !ok {
+		return nil, fmt.Errorf("unexpected value is not string: %v", v)
+	}
+	if len(val) == 0 {
+		return nil, nil
+	}
+	n, err := strconv.Atoi(val[:len(val)-1])
+	if err != nil {
+		return nil, fmt.Errorf("unexpected value is not duration: %v", v)
+	}
+	// time.ParseDuration does not support 'd'
+	var seconds int
+	switch val[len(val)-1] {
+	case 's':
+		seconds = n
+	case 'm':
+		seconds = n * 60
+	case 'h':
+		seconds = n * 3600
+	case 'd':
+		seconds = n * 86400
+	default:
+		return nil, fmt.Errorf("unexpected duration has unknown unit: %c", val[len(val)-1])
+	}
+	return fmt.Sprintf("%ds", seconds), nil
 }
 
 func expandArtifactRegistryRepositoryCleanupPoliciesConditionNewerThan(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
-	return v, nil
+	if v == nil {
+		return nil, nil
+	}
+	val, ok := v.(string)
+	if !ok {
+		return nil, fmt.Errorf("unexpected value is not string: %v", v)
+	}
+	if len(val) == 0 {
+		return nil, nil
+	}
+	n, err := strconv.Atoi(val[:len(val)-1])
+	if err != nil {
+		return nil, fmt.Errorf("unexpected value is not duration: %v", v)
+	}
+	// time.ParseDuration does not support 'd'
+	var seconds int
+	switch val[len(val)-1] {
+	case 's':
+		seconds = n
+	case 'm':
+		seconds = n * 60
+	case 'h':
+		seconds = n * 3600
+	case 'd':
+		seconds = n * 86400
+	default:
+		return nil, fmt.Errorf("unexpected duration has unknown unit: %c", val[len(val)-1])
+	}
+	return fmt.Sprintf("%ds", seconds), nil
 }
 
 func expandArtifactRegistryRepositoryCleanupPoliciesMostRecentVersions(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {

diff --git a/...le-beta/services/artifactregistry/resource_artifact_registry_repository_generated_test.go b/...le-beta/services/artifactregistry/resource_artifact_registry_repository_generated_test.go
@@ -423,13 +423,28 @@ resource "google_artifact_registry_repository" "my-repo" {
   description   = "example docker repository with cleanup policies%{random_suffix}"
   format        = "DOCKER"
   cleanup_policy_dry_run = false
+  cleanup_policies {
+    id     = "delete-untagged"
+    action = "DELETE"
+    condition {
+      tag_state    = "UNTAGGED"
+    }
+  }
+  cleanup_policies {
+    id     = "keep-new-untagged"
+    action = "KEEP"
+    condition {
+      tag_state    = "UNTAGGED"
+      newer_than   = "7d"
+    }
+  }
   cleanup_policies {
     id     = "delete-prerelease"
     action = "DELETE"
     condition {
       tag_state    = "TAGGED"
       tag_prefixes = ["alpha", "v0"]
-      older_than   = "2592000s"
+      older_than   = "30d"
     }
   }
   cleanup_policies {

diff --git a/website/docs/r/artifact_registry_repository.html.markdown b/website/docs/r/artifact_registry_repository.html.markdown
@@ -242,13 +242,28 @@ resource "google_artifact_registry_repository" "my-repo" {
   description   = "example docker repository with cleanup policies"
   format        = "DOCKER"
   cleanup_policy_dry_run = false
+  cleanup_policies {
+    id     = "delete-untagged"
+    action = "DELETE"
+    condition {
+      tag_state    = "UNTAGGED"
+    }
+  }
+  cleanup_policies {
+    id     = "keep-new-untagged"
+    action = "KEEP"
+    condition {
+      tag_state    = "UNTAGGED"
+      newer_than   = "7d"
+    }
+  }
   cleanup_policies {
     id     = "delete-prerelease"
     action = "DELETE"
     condition {
       tag_state    = "TAGGED"
       tag_prefixes = ["alpha", "v0"]
-      older_than   = "2592000s"
+      older_than   = "30d"
     }
   }
   cleanup_policies {