Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stop using requests #1586

Draft
wants to merge 11 commits into
base: develop
Choose a base branch
from
Draft
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ $ ansible-galaxy collection install ./theforeman-foreman-*.tar.gz
These dependencies are required for the Ansible controller, not the Foreman server.

* [`PyYAML`](https://pypi.org/project/PyYAML/)
* [`requests`](https://pypi.org/project/requests/)
* `rpm` for the RPM support in the `content_upload` module
* `debian` for the DEB support in the `content_upload` module

Expand Down
1 change: 0 additions & 1 deletion bindep.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
python3-rpm [(platform:redhat platform:base-py3)]
python38-requests [platform:centos-8 platform:rhel-8]
14 changes: 4 additions & 10 deletions plugins/callback/foreman.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
- This callback will report facts and task events to Foreman
requirements:
- whitelisting in configuration
- requests (python library)
options:
report_type:
description:
Expand Down Expand Up @@ -112,10 +111,9 @@
import time

try:
import requests
HAS_REQUESTS = True
from ansible_collections.theforeman.foreman.plugins.module_utils.ansible_requests import RequestSession
except ImportError:
HAS_REQUESTS = False
from plugins.module_utils.ansible_requests import RequestSession

from ansible.module_utils._text import to_text
from ansible.module_utils.common.json import AnsibleJSONEncoder
Expand Down Expand Up @@ -208,10 +206,7 @@ def set_options(self, task_keys=None, var_options=None, direct=None):
ssl_key = self.get_option('client_key')
self.dir_store = self.get_option('dir_store')

if not HAS_REQUESTS:
self._disable_plugin(u'The `requests` python module is not installed')

self.session = requests.Session()
self.session = RequestSession()
if self.foreman_url.startswith('https://'):
if not os.path.exists(ssl_cert):
self._disable_plugin(u'FOREMAN_SSL_CERT %s not found.' % ssl_cert)
Expand All @@ -236,7 +231,6 @@ def _ssl_verify(self, option):
verify = option

if verify is False: # is only set to bool if try block succeeds
requests.packages.urllib3.disable_warnings()
self._display.warning(
u"SSL verification of %s disabled" % self.foreman_url,
)
Expand Down Expand Up @@ -265,7 +259,7 @@ def _send_data(self, data_type, report_type, host, data):
headers = {'content-type': 'application/json'}
response = self.session.post(url=url, data=json_data, headers=headers)
response.raise_for_status()
except requests.exceptions.RequestException as err:
except Exception as err:
self._display.warning(u'Sending data to Foreman at {url} failed for {host}: {err}'.format(
host=to_text(host), err=to_text(err), url=to_text(self.foreman_url)))

Expand Down
2 changes: 0 additions & 2 deletions plugins/doc_fragments/foreman.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ class ModuleDocFragment(object):

# Foreman documentation fragment
DOCUMENTATION = '''
requirements:
- requests
options:
server_url:
description:
Expand Down
20 changes: 5 additions & 15 deletions plugins/inventory/foreman.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
DOCUMENTATION = '''
name: foreman
short_description: Foreman inventory source
requirements:
- requests >= 1.1
description:
- Get inventory hosts from Foreman.
- Can use the Reports API (default) or the Hosts API to fetch information about the hosts.
Expand Down Expand Up @@ -195,19 +193,14 @@
from ansible_collections.theforeman.foreman.plugins.module_utils._version import LooseVersion
from time import sleep
from ansible.errors import AnsibleError
from ansible.module_utils._text import to_bytes, to_native, to_text
from ansible.module_utils._text import to_native, to_text
from ansible.module_utils.common._collections_compat import MutableMapping
from ansible.plugins.inventory import BaseInventoryPlugin, Cacheable, to_safe_group_name, Constructable

# 3rd party imports
try:
import requests
if LooseVersion(requests.__version__) < LooseVersion('1.1.0'):
raise ImportError
from requests.auth import HTTPBasicAuth
HAS_REQUESTS = True
from ansible_collections.theforeman.foreman.plugins.module_utils.ansible_requests import RequestSession
except ImportError:
HAS_REQUESTS = False
from plugins.module_utils.ansible_requests import RequestSession


class InventoryModule(BaseInventoryPlugin, Cacheable, Constructable):
Expand All @@ -226,9 +219,6 @@ def __init__(self):
self.cache_key = None
self.use_cache = None

if not HAS_REQUESTS:
raise AnsibleError('This script requires python-requests 1.1 as a minimum version')

def verify_file(self, path):

valid = False
Expand All @@ -241,8 +231,8 @@ def verify_file(self, path):

def _get_session(self):
if not self.session:
self.session = requests.session()
self.session.auth = HTTPBasicAuth(self.get_option('user'), to_bytes(self.get_option('password')))
self.session = RequestSession()
self.session.auth = (self.get_option('user'), self.get_option('password'))
self.session.verify = self.get_option('validate_certs')
return self.session

Expand Down
6 changes: 1 addition & 5 deletions plugins/module_utils/_apypie.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,10 +243,6 @@ def _prepare_route_params(self, input_dict):

import os
from urllib.parse import urljoin # type: ignore
try:
import requests
except ImportError:
pass

NO_CONTENT = 204

Expand Down Expand Up @@ -296,7 +292,7 @@ def __init__(self, **kwargs):
self.apidoc_cache_dir = kwargs.get('apidoc_cache_dir', apidoc_cache_dir_default)
self.apidoc_cache_name = kwargs.get('apidoc_cache_name', self._find_cache_name())

self._session = kwargs.get('session') or requests.Session()
self._session = kwargs.get('session')
self._session.verify = kwargs.get('verify_ssl', True)

self._session.headers['Accept'] = 'application/json;version={}'.format(self.api_version)
Expand Down
148 changes: 148 additions & 0 deletions plugins/module_utils/ansible_requests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
# -*- coding: utf-8 -*-

# pylint: disable=raise-missing-from
# pylint: disable=super-with-arguments

from __future__ import absolute_import, division, print_function
__metaclass__ = type


import json
from ansible.module_utils._text import to_native
from ansible.module_utils import six
from ansible.module_utils.urls import Request
try:
from ansible.module_utils.urls import prepare_multipart
except ImportError:
def prepare_multipart(fields):
raise NotImplementedError


class RequestException(Exception):
def __init__(self, msg, response):
super(RequestException, self).__init__(msg)
self.response = response


class RequestResponse(object):
def __init__(self, resp):
self._resp = resp
self._body = None

@property
def status_code(self):
if hasattr(self._resp, 'status'):
status = self._resp.status
elif hasattr(self._resp, 'code'):
status = self._resp.code
else:
status = self._resp.getcode()
return status

@property
def headers(self):
return self._resp.headers

@property
def url(self):
if hasattr(self._resp, 'url'):
url = self._resp.url
elif hasattr(self._resp, 'geturl'):
url = self._resp.geturl()
else:
url = ""
return url

@property
def reason(self):
if hasattr(self._resp, 'reason'):
reason = self._resp.reason
else:
reason = ""
return reason

@property
def body(self):
if self._body is None:
try:
self._body = self._resp.read()
except Exception:
pass
return self._body

@property
def text(self):
return to_native(self.body)

def raise_for_status(self):
http_error_msg = ""

if 400 <= self.status_code < 500:
http_error_msg = "{0} Client Error: {1} for url: {2}".format(self.status_code, self.reason, self.url)
elif 500 <= self.status_code < 600:
http_error_msg = "{0} Server Error: {1} for url: {2}".format(self.status_code, self.reason, self.url)

if http_error_msg:
raise RequestException(http_error_msg, response=self)

def json(self, **kwargs):
return json.loads(to_native(self.body), **kwargs)


class RequestSession(Request):
@property
def auth(self):
return (self.url_username, self.url_password)

@auth.setter
def auth(self, value):
self.url_username, self.url_password = value
self.force_basic_auth = True

@property
def verify(self):
return self.validate_certs

@verify.setter
def verify(self, value):
self.validate_certs = value

@property
def cert(self):
return (self.client_cert, self.client_key)

@cert.setter
def cert(self, value):
self.client_cert, self.client_key = value

def request(self, method, url, **kwargs):
validate_certs = kwargs.pop('verify', None)
params = kwargs.pop('params', None)
if params:
url += '?' + six.moves.urllib.parse.urlencode(params)
headers = kwargs.pop('headers', None)
data = kwargs.pop('data', None)
if data:
if not isinstance(data, six.string_types):
data = six.moves.urllib.parse.urlencode(data, doseq=True)
files = kwargs.pop('files', None)
if files:
ansible_files = {k: {'filename': v[0], 'content': v[1].read(), 'mime_type': v[2]} for (k, v) in files.items()}
_content_type, data = prepare_multipart(ansible_files)
if 'json' in kwargs:
# it can be `json=None`…
data = json.dumps(kwargs.pop('json'))
if headers is None:
headers = {}
headers['Content-Type'] = 'application/json'
try:
result = self.open(method, url, validate_certs=validate_certs, data=data, headers=headers, **kwargs)
return RequestResponse(result)
except six.moves.urllib.error.HTTPError as e:
return RequestResponse(e)

def get(self, url, **kwargs):
return self.request('GET', url, **kwargs)

def post(self, url, data=None, json=None, **kwargs):
return self.request('POST', url, data=data, json=json, **kwargs)
12 changes: 7 additions & 5 deletions plugins/module_utils/foreman_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from functools import wraps

from ansible.module_utils.basic import AnsibleModule, missing_required_lib, env_fallback
from ansible.module_utils._text import to_bytes, to_native
from ansible.module_utils._text import to_native
from ansible.module_utils import six

try:
Expand All @@ -32,9 +32,10 @@
try:
try:
from ansible_collections.theforeman.foreman.plugins.module_utils import _apypie as apypie
from ansible_collections.theforeman.foreman.plugins.module_utils.ansible_requests import RequestSession
except ImportError:
from plugins.module_utils import _apypie as apypie
import requests.exceptions
from plugins.module_utils.ansible_requests import RequestSession
HAS_APYPIE = True
APYPIE_IMP_ERR = None
inflector = apypie.Inflector()
Expand Down Expand Up @@ -608,10 +609,11 @@ def connect(self):

self.foremanapi = apypie.ForemanApi(
uri=self._foremanapi_server_url,
username=to_bytes(self._foremanapi_username),
password=to_bytes(self._foremanapi_password),
Comment on lines -611 to -612
Copy link
Member Author

Choose a reason for hiding this comment

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

this was done as part of #609, but it seems ansible's Request does that for us instead.

username=self._foremanapi_username,
password=self._foremanapi_password,
verify_ssl=self._foremanapi_validate_certs,
task_timeout=self.task_timeout,
session=RequestSession(),
)

_status = self.status()
Expand Down Expand Up @@ -1169,7 +1171,7 @@ def wait_for_task(self, task, ignore_errors=False):

def fail_from_exception(self, exc, msg):
fail = {'msg': msg}
if isinstance(exc, requests.exceptions.HTTPError):
if hasattr(exc, 'response'):
evgeni marked this conversation as resolved.
Show resolved Hide resolved
try:
response = exc.response.json()
if 'error' in response:
Expand Down
1 change: 0 additions & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
requests>=2.4.2
PyYAML
2 changes: 1 addition & 1 deletion tests/vcr_python_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def body_json_l2_matcher(r1, r2):
for i, v in enumerate(body1):
assert body1[i] == body2[i], "body contents at position {} dont't match: '{}' vs '{}'".format(i, body1[i], body2[i]) # pylint:disable=all
else:
assert r1.body == r2.body, "{} != {}".format(r1.body, r2.body)
assert r1.body == r2.body, "{} != {} ({}, {})".format(r1.body, r2.body, r1.headers, r2.headers)


def _query_without_search_matcher(r1, r2, path):
Expand Down
4 changes: 2 additions & 2 deletions vendor.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@
# drop apypie imports (we have one file now) and future imports (they are already present in the header)
elif line.startswith('from apypie') or line.startswith('from __future__'):
continue
# we can't just import requests, Ansible's "import" sanity test fails without the try/except
# drop requests import, we use a different implementation
elif line == 'import requests':
output_lines.extend(['try:', ' import requests', 'except ImportError:', ' pass'])
continue
# drop blocks that only handle typing imports (fenced by either try or if TYPE_CHECKING)
elif line in ['try:', 'if TYPE_CHECKING:'] or buffer_lines:
buffer_lines.append(line)
Expand Down
Loading