Skip to content

Conversation

@ching-kuo
Copy link
Contributor

users should be able to use OS_USER_DOMAIN_NAME, OS_USER_DOMAIN_ID or
OS_DEFAULT_DOMAIN to specify the DOMAIN of user.

Fixes: #2418

Reference:
https://docs.openstack.org/python-openstackclient/latest/cli/authentication.html#authenticating-using-identity-server-api-v3

users should be able to use OS_USER_DOMAIN_NAME, OS_USER_DOMAIN_ID or
OS_DEFAULT_DOMAIN to specify the DOMAIN of user.

Fixes: gophercloud#2418
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 79.953% when pulling 2c86b0c on ching-kuo:issue_2418 into 3ce7b72 on gophercloud:master.

if username != "" && domainID == "" && domainName == "" {
return nilOptions, gophercloud.ErrMissingAnyoneOfEnvironmentVariables{
EnvironmentVariables: []string{"OS_DOMAIN_ID", "OS_DOMAIN_NAME"},
if userDomainID == "" && userDomainName == "" && defaultDomain == "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look right. The new code only runs when using application credentials with missing OS_APPLICATION_CREDENTIAL_ID, which may not be what we want.

As far as I understand from https://docs.openstack.org/python-openstackclient/latest/cli/authentication.html#authenticating-using-identity-server-api-v3, the logic should be outside of the appcreds conditional branch, but I'm clearly not an expert on keystone authentication so I could very well be totally wrong 😄

Would love to have more eyes on this patch.

@majewsky
Copy link
Contributor

majewsky commented Jul 1, 2022

My understanding is that AuthOptionsFromEnv is frozen to preserve backwards-compatibility. You should use clientconfig.AuthOptions instead.

@pierreprinetti pierreprinetti added the needinfo Additional information requested label Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needinfo Additional information requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adding Support for User Domain Name/ID and Default Domain

5 participants