-
Notifications
You must be signed in to change notification settings - Fork 393
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
Use Go templates for stdout #630
Conversation
Signed-off-by: Simarpreet Singh <simar@linux.com>
Signed-off-by: Simarpreet Singh <simar@linux.com>
Signed-off-by: Simarpreet Singh <simar@linux.com>
Signed-off-by: Simarpreet Singh <simar@linux.com>
Signed-off-by: Simarpreet Singh <simar@linux.com>
Signed-off-by: Simarpreet Singh <simar@linux.com>
Signed-off-by: Simarpreet Singh <simar@linux.com>
Signed-off-by: Simarpreet Singh <simar@linux.com>
type FindingWithMetadata struct { | ||
Finding | ||
SignatureMetadata | ||
} |
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.
@simar7 what do you think on changing Finding type to include the SignatureMetadata instead of the Signature interface? @yanivagman also curios what you think
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.
Signature interface has GetMetadata() method - why not using it when needed?
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.
Yaniv and I discussed this offline, we should replace signature field with signaturemetadata in the finding. This will simplify many cases without sacrificing much.
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 do that as part of this PR? I'm happy to do it just feel it's unrelated to this PR as modifying an existing interface requires all users of it (example signatures in this case) to be changed as well.
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.
whatever you prefer
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'll draft a new PR
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.
Signed-off-by: Simarpreet Singh <simar@linux.com>
Signed-off-by: Simarpreet Singh <simar@linux.com>
tracee-rules/output.go
Outdated
@@ -37,6 +38,7 @@ func setupTemplate(inputTemplateFile string, clock Clock) (*template.Template, e | |||
case inputTemplateFile != "": | |||
return template.New(filepath.Base(inputTemplateFile)). | |||
Funcs(funcMap). |
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.
NB: I'm still keeping this in because to my knowledge there's no way to inject a custom time interface into sprig which makes it untestable for our most common case of template (simple and verbose) where we need the current time.
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 do we need to test it specifically? if we want to test that the templating engine works, can we craft a simpler template for the test?
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.
Yes fair point. I wanted to test the default output (without any templates, which includes a timestamp). I found a another way to do it bf6a78f and cleaned this up.
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.
LGTM, although I do prefer to remove the timenow function before merging if you agree with my comment
Signed-off-by: Simarpreet Singh <simar@linux.com>
This PR adds the following:
--output
flag to tracee-rules, which enables to BYOT (bring your own template) for stdout.--output
, tracee-rules uses a default predefined template.Fixes: #612, #611 and #614
Signed-off-by: Simarpreet Singh simar@linux.com