Fix For read_health_status() Exception Handling (#347)

* Add url param for create_client

* Install consul for test cases involving Vault HA

* Update test harness with optional Vault HA / consul set up

* Add regerssion test cases for issue #339

* Add raise_exception param to requests

* Use raise_exception param in read_health_status method

* Add ipaddress module per github.com/urllib3/urllib3/issues/1117?

Somehow ended up with this urllib3 error for python 2.7 otherwise:
"urllib3.connection: ERROR: Certificate did not match expected hostname:
127.0.0.1. Certificate: {'serialNumber': u'8D267F50728FF454',
'subject': ((('commonName', u'localhost'),),),
'notAfter': 'May 14 22:44:13 2025 GMT',
'notBefore': u'May 17 22:44:13 2015 GMT',
'subjectAltName': (('DNS', 'localhost'), ('IP Address', '127.0.0.1')),
'issuer': ((('commonName', u'localhost'),),), 'version': 3L}

* Clarify docstring a bit

* Also add cases to cover both HEAD and GET methods

* Remove standby node magic strings; use method instead

.travis.yml

@@ -85,6 +85,7 @@ matrix:
         - TOXENV=py36
         - HVAC_VAULT_VERSION=HEAD
 install:
+  - tests/scripts/install-consul.sh
   - tests/scripts/install-vault.sh ${HVAC_VAULT_VERSION}
   - pip install tox
 script:

hvac/adapters.py

@@ -4,12 +4,12 @@
 
 """
 import logging
+from abc import ABCMeta, abstractmethod
 
 import requests
 import requests.exceptions
 
 from hvac import utils
-from abc import ABCMeta, abstractmethod
 
 logger = logging.getLogger(__name__)
 
@@ -193,7 +193,7 @@ def auth(self, url, use_token=True, **kwargs):
         )
 
     @abstractmethod
-    def request(self, method, url, headers=None, **kwargs):
+    def request(self, method, url, headers=None, raise_exception=True, **kwargs):
         """Main method for routing HTTP requests to the configured Vault base_uri. Intended to be implement by subclasses.
 
         :param method: HTTP method to use with the request. E.g., GET, POST, etc.
@@ -205,6 +205,9 @@ def request(self, method, url, headers=None, **kwargs):
         :type headers: dict
         :param kwargs: Additional keyword arguments to include in the requests call.
         :type kwargs: dict
+        :param raise_exception: If True, raise an exception via utils.raise_for_error(). Set this parameter to False to
+            bypass this functionality.
+        :type raise_exception: bool
         :return: The response of the request.
         :rtype: requests.Response
         """
@@ -214,7 +217,7 @@ def request(self, method, url, headers=None, **kwargs):
 class Request(Adapter):
     """The Request adapter class"""
 
-    def request(self, method, url, headers=None, **kwargs):
+    def request(self, method, url, headers=None, raise_exception=True, **kwargs):
         """Main method for routing HTTP requests to the configured Vault base_uri.
 
         :param method: HTTP method to use with the request. E.g., GET, POST, etc.
@@ -224,6 +227,9 @@ def request(self, method, url, headers=None, **kwargs):
         :type url: str | unicode
         :param headers: Additional headers to include with the request.
         :type headers: dict
+        :param raise_exception: If True, raise an exception via utils.raise_for_error(). Set this parameter to False to
+            bypass this functionality.
+        :type raise_exception: bool
         :param kwargs: Additional keyword arguments to include in the requests call.
         :type kwargs: dict
         :return: The response of the request.
@@ -256,7 +262,7 @@ def request(self, method, url, headers=None, **kwargs):
             response = self.session.request(method, url, headers=headers,
                                             allow_redirects=False, **_kwargs)
 
-        if response.status_code >= 400 and response.status_code < 600:
+        if raise_exception and 400 <= response.status_code < 600:
             text = errors = None
             if response.headers.get('Content-Type') == 'application/json':
                 errors = response.json().get('errors')

hvac/api/system_backend/health.py

@@ -57,13 +57,15 @@ def read_health_status(self, standby_ok=False, active_code=200, standby_code=429
             api_path = '/v1/sys/health'.format()
             response = self._adapter.head(
                 url=api_path,
+                raise_exception=False,
             )
             return response
         elif method == 'GET':
             api_path = '/v1/sys/health'.format()
             response = self._adapter.get(
                 url=api_path,
                 json=params,
+                raise_exception=False,
             )
             return response.json()
         else:

requirements-dev.txt

@@ -1,5 +1,6 @@
 codecov
 coverage
+ipaddress>=1.0.22
 mock
 nose
 parameterized

tests/config_files/vault-ha-node1.hcl

@@ -0,0 +1,15 @@
+listener "tcp" {
+  address = "127.0.0.1:8200"
+  tls_cert_file = "tests/config_files/server-cert.pem"
+  tls_key_file  = "tests/config_files/server-key.pem"
+}
+
+disable_mlock = true
+
+default_lease_ttl = "768h"
+max_lease_ttl = "768h"
+
+storage "consul" {
+  address = "127.0.0.1:8500"
+  path    = "vault"
+}

tests/config_files/vault-ha-node2.hcl

@@ -0,0 +1,16 @@
+listener "tcp" {
+  address = "127.0.0.1:8199"
+  cluster_address = "127.0.0.1:8201"
+  tls_cert_file = "tests/config_files/server-cert.pem"
+  tls_key_file  = "tests/config_files/server-key.pem"
+}
+
+disable_mlock = true
+
+default_lease_ttl = "768h"
+max_lease_ttl = "768h"
+
+storage "consul" {
+  address = "127.0.0.1:8500"
+  path    = "vault"
+}

tests/integration_tests/api/system_backend/test_health.py

@@ -2,30 +2,95 @@
 from unittest import TestCase
 
 from parameterized import parameterized, param
-
+from tests.utils import create_client
 from tests.utils.hvac_integration_test_case import HvacIntegrationTestCase
 
 
 class TestHealth(HvacIntegrationTestCase, TestCase):
+    enable_vault_ha = True
+
+    def tearDown(self):
+        # If one of our test cases left the Vault cluster sealed, unseal it here.
+        self.manager.unseal()
+        super(TestHealth, self).tearDown()
 
     @parameterized.expand([
         param(
             'default params',
         ),
+        param(
+            'unsealed standby node HEAD method',
+            use_standby_node=True,
+            method='HEAD',
+            expected_status_code=429,
+            ha_required=True,
+        ),
+        param(
+            'unsealed standby node GET method',
+            use_standby_node=True,
+            method='GET',
+            expected_status_code=429,
+            ha_required=True,
+        ),
+        param(
+            'sealed standby node HEAD method',
+            use_standby_node=True,
+            method='HEAD',
+            expected_status_code=503,
+            seal_first=True,
+            ha_required=True,
+        ),
+        param(
+            'sealed standby node GET method',
+            use_standby_node=True,
+            method='GET',
+            expected_status_code=503,
+            seal_first=True,
+            ha_required=True,
+        ),
         param(
             'GET method',
             method='GET'
         ),
     ])
-    def test_read_health_status(self, label, method='HEAD'):
-        read_status_response = self.client.sys.read_health_status(
+    def test_read_health_status(self, label, method='HEAD', use_standby_node=False, expected_status_code=200, seal_first=False, ha_required=False):
+        """Test the Health system backend class's "read_health_status" method.
+
+        :param label: Label for a given parameterized test case.
+        :type label: str
+        :param method: HTTP method to use when invoking the method under test (GET or HEAD available).
+        :type method: str
+        :param use_standby_node: If True, send the request to a standby Vault node address
+        :type use_standby_node: bool
+        :param expected_status_code: The status code code expected in the response from Vault.
+        :type expected_status_code: int
+        :param seal_first: If True, seal the Vault node(s) before running the test cases.
+        :type seal_first: bool
+        :param ha_required: If True, skip the test case when consul / a HA Vault integration cluster is unavailable.
+        :type ha_required: bool
+        """
+        if ha_required and not self.enable_vault_ha:
+            # Conditional to allow folks to run this test class without requiring consul to be installed locally.
+            self.skipTest('Skipping test case, Vault HA required but not available.')
+        if seal_first:
+            # Standby nodes can't be sealed directly.
+            # I.e.: "vault cannot seal when in standby mode; please restart instead"
+            self.manager.restart_vault_cluster()
+        if use_standby_node:
+            standby_vault_addr = self.get_standby_vault_addr()
+            logging.debug('standby_vault_addr being used: %s' % standby_vault_addr)
+            client = create_client(url=standby_vault_addr)
+        else:
+            client = self.client
+        logging.debug('vault processes: %s' % self.manager._processes)
+        read_status_response = client.sys.read_health_status(
             method=method,
         )
         logging.debug('read_status_response: %s' % read_status_response)
         if method == 'HEAD':
             self.assertEqual(
                 first=read_status_response.status_code,
-                second=200
+                second=expected_status_code,
             )
         else:
             self.assertTrue(

tests/scripts/install-consul.sh

@@ -0,0 +1,17 @@
+#!/bin/bash
+set -eux
+
+DEFAULT_CONSUL_VERSION=1.4.0
+HVAC_CONSUL_VERSION=${1:-$DEFAULT_CONSUL_VERSION}
+
+function install_consul_release() {
+    mkdir -p $HOME/bin
+
+    cd /tmp
+
+    curl -sOL https://releases.hashicorp.com/consul/${HVAC_CONSUL_VERSION}/consul_${HVAC_CONSUL_VERSION}_linux_amd64.zip
+    unzip consul_${HVAC_CONSUL_VERSION}_linux_amd64.zip
+    mv consul $HOME/bin
+}
+
+install_consul_release

tests/utils/__init__.py

@@ -60,9 +60,11 @@ def get_generate_root_otp():
     return test_otp
 
 
-def create_client(**kwargs):
+def create_client(url='https://localhost:8200', **kwargs):
     """Small helper to instantiate a :py:class:`hvac.v1.Client` class with the appropriate parameters for the test env.
 
+    :param url: Vault address to configure the client with.
+    :type url: str
     :param kwargs: Dictionary of additional keyword arguments to pass through to the Client instance being created.
     :type kwargs: dict
     :return: Instantiated :py:class:`hvac.v1.Client` class.
@@ -73,7 +75,7 @@ def create_client(**kwargs):
     server_cert_path = get_config_file_path('server-cert.pem')
 
     return Client(
-        url='https://localhost:8200',
+        url=url,
         cert=(client_cert_path, client_key_path),
         verify=server_cert_path,
         **kwargs

tests/utils/hvac_integration_test_case.py

@@ -1,12 +1,14 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
+import logging
 import re
 import warnings
 
 from mock import patch
 
 from tests.utils import get_config_file_path, create_client
 from tests.utils.server_manager import ServerManager
+import distutils.spawn
 
 
 class HvacIntegrationTestCase(object):
@@ -15,13 +17,24 @@ class HvacIntegrationTestCase(object):
     manager = None
     client = None
     mock_warnings = None
+    enable_vault_ha = False
 
     @classmethod
     def setUpClass(cls):
         """Use the ServerManager class to launch a vault server process."""
+        config_paths = [get_config_file_path('vault-tls.hcl')]
+        if distutils.spawn.find_executable('consul') is None and cls.enable_vault_ha:
+            logging.warning('Unable to run Vault in HA mode, consul binary not found in path.')
+            cls.enable_vault_ha = False
+        if cls.enable_vault_ha:
+            config_paths = [
+                get_config_file_path('vault-ha-node1.hcl'),
+                get_config_file_path('vault-ha-node2.hcl'),
+            ]
         cls.manager = ServerManager(
-            config_path=get_config_file_path('vault-tls.hcl'),
-            client=create_client()
+            config_paths=config_paths,
+            client=create_client(),
+            use_consul=cls.enable_vault_ha,
         )
         cls.manager.start()
         cls.manager.initialize()
@@ -141,3 +154,15 @@ def disable_pki(self, mount_point='pki'):
         :type mount_point: str
         """
         self.client.disable_secret_backend(mount_point)
+
+    def get_standby_vault_addr(self):
+        """Get an address for a Vault HA node currently in standby.
+
+        :return: Standby Vault address.
+        :rtype: str
+        """
+        vault_addresses = self.manager.get_active_vault_addresses()
+        for vault_address in vault_addresses:
+            health_status = create_client(url=vault_address).sys.read_health_status(method='GET')
+            if health_status['standby']:
+                return vault_address