review comments

cyral/internal/policy/v2/model.go

@@ -3,6 +3,7 @@ package policyv2
 import (
 	"fmt"
 
+	"github.com/cyralinc/terraform-provider-cyral/cyral/utils"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
@@ -13,6 +14,15 @@ type ChangeInfo struct {
 	Timestamp string `json:"timestamp,omitempty"`
 }
 
+// ToMap converts ChangeInfo to a map
+func (c ChangeInfo) ToMap() map[string]interface{} {
+	return map[string]interface{}{
+		"actor":      c.Actor,
+		"actor_type": c.ActorType,
+		"timestamp":  c.Timestamp,
+	}
+}
+
 // PolicyV2 represents the top-level policy structure
 type PolicyV2 struct {
 	Policy Policy `json:"policy,omitempty"`
@@ -22,6 +32,15 @@ type Scope struct {
 	RepoIds []string `json:"repoIds,omitempty"`
 }
 
+// ToMap converts Scope to a list of maps
+func (s *Scope) ToMap() []map[string]interface{} {
+	return []map[string]interface{}{
+		{
+			"repo_ids": s.RepoIds,
+		},
+	}
+}
+
 // Policy represents the policy details
 type Policy struct {
 	ID          string     `json:"id,omitempty"`
@@ -62,23 +81,15 @@ func (r PolicyV2) WriteToSchema(d *schema.ResourceData) error {
 	if err := d.Set("valid_until", r.Policy.ValidUntil); err != nil {
 		return fmt.Errorf("error setting 'valid_until' field: %w", err)
 	}
-
 	if err := d.Set("document", r.Policy.Document); err != nil {
 		return fmt.Errorf("error setting 'document' field: %w", err)
 	}
 
-	if err := d.Set("last_updated", map[string]interface{}{
-		"actor":      r.Policy.LastUpdated.Actor,
-		"actor_type": r.Policy.LastUpdated.ActorType,
-		"timestamp":  r.Policy.LastUpdated.Timestamp,
-	}); err != nil {
+	// Use the ToMap method to set the last_updated and created fields
+	if err := d.Set("last_updated", r.Policy.LastUpdated.ToMap()); err != nil {
 		return fmt.Errorf("error setting 'last_updated' field: %w", err)
 	}
-	if err := d.Set("created", map[string]interface{}{
-		"actor":      r.Policy.Created.Actor,
-		"actor_type": r.Policy.Created.ActorType,
-		"timestamp":  r.Policy.Created.Timestamp,
-	}); err != nil {
+	if err := d.Set("created", r.Policy.Created.ToMap()); err != nil {
 		return fmt.Errorf("error setting 'created' field: %w", err)
 	}
 	if err := d.Set("enforced", r.Policy.Enforced); err != nil {
@@ -89,7 +100,7 @@ func (r PolicyV2) WriteToSchema(d *schema.ResourceData) error {
 			return fmt.Errorf("error setting 'type' field: %w", err)
 		}
 	}
-	if err := d.Set("scope", flattenScope(r.Policy.Scope)); err != nil {
+	if err := d.Set("scope", r.Policy.Scope.ToMap()); err != nil { // Use the new ToMap method for Scope
 		return fmt.Errorf("error setting 'scope' field: %w", err)
 	}
 	d.SetId(r.Policy.ID)
@@ -102,7 +113,7 @@ func (r *PolicyV2) ReadFromSchema(d *schema.ResourceData) error {
 	r.Policy.Name = d.Get("name").(string)
 	r.Policy.Description = d.Get("description").(string)
 	r.Policy.Enabled = d.Get("enabled").(bool)
-	r.Policy.Tags = expandStringList(d.Get("tags").([]interface{}))
+	r.Policy.Tags = utils.ConvertFromInterfaceList[string](d.Get("tags").([]interface{}))
 	r.Policy.ValidFrom = d.Get("valid_from").(string)
 	r.Policy.ValidUntil = d.Get("valid_until").(string)
 	r.Policy.Document = d.Get("document").(string)
@@ -114,36 +125,14 @@ func (r *PolicyV2) ReadFromSchema(d *schema.ResourceData) error {
 	return nil
 }
 
-// expandStringList converts a list of interface{} to a list of strings
-func expandStringList(list []interface{}) []string {
-	result := make([]string, len(list))
-	for i, v := range list {
-		result[i] = v.(string)
-	}
-	return result
-}
-
-// flattenScope converts the Scope struct to a list of maps
-func flattenScope(scope *Scope) []map[string]interface{} {
-	if scope == nil {
-		return nil
-	}
-	scopeMap := []map[string]interface{}{
-		{
-			"repo_ids": scope.RepoIds,
-		},
-	}
-	return scopeMap
-}
-
 // scopeFromInterface converts the map to a Scope struct
 func scopeFromInterface(s []interface{}) *Scope {
 	if len(s) == 0 || s[0] == nil {
 		return nil
 	}
 	m := s[0].(map[string]interface{})
 	scope := Scope{
-		RepoIds: expandStringList(m["repo_ids"].([]interface{})),
+		RepoIds: utils.ConvertFromInterfaceList[string](m["repo_ids"].([]interface{})),
 	}
 	return &scope
 }

cyral/internal/policy/v2/resource.go

@@ -3,6 +3,7 @@ package policyv2
 import (
 	"context"
 	"fmt"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 
@@ -29,6 +30,10 @@ var resourceContextHandler = core.DefaultContextHandler{
 	},
 }
 
+func PolicyTypes() []string {
+	return []string{"POLICY_TYPE_GLOBAL", "global", "POLICY_TYPE_LOCAL", "local", "POLICY_TYPE_APPROVAL", "approval"}
+}
+
 func resourceSchema() *schema.Resource {
 	return &schema.Resource{
 		Description:   "This resource allows management of various types of policies in the Cyral platform. Policies can be used to define access controls, data governance rules to ensure compliance and security within your database environment.",
@@ -118,23 +123,10 @@ func resourceSchema() *schema.Resource {
 				Optional:    true,
 			},
 			"type": {
-				Description: "Type of the policy, one of [`local`, `global`]",
-				Type:        schema.TypeString,
-				Required:    true,
-				ValidateFunc: func(val interface{}, key string) (warns []string, errs []error) {
-					v := val.(string)
-					// Valid values for the type fields have aliases ie POLICY_TYPE_GLOBAL and global are the same.
-					// This is consistent with the API, which accepts both.
-					// However, the recommendation is to stick to [local, global, approval].
-					validTypes := []string{"POLICY_TYPE_GLOBAL", "global", "POLICY_TYPE_LOCAL", "local", "POLICY_TYPE_APPROVAL", "approval"}
-					for _, validType := range validTypes {
-						if v == validType {
-							return
-						}
-					}
-					errs = append(errs, fmt.Errorf("%q must be one of %v, got: %s", key, validTypes, v))
-					return
-				},
+				Description:  "Type of the policy, one of [`local`, `global`]",
+				Type:         schema.TypeString,
+				Required:     true,
+				ValidateFunc: validation.StringInSlice(append(PolicyTypes(), ""), false),
 			},
 		},
 	}

cyral/utils/testutils.go

@@ -288,10 +288,10 @@ resource "cyral_policy_v2" "%s" {
 		config.WriteString(fmt.Sprintf("  enabled     = %v\n", enabled))
 	}
 	if tags, ok := policy["tags"]; ok {
-		config.WriteString(fmt.Sprintf("  tags        = %s\n", FormatTags(tags)))
+		config.WriteString(fmt.Sprintf("  tags        = %s\n", formatTags(tags)))
 	}
 	if scope, ok := policy["scope"]; ok {
-		config.WriteString(fmt.Sprintf("  scope       %s\n", FormatScope(scope)))
+		config.WriteString(fmt.Sprintf("  scope       %s\n", formatScope(scope)))
 	}
 	if validFrom, ok := policy["valid_from"]; ok {
 		config.WriteString(fmt.Sprintf("  valid_from  = \"%s\"\n", validFrom))
@@ -315,10 +315,10 @@ resource "cyral_policy_v2" "%s" {
 	return config.String()
 }
 
-// FormatScope formats the scope data into a string suitable for Terraform configuration.
+// formatScope formats the scope data into a string suitable for Terraform configuration.
 //
 // scope is an interface that should be a map with string keys and slice of strings as values.
-func FormatScope(scope interface{}) string {
+func formatScope(scope interface{}) string {
 	if scope == nil {
 		return "= null"
 	}
@@ -331,17 +331,17 @@ func FormatScope(scope interface{}) string {
 	var formattedScope strings.Builder
 	formattedScope.WriteString("{\n")
 	for _, value := range scopeMap {
-		formattedScope.WriteString(fmt.Sprintf(`  %s = %s`, "repo_ids", FormatTags(value)))
+		formattedScope.WriteString(fmt.Sprintf(`  %s = %s`, "repo_ids", formatTags(value)))
 	}
 	formattedScope.WriteString("\n}")
 
 	return formattedScope.String()
 }
 
-// FormatTags formats a slice of tags into a string suitable for Terraform configuration.
+// formatTags formats a slice of tags into a string suitable for Terraform configuration.
 //
 // tags is an interface that should be a slice of strings.
-func FormatTags(tags interface{}) string {
+func formatTags(tags interface{}) string {
 	if tags == nil {
 		return "null"
 	}

docs/resources/policy_v2.md

@@ -13,13 +13,27 @@ This resource allows management of various types of policies in the Cyral platfo
 ## Example Usage
 
 ```terraform
+resource "cyral_repository" "myrepo" {
+    type = "mongodb"
+    name = "myrepo"
+
+    repo_node {
+        name = "node-1"
+        host = "mongodb.cyral.com"
+        port = 27017
+    }
+
+    mongodb_settings {
+      server_type = "standalone"
+    }
+}
 resource "cyral_policy_v2" "local_policy_example" {
   name        = "local_policy"
   description = "Local policy to allow gym users to read their own data"
   enabled     = true
   tags        = ["gym", "local"]
   scope {
-    repo_ids = ["2gaWEAyeKbETyUy1LSx985gVqrk"]
+    repo_ids = ["cyral_repository.myrepo.id"]
   }
   document    = jsonencode({
     governedData = {
@@ -50,7 +64,7 @@ resource "cyral_policy_v2" "global_policy_example" {
   enabled     = true
   tags        = ["finance", "global"]
   scope {
-    repo_ids = ["2gaWEAyeKbETyUy1LSx985gVqrk"]
+    repo_ids = ["cyral_repository.myrepo.id"]
   }
   document    = jsonencode({
     governedData = {

examples/resources/cyral_policy_v2/resource.tf

@@ -1,10 +1,24 @@
+resource "cyral_repository" "myrepo" {
+    type = "mongodb"
+    name = "myrepo"
+
+    repo_node {
+        name = "node-1"
+        host = "mongodb.cyral.com"
+        port = 27017
+    }
+
+    mongodb_settings {
+      server_type = "standalone"
+    }
+}
 resource "cyral_policy_v2" "local_policy_example" {
   name        = "local_policy"
   description = "Local policy to allow gym users to read their own data"
   enabled     = true
   tags        = ["gym", "local"]
   scope {
-    repo_ids = ["2gaWEAyeKbETyUy1LSx985gVqrk"]
+    repo_ids = ["cyral_repository.myrepo.id"]
   }
   document    = jsonencode({
     governedData = {
@@ -35,7 +49,7 @@ resource "cyral_policy_v2" "global_policy_example" {
   enabled     = true
   tags        = ["finance", "global"]
   scope {
-    repo_ids = ["2gaWEAyeKbETyUy1LSx985gVqrk"]
+    repo_ids = ["cyral_repository.myrepo.id"]
   }
   document    = jsonencode({
     governedData = {