-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(secure): Add Malware, Drift, ML and AWS ML policy resources #476
Conversation
Error: Invalid argument name. Argument names must not be quoted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late review, still checking some files
} | ||
|
||
func dataSourceSysdigSecureAWSMLPolicyRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { | ||
return awsMLPolicyDataSourceRead(ctx, d, meta, "custom policy", isCustomCompositePolicy) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the message be more clear here "custom AWS ML policy"? Similarly for other types
} | ||
} | ||
|
||
func malwarePolicyDataSourceRead(ctx context.Context, d *schema.ResourceData, meta interface{}, resourceName string, validationFunc func(v2.PolicyRulesComposite) bool) diag.Diagnostics { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is possible to extract most of this method to make it common for all types by taking the policyType
as a parameter. Only the final transformation from resource data to the appropriate policy type seems to be different
if findDetails.FindType.RuleType == "DRIFT" { | ||
d1 := &DriftRuleDetails{} | ||
err = json.Unmarshal(getRawDetails.RawDetails, d1) | ||
if err != nil { | ||
return err | ||
} | ||
if d1.Exceptions != nil && d1.ProhibitedBinaries != nil { | ||
d = d1 | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed as you are already doing the check for DRIFT type on line 333 and performing an unmarshal on line 355?
layout: "sysdig" | ||
page_title: "Sysdig: sysdig_secure_aws_ml_policy" | ||
description: |- | ||
Retrieves a Sysdig Secure ML Policy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Retrieves a Sysdig Secure AWS ML Policy
|
||
### `rule` block | ||
|
||
The rule block is required and supports: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the data source, isn't the rule block exported? Would it be required?
|
||
In addition to all arguments above, the following attributes are exported: | ||
|
||
* `id` - The id for the managed policy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove the word managed
from here and description please? Also for other types
Computed: true, | ||
}, | ||
"threshold": { | ||
Type: schema.TypeInt, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be an enum? On the UI I see the dropdown for Default
, Higher
and Highest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or probably keep it as 1, 2, 3 but in the doc mention what they mean internally
Computed: true, | ||
}, | ||
"severity": { | ||
Type: schema.TypeInt, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't expose severity of rule for ML, it is defined at the policy level
|
||
* `prevent_drift` - (Optional) Prevent the execution of drifted binaries and specified prohibited binaries. | ||
|
||
* `container` - (Optional) The action applied to container when this Policy is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we mention the agent version that these container actions are supported by?
* refactor: Add common schema * feat: Add composite policy resource * feat: Add composite policy data source * docs: Add docs for composite policy resource and data source * chore: Fix formatting issues * chore: Update TODO comments in composite policies client * feat: Filter policies by type and name * chore: Clean up code * chore: Update the docs * chore: Fix formatting issue * chore: Fix Sprintf args in test code * fix: Remove ability enable or disable individual rules * chore: Fix lint issues * refactor: Introduce reducer pattern * refactor: Use a consistent way to get policy ID * fix: Resolve copylock error * refactor: Rename Composite to Malware policy * chore: Fix Linter errors * Fix: Resolve compilation error * feat: Add Drift policy resource and data source * test: Update Malware policy tests * docs: Update Malware policy docs * feat: Add ML policy * fix: Ensure the version number is sent to Policies API in order to update resources * chore: Update Drift policy docs * fix: Resolve Lin issue * feat: Add AWS ML policy * chore: Fix docs typo * fix: Add AWS ML policy to TF provider * fix: Resolve "Setting state: Invalid address to set" error * fix: Resolve compilation error * fix: Skip version 0 to resolve resource update error * fix: Resolve tfproviderdocs check error * fix: Resolve "Invalid address to set" in drift policy resource * fix: Fix resource_sysdig_secure_policy_test test failure Error: expected [{pathStepImpl:{} Name:type}] to be one of [falco list_matching k8s_audit aws_cloudtrail gcp_auditlog azure_platformlogs okta github malware drift aws_machine_learning machine_learning], got awscloudtrail * fix: Fix TestAccMalwarePolicy test failure Error: Invalid argument name. Argument names must not be quoted. * fix: Fix TestAccDriftPolicy test failure * feat: Hide tags * feat: Replace Drift rule Mode attribute with a boolean Enabled attribute * fix: Fix Drift rule reducer * fix: Hide Drift rule's match_items attribute * fix: Fix "Invalid resource type" in TestAccMalwarePolicy test * refactor: Remove details block and rename rules to rule * fix: Fix "provide at least one rule name" test error * docs: Update docs to include the latest TF resource changes * test: Update tests * chore: Update schema formatting * fix: Escape query param * refactor: Use const values * tests: Generate unique policy name in data source tests * tests: Add more tests to resources * remove unnecessary changes * more small fixes * set hash alias field as optional * remove hash alias references --------- Co-authored-by: kmvachhani <[email protected]> Co-authored-by: kmvachhani <[email protected]>
Summary
Testing steps
make install
in localterraform-provider-sysdig
directorymain.tf
in that directoryterraform init
(precede withrm .terraform.lock.hcl
if you've changed the code), update the values if needed, and runterraform apply
. Enter "yes" to confirm the changes.Draft design