Slack Webhook Pipeline for Notifier #563
Conversation
acotenoff
changed the title
Codecov Report
@@ Coverage Diff @@
## dev #563 +/- ##
=========================================
- Coverage 83.5% 83.49% -0.01%
=========================================
Files 164 165 +1
Lines 8185 8217 +32
=========================================
+ Hits 6835 6861 +26
- Misses 1350 1356 +6
|
|
@blueandgold here comes the test case! |
configs/forseti_conf.yaml.sample
Outdated
| @@ -140,3 +140,10 @@ notifier: | |||
| sendgrid_api_key: '' | |||
| sender: '' | |||
| recipient: '' | |||
| # Slack webhook pipeline | |||
| # Create an incoming webhook in your organization's Slack located at: | |||
There was a problem hiding this comment.
nit: Sorry to point this out, but would adding the word setting such as below, read better?
# Create an incoming webhook in your organization's Slack setting, located at:
| @@ -0,0 +1,95 @@ | |||
|
|
|||
| # Copyright 2017 Google Inc. | |||
There was a problem hiding this comment.
Nenad made us change to the shared copyright as below: 😄
# Copyright 2017 The Forseti Security Authors. All rights reserved.
| @@ -0,0 +1,95 @@ | |||
|
|
|||
There was a problem hiding this comment.
Please remove the blank line.
| @@ -0,0 +1,54 @@ | |||
| # Copyright 2017 Google Inc. | |||
There was a problem hiding this comment.
ditto on shared copyright
| webhook_payload: a string formatted violation | ||
| """ | ||
|
|
||
| def slack_output_dump(data, indent=0): |
There was a problem hiding this comment.
What is the consideration to define this slack_output_dump() as a nested method of _compose()? I would seem to be more clear if it is moved out as a private method within the class.
Note the underscore in the method name, to denote the private.
class SlackWebhookPipeline(bnp.BaseNotificationPipeline):
def _slack_output_dump(data, indent=0):
blah blah blah
def _compose(self, **kwargs):
blah blah blah
| webhook_payload: a string formatted violation | ||
| """ | ||
|
|
||
| def slack_output_dump(data, indent=0): |
There was a problem hiding this comment.
Another nit: method names usually start with a verb. So, in this case, the method name would be more awesome as:
_dump_slack_output()
|
@blueandgold fixed, thanks for the nits! |
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| """Slack webhook pipeline to perform notifications""" |
There was a problem hiding this comment.
super-nit: Don't forget the period to end the sentence.
mirons-google
changed the title
mirons-google
changed the title
|
thank you! |
Thanks for opening a Pull Request!
Here's a handy checklist to ensure your PR goes smoothly.
pylint --rcfile=pylintrcpasses.These guidelines and more can be found in our contributing guidelines.
This PR adds support for posting violations to a Slack webhook.