Add support for getting api_key from hostvars and thus from vault #25
Conversation
|
Would be great if this could be merged finally |
hush-hush
force-pushed
the
source-api-key-from-vault
branch
from
9116406 to
9517a5b
Compare
There was a problem hiding this comment.
made a few comments, looks good overall, @hush-hush let me know what you think!
datadog_callback.py
Outdated
| self._options = cli.options | ||
|
|
||
| # self.playbook is either set by Ansible (v1), or by us in the `playbook_start` callback method (v2) | ||
| # self.playbook is either set in the `v2_playbook_on_start` callback method |
There was a problem hiding this comment.
self.playbook is either set -> self.playbook is set
datadog_callback.py
Outdated
| # self.playbook is either set in the `v2_playbook_on_start` callback method | ||
| self.playbook = None | ||
| # self.play is either set by Ansible (v1), or by us in the `playbook_on_play_start` callback method (v2) | ||
| # self.play is either set in the `playbook_on_play_start` callback method |
There was a problem hiding this comment.
self.play is either set -> self.play is set
| getpass.getuser(), | ||
| self._inventory_name), | ||
| event_type='start', | ||
| ) |
There was a problem hiding this comment.
by moving this logic (reading config + sending start event) from v2_playbook_on_start to v2_playbook_on_play_start, the callback sends a start event for every play in a playbook which is confusing given the contents of the event.
If I understand correctly we can't extract the api key sooner since we can't access the hostvars in v2_playbook_on_start, is that correct? If there's no workaround we should update the event here so that it says the the play has started, not the playbook :)
There was a problem hiding this comment.
Indeed, renaming the event to play seems the best.
|
|
||
| # Set up API client and send a start event | ||
| if not self.disabled: | ||
| datadog.initialize(api_key=api_key, api_host=url) |
There was a problem hiding this comment.
does this work if it's called multiple times? (i.e. when the playbook has multiple plays?)
There was a problem hiding this comment.
Yes, the latest call reset the key/url.
hush-hush
force-pushed
the
source-api-key-from-vault
branch
from
9517a5b to
a601edc
Compare
olivielpeau
left a comment
olivielpeau
left a comment
There was a problem hiding this comment.
One last detail, feel free to merge once it's addressed! 👍
|
|
||
| self.send_playbook_event( | ||
| 'Ansible play "{0}" started by "{1}" against "{2}"'.format( | ||
| self._playbook_name, |
There was a problem hiding this comment.
this should be self.play.name. Could also be worth keeping the playbook name in the message (could be Ansible play "{self.play.name}" started in playbook "{self._playbook_name}" by [...])
hush-hush
force-pushed
the
source-api-key-from-vault
branch
from
a601edc to
e3896fc
Compare
4 participants
Primarily for #10 but also partially solves #4
Supports only Ansible >= 2