gophercloud/utils

clientconfig.NewServiceClient : OS_CLOUD overrides clientOpts.Cloud

az-blip opened this issue ยท 16 comments

Hi,

I found out that in the clientconfig.GetCloudFromYAML method, the environment value of OS_CLOUD always overrides the specified value in clientOpts.Cloud.

// Determine which cloud to use.
// First see if a cloud name was explicitly set in opts.
var cloudName string
if opts != nil && opts.Cloud != "" {
cloudName = opts.Cloud
}
// Next see if a cloud name was specified as an environment variable.
// This is supposed to override an explicit opts setting.
envPrefix := "OS_"
if opts.EnvPrefix != "" {
envPrefix = opts.EnvPrefix
}
if v := env.Getenv(envPrefix + "CLOUD"); v != "" {
cloudName = v
}

It seems the wrong priority to me, as the value defined in my program is more precise and could even be changed by the user (command line arg for example), while the environnement variable is more like a default value.
Could it be possible to modify this behavior ?

Alain Z.

Yes, I also wasted half a day and half a head of hair before working this one out.

This would make a tip-top breaking change for 1.0

Here's a way to make Gophercloud ignore environment variables when parsing clouds.yaml:

cloud, err := clientconfig.GetCloudFromYAML(&clientconfig.ClientOpts{
	Cloud:     cloudName,
	EnvPrefix: "NO_OPENSTACK_ENV_VARIABLES_",
})

Not what we want, but what we need.

In the meantime, shall we create a similar function that does NOT take into account env vars, and deprecate this one (without removing it)?

Just realising that my problem was related, but different. Specifically in my case I was passing a populated ClientOpts (containing AuthInfo) to clientconfig.AuthOptions(). In this case AuthOptions tries to fetch a cloud from the environment, and only uses the explicitly passed AuthInfo if it doesn't find one:

// If a cloud name was determined, try to look it up in clouds.yaml.
if cloudName != "" {
// Get the requested cloud.
var err error
cloud, err = GetCloudFromYAML(opts)
if err != nil {
return nil, err
}
}
// If cloud.AuthInfo is nil, then no cloud was specified.
if cloud.AuthInfo == nil {
// If opts.AuthInfo is not nil, then try using the auth settings from it.
if opts.AuthInfo != nil {
cloud.AuthInfo = opts.AuthInfo
}
// If cloud.AuthInfo is still nil, then set it to an empty Auth struct
// and rely on environment variables to do the authentication.
if cloud.AuthInfo == nil {
cloud.AuthInfo = new(AuthInfo)
}
}

The behaviour I expect is the opposite of this. i.e. If AuthInfo was explicitly passed to the function we use it directly, and if it wasn't we look for a cloud from the environment. So don't remove the (very useful) ability to fetch a cloud from the environment, it just shouldn't override explicitly passed AuthInfo.

Some background on this file: The bulk of the work was done in April/May 2018. Unfortunately my code comments link to the "master" branch, which wasn't good for posterity, but this would have been the approximate commit that I based clientconfig/requests.go off of:

https://github.com/openstack/os-client-config/blob/64b28d42eda595a6fb4ee8b46d93cd61e612aae1/os_client_config/config.py

It looks like I either missed or disagreed with the way OS_CLOUD is handled: if the environment variable is set and if there's a matching entry in clouds.yaml, then an error is returned. So technically this file should do the same, though I'm surprised at not being able to use OS_CLOUD to choose the entry from clouds.yaml.

To confirm though: all other OS_* variables should have priority over any other value passed in. Unless I'm mistaken, this is how the Python library works.

It looks like OS_CLOUD should be handled as a special case, though.

Some background on this file: The bulk of the work was done in April/May 2018. Unfortunately my code comments link to the "master" branch, which wasn't good for posterity, but this would have been the approximate commit that I based clientconfig/requests.go off of:

https://github.com/openstack/os-client-config/blob/64b28d42eda595a6fb4ee8b46d93cd61e612aae1/os_client_config/config.py

It looks like I either missed or disagreed with the way OS_CLOUD is handled: if the environment variable is set and if there's a matching entry in clouds.yaml, then an error is returned. So technically this file should do the same, though I'm surprised at not being able to use OS_CLOUD to choose the entry from clouds.yaml.

To confirm though: all other OS_* variables should have priority over any other value passed in. Unless I'm mistaken, this is how the Python library works.

It looks like OS_CLOUD should be handled as a special case, though.

I wonder if @stephenfin could comment on the behaviour of the python library. Is there an analogous case?

Unfortunately the answer is "it's complicated". openstackclient is a consumer of openstack so it's a good case study. Consider the following clouds.yaml:

clouds:                                                                                                                                                                                                            
    devstack:                                                                                                                                                                                                      
        auth:                                                                                                                                                                                                      
            auth_url: http://10.0.108.84/identity                                                                                                                                                                  
            password: password                                                                                                                                                                                     
            project_domain_id: default                                                                                                                                                                             
            project_name: demo                                                                                                                                                                                     
            user_domain_id: default                                                                                                                                                                                
            username: demo                                                                                                                                                                                         
        identity_api_version: '3'                                                                                                                                                                                  
        region_name: RegionOne                                                                                                                                                                                     
        volume_api_version: '3'                                                                                                                                                                                    
    devstack-admin:                                                                                                                                                                                                
        auth:                                                                                                                                                                                                      
            auth_url: http://10.0.108.84/identity                                                                                                                                                                  
            password: password                                                                                                                                                                                     
            project_domain_id: default                                                                                                                                                                             
            project_name: admin                                                                                                                                                                                    
            user_domain_id: default                                                                                                                                                                                
            username: admin                                                                                                                                                                                        
        identity_api_version: '3'                                                                                                                                                                                  
        region_name: RegionOne
        volume_api_version: '3'

Firstly, if you have an environment variable and an explicit option (e.g. OS_CLOUD and --os-cloud) then the latter takes priority:

# using environment variable for admin cloud credentials
$ OS_CLOUD=devstack-admin openstack compute service list
+----+----------------+--------------------------------+----------+---------+-------+----------------------------+
| ID | Binary         | Host                           | Zone     | Status  | State | Updated At                 |
+----+----------------+--------------------------------+----------+---------+-------+----------------------------+
|  2 | nova-scheduler | stephenfin-devstack-controller | internal | enabled | up    | 2021-11-30T17:25:48.000000 |
|  5 | nova-conductor | stephenfin-devstack-controller | internal | enabled | up    | 2021-11-30T17:25:48.000000 |
|  1 | nova-conductor | stephenfin-devstack-controller | internal | enabled | up    | 2021-11-30T17:25:52.000000 |
|  3 | nova-compute   | stephenfin-devstack-controller | nova     | enabled | up    | 2021-11-30T17:25:48.000000 |
|  4 | nova-compute   | stephenfin-devstack-compute    | nova     | enabled | up    | 2021-11-30T17:25:50.000000 |
+----+----------------+--------------------------------+----------+---------+-------+----------------------------+

# using explicit flag for admin cloud credentials
$ openstack --os-cloud=devstack-admin compute service list
+----+----------------+--------------------------------+----------+---------+-------+----------------------------+
| ID | Binary         | Host                           | Zone     | Status  | State | Updated At                 |
+----+----------------+--------------------------------+----------+---------+-------+----------------------------+
|  2 | nova-scheduler | stephenfin-devstack-controller | internal | enabled | up    | 2021-11-30T17:25:48.000000 |
|  5 | nova-conductor | stephenfin-devstack-controller | internal | enabled | up    | 2021-11-30T17:25:48.000000 |
|  1 | nova-conductor | stephenfin-devstack-controller | internal | enabled | up    | 2021-11-30T17:25:52.000000 |
|  3 | nova-compute   | stephenfin-devstack-controller | nova     | enabled | up    | 2021-11-30T17:25:48.000000 |
|  4 | nova-compute   | stephenfin-devstack-compute    | nova     | enabled | up    | 2021-11-30T17:25:50.000000 |
+----+----------------+--------------------------------+----------+---------+-------+----------------------------+

# using explicit flag for non-admin cloud credentials
$ openstack --os-cloud=devstack compute service list
Policy doesn't allow os_compute_api:os-services:list to be performed. (HTTP 403) (Request-ID: req-835a0142-e3d2-4024-abd7-012e1ed799b6)

# using explicit flag for non-admin cloud credentials and environment variable for admin cloud credentials
$ OS_CLOUD=devstack-admin openstack --os-cloud=devstack compute service list
Policy doesn't allow os_compute_api:os-services:list to be performed. (HTTP 403) (Request-ID: req-17bddec0-d078-4780-acd8-ca7ccca89db9)

However, in all cases, what's provided via environment variable or an explicit config option takes priority over whats in cloud.yaml itself. For example:

# using explicit flag for non-admin cloud credentials but with environment variable for other knobs
$ OS_USERNAME=admin OS_PROJECT_NAME=admin openstack --os-cloud=devstack compute service list
+----+----------------+--------------------------------+----------+---------+-------+----------------------------+
| ID | Binary         | Host                           | Zone     | Status  | State | Updated At                 |
+----+----------------+--------------------------------+----------+---------+-------+----------------------------+
|  2 | nova-scheduler | stephenfin-devstack-controller | internal | enabled | up    | 2021-11-30T17:29:49.000000 |
|  5 | nova-conductor | stephenfin-devstack-controller | internal | enabled | up    | 2021-11-30T17:29:48.000000 |
|  1 | nova-conductor | stephenfin-devstack-controller | internal | enabled | up    | 2021-11-30T17:29:44.000000 |
|  3 | nova-compute   | stephenfin-devstack-controller | nova     | enabled | up    | 2021-11-30T17:29:48.000000 |
|  4 | nova-compute   | stephenfin-devstack-compute    | nova     | enabled | up    | 2021-11-30T17:29:40.000000 |
+----+----------------+--------------------------------+----------+---------+-------+----------------------------+

But then, once again, for individual fields, the explicit config option takes priority:

$ OS_USERNAME=admin OS_PROJECT_NAME=admin openstack --os-cloud=devstack --os-username demo --os-project-name demo compute service list
Policy doesn't allow os_compute_api:os-services:list to be performed. (HTTP 403) (Request-ID: req-e927278b-1da3-43b4-b818-54e0081e24f7)

EDIT So to be clear, I'm thinking that, given an entire explicit cloud config, we should ignore OS_CLOUD. Similarly, given an explicit user config, then we should ignore OS_USERNAME (asssuming that makes sense in the gophercloud context). However, I'm not sure what we'd do about OS_USERNAME if we provide a cloud config sourced from some clouds.yaml file...

Using openstacksdk directly results in a similar behavior, though it seems to wholly ignore the environment variables if an explicit cloud configuration is passed. Consider the following script:

#!/usr/bin/env python3
import openstack

# Initialize and turn on debug logging
openstack.enable_logging(debug=False)

# Initialize connection
conn = openstack.connect(cloud='devstack-admin')

services = conn.compute.services()
print([s.binary for s in services])

We've hardcoded the cloud configuration. Attempting to override that via environment variables makes no difference:

$ ./test.py 
['nova-scheduler', 'nova-conductor', 'nova-conductor', 'nova-compute', 'nova-compute']

$ OS_CLOUD=devstack ./test.py 
['nova-scheduler', 'nova-conductor', 'nova-conductor', 'nova-compute', 'nova-compute']

If you remove the explicit cloud configuration from the script then OS_CLOUD is picked up:

$ OS_CLOUD=devstack-admin python test.py 
['nova-scheduler', 'nova-conductor', 'nova-conductor', 'nova-compute', 'nova-compute']

$ OS_CLOUD=devstack python test.py 
Traceback (most recent call last):
  File "test.py", line 12, in <module>
    print([s.binary for s in services])
  File "test.py", line 12, in <listcomp>
    print([s.binary for s in services])
  File ".../openstacksdk/openstack/resource.py", line 1775, in list
    exceptions.raise_from_response(response)
  File ".../openstacksdk/openstack/exceptions.py", line 238, in raise_from_response
    http_status=http_status, request_id=request_id
openstack.exceptions.HttpException: HttpException: 403: Client Error for url: http://10.0.108.84/compute/v2.1/os-services, Policy doesn't allow os_compute_api:os-services:list to be performed.

However, none of the other OS_* environment options we're using are picked up. This is presumably because openstacksdk doesn't use os_client_config under the hood.

$ OS_CLOUD=devstack OS_USERNAME=admin OS_PROJECT_NAME=admin python test.py 
Traceback (most recent call last):
  File "test.py", line 12, in <module>
    print([s.binary for s in services])
  File "test.py", line 12, in <listcomp>
    print([s.binary for s in services])
  File ".../openstacksdk/openstack/resource.py", line 1775, in list
    exceptions.raise_from_response(response)
  File ".../openstacksdk/openstack/exceptions.py", line 238, in raise_from_response
    http_status=http_status, request_id=request_id
openstack.exceptions.HttpException: HttpException: 403: Client Error for url: http://10.0.108.84/compute/v2.1/os-services, Policy doesn't allow os_compute_api:os-services:list to be performed.

This looks like the behavior we want?

oops - looks like I was mixing up OS_CLOUD and OS_CLOUD_NAME. ๐Ÿคฆ

@stephenfin's input is valuable here and I think correctly determines that both @az-blip and @mdbooth's expected behaviour should be the norm when selecting a cloud or passing in explicit options.

But I believe that all other behaviour should stay the same and is already in place: if an explicit value is not being passed in, then an environment variable can be used, which should override a clouds.yaml setting.

However, none of the other OS_* environment options we're using are picked up. This is presumably because openstacksdk doesn't use os_client_config under the hood.
This looks like the behavior we want?

hm... I don't think so. The code in this repo should be thought of as the os_client_config equivalent. The auth code in the "core" gophercloud/gophercloud repo can probably be thought of as the openstacksdk equivalent - or some variation of that.

However, none of the other OS_* environment options we're using are picked up. This is presumably because openstacksdk doesn't use os_client_config under the hood.
This looks like the behavior we want?

hm... I don't think so. The code in this repo should be thought of as the os_client_config equivalent. The auth code in the "core" gophercloud/gophercloud repo can probably be thought of as the openstacksdk equivalent - or some variation of that.

Sorry, the "this looks the behavior we want" statement was referring to OS_CLOUD being ignored if explicit cloud configuration was provided. I'm as yet undecided what the behavior should be when e.g. OS_USERNAME overrides what's in clouds.{cloud}.auth.username in your clouds.yaml. It's more obvious for a CLI. Less so for a script.

Also, this isn't really relevant but it seems nothing uses os_client_config now. The whole thing has been replaced by the openstack.config module, as the docs indicate

hm... I don't think so. The code in this repo should be thought of as the os_client_config equivalent. The auth code in the "core" gophercloud/gophercloud repo can probably be thought of as the openstacksdk equivalent - or some variation of that.

Actually, I'm going to have to disagree with this (sorry!) because it's not what os_client_config seems to be doing in reality ๐Ÿ˜„ This is a bit long so ๐Ÿป with me.

Here's the code that openstackclient is using (via osc-lib) to allow configuration of the cloud via config options and environment variables, in addition to clouds.yaml. I've already shown above that both environment variables and CLI options override things in clouds.yaml for openstackclient commands (except for OS_CLOUD), however, we can also demonstrate that CLI options override the environment variables:

$ OS_USERNAME=admin openstack \
    --os-auth-url http://10.0.108.84/identity --os-project=demo \
    --os-username demo --os-password password \
    --os-user-domain-id default --os-project-domain-id default \
    --os-project-name demo compute service list
Policy doesn't allow os_compute_api:os-services:list to be performed. (HTTP 403) (Request-ID: req-4b4df6e4-0628-48ae-9169-d8745cae0980)

# dropping the '--os-username' option
$ OS_USERNAME=admin openstack \
    --os-auth-url http://10.0.108.84/identity --os-project=demo \
    --os-password password \
    --os-user-domain-id default --os-project-domain-id default \
    --os-project-name demo compute service list
+----+----------------+--------------------------------+----------+---------+-------+----------------------------+
| ID | Binary         | Host                           | Zone     | Status  | State | Updated At                 |
+----+----------------+--------------------------------+----------+---------+-------+----------------------------+
|  1 | nova-conductor | stephenfin-devstack-controller | internal | enabled | up    | 2021-11-30T18:53:16.000000 |
|  3 | nova-compute   | stephenfin-devstack-controller | nova     | enabled | up    | 2021-11-30T18:53:18.000000 |
|  4 | nova-compute   | stephenfin-devstack-compute    | nova     | enabled | up    | 2021-11-30T18:53:20.000000 |
|  2 | nova-scheduler | stephenfin-devstack-controller | internal | enabled | up    | 2021-11-30T18:53:20.000000 |
|  5 | nova-conductor | stephenfin-devstack-controller | internal | enabled | up    | 2021-11-30T18:53:20.000000 |
+----+----------------+--------------------------------+----------+---------+-------+----------------------------+

So the precedence for openstackclient is CLI options > environment variables > clouds.yaml. However, what you probably don't know though is that openstackclient (again, via osc-lib) is kind of reinventing the wheel here. Both os_client_config and openstack.config will handle environment variable-based configuration for us just fine but they do not munge them into the data from clouds.yaml. Take the following additional script, which I'm calling test2.py:

import os_client_config.config

cc = os_client_config.config.OpenStackConfig()
print(cc.get_cloud_names())

Using my clouds.yaml from earlier, I run this and get the following:

$ python test2.py 
['devstack', 'devstack-admin']

However, if I now configure environment variables then I see a new "cloud" called envvars (I can override this name with OS_CLOUD_NAME, fwiw):

$ OS_AUTH_URL=http://10.0.108.84/identity OS_PROJECT_NAME=admin OS_USERNAME=admin OS_PASSWORD=password OS_USER_DOMAIN_ID=default OS_PROJECT_DOMAIN_ID=default python test2.py
['devstack', 'devstack-admin', 'envvars']

You can see the code that does this is openstacksdk here. I'm sure os_client_config has something similar or simply wraps this, since I've tried with both and see the same behavior.

Going further with this, if I used the test.py script from earlier with all these environment options, I see that these environment options are respected:

$ OS_AUTH_URL=http://10.0.108.84/identity OS_PROJECT_NAME=admin OS_USERNAME=admin OS_PASSWORD=password OS_USER_DOMAIN_ID=default OS_PROJECT_DOMAIN_ID=default python test.py
['nova-scheduler', 'nova-conductor', 'nova-conductor', 'nova-compute', 'nova-compute']

However, the second I add OS_CLOUD to the mix, that stuff is ignored:

$ OS_CLOUD=devstack OS_USERNAME=admin OS_PROJECT_NAME=admin OS_AUTH_URL=http://10.0.108.84/identity OS_PASSWORD=password OS_USER_DOMAIN_ID=default OS_PROJECT_DOMAIN_ID=default OS_PROJECT_NAME=demo python test.py 
Traceback (most recent call last):
  File "test.py", line 12, in <module>
    print([s.binary for s in services])
  File "test.py", line 12, in <listcomp>
    print([s.binary for s in services])
  File ".../openstacksdk/openstack/resource.py", line 1775, in list
    exceptions.raise_from_response(response)
  File ".../openstacksdk/openstack/exceptions.py", line 238, in raise_from_response
    http_status=http_status, request_id=request_id
openstack.exceptions.HttpException: HttpException: 403: Client Error for url: http://10.0.108.84/compute/v2.1/os-services, Policy doesn't allow os_compute_api:os-services:list to be performed.

And again, if I pass an explicit cloud name to openstack.connect (for the openstacksdk variant) then all config options are ignored. So openstacksdk and os_client_config have the following priority:

  1. If the user requested an explicit cloud via a parameter then use that, otherwise...
  2. If the user requested an explicit cloud via the OS_CLOUD environment option then use that, otherwise...
  3. If the user configured their cloud via misc. OS_* environment options then use these, otherwise...
  4. ๐Ÿ’€

This seems like the way to go, IMO. What OSC is doing makes sense for them, but they're a CLI. If someone wrote a CLI around gophercloud, it would probably replicate that. However, I don't think we should be munging as a matter of course.

Thoughts?

This all makes sense to me and I could be misusing library names since 1) it's been so long and 2) I don't have as much of an in-depth understanding as the actual OpenStack developers.

Basically, the original goal of this clientconfig package in this repo was to be a pretty good solution to use in user-facing applications, whether command-line or something like Terraform (which also supports environment variable processing on its own. For reference, here's how Terraform is using clientconfig). So comparing it to osc or some kind of command-line tool should be appropriate. In that way, short of the current OS_CLOUD behaviour, I think it works. I could be wrong, though.

With all of this in mind, I'm in no way going to step in the way of anyone changing this. I was only providing some historical context to the original intent of this package ๐Ÿ™‚

For reference, in my specific use case I need to use 2 different sets of cloud credentials in the same application, hence environment variables are not an option at all as they can only reference one. The problem I had was that, because an environment variable was set, the values I carefully and explicitly passed to AuthOptions() were ignored. The workaround I'm currently going for is to explicitly unset OS_CLOUD, otherwise it's not possible to do what I need to do.

My 2ยข
Gophercloud being a library, and not a tool, I think we should favour simple over easy.
I think it's great that Gophercloud exposes a function that mimics the complicated algorithm of the OpenStack Python reference library.

However, the individual fromYAML or fromEnv or fromCliArgs would be less confusing (to me) if they did exactly one thing.

@mdbooth this is the definitive workaround for your case: #164 (comment)

If it's not definitive, please LMK because we're using that in OpenShift