Honor falsy config values, and configured OpenStack credentials over the environment - #348
Merged
Merged
Conversation
nuwang
had a problem deploying
to
cloud-integration
September 17, 2026 09:18 — with
GitHub Actions
Failure
nuwang
had a problem deploying
to
cloud-integration
September 17, 2026 10:00 — with
GitHub Actions
Failure
…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.
nuwang
force-pushed
the
config-value-and-credential-precedence
branch
from
September 18, 2026 06:36
0d717b9 to
76ef947
Compare
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two fixes surfaced during review of galaxyproject/galaxy#23098 (thanks @mvdbeek), both in how a provider resolves its configuration.
A config value of
Falseor0is now honored_get_config_valuetestedif self.config.get(key), so every falsy value read as "not configured" and the default was returned instead.s3_validate_certs: False(andec2_validate_certs) silently left certificate verification switched on — the opposite of what the caller asked for, with no error — and a0for any numeric option was quietly swapped for its default.Only
Noneand the empty string now count as unset. That is what an absent value looks like arriving from YAML, a blank environment variable or a blank ini option, and no option has a meaningful empty-string value;Falseand0reach the SDK as configured. The three sources keep their order (config dict, then a config attribute, then the[<provider>]ini section), and the semantics are stated in the docstring and the setup docs.Behaviour change to be aware of: a value that used to be ignored for being falsy now takes effect. A misconfigured
0— saymultipart_max_concurrency: 0— therefore errors where it used to be masked by the default, which I think is the right outcome.Explicitly configured OpenStack credentials take precedence over
OS_*The provider filled in every credential field from the environment whenever the config didn't name it, and
_keystone_sessionprefers password authentication when both a password and an application credential are present. So a provider configured with only an application credential, running in a process that carriedOS_USERNAME/OS_PASSWORD, authenticated as that ambient identity rather than the credential it was given. In Galaxy that meant a user-defined store could act as the server.The two credential sets are now resolved separately: the environment only completes the set the config names (a configured
os_usernamewith the password inOS_PASSWORDstill works), and is consulted for both sets only when neither is configured — the existing behaviour for the env-only case. The precedence between two configured sets is unchanged.Tests
tests/test_cloud_helpers.py:Falseand0are returned as configured;Noneand''fall back to the default.tests/test_openstack_credentials.py(new): configured app credential ignores ambient password creds and yields av3.ApplicationCredentialauth; configured password creds ignore ambient app credential; env-only still works; the environment completes a partially configured set and only that set. Offline: the Keystone version probe is patched and keystoneauth plugins are inert until used.Docs:
setup.rstnow states the lookup order and unset semantics, listsos_application_credential_id/_secretin the OpenStack table (they were missing, and username/password were marked required), and notes the per-set environment rule. Changelog entry under4.4.2 - unreleased.