Skip to content

Commit

Permalink
Assume all nodes matching the search query are valid nodes
Browse files Browse the repository at this point in the history
Previously we checked if nodes were matching search query AND had an existing chef key.
This patch assumes that all nodes have exiting chef keys.

Experience I have with chef is that nodes without proper keys are bugs and needs
to be fixed anyway (either have a key or be removed).

This patch allows to make usage of --clean-unknown-clients much faster (avoid
querying chef-server for each node) especially on secrets encrypted for many
nodes (where search query time is small compared to querying all clients)

This introduces a small change in behavior:
- before: existing nodes without client would be cleanup by --clean-unknown-clients
- now: such nodes are not cleaned. They will be cleaned when node will be removed

Change-Id: If45da1faec5c36026a75762afe4bad08cab97f64
Signed-off-by: Grégoire Seux <g.seux@criteo.com>
  • Loading branch information
kamaradclimber committed Apr 27, 2017
1 parent ddb3700 commit 342e9c5
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 22 deletions.
2 changes: 1 addition & 1 deletion features/clean_on_refresh.feature
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Feature: clean unknown clients on vault refresh
Given a local mode chef repo with nodes 'one,two,three'
And I create a vault item 'test/item' containing the JSON '{"foo": "bar"}' encrypted for 'one,two,three'
Then the vault item 'test/item' should be encrypted for 'one,two,three'
And I delete client 'one' from the Chef server
And I delete node 'one' from the Chef server
And I refresh the vault item 'test/item' with the 'clean-unknown-clients' option
Then the output should contain "Removing unknown client 'one'"
And the vault item 'test/item' should be encrypted for 'two,three'
Expand Down
18 changes: 3 additions & 15 deletions features/clean_unknown_clients.feature
Original file line number Diff line number Diff line change
@@ -1,29 +1,17 @@
Feature: clean unknown clients on key rotation
When removing a client from a vault item, chef-vault normally
removes the key and then rotates the key. If a client has been
removes the key and then rotates the key. If a node has been
deleted in the meantime from the Chef server but not the vault,
the rotation will fail due to that client's public key missing.
Using the --clean-unknown-clients switch will cause any clients
that have been removed to be removed from the vault item's
access list as well

Scenario: Prune clients when removing a client
Given a local mode chef repo with nodes 'one,two,three'
And I create a vault item 'test/item' containing the JSON '{"foo": "bar"}' encrypted for 'one,two,three'
Then the vault item 'test/item' should be encrypted for 'one,two,three'
And I delete client 'one' from the Chef server
And I remove client 'two' from vault item 'test/item' with the 'clean-unknown-clients' option
Then the output should contain "Removing unknown client 'one'"
And the vault item 'test/item' should be encrypted for 'three'
And the vault item 'test/item' should not be encrypted for 'one,two'
And 'three' should be a client for the vault item 'test/item'
And 'one,two' should not be a client for the vault item 'test/item'

Scenario: Prune clients when rotating keys
Given a local mode chef repo with nodes 'one,two,three'
And I create a vault item 'test/item' containing the JSON '{"foo": "bar"}' encrypted for 'one,two,three'
Then the vault item 'test/item' should be encrypted for 'one,two,three'
And I delete client 'one' from the Chef server
And I delete node 'one' from the Chef server
And I rotate the keys for vault item 'test/item' with the 'clean-unknown-clients' option
Then the output should contain "Removing unknown client 'one'"
And the vault item 'test/item' should be encrypted for 'two,three'
Expand All @@ -35,7 +23,7 @@ Feature: clean unknown clients on key rotation
Given a local mode chef repo with nodes 'one,two,three'
And I create a vault item 'test/item' containing the JSON '{"foo": "bar"}' encrypted for 'one,two,three'
Then the vault item 'test/item' should be encrypted for 'one,two,three'
And I delete clients 'one,two' from the Chef server
And I delete nodes 'one,two' from the Chef server
And I rotate all keys with the 'clean-unknown-clients' option
Then the output should contain "Removing unknown client 'one'"
And the output should contain "Removing unknown client 'two'"
Expand Down
8 changes: 2 additions & 6 deletions lib/chef-vault/item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -411,15 +411,11 @@ def remove_unknown_nodes

# checks if a node exists on the Chef server by performing
# a search against the node index. If the search returns no
# results, the node does not exist. If it does return results,
# check if there is a matching client
# results, the node does not exist.
# @param nodename [String] the name of the node
# @return [Boolean] whether the node exists or not
def node_exists?(nodename)
# if we don't have a client it really doesn't matter if we have a node.
if client_exists?(nodename)
search_results.include?(nodename)
end
search_results.include?(nodename)
end

# checks if a client exists on the Chef server. If we get back
Expand Down

0 comments on commit 342e9c5

Please sign in to comment.