-
Notifications
You must be signed in to change notification settings - Fork 14
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: add new lacework_managed_policies resource #516
Conversation
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.
Checked out the code, and ran the integration tests - all passed!!
Co-authored-by: lacework-aaronscheiber <54645734+lacework-aaronscheiber@users.noreply.github.com>
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.
bulkUpdatePolicies, err := getBulkUpdatePolicies(d, meta) | ||
|
||
if err != nil { | ||
// Return nil so that `destroy` can succeed |
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.
why is this necessary?
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.
destroy
will call the read
first which could throw errors. This makes sure destroy
can delete the resource regardless of the errors.
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 do not follow how this function getBulkUpdatePolicies()
could throw errors on destroy. The function is not even accessing APIs.
What is the error you are seeing?
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 integration tests, getBulkUpdatePolicies
will throw errors when destroying the state of TestManagedPoliciesWithDuplicateIDs
. Then the old state persists and will fail TestManagedPoliciesWithCustomIDs
.
for _, v := range list { | ||
val := v.(map[string]interface{}) | ||
|
||
if val["id"] == nil || val["id"] == "" { |
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 id
s be nil or empty? 🤔
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.
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.
Talking with @PengyuanZhao we agreed that this is the best approach we can come up with.
Description:
Create a new resource
lacework_managed_policies
to manage the state (enable/disable) and the severity of Lacework defined policies (policies that start withlacework-global
)Example Usage
Argument Reference
The following arguments are supported:
id
- (Required) The Lacework defined policy id.enabled
- (Required) Whether the policy is enabled or disabled.severity
- (Optional) The list of the severities. Valid severities include:Critical
,High
,Medium
,Low
andInfo
.Additional Info:
Run
make integration-test regex=TestManagedPolicies
to test.Verify that the resource can update the data correctly via the following steps: