ENG-12949: Policy engine providers #547

gengdahlCyral · 2024-06-10T18:15:34Z

Description of the change

Adding terraform provider for policy v2 API: /v2/policies that are planned for relase in v4.15.

The resource handles policy resource life-cycle (create/update/delete and import)
The datasource handles read.
There is no TF resource that handles the filtering API (ie where you can search using posting of predicates).

Type of change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklists

Development

Lint rules pass locally
The code changed/added as part of this pull request has been covered with tests
All tests related to the changed code pass in development

Code review

This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
Jira issue referenced in commit message and/or PR title

Testing

Resource, creating three policies

terraform apply

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # cyral_policy_v2.approval_policy_example will be created
  + resource "cyral_policy_v2" "approval_policy_example" {
      + created      = (known after apply)
      + description  = "Approval policy for emergency access"
      + document     = jsonencode(
            {
              + approvals  = {
                  + delete = {
                      + locations = [
                          + "gym_db.users",
                        ]
                      + tags      = [
                          + "PII",
                        ]
                    }
                  + read   = {
                      + locations = [
                          + "gym_db.users",
                        ]
                      + tags      = [
                          + "PII",
                        ]
                    }
                  + update = {
                      + locations = [
                          + "gym_db.users",
                        ]
                      + tags      = [
                          + "PII",
                        ]
                    }
                }
              + conditions = [
                  + {
                      + attribute = "role"
                      + operator  = "equals"
                      + value     = "admin"
                    },
                ]
              + identity   = {
                  + email = "[email protected]"
                }
            }
        )
      + enabled      = true
      + id           = (known after apply)
      + last_updated = (known after apply)
      + name         = "approval_policy"
      + tags         = [
          + "approval",
          + "emergency",
        ]
      + type         = "approval"
      + valid_from   = "2024-06-12T00:00:00Z"
      + valid_until  = "2024-06-13T00:00:00Z"

      + scope {
          + repo_ids = [
              + "gym_repo",
            ]
        }
    }

  # cyral_policy_v2.global_policy_example will be created
  + resource "cyral_policy_v2" "global_policy_example" {
      + created      = (known after apply)
      + description  = "Global policy for finance users with row limit for PII data"
      + document     = jsonencode(
            {
              + governedData = {
                  + tags = [
                      + "PII",
                    ]
                }
              + readRules    = [
                  + {
                      + conditions  = [
                          + {
                              + attribute = "identity.userGroups"
                              + operator  = "contains"
                              + value     = "finance"
                            },
                        ]
                      + constraints = {
                          + maxRows = 5
                        }
                    },
                  + {
                      + conditions  = []
                      + constraints = {}
                    },
                ]
            }
        )
      + enabled      = true
      + enforced     = true
      + id           = (known after apply)
      + last_updated = (known after apply)
      + name         = "global_policy"
      + tags         = [
          + "finance",
          + "global",
        ]
      + type         = "global"

      + scope {
          + repo_ids = [
              + "2gaWEAyeKbETyUy1LSx985gVqrk",
            ]
        }
    }

  # cyral_policy_v2.local_policy_example will be created
  + resource "cyral_policy_v2" "local_policy_example" {
      + created      = (known after apply)
      + description  = "Local policy to allow gym users to read their own data"
      + document     = jsonencode(
            {
              + governedData = {
                  + locations = [
                      + "gym_db.users",
                    ]
                }
              + readRules    = [
                  + {
                      + conditions  = [
                          + {
                              + attribute = "identity.userGroups"
                              + operator  = "contains"
                              + value     = "users"
                            },
                        ]
                      + constraints = {
                          + datasetRewrite = "SELECT * FROM ${dataset} WHERE email = '${identity.endUserEmail}'"
                        }
                    },
                ]
            }
        )
      + enabled      = true
      + enforced     = true
      + id           = (known after apply)
      + last_updated = (known after apply)
      + name         = "local_policy"
      + tags         = [
          + "gym",
          + "local",
        ]
      + type         = "local"

      + scope {
          + repo_ids = [
              + "2gaWEAyeKbETyUy1LSx985gVqrk",
            ]
        }
    }

Plan: 3 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

cyral_policy_v2.local_policy_example: Creating...
cyral_policy_v2.global_policy_example: Creating...
cyral_policy_v2.approval_policy_example: Creating...
cyral_policy_v2.local_policy_example: Creation complete after 2s [id=c11d51eb-48f6-47d2-b39d-fdf694060885]
cyral_policy_v2.global_policy_example: Creation complete after 2s [id=33aa02d2-2007-4dcf-b2dd-4841c7970a2f]
cyral_policy_v2.approval_policy_example: Creation complete after 2s [id=107ebed2-c54c-4c00-9e3e-9435a04fb179]

Apply complete! Resources: 3 added, 0 changed, 0 destroyed.

Datasource, read a policy

terraform apply
data.cyral_policy_v2.test_policy: Reading...
cyral_policy_v2.approval_policy_example: Refreshing state... [id=107ebed2-c54c-4c00-9e3e-9435a04fb179]
cyral_policy_v2.local_policy_example: Refreshing state... [id=c11d51eb-48f6-47d2-b39d-fdf694060885]
cyral_policy_v2.global_policy_example: Refreshing state... [id=33aa02d2-2007-4dcf-b2dd-4841c7970a2f]
data.cyral_policy_v2.test_policy: Read complete after 1s [id=504f0cb9-4556-4f90-9819-bfe4a96910db]

Changes to Outputs:
  + policy_description = "Local policies for users table access"
  + policy_document    = jsonencode(
        {
          + governedData = {
              + locations = [
                  + "gym_db.users",
                ]
            }
          + readRules    = [
              + {
                  + conditions  = [
                      + {
                          + attribute = "identity.userGroups"
                          + operator  = "contains"
                          + value     = "USERS"
                        },
                    ]
                  + constraints = {
                      + datasetRewrite = "SELECT * FROM ${dataset} WHERE email = '${identity.endUserEmail}'"
                    }
                },
              + {
                  + conditions  = []
                  + constraints = {}
                },
            ]
        }
    )
  + policy_name        = "policy1"

You can apply this plan to save these new output values to the Terraform state, without changing any real infrastructure.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes


Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

policy_description = "Local policies for users table access"
policy_document = "{\"governedData\":{\"locations\":[\"gym_db.users\"]},\"readRules\":[{\"conditions\":[{\"attribute\":\"identity.userGroups\",\"operator\":\"contains\",\"value\":\"USERS\"}],\"constraints\":{\"datasetRewrite\":\"SELECT * FROM ${dataset} WHERE email = '${identity.endUserEmail}'\"}},{\"conditions\":[],\"constraints\":{}}]}"
policy_name = "policy1"

resource, update policies

goran@goran-engdahl-ThinkPad-P1-Gen-6:~/work/terraform$ terraform apply
cyral_policy_v2.local_policy_example: Refreshing state... [id=c11d51eb-48f6-47d2-b39d-fdf694060885]
cyral_policy_v2.global_policy_example: Refreshing state... [id=33aa02d2-2007-4dcf-b2dd-4841c7970a2f]
cyral_policy_v2.approval_policy_example: Refreshing state... [id=107ebed2-c54c-4c00-9e3e-9435a04fb179]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # cyral_policy_v2.approval_policy_example will be updated in-place
  ~ resource "cyral_policy_v2" "approval_policy_example" {
      ~ description  = "Approval policy for emergency access" -> "Approval policy for emergency access (updated)"
        id           = "107ebed2-c54c-4c00-9e3e-9435a04fb179"
        name         = "approval_policy"
        tags         = [
            "approval",
            "emergency",
        ]
        # (8 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

  # cyral_policy_v2.global_policy_example will be updated in-place
  ~ resource "cyral_policy_v2" "global_policy_example" {
      ~ description  = "Global policy for finance users with row limit for PII data" -> "Global policy for finance users with row limit for PII data (updated)"
        id           = "33aa02d2-2007-4dcf-b2dd-4841c7970a2f"
        name         = "global_policy"
        tags         = [
            "finance",
            "global",
        ]
        # (8 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

  # cyral_policy_v2.local_policy_example will be updated in-place
  ~ resource "cyral_policy_v2" "local_policy_example" {
      ~ description  = "Local policy to allow gym users to read their own data" -> "Local policy to allow gym users to read their own data (updated)"
        id           = "c11d51eb-48f6-47d2-b39d-fdf694060885"
        name         = "local_policy"
        tags         = [
            "gym",
            "local",
        ]
        # (8 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 3 to change, 0 to destroy.

Changes to Outputs:
  - policy_description = "Local policies for users table access" -> null
  - policy_document    = jsonencode(
        {
          - governedData = {
              - locations = [
                  - "gym_db.users",
                ]
            }
          - readRules    = [
              - {
                  - conditions  = [
                      - {
                          - attribute = "identity.userGroups"
                          - operator  = "contains"
                          - value     = "USERS"
                        },
                    ]
                  - constraints = {
                      - datasetRewrite = "SELECT * FROM ${dataset} WHERE email = '${identity.endUserEmail}'"
                    }
                },
              - {
                  - conditions  = []
                  - constraints = {}
                },
            ]
        }
    ) -> null
  - policy_name        = "policy1" -> null

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

cyral_policy_v2.global_policy_example: Modifying... [id=33aa02d2-2007-4dcf-b2dd-4841c7970a2f]
cyral_policy_v2.approval_policy_example: Modifying... [id=107ebed2-c54c-4c00-9e3e-9435a04fb179]
cyral_policy_v2.local_policy_example: Modifying... [id=c11d51eb-48f6-47d2-b39d-fdf694060885]
cyral_policy_v2.approval_policy_example: Modifications complete after 7s [id=107ebed2-c54c-4c00-9e3e-9435a04fb179]
cyral_policy_v2.local_policy_example: Modifications complete after 7s [id=c11d51eb-48f6-47d2-b39d-fdf694060885]
cyral_policy_v2.global_policy_example: Modifications complete after 7s [id=33aa02d2-2007-4dcf-b2dd-4841c7970a2f]

Apply complete! Resources: 0 added, 3 changed, 0 destroyed.
goran@goran-engdahl-ThinkPad-P1-Gen-6:~/work/terraform$

Deleting the policies

goran@goran-engdahl-ThinkPad-P1-Gen-6:~/work/terraform$ terraform apply
cyral_policy_v2.local_policy_example: Refreshing state... [id=c11d51eb-48f6-47d2-b39d-fdf694060885]
cyral_policy_v2.approval_policy_example: Refreshing state... [id=107ebed2-c54c-4c00-9e3e-9435a04fb179]
cyral_policy_v2.global_policy_example: Refreshing state... [id=33aa02d2-2007-4dcf-b2dd-4841c7970a2f]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  - destroy

Terraform will perform the following actions:

  # cyral_policy_v2.approval_policy_example will be destroyed
  # (because cyral_policy_v2.approval_policy_example is not in configuration)
  - resource "cyral_policy_v2" "approval_policy_example" {
      - created      = {
          - "actor"      = "sa/default/de84bc5b-8204-43cf-a6ab-e101db2c67f4"
          - "actor_type" = "ACTOR_TYPE_API_CLIENT"
          - "timestamp"  = "2024-06-12T12:04:52.411516731Z"
        } -> null
      - description  = "Approval policy for emergency access (updated)" -> null
      - document     = jsonencode(
            {
              - approvals  = {
                  - delete = {
                      - locations = [
                          - "gym_db.users",
                        ]
                      - tags      = [
                          - "PII",
                        ]
                    }
                  - read   = {
                      - locations = [
                          - "gym_db.users",
                        ]
                      - tags      = [
                          - "PII",
                        ]
                    }
                  - update = {
                      - locations = [
                          - "gym_db.users",
                        ]
                      - tags      = [
                          - "PII",
                        ]
                    }
                }
              - conditions = [
                  - {
                      - attribute = "role"
                      - operator  = "equals"
                      - value     = "admin"
                    },
                ]
              - identity   = {
                  - email = "[email protected]"
                }
            }
        ) -> null
      - enabled      = true -> null
      - enforced     = false -> null
      - id           = "107ebed2-c54c-4c00-9e3e-9435a04fb179" -> null
      - last_updated = {
          - "actor"      = "sa/default/de84bc5b-8204-43cf-a6ab-e101db2c67f4"
          - "actor_type" = "ACTOR_TYPE_API_CLIENT"
          - "timestamp"  = "2024-06-12T12:11:47.505772598Z"
        } -> null
      - name         = "approval_policy" -> null
      - tags         = [
          - "approval",
          - "emergency",
        ] -> null
      - type         = "approval" -> null
      - valid_from   = "2024-06-12T00:00:00Z" -> null
      - valid_until  = "2024-06-13T00:00:00Z" -> null

      - scope {
          - repo_ids = [
              - "gym_repo",
            ] -> null
        }
    }

  # cyral_policy_v2.global_policy_example will be destroyed
  # (because cyral_policy_v2.global_policy_example is not in configuration)
  - resource "cyral_policy_v2" "global_policy_example" {
      - created      = {
          - "actor"      = "sa/default/de84bc5b-8204-43cf-a6ab-e101db2c67f4"
          - "actor_type" = "ACTOR_TYPE_API_CLIENT"
          - "timestamp"  = "2024-06-12T12:04:52.424978774Z"
        } -> null
      - description  = "Global policy for finance users with row limit for PII data (updated)" -> null
      - document     = jsonencode(
            {
              - governedData = {
                  - tags = [
                      - "PII",
                    ]
                }
              - readRules    = [
                  - {
                      - conditions  = [
                          - {
                              - attribute = "identity.userGroups"
                              - operator  = "contains"
                              - value     = "finance"
                            },
                        ]
                      - constraints = {
                          - maxRows = 5
                        }
                    },
                  - {
                      - conditions  = []
                      - constraints = {}
                    },
                ]
            }
        ) -> null
      - enabled      = true -> null
      - enforced     = true -> null
      - id           = "33aa02d2-2007-4dcf-b2dd-4841c7970a2f" -> null
      - last_updated = {
          - "actor"      = "sa/default/de84bc5b-8204-43cf-a6ab-e101db2c67f4"
          - "actor_type" = "ACTOR_TYPE_API_CLIENT"
          - "timestamp"  = "2024-06-12T12:11:47.503363406Z"
        } -> null
      - name         = "global_policy" -> null
      - tags         = [
          - "finance",
          - "global",
        ] -> null
      - type         = "global" -> null
        # (2 unchanged attributes hidden)

      - scope {
          - repo_ids = [
              - "2gaWEAyeKbETyUy1LSx985gVqrk",
            ] -> null
        }
    }

  # cyral_policy_v2.local_policy_example will be destroyed
  # (because cyral_policy_v2.local_policy_example is not in configuration)
  - resource "cyral_policy_v2" "local_policy_example" {
      - created      = {
          - "actor"      = "sa/default/de84bc5b-8204-43cf-a6ab-e101db2c67f4"
          - "actor_type" = "ACTOR_TYPE_API_CLIENT"
          - "timestamp"  = "2024-06-12T12:04:52.418589521Z"
        } -> null
      - description  = "Local policy to allow gym users to read their own data (updated)" -> null
      - document     = jsonencode(
            {
              - governedData = {
                  - locations = [
                      - "gym_db.users",
                    ]
                }
              - readRules    = [
                  - {
                      - conditions  = [
                          - {
                              - attribute = "identity.userGroups"
                              - operator  = "contains"
                              - value     = "users"
                            },
                        ]
                      - constraints = {
                          - datasetRewrite = "SELECT * FROM ${dataset} WHERE email = '${identity.endUserEmail}'"
                        }
                    },
                ]
            }
        ) -> null
      - enabled      = true -> null
      - enforced     = true -> null
      - id           = "c11d51eb-48f6-47d2-b39d-fdf694060885" -> null
      - last_updated = {
          - "actor"      = "sa/default/de84bc5b-8204-43cf-a6ab-e101db2c67f4"
          - "actor_type" = "ACTOR_TYPE_API_CLIENT"
          - "timestamp"  = "2024-06-12T12:11:47.507796234Z"
        } -> null
      - name         = "local_policy" -> null
      - tags         = [
          - "gym",
          - "local",
        ] -> null
      - type         = "local" -> null
        # (2 unchanged attributes hidden)

      - scope {
          - repo_ids = [
              - "2gaWEAyeKbETyUy1LSx985gVqrk",
            ] -> null
        }
    }

Plan: 0 to add, 0 to change, 3 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

cyral_policy_v2.local_policy_example: Destroying... [id=c11d51eb-48f6-47d2-b39d-fdf694060885]
cyral_policy_v2.approval_policy_example: Destroying... [id=107ebed2-c54c-4c00-9e3e-9435a04fb179]
cyral_policy_v2.global_policy_example: Destroying... [id=33aa02d2-2007-4dcf-b2dd-4841c7970a2f]
cyral_policy_v2.local_policy_example: Destruction complete after 1s
cyral_policy_v2.global_policy_example: Destruction complete after 1s
cyral_policy_v2.approval_policy_example: Destruction complete after 2s

Apply complete! Resources: 0 added, 0 changed, 3 destroyed.

Import example

goran@goran-engdahl-ThinkPad-P1-Gen-6:~/work/terraform$ cp import.tf_example import.tf
goran@goran-engdahl-ThinkPad-P1-Gen-6:~/work/terraform$ terraform import cyral_policy_v2.imported_local_policy POLICY_TYPE_LOCAL/a296e3e5-cdb9-496c-a7bd-3088921be32f
cyral_policy_v2.imported_local_policy: Importing from ID "POLICY_TYPE_LOCAL/a296e3e5-cdb9-496c-a7bd-3088921be32f"...
cyral_policy_v2.imported_local_policy: Import prepared!
  Prepared cyral_policy_v2 for import
cyral_policy_v2.imported_local_policy: Refreshing state... [id=a296e3e5-cdb9-496c-a7bd-3088921be32f]

Import successful!

The resources that were imported are shown above. These resources are now in
your Terraform state and will henceforth be managed by Terraform.

goran@goran-engdahl-ThinkPad-P1-Gen-6:~/work/terraform$ grep -A10 a296e3e5-cdb9-496c-a7bd-3088921be32f terraform.tfstate
            "id": "a296e3e5-cdb9-496c-a7bd-3088921be32f",
            "last_updated": {
              "actor": "sa/default/de84bc5b-8204-43cf-a6ab-e101db2c67f4",
              "actor_type": "ACTOR_TYPE_API_CLIENT",
              "timestamp": "2024-06-12T09:24:11.790979830Z"
            },
            "name": "local_policy",
            "scope": [
              {
                "repo_ids": [
                  "2gaWEAyeKbETyUy1LSx985gVqrk"

yoursnerdly · 2024-06-11T02:03:44Z

cyral/internal/policyv2/datasource.go

+			"last_updated": {
+				Description: "Information about when and by whom the policy was last updated.",
+				Type:        schema.TypeMap,
+				Optional:    true,


This should be:

Suggested change

Optional: true,

Computed: true,

thanks; 4526ad6

yoursnerdly · 2024-06-11T02:04:02Z

cyral/internal/policyv2/datasource.go

+			"created": {
+				Description: "Information about when and by whom the policy was created.",
+				Type:        schema.TypeMap,
+				Optional:    true,


Suggested change

Optional: true,

Computed: true,

thanks 4526ad6

yoursnerdly · 2024-06-11T02:05:58Z

cyral/internal/policyv2/datasource.go

+			"document": {
+				Description: "The actual policy document in JSON format.",
+				Type:        schema.TypeString,
+				Optional:    true,


document is not optional, right?

yoursnerdly · 2024-06-11T02:07:21Z

cyral/internal/policyv2/datasource.go

+			"enforced": {
+				Description: "Indicates if the policy is enforced.",
+				Type:        schema.TypeBool,
+				Optional:    true,


Is there a way to specify what the default is? Not needed if tf assumes the default is the go default for the type.

You can specify, but looks right to me.
TF:

resource "cyral_policy_v2" "local_policy_example" { name = "minimal_local_policy" document = jsonencode({ governedData = { locations = ["gym_db.users"] } readRules = [ { conditions = [ { attribute = "identity.userGroups" operator = "contains" value = "users" } ] constraints = { datasetRewrite = "SELECT * FROM $${dataset} WHERE email = '$${identity.endUserEmail}'" } } ] }) type = "local" }

Created policy (GET from API):

{ "policy": { "id": "5250617c-c5de-43cc-954a-15b6682e97f6", "name": "minimal_local_policy", "description": "", "enabled": false, "scope": null, "tags": [], "validFrom": null, "validUntil": null, "document": "{\"governedData\":{\"locations\":[\"gym_db.users\"]},\"readRules\":[{\"conditions\":[{\"attribute\":\"identity.userGroups\",\"operator\":\"contains\",\"value\":\"users\"}],\"constraints\":{\"datasetRewrite\":\"SELECT * FROM ${dataset} WHERE email = '${identity.endUserEmail}'\"}}]}", "lastUpdated": null, "created": { "actor": "sa/default/de84bc5b-8204-43cf-a6ab-e101db2c67f4", "actorType": "ACTOR_TYPE_API_CLIENT", "timestamp": "2024-06-12T12:53:44.310243758Z" }, "enforced": false } }

There is a Default attribute in the Schema that can be used for that.

yoursnerdly · 2024-06-11T02:11:07Z

cyral/internal/policyv2/model.go

+	}
+
+	// Normalize the document field before setting it
+	normalizedDocument, err := normalizeJSON(r.Policy.Document)


Why do we need to normalize the JSON?

code was refactored

yoursnerdly · 2024-06-11T02:12:31Z

cyral/internal/policyv2/model.go

+
+	// Normalize the document field before storing it
+	document := d.Get("document").(string)
+	normalizedDocument, err := normalizeJSON(document)


As soon as we do this, we lose the original value of document provided by the resource definition (whitespace changes) - this may be the cause of the state mismatch that you're seeing.

code was refactored

yoursnerdly · 2024-06-11T02:13:05Z

cyral/internal/policyv2/model.go

+	r.Policy.LastUpdated = expandChangeInfo(d.Get("last_updated").(map[string]interface{}))
+	r.Policy.Created = expandChangeInfo(d.Get("created").(map[string]interface{}))


These should probably not be set here since these are computed fields.

yoursnerdly · 2024-06-11T02:14:21Z

cyral/internal/policyv2/model.go

+	if v == nil {
+		return ""
+	}
+	return v.(string)


This could panic if the actual value is not a string. Maybe you should return "" in the (unlikely) case that v is not a string?

yoursnerdly · 2024-06-11T02:15:05Z

cyral/internal/policyv2/resource.go

+				Computed: true,
+			},
+			"name": {
+				Description: "...",


needs to be fixed. Same for description below.

yoursnerdly · 2024-06-11T02:15:24Z

cyral/internal/policyv2/resource.go

+
+func resourceSchema() *schema.Resource {
+	return &schema.Resource{
+		Description:   "Some description.",


… in datasource.

…eing transferred from the schema to the API."

yoursnerdly

Looks OK to me at a high level - will lean on @wcmjunior for more detailed review.

I have some small suggestions that we might want to discuss.

yoursnerdly · 2024-06-12T14:57:09Z

cyral/internal/policyv2/constants.go

+package policyv2
+
+const (
+	resourceName    = "cyral_policy_v2"


Maybe something like cyral_custom_policy?

Let's keep as cyral_policy_v2 we will replace cyral_policy by this new resource in the next major version of the provider. Please also mark the cyral_policy as deprecated as we should encourage the move.

just rename the policy package with deprecated? Looks like that is what we've done before? Moving the files to deprecated folder won't work with this new model (as it will eventually lead to filename collisions)?

cyral/internal/policyv2/model.go

docs/resources/policy_v2.md

yoursnerdly · 2024-06-14T15:13:38Z

docs/resources/policy_v2.md

+}
+
+
+resource "cyral_policy_v2" "approval_policy_example" {


Approval policies are currently "invisible" - also proposed to be not documented to begin with.

Let's have a brief discussion in the next meeting on whether it's desirable to expose this construct in the docs (I don't have any mortal objection against this but we just need to be consistent - if we have them here, the documentation should also cover it).

fixed in 7833171

wcmjunior

Thanks for your PR @gengdahlCyral! Hopefully it was easier to create a new resource with the new abstractions. I added a few comments that may help shrink the code as you will reuse some existing functions and some other minor improvements. I will work on a more comprehensive review of the tests and examples in my next review.

wcmjunior · 2024-06-28T22:44:50Z

cyral/internal/policyv2/constants.go

+package policyv2
+
+const (
+	resourceName    = "cyral_policy_v2"


Let's keep as cyral_policy_v2 we will replace cyral_policy by this new resource in the next major version of the provider. Please also mark the cyral_policy as deprecated as we should encourage the move.

wcmjunior · 2024-06-28T22:46:01Z

cyral/internal/policyv2/datasource.go

@@ -0,0 +1,111 @@
+package policyv2


Instead of creating a package policyv2, it seems to me that creating a v2 package inside policy is better.

wcmjunior · 2024-06-28T23:52:37Z

cyral/internal/policyv2/model.go

+	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{}))


Suggested change

r.Policy.Tags = expandStringList(d.Get("tags").([]interface{}))

r.Policy.Tags = utils.ConvertFromInterfaceList(d.Get("tags").([]interface{}))

wcmjunior · 2024-06-28T23:54:03Z

cyral/internal/policyv2/model.go

+	}
+	m := s[0].(map[string]interface{})
+	scope := Scope{
+		RepoIds: expandStringList(m["repo_ids"].([]interface{})),


Suggested change

RepoIds: expandStringList(m["repo_ids"].([]interface{})),

RepoIds: utils.ConvertFromInterfaceList(m["repo_ids"].([]interface{})),

wcmjunior · 2024-06-29T00:03:37Z

cyral/internal/policyv2/model.go

+}
+
+// flattenScope converts the Scope struct to a list of maps
+func flattenScope(scope *Scope) []map[string]interface{} {


Scope should be taught how to perform this operation itself so you would call it like scope.ToMap() or some other more go-lang appropriate name. :-)

Suggested change

func flattenScope(scope *Scope) []map[string]interface{} {

func (s *Scope) ToMap() []map[string]interface{} {

wcmjunior · 2024-06-29T00:24:17Z

cyral/internal/policyv2/datasource.go

+	return &schema.Resource{
+		Description: "This data source provides information about a policy.",
+		ReadContext: dsContextHandler.ReadContext(),
+		Schema: map[string]*schema.Schema{


This schema is identical to the resource. You may simplify this code by calling utils.ConvertSchemaFieldsToComputed as shown here. The resource for token settings may help you simplify your code.

It's not, is it. I mean for the resources, you have to have some four fields, the rest are optional. For datasource you have to have two fields (type, id) - but not name & document or any other field.
Simplification/re-use can still be done but I kept as-is.

wcmjunior · 2024-06-29T00:30:19Z

cyral/utils/testutils.go

+// 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 {


I understand that FormatPolicyIntoConfig was put here so it can be reused for other tests, but it doesn't seem to be the case that we would reuse FormatScope or FormatTags, correct? If I'm correct, they could be changed to formatScope and formatTags/

docs/resources/policy_v2.md

wcmjunior · 2024-06-29T00:32:59Z

docs/resources/policy_v2.md

+  enabled     = true
+  tags        = ["finance", "global"]
+  scope {
+    repo_ids = ["2gaWEAyeKbETyUy1LSx985gVqrk"]


Please create a repo as part of the example using cyral_repository and provide its ID from the attribute instead:

Suggested change

repo_ids = ["2gaWEAyeKbETyUy1LSx985gVqrk"]

repo_ids = [cyral_repository.myrepo.id]

wcmjunior · 2024-06-29T00:33:24Z

examples/resources/cyral_policy_v2/resource.tf

+  enabled     = true
+  tags        = ["gym", "local"]
+  scope {
+    repo_ids = ["2gaWEAyeKbETyUy1LSx985gVqrk"]


Same as previous repo comment.

gengdahlCyral · 2024-07-02T09:28:30Z

Not sure how we want to deprecate, tried update md file - but that is overwritten part of make targets.
@wcmjunior @yoursnerdly
Other than that I think we are good.

sonarqubecloud · 2024-07-03T06:04:14Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

wcmjunior · 2024-07-03T06:05:32Z

Changes I've made:

Deprecated the old resource cyral_poliy
Fixed a bug introduced in the refactoring
Fixed docs as the example would not work if deployed as is
Bumped dependencies and golang

yoursnerdly reviewed Jun 11, 2024

View reviewed changes

Consolidated changes for policy engine providers

b2ad3ba

gengdahlCyral force-pushed the policy-engine-providers branch from c8f08ec to b2ad3ba Compare June 12, 2024 09:47

gengdahlCyral added 6 commits June 12, 2024 13:43

Added tf resource example, updated descriptions and tmpl file

9636a60

Added tf resource example, updated descriptions and tmpl file

83911e5

Review comment; changed optional to computed for read-only attributes…

4526ad6

… in datasource.

Review comment; The 'last updated' and 'created' attributes are not b…

b02c3af

…eing transferred from the schema to the API."

merge main

84dc008

go sum

b295c84

gengdahlCyral marked this pull request as ready for review June 12, 2024 13:09

gengdahlCyral requested review from yoursnerdly and wcmjunior June 12, 2024 13:09

gengdahlCyral changed the title ~~Policy engine providers~~ ENG-12949: Policy engine providers Jun 12, 2024

yoursnerdly reviewed Jun 14, 2024

View reviewed changes

gengdahlCyral added 2 commits June 18, 2024 10:01

Removed unused function, hiding approval from examples and docs, up-deps

7833171

Hiding approval from import syntax in docs as well

a0b8969

wcmjunior reviewed Jun 29, 2024

View reviewed changes

gengdahlCyral added 3 commits July 1, 2024 13:08

File structure refactor

ad95877

review comments

6c598d5

Removed a comment and update dependencies

8f0e30a

gengdahlCyral requested review from wcmjunior and yoursnerdly July 2, 2024 09:27

wcmjunior added 5 commits July 2, 2024 17:45

Deprecate resource cyral_policy

71049f1

Bump golang to v1.22

bf2eae2

Fix test error

b5d553f

Fix docs

a3fd663

Bump golang to v1.22

5736787

wcmjunior approved these changes Jul 3, 2024

View reviewed changes

wcmjunior merged commit 7e99c93 into main Jul 3, 2024
2 checks passed

wcmjunior deleted the policy-engine-providers branch July 3, 2024 06:05

		r.Policy.LastUpdated = expandChangeInfo(d.Get("last_updated").(map[string]interface{}))
		r.Policy.Created = expandChangeInfo(d.Get("created").(map[string]interface{}))

	r.Policy.Tags = expandStringList(d.Get("tags").([]interface{}))
	r.Policy.Tags = utils.ConvertFromInterfaceList(d.Get("tags").([]interface{}))

	RepoIds: expandStringList(m["repo_ids"].([]interface{})),
	RepoIds: utils.ConvertFromInterfaceList(m["repo_ids"].([]interface{})),

	func flattenScope(scope *Scope) []map[string]interface{} {
	func (s *Scope) ToMap() []map[string]interface{} {

	repo_ids = ["2gaWEAyeKbETyUy1LSx985gVqrk"]
	repo_ids = [cyral_repository.myrepo.id]

ENG-12949: Policy engine providers #547

ENG-12949: Policy engine providers #547

Conversation

gengdahlCyral commented Jun 10, 2024 • edited Loading

Description of the change

Type of change

Checklists

Development

Code review

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yoursnerdly left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wcmjunior left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gengdahlCyral commented Jul 2, 2024

sonarqubecloud bot commented Jul 3, 2024

Quality Gate passed

wcmjunior commented Jul 3, 2024

gengdahlCyral commented Jun 10, 2024 •

edited

Loading