libjuju 2.6.1 breaks juju.model.Model.get_config() against juju 2.6.6 model

[afreiberger]

The following code works against juju==2.6.0, but just started failing today against juju==2.6.1.

It appears that the code that validates ConfigValues is now expecting value to be either a dict or a set, but does not allow for values to be strings.

I've reverted my testing environment to use juju==2.6.0 as a workaround.

Here is my testing traceback:

tests/functional/test_deploy.py:51: in get_http_proxy
    config = await model.get_config()
.tox/functional/lib/python3.6/site-packages/juju/model.py:1575: in get_config
    config[key] = ConfigValue.from_json(value)
.tox/functional/lib/python3.6/site-packages/juju/client/facade.py:614: in from_json
    return cls(**d)

---

self = <[AttributeError("'ConfigValue' object has no attribute 'source'") raised in repr()] ConfigValue object at 0x7f43eefd5f28>, source = 'default', value = '', unknown_fields = {}
source_ = 'default', value_ = ''

    def __init__(self, source=None, value=None, **unknown_fields):
        '''
        source : str
        value : typing.Mapping[str, typing.Any]
        '''
        source_ = source
        value_ = value
    
        # Validate arguments against known Juju API types.
        if source_ is not None and not isinstance(source_, (bytes, str)):
            raise Exception("Expected source_ to be a str, received: {}".format(type(source_)))
    
        if value_ is not None and not isinstance(value_, (dict, set)):
>           raise Exception("Expected value_ to be a Mapping, received: {}".format(type(value_)))
E           Exception: Expected value_ to be a Mapping, received: <class 'str'>

.tox/functional/lib/python3.6/site-packages/juju/client/_definitions.py:5194: Exception

$ juju status -m controller
Model       Controller  Cloud/Region         Version  SLA          Timestamp
controller  lxd         localhost/localhost  2.6.6    unsupported  19:04:21Z

Machine  State    DNS         Inst id        Series  AZ  Message
0        started  10.0.8.223  juju-fef846-0  bionic      Running

drew@ewah:/var/lib/lxd/containers$ juju status -m test|head
Model  Controller  Cloud/Region         Version  SLA          Timestamp
test   lxd         localhost/localhost  2.6.6    unsupported  19:04:30Z

App            Version  Status   Scale  Charm   Store       Rev  OS      Notes
fce-xenial     18.04    active       1  ubuntu  jujucharms   12  ubuntu  
maas-xenial    16.04    active       3  ubuntu  jujucharms   12  ubuntu  
nagios-bionic           active       1  nagios  jujucharms   33  ubuntu  
nagios-xenial           active       1  nagios  jujucharms   33  ubuntu  
nrpe-bionic             waiting      0  nrpe    jujucharms   58  ubuntu  
nrpe-xenial             waiting      0  nrpe    jujucharms   58  ubuntu  
$ juju version
2.6.6-bionic-amd64
$ which juju
/snap/bin/juju
$ snap list |grep juju
juju    2.6.6     8594  stable    canonical*  classic
$ juju model-config --format yaml
agent-metadata-url:
  value: ""
  source: default
agent-stream:
  value: released
  source: default
agent-version:
  value: 2.6.6
  source: model
apt-ftp-proxy:
  value: ""
  source: default
apt-http-proxy:
  value: http://squid.internal:3128
  source: model
apt-https-proxy:
  value: http://squid.internal:3128
  source: model
apt-mirror:
  value: ""
  source: default
apt-no-proxy:
  value: localhost,127.0.0.1,ppa.launchpad.net,launchpad.net
  source: model
automatically-retry-hooks:
  value: true
  source: default
backup-dir:
  value: ""
  source: default
cloudinit-userdata:
  value: ""
  source: default
container-image-metadata-url:
  value: ""
  source: default
container-image-stream:
  value: released
  source: default
container-inherit-properties:
  value: ""
  source: default
container-networking-method:
  value: local
  source: model
default-series:
  value: bionic
  source: default
development:
  value: false
  source: default
disable-network-management:
  value: false
  source: default
egress-subnets:
  value: ""
  source: default
enable-os-refresh-update:
  value: true
  source: default
enable-os-upgrade:
  value: true
  source: default
fan-config:
  value: ""
  source: default
firewall-mode:
  value: instance
  source: default
ftp-proxy:
  value: ""
  source: default
http-proxy:
  value: ""
  source: default
https-proxy:
  value: ""
  source: default
ignore-machine-addresses:
  value: false
  source: default
image-metadata-url:
  value: ""
  source: default
image-stream:
  value: released
  source: default
juju-ftp-proxy:
  value: ""
  source: default
juju-http-proxy:
  value: http://squid.internal:3128
  source: model
juju-https-proxy:
  value: http://squid.internal:3128
  source: model
juju-no-proxy:
  value: localhost,127.0.0.1,0.0.0.0,ppa.launchpad.net,launchpad.net,10.0.8.0/24
  source: model
logforward-enabled:
  value: false
  source: default
logging-config:
  value: <root>=DEBUG;unit=DEBUG
  source: model
max-action-results-age:
  value: 336h
  source: default
max-action-results-size:
  value: 5G
  source: default
max-status-history-age:
  value: 336h
  source: default
max-status-history-size:
  value: 5G
  source: default
net-bond-reconfigure-delay:
  value: 17
  source: default
no-proxy:
  value: 127.0.0.1,localhost,::1
  source: default
provisioner-harvest-mode:
  value: destroyed
  source: default
proxy-ssh:
  value: false
  source: default
resource-tags:
  value: {}
  source: model
snap-http-proxy:
  value: http://squid.internal:3128
  source: model
snap-https-proxy:
  value: http://squid.internal:3128
  source: model
snap-store-assertions:
  value: ""
  source: default
snap-store-proxy:
  value: ""
  source: default
ssl-hostname-verification:
  value: true
  source: default
storage-default-filesystem-source:
  value: lxd
  source: model
test-mode:
  value: false
  source: default
transmit-vendor-metrics:
  value: true
  source: default
update-status-hook-interval:
  value: 5m
  source: default

[mitechie]

Thanks for this, I'm trying to understand the code you're running/using the library with. Is it possible to get at the test details in the traceback. My reading of things is that the config is looking for the entire mapped config vs a single value. I'm wondering if this is a case of something that used to either say it was working, but not really, or something that worked accidentally but now that we're more strict should be tweaked going forward.

Thanks for the additional details!

[mitechie]

Of course the issue is that the code you've got here seems to just be reading config and not setting anything...

[mitechie]

Ok, just to note what the code is doing in L51

async def get_http_proxy(model):
    config = await model.get_config()
    if "juju-http-proxy" in config:
        return config["juju-http-proxy"].value
    else:
        return str("")

So it's just doing model.get_config() causing this.

[afreiberger]

Yes, exactly. Sorry if that was unclear from the original note. I've edited to improve formatting of the original issue.

I believe the issue is a missing allowed instance-type for the ConfigValue object's 'value_' of type 'str' here:

    if value_ is not None and not isinstance(value_, (dict, set)):

[SimonRichardson]

@afreiberger it turns out there was a bug in the way we generated the facade and definitions code. The problem was that certain parameter fields have the type of interface{}, which then got translated into an argument of map[string]interface{}, which then became a dict. The fix was to correctly handle interface{} as typing.Any and then push that through the code so we didn't end up validating the type, as we don't know what the type is.

Thanks for the detailed report, it helped diagnosing the issue.

[ovod88]

Hello.
The same behavior is observed with juju==2.6.1 and model 2.6.4:

File "/usr/local/lib/python3.7/site-packages/juju/model.py", line 1575, in get_config
config[key] = ConfigValue.from_json(value)
File "/usr/local/lib/python3.7/site-packages/juju/client/facade.py", line 614, in from_json
return cls(**d)
File "/usr/local/lib/python3.7/site-packages/juju/client/definitions.py", line 5194, in __init_
raise Exception("Expected value_ to be a Mapping, received: {}".format(type(value_)))
Exception: Expected value_ to be a Mapping, received: <class 'str'>

[SimonRichardson]

@afreiberger, @ovod88 2.6.2 has been released with the fixes for the ConfigValue issue, where the value type was being wrongly interpreted.

Let me know if you have any other issues.