Skip to content
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

Defer evaluation of api and app keys + read from `run_state` #395

Merged
merged 3 commits into from Jan 24, 2017

Conversation

Projects
None yet
3 participants
@olivielpeau
Copy link
Member

commented Jan 19, 2017

Based on #345 (thanks @mfischer), rebased on top of master, and minus the changes on the compile_time property in dd-handler. Details in the commit messages and in #345.

Tested on both Chef 11 and 12, seems to work fine.

Related to #377

@olivielpeau

This comment has been minimized.

Copy link
Member Author

commented Jan 20, 2017

Hmm actually I realize now that enabling the datadog handler at compile time could defeat the purpose of pulling the keys from the run_state. Unless other recipes set the run_state at compile time before dd-handler is compiled the run_state won't be populated with the keys.

We could add an attribute to only enable the handler at converge time.


private

def run_state_or_attribute(node, attr)

This comment has been minimized.

Copy link
@degemer

degemer Jan 23, 2017

Member

Should we rename attr? It's already used by Ruby (not in this context though) - http://ruby-doc.org/core-2.0.0/Module.html#method-i-attr

```
2. Add your API Key as a node attribute via an `environment` or `role` or by declaring it in another cookbook at a higher precedence level.
3. Create an 'application key' for `chef_handler` [here](https://app.datadoghq.com/account/settings#api), and add it as a node attribute, as in Step #2.
2. Add your API Key either:

This comment has been minimized.

Copy link
@degemer

degemer Jan 23, 2017

Member

Should we add at the top of the README that these instructions are not for 2.7.0?

This comment has been minimized.

Copy link
@olivielpeau

olivielpeau Jan 23, 2017

Author Member

Done

@@ -51,27 +51,25 @@
# To add integration-specific configurations, add 'datadog::config_name' to
# the node's run_list and set the relevant attributes
#
raise "Add a ['datadog']['api_key'] attribute to configure this node's Datadog Agent." if node['datadog'] && node['datadog']['api_key'].nil?

This comment has been minimized.

Copy link
@degemer

degemer Jan 23, 2017

Member

Should we keep it? We'd have to change the message. "Add a ['datadog']['api_key'] attribute or set the ['datadog']['api_key'] run_state to configure this node's Datadog Agent."

This comment has been minimized.

Copy link
@olivielpeau

olivielpeau Jan 23, 2017

Author Member

To keep it we'd need to evaluate it at execution time. I'll try that.

# ========================================================================== #
# Other config options
# ========================================================================== #
<% @extra_config.each do |k, v| -%>
<% @node['datadog']['extra_config'].each do |k, v| -%>

This comment has been minimized.

Copy link
@degemer

This comment has been minimized.

Copy link
@degemer

degemer Jan 23, 2017

Member

Weird it's wasn't caught by the tests...

This comment has been minimized.

Copy link
@olivielpeau

olivielpeau Jan 23, 2017

Author Member

ha, good catch, apparently @node also works... Changing this to node anyway

# Fail here at converge time if no api_key is set
ruby_block 'datadog-api-key-set' do
block do
raise "Set ['datadog']['api_key'] as an attribute or on the node's run_state to configure this node's Datadog Agent." if Chef::Datadog.api_key(node).nil?

This comment has been minimized.

Copy link
@degemer

@olivielpeau olivielpeau force-pushed the olivielpeau/pr345 branch from 8c84f84 to 97d18d6 Jan 23, 2017

mfischer-zd added some commits Aug 12, 2016

Defer evaluation of api and application keys (#328)
Some users may wish to assign the api_key and application_key attributes during
runtime instead of compile time -- particularly, users who want to obtain the
value of the key(s) from an external service. Allow doing so by deferring
evaluation of these attributes.

Closes #328
Read API and application keys from run_state (#341)
If the user has placed API and/or application keys in the
`node.run_state` hash, read them from there instead of from the
node's attributes.  If run_state is used, implementors can obtain the
keys from other data sources (e.g. Hashicorp Vault) and prevent the keys
from being inadvertently stored in Chef Server.

Closes #341

@olivielpeau olivielpeau force-pushed the olivielpeau/pr345 branch from 97d18d6 to 135d776 Jan 24, 2017

[run_state] Improve CI, documentation and backwards compatibility
CI and documentation:
* make rubocop pass, mostly by disabling some cops on a few
blocks/methods where I couldn't find good alternatives.
* move the extra_config logic to the `datadog.conf.erb` template.
* add spec tests
* document pulling api/app keys from node `run_state` in README
and default attributes file

BC improvemnts:
* rename `attr` to `attribute` in `recipe_helpers.rb`
* keep raising an exception when no `api_key` is present (at converge
time instead of compile time)

@olivielpeau olivielpeau force-pushed the olivielpeau/pr345 branch from 135d776 to d20d9b9 Jan 24, 2017

@olivielpeau olivielpeau merged commit f67d71a into master Jan 24, 2017

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@olivielpeau olivielpeau deleted the olivielpeau/pr345 branch Jan 24, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.