-
Notifications
You must be signed in to change notification settings - Fork 2
🧹 add createException api, deleteException api, add test #285
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
Conversation
7871358
to
b6fcccb
Compare
Action types.String `tfsdk:"action"` | ||
CheckMrns types.List `tfsdk:"check_mrns"` | ||
VulnerabilityMrns types.List `tfsdk:"vulnerability_mrns"` | ||
ExceptionId types.String `tfsdk:"exception_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.
what happens to existing state files where this doesnt exist
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.
hmm. good question. ill check
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.
good call, i tested and that then would fail to delete the exception on update.
im implementing a change that will searchf or the exception if we dont have the exception id. if we search for it and find it, great.
if not, then my thought was we return an error to the user and then allow them to import the exception resource
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.
ok. i tested this all out.
- if a user has an exception in their state file and there are no changes between the rest of the data in the exception and the state file, the tf wont complain because the exception id is optional
- if a user has an exception in their state file and there are changes between what exists and the state file, then the tf will trigger an update, which will now search for the exception, if found delete and recreate, if not found return error and instruct to import
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.
working on import resource now
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.
import resource should be working now as well
f62c206
to
45b8110
Compare
i tried debugging it but got nowhere. it fails when i include the id in the query for the import test. but when i run import manually everything works. and the import test works if i just query for space exceptions without the id.
45b8110
to
43e0434
Compare
resource.TestCheckResourceAttr("mondoo_exception.windows_defender_exception", "action", "FALSE_POSITIVE"), | ||
), | ||
}, | ||
// // import testing |
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.
Leftover ?
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.
it is, but i want to try to get it working in the future. we should have the imports tested. this one is being real tricky on me so i was hoping to address the test itself in a followup
oh nice thanks for the review @kkereziev ill address your feedback this afternoon |
fixed bug with creating exceptions
to test this, set jsonbase64 service account to your local dev and run:
you'll see it fail. then checkout this branch, run make install, make dev/enter, and remove any old tf state files and run this tf again, itll work
added import
more details
the update path is kinda wild. we didnt have exception ids exposed before, so it was search based. now that we have an id, that's much more ideal. but it means we have to ensure we account for that during the update path (which is why we check for exception id, then search for the exception and get the id if it's not present). the fallback there is that if we cant find the exception the user will need to import it.
since exception id is optional, the tf wont care about this as long as there r no changes to apply to the exception resource
i tested updating from older tf manually by installing version 0.23.0 locally, running the tf, and then getting on this branch version of the plugin and re-applying from there