Skip to content

Commit

Permalink
Address review comments and improve auth
Browse files Browse the repository at this point in the history 
Auth now chooses between cert (#1) and bearer token (#2)
instead of sending both. Also renamed ssl --> tls for correctness
  • Loading branch information
@hkaj
hkaj committed Jan 27, 2017
1 parent dbf5b46 commit 17ea82e
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 100 deletions.
2 changes: 1 addition & 1 deletion checks.d/kubernetes.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def _perform_kubelet_checks(self, url):
service_check_base = NAMESPACE + '.kubelet.check'
is_ok = True
try:
req = self.kubeutil.retrieve_kubelet_url(url)
req = self.kubeutil.perform_kubelet_query(url)
for line in req.iter_lines():

# avoid noise; this check is expected to fail since we override the container hostname
Expand Down
21 changes: 12 additions & 9 deletions conf.d/kubernetes.yaml.example
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,29 +66,32 @@ instances:
# kubelet port. It needs to be set if you are not using a default one (10250 or 10255)
# kubelet_port: 10255
#
# SSL configuration for querying kubelet.
# TLS configuration for querying kubelet.
#
# Available options are:
# - simple SSL validation (with readily available certificates)
# - disabling SSL validation
# (server auth)
# - simple TLS validation (with readily available certificates)
# - disabling TLS validation
# - providing the server certificate
# (client auth)
# - providing a client cert/key pair
# - using a service account bearer token
#
# The default is to try and use the read-only port that doesn't require SSL
# And to fall back to the HTTPS API with simple SSL validation.
# The default is to try and use the read-only port that doesn't require TLS
# And to fall back to the HTTPS API with simple TLS validation.
#
# Explicitly disabling ssl_verify should be used with caution:
# Explicitly disabling tls_verify should be used with caution:
# if an attacker sniffs the agent requests they will see the agent's service account bearer token.
#
# Providing a server cert enables SSL validation.
# Providing a server cert enables TLS validation.
#
# It is possible to providing a client cert/key pair for client auth.
# The agent also uses its pod bearer token (if available) when querying kubelet and the apiserver.
# If no such cert is passed, the agent authenticates using its pod bearer token (if available) when querying kubelet and the apiserver.
#
# The recommended way to pass certificates and keys to the agent is by using
# "Secret" Kubernetes object (and to mount them as volumes).
#
# kubelet_ssl_verify: True
# kubelet_tls_verify: True
# kubelet_cert: /path/to/ca.pem
# kubelet_client_crt: /path/to/client.crt
# kubelet_client_key: /path/to/client.key
Expand Down
101 changes: 45 additions & 56 deletions tests/checks/mock/test_kubernetes.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,22 +385,31 @@ class TestKubeutil(unittest.TestCase):
def setUp(self, _locate_kubelet):
self.kubeutil = KubeUtil()

def test_init_ssl_settings(self):
@mock.patch('os.path.exists', return_value=True)
@mock.patch('utils.kubernetes.kubeutil.KubeUtil.get_auth_token', return_value='tkn')
def test_init_tls_settings(self, *args):
# kubelet
instances = [
# (instance, expected_result)
({}, {'verify': True}),
({'kubelet_ssl_verify': False}, {'verify': False}),
({'kubelet_ssl_verify': True}, {'verify': True}),
({'kubelet_ssl_verify': 'foo.pem'}, {'verify': 'foo.pem'}),
({'kubelet_cert': 'foo.pem'}, {'verify': 'foo.pem'}),
({}, {'kubelet_verify': True, 'bearer_token': 'tkn'}),
({'kubelet_tls_verify': False}, {'kubelet_verify': False, 'bearer_token': 'tkn'}),
({'kubelet_tls_verify': True}, {'kubelet_verify': True, 'bearer_token': 'tkn'}),
({'kubelet_tls_verify': 'foo.pem'}, {'kubelet_verify': 'foo.pem', 'bearer_token': 'tkn'}),
({'kubelet_cert': 'foo.pem'}, {'kubelet_verify': 'foo.pem', 'bearer_token': 'tkn'}),
({'kubelet_client_crt': 'client.crt', 'kubelet_client_key': 'client.key'},
{'verify': True, 'kubelet_client_cert': ('client.crt', 'client.key')}),
({'kubelet_ssl_verify': True, 'kubelet_client_crt': 'client.crt'}, {'verify': True}),
({'kubelet_client_crt': 'client.crt'}, {'verify': True})
{'kubelet_verify': True, 'kubelet_client_cert': ('client.crt', 'client.key'), 'bearer_token': 'tkn'}),
({'kubelet_tls_verify': True, 'kubelet_client_crt': 'client.crt'}, {'kubelet_verify': True, 'bearer_token': 'tkn'}),
({'kubelet_client_crt': 'client.crt'}, {'kubelet_verify': True, 'bearer_token': 'tkn'})
]
for instance, result in instances:
self.assertEqual(self.kubeutil._init_ssl_settings(instance), result)
self.assertEqual(self.kubeutil._init_tls_settings(instance), result)

# apiserver
instance = {'apiserver_client_crt': 'foo.crt', 'apiserver_client_key': 'foo.key'}
expected_res = {'apiserver_client_cert': ('foo.crt', 'foo.key'), 'kubelet_verify': True, 'bearer_token': 'tkn'}
self.assertEqual(self.kubeutil._init_tls_settings(instance), expected_res)
with mock.patch('utils.kubernetes.kubeutil.os.path.exists', return_value=False):
self.assertEqual(self.kubeutil._init_tls_settings(instance), {'kubelet_verify': True, 'bearer_token': 'tkn'})

##### Test _locate_kubelet #####

Expand All @@ -419,7 +428,7 @@ def test_locate_kubelet_no_auth_no_ssl(self, _get_hostname):
({'kubelet_port': '1337'}, 'http://test_docker_host:1337'),
({'host': 'test_explicit_host', 'kubelet_port': '1337'}, 'http://test_explicit_host:1337')
]
with mock.patch('utils.kubernetes.kubeutil.KubeUtil.retrieve_kubelet_url', return_value=True):
with mock.patch('utils.kubernetes.kubeutil.KubeUtil.perform_kubelet_query', return_value=True):
for instance, result in no_auth_no_ssl_instances:
self.assertEqual(self.kubeutil._locate_kubelet(instance), result)

Expand All @@ -434,13 +443,13 @@ def test_locate_kubelet_no_auth_no_verify(self, _get_hostname):
]

def side_effect(url):
"""Mock KubeUtil.retrieve_kubelet_url"""
"""Mock KubeUtil.perform_kubelet_query"""
if url.startswith('https://'):
return True
else:
raise Exception()

with mock.patch('utils.kubernetes.kubeutil.KubeUtil.retrieve_kubelet_url', side_effect=side_effect):
with mock.patch('utils.kubernetes.kubeutil.KubeUtil.perform_kubelet_query', side_effect=side_effect):
for instance, result in no_auth_no_verify_instances:
self.assertEqual(self.kubeutil._locate_kubelet(instance), result)

Expand All @@ -449,85 +458,65 @@ def side_effect(url):
@mock.patch('utils.kubernetes.kubeutil.KubeUtil.get_auth_token', return_value='foo')
def test_locate_kubelet_verify_and_auth(self, *args):
"""
Test kubelet connection with SSL. Also look for auth token.
Test kubelet connection with TLS. Also look for auth token.
"""
no_auth_instances = [
# instance, ssl_settings, expected_result
({}, {'verify': True}, 'https://test_k8s_host:10250'),
({'kubelet_port': '1337'}, {'verify': 'test.pem'}, 'https://test_k8s_host:1337'),
# instance, tls_settings, expected_result
({}, {'kubelet_verify': True}, 'https://test_k8s_host:10250'),
({'kubelet_port': '1337'}, {'kubelet_verify': 'test.pem'}, 'https://test_k8s_host:1337'),
(
{'host': 'test_explicit_host'},
{'verify': True, 'kubelet_client_cert': ('client.crt', 'client.key')},
{'kubelet_verify': True, 'kubelet_client_cert': ('client.crt', 'client.key')},
'https://test_explicit_host:10250'
),
(
{'host': 'test_explicit_host', 'kubelet_port': '1337'},
{'verify': True},
{'kubelet_verify': True},
'https://test_explicit_host:1337'
),
]

def side_effect(url, **kwargs):
"""Mock KubeUtil.retrieve_kubelet_url"""
"""Mock KubeUtil.perform_kubelet_query"""
if url.startswith('https://') and '10255' not in url:
return True
else:
raise Exception()

# no auth / SSL with verify
for instance, ssl_settings, result in no_auth_instances:
# no auth / TLS with verify
for instance, tls_settings, result in no_auth_instances:
with mock.patch('utils.kubernetes.kubeutil.requests') as req:
req.get = mock.MagicMock(side_effect=side_effect)
self.kubeutil.ssl_settings = ssl_settings
self.kubeutil.tls_settings = tls_settings
self.assertEqual(self.kubeutil._locate_kubelet(instance), result)
req.get.assert_called_with(result + '/healthz', # test endpoint
timeout=10,
verify=ssl_settings.get('verify', False),
cert=ssl_settings.get('kubelet_client_cert'),
headers={'Authorization': 'Bearer foo'}, # auth
verify=tls_settings.get('kubelet_verify', True),
headers={'Authorization': 'Bearer foo'} if 'kubelet_client_cert' not in tls_settings else None,
cert=tls_settings.get('kubelet_client_cert'),
params={'verbose': True}
)

@mock.patch('utils.kubernetes.kubeutil.KubeUtil.get_auth_token', return_value='foo')
def test_get_node_hostname(self, _get_auth_tkn):
node_lists = [
(json.loads(Fixtures.read_file('filtered_node_list_1_4.json', string_escape=False)), 'ip-10-0-0-179'),
({'items': [{'foo': 'bar'}]}, None)
]

exception_node_lists = [
{'items': []},
{'items': [{'foo': 'bar'}, {'bar': 'foo'}]}
({'items': [{'foo': 'bar'}]}, None),
({'items': []}, None),
({'items': [{'foo': 'bar'}, {'bar': 'foo'}]}, None)
]

for node_list, expected_result in node_lists:
with mock.patch('utils.kubernetes.kubeutil.KubeUtil.retrieve_json_auth', return_value=node_list):
self.assertEqual(self.kubeutil.get_node_hostname('ip-10-0-0-179'), expected_result)

for node_list in exception_node_lists:
with mock.patch('utils.kubernetes.kubeutil.KubeUtil.retrieve_json_auth', return_value=node_list):
self.assertRaises(Exception, self.kubeutil.get_node_hostname, 'ip-10-0-0-179')

@mock.patch('utils.kubernetes.KubeUtil.retrieve_pods_list', side_effect=['foo'])
@mock.patch('utils.kubernetes.KubeUtil.extract_kube_labels')
def test_get_kube_labels(self, extract_kube_labels, retrieve_pods_list):
self.kubeutil.get_kube_labels(excluded_keys='bar')
retrieve_pods_list.assert_called_once()
extract_kube_labels.assert_called_once_with('foo', excluded_keys='bar')

@mock.patch('utils.kubernetes.kubeutil.KubeUtil.get_auth_token', return_value=None)
def test_init_tls_settings(self, get_tkn):
self.assertEqual(self.kubeutil._init_tls_settings({}), {})
self.assertEqual(self.kubeutil._init_tls_settings({'apiserver_client_crt': 'foo.crt'}), {})

instance = {'apiserver_client_crt': 'foo.crt', 'apiserver_client_key': 'foo.key'}
with mock.patch('utils.kubernetes.kubeutil.os.path.exists', return_value=True):
expected_res = {'apiserver_client_cert': ('foo.crt', 'foo.key')}
self.assertEqual(self.kubeutil._init_tls_settings(instance), expected_res)

with mock.patch('utils.kubernetes.kubeutil.os.path.exists', return_value=False):
self.assertEqual(self.kubeutil._init_tls_settings(instance), {})

def test_extract_kube_labels(self):
"""
Test with both 1.1 and 1.2 version payloads
Expand All @@ -551,7 +540,7 @@ def test_extract_kube_labels(self):
labels = set(inn for out in res.values() for inn in out)
self.assertEqual(len(labels), 3)

@mock.patch('utils.kubernetes.kubeutil.KubeUtil.retrieve_kubelet_url')
@mock.patch('utils.kubernetes.kubeutil.KubeUtil.perform_kubelet_query')
def test_retrieve_pods_list(self, retrieve_url):
self.kubeutil.retrieve_pods_list()
retrieve_url.assert_called_twice_with(self.kubeutil.pods_list_url, verbose=True, timeout=10)
Expand All @@ -568,7 +557,7 @@ def test_retrieve_metrics(self, retrieve_json):

@mock.patch('utils.kubernetes.kubeutil.KubeUtil.get_auth_token', return_value='foo')
@mock.patch('utils.kubernetes.kubeutil.requests')
def test_retrieve_kubelet_url(self, req, _get_auth_tkn):
def test_perform_kubelet_query(self, req, _get_auth_tkn):
base_params = {'timeout': 10, 'verify': False,
'params': {'verbose': True}, 'cert': None, 'headers': None}

Expand All @@ -580,15 +569,15 @@ def test_retrieve_kubelet_url(self, req, _get_auth_tkn):
instances = [
('http://test.com', {}, dict(base_params.items() + verify_true.items())),
('https://test.com', {}, dict(base_params.items() + verify_true.items() + auth_token_header.items())),
('https://test.com', {'verify': True}, dict(base_params.items() + verify_true.items() + auth_token_header.items())),
('https://test.com', {'verify': 'kubelet.pem'}, dict(base_params.items() + verify_cert.items() + auth_token_header.items())),
('https://test.com', {'kubelet_verify': True}, dict(base_params.items() + verify_true.items() + auth_token_header.items())),
('https://test.com', {'kubelet_verify': 'kubelet.pem'}, dict(base_params.items() + verify_cert.items() + auth_token_header.items())),
('https://test.com', {'kubelet_client_cert': ('client.crt', 'client.key')},
dict(base_params.items() + verify_true.items() + client_cert.items() + auth_token_header.items())),
dict(base_params.items() + verify_true.items() + client_cert.items())),
]
for url, ssl_context, expected_params in instances:
req.get.reset_mock()
self.kubeutil.ssl_settings = ssl_context
self.kubeutil.retrieve_kubelet_url(url)
self.kubeutil.tls_settings = ssl_context
self.kubeutil.perform_kubelet_query(url)
req.get.assert_called_with(url, **expected_params)

@mock.patch('utils.kubernetes.kubeutil.requests')
Expand Down
Loading

0 comments on commit 17ea82e

Please sign in to comment.