openstack/ironic-python-agent
Code Issues Proposed changes

Merge "implement basic-auth support for user-image download process"

Browse Source
This commit is contained in:
Zuul 2023-10-13 17:08:28 +00:00 committed by Gerrit Code Review
commit b42f0be422
4 changed files with 339 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
ironic_python_agent/config.py
View File

@ -262,6 +262,31 @@ cli_opts = [
help='Pre-shared token to use when working with the '
'ironic API. This value is typically supplied by '
'ironic automatically.'),
cfg.StrOpt('image_server_user',
default=None,
help='Pre-shared username used in Basic Auth process '
'against the http(s) server that hosts the disk image.'
'This variable can be also configured via image_info.'
'Value coming from image_info takes precedence over'
'value coming from command line or configuration file.'),
cfg.StrOpt('image_server_password',
default=None,
help='Pre-shared password used in Basic Auth process '
'against the http(s) server that hosts the disk image.'
'This variable can be also configured via image_info.'
'Value coming from image_info takes precedence over'
'value coming from command line or configuration file.'),
cfg.StrOpt('image_server_auth_strategy',
default="noauth",
help='Option to select authentication strategy used during'
'communication with the server that hosts the disk images'
'and related checksums. This option also turns'
'image_server_password and image_server_user'
'into mandatory variables for those authentication'
'strategies that require username + password credentials.'
'This variable can be also configured via image_info.'
'Value coming from image_info takes precedence over'
'value coming from command line or configuration file.'),
cfg.BoolOpt('agent_token_required',
default=APARAMS.get('ipa-agent-token-required', False),
help='Control to enforce if API command requests should '

110
ironic_python_agent/extensions/standby.py
View File

@ -47,6 +47,93 @@ def _image_location(image_info):
return os.path.join(tempfile.gettempdir(), image_info['id'])
def _verify_basic_auth_creds(user, password, image_id):
"""Verify the basic auth credentials used for image download are present.
:param user: Basic auth username
:param password: Basic auth password
:param image_id: id of the image that is being acted upon
:raises ImageDownloadError if the credentials are not present
"""
expected_creds = {'user': user, 'password': password}
missing_creds = []
for key, value in expected_creds.items():
if not value:
missing_creds.append(key)
if missing_creds:
raise errors.ImageDownloadError(
image_id,
"Missing {} fields from HTTP(S) "
"basic auth config".format(missing_creds)
)
def _gen_auth_from_image_info_user_pass(image_info, image_id):
"""This function is used to pass the credentials to the chosen
credential verifier and in case the verification is successful
generate the compatible authentication object that will be used
with the request(s). This function handles the authentication object
generation for authentication strategies that are username+password
based. Credentials are collected via image_info.
:param image_info: Image information dictionary.
:param image_id: id of the image that is being acted upon
:return: Authentication object used directly by the request library
:rtype: requests.auth.HTTPBasicAuth
"""
image_server_user = None
image_server_password = None
if image_info.get('image_server_auth_strategy') == 'http_basic':
image_server_user = image_info.get('image_server_user')
image_server_password = image_info.get('image_server_password')
_verify_basic_auth_creds(
image_server_user,
image_server_password,
image_id
)
else:
return None
return requests.auth.HTTPBasicAuth(image_server_user,
image_server_password)
def _gen_auth_from_oslo_conf_user_pass(image_id):
"""This function is used to pass the credentials to the chosen
credential verifier and in case the verification is successful
generate the compatible authentication object that will be used
with the request(s). This function handles the authentication object
generation for authentication strategies that are username+password
based. Credentials are collected from the oslo.config framework.
:param image_id: id of the image that is being acted upon
:return: Authentication object used directly by the request library
:rtype: requests.auth.HTTPBasicAuth
"""
image_server_user = None
image_server_password = None
if CONF.image_server_auth_strategy == 'http_basic':
_verify_basic_auth_creds(
CONF.image_server_user,
CONF.image_server_password,
image_id)
image_server_user = CONF.image_server_user
image_server_password = CONF.image_server_password
else:
return None
return requests.auth.HTTPBasicAuth(image_server_user,
image_server_password)
def _download_with_proxy(image_info, url, image_id):
"""Opens a download stream for the given URL.
@ -56,13 +143,31 @@ def _download_with_proxy(image_info, url, image_id):
:raises: ImageDownloadError if the download stream was not started
properly.
:return: HTTP(s) server response for the image/hash download request
:rtype: requests.Response
"""
no_proxy = image_info.get('no_proxy')
if no_proxy:
os.environ['no_proxy'] = no_proxy
proxies = image_info.get('proxies', {})
verify, cert = utils.get_ssl_client_options(CONF)
resp = None
image_download_attributes = {
"stream": True,
"proxies": proxies,
"verify": verify,
"cert": cert,
"timeout": CONF.image_download_connection_timeout
}
# NOTE(Adam) `image_info` is prioritized over `oslo.conf` for credential
# collection and auth strategy selection
auth_object = _gen_auth_from_image_info_user_pass(image_info, image_id)
if auth_object is None:
auth_object = _gen_auth_from_oslo_conf_user_pass(image_id)
if auth_object is not None:
image_download_attributes['auth'] = auth_object
for attempt in range(CONF.image_download_connection_retries + 1):
try:
# NOTE(TheJulia) The get request below does the following:
@ -78,9 +183,8 @@ def _download_with_proxy(image_info, url, image_id):
# exactly just as the timeout value exists. The risk in transitory
# failure is more so once we've started the download and we are
# processing the incoming data.
resp = requests.get(url, stream=True, proxies=proxies,
verify=verify, cert=cert,
timeout=CONF.image_download_connection_timeout)
# B113 issue is covered is the image_download_attributs list
resp = requests.get(url, **image_download_attributes) # nosec
if resp.status_code != 200:
msg = ('Received status code {} from {}, expected 200. '
'Response body: {} Response headers: {}').format(

180
ironic_python_agent/tests/unit/extensions/test_standby.py
View File

@ -204,6 +204,86 @@ class TestStandbyExtension(base.IronicAgentTest):
expected_loc = os.path.join(tempfile.gettempdir(), 'fake_id')
self.assertEqual(expected_loc, location)
def test_verify_basic_auth_creds(self):
image_info = _build_fake_image_info()
self.assertIsNone(standby._verify_basic_auth_creds("SpongeBob",
"SquarePants",
image_info['id']))
def test_gen_auth_from_image_info_user_pass_success(self):
image_info = _build_fake_image_info()
image_info['image_server_auth_strategy'] = 'http_basic'
image_info['image_server_user'] = 'SpongeBob'
image_info['image_server_password'] = 'SquarePants'
exp_auth = requests.auth.HTTPBasicAuth('SpongeBob', 'SquarePants')
return_auth = \
standby._gen_auth_from_image_info_user_pass(image_info,
image_info['id'])
self.assertEqual(exp_auth, return_auth)
def test_gen_auth_from_image_info_user_pass_none(self):
image_info = _build_fake_image_info()
image_info['image_server_auth_strategy'] = ''
image_info['image_server_user'] = 'SpongeBob'
image_info['image_server_password'] = 'SquarePants'
return_auth = \
standby._gen_auth_from_image_info_user_pass(image_info,
image_info['id'])
self.assertIsNone(return_auth)
def test_gen_auth_from_oslo_conf_user_pass_success(self):
image_info = _build_fake_image_info()
CONF.set_override('image_server_auth_strategy', 'http_basic')
CONF.set_override('image_server_password', 'SpongeBob')
CONF.set_override('image_server_user', 'SquarePants')
correct_auth = \
requests.auth.HTTPBasicAuth(CONF['image_server_user'],
CONF['image_server_password'])
return_auth = \
standby._gen_auth_from_oslo_conf_user_pass(image_info['id'])
self.assertEqual(correct_auth, return_auth)
def test_gen_auth_from_oslo_conf_user_pass_none(self):
image_info = _build_fake_image_info()
CONF.set_override('image_server_auth_strategy', 'noauth')
CONF.set_override('image_server_password', 'SpongeBob')
CONF.set_override('image_server_user', 'SquarePants')
return_auth = \
standby._gen_auth_from_oslo_conf_user_pass(image_info['id'])
self.assertIsNone(return_auth)
def test_verify_basic_auth_creds_empty_user(self):
image_info = _build_fake_image_info()
self.assertRaises(errors.ImageDownloadError,
standby._verify_basic_auth_creds,
"",
"SquarePants",
image_info['id'])
def test_verify_basic_auth_creds_empty_password(self):
image_info = _build_fake_image_info()
self.assertRaises(errors.ImageDownloadError,
standby._verify_basic_auth_creds,
"SpongeBob",
"",
image_info['id'])
def test_verify_basic_auth_creds_none_user(self):
image_info = _build_fake_image_info()
self.assertRaises(errors.ImageDownloadError,
standby._verify_basic_auth_creds,
None,
"SquarePants",
image_info['id'])
def test_verify_basic_auth_creds_none_password(self):
image_info = _build_fake_image_info()
self.assertRaises(errors.ImageDownloadError,
standby._verify_basic_auth_creds,
"SpongeBob",
None,
image_info['id'])
@mock.patch('ironic_lib.disk_utils.fix_gpt_partition', autospec=True)
@mock.patch('ironic_lib.disk_utils.trigger_device_rescan', autospec=True)
@mock.patch('ironic_lib.disk_utils.convert_image', autospec=True)
@ -504,6 +584,106 @@ class TestStandbyExtension(base.IronicAgentTest):
standby._download_image,
image_info)
@mock.patch('hashlib.new', autospec=True)
@mock.patch('builtins.open', autospec=True)
@mock.patch('requests.get', autospec=True)
def test_download_image_basic_auth_conf_success(self, requests_mock,
open_mock, hash_mock):
image_info = _build_fake_image_info()
CONF.set_override('image_server_auth_strategy', 'http_basic')
CONF.set_override('image_server_password', 'SpongeBob')
CONF.set_override('image_server_user', 'SquarePants')
user = CONF.image_server_user
password = CONF.image_server_password
correct_auth = requests.auth.HTTPBasicAuth(user, password)
response = requests_mock.return_value
response.status_code = 200
response.iter_content.return_value = ['some', 'content']
file_mock = mock.Mock()
open_mock.return_value.__enter__.return_value = file_mock
file_mock.read.return_value = None
hexdigest_mock = hash_mock.return_value.hexdigest
hexdigest_mock.return_value = image_info['os_hash_value']
standby._download_image(image_info)
requests_mock.assert_called_once_with(image_info['urls'][0],
cert=None, verify=True,
stream=True, proxies={},
timeout=60, auth=correct_auth)
write = file_mock.write
write.assert_any_call('some')
write.assert_any_call('content')
self.assertEqual(2, write.call_count)
@mock.patch('hashlib.new', autospec=True)
@mock.patch('builtins.open', autospec=True)
@mock.patch('requests.get', autospec=True)
def test_download_image_basic_auth_image_info_success(self,
requests_mock,
open_mock,
hash_mock):
image_info = _build_fake_image_info()
image_info['image_server_auth_strategy'] = 'http_basic'
image_info['image_server_password'] = 'SpongeBob'
image_info['image_server_user'] = 'SquarePants'
user = image_info['image_server_user']
password = image_info['image_server_password']
correct_auth = requests.auth.HTTPBasicAuth(user, password)
response = requests_mock.return_value
response.status_code = 200
response.iter_content.return_value = ['some', 'content']
file_mock = mock.Mock()
open_mock.return_value.__enter__.return_value = file_mock
file_mock.read.return_value = None
hexdigest_mock = hash_mock.return_value.hexdigest
hexdigest_mock.return_value = image_info['os_hash_value']
standby._download_image(image_info)
requests_mock.assert_called_once_with(image_info['urls'][0],
cert=None, verify=True,
stream=True, proxies={},
timeout=60, auth=correct_auth)
write = file_mock.write
write.assert_any_call('some')
write.assert_any_call('content')
self.assertEqual(2, write.call_count)
def test_download_image_bad_basic_auth_conf_credential(self):
self.config(image_download_connection_retry_interval=0)
image_info = _build_fake_image_info()
CONF.set_override('image_server_auth_strategy', 'http_basic')
self.assertRaises(errors.ImageDownloadError,
standby._download_image,
image_info)
def test_download_image_bad_basic_auth_image_info_credential(self):
self.config(image_download_connection_retry_interval=0)
image_info = _build_fake_image_info()
image_info['image_server_auth_strategy'] = 'http_basic'
self.assertRaises(errors.ImageDownloadError,
standby._download_image,
image_info)
def test_download_image_bad_basic_auth_mixed_credential(self):
self.config(image_download_connection_retry_interval=0)
image_info = _build_fake_image_info()
image_info['image_server_auth_strategy'] = 'http_basic'
CONF.set_override('image_server_password', 'SpongeBob')
CONF.set_override('image_server_user', 'SquarePants')
self.assertRaises(errors.ImageDownloadError,
standby._download_image,
image_info)
def test_download_image_bad_basic_auth_mixed_credential_second(self):
self.config(image_download_connection_retry_interval=0)
image_info = _build_fake_image_info()
CONF.set_override('image_server_auth_strategy', 'http_basic')
image_info['image_server_password'] = 'SpongeBob'
image_info['image_server_user'] = 'SquarePants'
self.assertRaises(errors.ImageDownloadError,
standby._download_image,
image_info)
@mock.patch('hashlib.new', autospec=True)
@mock.patch('builtins.open', autospec=True)
@mock.patch('requests.get', autospec=True)

27
releasenotes/notes/basic-auth-for-user-image-server-150835e7567444da.yaml Normal file
View File

@ -0,0 +1,27 @@
---
features:
- |
Introducing basic authentication and configurable authentication strategy
support for image and image checksum download processes. This feature
introduces 3 new variables that could be set (either via oslo.config or
image_info) to select the authentication strategy an provide credentials
for HTTP(S) basic authentication. The 3 variables are structured in way
that 1 of them 'image_server_auth_strategy' (string) provides the ability
to select between authentication strategies by specifying the name of
the strategy. Currently the only supported authentication strategy is the
'http-basic' which will make IPA use HTTP(S) basic authentication also
known as the 'RFC 7617' standard. The other 2 variables
'image_server_password' and 'image_server_user' provide username and
password credentials for image download processes. The
'image_server_password' and 'image_server_user' are not strategy specific
and could be reused for any username + password based authentication
strategy, but for the moment these 2 variables are only used for the
'http-basic' strategy. 'image_server_basic_auth' not just enables the
feature but enforces checks on the values of the 2 related credentials.
When the 'http-basic' strategy is enabled for image server download
workflow the download logic will make sure to raise an exception in case
any of the credentials are None or an empty string. Values coming from
'image_info' are prioritized over values coming from the 'oslo.config'
framework and the 2 different credential source can't be mixed. Passing 1
or 2 out of the 3 from and source and the remaining values from an other
source will result in a exception.