openstack
/
charm-vault
Code Issues Proposed changes

Refactor status assessment 

Perform a number of new checks to assess the status of the
local unit:

 - Presence and completeness of relations
 - Status of vault (initialized, sealed)
 - Configuration of memory locking

This review also switches a number of 'status_set' calls to
be 'log' calls to avoid continually polluting the Juju status
display with non-pertinent information during update-status
executions.

Change-Id: I4e467a76bb0951deda8a88c609f4c79f4b0b33f4
This commit is contained in:
James Page 2018-04-17 07:55:11 +01:00
parent c26b35f722
commit 68068c64d8
4 changed files with 250 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

165
src/reactive/vault.py
View File

@ -3,6 +3,7 @@ import hvac
import psycopg2
import requests
import subprocess
import tenacity
from charmhelpers.contrib.charmsupport.nrpe import (
@ -48,6 +49,10 @@ from charms.reactive.relations import (
endpoint_from_flag,
)
from charms.reactive.flags import (
is_flag_set
)
# See https://www.vaultproject.io/docs/configuration/storage/postgresql.html
VAULT_TABLE_DDL = """
@ -66,11 +71,21 @@ CREATE INDEX IF NOT EXISTS parent_path_idx ON vault_kv_store (parent_path);
VAULT_HEALTH_URL = '{vault_addr}/v1/sys/health'
OPTIONAL_INTERFACES = [
['etcd'],
]
REQUIRED_INTERFACES = [
['shared-db', 'db.master']
]
def get_client():
return hvac.Client(url=get_api_url())
@tenacity.retry(wait=tenacity.wait_exponential(multiplier=1, max=10),
stop=tenacity.stop_after_attempt(10),
reraise=True)
def get_vault_health():
response = requests.get(VAULT_HEALTH_URL.format(vault_addr=get_api_url()))
return response.json()
@ -126,33 +141,25 @@ def configure_vault(context):
context['vault_api_url']))
else:
log("Etcd not detected", level=DEBUG)
status_set('maintenance', 'creating vault config')
log("Rendering vault.hcl.j2", level=DEBUG)
render(
'vault.hcl.j2',
'/var/snap/vault/common/vault.hcl',
context,
perms=0o600)
status_set('maintenance', 'creating vault unit file')
log("Rendering vault systemd configuation", level=DEBUG)
render(
'vault.service.j2',
'/etc/systemd/system/vault.service',
{},
perms=0o644)
status_set('maintenance', 'starting vault')
if can_restart():
log("Restarting vault", level=DEBUG)
service_restart('vault')
else:
service_start('vault') # restart seals the vault
status_set('maintenance', 'opening vault port')
log("Opening vault port", level=DEBUG)
open_port(8200)
set_state('configured')
if config()['disable-mlock']:
status_set(
'active',
'WARNING: DISABLE-MLOCK IS SET -- SECRETS MAY BE LEAKED')
else:
status_set('active', '=^_^=')
def get_api_url():
@ -267,7 +274,6 @@ def configure_ssl():
subprocess.check_call(['update-ca-certificates', '--fresh'])
set_state('vault.ssl.configured')
status_set('active', 'SSL key and cert installed')
remove_state('configured')
@ -329,7 +335,6 @@ def update_nagios(svc):
)
nrpe.write()
set_state('vault.nrpe.configured')
status_set('active', 'Nagios checks configured')
@when('config.changed.nagios_context')
@ -347,10 +352,87 @@ def prime_assess_status():
atexit(_assess_status)
def _assess_status():
if service_running('vault'):
health = get_vault_health()
application_version_set(health.get('version'))
def _assess_interface(interface, optional,
missing_interfaces, incomplete_interfaces):
"""Assess a named interface for presence and completeness
Uses reactive flags 'connected' and 'available' to indicate whether
an interface is present and complete.
:param: interface: Name of interface to assess.
:param: options: Boolean indicating whether interface is optional
:param: missing_interfaces: List of missing interfaces to update
:param: incomplete_interfaces: List of incomplete interfaces to update
:returns: bool, bool: Tuple of booleans indicating (missing, incomplete)
"""
log("Assessing interface {}".format(interface), level=DEBUG)
base_name = interface.split('.')[0]
connected = (
is_flag_set('{}.connected'.format(interface)) or
is_flag_set('{}.connected'.format(base_name))
)
missing = False
incomplete = False
if not connected:
if not optional:
missing_interfaces.append(base_name)
missing = True
incomplete = True
elif connected and not is_flag_set('{}.available'.format(interface)):
incomplete_interfaces.append(base_name)
incomplete = True
return (missing, incomplete)
def _assess_interface_groups(interfaces, optional,
missing_interfaces, incomplete_interfaces):
"""Assess the relation state of a list of interface groups
:param: interfaces: List of interface groups
:param: options: Boolean indicating whether interfaces are optional
:param: missing_interfaces: List of missing interfaces to update
:param: incomplete_interfaces: List of incomplete interfaces to update
"""
for interface_group in interfaces:
log("Processing interface group: {}".format(interface_group),
level=DEBUG)
_potentially_missing = []
_potentially_incomplete = []
for interface in interface_group:
missing, incomplete = _assess_interface(
interface=interface, optional=optional,
missing_interfaces=_potentially_missing,
incomplete_interfaces=_potentially_incomplete)
if not missing and not incomplete:
break
else:
# NOTE(jamespage): If an interface group has an incomplete
# interface then the end user has made a
# choice as to which interface to use, so
# don't flag any interfaces as missing.
if (not optional and
_potentially_missing and not _potentially_incomplete):
formatted_interfaces = [
"'{}'".format(i) for i in _potentially_missing
]
missing_interfaces.append(
"{} missing".format(' or '.join(formatted_interfaces))
)
# NOTE(jamespage): Only display interfaces as incomplete if
# if they are not in the missing interfaces
# list for this interface group.
if _potentially_incomplete:
filtered_interfaces = [
i for i in _potentially_incomplete
if i not in _potentially_missing
]
formatted_interfaces = [
"'{}'".format(i) for i in filtered_interfaces
]
incomplete_interfaces.append(
"{} incomplete".format(' or '.join(formatted_interfaces))
)
@when('ha.connected')
@ -359,3 +441,52 @@ def cluster_connected(hacluster):
vip = config('vip')
hacluster.add_vip('vault', vip)
hacluster.bind_resources()
def _assess_status():
"""Assess status of relations and services for local unit"""
health = None
if service_running('vault'):
health = get_vault_health()
application_version_set(health.get('version'))
_missing_interfaces = []
_incomplete_interfaces = []
_assess_interface_groups(REQUIRED_INTERFACES, optional=False,
missing_interfaces=_missing_interfaces,
incomplete_interfaces=_incomplete_interfaces)
_assess_interface_groups(OPTIONAL_INTERFACES, optional=True,
missing_interfaces=_missing_interfaces,
incomplete_interfaces=_incomplete_interfaces)
if _missing_interfaces or _incomplete_interfaces:
state = 'blocked' if _missing_interfaces else 'waiting'
status_set(state, ', '.join(_missing_interfaces +
_incomplete_interfaces))
return
if not service_running('vault'):
status_set('blocked', 'Vault service not running')
return
if not health['initialized']:
status_set('blocked', 'Vault needs to be initialized')
return
if health['sealed']:
status_set('blocked', 'Unit is sealed')
return
if config('disable-mlock'):
status_set(
'active',
'WARNING: DISABLE-MLOCK IS SET -- SECRETS MAY BE LEAKED'
)
else:
status_set(
'active',
'Unit is ready '
'(active: {})'.format(str(not health['standby']).lower())
)

2
src/wheelhouse.txt
View File

@ -1 +1,3 @@
hvac
tenacity
pbr

2
test-requirements.txt
View File

@ -7,3 +7,5 @@ mock>=1.2
coverage>=3.6
psycopg2
git+https://github.com/openstack/charms.openstack#egg=charms.openstack
tenacity
pbr

114
unit_tests/test_vault.py
View File

@ -26,6 +26,26 @@ class TestHandlers(unittest.TestCase):
"cluster_id": "1ea3d74c-3819-fbaf-f780-bae0babc998f"
}
_health_response_needs_init = {
"initialized": False,
"sealed": False,
"standby": False,
"server_time_utc": 1523952750,
"version": "0.9.0",
"cluster_name": "vault-cluster-9dd8dd12",
"cluster_id": "1ea3d74c-3819-fbaf-f780-bae0babc998f"
}
_health_response_sealed = {
"initialized": True,
"sealed": True,
"standby": False,
"server_time_utc": 1523952750,
"version": "0.9.0",
"cluster_name": "vault-cluster-9dd8dd12",
"cluster_id": "1ea3d74c-3819-fbaf-f780-bae0babc998f"
}
def setUp(self):
super(TestHandlers, self).setUp()
self.patches = [
@ -87,13 +107,6 @@ class TestHandlers(unittest.TestCase):
'disable_mlock': False,
'ssl_available': True,
}
status_set_calls = [
mock.call('maintenance', 'creating vault config'),
mock.call('maintenance', 'creating vault unit file'),
mock.call('maintenance', 'starting vault'),
mock.call('maintenance', 'opening vault port'),
mock.call('active', '=^_^='),
]
render_calls = [
mock.call(
'vault.hcl.j2',
@ -107,7 +120,6 @@ class TestHandlers(unittest.TestCase):
perms=0o644)
]
self.open_port.assert_called_once_with(8200)
self.status_set.assert_has_calls(status_set_calls)
self.render.assert_has_calls(render_calls)
# Check flipping disable-mlock makes it to the context
@ -290,6 +302,13 @@ class TestHandlers(unittest.TestCase):
self.unit_private_ip.return_value = '1.2.3.4'
self.assertEqual(handlers.get_api_url(), 'http://1.2.3.4:8200')
def test_cluster_connected(self):
self.config.return_value = '10.1.1.1'
hacluster_mock = mock.MagicMock()
handlers.cluster_connected(hacluster_mock)
hacluster_mock.add_vip.assert_called_once_with('vault', '10.1.1.1')
hacluster_mock.bind_resources.assert_called_once_with()
@patch.object(handlers, 'get_api_url')
@patch.object(handlers, 'requests')
def test_get_vault_health(self, requests, get_api_url):
@ -303,24 +322,87 @@ class TestHandlers(unittest.TestCase):
"https://vault.demo.com:8200/v1/sys/health")
mock_response.json.assert_called_once()
@patch.object(handlers, '_assess_interface_groups')
@patch.object(handlers, 'get_vault_health')
def test_assess_status(self, get_vault_health):
def test_assess_status(self, get_vault_health,
_assess_interface_groups):
get_vault_health.return_value = self._health_response
_assess_interface_groups.return_value = []
self.config.return_value = False
self.service_running.return_value = True
handlers._assess_status()
self.application_version_set.assert_called_with(
self._health_response['version'])
self.status_set.assert_called_with(
'active', 'Unit is ready (active: true)')
self.config.assert_called_with('disable-mlock')
_assess_interface_groups.assert_has_calls([
mock.call(handlers.REQUIRED_INTERFACES,
optional=False,
missing_interfaces=mock.ANY,
incomplete_interfaces=mock.ANY),
mock.call(handlers.OPTIONAL_INTERFACES,
optional=True,
missing_interfaces=mock.ANY,
incomplete_interfaces=mock.ANY),
])
@patch.object(handlers, '_assess_interface_groups')
@patch.object(handlers, 'get_vault_health')
def test_assess_status_not_running(self, get_vault_health):
def test_assess_status_not_running(self, get_vault_health,
_assess_interface_groups):
get_vault_health.return_value = self._health_response
self.service_running.return_value = False
handlers._assess_status()
self.application_version_set.assert_not_called()
self.status_set.assert_called_with(
'blocked', 'Vault service not running')
def test_cluster_connected(self):
self.config.return_value = '10.1.1.1'
hacluster_mock = mock.MagicMock()
handlers.cluster_connected(hacluster_mock)
hacluster_mock.add_vip.assert_called_once_with('vault', '10.1.1.1')
hacluster_mock.bind_resources.assert_called_once_with()
@patch.object(handlers, '_assess_interface_groups')
@patch.object(handlers, 'get_vault_health')
def test_assess_status_vault_init(self, get_vault_health,
_assess_interface_groups):
get_vault_health.return_value = self._health_response_needs_init
_assess_interface_groups.return_value = []
self.service_running.return_value = True
handlers._assess_status()
self.status_set.assert_called_with(
'blocked', 'Vault needs to be initialized')
@patch.object(handlers, '_assess_interface_groups')
@patch.object(handlers, 'get_vault_health')
def test_assess_status_vault_sealed(self, get_vault_health,
_assess_interface_groups):
get_vault_health.return_value = self._health_response_sealed
_assess_interface_groups.return_value = []
self.service_running.return_value = True
handlers._assess_status()
self.status_set.assert_called_with(
'blocked', 'Unit is sealed')
@patch.object(handlers, 'is_flag_set')
def test_assess_interface_groups(self, is_flag_set):
flags = {
'db.master.available': True,
'db.connected': True,
'etcd.connected': True,
'baz.connected': True,
}
is_flag_set.side_effect = lambda flag: flags.get(flag, False)
missing_interfaces = []
incomplete_interfaces = []
handlers._assess_interface_groups(
[['db.master', 'shared-db'],
['etcd'],
['foo', 'bar'],
['baz', 'boo']],
optional=False,
missing_interfaces=missing_interfaces,
incomplete_interfaces=incomplete_interfaces
)
self.assertEqual(missing_interfaces,
["'foo' or 'bar' missing"])
self.assertEqual(incomplete_interfaces,
["'etcd' incomplete",
"'baz' incomplete"])