-
Notifications
You must be signed in to change notification settings - Fork 215
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
Set exit code if score drops below provided threshold #157
Conversation
README.md
Outdated
@@ -115,14 +118,12 @@ polaris --audit --audit-path ./deploy/ | |||
|
|||
##### Running with CI/CD | |||
You can integrate Polaris into CI/CD for repositories containing infrastructure-as-code. | |||
For example, to fail whenever the Polaris score drops below 90%: | |||
For example, to fail whenever the Polaris score drops below 90%, or Polaris detects error-level |
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 this maybe should call out errors first - since it's prioritized in the if logic. I, personally, would also call out:
"..to fail if Polaris detects any error-level issues, or if there are no error-level issues, fail if the score drops below 90%"
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.
👍
main.go
Outdated
@@ -181,7 +189,7 @@ func startWebhookServer(c conf.Configuration, disableWebhookConfigInstaller bool | |||
} | |||
} | |||
|
|||
func runAudit(c conf.Configuration, auditPath string, setExitCode bool, outputFile string, outputURL string, outputFormat string) { | |||
func runAudit(c conf.Configuration, auditPath string, outputFile string, outputURL string, outputFormat string) validator.AuditData { |
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.
nit pick - feels like the validator.AuditData class should hold the marshal
functionality of the auditdata itself, instead of in runAudit()
.
I can open this as a future issue change, vs addressing in this 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.
Not sure I agree here - marshaling is a pretty trivial task, and one I'd generally leave up to the user. Could be convinced though.
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.
In my mind it felt like a part of domain of the audit data. I just thought it would be nice to use methods like func (ad AuditData) toJSON() []bytes
and func (ad AuditData) toYAML() []bytes
. I don't feel super strongly about this, and it mostly came up as I'm building a mental model of the code relationships.
os.Exit(3) | ||
} else if *minScore != 0 && auditData.ClusterSummary.Score < uint(*minScore) { | ||
logrus.Infof("Audit score of %d is less than the provided minimum of %d", auditData.ClusterSummary.Score, *minScore) | ||
os.Exit(4) |
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 there any way to write tests for this behavior for future refactors?
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.
Kim and I thought about this - it's definitely possible, but non-trivial. It'd also be something stuffed into CircleCI rather than a golang 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.
Non-blocking suggestion on possible tests to avoid regressions and making the wording more explicitly coupled to the logic of the flag setting.
update README
No description provided.