ENG-14563: Policy Sets #579
Conversation
gengdahlCyral
requested review from
wcmjunior and
yoursnerdly
and removed request for
cyral-bot
| ResourceType: resourcetype.DataSource, | ||
| SchemaWriterFactoryGetMethod: func(_ *schema.ResourceData) core.SchemaWriter { return &PolicySet{} }, | ||
| ReadUpdateDeleteURLFactory: func(d *schema.ResourceData, c *client.Client) string { | ||
| return fmt.Sprintf("https://%s/%s/%s", c.ControlPlane, apiPathPolicySet, d.Get("id").(string)) |
There was a problem hiding this comment.
Question about type for the scope in the schema, otherwise looks fine to me.
I'm not too familiar with terraform, @wcmjunior is likely to have more feedback.
| }, | ||
| "scope": { | ||
| Description: "Scope of the policy set.", | ||
| Type: schema.TypeList, |
There was a problem hiding this comment.
Should this not be TypeMap?
cyral/internal/policyset/v1/model.go
Outdated
| } | ||
|
|
||
| // ToMap converts Scope to a list of maps | ||
| func (s *Scope) ToMap() []map[string]interface{} { |
There was a problem hiding this comment.
In line with my question on schema definition: I'm confused. Why should the scope be represented as a list of maps (the map containing a single key repo_ids for which the value is a list of strings)?
There was a problem hiding this comment.
Policy uses list, rego policies set, I think I had a discussion on this with Pedro but can't recall. Right now it's aligned with other resources we have, not sure we want to change this?
@wcmjunior any input here?
Here is a test I did.
Both the resource and the dataset use type list (as policy resource/datasource also does).
And it works as expected on create, eg you do:
resource "cyral_policy_set" "repo_lockdown_example" {
wizard_id = "repo-lockdown"
name = "no block"
description = "This default policy will block by default all queries for myrepo"
enabled = true
tags = ["default block", "fail closed"]
scope {
repo_ids = [cyral_repository.myrepo.id,cyral_repository.myrepo.id]
}
wizard_parameters = jsonencode({
denyByDefault = true
failClosed = true
})
}
which will create a set with those two repos in scope. Reading it back gives you this.
terraform apply
cyral_repository.myrepo2: Refreshing state... [id=2pRnxNKLVIvEnn2t5ZRRRfTICwO]
cyral_repository.myrepo: Refreshing state... [id=2pRnxKQivus0V1W884De2Bb9shX]
cyral_policy_set.repo_lockdown_example: Refreshing state... [id=eba9157f-ff97-4abc-ae9e-9c48d4eedb03]
data.cyral_policy_set.repo_lockdown_example: Reading...
data.cyral_policy_set.repo_lockdown_example: Read complete after 0s [id=eba9157f-ff97-4abc-ae9e-9c48d4eedb03]
Changes to Outputs:
+ description = "This default policy will block by default all queries for myrepo"
+ name = "no block"
+ scope = [
+ {
+ repo_ids = [
+ "2pRnxKQivus0V1W884De2Bb9shX",
+ "2pRnxKQivus0V1W884De2Bb9shX",
]
},
]
+ wizard_parameters = jsonencode(
{
+ denyByDefault = true
+ failClosed = true
}
)
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:
description = "This default policy will block by default all queries for myrepo"
name = "no block"
scope = tolist([
{
"repo_ids" = tolist([
"2pRnxKQivus0V1W884De2Bb9shX",
"2pRnxKQivus0V1W884De2Bb9shX",
])
},
])
wizard_parameters = "{\"denyByDefault\":true,\"failClosed\":true}"
There was a problem hiding this comment.
The SDK does not support defining a TypeMap with an Elem of *schema.Resource{}. Attempting to do so results in an error:
TypeMap with Elem *Resource not supported, use TypeList/TypeSet with Elem *Resource or TypeMap with Elem *Schema
The scope field represents a nested block that can contain multiple attributes and potentially be extended in the future. Using a TypeList with an Elem of *schema.Resource{} allows us to define a complex nested structure and is therefore what we need to do here (or possibly go with set).
There was a problem hiding this comment.
Would TypeMap with Elem *Schema work? Sorry this may be a dumb question, I don't quite understand the difference between Resource and Schema in this context.
| @@ -1,7 +1,8 @@ | |||
| package policywizardv1 | |||
| package policysetv1 | |||
There was a problem hiding this comment.
There is no need for v1.
| @@ -1,4 +1,4 @@ | |||
| package policywizardv1_test | |||
| package policysetv1_test | |||
… but only management of policy sets.
yoursnerdly
force-pushed
the
policy_wizards/1
branch
from
aff3acd to
2332696
Compare
yoursnerdly
force-pushed
the
policy_wizards/1
branch
from
2332696 to
375715e
Compare
|
Ran acceptance and manual tests again, and everything appears to be working as expected. |
| @@ -13,13 +13,6 @@ import ( | |||
| "github.com/cyralinc/terraform-provider-cyral/cyral/utils" | |||
| ) | |||
|
|
|||
| // ChangeInfo represents information about changes to the policy | |||
There was a problem hiding this comment.
This is unused and was inadvertently left over in the previous PR.
|
Description of the change
The pull request introduces a new Terraform resource and data source named
cyral_policy_setto the Cyral Terraform provider.This addition enables users to manage policy sets in the Cyral platform using the Policy Wizard V1 API through Terraform. The PR includes the implementation of the resource and data source, schema definitions, models, and accompanying tests. It also updates the provider's documentation and examples to reflect the new resource. Manual tests included in the PR confirm that users can successfully create, read, update, import, and delete policy sets using Terraform configurations with the new resource.
Type of change
Checklists
Development
Code review
Testing
Create a policy set and get it by curl
Update the policy set
Testing data source
Delete policy set
Importing a policy set