openstack
/
charm-vault
Code Issues Proposed changes

Do not check app role auth until ready 

The client_approle_authorized was checking for app role authorization
too early before the charm had been authorized by the authorize-charm
action.

Before checking on app role authorization verify the leader setting,
"local-charm-access-id",  set by the authorize-charm action.

Closes-Bug: #1889654
Change-Id: I53f2c357c06a5ac9846718654d35c9baa576cafd
This commit is contained in:
David Ames 2020-08-03 11:49:58 -07:00
parent 8352f7ba54
commit 3e48efe85b
3 changed files with 37 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/lib/charm/vault.py
View File

@ -235,7 +235,7 @@ def get_client(url=None):
@tenacity.retry(wait=tenacity.wait_exponential(multiplier=1, max=60),
stop=tenacity.stop_after_attempt(10),
stop=tenacity.stop_after_attempt(8),
reraise=True)
def get_local_client():
"""Provide a client for talking to the vault api

10
src/reactive/vault_handlers.py
View File

@ -33,6 +33,7 @@ from charmhelpers.core.hookenv import (
atexit,
local_unit,
leader_set,
leader_get,
)
from charmhelpers.core.host import (
@ -696,6 +697,12 @@ def _assess_status():
status_set('blocked', 'Unit is sealed')
return
if not leader_get(vault.CHARM_ACCESS_ROLE_ID):
status_set(
'blocked',
'Vault charm not yet authorized: run authorize-charm action.')
return
if not client_approle_authorized():
status_set('blocked', 'Vault cannot authorize approle')
return
@ -713,6 +720,9 @@ def _assess_status():
def client_approle_authorized():
if not leader_get(vault.CHARM_ACCESS_ROLE_ID):
log("Charm not yet authorized", "DEBUG")
return False
try:
vault.get_local_client()
return True

33
unit_tests/test_reactive_vault_handlers.py
View File

@ -316,15 +316,18 @@ class TestHandlers(unit_tests.test_utils.CharmTestCase):
self.is_flag_set.assert_called_with('etcd.tls.available')
self.config.assert_called_with('disable-mlock')
@patch.object(handlers, 'leader_get')
@patch.object(handlers, 'client_approle_authorized')
@patch.object(handlers, '_assess_interface_groups')
@patch.object(handlers.vault, 'get_vault_health')
def test_assess_status(self, get_vault_health,
_assess_interface_groups,
_client_approle_authorized):
_client_approle_authorized,
_leader_get):
self.is_flag_set.return_value = False
get_vault_health.return_value = self._health_response
_assess_interface_groups.return_value = []
_leader_get.return_value = True
_client_approle_authorized.return_value = True
self.config.return_value = False
self.service_running.return_value = True
@ -797,12 +800,15 @@ class TestHandlers(unit_tests.test_utils.CharmTestCase):
_vault.get_client.return_value = hvac_client
hvac_client.is_sealed.return_value = status
@mock.patch.object(handlers, 'client_approle_authorized')
@mock.patch.object(handlers, 'vault')
@mock.patch.object(handlers, 'vault_pki')
def test_publish_ca_info(self, vault_pki, _vault):
def test_publish_ca_info(
self, vault_pki, _vault, _client_approle_authorized):
self.is_unit_paused_set.return_value = False
self.service_running.return_value = True
self._set_sealed(_vault, False)
_client_approle_authorized.return_value = True
tls = self.endpoint_from_flag.return_value
vault_pki.get_ca.return_value = 'ca'
@ -811,9 +817,11 @@ class TestHandlers(unit_tests.test_utils.CharmTestCase):
tls.set_ca.assert_called_with('ca')
tls.set_chain.assert_called_with('chain')
@mock.patch.object(handlers, 'client_approle_authorized')
@mock.patch.object(handlers, 'vault')
@mock.patch.object(handlers, 'vault_pki')
def test_publish_ca_info_sealed(self, vault_pki, _vault):
def test_publish_ca_info_sealed(
self, vault_pki, _vault, _client_approle_authorized):
self.is_unit_paused_set.return_value = False
self.service_running.return_value = True
self._set_sealed(_vault, True)
@ -823,16 +831,21 @@ class TestHandlers(unit_tests.test_utils.CharmTestCase):
assert not tls.set_ca.called
assert not tls.set_chain.called
@mock.patch.object(handlers, 'client_approle_authorized')
@mock.patch.object(handlers, 'vault')
def test_publish_ca_info_paused(self, _vault):
def test_publish_ca_info_paused(self, _vault, _client_approle_authorized):
self.is_unit_paused_set.return_value = True
_client_approle_authorized.return_value = True
handlers.publish_ca_info()
assert not _vault.get_client.called
@mock.patch.object(handlers, 'client_approle_authorized')
@mock.patch.object(handlers, 'vault')
def test_publish_ca_info_service_notrunning(self, _vault):
def test_publish_ca_info_service_notrunning(
self, _vault, _client_approle_authorized):
self.is_unit_paused_set.return_value = False
self.service_running.return_value = False
_client_approle_authorized.return_value = True
handlers.publish_ca_info()
self.set_flag.assert_called_with('failed.to.start')
@ -963,30 +976,36 @@ class TestHandlers(unit_tests.test_utils.CharmTestCase):
ttl='8759h')
self.set_flag.assert_called_once_with('pki.backend.tuned')
@mock.patch.object(handlers, 'client_approle_authorized')
@mock.patch.object(handlers, 'vault')
@mock.patch.object(handlers, 'vault_pki')
def test_tune_pki_backend_config_changed(self, vault_pki, _vault):
def test_tune_pki_backend_config_changed(
self, vault_pki, _vault, _client_approle_authorized):
self.is_unit_paused_set.return_value = False
self._set_sealed(_vault, False)
self.config.return_value = {
'default-ttl': '8759h',
'max-ttl': '87600h',
}
_client_approle_authorized.return_value = True
handlers.tune_pki_backend_config_changed()
vault_pki.tune_pki_backend.assert_called_once_with(max_ttl='87600h',
ttl='8759h')
vault_pki.update_roles.assert_called_once_with(max_ttl='87600h')
@mock.patch.object(handlers, 'client_approle_authorized')
@mock.patch.object(handlers, 'vault')
@mock.patch.object(handlers, 'vault_pki')
def test_tune_pki_backend_config_changed_sealed(self, vault_pki, _vault):
def test_tune_pki_backend_config_changed_sealed(
self, vault_pki, _vault, _client_approle_authorized):
self.is_unit_paused_set.return_value = False
self._set_sealed(_vault, True)
self.config.return_value = {
'default-ttl': '8759h',
'max-ttl': '87600h',
}
_client_approle_authorized.return_value = True
handlers.tune_pki_backend_config_changed()
assert not vault_pki.tune_pki_backend.called