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

Add vault_kv2_delete module #304

Merged
merged 21 commits into from
Oct 30, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions plugins/modules/vault_kv2_delete.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@

from __future__ import (absolute_import, division, print_function)
__metaclass__ = type

DOCUMENTATION = r'''
module: vault_kv2_get
'''


import traceback

from ansible.module_utils._text import to_native
from ansible.module_utils.basic import missing_required_lib

from ansible_collections.community.hashi_vault.plugins.module_utils._hashi_vault_module import HashiVaultModule
from ansible_collections.community.hashi_vault.plugins.module_utils._hashi_vault_common import HashiVaultValueError

try:
import hvac
except ImportError:
HAS_HVAC = False
HVAC_IMPORT_ERROR = traceback.format_exc()
else:
HVAC_IMPORT_ERROR = None
HAS_HVAC = True

def run_module():


argspec = HashiVaultModule.generate_argspec(
engine_mount_point=dict(type='str', default='kv'),
briantist marked this conversation as resolved.
Show resolved Hide resolved
path=dict(type='str', required=True)
)

module = HashiVaultModule(
argument_spec=argspec,
supports_check_mode=True
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment, the module does not account for check mode, and so with this set to True, check mode would delete secrets 😱

One issue currently (same reason status is always changed) is that without checking the current status of the secret, check mode has nothing to do but return changed.

I'm mulling over the idea of using metadata reads to get current versions status/secret existence to be able to solve both of those, but their use should be optional (a client may have permission to delete but not to hit those other endpoints).

I'll look at this again tomorrow and think about whether that makes sense at all, and if so whether it should be in this PR or a follow-up.

The first thing we should do is add an integration test and unit test (example) for check mode before making any additional changes, and those tests should fail right now.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, I think for now the empty result with no Vault connection is fine.

The unit tests are great! Please also add an integration test for check mode; it should be easy, just checking that the result is changed and that the deletion did not happen.

Even though the unit test covers this a little more thoroughly, it's always good to have it covered by integration too just to have the code actually invoked by ansible.

)

if not HAS_HVAC:
module.fail_json(
msg=missing_required_lib('hvac'),
exception=HVAC_IMPORT_ERROR
)

engine_mount_point = module.params.get('engine_mount_point')
path = module.params.get('path')

module.connection_options.process_connection_options()
client_args = module.connection_options.get_hvac_connection_options()
client = module.helper.get_vault_client(**client_args)

try:
module.authenticator.validate()
module.authenticator.authenticate(client)
except (NotImplementedError, HashiVaultValueError) as e:
module.fail_json(msg=to_native(e), exception=traceback.format_exc())

try:
raw = client.secrets.kv.v2.delete_latest_version_of_secret(path=path, mount_point=engine_mount_point)
except hvac.exceptions.Forbidden as e:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to check for some more exceptions, depending on what hvac might raise/handle (best to check the source there to get an idea what to handle here).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like hvac does not do any error handling for this. I also peeked at the Vault source, and it doesn't do anything special in the delete itself, just passing through any errors it gets such as any errors updating the metadata. If theres anything else you can think of, I'd be happy to add it.

module.fail_json(msg="Forbidden: Permission Denied to path ['%s']." % path, exception=traceback.format_exc())

module.exit_json(changed=True, response_code=raw.status_code)
briantist marked this conversation as resolved.
Show resolved Hide resolved

def main():
run_module()


if __name__ == '__main__':
main()
154 changes: 154 additions & 0 deletions tests/unit/plugins/modules/test_vault_kv2_delete.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
# -*- coding: utf-8 -*-
# Copyright (c) 2022 Brian Scholer (@briantist)
briantist marked this conversation as resolved.
Show resolved Hide resolved
# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt)
# SPDX-License-Identifier: GPL-3.0-or-later

from __future__ import (absolute_import, division, print_function)
__metaclass__ = type

import pytest
import re
import json

from ansible.module_utils.basic import missing_required_lib

from ...compat import mock
from .....plugins.modules import vault_kv2_delete
from .....plugins.module_utils._hashi_vault_common import HashiVaultValueError


hvac = pytest.importorskip('hvac')


pytestmark = pytest.mark.usefixtures(
'patch_ansible_module',
'patch_authenticator',
'patch_get_vault_client',
)


def _connection_options():
return {
'auth_method': 'token',
'url': 'http://myvault',
'token': 'beep-boop',
}


def _sample_options():
return {
'engine_mount_point': 'secret',
'path': 'endpoint',
}


def _combined_options(**kwargs):
opt = _connection_options()
opt.update(_sample_options())
opt.update(kwargs)
return opt


@pytest.fixture
def kv2_delete_response():
class request_response:
status_code = 204

return request_response


class TestModuleVaultKv2Delete():

@pytest.mark.parametrize('patch_ansible_module', [_combined_options()], indirect=True)
@pytest.mark.parametrize('exc', [HashiVaultValueError('throwaway msg'), NotImplementedError('throwaway msg')])
def test_vault_kv2_delete_authentication_error(self, authenticator, exc, capfd):
authenticator.authenticate.side_effect = exc

with pytest.raises(SystemExit) as e:
vault_kv2_delete.main()

out, err = capfd.readouterr()
result = json.loads(out)

assert e.value.code != 0, "result: %r" % (result,)
assert result['msg'] == 'throwaway msg', "result: %r" % result

@pytest.mark.parametrize('patch_ansible_module', [_combined_options()], indirect=True)
@pytest.mark.parametrize('exc', [HashiVaultValueError('throwaway msg'), NotImplementedError('throwaway msg')])
def test_vault_kv2_get_auth_validation_error(self, authenticator, exc, capfd):
authenticator.validate.side_effect = exc

with pytest.raises(SystemExit) as e:
vault_kv2_delete.main()

out, err = capfd.readouterr()
result = json.loads(out)

assert e.value.code != 0, "result: %r" % (result,)
assert result['msg'] == 'throwaway msg'

@pytest.mark.parametrize('opt_engine_mount_point', ['secret', 'other'])
@pytest.mark.parametrize('patch_ansible_module', [[_combined_options(), 'engine_mount_point']], indirect=True)
def test_vault_kv2_delete_return_data(self, patch_ansible_module, kv2_delete_response, vault_client, opt_engine_mount_point, capfd):
client = vault_client
client.secrets.kv.v2.delete_latest_version_of_secret.return_value = kv2_delete_response

expected_response = 204

with pytest.raises(SystemExit) as e:
vault_kv2_delete.main()

out, err = capfd.readouterr()
result = json.loads(out)

assert e.value.code == 0, "result: %r" % (result,)

client.secrets.kv.v2.delete_latest_version_of_secret.assert_called_once_with(
path=patch_ansible_module['path'],
mount_point=patch_ansible_module['engine_mount_point'],
)

assert result['response_code'] == expected_response, (
"module result did not match expected result:\nmodule: %r\nexpected: %r" % (
result['response_code'], expected_response)
)

@pytest.mark.parametrize('patch_ansible_module', [_combined_options()], indirect=True)
def test_vault_kv2_delete_no_hvac(self, capfd):
with mock.patch.multiple(vault_kv2_delete, HAS_HVAC=False, HVAC_IMPORT_ERROR=None, create=True):
with pytest.raises(SystemExit) as e:
vault_kv2_delete.main()

out, err = capfd.readouterr()
result = json.loads(out)

assert e.value.code != 0, "result: %r" % (result,)
assert result['msg'] == missing_required_lib('hvac')

@pytest.mark.parametrize(
'exc',
[
(hvac.exceptions.Forbidden, "",
r"^Forbidden: Permission Denied to path \['([^']+)'\]"),
]
)
@pytest.mark.parametrize('patch_ansible_module', [[_combined_options(), 'path']], indirect=True)
@pytest.mark.parametrize('opt_path', ['path/1', 'second/path'])
def test_vault_kv2_get_vault_exception(self, vault_client, exc, opt_path, capfd):

client = vault_client
client.secrets.kv.v2.delete_latest_version_of_secret.side_effect = exc[0](
exc[1])

with pytest.raises(SystemExit) as e:
vault_kv2_delete.main()

out, err = capfd.readouterr()
result = json.loads(out)

assert e.value.code != 0, "result: %r" % (result,)
match = re.search(exc[2], result['msg'])
assert match is not None, "result: %r\ndid not match: %s" % (
result, exc[2])

assert opt_path == match.group(1)