Skip to content
This repository was archived by the owner on Jan 19, 2018. It is now read-only.

Add support for authenticating via client certificate for Openshift #696

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add support for authenticating via client certificate for Openshift #696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bd6fd20
Added cert param in `make_rest_request` to pass client side certs
surajssd Apr 13, 2016
ba16abd
Improve kubeconfig parsing
kadel Apr 18, 2016
f396a57
OpenShift: Add support for authentication using X509 certs.
kadel Apr 18, 2016
cc1fe89
Add unittests for kubeconfig
kadel Apr 20, 2016
bdc3a2b
Remove temporary files generated to store certs and keys
surajssd Apr 21, 2016
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
65 changes: 55 additions & 10 deletions atomicapp/providers/lib/kubeconfig.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import anymarkup
import os
from base64 import b64decode

from atomicapp.plugin import ProviderFailedException
from atomicapp.constants import (PROVIDER_AUTH_KEY,
Expand All @@ -8,6 +10,8 @@
PROVIDER_TLS_VERIFY_KEY,
PROVIDER_CA_KEY)
import logging
import atexit
from atomicapp.utils import Utils
logger = logging.getLogger(LOGGER_DEFAULT)


Expand Down Expand Up @@ -68,7 +72,7 @@ def parse_kubeconf_data(kubecfg):
dict of parsed values from config
"""
url = None
token = None
auth = None
namespace = None
tls_verify = True
ca = None
Expand Down Expand Up @@ -103,16 +107,57 @@ def parse_kubeconf_data(kubecfg):
logger.debug("user: %s", user)

url = cluster["cluster"]["server"]
token = user["user"]["token"]
if "namespace" in context["context"]:
namespace = context["context"]["namespace"]
if "insecure-skip-tls-verify" in cluster["cluster"]:
tls_verify = not cluster["cluster"]["insecure-skip-tls-verify"]
elif "certificate-authority" in cluster["cluster"]:
ca = cluster["cluster"]["certificate-authority"]

auth = user["user"].get("token")
namespace = context["context"].get("namespace")
tls_verify = not cluster["cluster"].get("insecure-skip-tls-verify")

temporary_files = []
if tls_verify:
ca_data = cluster["cluster"].get("certificate-authority-data")
if ca_data:
ca = Utils.getTmpFile(b64decode(ca_data))
temporary_files.append(ca)
else:
if "certificate-authority" in cluster["cluster"]:
# if we are in container translate path to path on host
ca = os.path.join(Utils.getRoot(),
cluster["cluster"].get("certificate-authority").lstrip('/'))
Copy link
Contributor

Choose a reason for hiding this comment

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

really long line.. can we break it up into two? maybe cluster["cluster"].get("certificate-authority").lstrip('/') on the first and the join on the 2nd.

Copy link
Contributor

@dustymabe dustymabe Apr 20, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

"into two" meaning we would need to assignments probably

ca = cluster["cluster"].get("certificate-authority").lstrip('/')
ca = os.path.join(Utils.getRoot(), ca)

Copy link
Collaborator Author

@kadel kadel Apr 21, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Now when #705 is merged, I'm going to shorten this by using get_real_abspath

Copy link
Member

Choose a reason for hiding this comment

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

hey @kadel and @dustymabe feel free to look at my implementation def cert_file under https://github.com/projectatomic/atomicapp/pull/712/files (specifically, kubebase.py) for how I implemented the base64 encoding / files and such.


Copy link
Contributor

Choose a reason for hiding this comment

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

Should probably add a comment above the if statement below that states something like: "We will give precedence to an access token if it was provided in the kube-config, if not given an access token then check for client certs."

if not auth:
# If token not specified, check for certificate auth.

# client-certificate-data and client-key-data options overrides
# client-certificate and client-key
# https://github.com/kubernetes/kubernetes/blob/v1.2.2/pkg/client/unversioned/clientcmd/api/types.go#L78
# `{client-certificate,client-key}-data` keys in Kubernetes config
# file are inline base64 encoded certificates, requests library
# requires certs in files, this is why we are putting them to tmp
# files.
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add a few more users with examples of these cases to the Example of expected file format: section of the parse_kubeconf function description?

you can truncate output for brevity, just would like a code reader to be able to reference something so they can better understand this code.


cert_data = user["user"].get("client-certificate-data")
key_data = user["user"].get("client-key-data")

if cert_data:
cert = Utils.getTmpFile(b64decode(cert_data))
Copy link
Contributor

@dustymabe dustymabe Apr 20, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Do these tmp files ever get cleaned up? If not we should probably try to make sure they do. I know this makes it harder :(

Copy link
Contributor

Choose a reason for hiding this comment

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

might be able to use atexit to register these to be deleted. https://docs.python.org/2/library/atexit.html

temporary_files.append(cert)
else:
if "client-certificate" in user["user"]:
cert = os.path.join(Utils.getRoot(),
user["user"].get("client-certificate").lstrip('/'))
Copy link
Contributor

Choose a reason for hiding this comment

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

really long line


if key_data:
key = Utils.getTmpFile(b64decode(key_data))
temporary_files.append(key)
else:
if "client-key" in user["user"]:
key = os.path.join(Utils.getRoot(),
user["user"].get("client-key").lstrip('/'))
Copy link
Contributor

Choose a reason for hiding this comment

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

really long line


auth = "{}:{}".format(cert, key)
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a comment that explains what this format is.. i.e. certfile:keyfile, because that is the format the provider expects it in.


atexit.register(Utils.rm_files, *temporary_files)
return {PROVIDER_API_KEY: url,
PROVIDER_AUTH_KEY: token,
PROVIDER_AUTH_KEY: auth,
NAMESPACE_KEY: namespace,
PROVIDER_TLS_VERIFY_KEY: tls_verify,
PROVIDER_CA_KEY: ca}
63 changes: 41 additions & 22 deletions atomicapp/providers/openshift.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,12 @@
class OpenshiftClient(object):

def __init__(self, providerapi, access_token,
provider_tls_verify, provider_ca):
provider_tls_verify, provider_ca, certs):
self.providerapi = providerapi
self.access_token = access_token
self.provider_tls_verify = provider_tls_verify
self.provider_ca = provider_ca
self.certs = certs

# construct full urls for api endpoints
self.kubernetes_api = urljoin(self.providerapi, "api/v1/")
Expand All @@ -80,7 +81,8 @@ def test_connection(self):
(status_code, return_data) = \
Utils.make_rest_request("get",
self.openshift_api,
verify=self._requests_tls_verify())
verify=self._requests_tls_verify(),
cert=self.certs)
except SSLError as e:
if self.provider_tls_verify:
msg = "SSL/TLS ERROR: invalid certificate. " \
Expand All @@ -100,7 +102,8 @@ def get_oapi_resources(self):
(status_code, return_data) = \
Utils.make_rest_request("get",
self.openshift_api,
verify=self._requests_tls_verify())
verify=self._requests_tls_verify(),
cert=self.certs)
if status_code == 200:
oapi_resources = return_data["resources"]
else:
Expand All @@ -121,7 +124,8 @@ def get_kapi_resources(self):
(status_code, return_data) = \
Utils.make_rest_request("get",
self.kubernetes_api,
verify=self._requests_tls_verify())
verify=self._requests_tls_verify(),
cert=self.certs)
if status_code == 200:
kapi_resources = return_data["resources"]
else:
Expand All @@ -139,7 +143,8 @@ def deploy(self, url, artifact):
Utils.make_rest_request("post",
url,
verify=self._requests_tls_verify(),
data=artifact)
data=artifact,
cert=self.certs)
if status_code == 201:
logger.info("Object %s successfully deployed.",
artifact['metadata']['name'])
Expand All @@ -162,7 +167,8 @@ def delete(self, url):
(status_code, return_data) = \
Utils.make_rest_request("delete",
url,
verify=self._requests_tls_verify())
verify=self._requests_tls_verify(),
cert=self.certs)
if status_code == 200:
logger.info("Successfully deleted.")
else:
Expand All @@ -186,7 +192,8 @@ def scale(self, url, replicas):
Utils.make_rest_request("patch",
url,
data=patch,
verify=self._requests_tls_verify())
verify=self._requests_tls_verify(),
cert=self.certs)
if status_code == 200:
logger.info("Successfully scaled to %s replicas", replicas)
else:
Expand All @@ -199,7 +206,8 @@ def process_template(self, url, template):
Utils.make_rest_request("post",
url,
verify=self._requests_tls_verify(),
data=template)
data=template,
cert=self.certs)
if status_code == 201:
logger.info("template processed %s", template['metadata']['name'])
logger.debug("processed template %s", return_data)
Expand Down Expand Up @@ -306,7 +314,10 @@ def get_pod_status(self, namespace, pod):
'namespaces/{namespace}/pods/{pod}?'
'access_token={access_token}'.format(**args))
(status_code, return_data) = \
Utils.make_rest_request("get", url, verify=self._requests_tls_verify())
Utils.make_rest_request("get",
url,
verify=self._requests_tls_verify(),
cert=self.certs)

if status_code != 200:
raise ProviderFailedException(
Expand All @@ -331,6 +342,8 @@ class OpenShiftProvider(Provider):
provider_tls_verify = True
# path to file or dir with CA certificates
provider_ca = None
# client certificate and key for authentication
certs = None

def init(self):
# Parsed artifacts. Key is kind of artifacts. Value is list of artifacts.
Expand All @@ -341,7 +354,8 @@ def init(self):
self.oc = OpenshiftClient(self.providerapi,
self.access_token,
self.provider_tls_verify,
self.provider_ca)
self.provider_ca,
self.certs)
self.openshift_api = self.oc.openshift_api
self.kubernetes_api = self.oc.kubernetes_api

Expand Down Expand Up @@ -432,7 +446,10 @@ def stop(self):
"replicationcontroller",
params=params)
(status_code, return_data) = \
Utils.make_rest_request("get", url, verify=self.oc._requests_tls_verify())
Utils.make_rest_request("get",
url,
verify=self.oc._requests_tls_verify(),
cert=self.certs)
if status_code != 200:
raise ProviderFailedException("Cannot get Replication"
"Controllers for Deployment"
Expand Down Expand Up @@ -593,10 +610,11 @@ def _get_url(self, namespace, kind, name=None, params=None):
if name:
url = urljoin(url, name)

if params:
params["access_token"] = self.access_token
else:
params = {"access_token": self.access_token}
if not params:
params = {}
Copy link
Contributor

Choose a reason for hiding this comment

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

just set params={} in the function declaration. That way {} is the default value given if params isn't provided right? I guess someone could provide params=None though :(


if self.access_token:
params["access_token"] = self.access_token

url = urljoin(url, "?%s" % urlencode(params))
logger.debug("url: %s", url)
Expand Down Expand Up @@ -677,15 +695,16 @@ def _set_config_values(self):

# set config values
self.providerapi = result[PROVIDER_API_KEY]
self.access_token = result[PROVIDER_AUTH_KEY]
if ":" in result[PROVIDER_AUTH_KEY]:
self.access_token = None
self.certs = tuple(result[PROVIDER_AUTH_KEY].split(":"))
else:
self.access_token = result[PROVIDER_AUTH_KEY]
self.certs = None

self.namespace = result[NAMESPACE_KEY]
self.provider_tls_verify = result[PROVIDER_TLS_VERIFY_KEY]
if result[PROVIDER_CA_KEY]:
# if we are in container translate path to path on host
self.provider_ca = os.path.join(Utils.getRoot(),
result[PROVIDER_CA_KEY].lstrip('/'))
else:
self.provider_ca = None
self.provider_ca = result[PROVIDER_CA_KEY]

def extract(self, image, src, dest, update=True):
"""
Expand Down
32 changes: 26 additions & 6 deletions atomicapp/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,21 @@ def rm_dir(directory):
logger.debug('Recursively removing directory: %s' % directory)
distutils.dir_util.remove_tree(directory)

@staticmethod
def rm_files(*files):
"""
Remove the files whose paths are provided in 'files'

Args:
files (list/tuple): each item in iterable is a path to file
as string

Returns: None
"""
for file in files:
if os.path.exists(file):
os.remove(file)

@staticmethod
def getUserName():
"""
Expand Down Expand Up @@ -432,7 +447,7 @@ def getUserHome():
return home

@staticmethod
def make_rest_request(method, url, verify=True, data=None):
def make_rest_request(method, url, verify=True, data=None, cert=None):
"""
Make HTTP request to url

Expand All @@ -444,6 +459,10 @@ def make_rest_request(method, url, verify=True, data=None):
of trusted CAs
data (dict/list): object to be serialised to json and send as http
data (when method=post/put/delete)
cert (tuple/string): Path to local cert as client side certificate,
as a single file (containing the certificate
and the private key) or as a tuple of both
file's path

Returns:
tuple (status_code, return_data): status_code - http status code
Expand All @@ -459,16 +478,17 @@ def make_rest_request(method, url, verify=True, data=None):

try:
if method.lower() == "get":
res = requests.get(url, verify=verify)
res = requests.get(url, verify=verify, cert=cert)
elif method.lower() == "post":
res = requests.post(url, json=data, verify=verify)
res = requests.post(url, json=data, verify=verify, cert=cert)
elif method.lower() == "put":
res = requests.put(url, json=data, verify=verify)
res = requests.put(url, json=data, verify=verify, cert=cert)
elif method.lower() == "delete":
res = requests.delete(url, json=data, verify=verify)
res = requests.delete(url, json=data, verify=verify, cert=cert)
elif method.lower() == "patch":
headers = {"Content-Type": "application/json-patch+json"}
res = requests.patch(url, json=data, verify=verify, headers=headers)
res = requests.patch(url, json=data, verify=verify,
headers=headers, cert=cert)

status_code = res.status_code
return_data = res.json()
Expand Down
Loading