Преглед изворни кода

Honor falsy config values, and configured OpenStack credentials over the environment

_get_config_value treated every falsy value as "not configured", so
s3_validate_certs: False (and ec2_validate_certs) silently left
certificate verification switched on, and a 0 for any numeric option was
quietly swapped for its default. Only None and the empty string now
count as unset - what an absent value looks like coming from YAML, a
blank environment variable or a blank ini option - so False and 0 reach
the SDK as configured. A misconfigured 0 therefore errors where it used
to be masked.

The OpenStack provider filled in every credential field from the OS_*
environment whenever the config did not name it, and Keystone password
authentication won when both a password and an application credential
were present. A provider configured with only an application
credential, running in a process that carried OS_USERNAME and
OS_PASSWORD, therefore authenticated as that ambient identity rather
than the credential it was given - in Galaxy, a user-defined store
could end up acting as the server. Resolve the two credential sets
separately: the environment completes only the set the config names,
so a configured os_username with the password in OS_PASSWORD still
works, and is consulted for both sets only when neither is configured.

Both surfaced in review of galaxyproject/galaxy#23098.
Nuwan Goonasekera пре 6 дана
родитељ
комит
76ef947c74

+ 21 - 0
CHANGELOG.rst

@@ -22,6 +22,27 @@
   ``DuplicateResourceException``, so a retry never appends a second copy;
   and ``remove_metadata_item`` returns ``False`` when there was nothing to
   remove, as its callers always assumed.
+* **A config value of ``False`` or ``0`` is now honored instead of being
+  replaced by the default.** ``_get_config_value`` treated every falsy value
+  as "not configured", so ``s3_validate_certs: False`` (and
+  ``ec2_validate_certs``) silently left certificate verification switched on,
+  and a ``0`` for any numeric option was quietly swapped for its default.
+  Only ``None`` and the empty string now count as unset - what an absent
+  value looks like coming from YAML, a blank environment variable or a blank
+  ini option. A value that was previously ignored for being falsy now takes
+  effect; a misconfigured ``0`` (say for ``multipart_max_concurrency``)
+  therefore errors where it used to be masked.
+* **Explicitly configured OpenStack credentials take precedence over the
+  ``OS_*`` environment.** The provider filled in every credential field from
+  the environment whenever the config did not name it, and Keystone password
+  authentication was preferred when both a password and an application
+  credential were present. So a provider configured with only an application
+  credential, running in a process that carried ``OS_USERNAME`` and
+  ``OS_PASSWORD``, authenticated as that ambient identity rather than the
+  credential it was given. The two credential sets are now resolved
+  separately: the environment only completes the set the config names (a
+  configured ``os_username`` with the password in ``OS_PASSWORD`` still
+  works), and is consulted for both only when neither is configured.
 
 4.4.1 - August 21, 2026 (sha 093ef669598d9f324be28d400a851396739cf1d8)
 ----------------------------------------------------------------------

+ 18 - 8
cloudbridge/base/provider.py

@@ -193,6 +193,13 @@ class BaseCloudProvider(CloudProvider):
         """
         A convenience method to extract a configuration value.
 
+        The config dict is consulted first, then an attribute of the same
+        name on the config object, then the ``[<provider id>]`` section of
+        the cloudbridge ini file. A value is taken from the first source
+        that has it set; only ``None`` and the empty string count as unset,
+        so ``False`` and ``0`` are returned as configured rather than
+        replaced by the default.
+
         :type key: str
         :param key: a field to look for in the ``self.config`` field
 
@@ -204,12 +211,15 @@ class BaseCloudProvider(CloudProvider):
         """
         log.debug("Getting config key %s, with supplied default value: %s",
                   key, default_value)
-        value = default_value
-        if isinstance(self.config, dict) and self.config.get(key):
-            value = self.config.get(key, default_value)
-        elif hasattr(self.config, key) and getattr(self.config, key):
-            value = getattr(self.config, key)
-        elif (self._config_parser.has_option(self.PROVIDER_ID, key) and
-              self._config_parser.get(self.PROVIDER_ID, key)):
+        value = self.config.get(key) if isinstance(self.config, dict) else None
+        if not self._is_set(value):
+            value = getattr(self.config, key, None)
+        if (not self._is_set(value) and
+                self._config_parser.has_option(self.PROVIDER_ID, key)):
             value = self._config_parser.get(self.PROVIDER_ID, key)
-        return value
+        return value if self._is_set(value) else default_value
+
+    @staticmethod
+    def _is_set(value: Any) -> bool:
+        """Whether a config value was supplied, as opposed to left blank."""
+        return value is not None and value != ''

+ 23 - 8
cloudbridge/providers/openstack/provider.py

@@ -44,14 +44,29 @@ class OpenStackCloudProvider(BaseCloudProvider):
         super(OpenStackCloudProvider, self).__init__(config)
 
         # Initialize cloud connection fields
-        self.app_cred_id = self._get_config_value(
-            'os_application_credential_id', get_env('OS_APPLICATION_CREDENTIAL_ID'))
-        self.app_cred_secret = self._get_config_value(
-            'os_application_credential_secret', get_env('OS_APPLICATION_CREDENTIAL_SECRET'))
-        self.username = self._get_config_value(
-            'os_username', get_env('OS_USERNAME'))
-        self.password = self._get_config_value(
-            'os_password', get_env('OS_PASSWORD'))
+        # Credentials come from the config first and the OS_* environment
+        # second. The two credential sets are resolved separately: the
+        # environment only completes the set the config names, so a provider
+        # configured with an application credential never picks up the
+        # process's own OS_USERNAME/OS_PASSWORD and signs in as that identity
+        # instead (and vice versa). With neither set configured, both are read
+        # from the environment as before.
+        app_cred_id = self._get_config_value('os_application_credential_id')
+        app_cred_secret = self._get_config_value(
+            'os_application_credential_secret')
+        username = self._get_config_value('os_username')
+        password = self._get_config_value('os_password')
+        env_app_cred = not (username or password)
+        env_password = not (app_cred_id or app_cred_secret)
+        self.app_cred_id = app_cred_id or (
+            get_env('OS_APPLICATION_CREDENTIAL_ID') if env_app_cred else None)
+        self.app_cred_secret = app_cred_secret or (
+            get_env('OS_APPLICATION_CREDENTIAL_SECRET') if env_app_cred
+            else None)
+        self.username = username or (
+            get_env('OS_USERNAME') if env_password else None)
+        self.password = password or (
+            get_env('OS_PASSWORD') if env_password else None)
         self.project_name = self._get_config_value(
             'os_project_name', get_env('OS_PROJECT_NAME')
             or get_env('OS_TENANT_NAME'))

+ 66 - 49
docs/topics/setup.rst

@@ -16,8 +16,12 @@ available credentials in one of following ways:
 Providing access credentials through a dictionary
 -------------------------------------------------
 You can initialize a simple config as follows. The key names are the same
-as the environment variables, in lower case. Note that the config dictionary
-will override environment values.
+as the environment variables, in lower case. A value is looked up in the
+config dictionary first, then in the `CloudBridge config file`_, and only
+then in the environment; ``None`` and the empty string count as not set, so
+``False`` and ``0`` are honored as configured.
+
+.. _CloudBridge config file: #providing-access-credentials-in-a-cloudbridge-config-file
 
 .. code-block:: python
 
@@ -174,53 +178,60 @@ GCP
 OpenStack
 ~~~~~~~~~
 
-+-------------------------+--------------------------------------------------------------+
-| Variable                | Description                                                  |
-+=========================+==============================================================+
-| os_auth_url             | Required. OpenStack authentication endpoint.                 |
-|                         | eg: https://my-openstack.com:5000/v3                         |
-+-------------------------+--------------------------------------------------------------+
-| os_username             | Required. Username for authentication.                       |
-+-------------------------+--------------------------------------------------------------+
-| os_password             | Required. password for authentication.                       |
-+-------------------------+--------------------------------------------------------------+
-| os_project_name         | Required. The project in which to manage resources.          |
-+-------------------------+--------------------------------------------------------------+
-| os_region_name          | Required. Region in which to manage resources.               |
-+-------------------------+--------------------------------------------------------------+
-| os_zone_name            | Default Availability Zone in which to manage resources.      |
-|                         | If not provided, will default to the first available zone    |
-|                         | in the region. This zone will be the default for all services|
-|                         | unless overwritten by service-specific zone configs          |
-+-------------------------+--------------------------------------------------------------+
-| os_compute_zone_name    | Default Availability Zone for Compute servies.               |
-|                         | If not provided, will default to `os_zone_name`              |
-+-------------------------+--------------------------------------------------------------+
-| os_networking_zone_name | Default Availability Zone for Networking servies.            |
-|                         | If not provided, will default to `os_zone_name`              |
-+-------------------------+--------------------------------------------------------------+
-| os_security_zone_name   | Default Availability Zone for Security servies.              |
-|                         | If not provided, will default to `os_zone_name`              |
-+-------------------------+--------------------------------------------------------------+
-| os_storage_zone_name    | Default Availability Zone for Storage servies.               |
-|                         | If not provided, will default to `os_zone_name`              |
-+-------------------------+--------------------------------------------------------------+
-| nova_service_name       | Service name for the NOVA client.                            |
-+-------------------------+--------------------------------------------------------------+
-| os_auth_token           | Authentication token, if applicable.                         |
-+-------------------------+--------------------------------------------------------------+
-| os_compute_api_version  | Compute API version, if applicable.                          |
-+-------------------------+--------------------------------------------------------------+
-| os_volume_api_version   | Volume API version, if applicable.                           |
-+-------------------------+--------------------------------------------------------------+
-| os_storage_url          | Storage endpoint URL, if applicable                          |
-+-------------------------+--------------------------------------------------------------+
-| os_project_domain_id    | Project domain id for authentication.                        |
-+-------------------------+--------------------------------------------------------------+
-| os_project_domain_name  | Project domain name for authentication.                      |
-+-------------------------+--------------------------------------------------------------+
-| os_user_domain_name     | User domain name for authentication.                         |
-+-------------------------+--------------------------------------------------------------+
++----------------------------------+--------------------------------------------------------------+
+| Variable                         | Description                                                  |
++==================================+==============================================================+
+| os_auth_url                      | Required. OpenStack authentication endpoint.                 |
+|                                  | eg: https://my-openstack.com:5000/v3                         |
++----------------------------------+--------------------------------------------------------------+
+| os_username                      | Username for password authentication. Required unless an     |
+|                                  | application credential is given.                             |
++----------------------------------+--------------------------------------------------------------+
+| os_password                      | Password for password authentication. Required unless an     |
+|                                  | application credential is given.                             |
++----------------------------------+--------------------------------------------------------------+
+| os_application_credential_id     | Keystone application credential ID. Together with the secret,|
+|                                  | an alternative to a username and password.                   |
++----------------------------------+--------------------------------------------------------------+
+| os_application_credential_secret | Keystone application credential secret.                      |
++----------------------------------+--------------------------------------------------------------+
+| os_project_name                  | Required. The project in which to manage resources.          |
++----------------------------------+--------------------------------------------------------------+
+| os_region_name                   | Required. Region in which to manage resources.               |
++----------------------------------+--------------------------------------------------------------+
+| os_zone_name                     | Default Availability Zone in which to manage resources.      |
+|                                  | If not provided, will default to the first available zone    |
+|                                  | in the region. This zone will be the default for all services|
+|                                  | unless overwritten by service-specific zone configs          |
++----------------------------------+--------------------------------------------------------------+
+| os_compute_zone_name             | Default Availability Zone for Compute servies.               |
+|                                  | If not provided, will default to `os_zone_name`              |
++----------------------------------+--------------------------------------------------------------+
+| os_networking_zone_nam         e | Default Availability Zone for Networking servies.            |
+|                                  | If not provided, will default to `os_zone_name`              |
++----------------------------------+--------------------------------------------------------------+
+| os_security_zone_name            | Default Availability Zone for Security servies.              |
+|                                  | If not provided, will default to `os_zone_name`              |
++----------------------------------+--------------------------------------------------------------+
+| os_storage_zone_name             | Default Availability Zone for Storage servies.               |
+|                                  | If not provided, will default to `os_zone_name`              |
++----------------------------------+--------------------------------------------------------------+
+| nova_service_name                | Service name for the NOVA client.                            |
++----------------------------------+--------------------------------------------------------------+
+| os_auth_token                    | Authentication token, if applicable.                         |
++----------------------------------+--------------------------------------------------------------+
+| os_compute_api_version           | Compute API version, if applicable.                          |
++----------------------------------+--------------------------------------------------------------+
+| os_volume_api_version            | Volume API version, if applicable.                           |
++----------------------------------+--------------------------------------------------------------+
+| os_storage_url                   | Storage endpoint URL, if applicable                          |
++----------------------------------+--------------------------------------------------------------+
+| os_project_domain_id             | Project domain id for authentication.                        |
++----------------------------------+--------------------------------------------------------------+
+| os_project_domain_name           | Project domain name for authentication.                      |
++----------------------------------+--------------------------------------------------------------+
+| os_user_domain_name              | User domain name for authentication.                         |
++----------------------------------+--------------------------------------------------------------+
 
 Providing access credentials through environment variables
 ----------------------------------------------------------
@@ -345,6 +356,12 @@ OpenStack
 | OS_USER_DOMAIN_NAME              |           |
 +----------------------------------+-----------+
 
+``OS_USERNAME``/``OS_PASSWORD`` and ``OS_APPLICATION_CREDENTIAL_ID``/
+``OS_APPLICATION_CREDENTIAL_SECRET`` are alternatives. When a config
+dictionary names either set, the environment only completes that set: a
+provider configured with an application credential does not pick up an
+``OS_USERNAME`` and ``OS_PASSWORD`` that happen to be in its environment.
+
 Once the environment variables are set, you can create a connection as follows,
 replacing ``ProviderList.AWS`` with the desired provider (AZURE, GCP, or
 OPENSTACK):

+ 26 - 0
tests/test_cloud_helpers.py

@@ -98,3 +98,29 @@ class CloudHelpersTestCase(ProviderTestBase):
         int_value = self.provider._get_config_value(
             'default_result_limit', None)
         self.assertIsInstance(int_value, int)
+
+    def test_config_value_set_to_false_is_honored(self):
+        # A boolean option turned off must not be mistaken for an unset one:
+        # `s3_validate_certs: False` has to reach the SDK as False, not as
+        # the default of True.
+        self.provider.config['falsy_bool_check'] = False
+        # pylint:disable=protected-access
+        self.assertIs(
+            self.provider._get_config_value('falsy_bool_check', True), False)
+
+    def test_config_value_set_to_zero_is_honored(self):
+        self.provider.config['falsy_int_check'] = 0
+        # pylint:disable=protected-access
+        self.assertEqual(
+            self.provider._get_config_value('falsy_int_check', 4), 0)
+
+    def test_config_value_none_or_blank_falls_back_to_default(self):
+        # None and the empty string are what an absent value looks like
+        # coming from YAML, a blank environment variable or a blank ini
+        # option, so those alone mean "not configured".
+        for unset in (None, ''):
+            self.provider.config['unset_check'] = unset
+            # pylint:disable=protected-access
+            self.assertEqual(
+                self.provider._get_config_value('unset_check', 'default'),
+                'default', repr(unset))

+ 109 - 0
tests/test_openstack_credentials.py

@@ -0,0 +1,109 @@
+"""Which OpenStack credentials a provider authenticates with.
+
+The provider reads credentials from its config dict and, failing that, from
+the ``OS_*`` environment. These tests pin the precedence between the two:
+credentials configured explicitly win over whatever happens to be in the
+process environment, so a provider built for one identity never signs in as
+another. Nothing here touches a network: the Keystone version probe is
+patched and keystoneauth plugins are plain objects until used.
+"""
+
+import os
+import unittest
+from unittest import mock
+
+from keystoneauth1.identity import v3
+
+from cloudbridge.providers.openstack.provider import OpenStackCloudProvider
+
+AUTH_URL = 'https://keystone.example.org:5000/v3'
+
+PASSWORD_ENV = {
+    'OS_USERNAME': 'ambient-user',
+    'OS_PASSWORD': 'ambient-password',
+    'OS_PROJECT_NAME': 'ambient-project',
+}
+
+APP_CRED_ENV = {
+    'OS_APPLICATION_CREDENTIAL_ID': 'ambient-app-cred-id',
+    'OS_APPLICATION_CREDENTIAL_SECRET': 'ambient-app-cred-secret',
+}
+
+ALL_CREDENTIAL_VARS = tuple(PASSWORD_ENV) + tuple(APP_CRED_ENV)
+
+
+def _environment(**values):
+    """The process environment with only the given OS_* credentials set."""
+    env = {k: v for k, v in os.environ.items() if k not in ALL_CREDENTIAL_VARS}
+    env.update(values)
+    return mock.patch.dict(os.environ, env, clear=True)
+
+
+def _provider(**config):
+    # A configured zone keeps the compute service from asking Nova for one
+    # while the provider is being built.
+    return OpenStackCloudProvider(
+        dict(config, os_auth_url=AUTH_URL, os_zone_name='nova'))
+
+
+def _keystone_auth(provider):
+    with mock.patch.object(OpenStackCloudProvider, '_keystone_version',
+                           new_callable=mock.PropertyMock, return_value=3):
+        # pylint:disable=protected-access
+        return provider._keystone_session.auth
+
+
+class OpenStackCredentialPrecedenceTestCase(unittest.TestCase):
+
+    def test_configured_application_credential_ignores_ambient_password(self):
+        # The process may carry the server's own OS_USERNAME/OS_PASSWORD; a
+        # provider configured with an application credential must use that
+        # credential, not the ambient identity.
+        with _environment(**PASSWORD_ENV):
+            provider = _provider(
+                os_application_credential_id='configured-id',
+                os_application_credential_secret='configured-secret')
+            self.assertIsNone(provider.username)
+            self.assertIsNone(provider.password)
+            auth = _keystone_auth(provider)
+        self.assertIsInstance(auth, v3.ApplicationCredential)
+        self.assertEqual(auth.auth_methods[0].application_credential_id,
+                         'configured-id')
+
+    def test_configured_password_ignores_ambient_application_credential(self):
+        with _environment(**APP_CRED_ENV):
+            provider = _provider(os_username='configured-user',
+                                 os_password='configured-password',
+                                 os_project_name='configured-project')
+            self.assertIsNone(provider.app_cred_id)
+            self.assertIsNone(provider.app_cred_secret)
+            auth = _keystone_auth(provider)
+        self.assertIsInstance(auth, v3.Password)
+        self.assertEqual(auth.auth_methods[0].username, 'configured-user')
+
+    def test_environment_is_used_when_nothing_is_configured(self):
+        with _environment(**PASSWORD_ENV):
+            provider = _provider()
+            self.assertEqual(provider.username, 'ambient-user')
+            self.assertEqual(provider.password, 'ambient-password')
+            auth = _keystone_auth(provider)
+        self.assertIsInstance(auth, v3.Password)
+
+    def test_environment_completes_a_partially_configured_credential(self):
+        # Keeping the secret out of the config file and in the environment is
+        # legitimate: the environment fills in the missing half of the set
+        # that is configured, and only that set.
+        with _environment(**PASSWORD_ENV, **APP_CRED_ENV):
+            provider = _provider(os_username='configured-user')
+            self.assertEqual(provider.username, 'configured-user')
+            self.assertEqual(provider.password, 'ambient-password')
+            self.assertIsNone(provider.app_cred_id)
+            self.assertIsNone(provider.app_cred_secret)
+
+        with _environment(**PASSWORD_ENV, **APP_CRED_ENV):
+            provider = _provider(os_application_credential_id='configured-id')
+            self.assertEqual(provider.app_cred_id, 'configured-id')
+            self.assertEqual(provider.app_cred_secret,
+                             'ambient-app-cred-secret')
+            self.assertIsNone(provider.username)
+            self.assertIsNone(provider.password)